Add application/x-font-otf to font_mimetypes
#15387
Conversation
|
Could you adjust the commit message to |
|
Download the artifacts for this pull request: |
|
Why we need this change? There was a bug in MKVToolNix to not map the MIME types to proper ones, but it is only one release and I don't see why we should support this bug. |
moi15moi
force-pushed
the
Add-a-font-mimetype
branch
from
cdc5f5c to
74cbebb
Compare
|
The issue was more related to a regression than to map to an incorrect MIME type
So, now, the question is more do we think
So, since there seems to be some application that have defined |
This is not valid Matroska MIME. And since the regression was present only for 1 month, I don't think it is important to add this. The impact is minimal.
We do it, because Matroska existed long before MIME types were standardized. And even current spec recommends to support legacy MIME types for compatibility. See: https://datatracker.ietf.org/doc/rfc9559/ But I don't think we should add supports for short lived bugs. Broken files are brokem, we shouldn't encourage use of it. |
I decided to create a PR on the matroska specification repos: ietf-wg-cellar/matroska-specification#921 |
|
I don’t see how it hurts to add these types. It will fix some real files, and there’s no downside. And apparently (Quite frankly, I’d even welcome ignoring MIME types completely and passing every single attachment to libass.) |
Historical reasons for supporting files that were produced for years is different than adding workaround for a short-lived bug in other software. Are we bug driven development now? We can add it, I'm just saying that it is not really needed.
Not every attachment is a font. What do you mean? |
|
TL;DR robustness principle Pragmatically: I do think it’s good to have compatibility with bugged files where feasible. Windows mkvmerge 88 has been producing these files for more than a month (I’ve seen two separate reports now of these MIME types causing issues), and not everyone will upgrade as soon as mkvmerge 89 is released. And these files work in practice due to fallbacks in mpv and LAV Splitter to file extension matching, so they will likely not be re-released to fix the MIME types. Ideologically: I don’t think it’s fair to call this a “bug”. mkvmerge isn’t the only Matroska muxer. Even mkvmerge lets the user override MIME types. The Matroska spec just references MIME types, not a Matroska-specific enum (which is the root of the problem!). Official MIME types for fonts didn’t exist before Frankly, historical reasons are shabby for a good many of the types on the list:
I mean that libass stands a better chance to figure out which attachment is a font and which isn’t by trying to parse them, rather than by trusting some third-party muxer’s unofficial MIME type. If the file turns out not to be a font, libass will just log a message and discard it. The only non-font attachments I’m aware of in practice are cover images, which IIUC are at most one per MKV, so this shouldn’t cause a performance regression, but it would avoid the endless MIME type issues that have plagued font attachments for many years. (Being not standardized at the time, MIME types should never have been used for this IMO.) |
|
Thank you for detailed response. Not sure it was that needed, but much appreciated anyway. I'm fine with merging it, but as I see the situation it is theoretical "fix" for some affected files, that may not exist. I see nothing wrong with the patch itself, just discussion if that is really a problem. Someone would have to attach a file, without extension and with this specific mime type.
This argument is invalid. Because apparently they upgraded to mkvmerge 88 as soon as it was released and now they won't upgrade to 89? Unless 88 was special, there will be no statistical difference in adaptation of the mkvmerge. Sorry to nitpick this, but it grinds my gears when bias is applied to one or the other to push some narrative. Not that it matters for this discussion, just something that I picked up.
If someone is attaching font with wrong MIME type and wrong font file extension it is on them. I don't think we should brute-force all garbage to libass, only because it may be a font. I just don't see an issue, except theoretical one... How wide-spread are font attachment without actual "font-like" filename? |
Ah: I can confirm they do. Don’t know how many, but I’ve seen a report about at least two publicly released files back at the end of October: possibly more, because it doesn’t look like the muxer confirmed they’d reconfigure their mkvmerge. And today someone else asked about the same thing but IIUC for a work-in-progress, so it might not see the light of day publicly.
I don’t follow. What makes you assume that someone who has 88 has it because they upgrade to every new version immediately? Anyone who simply installed mkvmerge during the last month has received 88. IME most people don’t follow updates religiously.
None of these MIME types are wrong. If you refer to mpv’s fallback to name extension matching, it equally applies to most (or all) of the MIME types, as I detailed above. |
Honestly, I don’t know. I assume you’re thinking “do we even need the MIME type check at all if we have the file name check?” That’s something I’m wondering about myself. |
some groups attach their filtering scripts (avisynth, vapursynth) as attachment to the mkv files, usually as zip. just thought i would mention it. |
mkvtoolnix recently introduced this mimetype in https://gitlab.com/mbunkus/mkvtoolnix/-/commit/d858c2bf138009b8316b32c4554397ecb609d938 which is available since the version 88.0, so I tought we would also need it.