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 not turning on when not transcoding, and burnt in subs not turning off #1437

Conversation

mueslimak3r
Copy link
Member

@mueslimak3r mueslimak3r commented Feb 12, 2022

Changes

  • made the play() parameter transcodedSubtitle a nullable Integer so that we can specifiy null to defer to the server default, -1 to disable subs, and >=0 to select burnt in/transcoded subs
  • reworked the logic for determining if a newly started stream is using burnt in subs, and set relevant flags locally so no additional restarts are needed
  • always set mDefaultSubIndex to the server default so that users will always get subs enabled by default if they have such a preference set.

Issues

  • default subtitles for an item wouldn't turn on by default when direct playing
  • turning on burnt in subtitles would start the stream twice. After the first startup, switchSubtitleStream would incorrectly assume that the stream didn't already have the burnt in subs enabled, and restart the stream - this time the necessary flags were set to not require more restarts.
  • burnt in subtitles couldn't be disabled during playback
  • potentially fixes subtitle settings for a previous episode persisting into the next one, and showing the wrong subs

@mueslimak3r mueslimak3r marked this pull request as draft February 12, 2022 01:27
@mueslimak3r mueslimak3r marked this pull request as ready for review February 12, 2022 03:15
@nielsvanvelzen nielsvanvelzen merged commit 694cd02 into jellyfin:master Feb 12, 2022
@mueslimak3r mueslimak3r deleted the default-subtitle-selection-burnin-fixes branch February 12, 2022 12:10
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