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

Next episode has mismatched language between server and ui (rebased) #5998

Closed

Conversation

Kinsteen
Copy link

Simple rebase on new master to remove merge conflicts of #4852. Didn't test yet, would be happy if people could!

Root cause

The autoset options only seem to take effect on non-transcoded media-- the transcode URL is never updated! This means that if the selected audio does not match the default, the audio/subs are in the default but the UI reflects the auto-set variant (showing a language that isn't playing).

Changes

When setting the track in the the auto-set logic, also trivially change the audio index in the transcode URL to match.

This fixes it in most cases, but there may be some edge cases that need to go through the full validation to recreate the URL. I don't know the code well enough for that, though.

Issues

Fixes #3994

jmerdich and others added 6 commits August 30, 2024 14:25
Addresses jellyfin-web#3994

The autoset options only take effect on non-transcoded media-- the
transcode URL is never updated! This means that if the selected
audio does not match the default, the audio/subs are in the default
but the UI reflects the auto-set variant (showing the wrong track).

When doing the auto-set logic, also trivially change the audio index in
the transcode URL to match.

This fixes it in most cases, but might have some edge cases that need
to go through the full validation to recreate the URL. I don't know the
code well enough for that, though.
Note other existing suppressions for the 'URLSearchParams' compat
Note this makes the URLs absolute.
@Kinsteen Kinsteen requested a review from a team as a code owner August 30, 2024 12:33
@Kinsteen Kinsteen changed the title Jmerdich/next ep wrong language Next episode has mismatched language between server and ui (rebased) Aug 30, 2024
Copy link

@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit 13e3b795a8ca9bcf65b27e13c9923bf21665a5ff
Status ✅ Deployed!
Preview URL https://8c59d236.jellyfin-web.pages.dev
Type 🔀 Preview

@viown
Copy link
Member

viown commented Sep 5, 2024

Was able to reproduce the mentioned issue, and after testing this, it does seem to fix it.

@dmitrylyzo
Copy link
Contributor

In addition to #4852 (comment)

Changing streams after receiving PlaybackInfo may result in unwanted remuxing/transcoding:

  • The "best" subtitles may not require transcoding.
  • The "best" audio may not require remuxing. <- less possible

It's better to do this on the server side so that every client gets this auto-select feature.

But speaking of web, we should do ranking and update corresponding fields before getting PlaybackInfo:

audioStreamIndex,
subtitleStreamIndex,

@viown
Copy link
Member

viown commented Sep 23, 2024

In addition to #4852 (comment)

Changing streams after receiving PlaybackInfo may result in unwanted remuxing/transcoding:

* The "best" subtitles may not require transcoding.

* The "best" audio may not require remuxing. _<- less possible_

It's better to do this on the server side so that every client gets this auto-select feature.

But speaking of web, we should do ranking and update corresponding fields before getting PlaybackInfo:

audioStreamIndex,
subtitleStreamIndex,

Setting the audioStreamIndex/subtitleStreamIndex in options does not seem to affect the stream URL in some scenarios (only when remuxing/direct playing). Any idea why that is?

@dmitrylyzo
Copy link
Contributor

dmitrylyzo commented Sep 23, 2024

Setting the audioStreamIndex/subtitleStreamIndex in options does not seem to affect the stream URL in some scenarios (only when remuxing/direct playing). Any idea why that is?

Because they are processed on the client side when DirectPlay. It should use mediaSource.DefaultAudioStreamIndex and mediaSource.DefaultSubtitleStreamIndex.

@jellyfin-bot
Copy link
Collaborator

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@ebkalderon
Copy link

It seems that #6112 has been merged, so I believe this might be safe to close?

@Kinsteen
Copy link
Author

Seems so, closing

@Kinsteen Kinsteen closed this Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge conflict Conflicts prevent merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue]: Audio track reset on next episode
6 participants