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

Fix default subtitle track selection #1604

Merged
merged 6 commits into from
Jan 6, 2024
Merged

Fix default subtitle track selection #1604

merged 6 commits into from
Jan 6, 2024

Conversation

1hitsong
Copy link
Member

@1hitsong 1hitsong commented Dec 30, 2023

Changes

Add code to determine which subtitle to select and enable by default - without the user needing to manually select it.

Several functions were copied from subtitles.bs. Once we're fully on the new global queue player, this file should no longer be needed.

Modify selection logic to be:

Default: IsForced, IsDefault

Always: IsForced, IsDefault, isExternal

OnForced: IsForced

Smart: 
if audio languge <> preferred subtitle language
	Search for match where subtitle.language = preferred subtitle language
		IsForced, IsDefault, IsExternal
	if no match found, fallback to default methodology
		IsForced, IsDefault

Issues

Fixes #1571
Fixes #850

@1hitsong 1hitsong added the bug-fix This fixes a bug. label Dec 30, 2023
@1hitsong 1hitsong requested a review from a team as a code owner December 30, 2023 00:26
Copy link
Member

@cewert cewert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes #1571. Is there anything else that needs tested with the copied functions?

@1hitsong
Copy link
Member Author

1hitsong commented Jan 2, 2024

The big things are ensuring the selection paths work as expected, and that the language check works.

I had to update the language selection code because it was not selecting any subtitle by default because my SubtitleLanguagePreference is set to "Any Language" on the server, which equals "" in code. This caused the language check to never pass.

image

This was referenced Jan 2, 2024
@cewert cewert merged commit 2640369 into jellyfin:2.0.z Jan 6, 2024
11 checks passed
@1hitsong 1hitsong deleted the fixDefaultSubtitleTrack branch January 6, 2024 01:27
@lakerssuperman
Copy link

I just tried this and it didn't work unfortunately. Watching a movie with a forced subtitled track results in the following:

  1. The forced track is shown to be selected in the subtitle selection dialog.
  2. The forced subtitles are not displayed during playback.

If I manually toggle subtitles off and then back on it will display the subtitles. However, if I then go to another movie with standard English not forced subtitles those will be displayed as well despite the subtitle selection dialog indicating subtitles are off. Clicking None, even if it is already selected and then clicking ok turns subtitles back off.

@1hitsong
Copy link
Member Author

1hitsong commented Jan 8, 2024

Can you post a screenshot of your subtitle settings on the server? Specifically the 1st 3 items, Preferred subtitle language, Subtitle mode, and Burn subtitles.

@lakerssuperman
Copy link

Can you post a screenshot of your subtitle settings on the server? Specifically the 1st 3 items, Preferred subtitle language, Subtitle mode, and Burn subtitles.

Screenshot from 2024-01-08 08-59-48

Let me know if you need anything else! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix This fixes a bug.
Projects
Development

Successfully merging this pull request may close these issues.

3 participants