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

blend-subtitles=yes breaks subtitles when using video-crop #13423

Open
dawidpotocki opened this issue Feb 6, 2024 · 15 comments
Open

blend-subtitles=yes breaks subtitles when using video-crop #13423

dawidpotocki opened this issue Feb 6, 2024 · 15 comments

Comments

@dawidpotocki
Copy link

dawidpotocki commented Feb 6, 2024

Important Information

Provide following Information:

  • mpv version: master
  • Platform and Version: Arch Linux
  • Source of the mpv binary: own build
  • If known which version of mpv introduced the problem: f3f1a79; i.e. addition of video-crop
  • GPU model, driver and version: N/A
  • Possible screenshot or video of visual glitches: N/A

Reproduction steps/Actual behaviour

Download the example video files.

  • 816p-native.mkv - 1920x816 video
  • 1080p-crop.mkv - 1920x816 video (cropped from 1920x1080 using Matroska PixelCrop properties)
  • 1080p-crophalf.mkv - 1920x948 (cropped from 1920x1080 using Matroska PixelCrop properties)
  • 1080p-native.mkv - 1920x1080 (no crop applied, can apply yourself with video-crop, will behave the same)

Inconsistent SRT font size

  1. Set blend-subtitles=yes
  2. Play 816p-native.mkv video
  3. Play 1080p-crop.mkv video, font size is smaller than 816p-native.mkv

Cropped ASS text

  1. Set blend-subtitles=yes and sid=2
  2. Play 816p-native.mkv video, text appears correctly
  3. Play 1080p-crop.mkv video, text is not visible
  4. Play 1080p-crophalf.mkv video, text is half cropped and not blended with video

Expected behaviour

Inconsistent SRT font size

816p-native.mkv and 1080p-crop.mkv should have the same font size.

Cropped ASS text

816p-native.mkv, 1080p-crop.mkv should look the same.

1080p-crop.mkv with blend-subtitles=no looks identically to 816p-native.mkv with blend-subtitles=yes

(not sure how 1080p-crophalf.mkv should look like regardless of the blend-subtitles)

Log file

Zipped logs of all test runs

Sample files

(UPDATED)
Zipped example Matroska files

Contents of the 816p ASS subtitles muxed in
[Script Info]
ScriptType: v4.00+
PlayResX: 1920
LayoutResX: 1920
LayoutResY: 816
PlayResY: 816
ScaledBorderAndShadow: yes
YCbCr Matrix: TV.709

[V4+ Styles]
Format: Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, OutlineColour, BackColour, Bold, Italic, Underline, StrikeOut, ScaleX, ScaleY, Spacing, Angle, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, MarginV, Encoding
Style: Default,Arial,70,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,1,1,0,2,10,10,10,1

[Events]
Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
Dialogue: 0,0:00:00.00,0:00:10.00,Default,,0,0,0,,Consistency goes brrrrrr
Contents of the 1080p ASS subtitles muxed in
[Script Info]
ScriptType: v4.00+
PlayResX: 1920
LayoutResX: 1920
LayoutResY: 1080
PlayResY: 1080
ScaledBorderAndShadow: yes
YCbCr Matrix: TV.709

[V4+ Styles]
Format: Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, OutlineColour, BackColour, Bold, Italic, Underline, StrikeOut, ScaleX, ScaleY, Spacing, Angle, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, MarginV, Encoding
Style: Default,Arial,70,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,1,1,0,2,10,10,10,1

[Events]
Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
Dialogue: 0,0:00:00.00,0:00:10.00,Default,,0,0,0,,Consistency goes brrrrrr

Screenshots

Inconsistent SRT font size

816p-native.mkv with blend-subtitles=yes

Subtitles with correct size

1080p-crop.mkv with blend-subtitles=yes

Subtitles with smaller size

Cropped ASS text

816p-native.mkv with blend-subtitles=yes

Subtitles being correctly positioned

1080p-crop.mkv with blend-subtitles=yes

Subtitles being cropped out

1080p-crophalf.mkv with blend-subtitles=yes

Subtitles being half cropped out


816p-native.mkv with blend-subtitles=no

Subtitles being correctly positioned

1080p-crop.mkv with blend-subtitles=no

Subtitles being correctly positioned

1080p-crophalf.mkv with blend-subtitles=no

Subtitles being stretched out

@llyyr
Copy link
Contributor

llyyr commented Feb 6, 2024

Does this issue still exist if you downgrade libass to 0.16.0?

@dawidpotocki
Copy link
Author

dawidpotocki commented Feb 6, 2024

Yes, the issue still persists with libass-0.16.0.

Only the rendering of 1080p-crophalf.mkv changed.

Neither seems particularly correct to me.

Screenshots of 1080p-crophalf.mkv

libass-0.16.0 with blend-subtitles=no
Normal looking text

libass-0.17.2 with blend-subtitles=no
Squeezed in text

libass-0.16.0 with blend-subtitles=yes
Big text

libass-0.17.2 with blend-subtitles=yes
Squeezed in text

@kasper93
Copy link
Contributor

kasper93 commented Feb 6, 2024

Inconsistent SRT font size

--sub-scale-with-window=no

1080p-crop.mkv with blend-subtitles=yes

Looks ok, your subtitles have PlayResY: 816, so they are stretched to black bars area which is cropped.

1080p-crophalf.mkv with blend-subtitles=yes

The same as above, but you crop only half.

1080p-crop.mkv with blend-subtitles=no

Now that you disabled blending your subtitles are rendered at target resolution instead of video.

1080p-crophalf.mkv with blend-subtitles=no

This is rightfully stretched, because PlayResY: 816 is subtitle is not correct for this video, so it got stretched.


All examples looks correct to me. Could you explain what do you expect and why?

@dawidpotocki
Copy link
Author

1080p-crop.mkv with blend-subtitles=yes

--sub-scale-with-window=no

Shouldn't a cropped video behave the same though?

Make the subtitle font size relative to the window, instead of the
video. This is useful if you always want the same font size, even
if the video doesn't cover the window fully, e.g. because screen
aspect and window aspect mismatch (and the player adds black bars).

The video covers the entire window when cropped by the player, so not
sure why it shouldn't behave the same.

Looks ok, your subtitles have PlayResY: 816, so they are stretched to black bars area which is cropped.

So… you think that PlayRes is supposed to be set for the resolution before cropping?

If that is true, why does it behave differently with blend-subtitles=no?

@kasper93
Copy link
Contributor

kasper93 commented Feb 6, 2024

The video covers the entire window when cropped by the player, so not
sure why it shouldn't behave the same.

I'm not touching this option. It has been in my opinion broken for years or I just don't understand it's purpose. Just put sub-scale-with-window=no in config and forget about it. You can see what happens when you use --video-zoom... this option is weird and default behavior is weird.

So… you think that PlayRes is supposed to be set for the resolution before cropping?
If that is true, why does it behave differently with blend-subtitles=no?

It is kind of explained in docs, but in the same time they are not fully updated for gpu-next.

Blend subtitles directly onto upscaled video frames, before interpolation and/or color management (default: no). Enabling this causes subtitles to be affected by --icc-profile, --target-prim, --target-trc, --interpolation, --gamma-factor and --glsl-shaders. It also increases subtitle performance when using --interpolation.

The downside of enabling this is that it restricts subtitles to the visible portion of the video, so you can't have subtitles exist in the black margins below a video (for example).

If video is selected, the behavior is similar to yes, but subs are drawn at the video's native resolution, and scaled along with the video.

Either way, blend-subtitles changes between drawing subtitles at video's native resolution or video target resolution. I think current options can control pretty well subtitle placement. There is also sub-use-margins. We can revamp all this, it is only about order of operations, like when we apply the crop values. I frankly don't care much about subtitles, I think I need to educate myself on LayoutRes and PlayRes impact and what would be best approach to handle cropping in all this. There was also #12422 but it went away.

@guidocella
Copy link
Contributor

--sub-scale-with-window scales subtitles with the ratio between the video's source height and the scaled output height, i.e. it makes subtitles bigger with top and bottom black bars, and smaller when the video's height doesn't fit in the window. It is meant to make the subtitle size indipendent of black bars at the same window height by compensating for libass' scaling, but it also leads to unintuitive behavior of subtitles getting smaller as you zoom in. I tried to improve its documentation but then concluded that it just should be removed and scaling should be sane by default, which was also wm4's opinion.

@kasper93
Copy link
Contributor

kasper93 commented Feb 6, 2024

So… you think that PlayRes is supposed to be set for the resolution before cropping?

Yes, both PlayRes and LayoutRes should be native video resolution values before cropping at least this is the current expectation. I asked around about this to make sure.

EDIT: Intuitively, maybe it is true, that having PlayRes and LayoutRes apply to cropped res make cropping more flexible and makes also subtitles compatible with more files... possibly. But it would conflict with current behavior, and I think this is what you can control with blend-subs option.

@dawidpotocki
Copy link
Author

Okay, so:

PlayRes/LayoutRes are supposed to be set to the original video
uncropped resolution.

In that case, there is an issue with the default mpv behaviour with
blend-subtitles=no as it squishes on 1080p subs but not on 816p.

I tried setting a crop with subtitles having 1920x1080
PlayRes/LayoutRes but actually it just makes it worse.

Screenshots with blend-subtitles=no

1080p-crop.mkv and 816p subs
obraz
1080p-crop.mkv and 1080p subs
obraz


