Fix for Shaka 2.2.x to be able to switch languages #43

tchakabam · 2017-09-21T10:44:42Z

Fix for Shaka 2.2.x to be able to switch languages

The 2.2.x API basically makes variant tracks such that they are the combination of all possible audio/video combinations and appear as mime-type "video/..". only audio-only tracks will appear as mime-type "audio/..". an MPD with 7 video qualities and 2 languages will result in 14 Shaka renditions all with video mime-type. To make things simpler, Shaka has now a language-switching API that will not need us to deal with all the variants when switching the language.

This PR aims to make it possible to use the language switching.

Moreover it should be clarified how we expose the "variant" type tracks exactly.

Currently setting a variant track obtained from "videoTracks" might also switch the language :)

Changes:

add selectLanguage method with a "role" parameter. this can allow to choose between "variants" of one language (if there are any, they can be find in the audioTracks property)
improve selectTrack (use switch-case)
add isAudioOnly implementation to plugin
fix lint script calls
add .editorconfig file

…" type tracks for audio payload. We just get a list of unique languages. Therefore we need to extend the plugin API to be able to select languages that are not "variant". - add selectLanguage method with a "role" parameter. this can allow to choose between "variants" of one language (if there are any, they can be find in the audioTracks property) - improve selectTrack (use switch-case)

leandromoreira

Thank you so much 👍

leandromoreira · 2017-09-21T13:16:43Z

src/clappr-dash-shaka-playback.js

@@ -261,6 +294,9 @@ class DashShakaPlayback extends HTML5Video {
    this._options.shakaConfiguration && this._player.configure(this._options.shakaConfiguration)
    this._options.shakaOnBeforeLoad && this._options.shakaOnBeforeLoad(this._player)

+    const preferredAudioLanguage = this._player.getConfiguration().preferredAudioLanguage


Isn't this something that can be overridden via shakaConfiguration or _options?

just by _options not, but it is actually setup by the this._player.configure(this._options.shakaConfiguration) call, two lines above ^

now we need to initialize the state of _activeAudioLanguage because the initial language will be the one set in preferredAudioLanguage if it is set there.

otherwise me start out with saying language A is active because it's first in the list, but actually language B is because it has been configured as preferred.

not sure i got the question right, but does that answer your question actually? :)

I think so, this this._options.shakaConfiguration.preferredAudioLanguage is equivalent to this this._player.getConfiguration().preferredAudioLanguage.

leandromoreira · 2017-09-21T13:18:13Z

src/clappr-dash-shaka-playback.js

-        }
-    } else {
-        throw new Error('Unhandled track type:', track.type);
+  selectTrack (track, clearBuffer) {


I don't see too much value by changing an if else if else by a switch, there was another reason for this refactoring?

hm well no there is no change in logic.

from a code-style pov i would say that a switch is preferrable here since the condition is solely around the value of a single variable and no other conditions, and there are more than two cases :) so perfect time for a "switch"

so my answer is: this shouldn't have been an if-else in the first place. although i had written this code right there, just correcting myself now ;)

Yes, I see your points :) Later I'll try to break this single class into at least 4 different components.

i think it's still quite maintainable like this.

we should however add tests + enforce JSdocs with the eslint plugin for that on each method :)

Yeah, tests are way up on the priority list. 🥂

vagnervjs · 2017-09-21T17:06:15Z

src/clappr-dash-shaka-playback.js

+  selectTrack (track, clearBuffer) {
+    if (this.isReady) {
+      return false
+    }


Should we check if isReady or !isReady?

nice catch! didn't test selecting variant tracks here again. thanks!!

my next aim on this codebase is to add some unit tests :)

tchakabam · 2017-09-22T11:42:41Z

Hey guys, actually i understood that my understanding about the Shaka 2.2 API was not complete :)

Basically, to explain really simply, audio-and video tracks are under one hood of the "variant" type tracks.

What we currently do (checking for mimeType) is really going to give us only audio-only tracks when we look for "audio/..". The tracks that contain both a/v have the mimeType "video/..". Now when i have say 2 languages and 7 video qualies, that results in 14 variant tracks with respective properties in width/height/language.

The new Shaka language API simplifies to deal with this.

Now the question is: Should the plugin API here try to deal with this differently that what we do now?

I may make sense on the one hand to filter out only the variant tracks that correspond to the currently active language?

Or really expose everything Shaka gives us? Or we try to make a more convenient API?

I would rather really prefer an API that matches the data-model one would need in a player UI.

