mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
28.55k stars 2.92k forks source link

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

Open dawidpotocki opened 9 months ago

dawidpotocki commented 9 months ago

Important Information

Provide following Information:

Reproduction steps/Actual behaviour

Download the example video files.

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 ```ini [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 ```ini [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](https://github.com/mpv-player/mpv/assets/38681822/fb701295-dfb9-4abd-a11b-c42c57172e9f) #### `1080p-crop.mkv` with `blend-subtitles=yes` ![Subtitles with smaller size](https://github.com/mpv-player/mpv/assets/38681822/7b71ca74-62d1-488c-a523-784be4423931)

Cropped ASS text

#### `816p-native.mkv` with `blend-subtitles=yes` ![Subtitles being correctly positioned](https://github.com/mpv-player/mpv/assets/38681822/71169a5e-b8a4-4cac-afbc-bd3796d46ca4) #### `1080p-crop.mkv` with `blend-subtitles=yes` ![Subtitles being cropped out](https://github.com/mpv-player/mpv/assets/38681822/e1f09f1b-34b0-4d00-94fe-c166bf31fd5c) #### `1080p-crophalf.mkv` with `blend-subtitles=yes` ![Subtitles being half cropped out](https://github.com/mpv-player/mpv/assets/38681822/484babdb-0b44-4bf3-aedb-cd2604da4401) --- #### `816p-native.mkv` with `blend-subtitles=no` ![Subtitles being correctly positioned](https://github.com/mpv-player/mpv/assets/38681822/cc1de035-6527-4ff1-bf04-c22d9eed78c1) #### `1080p-crop.mkv` with `blend-subtitles=no` ![Subtitles being correctly positioned](https://github.com/mpv-player/mpv/assets/38681822/f91558f1-befc-455c-ba86-c9bfe1b53886) #### `1080p-crophalf.mkv` with `blend-subtitles=no` ![Subtitles being stretched out](https://github.com/mpv-player/mpv/assets/38681822/0d6d8cd6-8932-41ce-a4e1-587d5a8446ee)
llyyr commented 9 months ago

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

dawidpotocki commented 9 months ago

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](https://github.com/mpv-player/mpv/assets/38681822/940f0bce-be01-4bb0-b426-7c6907066b74) **libass-0.17.2 with `blend-subtitles=no`** ![Squeezed in text](https://github.com/mpv-player/mpv/assets/38681822/1ebd5356-145c-423f-b85f-0d8b9cd81b48) **libass-0.16.0 with `blend-subtitles=yes`** ![Big text](https://github.com/mpv-player/mpv/assets/38681822/5868a79f-c045-4516-888b-391449078f37) **libass-0.17.2 with `blend-subtitles=yes`** ![Squeezed in text](https://github.com/mpv-player/mpv/assets/38681822/8ddf8b3e-d40d-4102-a6bd-6f10a686d500)
kasper93 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 https://github.com/mpv-player/mpv/issues/12422 but it went away.

guidocella commented 9 months ago

--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 commented 9 months ago

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 commented 9 months ago

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](https://github.com/mpv-player/mpv/assets/38681822/6d4b67f4-322e-4e5f-8c0b-a063d8eb8b84) **`1080p-crop.mkv` and 1080p subs** ![obraz](https://github.com/mpv-player/mpv/assets/38681822/6e456e2e-915b-4abe-9443-a6d42f86f048) --- **`1080p-crophalf.mkv` and 816p subs** ![Normal looking text](https://github.com/mpv-player/mpv/assets/38681822/cc49eda9-79f1-4364-9a8c-3042e7cbb6f4) **`1080p-crophalf.mkv` and 1080p subs** ![Squished text](https://github.com/mpv-player/mpv/assets/38681822/ec656490-1abb-4f7d-8103-04b15c3b0e0e)
Screenshots with blend-subtitles=yes **`1080p-crop.mkv` and 816p subs** ![Completely cropped out text](https://github.com/mpv-player/mpv/assets/38681822/c6c2f5a6-a35b-4e42-857f-b2ad7f8a04d2) **`1080p-crop.mkv` and 1080p subs** ![Completely cropped out text](https://github.com/mpv-player/mpv/assets/38681822/766c6868-ae65-4050-ab57-87bc2126148a) --- **`1080p-crophalf.mkv` and 816p subs** ![Partially cropped text](https://github.com/mpv-player/mpv/assets/38681822/f0e68574-b1a4-42dc-984b-345065bc1973) **`1080p-crophalf.mkv` and 1080p subs** ![Nearly completely cropped text](https://github.com/mpv-player/mpv/assets/38681822/df0a7570-fe27-4bbc-8653-0f66cbcae3c3)

New example files with 816p and 1080p subs included

kasper93 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 ![](https://github.com/mpv-player/mpv/assets/515193/0bbec30c-6a36-4dc9-a834-69993089760e) 1080p-crop ![](https://github.com/mpv-player/mpv/assets/515193/06686167-ecd0-4e9c-9b60-8c7139bbece2) 1080p-crophalf ![](https://github.com/mpv-player/mpv/assets/515193/c16e99f6-cbbb-48ce-ab42-5c6bbc8c885b) 1080p-native ![](https://github.com/mpv-player/mpv/assets/515193/bdcee671-8203-467f-97d2-0a06d3ae64a6)
816p ASS 816p-native ![](https://github.com/mpv-player/mpv/assets/515193/a7add536-9a28-4613-9db1-5216d92c7c1d) 1080p-crop ![](https://github.com/mpv-player/mpv/assets/515193/c2024a34-bccb-4ad9-8d40-ffdf4f6f0934) 1080p-crophalf ![](https://github.com/mpv-player/mpv/assets/515193/ace0e527-2af1-4392-af79-3f4b292165ad) 1080p-native ![](https://github.com/mpv-player/mpv/assets/515193/4ae6e20a-ef27-462f-9b4a-e303706cda3c)
1080p ASS 1080p-crop ![](https://github.com/mpv-player/mpv/assets/515193/aeefdb1d-5f6a-4591-9e1d-33192705d29d) 1080p-crophalf ![](https://github.com/mpv-player/mpv/assets/515193/da3ba1d0-18f4-4e22-826b-8bd0e0acaca0) 1080p-native ![](https://github.com/mpv-player/mpv/assets/515193/f1c40eab-90f5-46e6-b15b-e48d070aa0dc)

The cropping happens in Haali[^1] 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.

[^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 commented 9 months ago

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 commented 9 months ago

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 https://github.com/mpv-player/mpv/commit/81102b0f6c7db956cd222ea3e81debda36de5a78 (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.