[RFC] options: add --{sub,audio,video}-file-priority options #15855
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
The manual description was not really accurate. It only prevented loading external files in some cases but not always. It also had an undocumented effect on default subtitle selection. Thinking about it though, this option is stupid in the first place. Just set the appropriate --sub-auto, etc. if you don't want external tracks loaded. And also just don't use --external-files and friends in the first place if you don't want external files. The effect on default subtitle selection is desirable but this will be reworked in a new option in the following commits.
Dudemanguy
force-pushed
the
external-track-rewrite
branch
from
51a6089 to
e928b76
Compare
no_default is actually just redundant with no_auto_select and also something that is irrelevant most of the time. no_auto_select was changed in 8ce5e79 to be exactly the same as t->no_default. Furthermore, the select_default_track loop explicitly skips anything marked as t->no_auto_select. So in fact, all of the code checking for t->no_default is totally pointless as it being true could never happen in those places. In command, adding a track as auto also has no relevance to this particular boolean. You just don't switch/mark the track in the first place.
Dudemanguy
force-pushed
the
external-track-rewrite
branch
from
e928b76 to
5912e35
Compare
|
Download the artifacts for this pull request: |
kasper93
reviewed
Feb 13, 2025
| (e.g. slang or various --subs-fallback options). This defaults to ``yes`` | ||
| meaning that external subtitle files have priority over internal subtitles. | ||
| ``no`` removes any special prioritization whereas ``never`` will prefer | ||
| internal tracks. |
Member
There was a problem hiding this comment.
whereas
neverwill prefer internal tracks.
never should more like "never select". This doesn't sound like it.
Member
Author
There was a problem hiding this comment.
I'm not sure. It's more like deprioritize but that sounds weird.
Contributor
There was a problem hiding this comment.
How about just call it inter ...
Member
Author
There was a problem hiding this comment.
Did you typo? Not sure what you meant. I was thinking maybe reverse or invert but that also sounds weird.
Member
There was a problem hiding this comment.
--sub-file-priority=<above|normal|below>
Member
Author
There was a problem hiding this comment.
That works. Let's go with it.
Historically, mpv has always blindly preferred external tracks over internal ones during autoselection. However there are plenty of people out there that load up a ton of tracks at once and don't necessarily want the external one to always be choosen by default. Add some customization by adding a new option for sub, audio, and video files to choose how external tracks should be prioritized if at all. The default for all of these is "true" to match the old historic behavior.
Dudemanguy
force-pushed
the
external-track-rewrite
branch
from
5912e35 to
982026c
Compare
kasper93
reviewed
Feb 16, 2025
| .deprecation_message = "this option is deprecated and does nothing"}, | ||
|
|
||
| {"sub-file-priority", OPT_CHOICE(sub_file_priority, | ||
| {"below", -1}, {"normal", 0}, {"above", 1})}, |
Member
There was a problem hiding this comment.
here it's updated, but not in docs.
kasper93
reviewed
Feb 16, 2025
Comment on lines
+1043
to
+1045
| .sub_file_priority = true, | ||
| .audio_file_priority = true, | ||
| .video_file_priority = true, |
Member
There was a problem hiding this comment.
Suggested change
| .sub_file_priority = true, | |
| .audio_file_priority = true, | |
| .video_file_priority = true, | |
| .sub_file_priority = 1, | |
| .audio_file_priority = 1, | |
| .video_file_priority = 1, |
kasper93
reviewed
Feb 16, 2025
| deprecate `--autoload-files` | ||
| add `--sub-file-priority` | ||
| add `--audio-file-priority` | ||
| add `--video-file-priority` |
Member
There was a problem hiding this comment.
Personally not sure if we need three separate options, it seem like too much fine-grained control. External file is external file. But whatever if you like to have it like that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based off of the branch in #15854 since that touches the same general code places and presumably I'll merge it reasonably soon.
Did some minor internal cleanups and added options to allow customization of whether or not external tracks should be preferred. I kind of feel like the default should be
no. In other words, external tracks are treated exactly like internal tracks and there is no preference either way but the PR currently has it set toyesfor compatibility reasons. Not sure how people feel about that.Obviously needs tests. Will do that later.