mpv icon indicating copy to clipboard operation
mpv copied to clipboard

--sub-blur not applied when --sub-ass-override is set to force

Open low-batt opened this issue 1 year ago • 8 comments

mpv Information

mpv v0.38.0-771-gef19a4a09d Copyright © 2000-2024 mpv/MPlayer/mplayer2 projects
 built on Sep 14 2024 20:23:14
libplacebo version: v7.349.0 (v7.349.0-10-g76bb9718)
FFmpeg version: n7.0.2
FFmpeg library versions:
   libavcodec      61.3.100
   libavdevice     61.1.100
   libavfilter     10.1.100
   libavformat     61.1.100
   libavutil       59.8.100
   libswresample   5.1.100
   libswscale      8.1.100

Other Information

- macOS version: macOS 13.6.9 (22G830)
- Source of mpv: Local build using mpv-build
- Introduced in version: Unknown

Reproduction Steps

  • Download the attached big_buck_bunny.ass.txt file
  • Rename it to big_buck_bunny.ass
  • Copy some video to big_buck_bunny.mp4 to match up with subtitle file
  • Play the video with --sub-blur=3 --sub-ass-override=force
  • Play the video with --sub-blur=3 --sub-ass-override=strip
  • Notice the blur factor is not applied to the sub font border with --sub-ass-override set to force

Expected Behavior

This report originates from IINA issue https://github.com/iina/iina/issues/5174. User @sagieinav expects that IINA will apply a blur factor to the sub font border when sub-ass-override is set to force.

I tested mpv and neither sub-blur nor sub-pos are applied with sub-ass-override is set to force. As force is documented as forcing all --sub-* options I expect that to include both sub-blur and sub-pos.

Actual Behavior

With --sub-ass-override set to force neither sub-blur nor sub-pos are applied.

With --sub-ass-override=no:

mpv/build/mpv --no-config --autofit=2048 --pause --start=1:08 --sub-blur=3 --sub-pos=50 --sub-scale=2 --sub-ass-override=no ~/Movies/big_buck_bunny.mp4

no

With --sub-ass-override=force neither --sub-blur nor --sub-pos are applied:

mpv/build/mpv --no-config --gpu-debug --log-file=output.txt --autofit=2048 --pause --start=1:08 --sub-blur=3 --sub-pos=50 --sub-scale=2 --sub-ass-override=force ~/Movies/big_buck_bunny.mp4

force

With --sub-ass-override=strip both --sub-blur and --sub-pos are applied::

mpv/build/mpv --no-config --autofit=2048 --pause --start=1:08 --sub-blur=3 --sub-pos=50 --sub-scale=2 --sub-ass-override=strip ~/Movies/big_buck_bunny.mp4

strip

Log File

output.txt

Sample Files

big_buck_bunny.ass.txt

I carefully read all instruction and confirm that I did the following:

  • [X] I tested with the latest mpv version to validate that the issue is not already fixed.
  • [X] I provided all required information including system and mpv version.
  • [X] I produced the log file with the exact same set of files, parameters, and conditions used in "Reproduction Steps", with the addition of --log-file=output.txt.
  • [X] I produced the log file while the behaviors described in "Actual Behavior" were actively observed.
  • [X] I attached the full, untruncated log file.
  • [X] I attached the backtrace in the case of a crash.

low-batt avatar Sep 15 '24 00:09 low-batt

I have a fix for this but I need a bit more time to make sure what sort of behavior makes sense, and also how to update the documentation to avoid such confusion in the future.

llyyr avatar Sep 15 '24 08:09 llyyr

The blur part will be fixed by https://github.com/libass/libass/pull/827

~~the position part will be fixed by https://github.com/mpv-player/mpv/pull/14858~~

But maybe we don't want to fix the positioning part, see: https://github.com/mpv-player/mpv/commit/2f1eb49e9f130c728c09a3458ff8449bc571c0c0

edit:

Yeah, the sub-pos part is a no-fix, it completely breaks ass rendering by forcing every dialogue at the default sub-pos value.

llyyr avatar Sep 15 '24 10:09 llyyr

@low-batt Thank you for reporting this!

@llyyr I was about to comment that the position part is ok as it is, as the expected behavior of force mode should be to NOT force positioning, as I understand it. Isn't if what differs --force from --strip?

So the only thing that was needed to be fixed is the blur styling not taking effect.

Thank you for the quick response and fix. Much appreciated.

sagieinav avatar Sep 16 '24 15:09 sagieinav

The case where it is critical to override the position of ASS subtitles is when mpv's ability to display a secondary subtitle is being used. The position specified in the ASS subtitles may put them both in the same location of the screen.

