Skip to content
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

cropping: make DisplayWidth/DisplayHeight include cropped area #947

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kasper93
Copy link

@kasper93 kasper93 commented Feb 4, 2025

The primary motivation for this change is to ensure that Matroska files with PixelCrop* elements are displayed consistently, regardless of whether cropping is applied. This makes such files compatible across software that either supports or ignores these elements. Without this change, files suffer from limited compatibility, whereas cropping tags should enhance the experience rather than be a requirement for proper image display.

A secondary motivation is that an overwhelming number of files in the wild do not actually adjust the DisplayWidth/DisplayHeight elements as expected by the specification. Most of the time, these values are left uncropped, with the expectation that cropping will be applied later. This issue arises partly because MKVToolNix does not automatically perform this calculation, it only sets the PixelCrop* elements unless explicitly instructed by the user. Observing the existing files in circulation, it is evident that almost no one manually adjusts these values.

Lastly, some subtitle formats depend on the video size for positioning and rendering, making them sensitive to cropping. Currently, most authoring tools do not take cropping into account. By making the Matroska specification more flexible and allowing subtitles to be cropped along with the video, it becomes easier to produce files that are compatible with various Matroska players.

For a more detailed discussion on this topic, please refer to the links below:

See: https://gitlab.com/mbunkus/mkvtoolnix/-/issues/2389
See: mpv-player/mpv#13446

The primary motivation for this change is to ensure that Matroska files
with `PixelCrop*` elements are displayed consistently, regardless of
whether cropping is applied. This makes such files compatible across
software that either supports or ignores these elements. Without this
change, files suffer from limited compatibility, whereas cropping tags
should enhance the experience rather than be a requirement for proper
image display.

A secondary motivation is that an overwhelming number of files in the
wild do not actually adjust the `DisplayWidth`/`DisplayHeight` elements
as expected by the specification. Most of the time, these values are
left uncropped, with the expectation that cropping will be applied
later. This issue arises partly because MKVToolNix does not
automatically perform this calculation, it only sets the `PixelCrop*`
elements unless explicitly instructed by the user. Observing the
existing files in circulation, it is evident that almost no one manually
adjusts these values.

Lastly, some subtitle formats depend on the video size for positioning
and rendering, making them sensitive to cropping. Currently, most
authoring tools do not take cropping into account. By making the
Matroska specification more flexible and allowing subtitles to be
cropped along with the video, it becomes easier to produce files that
are compatible with various Matroska players.

For a more detailed discussion on this topic, please refer to the links
below:

See: https://gitlab.com/mbunkus/mkvtoolnix/-/issues/2389
See: mpv-player/mpv#13446
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant