-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tweaks for --subs-with-matching-audio #15854
base: master
Are you sure you want to change the base?
Conversation
A user got bit by this recently and I had to check the implementation details to remember the behavior, so let's change it to be more intuitive. Previously, --subs-with-matching-audio would *only* match against the list of preferred subtitle languages selected with --slang. If that was unset, it did nothing, as vaguely documented, but this is probably unexpected. We keep the --slang matching if applicable, but otherwise check to see if the subtitle track and audio track languages match when going through the --subs-with-matching-audio logic. This prevent users from having to do something like --slang=en --subs-with-matching-audio=no which they probably don't expect.
Old description was misleading in a few ways.
This got refactored so it only loads one file at a time and not everything in a loop. The option resetting at the end of conditional branches is pointless.
Previously only the --slang case existed.
ab1907b
to
f150ea8
Compare
Download the artifacts for this pull request: |
stream matches a preferred subtitle language selected with ``--slang`` | ||
(default: yes). If no ``--slang`` is explicitly set, then this option will | ||
instead check for a match between the audio stream and subtitle track. If | ||
this option is set to ``no``, then no subtitle track that matches the audio |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand. What is the difference between --slang
cases and why it matters? I think how subtitle track is selected is outside the scope of this function. If it matches audio track it should be unselected, unless =yes or =forced and track is forced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of this option is to match against the list of languages you have in --slang
. The idea being that you might have multiple languages where you do not want subtitles if it matches the audio language. This is how it currently behaves. This PR adds a special case --slang
being empty where it simply matches the subtitle language and audio language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the purpose to match against audio track always? What is the difference if --slang
matches subtitle language? If subtitle language matches audio it should work the same way regardless of --slang
option.
i.e. subtitle track has been selected. Now depending on this option it can be unselected or not, if it matches audio. How subtitle were selected doesn't matter, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If subtitle language matches audio it should work the same way regardless of --slang option.
It won't. Consider --alang=jpn
and --slang=eng
. If the subtitle track is jpn
, --subs-with-matching-audio=no
is ignored in this case because jpn is not in the user's list of preferred languages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, obviously it is ignored, because the jpn
track wouldn't be selected anyway, regardless of any audio language or audio related option.
When autoselecting a subtitle track, select it even if the selected audio stream matches...
...audio track.
And that's all there is to it. If subtitle would be selected, check if it matches audio track and select or not. There is no need to match slang, because as you already noticed empty slang has to work the same way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK sorry obviously dumb example. Consider this: jpn audio and --slang=eng,jpn and multiple subtitle tracks in that file (english and japanese). With --subs-with-matching-audio=no
, no subtitles are selected because it is all matched to --slang
. If you only match the audio stream's language, then eng
would be selected. That's the difference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you only match the audio stream's language, then eng would be selected.
And this is what I would expect. But fair enough, I understand the idea of not selecting any subtitles if any language matches, even if the priority is lower.
The idea is if you can read subtitles you can also understand audio. Thinking about it, I would match this "against" alang
in fact. If preferred audio has been found, don't use subtitles. So considering alang=eng,jpn
and we have audio match, we can skip all subtitles. I think this convey original intent of Disable this if you'd like to only show subtitles for foreign audio or onscreen text.
better.
"Don't use subtitles if preferred audio is available." instead "Don't use subtitles if any possible subtitle language matches audio".
I hope you see my point. There is kind of symmetry in this, because people most of the time would have similar slang and alang.
But I think it is backward :) Imagine having --alang=pol
and --slang=eng
, it will still select eng subtitles, regardless of this option. While clearly pol
is not foreign language. And sure this is fixable, with --slang=eng,pol
, but then I never want pol
subtitles, regardless of anything. And this fails.
EDIT:
Honestly that's why I don't use autoselection. Because it does things that are sometimes weird.
track. Note that this option acts independently of ``--subs-fallback`` and it is possible that | ||
a track that is not selected by this option will get selected anyway because of | ||
``--subs-fallback``. E.g. a track that is tagged as default + forced would be selected with | ||
``--subs-fallback=default`` and ``--subs-fallback-forced=no``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I don't see how can we say that those options work "independently".
I understand that --subs-fallback-forced=no
, can still select default track or other track when --subs-fallback=yes
or --subs-fallback=default
But if --subs-fallback-forced=always
it should fallback only to forced and never to anything else.
I think both of those options are tightly connected. I would even say it could be one option with more choices.
I think the main use case is too select only forced tracks, for matching languages, instead of default track.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if --subs-fallback-forced=always it should fallback only to forced and never to anything else.
That's not how it works nor how it ever worked though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it was always broken, you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No there's just multiple fallbacks. --subs-fallback
is one. --subs-fallback-forced
is another one. If there's no forced subtitle track to choose from, then --subs-fallback-forced
won't have anything to fallback to. But that doesn't prevent --subs-fallback
from selecting something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are too many layers to something simple as selecting forced subtitle track and we step on our own feet by changing it back and forth.
Look at the initial description of this option 991a2f7:
When autoselecting a subtitle track, if no tracks match your preferred languages, select a forced track that matches the language of the selected audio track (default: yes).
Fallback logic that makes perfect sense, if we don't have any subtitles selected, fallback to forced track matching audio.
Now, current docs, without this PR, says:
When autoselecting a subtitle track, the default value of yes will prefer using a forced subtitle track if the subtitle language matches the audio language and matches your list of preferred languages.
Which is ??? If it matches your preferred languages, than it is already selected and if not. Option like that should be called prefer-forced-track
, this is not a fallback. It was fallback originally.
And now this PR adds even more confusing language on top. Don't get me wrong, I get it, but this is hardly possible to understand for average joe, without looking at the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to something simple as selecting forced subtitle track
There's nothing simple about it. People want a gazillion options for this.
Which is ??? If it matches your preferred languages, than it is already selected and if not.
There's multiple tracks that can potentially match your preferred language.
And now this PR adds even more confusing language on top.
Well i don't know what to do. The old docs lie and this seems more accurate to me but you seem confused about it anyway so I guess it's no good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I have to read 5 times the description of the option and on top of that the name of the option seems to imply something else, I'm indeed confused. I don't think adding more details to docs make it more understandable, it make it more correct with is a good thing though.
I'm just thinking at loud, no need to change this. But for me this option would make more intuitive sense (for me at least)
--sub-forced=<prefer|ignore|force>
- prefer - prefers forced subs if matches audio lang and slang
- ignore - never select forced subs
- force - always prefer forced subs, even if not matching audio language
but then there is also relation to subs-fallback
.
My biggest problem is that it it unclear to me, when default
subtitles would be selected based on subs-fallback
and when forced
.
Simply speaking the "independently". There must be some order in the selection, sub-fallback takes precedence over sub-fallback-forced or something like that. I don't know how it works still ;p Maybe would be easier if it were one option. Anyway, not sure you get my point of view.
No description provided.