So just the video variants available under a certain language chosen for example.

Discussion is open ... :)

tchakabam · 2017-09-22T11:47:35Z

Thanks already for all the reviews :)

Problem remaining in our API here as I see it, in one sentence:

Currently setting a variant track obtained from "videoTracks" might also switch the language :)

From the way the current API is designed, this behavior is very counter-intuitive.

Also I wonder how we can further integrate such pattern into Clappr ...

Looking forward to see how we can do this!

tchakabam · 2017-09-22T11:50:00Z

Our workaround here would be like:

videoTracks
            .filter((track) => track.language === dashShakaPlayback.activeAudioLanguage)

leandromoreira · 2017-09-22T13:07:50Z

I'll put @joeyparrish into the discussion maybe he can also give his valuable input and I'll do it later as well.

About #43 (comment)

tchakabam · 2017-12-19T12:55:10Z

@leandromoreira Hey guys, unfortunately we haven't been able to continue the conversation. Let's hope we do that next year! I'm a bit outside of the topic right now, but it will become important soon ;) Cheers 🎆

joeyparrish · 2017-12-19T22:13:11Z

Hi all,

The v2.2 API exposes all variants to the application so that we don't arbitrarily limit applications, but our demo app only shows the variants for the current language to the user.

Here is the code for that in our latest release (v2.2.8): https://github.com/google/shaka-player/blob/5c992c734870fade36dfdf4751b4b2cf49c0cf93/demo/info_section.js#L128

The getAudioLanguages() and selectAudioLanguage() methods allow you to change languages explicitly, rather than using the tracks API for that. There are corresponding methods for text tracks. If you expose language selection through your API, you should definitely filter the track list for the current language, as we do in our demo. We do not recommend using selectVariant() to change languages, although it may be appropriate for some applications in some circumstances.

In v2.3, we introduce a new method that lists language/role combos. In the latest nightly build of our demo app, we only show the variants that match the current language and role, and we allow the user to change languages and roles at the same time. The new methods are getAudioLanguagesAndRoles() and getTextLanguagesAndRoles(). The track-filtering code for our upcoming v2.3 release is here: https://github.com/google/shaka-player/blob/c78e4a258b8c2924de0ad90d18d16526c74ee1af/demo/info_section.js#L137

Earlier in the conversation, there was some concern over detecting audio-video tracks vs. audio-only vs. video-only. Instead of using mimeType, you can look at the audioCodec and videoCodec fields. In audio-video variants, both fields will be non-null. For audio-only content, the videoCodec will be null, and for video-only content, the audioCodec will be null. The isAudioOnly() method on player may also be useful for this.

Shaka Player will never expose a mix of video-only, audio-only, and audio-video tracks. All variant tracks will be the same type, because we don't dynamically add or remove SourceBuffers and can't switch between audio-video and audio-only, for example.

I hope this helps! Happy holidays!

leandromoreira · 2017-12-19T22:59:31Z

Thanks, @tchakabam for your effort and @joeyparrish for your teachings. Happy holidays! 🎄

tchakabam · 2017-12-21T19:12:53Z

Hey! :) Thanks for getting back and for those explanations. It's useful to be in contact about this topic. And good to know that v2.3 exposes some convenience methods.

It just seems a bit confusing to access the combinations "folded" together in a way. Logically it's not wrong of course, and I understand why you do it, as in principle a DASH stream may impose inter-dependent audio/video combinations, in theory.

At this point I don't have any more questions about Shaka. But we have to figure how Clappr can integrate with it :) I would like to kick off a discussion with Clappr people about how to have Clappr best work with adaptive media engines @leandromoreira

Maybe we can have that discussion best around a Clappr player PR I am currently prep'ing in order to add generic adaptive-playback config and API to Clappr. Currently none of the concepts of adaptive audio/video selection or languages is defined/abstracted in the Clappr base components/interfaces. Plugins like level-selector use inofficial methods that HLS.js and Shaka playbacks expose. It could be great to come down to something meaningful here. The Shaka API is a good example for how to do that, even if that one may not be the easiest for apps/UIs to build against it. But I would say on the other side, Hls.js API can learn a bit from it, things are also confusing in a different way there.

Clappr could take a chance at generalizing these quality-adaptation and language-selection problems into a nice set of config and interfaces on the Playback base component. What do you think guys? @leandromoreira @vagnervjs @towerz 🙌

Concerning the Shaka API, I find it cumbersome from an application point of view to get all the possible resolution/language combinations flattened, when you actually usually want to give the user or application logic independent choice over both audio and video tracks available (yes I know, they are not always and by definiton indepedent, they can be coupled 😬 )

Now how the DASH streams we usually deal with are conceived, they have independent audio/video fragment, so you can switch both things independently. That's also the 99% use-case of video applications btw I would say. For example, even if MPEG-DASH allows it, really when will I have a video-resolution with say "pt-BR" available as audio language, and another resolution, where that is not available? The only meaningful thing that comes to mind is when you have on the server-side fragments muxed together audio&video, with say HD quality muxed only with 5.1-channels audio, so you can have the stereo track only with the lower resolutions (just an example). So I wonder if that is also covered in Shaka API, as it is about languages and roles. Shouldn't you actually need a method then like getAudioLanguagesAndRolesAndChannelsAndCodec ? :)

Switching audio languages and video bitrates independently is obviously a thing that Shaka API allows to do in principle, when the stream allows it.

It just seems it's going to be a bit the same problem every application will have to solve in order to use your API to solve the 99%-use-case.

For example to expose all available video bitrates, I need to:

first figure what is the active language
then filter all the variants with this language
only then i get the video bitrates that would be currently available with this language

@joeyparrish One would really like to have a method that just does that from the Shaka side :) Or am I missing something here?

Same as you already do for the language/role combinations, but for video qualities.

But then again, it's easy to have that in the ShakaPlayback here as well, kind of like we do in this PR.

And also, what do you think about a method that also exposes the available channel/codec feature-set ?

Most of the applications I know usually offer both AAC stereo and Dolby 5.1, for example.

tchakabam added 6 commits September 21, 2017 12:37

fix lint script calls

1294859

add isAudioOnly implementation to plugin

0bf13de

add .editorconfig file

0703004

allow to manage preferred/active audio language correctly

ca9d2d4

fix lint / typo

a10001a

leandromoreira reviewed Sep 21, 2017

View reviewed changes

leandromoreira requested review from towerz and vagnervjs September 21, 2017 13:19

leandromoreira approved these changes Sep 21, 2017

View reviewed changes

vagnervjs reviewed Sep 21, 2017

View reviewed changes

fix typo: we check in selectTrack for non-readiness of Shaka

726da91

vagnervjs approved these changes Sep 21, 2017

View reviewed changes

iSchluff mentioned this pull request Apr 5, 2020

Add minimal API for language switching #80

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for Shaka 2.2.x to be able to switch languages #43

Fix for Shaka 2.2.x to be able to switch languages #43

tchakabam commented Sep 21, 2017 •

edited

leandromoreira left a comment

leandromoreira Sep 21, 2017

tchakabam Sep 21, 2017

leandromoreira Sep 21, 2017

tchakabam Sep 21, 2017

leandromoreira Sep 21, 2017

tchakabam Sep 21, 2017

leandromoreira Sep 21, 2017

tchakabam Sep 21, 2017

leandromoreira Sep 21, 2017

vagnervjs Sep 21, 2017

tchakabam Sep 21, 2017

tchakabam commented Sep 22, 2017

tchakabam commented Sep 22, 2017 •

edited

tchakabam commented Sep 22, 2017

leandromoreira commented Sep 22, 2017 •

edited

tchakabam commented Dec 19, 2017

joeyparrish commented Dec 19, 2017 •

edited

leandromoreira commented Dec 19, 2017

tchakabam commented Dec 21, 2017 •

edited

Fix for Shaka 2.2.x to be able to switch languages #43

Are you sure you want to change the base?

Fix for Shaka 2.2.x to be able to switch languages #43

Conversation

tchakabam commented Sep 21, 2017 • edited

Fix for Shaka 2.2.x to be able to switch languages

leandromoreira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tchakabam commented Sep 22, 2017

tchakabam commented Sep 22, 2017 • edited

tchakabam commented Sep 22, 2017

leandromoreira commented Sep 22, 2017 • edited

tchakabam commented Dec 19, 2017

joeyparrish commented Dec 19, 2017 • edited

leandromoreira commented Dec 19, 2017

tchakabam commented Dec 21, 2017 • edited

tchakabam commented Sep 21, 2017 •

edited

tchakabam commented Sep 22, 2017 •

edited

leandromoreira commented Sep 22, 2017 •

edited

joeyparrish commented Dec 19, 2017 •

edited

tchakabam commented Dec 21, 2017 •

edited