fix: keep media update timeout alive so live playlists can recover fr…

…om network issues (#1176)
videojs · Aug 17, 2021 · 8b3533c · 8b3533c
1 parent e8230a9
commit 8b3533c
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 19 deletions.
diff --git a/src/master-playlist-controller.js b/src/master-playlist-controller.js
@@ -63,20 +63,25 @@ const shouldSwitchToMedia = function({
 
   const sharedLogLine = `allowing switch ${currentPlaylist && currentPlaylist.id || 'null'} -> ${nextPlaylist.id}`;
 
-  // If the playlist is live, then we want to not take low water line into account.
-  // This is because in LIVE, the player plays 3 segments from the end of the
-  // playlist, and if `BUFFER_LOW_WATER_LINE` is greater than the duration availble
-  // in those segments, a viewer will never experience a rendition upswitch.
-  if (!currentPlaylist || !currentPlaylist.endList) {
-    log(`${sharedLogLine} as current playlist ` + (!currentPlaylist ? 'is not set' : 'is live'));
+  if (!currentPlaylist) {
+    log(`${sharedLogLine} as current playlist is not set`);
     return true;
   }
 
-  // no need to switch playlist is the same
+  // no need to switch if playlist is the same
   if (nextPlaylist.id === currentPlaylist.id) {
     return false;
   }
 
+  // If the playlist is live, then we want to not take low water line into account.
+  // This is because in LIVE, the player plays 3 segments from the end of the
+  // playlist, and if `BUFFER_LOW_WATER_LINE` is greater than the duration availble
+  // in those segments, a viewer will never experience a rendition upswitch.
+  if (!currentPlaylist.endList) {
+    log(`${sharedLogLine} as current playlist is live`);
+    return true;
+  }
+
   const maxBufferLowWaterLine = experimentalBufferBasedABR ?
     Config.EXPERIMENTAL_MAX_BUFFER_LOW_WATER_LINE : Config.MAX_BUFFER_LOW_WATER_LINE;
 
@@ -351,7 +356,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
   checkABR_() {
     const nextPlaylist = this.selectPlaylist();
 
-    if (this.shouldSwitchToMedia_(nextPlaylist)) {
+    if (nextPlaylist && this.shouldSwitchToMedia_(nextPlaylist)) {
       this.switchMedia_(nextPlaylist, 'abr');
     }
   }

diff --git a/src/playlist-loader.js b/src/playlist-loader.js
@@ -505,13 +505,7 @@ export default class PlaylistLoader extends EventTarget {
       this.trigger('playlistunchanged');
     }
 
-    // refresh live playlists after a target duration passes
-    if (!this.media().endList) {
-      window.clearTimeout(this.mediaUpdateTimeout);
-      this.mediaUpdateTimeout = window.setTimeout(() => {
-        this.trigger('mediaupdatetimeout');
-      }, refreshDelay(this.media(), !!update));
-    }
+    this.updateMediaUpdateTimeout_(refreshDelay(this.media(), !!update));
 
     this.trigger('loadedplaylist');
   }
@@ -619,6 +613,13 @@ export default class PlaylistLoader extends EventTarget {
       return;
     }
 
+    // We update/set the timeout here so that live playlists
+    // that are not a media change will "start" the loader as expected.
+    // We expect that this function will start the media update timeout
+    // cycle again. This also prevents a playlist switch failure from
+    // causing us to stall during live.
+    this.updateMediaUpdateTimeout_(refreshDelay(playlist, true));
+
     // switching to the active playlist is a no-op
     if (!mediaChange) {
       return;
@@ -679,8 +680,12 @@ export default class PlaylistLoader extends EventTarget {
    * pause loading of the playlist
    */
   pause() {
+    if (this.mediaUpdateTimeout) {
+      window.clearTimeout(this.mediaUpdateTimeout);
+      this.mediaUpdateTimeout = null;
+    }
+
     this.stopRequest();
-    window.clearTimeout(this.mediaUpdateTimeout);
     if (this.state === 'HAVE_NOTHING') {
       // If we pause the loader before any data has been retrieved, its as if we never
       // started, so reset to an unstarted state.
@@ -705,14 +710,20 @@ export default class PlaylistLoader extends EventTarget {
    * start loading of the playlist
    */
   load(shouldDelay) {
-    window.clearTimeout(this.mediaUpdateTimeout);
-
+    if (this.mediaUpdateTimeout) {
+      window.clearTimeout(this.mediaUpdateTimeout);
+      this.mediaUpdateTimeout = null;
+    }
     const media = this.media();
 
     if (shouldDelay) {
       const delay = media ? ((media.partTargetDuration || media.targetDuration) / 2) * 1000 : 5 * 1000;
 
-      this.mediaUpdateTimeout = window.setTimeout(() => this.load(), delay);
+      this.mediaUpdateTimeout = window.setTimeout(() => {
+        this.mediaUpdateTimeout = null;
+        this.load();
+      }, delay);
+
       return;
     }
 
@@ -728,6 +739,24 @@ export default class PlaylistLoader extends EventTarget {
     }
   }
 
+  updateMediaUpdateTimeout_(delay) {
+    if (this.mediaUpdateTimeout) {
+      window.clearTimeout(this.mediaUpdateTimeout);
+      this.mediaUpdateTimeout = null;
+    }
+
+    // we only have use mediaupdatetimeout for live playlists.
+    if (!this.media() || this.media().endList) {
+      return;
+    }
+
+    this.mediaUpdateTimeout = window.setTimeout(() => {
+      this.mediaUpdateTimeout = null;
+      this.trigger('mediaupdatetimeout');
+      this.updateMediaUpdateTimeout_(delay);
+    }, delay);
+  }
+
   /**
    * start loading of the playlist
    */

diff --git a/test/playlist-loader.test.js b/test/playlist-loader.test.js
@@ -683,6 +683,22 @@ QUnit.module('Playlist Loader', function(hooks) {
     );
   });
 
+  QUnit.test('can delay load', function(assert) {
+    const loader = new PlaylistLoader('master.m3u8', this.fakeVhs);
+
+    assert.notOk(loader.mediaUpdateTimeout, 'no media update timeout');
+
+    loader.load(true);
+
+    assert.ok(loader.mediaUpdateTimeout, 'have a media update timeout now');
+    assert.strictEqual(this.requests.length, 0, 'have no requests');
+
+    this.clock.tick(5000);
+
+    assert.notOk(loader.mediaUpdateTimeout, 'media update timeout is gone');
+    assert.strictEqual(this.requests.length, 1, 'playlist request after delay');
+  });
+
   QUnit.test('starts without any metadata', function(assert) {
     const loader = new PlaylistLoader('master.m3u8', this.fakeVhs);
 
@@ -2101,6 +2117,62 @@ QUnit.module('Playlist Loader', function(hooks) {
     );
   });
 
+  QUnit.test('mediaupdatetimeout works as expected for live playlists', function(assert) {
+    const loader = new PlaylistLoader('master.m3u8', this.fakeVhs);
+    let media =
+      '#EXTM3U\n' +
+      '#EXT-X-MEDIA-SEQUENCE:0\n' +
+      '#EXTINF:5,\n' +
+      'low-0.ts\n' +
+      '#EXTINF:5,\n' +
+      'low-1.ts\n';
+
+    loader.load();
+
+    this.requests.shift().respond(
+      200, null,
+      '#EXTM3U\n' +
+      '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
+      'media.m3u8\n' +
+      '#EXT-X-STREAM-INF:BANDWIDTH=1\n' +
+      'media2.m3u8\n'
+    );
+
+    this.requests.shift().respond(200, null, media);
+
+    assert.ok(loader.mediaUpdateTimeout, 'has an initial media update timeout');
+
+    this.clock.tick(5000);
+
+    media += '#EXTINF:5\nlow-2.ts\n';
+
+    this.requests.shift().respond(200, null, media);
+
+    assert.ok(loader.mediaUpdateTimeout, 'media update timeout created another');
+
+    loader.pause();
+    assert.notOk(loader.mediaUpdateTimeout, 'media update timeout cleared');
+
+    loader.media(loader.master.playlists[0]);
+
+    assert.ok(loader.mediaUpdateTimeout, 'media update timeout created again');
+    assert.equal(this.requests.length, 0, 'no request');
+
+    loader.media(loader.master.playlists[1]);
+
+    assert.ok(loader.mediaUpdateTimeout, 'media update timeout created');
+    assert.equal(this.requests.length, 1, 'playlist requested');
+
+    this.requests.shift().respond(500, null, 'fail');
+
+    assert.ok(loader.mediaUpdateTimeout, 'media update timeout exists after request failure');
+
+    this.clock.tick(5000);
+
+    assert.ok(loader.mediaUpdateTimeout, 'media update timeout created again');
+    assert.equal(this.requests.length, 1, 'playlist re-requested');
+  });
+
   QUnit.module('llhls', {
     beforeEach() {
       this.fakeVhs.options_ = {experimentalLLHLS: true};