diff --git a/src/playlist-controller.js b/src/playlist-controller.js index 5723f4c9a..e37ea48af 100644 --- a/src/playlist-controller.js +++ b/src/playlist-controller.js @@ -364,14 +364,14 @@ export class PlaylistController extends videojs.EventTarget { /** * Run selectPlaylist and switch to the new playlist if we should * + * @param {string} [reason=abr] a reason for why the ABR check is made * @private - * */ - checkABR_() { + checkABR_(reason = 'abr') { const nextPlaylist = this.selectPlaylist(); if (nextPlaylist && this.shouldSwitchToMedia_(nextPlaylist)) { - this.switchMedia_(nextPlaylist, 'abr'); + this.switchMedia_(nextPlaylist, reason); } } @@ -766,17 +766,26 @@ export class PlaylistController extends videojs.EventTarget { * @private */ setupSegmentLoaderListeners_() { - if (!this.bufferBasedABR) { - this.mainSegmentLoader_.on('bandwidthupdate', () => { - const nextPlaylist = this.selectPlaylist(); - - if (this.shouldSwitchToMedia_(nextPlaylist)) { - this.switchMedia_(nextPlaylist, 'bandwidthupdate'); - } + this.mainSegmentLoader_.on('bandwidthupdate', () => { + // Whether or not buffer based ABR or another ABR is used, on a bandwidth change it's + // useful to check to see if a rendition switch should be made. + this.checkABR_('bandwidthupdate'); + this.tech_.trigger('bandwidthupdate'); + }); - this.tech_.trigger('bandwidthupdate'); - }); + this.mainSegmentLoader_.on('timeout', () => { + if (this.bufferBasedABR) { + // If a rendition change is needed, then it would've be done on `bandwidthupdate`. + // Here the only consideration is that for buffer based ABR there's no guarantee + // of an immediate switch (since the bandwidth is averaged with a timeout + // bandwidth value of 1), so force a load on the segment loader to keep it going. + this.mainSegmentLoader_.load(); + } + }); + // `progress` events are not reliable enough of a bandwidth measure to trigger buffer + // based ABR. + if (!this.bufferBasedABR) { this.mainSegmentLoader_.on('progress', () => { this.trigger('progress'); }); diff --git a/src/segment-loader.js b/src/segment-loader.js index b36509ca8..3b26a2f04 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -2635,6 +2635,7 @@ export default class SegmentLoader extends videojs.EventTarget { this.bandwidth = 1; this.roundTrip = NaN; this.trigger('bandwidthupdate'); + this.trigger('timeout'); } /** diff --git a/test/loader-common.js b/test/loader-common.js index 9964fa5c2..6681c6094 100644 --- a/test/loader-common.js +++ b/test/loader-common.js @@ -364,6 +364,22 @@ export const LoaderCommonFactory = ({ assert.ok(isNaN(loader.roundTrip), 'reset round trip time'); }); + QUnit.test('segment request timeout triggers timeout event', function(assert) { + let timeoutEvents = 0; + + loader.on('timeout', () => timeoutEvents++); + loader.playlist(playlistWithDuration(10)); + + loader.load(); + this.clock.tick(1); + + this.requests[0].timedout = true; + // arbitrary length of time that should lead to a timeout + this.clock.tick(100 * 1000); + + assert.equal(timeoutEvents, 1, 'triggered timeout event'); + }); + QUnit.test('progress on segment requests are redispatched', function(assert) { let progressEvents = 0; diff --git a/test/playlist-controller.test.js b/test/playlist-controller.test.js index 2fb13d0ee..9d9806ea6 100644 --- a/test/playlist-controller.test.js +++ b/test/playlist-controller.test.js @@ -1790,11 +1790,8 @@ QUnit.test('selects a playlist after main/combined segment downloads', function( }); QUnit.test('does not select a playlist after segment downloads if only one playlist', function(assert) { - const origWarn = videojs.log.warn; let calls = 0; - const warnings = []; - videojs.log.warn = (text) => warnings.push(text); this.playlistController.selectPlaylist = () => { calls++; return null; @@ -1809,15 +1806,6 @@ QUnit.test('does not select a playlist after segment downloads if only one playl // "downloaded" a segment this.playlistController.mainSegmentLoader_.trigger('bandwidthupdate'); assert.strictEqual(calls, 2, 'selects after the initial segment'); - - assert.equal(warnings.length, 1, 'one warning logged'); - assert.equal( - warnings[0], - 'We received no playlist to switch to. Please check your stream.', - 'we logged the correct warning' - ); - - videojs.log.warn = origWarn; }); QUnit.test('re-triggers bandwidthupdate events on the tech', function(assert) { @@ -5759,17 +5747,40 @@ QUnit.test('Determines if playlist should change on bandwidthupdate/progress fro // "downloaded" a segment this.playlistController.mainSegmentLoader_.trigger('bandwidthupdate'); - assert.strictEqual(calls, 1, 'does not select after segment download'); + assert.strictEqual(calls, 2, 'selects after segment download'); this.clock.tick(250); - assert.strictEqual(calls, 2, 'selects after clock tick'); + assert.strictEqual(calls, 3, 'selects after clock tick'); this.clock.tick(1000); - assert.strictEqual(calls, 6, 'selects after clock tick, 1000 is 4x250'); + assert.strictEqual(calls, 7, 'selects after clock tick, 1000 is 4x250'); // verify stats assert.equal(this.player.tech_.vhs.stats.bandwidth, 4194304, 'default bandwidth'); }); +QUnit.test('loads main segment loader on timeout', function(assert) { + const mainSegmentLoader = this.playlistController.mainSegmentLoader_; + + this.playlistController.mediaSource.trigger('sourceopen'); + + // master + this.standardXHRResponse(this.requests.shift()); + // media + this.standardXHRResponse(this.requests.shift()); + + let loadCalls = 0; + + mainSegmentLoader.load = () => loadCalls++; + + this.playlistController.mainSegmentLoader_.trigger('bandwidthupdate'); + + assert.equal(loadCalls, 0, 'does not call load'); + + this.playlistController.mainSegmentLoader_.trigger('timeout'); + + assert.equal(loadCalls, 1, 'calls load'); +}); + QUnit.module('PlaylistController shouldSwitchToMedia', sharedHooks); QUnit.test('true if a no current playlist', function(assert) {