Redundant streams fallback behavior for alternate renditions with multiple media groups #1630

tchakabam · 2018-03-26T21:39:11Z

Description of the Changes

Until now, we are not supporting a typical live broadcast use-case: using backup streams for all alternate audio and subtitle rendition options, grouping these into respective group-IDs. For example group-ID A groups all the primary renditions and video-levels, B all the back ones. In this case, group-IDs of selected audio and subtitle renditions need to be matched to the group-ID of the running level and vice versa. So if we fallback on a level, we have to re-select the rendition so to match the group ID, and if we fallback on audio we may have to switch to the matching redundant level. That is because the grouped renditions/levels actually run on sync'ed encoders, while if the groups don't match, there can easily be A/V-sync issues. In fact, the clocks of the redundants streams across group IDs are not actually even specified to have anything like matching clocks or so, they could operate in completely different timeline.

=> This PR will fix the above and allow proper fallback behavior across all directions (video-levels <-> audio-and-subs-renditions)

CheckLists

changes have been done against master branch, and PR does not conflict
no commits have been done in dist folder (we will take care of updating it)
new unit / functional tests have been added (whenever applicable)
Travis tests are passing (or test results are not worse than on master branch :))
API or design changes are documented in API.md

tchakabam · 2018-03-26T21:44:53Z

May fix parts of #1438

netTrekfd · 2018-03-27T09:27:17Z

src/loader/playlist-loader.js

@@ -152,24 +152,26 @@ class PlaylistLoader extends EventHandler {
  }

  onManifestLoading (data) {
-    this.load(data.url, { type: ContextType.MANIFEST });
+    this.load(data.url, { type: ContextType.MANIFES, level: null, id: null });


There is a little type error, should be:
this.load(data.url, { type: ContextType.MANIFEST, level: null, id: null });

which is why stuff currently fails 👍

tchakabam · 2018-03-27T11:59:16Z

src/controller/audio-track-controller.js

  get audioTrack () {
    return this.trackId;
  }

-  /** select an audio track, based on its index in audio track lists**/
-  set audioTrack (audioTrackId) {
-    if (this.trackId !== audioTrackId || this.tracks[audioTrackId].details === undefined)


@mangui how did the condition to run the set internal make sense: this.tracks[audioTrackId].details === undefined ?

wouldn't you still want to switch even if the metadata is not loaded yet?

see 2284bc6#diff-b5ce37fcb02b92e3f287ca1a1e3ad800R65

condition is :
call set internal if new track id OR if same track id but details not available yet (in that case it will force a reload of the details)

of course, makes sense :) thanks for the explanation. i have added the check for the details prop in this branch now as well to make sure we are allowing to switch track in this case as well.

tchakabam · 2018-04-24T10:08:44Z

@mangui @johnBartos @tjenkinson Hey guys, do you think you could check this out? just to make sure i didn't break something badly :)

Will also add unit tests for this behavior hopefully as soon as I get to it!

… details lack (recovers previous logic after refactor)

mangui · 2018-05-01T05:41:02Z

src/controller/abr-controller.js

@@ -31,7 +31,12 @@ class AbrController extends EventHandler {
  }

  onFragLoading (data) {
-    let frag = data.frag;
+    const hls = this.hls;


hls and config objects were only initialized if useful before.
now you will have extra processing everytime a subtitle or audio frag is loading.
you could argue that this might look like micro-optimization, but still ;-)

mangui · 2018-05-01T05:41:20Z

src/controller/abr-controller.js

-          isLive = hls.levels[level].details.live,
-          config = hls.config,
-          ewmaFast, ewmaSlow;
+        let level = data.frag.level;


let level = frag.level is shorter

mangui · 2018-05-01T05:44:01Z

src/controller/abr-controller.js

@@ -55,8 +57,9 @@ class AbrController extends EventHandler {
        }
        this._bwEstimator = new EwmaBandWidthEstimator(hls, ewmaSlow, ewmaFast, config.abrEwmaDefaultEstimate);
      }
-      this.fragCurrent = frag;
+      this._bwEstimator = new EwmaBandWidthEstimator(hls, ewmaSlow, ewmaFast, config.abrEwmaDefaultEstimate);


looks like bwEstimator is created twice now ?

true. was debugging this stuff and left some mess around :/

mangui · 2018-05-01T05:44:51Z

src/controller/abr-controller.js

    }
+    this.fragCurrent = frag;


we should not update this.fragCurrent if frag is not from main

i was actually wondering about that. i think there was a bug however here, we were sometimes hitting an undefined fragCurrent..? but maybe it's fixed now, i was trying to debug this part and something was fishy.

…ned dependency tree

…ngly

…age of current track and using group ID currently running

…her controllers to ensure they are on the correct group ID

axten · 2018-05-23T08:55:26Z

Hi @tchakabam,
can you confirm that with this great PR a master playlist with multiple media groups (GROUP-ID) for video (with subs) and audio is handled correctly?
https://zdfhlstest2-i.akamaihd.net/hls/live/226660/zdf_ahw/high/master.m3u8

And if yes, does hls.js ensure that all active medias are from the same group?
Example: video, audio and subs from group A are playing. Now video fails and hls.js switches to fallback video stream from group B. Does hls.js switch also audio and subtitle streams to Group B in this case?

tchakabam · 2018-05-23T09:17:09Z

Hi @axten Yes. That's exactly the idea. In this branch this should work as expected now.

axten · 2018-05-23T09:33:54Z

ok great! this is exactly what we need! cannot await this to land :)

tchakabam · 2018-05-24T11:50:42Z

@axten That's what I had been told as well 👍 Very well, then this will need some clean-up and polishing, but the functionality is now there. We were actually missing the information inside our Quality-Level data-model where we are handling redundant streams by nesting various URLs for the same quality, but as a result were compressing away the various audio-group IDs for the respective redundants streams. This information is now being patched onto the data-model along with the redundant URLs.

Furthermore, the audio-track switched event is being handled now by the level-controller (which is triggered also when we switch for failure fallback) in order to make sure that the current level played is actually for the group ID of the latest selected audio-track. If that is not the case, we will correct that setting internally and re-load the video levels accordingly.

In a way like a mirror, the analogous logic is implemented for audio-tracks. As a quality level is switched (wether it be for any reason, manually or by recovery/fallback mechanisms) we check which actual stream is being played of the redundant options, and which audio-group-id that corresponds. If that doesn't match the audio-group-ID of the currently selected audio-track, we switch it accordingly, while trying to preserve the actual name/language setting which indicates the logical "role" of this audio track (to avoid switching say from original audio to audio for visibility handicap etc).

Further things to do now would be:

Add the analoguous logic for subtitles redundant groups (will be pretty much a copy of what we added for audio-tracks, so very easy)
Add some unit tests to spec this behavior clearly and avoid regression
Add these test cases of yours to our acceptance test suite

=> Then we should be able to merge this into our master branch.

You could anyhow already use this branch here for your testing to validate we got it all right, and in principle also for your prod player already, if it works for you (and after we removed a few hard-coded log lines maybe :)).

…in log overload

alkerway · 2018-06-05T18:01:12Z

src/controller/audio-track-controller.js

+      }
+      const newTrack = this.tracks[i];
+      if (newTrack.name === name &&
+       newTrack.language === language) {


Why check for both name and language here but only name on _selectInitialAudioTrack? From what I see in the spec only name is required to both exist and be consistent across groups. Any sane manifest will have matching and unique names and languages, but it may be worth being consistent

Good point. Name should be sufficient, you're right.

… an optional info

tchakabam · 2018-06-11T08:43:19Z

Build pipeline succeeded (not sure why it's greyed out however): https://travis-ci.org/video-dev/hls.js/builds/389764334

tchakabam requested a review from mangui March 26, 2018 21:39

tchakabam added Bug Needs review labels Mar 26, 2018

tchakabam changed the title ~~Audio track fallback to non-blacklisted on network failure~~ Audio track fallback to non-blacklisted on network failure, fixes #1438 Mar 26, 2018

tchakabam added the Enhancement label Mar 26, 2018

netTrekfd reviewed Mar 27, 2018

View reviewed changes

tchakabam commented Mar 27, 2018

View reviewed changes

This was referenced Mar 27, 2018

Redundant Multiple Audio Track Support #1247

Closed

Redundant Switching with Multiple Audio Tracks Bug #1248

Closed

tchakabam mentioned this pull request Apr 4, 2018

Faulty playback of Alternate-Audio Playlists with Primary/Backup, if Primary returns 404 (since hls.js 0.8.3) #1438

Closed

4 tasks

tchakabam force-pushed the feature/audio-track-fallback branch 2 times, most recently from 9ecd32e to 266d64c Compare April 23, 2018 15:11

tchakabam added 7 commits April 23, 2018 18:56

ABR controller isofunctional code readability improvements

c3a1e22

Audio stream ctrl error handling logs improvement

7989b2e

Audio track controller: Fallback on failure feature

239f269

level controller: rename to clearTimer

870dee7

stream ctrl: get rid of undefined

784ffaa

subs ctrl: get rid of undefined

e7a9ef5

playlist loader: harmonize data model on call to load

556cd06

tchakabam force-pushed the feature/audio-track-fallback branch from 266d64c to 556cd06 Compare April 23, 2018 16:59

Audio track ctrl: Call track switching logic on same track when track…

008e38e

… details lack (recovers previous logic after refactor)

mangui reviewed May 1, 2018

View reviewed changes

tchakabam added 6 commits May 4, 2018 16:38

Demo improvements: Serialize config, Tabbed tools display and more

6e49d77

add demo-utils module

4b6e808

demo html: visual fixes

c44f7f1

add lint:demo command

7b9b7e4

add package-lock.json to allow reproducible installs with equal flate…

d4850f7

…ned dependency tree

update package-lock

25948c5

tchakabam added 4 commits May 16, 2018 21:01

audio-track-controller: check for running group ID and select accordi…

e83aa2e

…ngly

level-controller: comment logs

bd6ee0a

audio-track-controller: correct fallback behavior matching name&langu…

49e6b53

…age of current track and using group ID currently running

rm file commited by mistake

3475845

tchakabam changed the title ~~Audio track fallback to non-blacklisted on network failure, fixes #1438~~ Redundant streams fallback behavior for alternate rendition with multiple media groups May 17, 2018

tchakabam changed the title ~~Redundant streams fallback behavior for alternate rendition with multiple media groups~~ Redundant streams fallback behavior for alternate renditions with multiple media groups May 17, 2018

allow media fallback ping-pong: audio-track/level switches trigger ot…

65a3d9d

…her controllers to ensure they are on the correct group ID

tchakabam added 4 commits June 4, 2018 16:32

testbench: prevent from circular structures or unserializable things …

d4d7b6b

…in log overload

controllers: comment console logs

63bef04

cea and vtt parsers: comment console logs

02ae45a

demo: remove console logs

7367415

tchakabam mentioned this pull request Jun 4, 2018

Multiple captions #1729

Closed

4 tasks

Merge branch 'master' into feature/audio-track-fallback

ad16891

alkerway reviewed Jun 5, 2018

View reviewed changes

tchakabam added 4 commits June 7, 2018 22:06

audio track fallback: don't check for language being equal since it's…

5dfa0a3

… an optional info

AudioTrackController: more extensive documentation

eb049de

AudioTrackController: unit tests

d450f71

TaskLoop: add some more explanation

4b47218

tchakabam removed the Needs review label Jun 8, 2018

Merge branch 'master' into feature/audio-track-fallback

fce0fb7

tchakabam merged commit 741d531 into master Jun 11, 2018

tchakabam added this to the 0.10.0 milestone Jun 13, 2018

tchakabam deleted the feature/audio-track-fallback branch June 16, 2018 16:09

OrenMe mentioned this pull request Jul 16, 2018

Regression: Immediate audio track change causes invalid state in audio stream controller #1833

Closed

ThePlainWhiteTee mentioned this pull request Jan 3, 2019

Basic Live Stream config setup #2066

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redundant streams fallback behavior for alternate renditions with multiple media groups #1630

Redundant streams fallback behavior for alternate renditions with multiple media groups #1630

tchakabam commented Mar 26, 2018 •

edited

Loading

tchakabam commented Mar 26, 2018

netTrekfd Mar 27, 2018

tchakabam Mar 27, 2018

tchakabam Mar 27, 2018 •

edited

Loading

tchakabam Mar 27, 2018

mangui Apr 3, 2018

tchakabam Apr 26, 2018

tchakabam commented Apr 24, 2018

mangui May 1, 2018

mangui May 1, 2018

mangui May 1, 2018

tchakabam May 2, 2018

mangui May 1, 2018

tchakabam May 2, 2018

axten commented May 23, 2018

tchakabam commented May 23, 2018 •

edited

Loading

axten commented May 23, 2018

tchakabam commented May 24, 2018 •

edited

Loading

alkerway Jun 5, 2018

tchakabam Jun 7, 2018

tchakabam commented Jun 11, 2018

Redundant streams fallback behavior for alternate renditions with multiple media groups #1630

Redundant streams fallback behavior for alternate renditions with multiple media groups #1630

Conversation

tchakabam commented Mar 26, 2018 • edited Loading

Description of the Changes

CheckLists

tchakabam commented Mar 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tchakabam Mar 27, 2018 • edited Loading

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 Apr 24, 2018

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

axten commented May 23, 2018

tchakabam commented May 23, 2018 • edited Loading

axten commented May 23, 2018

tchakabam commented May 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tchakabam commented Jun 11, 2018

tchakabam commented Mar 26, 2018 •

edited

Loading

tchakabam Mar 27, 2018 •

edited

Loading

tchakabam commented May 23, 2018 •

edited

Loading

tchakabam commented May 24, 2018 •

edited

Loading