mpv sub: add --(sub/osd)-border-style & refactoring option naming to match ASS spec

that draw background that tightly wraps each line of text.

Fixes #14194

May 21 '24 03:05 ruihe774

Download the artifacts for this pull request:

Windows

macOS

May 21 '24 03:05 github-actions[bot]

@KonoVitoDa plz test it.

May 21 '24 03:05 ruihe774

Works perfectly, thank you very much!

May 21 '24 03:05 KonoVitoDa

close https://github.com/mpv-player/mpv/issues/10464 instead

May 21 '24 13:05 hooke007

Can we also switch to tight by default?

May 21 '24 16:05 llyyr

Can we also switch to tight by default?

What's the point to make a breaking change? IMO this option is only related to personal preference. Users can easily set this option by themselves.

May 21 '24 17:05 ruihe774

How do I get the updated version with the new feature implemented?

May 21 '24 18:05 KonoVitoDa

How do I get the updated version with the new feature implemented?

Before this is merged, you can get the build artifacts from https://github.com/mpv-player/mpv/actions/workflows/build.yml. Keep in mind you need to find build artifacts of correct PR.

After this is merged, you can get it from unofficial nightly builds https://mpv.io/installation/

May 21 '24 19:05 ruihe774

What's the point to make a breaking change? IMO this option is only related to personal preference. Users can easily set this option by themselves.

Users can change many options by themselves, but there's still value in the default value being something that doesn't need to be changed for most users. I'd argue the current bounding box is not wanted by anyone.

May 21 '24 19:05 llyyr

I'd argue the current bounding box is not wanted by anyone.

Patches welcome. Please contain default changes and associated bikeshedding to a separate PR.

May 21 '24 20:05 na-na-hi

What's the point to make a breaking change? IMO this option is only related to personal preference. Users can easily set this option by themselves.

Users can change many options by themselves, but there's still value in the default value being something that doesn't need to be changed for most users. I'd argue the current bounding box is not wanted by anyone.

Worth noting that BorderStyle=3 has suboptimal behavior for non-opaque colors. e.g.:

I guess it was the reason mpv chose 4 as the default.

May 22 '24 04:05 ruihe774

Will this update also change the way --sub-back-color is selected? Because on my mpv.config it's set to #000000 (black), but in this specific part it seems to be using the sub's original border color: Erai-raws Sousou no Frieren - 01 1080p 81106F0B mkv_snapshot_01 31 425 【MPV】【LINES】 (Portuguese(Brazil))

Erai-raws Sousou no Frieren - 01 1080p 81106F0B mkv_snapshot_01 31 466 【MPV】【LINES】 (Portuguese(Brazil))

May 22 '24 05:05 KonoVitoDa

Will this update also change the way --sub-back-color is selected? Because on my mpv.config it's set to #000000 (black), but in this specific part it seems to be using the sub's original border color:

The behavior did not change. It only applies to SRT subtitles. If you want to override ASS subtitles that select a BackColor itself , you may use --sub-ass-style-overrides.

May 22 '24 06:05 ruihe774

ASS subtitles that select a BackColor itself

But this same subtitle is shown with the black back color in the old version, so it seems to be something related to the new feature: Erai-raws Sousou no Frieren - 01 1080p 81106F0B mkv_snapshot_01 31 508 【MPV】【LINES】 (Portuguese(Brazil))

May 22 '24 07:05 KonoVitoDa

ASS subtitles that select a BackColor itself

But this same subtitle is shown with the black back color in the old version, so it seems to be something related to the new feature:

Please attach your log file and subtitle files (you can extract it using ffmpeg) so that I can look into it.

May 22 '24 07:05 ruihe774

Please attach your log file and subtitle files (you can extract it using ffmpeg) so that I can look into it.

output-new.txt output-old.txt [Erai-raws] Sousou no Frieren - 01 [1080p][81106F0B]_Subtitles02.POR.zip

The video file is the 01 from here: https:// nyaa . si/view/1752250

May 22 '24 08:05 KonoVitoDa

I cannot reproduce it. With the same commit of mpv and same set of subtitle related options in your output-old.txt, the BackColor and BorderStyle are also selected by ASS subtitle file itself.

Screenshot: Screenshot from 2024-05-22 16-43-43

My log: mpv.log

My options:

[   0.001][v][cplayer] Setting option 'config' = 'no' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-files-append' = 'Erai-raws.Sousou.no.Frieren.-.01.1080p.81106F0B._Subtitles02.POR/[Erai-raws] Sousou no Frieren - 01 [1080p][81106F0B]_Subtitles02.POR.ass' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-back-color' = '#000000' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-border-color' = '#000000' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-ass-vsfilter-aspect-compat' = 'yes' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-scale-with-window' = 'no' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-ass-scale-with-window' = 'yes' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-font-size' = '53' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-pos' = '100' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-margin-x' = '25' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-margin-y' = '22' (flags = 8)
[   0.001][v][cplayer] Setting option 'sub-use-margins' = '' (flags = 8)
[   0.001][v][cplayer] Setting option 'osd-font-size' = '25' (flags = 8)
[   0.001][v][cplayer] Setting option 'osd-color' = '#FFFFFFFF' (flags = 8)
[   0.001][v][cplayer] Setting option 'osd-back-color' = '#000000' (flags = 8)
[   0.001][v][cplayer] Setting option 'log-file' = '/var/data/mpv.log' (flags = 8)

May 22 '24 08:05 ruihe774

Really weird. Is not it related to the used FFmpeg version? Btw, I don't know why the FFmpeg version on my output-old.txt is shown as N-115357-g0c1304ae1 while I have ffmpeg version 2024-05-20-git-127ded5078-full_build-www.gyan.dev on my PATH. mpv's updater also says FFmpeg doesn't exist (I downloaded my build from here).