1080p-crophalf.mkv and 816p subs
Normal looking text
1080p-crophalf.mkv and 1080p subs
Squished text

Screenshots with blend-subtitles=yes

1080p-crop.mkv and 816p subs
Completely cropped out text
1080p-crop.mkv and 1080p subs
Completely cropped out text


1080p-crophalf.mkv and 816p subs
Partially cropped text
1080p-crophalf.mkv and 1080p subs
Nearly completely cropped text

New example files with 816p and 1080p subs included

@kasper93
Copy link
Contributor

kasper93 commented Feb 6, 2024

I cannot reproduce, works fine for me on latest mpv and libass 0.17.1

EDIT: I didn't notice you mix different files with different subtitles...

blend-subtitles=no as it squishes on 1080p subs but not on 816p.

With blend-subtitles=no it stretches/squashes PlayRes to target video area after scaling, cropping.
With blend-subtitles=yes it stretches/squashes PlayRes to source video area before scaling, cropping.

If you feel something is wrong, please focus on one example and describe what should be expected behavior.

@dawidpotocki
Copy link
Author

If you feel something is wrong, please focus on one example and describe what should be expected behavior.

Fine… let's first focus on this clearly broken behaviour:

Subtitles with blend-subtitles=true are not shown regardless if PlayRes is set to source or target.

  1. Set blend-subtitles=true
  2. Play 1080p-crop.mkv
  3. Switch to sid=2 (target res)
  4. No subtitles visible
  5. Switch to sid=3 (source res)
  6. No subtitles visible

Expected behaviour… visible for at least one of them…

@kasper93
Copy link
Contributor

kasper93 commented Feb 7, 2024

Subtitles with blend-subtitles=true are not shown regardless if PlayRes is set to source or target.

With blend-subtitles=yes it stretches/squashes PlayRes to source video, meaning regardless if PlayResX is 816 or 1080, the subtitle line would end up in the black bar area fully. (for different test you can place something in the middle of screen) They would be stretched differently, but still end up there. After that we crop this area, so the subtitle that was there are also removed.

Expected behaviour… visible for at least one of them…

As explained above they wouldn't be visible. Or at least the part of subtitle that is active will be cropped off, but not all. You can use --sub-use-margins=yes --sub-ass-force-margins=yes which incidentally with --blend-subtitles=yes doesn't make much sense, because we don't allow to exit video area anyway, but those options make it so the rendering area is scaled to cropped area. This is arguably a bug and/or side-effect, because in this case it probably should do nothing. I will look at the code tomorrow to see why it does what it does.

I don't want to sound dismissive, but is works as it works and there is logic behind it. While maybe not intuitive, it kind works as "expected".

Note that I care only about gpu-next in this tests, I have not testes gpu. Also for gpu-next the difference with blend-subtitles is only target rectangle for subtitles, they are not in fact blended with source video, they are blended at the end always.

@dawidpotocki
Copy link
Author

Okay, I think I understand now.

Though even if technically it's "expected", I don't think it makes much
sense for it to behave this way as this will require people making 2
separate subtitle tracks just so it works for people with
blend-subtitles=yes and blend-subtitles=no.

Nobody will do that and one group will just end up with broken subs.

@astiob
Copy link
Contributor

astiob commented Feb 9, 2024

To the best of my knowledge, Haali Media Splitter is the only thing that has supported Matroska container-level crop until it was added to mpv. As such, any files that use Matroska crop and were created before mpv introduced support expect the same behaviour that Haali exhibited (unless they expect it to be ignored completely, of course). The standard for subtitles in the same era was DirectVobSub (VSFilter loaded as a DirectShow filter).

Here’s what the attached sample files look like with Haali + DirectVobSub. The report doesn’t explain which screenshots use which ASS track, so here are all of them for good measure.

NB about LayoutRes

The version of DirectVobSub I used in this test is older than the LayoutRes ASS headers, so layout resolution is always equal to DirectVobSub’s input video size, i. e. the cropped video size. However, nothing in this ASS script actually seems to depend on layout resolution. this only affects the horizontal stretching of the text. So please ignore differences in that. The 816p ASS should always look as wide as the 816p-native/1080p-crop pics, and the 1080p ASS should as the 1080p-native pics.

SRT

816p-native

1080p-crop

1080p-crophalf

1080p-native

816p ASS

816p-native

1080p-crop

1080p-crophalf

1080p-native

1080p ASS

1080p-crop

1080p-crophalf

1080p-native

The cropping happens in Haali1 and DirectVobSub gets the video already cropped (which I’ve confirmed from DirectShow pin info).

This is what mpv should emulate in my opinion.

