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

refactor: Update subtitle streams to use TextTrack indexes #2529

Merged
merged 2 commits into from
Dec 10, 2024

Conversation

EffakT
Copy link
Contributor

@EffakT EffakT commented Dec 4, 2024

Subtitles were using the MediaStream index to select what subtitles to play, rather than the TextTrack index.
This meant that the subtitles were playing incorrectly as it was taking into account the Video/Audio streams when selecting a subtitle.
E.G:
English 1
English 2
English (SDH)
Bulgarian
CZE

When selecting English 1, This plays the English (SDH).
When selecting English 2, This plays Bulgarian.
When selecting English (SDH), This plays CZE.

This is because the MediaStreams are:
Video
Audio
English 1
English 2
etc...

This PR refactors the subtitle selection code to set the Index and srcIndex to be based on the TextStream, instead of the MediaStream.

resolves #2519

@jellyfin-bot
Copy link

jellyfin-bot commented Dec 4, 2024

Cloudflare Pages deployment

Latest commit c9060b3
Status ✅ Deployed!
Preview URL https://0873c1a1.jf-vue.pages.dev
Preview alias https://subtitle-indexes-fix.jf-vue.pages.dev
Type 🔀 Preview

View build logs

@EffakT
Copy link
Contributor Author

EffakT commented Dec 5, 2024

Hmm,

Not sure why but it seems that when you add a External subtitle, and then attempt to play, the subtitles are off by 1...
So, if you have:

  1. English EXT
  2. English 1
  3. English 2
  4. English (SDH)
  5. Bulgarian
  6. CZE

When you select English (SDH)
This plays Bulgarian, However in the JavaScript it is reporting the correct subtitle.

It seems like the Delivery VTT is incorrect. Unsure if this is related to this change, or has always been an issue with external subtitles?

Nevermind, this part is a bug in Jellyfin, as is occurring in the official UI.

@ferferga
Copy link
Member

ferferga commented Dec 9, 2024

@EffakT Thank you very much for yet another contribution :)

So, beside the server bug, does this make Vue behave 1:1 like Jellyfin Web? Is there a ticket opened in server for this? A TODO comment with a reference to the server issue created would be very welcome, so we are aware of this bug until it's properly fixed. Imagine if later on I (or someone else) want to refactor/improve the subtitle logic a little, not remembering this PR will drive me nuts :).

@EffakT
Copy link
Contributor Author

EffakT commented Dec 9, 2024

Sure, will make a issue in Jellyfin Server.
After some conversations on Element, one thought is it may be related to a badly constructed file, as this happens on 2 out of the 3 files I tested.

@EffakT
Copy link
Contributor Author

EffakT commented Dec 9, 2024

From my testing, this now behaves 1:1 like Jellyfin Web.

Have made a issue in Jellyfin Server
jellyfin/jellyfin#13198

I've put the TODO comment into the currentItemParsedSubtitleTracks, as this is where you are most likely to see the comment when refactoring the subtitle code.

@ferferga ferferga force-pushed the subtitle-indexes-fix branch from d2c9b82 to c9060b3 Compare December 10, 2024 06:27
Copy link

@ferferga ferferga merged commit 297b4fe into jellyfin:master Dec 10, 2024
17 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Subtitle selector is playing incorrect subtitles
3 participants