Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Dudemanguy
Copy link
Member

No description provided.

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.
Copy link

Download the artifacts for this pull request:

Windows
macOS

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
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

@kasper93 kasper93 Feb 13, 2025

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?

Copy link
Member Author

@Dudemanguy Dudemanguy Feb 13, 2025

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.

Copy link
Contributor

@kasper93 kasper93 Feb 13, 2025

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.

Copy link
Member Author

@Dudemanguy Dudemanguy Feb 13, 2025

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.

Copy link
Contributor

@kasper93 kasper93 Feb 13, 2025

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``.
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants