-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rendering: Incorrect font variant being selected #770
Comments
I'm experiencing the exact same thing, and can confirm the issue seems like a regression with commit 3f153be. I built mpv with mpv-build on libass commit 0a63b95 and 3f153be, and fontselect exhibits this incorrect behaviour starting from the latter. And I also do have fontconfig assigning 'sans-serif' to Noto Sans Regular.
|
The change in 3f153be was made because VSFilter does not consider width when selecting a font. If an ASS file explicitly requested If you want to default to a specific width in fontconfig, I would expect you can do that in your configuration? |
So before 3f153be an ASS file asking for 'sans-serif' with no further specificity would get the Regular variant of the default sans-serif font (which is sane behaviour; it is exactly what anybody would be expecting), but after 3f153be we now get the Condensed variant of the default sans-serif font. Is this the intended behaviour of commit 3f153be? As far as I can tell, 3f153be is essentially a revert of 09edb29. It seems like before font width support (09edb29) there was a "HACK" 09edb29#diff-f279ae24a74ca3d1d177c1052cca831a5fb0ad8a743f0770759c35adcea0c830L101 which mitigated this exact issue we are facing now ("HACK: get the last family name. that fixes fonts like Arial Narrow in some versions"). May I suggest two possible ways to move forward:
Either option should, I believe, allow fontselect to serve the correct font variant. |
Hmm… With the entire Noto Sans meta-family installed, I can repro this:
So, I think we have 2 bugs here:
|
So it sounds like the first is #315 “we should pass Fontconfig fonts through our own name extraction code” (the problem being that we need lazy font matching for this not to delay startup by an eternity), while the second has been suggested in https://bugzilla.redhat.com/show_bug.cgi?id=2262410. |
While I understand the issue this is causing in mpv, it’s also worth noting that ASS files asking for |
Note, the old width selection code in practice never actually worked sensibly either. Since ASS has no way to request a width, it just always penalised fonts whose width wasn’t 100, even when explicitly requesting e.g.
yeah, Fontconfig “Makes no distinction between the various kinds of family name” https://github.com/libass/libass/wiki/Fonts-across-platforms#fontconfig To make sure, i confirmed this doesn’t affect cases where Noto Sans’ regular variant is attached and “Noto Sans Condensed” installed on the system. I.e. this only affects font-fallback which is already “best-effort, no guarantees” anyway, buuut this case is probably quite common and i agree current behaviour isn’t intuitive or desirable. It seems to get this right (as far as possible for fallbacks), we need to
Lazy loading and font source separation are both things we already planned anyway, but they might take a while and have a fair chance of initially regressing some other edge cases. |
I don’t see how this is relevant. |
We could sort the full list of Fontconfig fonts by |
it indeed isn’t for this issue, sorry |
The full system font list is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Given a comprehensive solution will take a while, sorting the list by how normal each font’s width is, seems like a low risk workaround at least alleviating the particularly prominent recent regression while we work on a proper solution. Other font providers are not affected by the issue, making sorting just fontconfig’s list preferable over reintroducing a bogus width penalty. To ensure sorting results are stable and doesn't introduce flaky results a custom mergesort is used instead of standard qsort. Fixes: libass#770
The full system font list is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Given a comprehensive solution will take a while, sorting the list by how normal each font’s width is, seems like a low risk workaround at least alleviating the particularly prominent recent regression while we work on a proper solution. Other font providers are not affected by the issue, making sorting just fontconfig’s list preferable over reintroducing a bogus width penalty. To ensure sorting results are stable and doesn't introduce flaky results a custom mergesort is used instead of standard qsort. Fixes: libass#770
The full system font list is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Given a comprehensive solution will take a while, sorting the list by how normal each font’s width is, seems like a low risk workaround at least alleviating the particularly prominent recent regression while we work on a proper solution. Other font providers are not affected by the issue, making sorting just fontconfig’s list preferable over reintroducing a bogus width penalty. To ensure sorting results are stable and doesn't introduce flaky results a custom mergesort is used instead of standard qsort. Fixes: libass#770
The full system font list is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Given a comprehensive solution will take a while, sorting the list by how normal each font’s width is, seems like a low risk workaround at least alleviating the particularly prominent recent regression while we work on a proper solution. Other font providers are not affected by the issue, making sorting just fontconfig’s list preferable over reintroducing a bogus width penalty. To ensure sorting results are stable and doesn't introduce flaky results a custom mergesort is used instead of standard qsort. Fixes: libass#770
The full system font list we used before is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Given a comprehensive solution will take a while, this patch provides an interim workaround alleviating at least the recent, particularly prominent regression giving us time to work on a proper solution. Fontconfig’s default pattern prefers normal widths, weight, etc matching exactly what we want here anyway. Notably this also adds a language parameter. Given its explicit removal for fallbacks it is unclear whether we really want this, but compared to an unspecified, potentially random order before this shouldn't hurt.
The full system font list we used before is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Given a comprehensive solution will take a while, this patch provides an interim workaround alleviating at least the recent, particularly prominent regression giving us time to work on a proper solution. Fontconfig’s default pattern prefers normal widths, weight, etc matching exactly what we want here anyway. Notably this also adds a language parameter. Given its explicit removal for fallbacks it is unclear whether we really want this, but compared to an unspecified, potentially random order before this shouldn't hurt.
The full system font list we used before is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Given a comprehensive solution will take a while, this patch provides an interim workaround alleviating at least the recent, particularly prominent regression giving us time to work on a proper solution. Fontconfig’s default pattern prefers normal widths, weight, etc matching exactly what we want here anyway. Notably this also adds a language parameter. Given its explicit removal for fallbacks it is unclear whether we really want this, but compared to an unspecified, potentially random order before this shouldn't hurt.
The full system font list we used before is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Given a comprehensive solution will take a while, this patch provides an interim workaround alleviating at least the recent, particularly prominent regression giving us time to work on a proper solution. Fontconfig’s default pattern prefers normal widths, weight, etc matching exactly what we want here anyway. Notably this also adds a language parameter. Given its explicit removal for fallbacks it is unclear whether we really want this, but compared to an unspecified, potentially random order before this shouldn't hurt.
The full system font list we used before is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Given a comprehensive solution will take a while, this patch provides an interim workaround alleviating at least the recent, particularly prominent regression giving us time to work on a proper solution. Fontconfig’s default pattern prefers normal widths, weight, etc matching exactly what we want here anyway. Notably this also adds a language parameter. Given its explicit removal for fallbacks it is unclear whether we really want this, but compared to an unspecified, potentially random order before this shouldn't hurt.
I appreciate the effort to maintain compatibility with VSFilter. However, I'm curious about libass's approach to establishing its own identity. While emulating VSFilter is important, it might be beneficial to consider how to address extensions of the format and behavioral changes moving forward. The behavior introduced in version 0.13.0 (9 years ago) with 09edb29 was removed just a week before release without any deprecation period. This sudden change can be challenging for the library users to adapt to.
In this case, it might be worthwhile to version it in Thank you for considering this feedback. |
The full system font list we used before is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Considering a comprehensive solution will take a while, this patch provides an interim workaround alleviating at least the recent, particularly prominent regression giving us time to work on a proper solution. Fontconfig’s default pattern prefers normal widths, weight, etc matching exactly what we want here anyway. Notably this also adds a language parameter. Given its explicit removal for fallbacks it is unclear whether we really want this, but compared to an unspecified, potentially random order before this shouldn't hurt.
The full system font list we used before is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Considering a comprehensive solution will take a while, this patch provides an interim workaround alleviating at least the recent, particularly prominent regression giving us time to work on a proper solution. Fontconfig’s default pattern prefers normal widths, weight, etc matching exactly what we want here anyway. Notably this also adds a language parameter. Given its explicit removal for fallbacks it is unclear whether we really want this, but compared to an unspecified, potentially random order before this shouldn't hurt.
The full system font list we used before is not guaranteed to be sorted by any meaningful criterion. Additionally fontconfig doesn’t distinguish between different kinds of family name, in particular also returning e.g. "Noto Sans" for "Noto Sans Condensed". Since width is not a criterion in ASS font selection and accordingly was removed from our logic in 3f153be. This means related font family of different widths may end up reporting one overlapping family name and tie on scoring, resulting in whatever was listed first being selected. This leads to issues like libass#770 Notably since nothing ever set the width value for queries, the old logic just always penalised non-regular widths, even if those were explicitly requested by name. In pathological cases this may have lead to mis-selections too. Addressing this and other issues in the current fontconfig provider properly requires at least switching it to lazy loading and possibly even a larger fontselect overhaul as proposed in libass#771. Considering a comprehensive solution will take a while, this patch provides an interim workaround alleviating at least the recent, particularly prominent regression giving us time to work on a proper solution. Fontconfig’s default pattern prefers normal widths, weight, etc matching exactly what we want here anyway. Notably this also adds a language parameter. Given its explicit removal for fallbacks it is unclear whether we really want this, but compared to an unspecified, potentially random order before this shouldn't hurt.
A workaround should now be in place for Fontconfig; please check everything works as expected and be on the lookout for other font selection oddities. If everything’s alright (or at least no worse than before) we’ll probably publish 0.17.3 in a few weeks. @kasper93 this issue was completely orthogonal to format extensions and no feature was removed. It’s just an unintended side-effect, affecting only fontconfig in specific scenarios we didn't catch before release. |
Screenshots
mpv w/libass 0.17.1
![Wolf Children (2012) 1080p x265 HEVC 10bit BluRay Dual Audio AAC 5 1 Prof mkv_snapshot_00:01:00_ 2024 05 22_16 36 24](https://private-user-images.githubusercontent.com/60193883/332953459-a79fea42-d578-42c7-bae0-5f7bc06b669d.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4ODE0NjAsIm5iZiI6MTcxODg4MTE2MCwicGF0aCI6Ii82MDE5Mzg4My8zMzI5NTM0NTktYTc5ZmVhNDItZDU3OC00MmM3LWJhZTAtNWY3YmMwNmI2NjlkLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA2MjAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNjIwVDEwNTkyMFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTUyNjYyMzViZWU4Nzk5Y2I5MzIzYjJiZTI5YmE2ZWQxYzQ1N2MzZDVjMjM1Mjg0YTg1MTNhMWIxNDdhZWY2NzImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.82pEDZ0LvBBWFZ4Og7-JGonhg4CFnh-RRUVwacryp2o)
mpv w/ libass 0.17.2
![Wolf Children (2012) 1080p x265 HEVC 10bit BluRay Dual Audio AAC 5 1 Prof mkv_snapshot_00:01:00_ 2024 05 22_16 37 38](https://private-user-images.githubusercontent.com/60193883/332953218-30966fbf-bd5f-4556-a84d-7b1e8d487db8.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTg4ODE0NjAsIm5iZiI6MTcxODg4MTE2MCwicGF0aCI6Ii82MDE5Mzg4My8zMzI5NTMyMTgtMzA5NjZmYmYtYmQ1Zi00NTU2LWE4NGQtN2IxZThkNDg3ZGI4LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA2MjAlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwNjIwVDEwNTkyMFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTY2ZjRjMWIwY2E0MThlNzVlYmYxZjMwNmI3MTRkMzYyY2UyNjNmMGU5Mzg3MWNlMTQ4MjI4MjJmN2UxYzBkODMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.4cuHOemRO_M37nGVaXhFWY-zVQyC3RZrAKOl5FvF07Y)
Description of the issue
Starting with version 0.17.2, alternative versions of fonts are erroneously being selected. I've configured fontconfig to assign
sans-serif
toNoto Sans
and up until recently this would have selected the Regular variant. Now, the Condensed variant is being selected. I'm not sure if this is an issue with how mpv is invoking libass.libass version
0.17.2
Is it a regression?
Yes. The regression originates with commit 3f153be.
ASS Sample
No response
Special Fonts
The issue doesn't depend on a specific font
System Information
Linux on x86-64
Log
mpv w/ libass 0.17.1
[osd/libass] fontselect: (sans-serif, 400, 0) -> /usr/share/fonts/noto/NotoSans-Regular.ttf, 0, NotoSans-Regular
mpv w/ libass 0.17.2
[osd/libass] fontselect: (sans-serif, 400, 0) -> /usr/share/fonts/noto/NotoSans-Condensed.ttf, 0, NotoSans-Condensed
Additional info
No response
The text was updated successfully, but these errors were encountered: