prioritize user enabled playlists over blacklist (#1269)

src/master-playlist-controller.js

@@ -839,6 +839,10 @@ export class MasterPlaylistController extends videojs.EventTarget {
     // out-of-date in this scenario
     currentPlaylist = error.playlist || this.masterPlaylistLoader_.media();
 
+    blacklistDuration = blacklistDuration ||
+                        error.blacklistDuration ||
+                        this.blacklistDuration;
+
     // If there is no current playlist, then an error occurred while we were
     // trying to load the master OR while we were disposing of the tech
     if (!currentPlaylist) {
@@ -862,8 +866,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
       return this.masterPlaylistLoader_.load(isFinalRendition);
     }
     // Blacklist this playlist
-    currentPlaylist.excludeUntil = Date.now() +
-      (blacklistDuration ? blacklistDuration : this.blacklistDuration) * 1000;
+    currentPlaylist.excludeUntil = Date.now() + (blacklistDuration * 1000);
     this.tech_.trigger('blacklistplaylist');
     this.tech_.trigger({type: 'usage', name: 'hls-rendition-blacklisted'});
 

src/playlist-selectors.js

@@ -154,14 +154,29 @@ export const simpleSelector = function(master,
   stableSort(sortedPlaylistReps, (left, right) => left.bandwidth - right.bandwidth);
 
   // filter out any playlists that have been excluded due to
-  // incompatible configurations or playback errors
+  // incompatible configurations
   sortedPlaylistReps = sortedPlaylistReps.filter(
+    (rep) => !Playlist.isIncompatible(rep.playlist)
+  );
+
+  // filter out any playlists that have been disabled manually through the representations
+  // api or blacklisted temporarily due to playback errors.
+  let enabledPlaylistReps = sortedPlaylistReps.filter(
     (rep) => Playlist.isEnabled(rep.playlist)
   );
 
+  if (!enabledPlaylistReps.length) {
+    // if there are no enabled playlists, then they have all been blacklisted or disabled
+    // by the user through the representations api. In this case, ignore blacklisting and
+    // fallback to what the user wants by using playlists the user has not disabled.
+    enabledPlaylistReps = sortedPlaylistReps.filter(
+      (rep) => !Playlist.isDisabled(rep.playlist)
+    );
+  }
+
   // filter out any variant that has greater effective bitrate
   // than the current estimated bandwidth
-  let bandwidthPlaylistReps = sortedPlaylistReps.filter(
+  let bandwidthPlaylistReps = enabledPlaylistReps.filter(
     (rep) => rep.bandwidth * Config.BANDWIDTH_VARIANCE < playerBandwidth
   );
 
@@ -218,12 +233,15 @@ export const simpleSelector = function(master,
   }
 
   // fallback chain of variants
-  return (
+  let chosenRep = (
     resolutionPlusOneRep ||
     resolutionBestRep ||
     bandwidthBestRep ||
+    enabledPlaylistReps[0] ||
     sortedPlaylistReps[0]
-  ).playlist;
+  );
+
+  return chosenRep ? chosenRep.playlist : null;
 };
 
 // Playlist Selectors
@@ -319,10 +337,25 @@ export const minRebufferMaxBandwidthSelector = function(settings) {
     syncController
   } = settings;
 
+  // filter out any playlists that have been excluded due to
+  // incompatible configurations
+  const compatiblePlaylists = master.playlists.filter(
+    playlist => !Playlist.isIncompatible(playlist));
+
+  // filter out any playlists that have been disabled manually through the representations
+  // api or blacklisted temporarily due to playback errors.
+  let enabledPlaylists = compatiblePlaylists.filter(Playlist.isEnabled);
+
+  if (!enabledPlaylists.length) {
+    // if there are no enabled playlists, then they have all been blacklisted or disabled
+    // by the user through the representations api. In this case, ignore blacklisting and
+    // fallback to what the user wants by using playlists the user has not disabled.
+    enabledPlaylists = compatiblePlaylists.filter(
+      playlist => !Playlist.isDisabled(playlist));
+  }
+
   const bandwidthPlaylists =
-    master.playlists.filter(playlist =>
-      Playlist.isEnabled(playlist) && Playlist.hasAttribute('BANDWIDTH', playlist)
-    );
+    enabledPlaylists.filter(Playlist.hasAttribute.bind(null, 'BANDWIDTH'));
 
   const rebufferingEstimates = bandwidthPlaylists.map((playlist) => {
     const syncPoint = syncController.getSyncPoint(playlist,

src/playlist.js

@@ -398,6 +398,18 @@ export const isBlacklisted = function(playlist) {
   return playlist.excludeUntil && playlist.excludeUntil > Date.now();
 };
 
+/**
+ * Check whether the playlist is compatible with current playback configuration or has
+ * been blacklisted permanently for being incompatible.
+ *
+ * @param {Object} playlist the media playlist object
+ * @return {boolean} whether the playlist is incompatible or not
+ * @function isIncompatible
+ */
+export const isIncompatible = function(playlist) {
+  return playlist.excludeUntil && playlist.excludeUntil === Infinity;
+};
+
 /**
  * Check whether the playlist is enabled or not.
  *
@@ -411,6 +423,17 @@ export const isEnabled = function(playlist) {
   return (!playlist.disabled && !blacklisted);
 };
 
+/**
+ * Check whether the playlist has been manually disabled through the representations api.
+ *
+ * @param {Object} playlist the media playlist object
+ * @return {boolean} whether the playlist is disabled manually or not
+ * @function isDisabled
+ */
+export const isDisabled = function(playlist) {
+  return playlist.disabled;
+};
+
 /**
  * Returns whether the current playlist is an AES encrypted HLS stream
  *
@@ -487,7 +510,9 @@ Playlist.duration = duration;
 Playlist.seekable = seekable;
 Playlist.getMediaInfoForTime = getMediaInfoForTime;
 Playlist.isEnabled = isEnabled;
+Playlist.isDisabled = isDisabled;
 Playlist.isBlacklisted = isBlacklisted;
+Playlist.isIncompatible = isIncompatible;
 Playlist.playlistEnd = playlistEnd;
 Playlist.isAes = isAes;
 Playlist.isFmp4 = isFmp4;

src/rendition-mixin.js

@@ -1,20 +1,20 @@
-import { isBlacklisted, isEnabled } from './playlist.js';
+import { isIncompatible, isEnabled } from './playlist.js';
+
 /**
- * Enable/disable playlist function. It is intended to have the first two
- * arguments partially-applied in order to create the final per-playlist
- * function.
+ * Returns a function that acts as the Enable/disable playlist function.
  *
- * @param {PlaylistLoader} playlist - The rendition or media-playlist
+ * @param {PlaylistLoader} loader - The master playlist loader
+ * @param {String} playlistUri - uri of the playlist
  * @param {Function} changePlaylistFn - A function to be called after a
  * playlist's enabled-state has been changed. Will NOT be called if a
  * playlist's enabled-state is unchanged
  * @param {Boolean=} enable - Value to set the playlist enabled-state to
  * or if undefined returns the current enabled-state for the playlist
- * @return {Boolean} The current enabled-state of the playlist
+ * @return {Function} Function for setting/getting enabled
  */
-const enableFunction = (loader, playlistUri, changePlaylistFn, enable) => {
+const enableFunction = (loader, playlistUri, changePlaylistFn) => (enable) => {
   const playlist = loader.master.playlists[playlistUri];
-  const blacklisted = isBlacklisted(playlist);
+  const incompatible = isIncompatible(playlist);
   const currentlyEnabled = isEnabled(playlist);
 
   if (typeof enable === 'undefined') {
@@ -27,7 +27,7 @@ const enableFunction = (loader, playlistUri, changePlaylistFn, enable) => {
     playlist.disabled = true;
   }
 
-  if (enable !== currentlyEnabled && !blacklisted) {
+  if (enable !== currentlyEnabled && !incompatible) {
     // Ensure the outside world knows about our changes
     changePlaylistFn();
     if (enable) {
@@ -70,10 +70,9 @@ class Representation {
 
     // Partially-apply the enableFunction to create a playlist-
     // specific variant
-    this.enabled = enableFunction.bind(this,
-                                       hlsHandler.playlists,
-                                       playlist.uri,
-                                       fastChangeFunction);
+    this.enabled = enableFunction(hlsHandler.playlists,
+                                  playlist.uri,
+                                  fastChangeFunction);
   }
 }
 
@@ -91,7 +90,7 @@ let renditionSelectionMixin = function(hlsHandler) {
     return playlists
       .master
       .playlists
-      .filter((media) => !isBlacklisted(media))
+      .filter((media) => !isIncompatible(media))
       .map((e, i) => new Representation(hlsHandler, e, e.uri));
   };
 };

src/segment-loader.js

@@ -1098,7 +1098,8 @@ export default class SegmentLoader extends videojs.EventTarget {
 
     if (illegalMediaSwitchError) {
       this.error({
-        message: illegalMediaSwitchError
+        message: illegalMediaSwitchError,
+        blacklistDuration: Infinity
       });
       this.trigger('error');
       return;

test/playlist.test.js

@@ -686,6 +686,81 @@ QUnit.test('estimates segment request time based on bandwidth', function(assert)
   assert.equal(estimate, 8, 'takes into account bytes already received from download');
 });
 
+QUnit.module('Playlist enabled states', {
+  beforeEach(assert) {
+    this.env = useFakeEnvironment(assert);
+    this.clock = this.env.clock;
+  },
+  afterEach() {
+    this.env.restore();
+  }
+});
+
+QUnit.test('determines if a playlist is incompatible', function(assert) {
+  // incompatible means that the playlist was blacklisted due to incompatible
+  // configuration e.g. audio only stream when trying to playback audio and video.
+  // incompaatibility is denoted by a blacklist of Infinity.
+  assert.notOk(Playlist.isIncompatible({}),
+    'playlist not incompatible if no excludeUntil');
+
+  assert.notOk(Playlist.isIncompatible({ excludeUntil: 1 }),
+    'playlist not incompatible if expired blacklist');
+
+  assert.notOk(Playlist.isIncompatible({ excludeUntil: Date.now() + 9999 }),
+    'playlist not incompatible if temporarily blacklisted');
+
+  assert.ok(Playlist.isIncompatible({ excludeUntil: Infinity }),
+    'playlist is incompatible if excludeUntil is Infinity');
+});
+
+QUnit.test('determines if a playlist is blacklisted', function(assert) {
+  assert.notOk(Playlist.isBlacklisted({}),
+    'playlist not blacklisted if no excludeUntil');
+
+  assert.notOk(Playlist.isBlacklisted({ excludeUntil: Date.now() - 1 }),
+    'playlist not blacklisted if expired excludeUntil');
+
+  assert.ok(Playlist.isBlacklisted({ excludeUntil: Date.now() + 9999 }),
+    'playlist is blacklisted');
+
+  assert.ok(Playlist.isBlacklisted({ excludeUntil: Infinity }),
+    'playlist is blacklisted if excludeUntil is Infinity');
+});
+
+QUnit.test('determines if a playlist is disabled', function(assert) {
+  assert.notOk(Playlist.isDisabled({}), 'playlist not disabled');
+
+  assert.ok(Playlist.isDisabled({ disabled: true }), 'playlist is disabled');
+});
+
+QUnit.test('playlists with no or expired blacklist are enabled', function(assert) {
+  // enabled means not blacklisted and not disabled
+  assert.ok(Playlist.isEnabled({}), 'playlist with no blacklist is enabled');
+  assert.ok(Playlist.isEnabled({ excludeUntil: Date.now() - 1 }),
+    'playlist with expired blacklist is enabled');
+});
+
+QUnit.test('blacklisted playlists are not enabled', function(assert) {
+  // enabled means not blacklisted and not disabled
+  assert.notOk(Playlist.isEnabled({ excludeUntil: Date.now() + 9999 }),
+    'playlist with temporary blacklist is not enabled');
+  assert.notOk(Playlist.isEnabled({ excludeUntil: Infinity }),
+    'playlist with permanent is not enabled');
+});
+
+QUnit.test('manually disabled playlists are not enabled regardless of blacklist state',
+function(assert) {
+  // enabled means not blacklisted and not disabled
+  assert.notOk(Playlist.isEnabled({ disabled: true }),
+    'disabled playlist with no blacklist is not enabled');
+  assert.notOk(Playlist.isEnabled({ disabled: true, excludeUntil: Date.now() - 1 }),
+    'disabled playlist with expired blacklist is not enabled');
+  assert.notOk(Playlist.isEnabled({ disabled: true, excludeUntil: Date.now() + 9999 }),
+    'disabled playlist with temporary blacklist is not enabled');
+  assert.notOk(Playlist.isEnabled({ disabled: true, excludeUntil: Infinity }),
+    'disabled playlist with permanent blacklist is not enabled');
+});
+
 QUnit.module('Playlist isAes and isFmp4', {
   beforeEach(assert) {
     this.env = useFakeEnvironment(assert);

test/rendition-mixin.test.js

@@ -61,7 +61,8 @@ const makeMockHlsHandler = function(playlistOptions) {
     hlsHandler.playlists.master.playlists[i] = makeMockPlaylist(playlist);
 
     if (playlist.uri) {
-      hlsHandler.playlists.master.playlists[playlist.uri] = hlsHandler.playlists.master.playlists[i];
+      hlsHandler.playlists.master.playlists[playlist.uri] =
+        hlsHandler.playlists.master.playlists[i];
     }
   });
 
@@ -77,7 +78,8 @@ QUnit.test('adds the representations API to HlsHandler', function(assert) {
 
   RenditionMixin(hlsHandler);
 
-  assert.equal(typeof hlsHandler.representations, 'function', 'added the representations API');
+  assert.equal(typeof hlsHandler.representations, 'function',
+    'added the representations API');
 });
 
 QUnit.test('returns proper number of representations', function(assert) {
@@ -143,7 +145,8 @@ QUnit.test('returns representations with width and height if present', function(
   assert.equal(renditions[2].height, undefined, 'rendition has a height of undefined');
 });
 
-QUnit.test('blacklisted playlists are not included in the representations list', function(assert) {
+QUnit.test('incompatible playlists are not included in the representations list',
+function(assert) {
   let hlsHandler = makeMockHlsHandler([
     {
       bandwidth: 0,
@@ -175,13 +178,15 @@ QUnit.test('blacklisted playlists are not included in the representations list',
 
   let renditions = hlsHandler.representations();
 
-  assert.equal(renditions.length, 3, 'blacklisted rendition not added');
+  assert.equal(renditions.length, 4, 'incompatible rendition not added');
   assert.equal(renditions[0].id, 'media1.m3u8', 'rendition is enabled');
-  assert.equal(renditions[1].id, 'media3.m3u8', 'rendition is enabled');
-  assert.equal(renditions[2].id, 'media4.m3u8', 'rendition is enabled');
+  assert.equal(renditions[1].id, 'media2.m3u8', 'rendition is enabled');
+  assert.equal(renditions[2].id, 'media3.m3u8', 'rendition is enabled');
+  assert.equal(renditions[3].id, 'media4.m3u8', 'rendition is enabled');
 });
 
-QUnit.test('setting a representation to disabled sets disabled to true', function(assert) {
+QUnit.test('setting a representation to disabled sets disabled to true',
+function(assert) {
   let renditiondisabled = 0;
   let hlsHandler = makeMockHlsHandler([
     {
@@ -211,11 +216,14 @@ QUnit.test('setting a representation to disabled sets disabled to true', functio
   assert.equal(renditiondisabled, 1, 'renditiondisabled event has been triggered');
   assert.equal(playlists[0].disabled, true, 'rendition has been disabled');
   assert.equal(playlists[1].disabled, undefined, 'rendition has not been disabled');
-  assert.equal(playlists[0].excludeUntil, 0, 'excludeUntil not touched when disabling a rendition');
-  assert.equal(playlists[1].excludeUntil, 0, 'excludeUntil not touched when disabling a rendition');
+  assert.equal(playlists[0].excludeUntil, 0,
+    'excludeUntil not touched when disabling a rendition');
+  assert.equal(playlists[1].excludeUntil, 0,
+    'excludeUntil not touched when disabling a rendition');
 });
 
-QUnit.test('changing the enabled state of a representation calls fastQualityChange_', function(assert) {
+QUnit.test('changing the enabled state of a representation calls fastQualityChange_',
+function(assert) {
   let renditionEnabledEvents = 0;
   let hlsHandler = makeMockHlsHandler([
     {
@@ -239,12 +247,14 @@ QUnit.test('changing the enabled state of a representation calls fastQualityChan
   let renditions = hlsHandler.representations();
 
   assert.equal(mpc.fastQualityChange_.calls, 0, 'fastQualityChange_ was never called');
-  assert.equal(renditionEnabledEvents, 0, 'renditionenabled event has not been triggered');
+  assert.equal(renditionEnabledEvents, 0,
+    'renditionenabled event has not been triggered');
 
   renditions[0].enabled(true);
 
   assert.equal(mpc.fastQualityChange_.calls, 1, 'fastQualityChange_ was called once');
-  assert.equal(renditionEnabledEvents, 1, 'renditionenabled event has been triggered once');
+  assert.equal(renditionEnabledEvents, 1,
+    'renditionenabled event has been triggered once');
 
   renditions[1].enabled(false);