Merge pull request #444 from videojs/blacklist
Fix two bugs in the blacklisting facility
Showing
3 changed files
with
41 additions
and
4 deletions
... | @@ -110,6 +110,7 @@ | ... | @@ -110,6 +110,7 @@ |
110 | 110 | ||
111 | if (error) { | 111 | if (error) { |
112 | loader.error = { | 112 | loader.error = { |
113 | playlist: loader.master.playlists[url], | ||
113 | status: xhr.status, | 114 | status: xhr.status, |
114 | message: 'HLS playlist request error at URL: ' + url, | 115 | message: 'HLS playlist request error at URL: ' + url, |
115 | responseText: xhr.responseText, | 116 | responseText: xhr.responseText, | ... | ... |
... | @@ -909,10 +909,19 @@ videojs.HlsHandler.prototype.setBandwidth = function(xhr) { | ... | @@ -909,10 +909,19 @@ videojs.HlsHandler.prototype.setBandwidth = function(xhr) { |
909 | this.tech_.trigger('bandwidthupdate'); | 909 | this.tech_.trigger('bandwidthupdate'); |
910 | }; | 910 | }; |
911 | 911 | ||
912 | /* | ||
913 | * Blacklists a playlist when an error occurs for a set amount of time | ||
914 | * making it unavailable for selection by the rendition selection algorithm | ||
915 | * and then forces a new playlist (rendition) selection. | ||
916 | */ | ||
912 | videojs.HlsHandler.prototype.blacklistCurrentPlaylist_ = function(error) { | 917 | videojs.HlsHandler.prototype.blacklistCurrentPlaylist_ = function(error) { |
913 | var currentPlaylist, nextPlaylist; | 918 | var currentPlaylist, nextPlaylist; |
914 | 919 | ||
915 | currentPlaylist = this.playlists.media(); | 920 | // If the `error` was generated by the playlist loader, it will contain |
921 | // the playlist we were trying to load (but failed) and that should be | ||
922 | // blacklisted instead of the currently selected playlist which is likely | ||
923 | // out-of-date in this scenario | ||
924 | currentPlaylist = error.playlist || this.playlists.media(); | ||
916 | 925 | ||
917 | // If there is no current playlist, then an error occurred while we were | 926 | // If there is no current playlist, then an error occurred while we were |
918 | // trying to load the master OR while we were disposing of the tech | 927 | // trying to load the master OR while we were disposing of the tech |
... | @@ -921,15 +930,15 @@ videojs.HlsHandler.prototype.blacklistCurrentPlaylist_ = function(error) { | ... | @@ -921,15 +930,15 @@ videojs.HlsHandler.prototype.blacklistCurrentPlaylist_ = function(error) { |
921 | return this.mediaSource.endOfStream('network'); | 930 | return this.mediaSource.endOfStream('network'); |
922 | } | 931 | } |
923 | 932 | ||
933 | // Blacklist this playlist | ||
934 | currentPlaylist.excludeUntil = Date.now() + blacklistDuration; | ||
935 | |||
924 | // Select a new playlist | 936 | // Select a new playlist |
925 | nextPlaylist = this.selectPlaylist(); | 937 | nextPlaylist = this.selectPlaylist(); |
926 | 938 | ||
927 | if (nextPlaylist) { | 939 | if (nextPlaylist) { |
928 | videojs.log.warn('Problem encountered with the current HLS playlist. Switching to another playlist.'); | 940 | videojs.log.warn('Problem encountered with the current HLS playlist. Switching to another playlist.'); |
929 | 941 | ||
930 | // Blacklist this playlist | ||
931 | currentPlaylist.excludeUntil = Date.now() + blacklistDuration; | ||
932 | |||
933 | return this.playlists.media(nextPlaylist); | 942 | return this.playlists.media(nextPlaylist); |
934 | } else { | 943 | } else { |
935 | videojs.log.warn('Problem encountered with the current HLS playlist. No suitable alternatives found.'); | 944 | videojs.log.warn('Problem encountered with the current HLS playlist. No suitable alternatives found.'); | ... | ... |
... | @@ -1514,6 +1514,33 @@ test('segment 404 should trigger blacklisting of media', function () { | ... | @@ -1514,6 +1514,33 @@ test('segment 404 should trigger blacklisting of media', function () { |
1514 | ok(media.excludeUntil > 0, 'original media blacklisted for some time'); | 1514 | ok(media.excludeUntil > 0, 'original media blacklisted for some time'); |
1515 | }); | 1515 | }); |
1516 | 1516 | ||
1517 | test('playlist 404 should blacklist media', function () { | ||
1518 | var media, url; | ||
1519 | |||
1520 | player.src({ | ||
1521 | src: 'manifest/master.m3u8', | ||
1522 | type: 'application/vnd.apple.mpegurl' | ||
1523 | }); | ||
1524 | openMediaSource(player); | ||
1525 | |||
1526 | player.tech_.hls.bandwidth = 1e10; | ||
1527 | requests[0].respond(200, null, | ||
1528 | '#EXTM3U\n' + | ||
1529 | '#EXT-X-STREAM-INF:BANDWIDTH=1000\n' + | ||
1530 | 'media.m3u8\n' + | ||
1531 | '#EXT-X-STREAM-INF:BANDWIDTH=1\n' + | ||
1532 | 'media1.m3u8\n'); // master | ||
1533 | |||
1534 | equal(player.tech_.hls.playlists.media_, undefined, 'no media is initially set'); | ||
1535 | |||
1536 | requests[1].respond(400); // media | ||
1537 | |||
1538 | url = requests[1].url.slice(requests[1].url.lastIndexOf('/') + 1); | ||
1539 | media = player.tech_.hls.playlists.master.playlists[url]; | ||
1540 | |||
1541 | ok(media.excludeUntil > 0, 'original media blacklisted for some time'); | ||
1542 | }); | ||
1543 | |||
1517 | test('seeking in an empty playlist is a non-erroring noop', function() { | 1544 | test('seeking in an empty playlist is a non-erroring noop', function() { |
1518 | var requestsLength; | 1545 | var requestsLength; |
1519 | 1546 | ... | ... |
-
Please register or sign in to post a comment