Add SampleAspectRatioNum/SampleAspectRatioDen elements

[arch1t3cht]

Currently, Matroska allows a video's sample aspect ratio to be set via the display aspect ratio through the DisplayWidth/DisplayHeight elements.

However, these elements are currently specified to encode the display aspect ratio after applying cropping. This creates inconsistencies with players that do not read the PixelCrop elements and, in theory, makes it impossible for Matroska writers to create files that display with the correct aspect ratio both in players that respect cropping and in players that do not.

Moreover, many existing files do not actually follow this convention that the display aspect ratio apply to the cropped video, and instead store the display aspect ratio of the uncropped video. In particular, MKVToolNix stores the DisplayWidth/DisplayHeight in this format.

In order to be able to resolve these discrepancies in the future without changing existing behavior, this commit adds explicit SampleAspectRatio{Num,Den} elements, which specify the sample aspect ratio rather than the display aspect ratio. This has the advantage of being independent of any cropping and is also consistent with the way the sample/display aspect ratios are stored in video coding formats like H.264 (see: ITU-T H.273, section 8.6 "Sample aspect ratio indicator"). In cases where the DisplayWidth/DisplayHeight and the SampleAspectRatioNum/SampleAspectRatioDen elements are both present and might conflict, it is clearly specified in what way the sample aspect ratio elements take precedence. Matroska writers can write both kinds of elements without risking conflicts.

Refer to the following links for previous discussion about these issues.

See: https://codeberg.org/mbunkus/mkvtoolnix/issues/2389
See: mpv-player/mpv#13446

---

This change is a second, different approach to solving the problem brought up in #947. I am, among other reasons, opening it to hopefully create more discussion on this issue: I'm not overly attached to this specific solution and would be equally happy if #947 was applied instead of this.

The advantages of this solution over #947 are:

• It adds new elements and thus does not change any existing behavior, hence not breaking backwards compatibility
  • However: As argued in cropping: make DisplayWidth/DisplayHeight include cropped area #947 it's not clear how much backwards compatibility cropping: make DisplayWidth/DisplayHeight include cropped area #947 would actually break since most files in the wild do not follow the current spec and follow what cropping: make DisplayWidth/DisplayHeight include cropped area #947 proposes instead.
• It moves to storing the sample aspect ratio as a sample aspect ratio, which is more consistent with video coding formats like H.264 and is independent of any cropping.

The disadvantages of this solution over #947 are:

• Due to adding a new element, it requires Matroska v5
• It does not fix any of the existing spec-noncompliant files in the wild and could instead only be applied to new files.

---

Making SampleAspectRatio{Num,Den} completely "disable" (i.e. render informative) the Display{Width,Height} was chosen because even with added SampleAspectRatio{Num,Den} elements there would still be no way to determine whether an existing file's Display{Width,Height} are meant in the spec-compliant sense or in the spec-noncompliant-but-established sense.
An alternative would be enforcing that DisplayWidth be automatically derived as DisplayHeight * (PixelWidth - PixelCropLeft - PixelCropRight) / (PixelHeight - PixelCropTop - PixelCropBottom) * SampleAspectRatioNum / SampleAspectRatioDen (and not be purely informative), overriding an existing DisplayWidth in the file if present. This would work for spec-compliant files, but would still display spec-noncompliant files with (the correct display aspect ratio but) incorrect display dimensions in spec-compliant players (Though players that currently ignore the spec and follow #947's proposal instead could opt to also adopt equivalent behavior here).
Hence, completely disabling Display{Width,Height} when SampleAspectRatio{Num,Den} are present seems like the simpler choice, but I'm happy to change it if there are disagreements.

[robUx4]

Hi,

That mkvtoolnix issue link doesn't exist.

This creates inconsistencies with players that do not read the PixelCrop elements

If players don't support the current specifications, I don't see why they would support new elements better.

Maybe adding DisplayUnit = 5 for Sample Aspect Ratio (SAR) would be better. The specifications are probably not clear enough about what to do with cropping values when the DisplayUnit is 3 (Display Aspect Ratio). Some clarifications would also benefit a SAR value.

[mbunkus]

The issue they refer to is problem this one (same issue number after the migration, topic fits).

[kasper93]

That mkvtoolnix issue link doesn't exist.

Apparently, mkvtoolnix moved to codeberg. It was this issue https://codeberg.org/mbunkus/mkvtoolnix/issues/2389 (snapshot of gitlab version, https://web.archive.org/web/20250327165455/https://gitlab.com/mbunkus/mkvtoolnix/-/issues/2389)

If players don't support the current specifications, I don't see why they would support new elements better.

That's fair point, and that's why I initially suggested to retroactively updated spec to reflect reality of Matroska files that exist in the wild. In #947. Obviously, such amend to the existing elements, is controversial.

In mpv media player, we have an --demuxer-mkv-crop-compat=<yes|no> option to control cropping behavior. Based on user feedback it, unfortunately, is set to spec non-compliant mode by default, as this is what majority of users expected us to do.

The main issue is clearly outlined in the MKVToolNix issue. As currently defined in the Matroska specification, cropping is not backward compatible. The natural expectation would be that cropping is optional and applied only if supported. Otherwise, the uncropped image should be displayed. However, this is not the case. Adding cropping makes these files incompatible with unaware media players. This lack of backward compatibility is likely why adoption of the cropping has been slow and inconsistent, as users expect their files to work in Windows Media Player, MPC-HC, and VLC, regardless of whether cropping is applied or they see some black bars.

[arch1t3cht]

That mkvtoolnix issue link doesn't exist.

Yes, indeed, thanks - the mkvtoolnix repository moved in the meantime. I updated the link.

If players don't support the current specifications, I don't see why they would support new elements better.

Yes, a player that does not read the PixelCrop elements would probably not read the SampleAspectRatio elements either - this is another disadvantage of this solution over #947. (And, again, I'd be completely fine with #947 being implemented instead of this. I just wanted to offer a second approach to create more discussion on this.)

But the argument in favor of new elements would be that players like mpv that do read and apply the PixelCrop elements and try to support the full spec are currently struggling with the fact that existing files don't follow the spec and have to opt to (add options to) intentionally go against the spec in order to correctly play back these files. I'd expect that if SampleAspectRatio elements were added, support for them would quickly be implemented in such players (the author of #947 is an mpv maintainer, for example), which would improve the situation for them.

But, yes, if the goal is to fix the majority of existing files and players then #947 would accomplish that goal better, though it entails changing existing behavior.

Maybe adding DisplayUnit = 5 for Sample Aspect Ratio (SAR) would be better.

I considered this; the main reason why I decided against it is that it results in somewhat confusing wording and inconsistent behavior, and possibly even break existing players. At the moment, DisplayWidth / DisplayHeight is always equal to the DAR, no matter what DisplayUnit is. (And most files in the wild, e.g. produced by MKVToolNix, do not even set a DisplayUnit, effectively resulting in DisplayUnit = 0 (pixels).) Adding a DisplayUnit = 5 meaning "DisplayWidth / DisplayHeight = SAR" would go against this and might break the display aspect ratio in players that don't read/validate DisplayUnit and just directly compute the DAR as DisplayWidth / DisplayHeight.

But if everyone here prefers this over new elements I can of course change this.