From daaeaac461d88b3b3bfcd6df0f39b66d2f8cebd4 Mon Sep 17 00:00:00 2001 From: Garrett Singer Date: Fri, 9 Apr 2021 13:07:10 -0400 Subject: [PATCH] fix: don't clear DASH minimum update period timeout on pause of a media loader Previously, for DASH, pausing a media loader would end up pausing the main loader as well (via removal of the main loader's minimum update period). For live streams which required manifest refreshes, this meant that on media changes, the main loader would be paused and sometimes never resumed. This change ensures that when a media loader is paused it won't remove the main loader's minimum update period timeout. In addition, a change was made so that loading a main DASH playlist loader recreates a minimum update period timeout if the main DASH loader was previously paused (and the timeout cleared). --- src/dash-playlist-loader.js | 15 ++++++- test/dash-playlist-loader.test.js | 68 +++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index f86e4d522..f963bdb58 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -517,9 +517,11 @@ export default class DashPlaylistLoader extends EventTarget { } this.stopRequest(); window.clearTimeout(this.mediaUpdateTimeout); - window.clearTimeout(this.masterPlaylistLoader_.minimumUpdatePeriodTimeout_); - this.masterPlaylistLoader_.minimumUpdatePeriodTimeout_ = null; this.mediaUpdateTimeout = null; + if (this.isMaster_) { + window.clearTimeout(this.masterPlaylistLoader_.minimumUpdatePeriodTimeout_); + this.masterPlaylistLoader_.minimumUpdatePeriodTimeout_ = null; + } if (this.state === 'HAVE_NOTHING') { // If we pause the loader before any data has been retrieved, its as if we never // started, so reset to an unstarted state. @@ -548,6 +550,15 @@ export default class DashPlaylistLoader extends EventTarget { } if (media && !media.endList) { + // Check to see if this is the master loader and the MUP was cleared (this happens + // when the loader was paused). `media` should be set at this point since one is always + // set during `start()`. + if (this.isMaster_ && !this.minimumUpdatePeriodTimeout_) { + // Trigger minimumUpdatePeriod to refresh the master manifest + this.trigger('minimumUpdatePeriod'); + // Since there was no prior minimumUpdatePeriodTimeout it should be recreated + this.updateMinimumUpdatePeriodTimeout_(); + } this.trigger('mediaupdatetimeout'); } else { this.trigger('loadedplaylist'); diff --git a/test/dash-playlist-loader.test.js b/test/dash-playlist-loader.test.js index 4ff1826ab..eaeff75bc 100644 --- a/test/dash-playlist-loader.test.js +++ b/test/dash-playlist-loader.test.js @@ -2690,3 +2690,71 @@ QUnit.test('load does not resume the media update timer for non live playlists', assert.notOk(loader.mediaUpdateTimeout, 'media update timeout not set'); }); + +QUnit.test('pause removes minimum update period timeout', function(assert) { + const loader = new DashPlaylistLoader('dash-live.mpd', this.fakeVhs); + + loader.load(); + this.standardXHRResponse(this.requests.shift()); + this.clock.tick(1); + + assert.ok(loader.minimumUpdatePeriodTimeout_, 'minimum update period timeout set'); + + loader.pause(); + + assert.notOk( + loader.minimumUpdatePeriodTimeout_, + 'minimum update period timeout not set' + ); +}); + +QUnit.test('load resumes minimum update period timeout for live', function(assert) { + const loader = new DashPlaylistLoader('dash-live.mpd', this.fakeVhs); + + loader.load(); + this.standardXHRResponse(this.requests.shift()); + this.clock.tick(1); + + // media should be selected at this point + loader.media(loader.master.playlists[0]); + + assert.ok(loader.minimumUpdatePeriodTimeout_, 'minimum update period timeout set'); + + loader.pause(); + + assert.notOk( + loader.minimumUpdatePeriodTimeout_, + 'minimum update period timeout not set' + ); + + loader.load(); + + assert.ok(loader.minimumUpdatePeriodTimeout_, 'minimum update period timeout set'); +}); + +QUnit.test('pause does not remove minimum update period timeout when not master', function(assert) { + const masterLoader = new DashPlaylistLoader('dash-live.mpd', this.fakeVhs); + + masterLoader.load(); + this.standardXHRResponse(this.requests.shift()); + this.clock.tick(1); + + const media = masterLoader.master.playlists[0]; + // media should be selected at this point + + masterLoader.media(media); + + const mediaLoader = new DashPlaylistLoader(media, this.fakeVhs, {}, masterLoader); + + assert.ok( + masterLoader.minimumUpdatePeriodTimeout_, + 'minimum update period timeout set' + ); + + mediaLoader.pause(); + + assert.ok( + masterLoader.minimumUpdatePeriodTimeout_, + 'minimum update period timeout set' + ); +});