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

Add secondary-sid support #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vhagedorn
Copy link

Add support for secondary-sid option in mpv via dlang field. Field is functionally identical to slang field except that it is optional.

@CogentRedTester
Copy link
Owner

Sorry for the delayed response, I was on holiday. If I have not reviewed this PR in a weeks time please ping this thread to remind me.

@zhongfly
Copy link

zhongfly commented May 8, 2024

Sorry for the delayed response, I was on holiday. If I have not reviewed this PR in a weeks time please ping this thread to remind me.

ping

@CogentRedTester
Copy link
Owner

CogentRedTester commented May 15, 2024

I have a number of problems with the current form of this PR, but before I go in to them there is a specific question that needs to be answered: how does the secondary subtitle selection and the normal subtitle selection influence each other.

The normal subtitle selection code is based on the currently selected audio. If you have audio-mode enabled, then the selection of both audio and subtitles depend on each other; a pair of audio and subtitle tracks must pass a match for them both to be selected.

How does the secondary subtitle option fit into this? Is it completely independent from the normal subtitles where it gets selected separately? Should the primary subtitles be selected first and the secondary subtitles on be selected afterwards based on that selection? Would both the primary and secondary subtitles need to match to pass a specific preference? This question needs to be answered and justified before this PR can be properly reviewed.

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.

None yet

3 participants