This also makes the most intuitive sense to me. I feel container-level cropping is a tool that pretends to hard-crop a video stream as if it is re-encoded without actually re-encoding it in order to avoid quality loss, size blow-up, codec change etc. As such, a container-cropped video stream should for all intents and purposes be treated the same as a hard-cropped video stream, including for subtitle display. This is a tool an author uses to get a video stream to design their subtitles for, not a tool someone uses to chop up an already-subtitled video.

By the way, note how Haali 1080p-crop is entirely green, whereas the mpv screenshots have black bars. If these screenshots are true, mpv is in the wrong here. The cropped sample files declares Matroska DisplayWidth/DisplayHeight of 1920×1080. The Matroska spec explicitly says:

Height of the video frames to display. Applies to the video frame after cropping (PixelCrop* Elements).

The spec notes reassert this:

Cropping has to be performed before resizing and the display dimensions given by DisplayWidth, DisplayHeight and DisplayUnit apply to the already cropped image.

(This also matches my own interpretation that the stream is meant to be treated as hard-cropped.)

So the video is first to be cropped to 1920×816, and then this area is to be rescaled for display back to 1920×1080. If the DisplayHeight element were dropped, the default would be 816, matching the cropped area and requiring no stretching.

Thus, if we drop the most likely unintended DisplayHeight from 1080p-crop.mkv, with Haali’s approach the 816p ASS can be used with either 1080p-crop.mkv or 816p-native.mkv without any changes and look exactly the same.

Footnotes

  1. I haven’t confirmed this personally, but I’ve heard that Haali actually implements it by overwriting codec-level cropping fields by the container-level field values.

@astiob
Copy link
Contributor

astiob commented Feb 9, 2024

However, nothing in this ASS script actually seems to depend on layout resolution.

Actually, the aspect ratio of the text does. With proper LayoutRes support, the width of the subtitles should stay the same regardless of the (vertical) video cropping. In my screenshots, the text is narrowest in 1080-crop, wider in 1080-crophalf and widest in 1080p-native. With LayoutRes, the 816p ASS should always look as wide as the 1080p-crop pic and the 1080p ASS as wide as the 1080p-native pic.

@kasper93
Copy link
Contributor

kasper93 commented Feb 9, 2024

I feel container-level cropping is a tool that pretends to hard-crop a video stream as if it is re-encoded without actually re-encoding it in order to avoid quality loss, size blow-up, codec change etc. As such, a container-cropped video stream should for all intents and purposes be treated the same as a hard-cropped video stream, including for subtitle display.

I agree, but we are past that point. Adding video-crop and subsequently container crop was not intended to break existing subtitles. And existing subtitles doesn't expect cropping. Nobody is using Haali anymore and to preserve compatibility with current subtitles and tooling the current behavior of crop exists which does not affect subtitles in practice. Well depending on settings.

I agree though it is good approach to draw subtitles on cropped plane, but in practice this means that I have to add compat option. Doable, but this is more disruptive than current approach. That's why I opted to do that, until someone complains that want to change to, and now we can do this change. Also note the subtitles made for cropped video would not work if cropping is not applied, so they would essentially work only in mpv (and haali) currently. Another way would be to tag subtitles what frame of reference they expect.

By the way, note how Haali 1080p-crop is entirely green, whereas the mpv screenshots have black bars. If these screenshots are true, mpv is in the wrong here. The cropped sample files declares Matroska DisplayWidth/DisplayHeight of 1920×1080.

I reverted this because people were complaining 81102b0 (there is one condition missing there, but not important for this argument) but it is my fault for listening to people with wrong files. We are just small media player I cannot break everything just like that. I think I will have too add another compat option for that though.

And just for the record I blame MKVToolNix for this.

Muxing a file with options likes that
image
results in file having

[b0] PixelWidth size: 2 value: uint 1920
[ba] PixelHeight size: 2 value: uint 1080
[54aa] PixelCropBottom size: 1 value: uint 200
[54bb] PixelCropTop size: 1 value: uint 200
[54b0] DisplayWidth size: 4 value: uint 1920
[54ba] DisplayHeight size: 4 value: uint 1080

Which is not expected IMHO and maybe it is my fault for not understanding how to use MKVToolNix, but here we are and I'm trying to make the least amount of people mad.

EDIT:

Also if you consider PGS or VobSub they are mastered with video resolution in mind. So if you add crop to your file, to remove black bars, you have to also crop PGS/VobSub. I don't think ASS in fact should work differently.

I think it was Haali + DirectVobSub limitation at the time, that crop were added by the decoder before subtitles were drawn. But this basically break all PGS subs. While of course we can preserve this behavior, I don't except you will find any files that actually are made for "Haali + DirectVobSub" combo, because they would work wrong with anything else. Anyway, we are here to discuss. I would like to establish some sane behavior in mpv, because I know this will be used as a point of reference for many.

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

No branches or pull requests

5 participants