fix: pause/abort loaders before an exclude, preventing bad appends (#902)

scripts/index-demo-page.js

@@ -18,9 +18,10 @@
     var id = selectedOption.value;
 
     window.vhs.representations().forEach(function(rep) {
-      rep.enabled(rep.id === id);
+      rep.playlist.disabled = rep.id !== id;
     });
 
+    window.mpc.smoothQualityChange_();
   });
   var hlsOptGroup = document.querySelector('[label="hls"]');
   var dashOptGroup = document.querySelector('[label="dash"]');
@@ -331,9 +332,7 @@
           if (player.vhs) {
             window.vhs = player.tech_.vhs;
             window.mpc = player.tech_.vhs.masterPlaylistController_;
-            window.mpc.masterPlaylistLoader_.on('mediachange', function() {
-              regenerateRepresentations();
-            });
+            window.mpc.masterPlaylistLoader_.on('mediachange', regenerateRepresentations);
             regenerateRepresentations();
 
           } else {

src/master-playlist-controller.js

@@ -952,6 +952,10 @@ export class MasterPlaylistController extends videojs.EventTarget {
     this.tech_.trigger({type: 'usage', name: 'vhs-rendition-blacklisted'});
     this.tech_.trigger({type: 'usage', name: 'hls-rendition-blacklisted'});
 
+    // TODO: should we select a new playlist if this blacklist wasn't for the currentPlaylist?
+    // Would be something like media().id !=== currentPlaylist.id and we  would need something
+    // like `pendingMedia` in playlist loaders to check against that too. This will prevent us
+    // from loading a new playlist on any blacklist.
     // Select a new playlist
     const nextPlaylist = this.selectPlaylist();
 
@@ -960,12 +964,25 @@ export class MasterPlaylistController extends videojs.EventTarget {
       this.trigger('error');
       return;
     }
+
     const logFn = error.internal ? this.logger_ : videojs.log.warn;
     const errorMessage = error.message ? (' ' + error.message) : '';
 
     logFn(`${(error.internal ? 'Internal problem' : 'Problem')} encountered with playlist ${currentPlaylist.id}.` +
       `${errorMessage} Switching to playlist ${nextPlaylist.id}.`);
 
+    // if audio group changed reset audio loaders
+    if (nextPlaylist.attributes.AUDIO !== currentPlaylist.attributes.AUDIO) {
+      this.delegateLoaders_('audio', ['abort', 'pause']);
+    }
+
+    // if subtitle group changed reset subtitle loaders
+    if (nextPlaylist.attributes.SUBTITLES !== currentPlaylist.attributes.SUBTITLES) {
+      this.delegateLoaders_('subtitle', ['abort', 'pause']);
+    }
+
+    this.delegateLoaders_('main', ['abort', 'pause']);
+
     return this.masterPlaylistLoader_.media(nextPlaylist, isFinalRendition);
   }
 
@@ -973,22 +990,65 @@ export class MasterPlaylistController extends videojs.EventTarget {
    * Pause all segment/playlist loaders
    */
   pauseLoading() {
-    // pause all segment loaders
-    this.mainSegmentLoader_.pause();
-    if (this.mediaTypes_.AUDIO.activePlaylistLoader) {
-      this.audioSegmentLoader_.pause();
+    this.delegateLoaders_('all', ['abort', 'pause']);
+  }
+
+  /**
+   * Call a set of functions in order on playlist loaders, segment loaders,
+   * or both types of loaders.
+   *
+   * @param {string} filter
+   *        Filter loaders that should call fnNames using a string. Can be:
+   *        * all - run on all loaders
+   *        * audio - run on all audio loaders
+   *        * subtitle - run on all subtitle loaders
+   *        * main - run on the main/master loaders
+   *
+   * @param {Array|string} fnNames
+   *        A string or array of function names to call.
+   */
+  delegateLoaders_(filter, fnNames) {
+    const loaders = [];
+
+    const dontFilterPlaylist = filter === 'all';
+
+    if (dontFilterPlaylist || filter === 'main') {
+      loaders.push(this.masterPlaylistLoader_);
     }
-    if (this.mediaTypes_.SUBTITLES.activePlaylistLoader) {
-      this.subtitleSegmentLoader_.pause();
+
+    const mediaTypes = [];
+
+    if (dontFilterPlaylist || filter === 'audio') {
+      mediaTypes.push('AUDIO');
     }
 
-    // pause all playlist loaders
-    this.masterPlaylistLoader_.pause();
-    Object.keys(this.mediaTypes_).forEach((type) => {
-      if (this.mediaTypes_[type].activePlaylistLoader) {
-        this.mediaTypes_[type].activePlaylistLoader.pause();
+    if (dontFilterPlaylist || filter === 'subtitle') {
+      mediaTypes.push('CLOSED-CAPTIONS');
+      mediaTypes.push('SUBTITLES');
+    }
+
+    mediaTypes.forEach((mediaType) => {
+      const loader = this.mediaTypes_[mediaType] &&
+        this.mediaTypes_[mediaType].activePlaylistLoader;
+
+      if (loader) {
+        loaders.push(loader);
       }
     });
+
+    ['main', 'audio', 'subtitle'].forEach((name) => {
+      const loader = this[`${name}SegmentLoader_`];
+
+      if (loader && (filter === name || filter === 'all')) {
+        loaders.push(loader);
+      }
+    });
+
+    loaders.forEach((loader) => fnNames.forEach((fnName) => {
+      if (typeof loader[fnName] === 'function') {
+        loader[fnName]();
+      }
+    }));
   }
 
   /**

test/master-playlist-controller.test.js

@@ -4414,47 +4414,42 @@ QUnit.test('disposes timeline change controller on dispose', function(assert) {
   assert.equal(disposes, 1, 'disposed timeline change controller');
 });
 
-QUnit.test('on error all segment and playlist loaders are paused', function(assert) {
-  const paused = {
-    audioSegment: false,
-    subtitleSegment: false,
-    mainSegment: false,
-    masterPlaylist: false
-  };
+QUnit.test('on error all segment and playlist loaders are paused and aborted', function(assert) {
+  const mpc = this.masterPlaylistController;
+  const calls = {};
+  const expected = {};
 
   Object.keys(this.masterPlaylistController.mediaTypes_).forEach((type) => {
     const key = `${type.toLowerCase()}Playlist`;
 
-    paused[key] = false;
+    calls[`${key}Abort`] = 0;
+    calls[`${key}Pause`] = 0;
+    expected[`${key}Abort`] = 1;
+    expected[`${key}Pause`] = 1;
 
     this.masterPlaylistController.mediaTypes_[type].activePlaylistLoader = {
-      pause() {
-        paused[key] = true;
-      }
+      pause: () => calls[`${key}Pause`]++,
+      abort: () => calls[`${key}Abort`]++
     };
   });
 
-  this.masterPlaylistController.audioSegmentLoader_.pause = () => {
-    paused.audioSegment = true;
-  };
-
-  this.masterPlaylistController.subtitleSegmentLoader_.pause = () => {
-    paused.subtitleSegment = true;
-  };
-
-  this.masterPlaylistController.mainSegmentLoader_.pause = () => {
-    paused.mainSegment = true;
-  };
-
-  this.masterPlaylistController.masterPlaylistLoader_.pause = () => {
-    paused.masterPlaylist = true;
-  };
+  [
+    'audioSegmentLoader',
+    'subtitleSegmentLoader',
+    'mainSegmentLoader',
+    'masterPlaylistLoader'
+  ].forEach(function(key) {
+    calls[`${key}Abort`] = 0;
+    calls[`${key}Pause`] = 0;
+    expected[`${key}Abort`] = 1;
+    expected[`${key}Pause`] = 1;
+    mpc[`${key}_`].pause = () => calls[`${key}Pause`]++;
+    mpc[`${key}_`].abort = () => calls[`${key}Abort`]++;
+  });
 
   this.masterPlaylistController.trigger('error');
 
-  Object.keys(paused).forEach(function(name) {
-    assert.ok(paused[name], `${name} was paused on error`);
-  });
+  assert.deepEqual(calls, expected, 'calls as expected');
 });
 
 QUnit.test('can pass or select a playlist for fastQualityChange', function(assert) {
@@ -5216,3 +5211,302 @@ QUnit.test('main & audio loader only trackinfo works as expected', function(asse
   assert.equal(createBuffers, 1, 'createBuffers not called');
   assert.equal(switchBuffers, 2, 'addOrChangeSourceBuffers called');
 });
+
+QUnit.module('MasterPlaylistController - exclusion behavior', {
+  beforeEach(assert) {
+    sharedHooks.beforeEach.call(this, assert);
+
+    this.mpc = this.masterPlaylistController;
+
+    openMediaSource(this.player, this.clock);
+
+    this.player.tech_.vhs.bandwidth = 1;
+
+    this.delegateLoaders = [];
+    this.mpc.delegateLoaders_ = (filter, fnNames) => {
+      this.delegateLoaders.push({filter, fnNames});
+    };
+
+    this.runTest = (master, expectedDelegates) => {
+      // master
+      this.requests.shift()
+        .respond(200, null, master);
+
+      // media
+      this.standardXHRResponse(this.requests.shift());
+
+      assert.equal(this.mpc.media(), this.mpc.master().playlists[0], 'selected first playlist');
+
+      this.mpc.blacklistCurrentPlaylist({
+        internal: true,
+        playlist: this.mpc.master().playlists[0],
+        blacklistDuration: Infinity
+      });
+
+      assert.equal(this.mpc.master().playlists[0].excludeUntil, Infinity, 'exclusion happened');
+      assert.deepEqual(this.delegateLoaders, expectedDelegates, 'called delegateLoaders');
+    };
+  },
+  afterEach(assert) {
+    sharedHooks.afterEach.call(this, assert);
+  }
+});
+
+QUnit.test('exclusions always pause/abort main/master loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5"
+    media.m3u8
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2"
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.test('exclusions that remove audio group abort/pause main/audio loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5",AUDIO="foo"
+    media.m3u8'
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2"
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'audio', fnNames: ['abort', 'pause']},
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.test('exclusions that change audio group abort/pause main/audio loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5",AUDIO="foo"
+    media.m3u8'
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2",AUDIO="bar"
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'audio', fnNames: ['abort', 'pause']},
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.test('exclusions that add audio group abort/pause main/audio loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5"
+    media.m3u8'
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2",AUDIO="bar"
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'audio', fnNames: ['abort', 'pause']},
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.test('exclusions that add subtitles group abort/pause main/subtitles loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5"
+    media.m3u8'
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2",SUBTITLES="foo
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'subtitle', fnNames: ['abort', 'pause']},
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.test('exclusions that remove subtitles group abort/pause main/subtitles loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5",SUBTITLES="foo"
+    media.m3u8'
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2"
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'subtitle', fnNames: ['abort', 'pause']},
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.test('exclusions that change subtitles group abort/pause main/subtitles loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5",SUBTITLES="foo"
+    media.m3u8'
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2",SUBTITLES="bar"
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'subtitle', fnNames: ['abort', 'pause']},
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.test('exclusions that change all groups abort/pause all loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5",AUDIO="foo",SUBTITLES="foo"
+    media.m3u8'
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2",AUDIO="bar",SUBTITLES="bar"
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'audio', fnNames: ['abort', 'pause']},
+    {filter: 'subtitle', fnNames: ['abort', 'pause']},
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.test('exclusions that remove all groups abort/pause all loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5",AUDIO="foo",SUBTITLES="foo"
+    media.m3u8'
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2"
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'audio', fnNames: ['abort', 'pause']},
+    {filter: 'subtitle', fnNames: ['abort', 'pause']},
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.test('exclusions that add all groups abort/pause all loaders', function(assert) {
+  const master = `
+    #EXTM3U
+    #EXT-X-STREAM-INF:BANDWIDTH=1,CODECS="avc1.4d400d,mp4a.40.5"
+    media.m3u8'
+    #EXT-X-STREAM-INF:BANDWIDTH=10,CODECS="avc1.4d400d,mp4a.40.2",AUDIO="foo",SUBTITLES="foo"
+    media1.m3u8
+  `;
+
+  const expectedDelegates = [
+    {filter: 'audio', fnNames: ['abort', 'pause']},
+    {filter: 'subtitle', fnNames: ['abort', 'pause']},
+    {filter: 'main', fnNames: ['abort', 'pause']}
+  ];
+
+  this.runTest(master, expectedDelegates);
+});
+
+QUnit.module('MasterPlaylistController delegate loaders', {
+  beforeEach(assert) {
+    sharedHooks.beforeEach.call(this, assert);
+
+    this.mpc = this.masterPlaylistController;
+    this.calls = {};
+    this.expected = {};
+
+    Object.keys(this.mpc.mediaTypes_).forEach((type) => {
+      const key = `${type.toLowerCase()}Playlist`;
+
+      this.calls[`${key}Abort`] = 0;
+      this.calls[`${key}Pause`] = 0;
+      this.expected[`${key}Abort`] = 0;
+      this.expected[`${key}Pause`] = 0;
+
+      this.mpc.mediaTypes_[type].activePlaylistLoader = {
+        abort: () => this.calls[`${key}Abort`]++,
+        pause: () => this.calls[`${key}Pause`]++
+      };
+    });
+
+    [
+      'audioSegmentLoader',
+      'subtitleSegmentLoader',
+      'mainSegmentLoader',
+      'masterPlaylistLoader'
+    ].forEach((key) => {
+      this.calls[`${key}Abort`] = 0;
+      this.calls[`${key}Pause`] = 0;
+      this.expected[`${key}Abort`] = 0;
+      this.expected[`${key}Pause`] = 0;
+      this.mpc[`${key}_`].abort = () => this.calls[`${key}Abort`]++;
+      this.mpc[`${key}_`].pause = () => this.calls[`${key}Pause`]++;
+    });
+  },
+  afterEach(assert) {
+    sharedHooks.afterEach.call(this, assert);
+  }
+});
+
+QUnit.test('filter all works', function(assert) {
+  this.mpc.delegateLoaders_('all', ['abort', 'pause']);
+
+  Object.keys(this.expected).forEach((key) => {
+    this.expected[key] = 1;
+  });
+
+  assert.deepEqual(this.calls, this.expected, 'calls as expected');
+});
+
+QUnit.test('filter main works', function(assert) {
+  this.mpc.delegateLoaders_('main', ['abort', 'pause']);
+
+  Object.keys(this.expected).forEach((key) => {
+    if ((/^(master|main)/).test(key)) {
+      this.expected[key] = 1;
+    }
+  });
+
+  assert.deepEqual(this.calls, this.expected, 'calls as expected');
+});
+
+QUnit.test('filter audio works', function(assert) {
+  this.mpc.delegateLoaders_('audio', ['abort', 'pause']);
+
+  Object.keys(this.expected).forEach((key) => {
+    if ((/^audio/).test(key)) {
+      this.expected[key] = 1;
+    }
+  });
+
+  assert.deepEqual(this.calls, this.expected, 'calls as expected');
+});
+
+QUnit.test('filter subtitle works', function(assert) {
+  this.mpc.delegateLoaders_('subtitle', ['abort', 'pause']);
+
+  Object.keys(this.expected).forEach((key) => {
+    if ((/^(subtitle|closed-captions)/).test(key)) {
+      this.expected[key] = 1;
+    }
+  });
+
+  assert.deepEqual(this.calls, this.expected, 'calls as expected');
+});