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: fix preferred track selection on Safari #5601

Merged

Conversation

tykus160
Copy link
Contributor

@tykus160 tykus160 commented Sep 5, 2023

Preferred track selection was implemented differently for MSE and native playback. In fact, native playback even had different implementations for audio and text. It leads to inconsistencies during track selection, i.e. if track language contains locale, but language preference not, on MSE we're looking for closest locale and on src= we're making direct string comparison which leads to different results.
To unify that, both MSE and native will now use StreamUtils.filterStreamsByLanguageAndRole() to find matching track. This method is designed to use on shaka.extern.Stream but luckily it uses the very same fields as defined in shaka.extern.Track so we can use it without major changes.
Moreover, using this more robust method also allows us to get rid of double-selection workaround used so far. Observe that we were first selecting track without preferred role and then with preferred role.

@shaka-bot
Copy link
Collaborator

Incremental code coverage: 58.82%

@avelad avelad added type: bug Something isn't working correctly priority: P2 Smaller impact or easy workaround labels Sep 5, 2023
@avelad avelad added this to the v4.5 milestone Sep 5, 2023
@joeyparrish
Copy link
Member

Well done, and thank you!

@joeyparrish joeyparrish merged commit d021d6f into shaka-project:main Sep 5, 2023
20 checks passed
joeyparrish pushed a commit that referenced this pull request Sep 5, 2023
Preferred track selection was implemented differently for MSE and native
playback. In fact, native playback even had different implementations
for audio and text. It leads to inconsistencies during track selection,
i.e. if track language contains locale, but language preference not, on
MSE we're looking for closest locale and on src= we're making direct
string comparison which leads to different results.
To unify that, both MSE and native will now use
`StreamUtils.filterStreamsByLanguageAndRole()` to find matching track.
This method is designed to use on `shaka.extern.Stream` but luckily it
uses the very same fields as defined in `shaka.extern.Track` so we can
use it without major changes.
Moreover, using this more robust method also allows us to get rid of
double-selection workaround used so far. Observe that we were first
selecting track without preferred role and then with preferred role.
joeyparrish pushed a commit that referenced this pull request Sep 5, 2023
Preferred track selection was implemented differently for MSE and native
playback. In fact, native playback even had different implementations
for audio and text. It leads to inconsistencies during track selection,
i.e. if track language contains locale, but language preference not, on
MSE we're looking for closest locale and on src= we're making direct
string comparison which leads to different results.
To unify that, both MSE and native will now use
`StreamUtils.filterStreamsByLanguageAndRole()` to find matching track.
This method is designed to use on `shaka.extern.Stream` but luckily it
uses the very same fields as defined in `shaka.extern.Track` so we can
use it without major changes.
Moreover, using this more robust method also allows us to get rid of
double-selection workaround used so far. Observe that we were first
selecting track without preferred role and then with preferred role.

Backported to v4.3.x
@tykus160 tykus160 deleted the wt-safari-pref-track-fix branch September 6, 2023 06:15
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Nov 4, 2023
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Nov 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants