clsid2 / mpc-hc

Media Player Classic
GNU General Public License v3.0
11.34k stars 497 forks source link

Attached fonts not loaded into libass when external subtitle track selected #2381

Closed astiob closed 10 months ago

astiob commented 11 months ago

When I play an MKV, the fonts used in the ASS and attached to (embedded in) the MKV display correctly in both internal VSFilter and libass.

When I extract the subtitle track and play the same MKV with the external subtitle track, the fonts display correctly in internal VSFilter but are replaced with system defaults in libass mode. Without checking the source code, I’m guessing that attached fonts are only being passed to libass when playing embedded subtitle tracks. They should be passed always, regardless of the selected track, to match MPC-HC’s own internal VSFilter, external VSFilter, XySubFilter and mpv.

clsid2 commented 11 months ago

It loads everything from Fonts subdir.

NBruderman commented 11 months ago

It loads everything from Fonts subdir.

can this be related? https://github.com/clsid2/mpc-hc/blob/develop/src%2FSubtitles%2FLibassContext.cpp#L740

If m_assfontloaded = true, then the entire font loading part is being skipped, which might cause that, as you can see here

https://github.com/clsid2/mpc-hc/blob/develop/src%2FSubtitles%2FLibassContext.cpp#L767-L769

I think right now the fonts are being loaded, but won't go through the logic which applies then on external files

clsid2 commented 11 months ago

That code is for embedded fonts.

CFontInstaller class is used for temporarily installing external fonts. Those are not enumerable, which does not matter for GDI, because we create them directly. But I guess libass needs enumeration, or having them added explicitly?

NBruderman commented 11 months ago

That code is for embedded fonts.

CFontInstaller class is used for temporarily installing external fonts. Those are not enumerable, which does not matter for GDI, because we create them directly. But I guess libass needs enumeration, or having them added explicitly?

Seems like it... I can't tag them, but maybe adipose will have further insight (I'm not a developer, so my knowledge is very limited)

adipose commented 11 months ago

Will take a look soon.

astiob commented 11 months ago

I’m not sure I am supposed to understand the discussion above, but just a few points in case they help clarify anything:

From where I stand, I suspect the code NBruderman linked is, in fact, relevant here, but this is only a guess.

clsid2 commented 11 months ago

I have changed it so that the code is also executed in case of external sub.

Yes, we are using current git head master of libass.

Is ass_add_font required when using freetype?

If so, additional code will be required to pass on the info of external loaded fonts to libass.

Btw, temporary installed fonts "override" system installed fonts. At least in GDI case. We had a bug in the past where a bad embedded font replaced a system font used in the GUI, messing things up.

NBruderman commented 11 months ago

I have changed it so that the code is also executed in case of external sub.

Yes, we are using current git head master of libass.

Is ass_add_font required when using freetype?

If so, additional code will be required to pass on the info of external loaded fonts to libass.

Btw, temporary installed fonts "override" system installed fonts. At least in GDI case. We had a bug in the past where a bad embedded font replaced a system font used in the GUI, messing things up.

@adipose do you think this can solve this? Or is there another "cleaner" way to take care of this bug?

astiob commented 11 months ago

Is ass_add_font required when using freetype?

I’m not sure how FreeType relates to this (it’s used all the same). Did you mean Fontconfig, in the sense of libass’s “system font provider”? If so, yes and even more so. The expected thing is that you call ass_add_font and then it doesn’t matter (from API usage perspective) which of libass’s system font providers is selected. But if you don’t call it and instead install the fonts temporarily in GDI, then they will only work when using libass’s “directwrite” system font provider (which is the default on Windows Vista+).

Btw, temporary installed fonts "override" system installed fonts. At least in GDI case.

If they’re an exact match. But you can have e. g. a Bold font installed system-wide, a Regular version of the same font attached (and temporarily installed) and a subtitle track that requests the font to be bold. GDI will select the system-wide font in this case.

adipose commented 11 months ago