May 22 '24 09:05 KonoVitoDa

Btw, I don't know why the FFmpeg version on my output-old.txt is shown as N-115357-g0c1304ae1 while I have ffmpeg version 2024-05-20-git-127ded5078-full_build-www.gyan.dev on my PATH.

mpv's CI statically links with ffmpeg. It's not overridable. Many unofficial builds also do that.

Really weird. Is not it related to the used FFmpeg version?

I don't think it is related to ffmpeg version. I think it is related to your scripts. You may try --no-config to disable them and have a check.

Anyway, imo --sub-back-color is intended to work with only SRT (and other plain text) subtitles that does not define styles. @na-na-hi I wonder whether I have a correct understanding.

May 22 '24 14:05 ruihe774

It's documented that most subtitle options don't affect ASS subtitles. --sub-ass-* options exist for that purpose.

May 22 '24 14:05 na-na-hi

Oh, I was sleepy and forgot to mention that I use this keybind to switch between sub's original style and my own style: d cycle-values sub-ass-override "force" "no"

Both of the screenshots I shared before were with sub-ass-override set to force, but I don't understand why on mpv-x86_64-20240522-git-0dd6321 it made the back color black while on the artifact version mpv-x86_64-w64-mingw32 it used original sub's border color.

May 22 '24 17:05 KonoVitoDa

Oh, I was sleepy and forgot to mention that I use this keybind to switch between sub's original style and my own style: d cycle-values sub-ass-override "force" "no"

Both of the screenshots I shared before were with sub-ass-override set to force, but I don't understand why on mpv-x86_64-20240522-git-0dd6321 it made the back color black while on the artifact version mpv-x86_64-w64-mingw32 it used original sub's border color.

Reproduced with --sub-ass-override=force.

Screenshot from 2024-05-23 01-35-51

May 22 '24 17:05 ruihe774

Well, if this only occurs with specific subtitle lines (this one is a typesetting), so I think it's not a big problem. Actually it makes sense to have a totally grey sub-back-color instead of a black sub-back-color with a grey sub-border over it.

May 22 '24 19:05 KonoVitoDa

Actually it makes sense to have a totally grey sub-back-color than a black sub-back-color with a grey sub-border over it.

But it would be great if this behavior could also be customized by user.

May 22 '24 19:05 KonoVitoDa

It is related to the selection of different colors in ASS. BorderStyle=3 will use OutlineColor and BorderStyle=4 will use BackColor. You can use --sub-border-color to customize OutlineColor. I'm still refactoring related options to make it more consistent (e.g. renaming --sub-border-color to --sub-outline-color to match the upstream ASS spec.)

May 22 '24 19:05 ruihe774

The problem is actually with how BorderStyle = 4 is implemented in libass. Normally, subtitle rendering works like this:

The text shadow is drawn with BackColour if the shadow is drawn (has non-zero offset). It respects the BorderStyle setting, so if it's set to 3, the rectangles are drawn.
The text border is drawn with OutlineColour if the border is drawn (has non-zero size). It respects the BorderStyle setting, so if it's set to 3, the rectangles are drawn.
The text glyph is drawn.

When BorderStyle = 4 was bolted on libass in https://github.com/libass/libass/commit/4648a02281ff0860cb06e3f56df9095b8a114294, step 1 is replaced as the following if BorderStyle = 4:

The background rectangle is drawn with BackColour.

Note that it disrespects the shadow offset, so if the offset is zero, the background is still drawn.

For the sample file, when the text border is drawn, it respects the BorderStyle setting and draws the background with the OutlineColour. Even if the text has a non-zero shadow offset, its color is overwritten by the later drawn border color. AFAIK this is expected ASS behavior, while the nonstandard BorderStyle = 4 disregards that. See https://github.com/libass/libass/pull/105#issuecomment-44789530.

BorderStyle=3 will use OutlineColor and BorderStyle=4 will use BackColor.

This is not true, see above. BorderStyle = 3 uses both colors. It's just the shadow color isn't visible when the shadow offset is zero and thus not drawn.

May 22 '24 20:05 na-na-hi

For the sample file, when the text border is drawn, it respects the BorderStyle setting and draws the background with the OutlineColour. Even if the text has a non-zero shadow offset, its color is overwritten by the later drawn border color. AFAIK this is expected ASS behavior, while the nonstandard BorderStyle = 4 disregards that. See https://github.com/libass/libass/pull/105#issuecomment-44789530.

BorderStyle=3 will use OutlineColor and BorderStyle=4 will use BackColor.

This is not true, see above. BorderStyle = 3 uses both colors. It's just the shadow color isn't visible when the shadow offset is zero and thus not drawn.

Yeah. OutlineColor, BackColor, and BorderStyle are orthogonal options.

May 22 '24 20:05 ruihe774

I've made some breaking changes to option naming to match ASS spec. Some option "dispatching" logic has been removed. IMO now it's more simple and intuitive.

May 23 '24 05:05 ruihe774

There is no benefit of the border -> outline renaming change. Just add some clarification in the documentation instead. Please revert.

May 23 '24 09:05 na-na-hi

There is no benefit of the border -> outline renaming change. Just add some clarification in the documentation instead. Please revert.

I do not agree.

It matches the naming in specs.
BorderStyle=4 does not use --sub-border-color, which leads to confusion.
It is OPT_REPLACED. Existing usages are not broken.
--(sub/osd)-shadow-color is removed. It is already a breaking change. No meaning to continue using the old names for compatibility reasons.

May 23 '24 09:05 ruihe774

mpv mpv copied to clipboard

sub: add --(sub/osd)-border-style & refactoring option naming to match ASS spec

mpv
mpv copied to clipboard