Sub ass style overrides always applied
Opened this issue · 8 comments
System and IINA version:
- macOS 13.6.3
- IINA 1.3.4
Expected behavior:
When the setting Ignore ASS styles
is not enabled style overrides are not applied.
Actual behavior:
When the setting Ignore ASS styles
is not enabled IINA sets the mpv sub-ass-override option to yes
causing style overrides to be applied. From the mpv manual entry for the sub-ass-override option:
--sub-ass-override=<yes|no|force|scale|strip>
Control whether user style overrides should be applied. Note that all of these overrides try to be somewhat smart about figuring out whether or not a subtitle is considered a "sign".
- no: Render subtitles as specified by the subtitle scripts, without overrides.
- yes: Apply all the
--sub-ass-*
style override options. Changing the default for any of these options can lead to incorrect subtitle rendering (default).- force: Like yes, but also force all
--sub-*
options. Can break rendering easily.- scale: Like yes, but also apply
--sub-scale
.- strip: Radically strip all ASS tags and styles from the subtitle. This is equivalent to the old
--no-ass
/--no-sub-ass
options.This also controls some bitmap subtitle overrides, as well as HTML tags in formats like SRT, despite the name of the option.
When the setting Ignore ASS styles
is not enabled IINA should be setting sub-ass-override
to no
.
Steps to reproduce:
- Start IINA
- Click on
Settings…
under theIINA
menu - The settings panel appears
- On the left side of the panel click on
Subtitle
- In the
ASS Subtitles
section uncheck theIgnore ASS styles
setting - Move the thumb of the
Override level
slider all the way to the left - On the left side of the panel click on
Advanced
- Slide the
Enable advanced settings
toggle button to be on (blue) - In the
Additional mpv options
section click on+
- A new entry appears in the table
- Double click on
name
and replace it withsub-ass-style-overrides
- Double click on
value
and replace it withFontName=Wingdings
- Confirm the panel looks like the screenshots below
- Restart IINA to activate the new settings
- Start a video playing with the attached subtitle file
- Even though
Ignore ASS styles
is not enabled the style override will be applied and the subtitles will look like the screenshot below
With Ignore ASS styles
disabled the styles in the subtitle file were still overridden:
The subtitle file used for testing (with .txt
added to make GitHub happy):
big-buck-bunny.ass.txt
- MPV does not have this problem.
How often does this happen?
Every time.
Does this mean when ignoreAssStyles
is false (which is the default), all ASS styles are override by mpv? That's a huge problem, but why no one raised an issue for that?
Likely because It is being set to yes
:
- yes: Apply all the
--sub-ass-*
style override options.
And IINA does not provide access to the --sub-ass-*
options. They give you a way to set styles specifically for the ASS subtitles and not change text subtitles.
However when working on this it seemed like mpv
was incorrectly applying sub-scale
with sub-ass-override
set to yes
. I noticed this when working on another problem with this setting, namely it does not provide the ability to set sub-ass-override
to scale
. I started working on that and have some changes, but as that would be for the feature release I put that work on the side and went back to RTL issues. I still need to enter an issue for the missing scale
support and possibly a mpv
issue as well. I will try and get back to that soon.
We need to check in with @lhc70000 as he may want this fix to be held until the feature release beta as if mpv
is applying sub-scale
then this fix could cause a change in behavior.
I noticed this when checking Hebrew translations and noticed the text showing the setting was not localized. That caused me to take a close look at this feature.
This issue shows why it is desirable to merge PR #4926. When setting mpv
properties and executing mpv
commands are logged it is possible to notice such problems.
It was set to yes
at the beginning, probably because yes
was the default value. If the user opens mpv with no options, its value will be yes
. However, if we merge #4928 and the user opens IINA without changing any settings, its value will be no
, causing a discrepancy between IINA's behavior and mpv's.
I think when implementing this I was trying to minimize such discrepancies. When disabled, it's the default mpv behavior; when enabled, it's set to strip
so all ass styles are controllable by using the options in the preference panel (so the user won't be confused by the different override levels).
I agree that it's an issue to set it to yes
when it explicitly says "disabled". If we want to be consistent, we should set the default to ignoreAssStyles: true
and subOverrideLevel: yes
. However, in this case, the user may not immediately know what to do if they want to override ASS styles (they need to drag the slider to the right vs. clicking the checkbox in previous versions).
With the default yes
value, it's also handy to scale the subtitle using the slider in the sidebar (although this may be considered as incorrect mpv behavior).
So, I'm not sure whether it's a bug because the help text says
If enabled, all ASS subtitles will be drawn using the styles below.
which should be true (right?)
So we have two choices
- Apply the fix in #4928, but I strongly recommend changing the default to
ignoreAssStyles: true
andsubOverrideLevel: yes
, otherwise the scale slider in the sidebar still stop working, which may surprise some users. However, users may find it difficult to enable ASS style overriding. - Leave it as-is.
If enabled, all ASS subtitles will be drawn using the styles below.
This is incorrect. There are multiple problems. IINA is not aligned with the mpv
functionality, if I'm understanding the mpv
behavior.
mpv
provides options such as sub-ass-scale-with-window
--sub-ass-scale-with-window=<yes|no>
Like--sub-scale-with-window
, but affects subtitles in ASS format only.
This allows a user to specify styles specifically for ASS subtitles. This matches up with --sub-ass-override=yes
:
- yes: Apply all the
--sub-ass-*
style override options.
For the settings IINA provides --sub-ass-override
must be set at least to force
:
- force: Like yes, but also force all
--sub-*
options
I'm pretty sure this:
With the default yes value, it's also handy to scale the subtitle using the slider in the sidebar (although this may be considered as incorrect mpv behavior).
Is a result of a mpv
defect. Caught me preparing to file an issue about it. If --sub-scale
is applied with --sub-ass-override
set to yes
then what does setting it to scale
do?:
- scale: Like yes, but also apply
--sub-scale
.
I started on changes to add scale
which IINA is missing, but wanted to check with mpv
as to why this feature doesn't seem to be working as documented.
Because sub-scale
is being applied with --sub-ass-override
set to yes
it is a problem that IINA users are unable to set --sub-ass-override
set to no
if they want scaling to only apply to text subtitles.
Using a checkbox and a slider is all fine. But currently styles are being overriden with the checkbox unchecked. That is not how this mpv
feature should work.
The slider should be:
yes
scale
force
strip
At least I think that is how this mpv
feature should work. I will try and get an mpv
issue posted and we can see if I am confused about how this feature works.
For sure lets not merge this for 1.3.5. Maybe we end up with ignoreAssSytles: true
and subOverrideLevel: scale
as defaults.
I've entered mpv
issue mpv-player/mpv#14229.
I've removed this issue from the 1.3.5 release and changed the PR to be a draft.
Seems like a correct UI representation of IINA's current behavior would not have a toggle and would just be a slider with scale
, force
and strip
.
I'm thinking wait until we hear from mpv
before doing anything more. Could be I'm confused about how this mpv
feature works.
The mpv
issue now has a proposed fix. The updated documentation for sub-ass-override
will look like:
--sub-ass-override=<no|yes|scale|force|strip>
Control whether user style overrides should be applied. Note that all of these overrides try to be somewhat smart about figuring out whether or not a subtitle is considered a "sign".
no: Render subtitles as specified by the subtitle scripts, without overrides.
yes: Apply all the
--sub-ass-*
style override options. Changing the default for any of these options can lead to incorrect subtitle rendering.scale: Like
yes
, but also apply--sub-scale
(default).force: Like
yes
, but also force all--sub-*
options. Can break rendering easily.strip: Radically strip all ASS tags and styles from the subtitle. This is equivalent to the old
--no-ass
/--no-sub-ass
options.This also controls some bitmap subtitle overrides, as well as HTML tags informats like SRT, despite the name of the option.
And of course there is now:
--secondary-sub-ass-override<yes|no|force|scale|strip>
Control whether user secondary substyle overrides should be applied. This works exactly like--sub-ass-override
.Default: strip.
And the mpv
key binding for u
is changing from cycle-values sub-ass-override "force" "yes"
to cycle-values sub-ass-override "force" "scale"
.
Thus mpv
is changing the default for sub-ass-override
from yes
to scale
. And correcting the behavior so that yes
does not apply sub-scale
. Therefore, if IINA wants sub-scale
to always be applied then IINA will need to be changed when upgrading to mpv
with the fix.
I'm still thinking IINA should at some point provide a full implementation of this mpv
feature. Other issues are more important at the moment.
Maybe we can remove the checkbox and only use one slider with five levels: no, yes, scale, force, strip. We don't show user the level names to avoid confusion, instead we display some help text that changes with the slider value. For example (of course we need to come up with better descriptions):
||---+---+---+---+
No override: render subtitles as specified by the subtitle scripts.
+---||---+---+---+
Only allow override using the --sub-ass-* mpv options.
+---+---||---+---+
Allow adjusting scale of ASS subtitles and override using the --sub-ass-* mpv options.
+---+---+---||---+
Allow adjusting scale and style of ASS subtitles.
+---+---+---+---||
Ignore all style styles from ASS subtitles and override them by IINA settings.
There is another reason to eliminate the checkbox and just use a slider. We need to add support for secondary-sub-ass-override. I think the UI would be better with just two sliders and no checkboxes.
I very much like the idea of help text that changes with the selection. I'm not in favor of eliminating the level names. Once a user understand what this does the level names are useful. We also could add a help button linked to the mpv
manual entry.
For this to make sense we have to also add support for at least some of the --sub-ass-*
options.
The fix for the mpv
defect I entered has been merged in mpv
master
. If we are fixing this option we probably should patch in that fix that corrects the yes
behavior when building dependencies.