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

Selecting Audio language auto switches the text track(using selectAudioLanguage, selectTextTrack API) #1299

Closed
shanmugapriyaEK opened this issue Feb 13, 2018 · 9 comments
Labels
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@shanmugapriyaEK
Copy link

NOTE: For bugs, if you delete this template, we will send it again and ask you to fill it out.

Have you read the FAQ and checked for duplicate open issues?:yes

What version of Shaka Player are you using?:2.3.3

Can you reproduce the issue with our latest release version?:yes

Can you reproduce the issue with the latest code from master?:yes

Are you using the demo app or your own custom app?:both

If custom app, can you reproduce the issue using our demo app?:yes

What browser and OS are you using?:Chrome MAV

What are the manifest and license server URIs?:Demo asset- 'Angel One (multicodec, multilingual)

What did you do? Using selectTextTrack (with all the tracks listed), setting selectAudioLanguage auto switches the text track.
Reproduce in sample app by doing

  1. Text track does not list all the texttracks. Hence made changes to list all the tracks without any language or role
  2. Switched audio language using selectAudioLanguage resulted in track change
  3. Observed that in streaming_engine.js assertions fails.
    if (!mediaState && stream.type == ContentType.TEXT &&
    this.config_.ignoreTextStreamFailures) {//this was set to false
    this.loadNewTextStream(stream);
    return;
    }
    goog.asserts.assert(mediaState, 'switch: expected mediaState to exist');

What did you expect to happen? It should not switch text track

What actually happened? changes text track

@michellezhuogg michellezhuogg added type: bug Something isn't working correctly and removed needs triage labels Feb 13, 2018
@michellezhuogg
Copy link
Contributor

Not able to reproduce the assertion fail, however, both switchVariant_() and switchTextStream_() are called when switching audio language.

@shanmugapriyaEK
Copy link
Author

shanmugapriyaEK commented Feb 14, 2018

I made changes in sample to reproduce it. Shaka demo application does not list all the track. It does a filter for language and role. I removed that piece of code in order to list all the tracks.
Removed the below code in info_section.js
// Split language and role
var res = languageAndRole.split(':');
var language = res[0];
var role = res[1] || '';

tracks = tracks.filter(function(track) {
var langMatch = track.language == language;
var roleMatch = role == '' || track.roles.indexOf(role) > -1;
return langMatch && roleMatch;
});

@joeyparrish joeyparrish added this to the v2.4.0 milestone Feb 14, 2018
@shanmugapriyaEK
Copy link
Author

By setting the ignoreTextStreamFailures flag to true , I was able to resolve this issue in my sample Application.
Also I tried removing this check in steaming_Engine.js switchInternal_ as below
if (!mediaState && stream.type == ContentType.TEXT ) {
this.loadNewTextStream(stream);
return;
}
Can you please explain me more in detail about this ?
Is there a possibility for fix in 2.3?

@ismena
Copy link
Contributor

ismena commented Feb 20, 2018

Hi @shanmugapriyaEK,

Could you clarify why you wanted all the text track listed at the same time? The way we intended for this to work was that you would choose a language first (selectTextLanguage()) and then select a track from in this language from a track list.

Do you think you could follow that path?

I'm pretty sure what you're seeing happens because you go around selectTextLanguage().
Here's how it works:
We have an internal variable that stores current text language. It's set to your preferred text language when the playback starts or to English(I think) by default if no preference has been provided.
This variable gets updated when you call selectTextLanguage(). If a different language has been selected, we switch to a new track in the newly selected language.
Because you go around this logic when you choose from all the text tracks regardless of language this variable doesn't get updated. When selectAudioLanguage() is called and we refresh the state your current text language is the old language (from before you switched the track), hence we update accordingly.

We might be able to work with you to accommodate your use case if there is a reason you can't call selectTextLanguage() first, but I want to check if the intended approach might work for you first.

@ismena ismena added status: working as intended The behavior is intended; this is not a bug status: waiting on response Waiting on a response from the reporter(s) of the issue and removed type: bug Something isn't working correctly labels Feb 20, 2018
@joeyparrish joeyparrish removed this from the v2.4.0 milestone Feb 20, 2018
@shanmugapriyaEK
Copy link
Author

We have exposed this API to our clients.
Consider the scenario where they wanted to switch based on bandwidth or kind. By this way I am forcing the clients to use two API (first selectTextLanguage and then set selectTextTrack )for switching. Also they had do the filtering in their application for listing the text tracks.

@shanmugapriyaEK
Copy link
Author

Unknowingly I pressed the button

@ismena ismena added type: enhancement New feature or request and removed status: waiting on response Waiting on a response from the reporter(s) of the issue status: working as intended The behavior is intended; this is not a bug labels Feb 22, 2018
@ismena ismena added this to the v2.4.0 milestone Feb 22, 2018
@ismena
Copy link
Contributor

ismena commented Feb 22, 2018

@shanmugapriyaEK I made the changes to accommodate your use case. It should be working now (in the latest master). We'll add it to the next release.

@shanmugapriyaEK
Copy link
Author

Thank you :)

joeyparrish pushed a commit that referenced this issue Mar 1, 2018
We've been asked to implement this to allow for a custom app
behavior where app exposes all the tracks independent of language
and role.
In that scenario changing audio language will affect current text
track and the other way around. This CL makes sure it doesn't.

Closes #1299.

Change-Id: I39a9748c033ea748173ca00bfa7168dff642a03e
@joeyparrish
Copy link
Member

Cherry-picked for v2.3.3.

@shaka-project shaka-project locked and limited conversation to collaborators Apr 23, 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.
Labels
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants