Fix sub ass override

[llyyr]

Fixes #14229

[llyyr]

Actually, I'm now not convinced the code is actually wrong. I think the documentation is incorrect here instead since 4d1ffecabc457e31e51c6331ed89304ff45dc671. Although it should be noted that the documentation before it was also incorrect, signfs specifically did not apply scale to signs.

'signfs' used to make it so that --sub-scale is only applied to to dialogue, not signs. It was renamed to 'scale' and the documentation was incorrectly changed. ASS_OVERRIDE_BIT_SELECTIVE_FONT_SCALE is a very misleading name, so it's understandable why it was misunderstood what it did.

    /**
     * Apply ass_set_font_scale() only on events which look like dialogue.
     * If not set, the font scale is applied to all events. (The behavior and
     * name of this flag are unintuitive, but exist for compatibility)
     */
    ASS_OVERRIDE_BIT_SELECTIVE_FONT_SCALE = 1 << 1,

I don't see the value in changing 'scale' to do what the current docs say, I think current behavior has much more value so I'm in favor of just fixing the docs instead of fixing the logic.

Any thoughts? @avih @kasper93 @na-na-hi

edit:

On another thought, sub-ass-override=yes should only apply sub-ass options, so sub-scale should not be applied on 'yes' so maybe this PR is the right solution after all

[github-actions[bot]]

Download the artifacts for this pull request:

Windows

* [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1545051116.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1545054402.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/1545073339.zip)

macOS

* [mpv-macos-12-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1545048678.zip) * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1545053126.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/1545057584.zip)

[na-na-hi]

sub-ass-override=yes should only apply sub-ass options, so sub-scale should not be applied on 'yes'

I agree with this. --sub-* options are documented to have no effect on ASS subtitles unless sub-ass-override is used.

[llyyr]

I think this is good now

[Jules-A]

:scale: Like yes, but also apply --sub-scale (default).

I'm confused, shouldn't yes include scale and scale only affect scaling? Say if I wanted to use this:

sub-scale=0.9
sub-border-color="#808080"
sub-border-size=1.5
sub-shadow-offset=2
sub-shadow-color="#000000"

but only want it to change the border/shadow if the subtitle doesn't already specify it, what would I use?

EDIT: Currently I'm not using sub-scale and using sub-ass-override=no because I didn't want it to override and previous logic seemed messed up if I wanted scaling.

[llyyr]

what would I use?

Aegisub. These options don't affect ASS subtitles, never have and never will. And sub-ass-override is an option specifically for ASS subtitles.

[Jules-A]

Aegisub. These options don't affect ASS subtitles, never have and never will. And sub-ass-override is an option specifically for ASS subtitles.

So there's no way without converting them first? Since even if I can get a similar affect in sub-ass-style-overrides it will always override anyway?

[low-batt]

I pulled this PR and rebuilt mpv. I tested all 5 sub-ass-override settings and only scale, force and strip applied sub-scale as expected. I also tested the updated u key binding and that cycled between scale and force as expected. And I looked over the updated documentation for sub-ass-override.

The changes look good to me.

[low-batt]

As you can see I spoke too soon. The values in the doc for secondary-sub-ass-override are still in the old order:

--secondary-sub-ass-override=<yes|no|force|scale|strip>