The documentation for sub-ass-override says:

force: Like yes, but also force all --sub-* options. Can break rendering easily.

Clearly --sub-pos is a --sub-* option and therefore should be applied according to the documentation. If the implementation is not going to be changed to apply --sub-pos then the documentation needs to be corrected.

The documentation also needs to address the situation where a --sub-ass* option provides the same setting as a --sub-* option, as in which takes precedence.

While we are discussing this feature…

The documentation for secondary-sub-ass-override says:

Control whether user secondary substyle overrides should be applied. This works exactly like `--sub-ass-override`.

Is that really true, as in exactly? Or is --secondary-sub-ass-override applying secondary-sub* options like secondary-sub-pos?

low-batt avatar Sep 17 '24 00:09 low-batt

The case where it is critical to override the position of ASS subtitles is when mpv's ability to display a secondary subtitle is being used. The position specified in the ASS subtitles may put them both in the same location of the screen.

This isn't true and you'd know it's not true if you tried using secondary-sid. The documentation for the option even says: "Styling and interpretation of any formatting tags is disabled for the secondary subtitle. Internally, the same mechanism as --sub-ass=no is used to strip the styling."

Secondary subtitles are converted, so they're allowed to be positioned wherever.

Clearly --sub-pos is a --sub-* option and therefore should be applied according to the documentation. If the implementation is not going to be changed to apply --sub-pos then the documentation needs to be corrected.

The documentation for the option also says "Note that all of these overrides try to be somewhat smart about figuring out whether or not a subtitle is considered a "sign"." The goal of this option is not to be destructive, if that's what you want then you're looking for sub-ass=no or sub-ass-override=strip, not sub-ass-override=force. wm4 disabled alignment for a reason, it completely breaks and makes the option pointless. I can make the documentation reflect reality better, if that's what you want.

The documentation also needs to address the situation where a --sub-ass* option provides the same setting as a --sub-* option, as in which takes precedence.

Such as?

Is that really true, as in exactly? Or is --secondary-sub-ass-override applying secondary-sub* options like secondary-sub-pos?

Yes, it is really true, because it says a line below that the default value for --secondary-sub-ass-override is strip. That is why secondary-sub-pos is applied.

llyyr avatar Sep 17 '24 01:09 llyyr

I could be remembering wrong. There was an IINA issue about position. From what I remember, the secondary subtitle was a SRT file and was displayed by mpv at the top of the screen and the primary subtitle was an ASS file that specified it should be displayed on top as well. I think we ended up setting the override level to strip.

The issue about precedence comes from #14800 where --sub-font overrides --sub-ass-style-overrides=FontName=Wingdings. I expected that --sub-ass-style-overrides would take priority.

I'm happy with however you would like to address this issue. I'm just reporting my own confusion in trying to understand the behavior of this option.

low-batt avatar Sep 17 '24 03:09 low-batt

the secondary subtitle was a SRT file and was displayed by mpv at the top of the screen and the primary subtitle was an ASS file that specified it should be displayed on top as well.

That seems to be working as expected? Nothing is stopping the primary subtitle from using {\an8} or something to move to the top. mpv can't know this, if you have primary subtitles that want to be displayed at the top then force the secondary subtitles to the bottom with --secondary-sub-pos.

I think we ended up setting the override level to strip.

IINA should not ship with the default override level set to strip if that's what it is doing currently, this effectively destroys ASS subtitles completely.

--sub-font overrides --sub-ass-style-overrides=FontName=Wingdings. I expected that --sub-ass-style-overrides would take priority.

This is an issue I intend to resolve in the future, by making it so that when --sub-ass-override=force is set, mpv doesn't set any options unless explicitly set by the user. Right now, --sub-font takes precedence over it because of implementation details and the order of operations in mpv.

llyyr avatar Sep 17 '24 03:09 llyyr

Yes, stripping worked in that case. That IINA issue was from a while ago before --secondary-sub-pos was added. I agree that is the better way of addressing this case. Sorry I wasn't clear. When I referred to setting the level to strip I meant having that user configure the override level to strip. I will check the IINA default settings.

In the current version of IINA the override level setting is not being handled correctly . For example the scale setting is missing. This is being corrected along with adding support for --secondary-sub-ass-override, --secondary-sub-delay and --secondary-sub-pos.

Would explicitly set by the user include using conditional auto profiles? Just curious.

I was looking into how IINA could tell if the user set a mpv option in order to allow users to override some of the settings IINA applies. I was coming to the conclusion that in IINA's case it was probably better to have the user explicitly configure IINA with the options IINA should not set.

low-batt avatar Sep 17 '24 04:09 low-batt