From bc94fbbb6e96dff751012324d7cdcf26e6346e2c Mon Sep 17 00:00:00 2001 From: Alex Barstow Date: Mon, 13 Aug 2018 12:22:12 -0400 Subject: [PATCH] fix: Remove buffered data on fast quality switches (#113) --- src/master-playlist-controller.js | 38 +++++++++++++-- src/segment-loader.js | 12 +++-- src/source-updater.js | 6 ++- src/videojs-http-streaming.js | 2 +- test/master-playlist-controller.test.js | 65 ++++++++++++++++++++++--- test/videojs-http-streaming.test.js | 4 +- 6 files changed, 107 insertions(+), 20 deletions(-) diff --git a/src/master-playlist-controller.js b/src/master-playlist-controller.js index f7ef9e71f..1200e3c61 100644 --- a/src/master-playlist-controller.js +++ b/src/master-playlist-controller.js @@ -314,6 +314,7 @@ export class MasterPlaylistController extends videojs.EventTarget { // update the SegmentLoader instead of doing it twice here and // on `loadedplaylist` this.mainSegmentLoader_.playlist(media, this.requestOptions_); + this.mainSegmentLoader_.load(); this.tech_.trigger({ @@ -496,13 +497,12 @@ export class MasterPlaylistController extends videojs.EventTarget { /** * Re-tune playback quality level for the current player - * conditions. This method may perform destructive actions, like - * removing already buffered content, to readjust the currently - * active playlist quickly. + * conditions without performing destructive actions, like + * removing already buffered content * * @private */ - fastQualityChange_() { + smoothQualityChange_() { let media = this.selectPlaylist(); if (media !== this.masterPlaylistLoader_.media()) { @@ -513,6 +513,36 @@ export class MasterPlaylistController extends videojs.EventTarget { } } + /** + * Re-tune playback quality level for the current player + * conditions. This method will perform destructive actions like removing + * already buffered content in order to readjust the currently active + * playlist quickly. This is good for manual quality changes + * + * @private + */ + fastQualityChange_() { + const media = this.selectPlaylist(); + + if (media === this.masterPlaylistLoader_.media()) { + return; + } + + this.masterPlaylistLoader_.media(media); + + // delete all buffered data to allow an immediate quality switch, then seek + // in place to give the browser a kick to remove any cached frames from the + // previous rendition + this.mainSegmentLoader_.resetEverything(() => { + // Since this is not a typical seek, we avoid the seekTo method which can cause + // segments from the previously enabled rendition to load before the new playlist + // has finished loading + this.tech_.setCurrentTime(this.tech_.currentTime()); + }); + + // don't need to reset audio as it is reset when media changes + } + /** * Begin playback. */ diff --git a/src/segment-loader.js b/src/segment-loader.js index f96e289e6..7d68e6caa 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -547,11 +547,13 @@ export default class SegmentLoader extends videojs.EventTarget { /** * Delete all the buffered data and reset the SegmentLoader + * @param {Function} [done] an optional callback to be executed when the remove + * operation is complete */ - resetEverything() { + resetEverything(done) { this.ended_ = false; this.resetLoader(); - this.remove(0, this.duration_()); + this.remove(0, this.duration_(), done); // clears fmp4 captions this.captionParser_.clearAllCaptions(); this.trigger('reseteverything'); @@ -582,10 +584,12 @@ export default class SegmentLoader extends videojs.EventTarget { * Remove any data in the source buffer between start and end times * @param {Number} start - the start time of the region to remove from the buffer * @param {Number} end - the end time of the region to remove from the buffer + * @param {Function} [done] - an optional callback to be executed when the remove + * operation is complete */ - remove(start, end) { + remove(start, end, done) { if (this.sourceUpdater_) { - this.sourceUpdater_.remove(start, end); + this.sourceUpdater_.remove(start, end, done); } removeCuesFromTrack(start, end, this.segmentMetadataTrack_); diff --git a/src/source-updater.js b/src/source-updater.js index 524beddef..b89993d3d 100644 --- a/src/source-updater.js +++ b/src/source-updater.js @@ -131,14 +131,16 @@ export default class SourceUpdater { * * @param {Number} start where to start the removal * @param {Number} end where to end the removal + * @param {Function} [done=noop] optional callback to be executed when the remove + * operation is complete * @see http://www.w3.org/TR/media-source/#widl-SourceBuffer-remove-void-double-start-unrestricted-double-end */ - remove(start, end) { + remove(start, end, done = noop) { if (this.processedAppend_) { this.queueCallback_(() => { this.logger_(`remove [${start} => ${end}]`); this.sourceBuffer_.remove(start, end); - }, noop); + }, done); } } diff --git a/src/videojs-http-streaming.js b/src/videojs-http-streaming.js index c45af5b55..557ecc4c9 100644 --- a/src/videojs-http-streaming.js +++ b/src/videojs-http-streaming.js @@ -320,7 +320,7 @@ class HlsHandler extends Component { document.msFullscreenElement; if (fullscreenElement && fullscreenElement.contains(this.tech_.el())) { - this.masterPlaylistController_.fastQualityChange_(); + this.masterPlaylistController_.smoothQualityChange_(); } }); this.on(this.tech_, 'error', function() { diff --git a/test/master-playlist-controller.test.js b/test/master-playlist-controller.test.js index 9ed06431a..1f82d4ee7 100644 --- a/test/master-playlist-controller.test.js +++ b/test/master-playlist-controller.test.js @@ -221,7 +221,7 @@ QUnit.test('selects lowest bitrate rendition when enableLowInitialPlaylist is se assert.equal(numCallsToSelectPlaylist, 0, 'selectPlaylist'); }); -QUnit.test('resyncs SegmentLoader for a fast quality change', function(assert) { +QUnit.test('resyncs SegmentLoader for a smooth quality change', function(assert) { let resyncs = 0; this.masterPlaylistController.mediaSource.trigger('sourceopen'); @@ -240,7 +240,7 @@ QUnit.test('resyncs SegmentLoader for a fast quality change', function(assert) { return this.masterPlaylistController.master().playlists[0]; }; - this.masterPlaylistController.fastQualityChange_(); + this.masterPlaylistController.smoothQualityChange_(); assert.equal(resyncs, 1, 'resynced the segmentLoader'); @@ -248,7 +248,7 @@ QUnit.test('resyncs SegmentLoader for a fast quality change', function(assert) { assert.equal(this.player.tech_.hls.stats.bandwidth, 4194304, 'default bandwidth'); }); -QUnit.test('does not resync the segmentLoader when no fast quality change occurs', +QUnit.test('does not resync the segmentLoader when no smooth quality change occurs', function(assert) { let resyncs = 0; @@ -264,14 +264,14 @@ QUnit.test('does not resync the segmentLoader when no fast quality change occurs resyncs++; }; - this.masterPlaylistController.fastQualityChange_(); + this.masterPlaylistController.smoothQualityChange_(); assert.equal(resyncs, 0, 'did not resync the segmentLoader'); // verify stats assert.equal(this.player.tech_.hls.stats.bandwidth, 4194304, 'default bandwidth'); }); -QUnit.test('fast quality change resyncs audio segment loader', function(assert) { +QUnit.test('smooth quality change resyncs audio segment loader', function(assert) { this.requests.length = 0; this.player = createPlayer(); this.player.src({ @@ -306,7 +306,7 @@ QUnit.test('fast quality change resyncs audio segment loader', function(assert) }; masterPlaylistController.audioSegmentLoader_.resyncLoader = () => resyncs++; - masterPlaylistController.fastQualityChange_(); + masterPlaylistController.smoothQualityChange_(); assert.equal(resyncs, 0, 'does not resync the audio segment loader when media same'); // force different media @@ -315,7 +315,7 @@ QUnit.test('fast quality change resyncs audio segment loader', function(assert) }; assert.equal(this.requests.length, 1, 'one request'); - masterPlaylistController.fastQualityChange_(); + masterPlaylistController.smoothQualityChange_(); assert.equal(this.requests.length, 2, 'added a request for new media'); assert.equal(resyncs, 0, 'does not resync the audio segment loader yet'); // new media request @@ -324,6 +324,57 @@ QUnit.test('fast quality change resyncs audio segment loader', function(assert) assert.equal(resets, 0, 'does not reset the audio segment loader when media changes'); }); +QUnit.test('resets everything for a fast quality change', function(assert) { + let resyncs = 0; + let resets = 0; + let removeFuncArgs = {}; + + this.masterPlaylistController.mediaSource.trigger('sourceopen'); + // master + this.standardXHRResponse(this.requests.shift()); + // media + this.standardXHRResponse(this.requests.shift()); + + let segmentLoader = this.masterPlaylistController.mainSegmentLoader_; + + segmentLoader.resyncLoader = () => resyncs++; + + segmentLoader.remove = function(start, end) { + removeFuncArgs = { + start, + end + }; + }; + + segmentLoader.duration_ = () => 60; + + segmentLoader.on('reseteverything', function() { + resets++; + }); + + // media is unchanged + this.masterPlaylistController.fastQualityChange_(); + + assert.equal(resyncs, 0, 'does not resync segment loader if media is unchanged'); + + assert.equal(resets, 0, 'reseteverything event not triggered if media is unchanged'); + + assert.deepEqual(removeFuncArgs, {}, 'remove() not called if media is unchanged'); + + // media is changed + this.masterPlaylistController.selectPlaylist = () => { + return this.masterPlaylistController.master().playlists[0]; + }; + + this.masterPlaylistController.fastQualityChange_(); + + assert.equal(resyncs, 1, 'resynced segment loader if media is changed'); + + assert.equal(resets, 1, 'reseteverything event triggered if media is changed'); + + assert.deepEqual(removeFuncArgs, {start: 0, end: 60}, 'remove() called with correct arguments if media is changed'); +}); + QUnit.test('audio segment loader is reset on audio track change', function(assert) { this.requests.length = 0; this.player = createPlayer(); diff --git a/test/videojs-http-streaming.test.js b/test/videojs-http-streaming.test.js index 6a00d79b0..aa0ae148c 100644 --- a/test/videojs-http-streaming.test.js +++ b/test/videojs-http-streaming.test.js @@ -3040,7 +3040,7 @@ QUnit.test('stats are reset on dispose', function(assert) { assert.equal(hls.stats.mediaBytesTransferred, 0, 'stat is reset'); }); -QUnit.test('detects fullscreen and triggers a quality change', function(assert) { +QUnit.test('detects fullscreen and triggers a smooth quality change', function(assert) { let qualityChanges = 0; let hls = HlsSourceHandler.handleSource({ src: 'manifest/master.m3u8', @@ -3056,7 +3056,7 @@ QUnit.test('detects fullscreen and triggers a quality change', function(assert) } }); - hls.masterPlaylistController_.fastQualityChange_ = function() { + hls.masterPlaylistController_.smoothQualityChange_ = function() { qualityChanges++; };