then they will only work when using libass’s “directwrite” system font provider (which is the default on Windows Vista+).

mpc-hc uses ASS_FONTPROVIDER_AUTODETECT, so shouldn't that always be directwrite?

adipose commented 11 months ago

I have changed it so that the code is also executed in case of external sub. Yes, we are using current git head master of libass. Is ass_add_font required when using freetype? If so, additional code will be required to pass on the info of external loaded fonts to libass. Btw, temporary installed fonts "override" system installed fonts. At least in GDI case. We had a bug in the past where a bad embedded font replaced a system font used in the GUI, messing things up.

@adipose do you think this can solve this? Or is there another "cleaner" way to take care of this bug?

Is there an example file I can test?

If I understand correctly, the MKV has embedded fonts, but the subtitle used is external. And the embedded fonts are being ignored in that one case?

I'm not that surprised it doesn't work, because it's a weird scenario to have an external sub that needs internal fonts. But I am a bit surprised as the embedded fonts are auto-loaded during mkv enumeration, I thought.

adipose commented 11 months ago

Is ass_add_font required when using freetype?

I don't think we use freetype in conjunction with libass today. The freetype codepath was used to parse some crazy fonts that GDI couldn't map out, for ISR.

astiob commented 11 months ago

mpc-hc uses ASS_FONTPROVIDER_AUTODETECT, so shouldn't that always be directwrite?

Yes (on Vista and newer, anyway, but I don’t know if you even support XP at all).

I'm not that surprised it doesn't work, because it's a weird scenario to have an external sub that needs internal fonts.

I’m sure I do it more often than most people, but I think it is a decently standard scenario. It happens when you extract an embedded subtitle track to edit something (or download a pre-edited one from someone else who did this, who may or may not be the original author). These are soft subs, after all: one of their selling points is that you can edit them.

adipose commented 11 months ago

mpc-hc uses ASS_FONTPROVIDER_AUTODETECT, so shouldn't that always be directwrite?

Yes (on Vista and newer, anyway, but I don’t know if you even support XP at all).

I'm not that surprised it doesn't work, because it's a weird scenario to have an external sub that needs internal fonts.

I’m sure I do it more often than most people, but I think it is a decently standard scenario. It happens when you extract an embedded subtitle track to edit something (or download a pre-edited one from someone else who did this, who may or may not be the original author). These are soft subs, after all: one of their selling points is that you can edit them.

That's what I was speculating was the scenario--it's not invalid but it is an edge case.

astiob commented 11 months ago

If there’s a binary that I can use, I can trivially run a test on my end, but you can replicate this by simply extracting (e. g. using mkvextract tracks or ffmpeg) an ASS track from pretty much any subtitled MKV.

adipose commented 11 months ago

I think @clsid2 's fix will actually work.

clsid2 commented 11 months ago

Embedded fonts are now always end to libass. But external font sub is not yet send to libass. But it is weird that problem exists, since it is not strictly needed?

adipose commented 11 months ago

Embedded fonts are now always end to libass. But external font sub is not yet send to libass. But it is weird that problem exists, since it is not strictly needed?

Not sure. If I understood correctly, the bug reported here is that an external sub may rely on an internal font, and fail because we didn't load it for the libass context created for the external sub.

That makes perfect sense to me, but maybe I am missing something. It is "weird" for an external sub to rely on an internal font, but the reason for it is easy to understand, if someone wants to override a few misspellings or translations.

If external font is not sent to libass, libass should still find it as long as it was loaded into GDI memory.

clsid2 commented 11 months ago

Embedded fonts are installed by the splitter, similar to how we load external fonts.

astiob is saying that sending the font details to libass is not strictly required. But it does not seem to me that this is how it is working in practice. The ass_add_font use seems to be needed for the embedded fonts. Then I think it must also be needed for the external ones?

adipose commented 11 months ago

Could it be a timing issue? External fonts are loaded before libass, but internal ones are loaded during MKV parsing?

