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.

Sign up for GitHub

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

Jump to bottom

selectAudioLanguage with a "role" doesn't immediately select correct audio track #948

Closed
bhh1988 opened this issue Jul 26, 2017 · 6 comments
Closed

selectAudioLanguage with a "role" doesn't immediately select correct audio track #948

bhh1988 opened this issue Jul 26, 2017 · 6 comments
Assignees
@joeyparrish
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone
v2.2.0

Comments

@bhh1988
Copy link
Contributor

bhh1988 commented Jul 26, 2017 •
edited
Loading

This bug can be reproduced only if there are audio tracks with the same language, but different roles. Selecting a new audio track is effectively a no-op. Stepping through the code, this is due to a bug in chooseStreams_() in player.js, which only checks for if language has changed between the selected and currently active track, when deciding whether to actually switch tracks or not (it does look like it's setting the correct list of variants for ABR though, since 4012194 for #767 , so I believe ABR would eventually set the desired audio track eventually).
bhh1988 pushed a commit to bhh1988/shaka-player that referenced this issue Jul 26, 2017
Immediately switch audio tracks if different role selected
115c0b1 
Previously, selecting a new audio track with selectAudioLanguage(), with
the same language but different role, would only set the variants to
switch to for ABR, but wouldn't actually switch to any of those variants
at the moment of selection due to a bug in chooseStreams_(), which was
still looking at only audio language as a differentiating mechanism.
This change honors the role as well, so that a switch to the correct
variants immediately happens.

Closes shaka-project#948
@bhh1988 bhh1988 mentioned this issue Jul 26, 2017
Closed
bhh1988 pushed a commit to bhh1988/shaka-player that referenced this issue Jul 26, 2017
Immediately switch audio tracks if different role selected
6958e95 
Previously, selecting a new audio track with selectAudioLanguage(), with
the same language but different role, would only set the variants to
switch to for ABR, but wouldn't actually switch to any of those variants
at the moment of selection due to a bug in chooseStreams_(), which was
still looking at only audio language as a differentiating mechanism.
This change honors the role as well, so that a switch to the correct
variants immediately happens.

Closes shaka-project#948
bhh1988 pushed a commit to bhh1988/shaka-player that referenced this issue Jul 31, 2017
Immediately switch audio/text tracks if different role selected
4fa451f 
Previously, selecting a new audio/text track with selectAudioLanguage()
or selectTextLanguage(), with the same language but different role,
would only set the variants to switch to for ABR, but wouldn't actually
switch to any of those variants at the moment of selection due to a bug
in chooseStreams_(), which was still looking at only audio language as a
differentiating mechanism. This change honors the role as well, so that
a switch to the correct variants immediately happens.

Closes shaka-project#948
@joeyparrish joeyparrish added the type: bug Something isn't working correctly label Aug 4, 2017
@joeyparrish joeyparrish self-assigned this Aug 4, 2017
@joeyparrish joeyparrish added this to the v2.2.0 milestone Aug 4, 2017
@joeyparrish
Copy link
Member

joeyparrish commented Aug 4, 2017 •
edited
Loading

@bhh1988, I believe this may have been fixed in the refactor in 7e0f469. Would you be willing to retest?

@joeyparrish joeyparrish added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Aug 4, 2017
@bhh1988
Copy link
Contributor Author

bhh1988 commented Aug 7, 2017

Yes this seems to be fixed now. Thanks!

@bhh1988 bhh1988 closed this as completed Aug 7, 2017
@bhh1988
Copy link
Contributor Author

bhh1988 commented Aug 13, 2017 •
edited
Loading

I realize this issue still exists in 2.1.6. Makes sense because 7e0f469 isn't on that branch yet. Is there a plan to backport the fix? What's the policy for closing issues under these kinds of circumstances?

@bhh1988 bhh1988 mentioned this issue Aug 13, 2017
Closed
@joeyparrish joeyparrish mentioned this issue Aug 14, 2017
Closed
@joeyparrish
Copy link
Member

I can confirm that this is still an issue on the v2.1.x branch. Confirming that was hard and involved a lot of hacks, which led me to file #967 to add roles to the demo app UI.

I need to determine if backporting that refactor to v2.1.x is feasible.

@joeyparrish
Copy link
Member

Backporting is not trivial, because the refactor touched some external interfaces. We can't change the AbrManager API mid-2.1.x.

I'm looking into other solutions, including a partial reimplementation.

joeyparrish added a commit that referenced this issue Aug 14, 2017
@joeyparrish
Fix audio track selection on role change
99eacb0 
A one-off implementation for v2.1.x.

This was solved for v2.2.x by a larger refactor in 7e0f469.

Issue #948

Change-Id: I76010a4c24a6f044453e522b67c6994376dcce8e
@joeyparrish
Copy link
Member

Fixed for v2.1.7. Thanks!

@joeyparrish joeyparrish removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Dec 5, 2017
@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@joeyparrish joeyparrish

Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Milestone
v2.2.0
Development

No branches or pull requests
3 participants
@bhh1988 @joeyparrish @shaka-bot