Skip to content
This repository has been archived by the owner on Jan 12, 2019. It is now read-only.

Commit

Permalink
clear the blacklist for other playlists if final rendition errors
Browse files Browse the repository at this point in the history
  • Loading branch information
Shan Zhuang committed Apr 11, 2017
1 parent 1fceb3e commit 3fabb71
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 15 deletions.
16 changes: 12 additions & 4 deletions src/master-playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -1059,14 +1059,22 @@ export class MasterPlaylistController extends videojs.EventTarget {
}

let isFinalRendition = this.masterPlaylistLoader_.isFinalRendition_();
let playlists = this.tech_.hls.playlists.master.playlists;

if (isFinalRendition) {
// Never blacklisting this playlist because it's final rendition
if (playlists.length === 1) {
// Never blacklisting this playlist because it's the only playlist
videojs.log.warn('Problem encountered with the current ' +
'HLS playlist. Trying again since it is the final playlist.');
'HLS playlist. Trying again since it is the only playlist.');

return this.masterPlaylistLoader_.load(isFinalRendition);
}
if (isFinalRendition) {
playlists.forEach((playlist) => {
// clear the blacklist duration for the other playlists when
// the final playlist errors
playlist.excludeUntil = Date.now();
});
}
// Blacklist this playlist
currentPlaylist.excludeUntil = Date.now() + BLACKLIST_DURATION;

Expand All @@ -1077,7 +1085,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
(error.message ? ' ' + error.message : '') +
' Switching to another playlist.');

return this.masterPlaylistLoader_.media(nextPlaylist);
return this.masterPlaylistLoader_.media(nextPlaylist, isFinalRendition);
}

/**
Expand Down
11 changes: 10 additions & 1 deletion src/playlist-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ const PlaylistLoader = function(srcUrl, hls, withCredentials) {
* object to switch to
* @return {Playlist} the current loaded media
*/
loader.media = function(playlist) {
loader.media = function(playlist, isFinalRendition) {
let startingState = loader.state;
let mediaChange;

Expand All @@ -295,6 +295,15 @@ const PlaylistLoader = function(srcUrl, hls, withCredentials) {
return loader.media_;
}

window.clearTimeout(mediaUpdateTimeout);

if (isFinalRendition) {
let refreshDelay = (playlist.targetDuration / 2) * 1000 || 5 * 1000;

mediaUpdateTimeout = window.setTimeout(loader.media.bind(loader, playlist, false), refreshDelay);
return;
}

// setter
if (loader.state === 'HAVE_NOTHING') {
throw new Error('Cannot switch media playlist from ' + loader.state);
Expand Down
19 changes: 9 additions & 10 deletions test/videojs-contrib-hls.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1099,11 +1099,8 @@ QUnit.test('playlist 404 should blacklist media', function(assert) {
media = this.player.tech_.hls.playlists.master.playlists[url];

// media wasn't blacklisted because it's final rendition
assert.ok(!media.excludeUntil, 'media not blacklisted after playlist 404');
assert.ok(media.excludeUntil, 'second media was blacklisted after playlist 404');
assert.equal(this.env.log.warn.calls, 1, 'warning logged for blacklist');
assert.equal(this.env.log.warn.args[1],
'Problem encountered with the current HLS playlist. Trying again since it is the final playlist.',
'log specific error message for final playlist');

this.clock.tick(2 * 1000);
// no new request was made since it hasn't been half the segment duration
Expand All @@ -1112,9 +1109,11 @@ QUnit.test('playlist 404 should blacklist media', function(assert) {
this.clock.tick(3 * 1000);
// continue loading the final remaining playlist after it wasn't blacklisted
// when half the segment duaration passed

assert.strictEqual(4, this.requests.length, 'one more request was made');
// the first media was unblacklisted after a refresh delay
assert.strictEqual(this.requests[3].url,
absoluteUrl('manifest/media1.m3u8'),
absoluteUrl('manifest/media.m3u8'),
'media playlist requested');

// verify stats
Expand Down Expand Up @@ -1192,11 +1191,11 @@ QUnit.test('never blacklist the playlist if it is the only playlist', function(a
this.requests.shift().respond(404);
media = this.player.tech_.hls.playlists.media();

// media wasn't blacklisted because it's final rendition
// media wasn't blacklisted because it's the only rendition
assert.ok(!media.excludeUntil, 'media was not blacklisted after playlist 404');
assert.equal(this.env.log.warn.calls, 1, 'warning logged for blacklist');
assert.equal(this.env.log.warn.args[0],
'Problem encountered with the current HLS playlist. Trying again since it is the final playlist.',
'Problem encountered with the current HLS playlist. Trying again since it is the only playlist.',
'log specific error message for final playlist');
});

Expand All @@ -1219,12 +1218,12 @@ QUnit.test('error on the first playlist request does not trigger an error ' +
let url = this.requests[1].url.slice(this.requests[1].url.lastIndexOf('/') + 1);
let media = this.player.tech_.hls.playlists.master.playlists[url];

// media wasn't blacklisted because it's final rendition
// media wasn't blacklisted because it's the only rendition
assert.ok(!media.excludeUntil, 'media was not blacklisted after playlist 404');
assert.equal(this.env.log.warn.calls, 1, 'warning logged for blacklist');
assert.equal(this.env.log.warn.args[0],
'Problem encountered with the current HLS playlist. Trying again since it is the final playlist.',
'log specific error message for final playlist');
'Problem encountered with the current HLS playlist. Trying again since it is the only playlist.',
'log specific error message for the only playlist');
});

QUnit.test('seeking in an empty playlist is a non-erroring noop', function(assert) {
Expand Down

0 comments on commit 3fabb71

Please sign in to comment.