clsid2 commented 11 months ago

That should also happen before libass gets loaded. Otherwise regular ISR would have trouble as well.

astiob commented 11 months ago

libass isn’t by any chance run in a separate process (which is outside the scope of the temporarily installed fonts), is it?

clsid2 commented 11 months ago

No

adipose commented 11 months ago

https://github.com/clsid2/mpc-hc/pull/2409

What I observed is that manually loaded subtitles don't get the pin set. I changed the code to find any pin, as the pin is just used to find the upstream interface holding all the streams (including fonts).

The ass_add_font is adding these fonts manually to the ASS_Library. I'm not sure if loading them into GDI memory manually would make that redundant, but the lack of pin was preventing it from loading it the way the internal subs do.

clsid2 commented 11 months ago

Embedded fonts are installed by the splitter: https://github.com/clsid2/LAVFilters/blob/f672f185cc91ef5a3f8a1d4d02a1af1b1d474b8a/demuxer/Demuxers/LAVFDemuxer.cpp#L837 (similar to what we do in player for external fonts)

Since ass_add_font does seem required, it looks like libass depends on enumeration ability (which isn't available for temporary installed fonts). So maybe something can be improved on libass side as well. Like trying to create a font instance for fonts that could not be enumerated. Once as fallback availability detection method.

For external loaded fonts, we could get list from fontinstaller, and pass it along to libass.

astiob commented 11 months ago

Ah hmm, I’ve just double-checked libass’s own code and indeed I must say I misremembered: we do call EnumFontFamilies, because we need to see all members of a family to be able to select bold/italic correctly. This may change in the future, but it’s not trivial, because currently our system font provider backends don’t know what weight or slant is requested; they have only the name.

It is on the results of that enumeration that we call CreateFontIndirect like ISR/VSFilter does it.

adipose commented 11 months ago

Incidentally, the font enumeration process in libass contains code nearly identical to mpc-hc code we recently disabled. What the code does is avoid fonts that don't match the expected face name. But when the font has two possible facenames, depending on charset, it actually causes a valid font to be wrongly disabled.

I would expect this to have the result that the "bug" we recently fixed in mpc-hc will actually remain unfixed when using libass, assuming it is found through enumeration.

    if (wcsncmp(selected_name, lpelf->elfLogFont.lfFaceName, LF_FACESIZE)) {
        // A different font was selected. This can happen if the requested
        // name is subject to charset-specific font substitution while
        // EnumFont... enumerates additional charsets contained in the font.
        // Alternatively, the font may have disappeared in the meantime.
        // Either way, there's no use looking at this font any further.
        goto cleanup;
    }
astiob commented 11 months ago

In this case, libass is requesting names (via CreateFontIndirect) that it received from GDI in the first place (via EnumerateFonts). This isn't the name specified in the ASS. IIRC the MPC-HC code was trying to compare GDI's preferred name to the ASS name, which may indeed differ.

adipose commented 11 months ago

We had the same issue I think. The sub file calls for a fontname, but when that font is loaded, it doesn't have the same facename as the fontname that was asked for. By choosing to "fail," it actually made the results worse.

Edit: I misread your comment. So the enumeration would always match the facename that is assigned?

astiob commented 11 months ago

Yeah, what I was trying to say is that the MPC-HC code compared the received font to what the ASS had, but that’s not what libass is doing. It’s taking a list from GDI, feeding it back into GDI, and comparing just to avoid some edge cases with aliases and disappearing fonts. It might not even be strictly required but is more a safeguard against accidentally mistaking one font for another or loading half of a family and thinking it’s been loaded in full.

So the enumeration would always match the facename that is assigned?

First, we request an enumeration of all fonts that GDI thinks match what the subtitles asked for. This matches all alternate names that GDI recognizes (but returns its preferred names, plus some aliases), and in particular it includes the font VSFilter’s direct query would select—except apparently in case of fonts temporarily installed into the process, which turn out to be non-enumerable. Then, we take each of these and request the actual font for it, because the enumeration returns only some metadata. But to ensure GDI gives us the same font that it enumerated, we feed it back the exact name, etc. that it itself produced in the enumeration.

clsid2 commented 11 months ago

So what happens if the enumeration does not find the requested font? Does libass conclude that the font is unavailable and switch to default?

Why not just call CreateFontIndirect when enumeration fails. When that also fails, finally switch to default font.

astiob commented 11 months ago

Why not just call CreateFontIndirect when enumeration fails.

Because it needs a weight and a slant style, which we don’t have there.

clsid2 commented 11 months ago

Do with with standard weight and slant? I mean if a font isn't found (at all) during enumeration, but can be created anyway, that suggests that it was provided by the library user. In that case just assume that all required weight/slant variants are available?

astiob commented 11 months ago

Do with with standard weight and slant?

The attached font may be bold and/or italic. And weight isn’t even a binary toggle; it’s an arbitrary 3-figure integer. If the attached font doesn’t match the request exactly, it’ll get a match penalty and there’s no guarantee that GDI will return this font.

I mean if a font isn't found (at all) during enumeration, but can be created anyway

We can’t even determine whether it can be created: CreateFontIndirect always succeeds and returns some font.

In that case just assume that all required weight/slant variants are available?

It’s not about abstract knowledge of what might be available. We need the actual fonts. We don’t just write down what to ask for later—this is the very place where we ask for them. It’s precisely to get all weight/slant variants that we do the enumeration.

At any rate, ass_add_font is the expected API to be called for attached fonts: it doesn’t depend on font provider/platform specifics and it gives libass the fullest information.

adipose commented 11 months ago

Do with with standard weight and slant? I mean if a font isn't found (at all) during enumeration, but can be created anyway, that suggests that it was provided by the library user. In that case just assume that all required weight/slant variants are available?

libass is actually using IDWriteGdiInterop::CreateFontFaceFromHdc to create the font during enumeration. The equivalent for logfonts is CreateFontFromLOGFONT, but unfortunately, it doesn't support fonts installed using AddFontMemResourceEx.

CreateFontIndirect of course works, but as pointed out, it always returns a font. The code I used in mpc-hc to detect the wrong font was not good enough due to different fontnames.

@astiob

We can’t even determine whether it can be created: CreateFontIndirect always succeeds and returns some font.

Yes, but if it returned a font with the same facename you were seeking...would that be worth using somehow? Actually I did not find a way for that to be useful under directwrite...

adipose commented 11 months ago

@astiob

diff --git a/libass/ass_directwrite.c b/libass/ass_directwrite.c
index f8c2a27..f1f50c4 100644
--- a/libass/ass_directwrite.c
+++ b/libass/ass_directwrite.c
@@ -24,6 +24,7 @@
 #include <wchar.h>

 #include "dwrite_c.h"
+#include "windowsx.h"

 #include "ass_directwrite.h"
 #include "ass_utils.h"
@@ -654,6 +655,7 @@ struct font_enum_priv {
     ASS_FontProvider *provider;
     IDWriteGdiInterop *gdi_interop;
     ASS_SharedHDC *shared_hdc;
+    bool found;
 };

 /*
@@ -761,7 +763,7 @@ static int CALLBACK font_enum_proc(const ENUMLOGFONTW *lpelf,
         goto cleanup;

     add_font_face(face, priv->provider, priv->shared_hdc);
-
+    priv->found = true;
 cleanup:
     if (hFont)
         DeleteObject(hFont);
@@ -858,6 +860,7 @@ static void match_fonts(void *priv, ASS_Library *lib,

 #if ASS_WINAPI_DESKTOP
     struct font_enum_priv enum_priv;
+    enum_priv.found = false;

     enum_priv.shared_hdc = calloc(1, sizeof(ASS_SharedHDC));
     if (!enum_priv.shared_hdc)
@@ -913,6 +916,29 @@ static void match_fonts(void *priv, ASS_Library *lib,
     EnumFontFamiliesW(hdc, lf.lfFaceName,
                       (FONTENUMPROCW) font_enum_proc, (LPARAM) &enum_priv);

+    if (!enum_priv.found) {
+      lf.lfCharSet = DEFAULT_CHARSET;
+      lf.lfOutPrecision = OUT_TT_PRECIS;
+      lf.lfClipPrecision = CLIP_DEFAULT_PRECIS;
+      lf.lfQuality = ANTIALIASED_QUALITY;
+      lf.lfPitchAndFamily = DEFAULT_PITCH | FF_DONTCARE;
+      for (int a = 0; a < 4; a++) {
+        if (a & 1) {
+          lf.lfWeight = FW_NORMAL;
+        } else {
+          lf.lfWeight = FW_BOLD;
+        }
+        if (a & 2) {
+          lf.lfItalic = 0;
+        } else {
+          lf.lfItalic = 255;
+        }
+
+        ENUMLOGFONTW elf;
+        memcpy(&elf.elfLogFont, &lf, sizeof(LOGFONTW));
+        font_enum_proc(&elf, 0, TRUETYPE_FONTTYPE, (LPARAM)&enum_priv);
+      }
+    }
     hdc_release(enum_priv.shared_hdc);
 #else
     HRESULT hr;

I added this code to match_fonts, and it works with the in memory fonts. It turns out CreateFontIndirect is the underlying method actually used, just inside of enumeration.

I guess the same question we asked about mpc-hc needs to be asked here: What's the worst that can happen if CreateFontIndirect can't find the font we're looking for? It will choose the "best" option which will typically be Arial, but that's better than no font at all, so why can't we accept that?

Most importantly, if we leave in the the fontname check, it still works for 99% of fonts, just not those with alternate names in different encodings. Again, if the font is loaded to GDI memory, that's surely better than just giving up on the font?

It's up to libass devs of course, but I don't really understand why this couldn't be used as a fallback method.

adipose commented 11 months ago

A further detail, when going through enumeration, it gets called like this for a regular font:

FW_NORMAL FW_BOLD FW_NORMAL + italic=255 FW_BOLD + italic=255

So my code above could be improved by doing those four scenarios, supporting normal, bold, italic and bold italic.

Edit: updated

adipose commented 10 months ago

@astiob , if the fonts are added using ass_add_font, does that mean it won't use GDI to load it?

astiob commented 10 months ago

if the fonts are added using ass_add_font, does that mean it won't use GDI to load it?

Correct. We use platform-independent code to load these fonts. There’s no point in involving GDI in it, and it would be more complicated.

We don’t use GDI to select the font that’s ultimately used, either. We have a platform-independent font selector that emulates what we know of GDI’s logic. It runs on the renderer’s in-memory font database, which is populated with the fonts fed directly to libass using ass_add_font etc. and additionally by system font providers: in most cases dynamically, whenever a font is requested and the database doesn’t contain any matches already. The system font provider’s job in this case is to load all fonts of a given family into the database—and no others. If a family gets loaded partially, this can ruin subsequent font selection decisions because there will be some matches in the database, but they won’t necessarily be the right ones.


As for the proposed code: I appreciate the attempts to help, and while I have some reservations about this code, it might work decently well in practice. At the same time, I must say it’s not clear what the specific goal is.

If it’s just about my earlier comment that I’d like to hear if our behaviour differs from VSFilter’s, I said it before I remembered that we enumerate fonts, and I’m satisfied that the cause of the discrepancy has been identified.

If you want to avoid calling ass_add_font, that ultimately amounts to an abuse of libass API: ass_add_font should be called for attached fonts. It’s true that we try to make system font providers as reliable as possible, but they remain best-effort, no-guarantee products. And a perennial source of bugs and glitches, too. Even right now, there’s at least one open issue about the Windows font provider returning wrong names for some fonts. Only fonts explicitly added to libass are guaranteed to be handled correctly. If nothing else, adding them explicitly ensures that you support more esoteric fonts (than hacks around non-enumerables can guarantee) and that you match other libass-powered players (although this may currently differ from VSFilter—if where we pick an attached font while VSFilter picks a system font—but some people argue our choice is better).

Some potential issues:

The code doesn’t enumerate all weights. As mentioned, weight is a 3-figure integer. GDI-compatible font families usually have only at most two different weights, but even then they’re not necessarily weights 400 and 700. And technically, GDI supports larger families just fine, so there might be real families with more than two different weights, although I haven’t seen any examples.

I suppose you could try to request 100 and 900 or whatever the maximum is instead of 400 and 700. But this would, of course, increase weight penalties for common fonts.

Even if a family does contain exactly two weights, if they don’t exactly match what you request, you can actually end up getting faux bold synthesized from the Regular variant by GDI instead of the real Bold. In libass, we discard system-provided faux variants because we synthesize them ourselves, so this will miss the real Bold font.

And you’d want to run this unconditionally, without the found flag, because even if you have some fonts of the same family installed system-wide, they may be different from the attached variants.

I alluded to this earlier, but I’ve been meaning to rework our system someday to remove any room for issues with partially loaded families. This rework might allow us to use CreateFontIndirect, heh, directly on Windows, 100% the same way that VSFilter does it, even if we might then reject the font due to it being too poor a match. Anyway, that would automatically cover the AddFontMemResourceEx case, too. But this is a big rework, so in all honesty it isn’t going to happen soon.

adipose commented 10 months ago

Thanks for all the details. Some comments:

  1. Is the font drawing independent of the method used to load/find the font? Or would using freetype vs. GDI/dw result in different drawing?
  2. "that emulates what we know of GDI’s logic"--ok, supposing we passed an in-memory font, what does that mean in practice? You attempt to "merge" font families, if necessary, so they can be correctly selected depending on properties?

The overwhelming majority of Windows fonts have 400/700 and italics=0/1. But if a font is called for, and not found, I suppose you could go through all the standard weights of 100-900 and combine with italics, and you should get every possible needed weight.

The scenario for this is simply to take advantage of systems like LAVSplitter that load attached fonts into memory and offer no record of what those actual font files were. To find them using a font family name is a bit of an ugly scenario, admittedly. But historically, it works pretty well, as 99% of subs are using at most regular, bold, and bold+italic. So as I mentioned before, it seems to have some value to at least try once you've failed to enumerate at all.

And you’d want to run this unconditionally, without the found flag, because even if you have some fonts of the same family installed system-wide, they may be different from the attached variants.

Yes, perhaps so. Actually I'm not sure what the system should do in such a scenario. Even if you filled the gaps, wouldn't the already enumerated versions take precedence, leading to a confusing situation anyway?

What does add_font_face do, exactly, if the font already is found with the same properties/family?

adipose commented 10 months ago

An example of enumerating Verdana, which is made up of four ttf files. Curious if the charset is important the way ssa operates?

"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=0 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=161 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=162 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=186 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=238 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=204 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=163 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=0 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=161 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=162 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=186 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=238 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=204 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=0 ;  Underline=0 ;   StrikeOut=0 ;   CharSet=163 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=0 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=161 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=162 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=186 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=238 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=204 
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=163 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=0 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=161 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=162 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=186 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=238 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=204 
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=255 ;    Underline=0 ;   StrikeOut=0 ;   CharSet=163 
astiob commented 10 months ago
  1. Is the font drawing independent of the method used to load/find the font? Or would using freetype vs. GDI/dw result in different drawing?

(I’m assuming that you mean either ass_add_font or Fontconfig when you say “freetype”.)

The goal is that it should be independent, but it actually does vary. The obvious thing is that we can miss a font if we request it from the system, as happened here. The less obvious thing is that we trust some of the metadata from the system, but we really shouldn’t, because the system providers return different names in some cases than GDI/our internal code does. We have a relatively easy solution for this (just run all fonts through our internal metadata extractor), but it’s not actually solved quite yet.

The physical font selection process is independent, because it runs outside of any font provider. But it is affected by the metadata that is partly filled by the font provider.

2. "that emulates what we know of GDI’s logic"--ok, supposing we passed an in-memory font, what does that mean in practice? You attempt to "merge" font families, if necessary, so they can be correctly selected depending on properties?

I’m not sure I fully understand the question, but: The complete details of the font selection process are a bit too much for a comment in this thread, but we enumerate all known (previously loaded) fonts, filter them down to matching names, compare the properties to what’s requested, compute mismatch penalties, and select the font with the lowest penalty.

What does add_font_face do, exactly, if the font already is found with the same properties/family?

It always simply adds the font to the end of our internal list. The font selector chooses the first font among the fonts that share the lowest penalty value.

The fonts loaded by add_font_face are placed in the list ahead of any system fonts, which are added later.

Curious if the charset is important the way ssa operates?

It is, but libass doesn’t currently emulate it. Styles have Encoding and override tags have \fe for specifying what charset’s font is to be used. See also: https://github.com/clsid2/mpc-hc/issues/1799#issuecomment-1312257860 https://github.com/libass/libass/issues/662

adipose commented 10 months ago

When I said "freetype" I was thinking that if it didn't load it with GDI, it must render with freetype. But maybe it renders with DW.

astiob commented 10 months ago

Rendering is yet another separate concern. FreeType does not select fonts, and we don’t use GDI or DirectWrite (or Core Text on macOS) to do anything beyond finding fonts. As a cross-platform library, libass shares as much code as possible across platforms, so all fonts, no matter where they come from, are parsed into tables and outlines (shapes) by FreeType, and all shapes (glyphs from fonts and SSA/ASS vector drawings) are rasterized by libass’s own rasterizer.

adipose commented 10 months ago

Very helpful information. So in general we wouldn't expect GDI font selection to drive rendering.

I'll say one other thing. Using freetype you can find the alternate names for the font. Combined with the code I posted, it effectively allows you to supplement libass fonts with in-memory GDI fonts. But the precedence may not be right.

Given the "precedence" of the ass_add_font, I think the correct solution is for mpc-hc to use that. Otherwise, system-installed fonts may take precedence that we don't want.

As long as our font collection extraction within mpc-hc is working, the solution we have should work. Libass could still use the Indirect option as a fallback, provided the facename loaded actually matches one of the TTF published names.

adipose commented 10 months ago

@clsid2 , I think we can close this issue, not sure there's much more we can do for the mpc-hc side.

adipose commented 10 months ago

By the way, enumerating Verdana before was with EnumFontFamiliesExW. With EnumFontFamiliesW, the enumeration is exactly what I proposed before, with Italics/Bold:

"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=0;
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=0;
"Verdana";  Height=68;  Width=32;   Escapement=0;   Orientation=0;  Weight=700;     Italic=255;
"Verdana";  Height=68;  Width=28;   Escapement=0;   Orientation=0;  Weight=400;     Italic=255;
clsid2 commented 10 months ago

ass_add_font needs to get called for this situation as well: external fonts (from Fonts subdir) + external subtitle

adipose commented 10 months ago

ass_add_font needs to get called for this situation as well: external fonts (from Fonts subdir) + external subtitle

Oh...

https://github.com/clsid2/mpc-hc/pull/2430/files

@astiob

I read https://github.com/libass/libass/issues/548. Is it best to use ass_set_fonts_dir as I have done here, or to load all into memory using ass_add_font ?

clsid2 commented 10 months ago

I am unsure if the external loaded font file might already be enumerable: https://learn.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-addfontresourceexw We use FR_PRIVATE but given alternate parameter is FR_NOT_ENUM, I think enumeration might work?