diff --git a/src/dash-playlist-loader.js b/src/dash-playlist-loader.js index 623a8178c..1c061501e 100644 --- a/src/dash-playlist-loader.js +++ b/src/dash-playlist-loader.js @@ -304,6 +304,8 @@ export default class DashPlaylistLoader extends EventTarget { constructor(srcUrlOrPlaylist, vhs, options = { }, mainPlaylistLoader) { super(); + this.isPaused_ = true; + this.mainPlaylistLoader_ = mainPlaylistLoader || this; if (!mainPlaylistLoader) { this.isMain_ = true; @@ -345,6 +347,10 @@ export default class DashPlaylistLoader extends EventTarget { } } + get isPaused() { + return this.isPaused_; + } + requestErrored_(err, request, startingState) { // disposed if (!this.request) { @@ -384,6 +390,7 @@ export default class DashPlaylistLoader extends EventTarget { // playlist lacks sidx or sidx segments were added to this playlist already. if (!playlist.sidx || !sidxKey || this.mainPlaylistLoader_.sidxMapping_[sidxKey]) { // keep this function async + window.clearTimeout(this.mediaRequest_); this.mediaRequest_ = window.setTimeout(() => cb(false), 0); return; } @@ -465,6 +472,7 @@ export default class DashPlaylistLoader extends EventTarget { } dispose() { + this.isPaused_ = true; this.trigger('dispose'); this.stopRequest(); this.loadedPlaylists_ = {}; @@ -553,6 +561,7 @@ export default class DashPlaylistLoader extends EventTarget { haveMetadata({startingState, playlist}) { this.state = 'HAVE_METADATA'; this.loadedPlaylists_[playlist.id] = playlist; + window.clearTimeout(this.mediaRequest_); this.mediaRequest_ = null; // This will trigger loadedplaylist @@ -569,6 +578,8 @@ export default class DashPlaylistLoader extends EventTarget { } pause() { + this.isPaused_ = true; + if (this.mainPlaylistLoader_.createMupOnMedia_) { this.off('loadedmetadata', this.mainPlaylistLoader_.createMupOnMedia_); this.mainPlaylistLoader_.createMupOnMedia_ = null; @@ -588,6 +599,8 @@ export default class DashPlaylistLoader extends EventTarget { } load(isFinalRendition) { + this.isPaused_ = false; + window.clearTimeout(this.mediaUpdateTimeout); this.mediaUpdateTimeout = null; @@ -629,6 +642,7 @@ export default class DashPlaylistLoader extends EventTarget { // We don't need to request the main manifest again // Call this asynchronously to match the xhr request behavior below if (!this.isMain_) { + window.clearTimeout(this.mediaRequest_); this.mediaRequest_ = window.setTimeout(() => this.haveMain_(), 0); return; } @@ -773,6 +787,7 @@ export default class DashPlaylistLoader extends EventTarget { handleMain_() { // clear media request + window.clearTimeout(this.mediaRequest_); this.mediaRequest_ = null; const oldMain = this.mainPlaylistLoader_.main; diff --git a/src/playlist-controller.js b/src/playlist-controller.js index 1c996e155..b69bb39c1 100644 --- a/src/playlist-controller.js +++ b/src/playlist-controller.js @@ -29,6 +29,7 @@ import {merge, createTimeRanges} from './util/vjs-compat'; import { addMetadata, createMetadataTrackIfNotExists, addDateRangeMetadata } from './util/text-tracks'; import ContentSteeringController from './content-steering-controller'; import { bufferToHexString } from './util/string.js'; +import {debounce} from './util/fn'; const ABORT_EARLY_EXCLUSION_SECONDS = 10; @@ -152,6 +153,11 @@ export class PlaylistController extends videojs.EventTarget { constructor(options) { super(); + // Adding a slight debounce to avoid duplicate calls during rapid quality changes, for example: + // When selecting quality from the quality list, + // where we may have multiple bandwidth profiles for the same vertical resolution. + this.fastQualityChange_ = debounce(this.fastQualityChange_.bind(this), 100); + const { src, withCredentials, @@ -701,7 +707,16 @@ export class PlaylistController extends videojs.EventTarget { if (this.sourceType_ === 'dash') { // we don't want to re-request the same hls playlist right after it was changed - this.mainPlaylistLoader_.load(); + + // Initially it was implemented as workaround to restart playlist loader for live + // when playlist loader is paused because of playlist exclusions: + // see: https://github.com/videojs/http-streaming/pull/1339 + // but this introduces duplicate "loadedplaylist" event. + // Ideally we want to re-think playlist loader life-cycle events, + // but simply checking "paused" state should help a lot + if (this.mainPlaylistLoader_.isPaused) { + this.mainPlaylistLoader_.load(); + } } // TODO: Create a new event on the PlaylistLoader that signals @@ -962,6 +977,24 @@ export class PlaylistController extends videojs.EventTarget { this.tech_.setCurrentTime(newTime); }); + this.timelineChangeController_.on('fixBadTimelineChange', () => { + // pause, reset-everything and load for all segment-loaders + this.logger_('Fix bad timeline change. Restarting al segment loaders...'); + this.mainSegmentLoader_.pause(); + this.mainSegmentLoader_.resetEverything(); + if (this.mediaTypes_.AUDIO.activePlaylistLoader) { + this.audioSegmentLoader_.pause(); + this.audioSegmentLoader_.resetEverything(); + } + if (this.mediaTypes_.SUBTITLES.activePlaylistLoader) { + this.subtitleSegmentLoader_.pause(); + this.subtitleSegmentLoader_.resetEverything(); + } + + // start segment loader loading in case they are paused + this.load(); + }); + this.mainSegmentLoader_.on('earlyabort', (event) => { // never try to early abort with the new ABR algorithm if (this.bufferBasedABR) { @@ -1109,13 +1142,19 @@ export class PlaylistController extends videojs.EventTarget { runFastQualitySwitch_() { this.waitingForFastQualityPlaylistReceived_ = false; - // Delete all buffered data to allow an immediate quality switch. this.mainSegmentLoader_.pause(); - this.mainSegmentLoader_.resetEverything(() => { - this.mainSegmentLoader_.load(); - }); + this.mainSegmentLoader_.resetEverything(); + if (this.mediaTypes_.AUDIO.activePlaylistLoader) { + this.audioSegmentLoader_.pause(); + this.audioSegmentLoader_.resetEverything(); + } + if (this.mediaTypes_.SUBTITLES.activePlaylistLoader) { + this.subtitleSegmentLoader_.pause(); + this.subtitleSegmentLoader_.resetEverything(); + } - // don't need to reset audio as it is reset when media changes + // start segment loader loading in case they are paused + this.load(); } /** diff --git a/src/rendition-mixin.js b/src/rendition-mixin.js index 8a2e35a60..cd0127712 100644 --- a/src/rendition-mixin.js +++ b/src/rendition-mixin.js @@ -39,8 +39,9 @@ const enableFunction = (loader, playlistID, changePlaylistFn) => (enable) => { if (enable !== currentlyEnabled && !incompatible) { // Ensure the outside world knows about our changes - changePlaylistFn(playlist); if (enable) { + // call fast quality change only when the playlist is enabled + changePlaylistFn(playlist); loader.trigger({ type: 'renditionenabled', metadata}); } else { loader.trigger({ type: 'renditiondisabled', metadata}); diff --git a/src/segment-loader.js b/src/segment-loader.js index d1339452a..4ec5f2565 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -423,21 +423,6 @@ export const shouldFixBadTimelineChanges = (timelineChangeController) => { return false; }; -/** - * Fixes certain bad timeline scenarios by resetting the loader. - * - * @param {SegmentLoader} segmentLoader - */ -export const fixBadTimelineChange = (segmentLoader) => { - if (!segmentLoader) { - return; - } - - segmentLoader.pause(); - segmentLoader.resetEverything(); - segmentLoader.load(); -}; - /** * Check if the pending audio timeline change is behind the * pending main timeline change. @@ -480,7 +465,7 @@ const checkAndFixTimelines = (segmentLoader) => { return; } - fixBadTimelineChange(segmentLoader); + segmentLoader.timelineChangeController_.trigger('fixBadTimelineChange'); } }; @@ -872,6 +857,7 @@ export default class SegmentLoader extends videojs.EventTarget { if (this.pendingSegment_) { this.pendingSegment_ = null; } + this.timelineChangeController_.clearPendingTimelineChange(this.loaderType_); return; } @@ -1118,6 +1104,14 @@ export default class SegmentLoader extends videojs.EventTarget { return; } + if (this.playlist_ && + this.playlist_.endList && + newPlaylist.endList && + this.playlist_.uri === newPlaylist.uri) { + // skip update if both prev and new are vod and have the same URI + return; + } + const oldPlaylist = this.playlist_; const segmentInfo = this.pendingSegment_; diff --git a/src/util/fn.js b/src/util/fn.js new file mode 100644 index 000000000..a5e29048a --- /dev/null +++ b/src/util/fn.js @@ -0,0 +1,11 @@ +export const debounce = (callback, wait) => { + let timeoutId = null; + + return (...args) => { + clearTimeout(timeoutId); + + timeoutId = setTimeout(() => { + callback.apply(null, args); + }, wait); + }; +}; diff --git a/test/playlist-controller.test.js b/test/playlist-controller.test.js index 597e9eb54..c62813039 100644 --- a/test/playlist-controller.test.js +++ b/test/playlist-controller.test.js @@ -726,8 +726,9 @@ QUnit.test('resets everything for a fast quality change then calls load', functi segmentLoader.remove = (start, end, doneFn) => { assert.equal(end, Infinity, 'on a remove all, end should be Infinity'); - assert.ok(doneFn); - doneFn(); + if (doneFn) { + doneFn(); + } origRemove.call(segmentLoader, start, end, doneFn); }; @@ -4871,6 +4872,7 @@ QUnit.test('can pass or select a playlist for fastQualityChange', function(asser }; pc.fastQualityChange_(pc.main().playlists[1]); + this.clock.tick(110); pc.runFastQualitySwitch_(); assert.deepEqual(calls, { resetEverything: 1, @@ -4880,6 +4882,7 @@ QUnit.test('can pass or select a playlist for fastQualityChange', function(asser }, 'calls expected function when passed a playlist'); pc.fastQualityChange_(); + this.clock.tick(110); pc.runFastQualitySwitch_(); assert.deepEqual(calls, { resetEverything: 2, diff --git a/test/rendition-mixin.test.js b/test/rendition-mixin.test.js index 0997a6894..72be954d6 100644 --- a/test/rendition-mixin.test.js +++ b/test/rendition-mixin.test.js @@ -294,7 +294,7 @@ QUnit.test( renditions[1].enabled(false); - assert.equal(pc.fastQualityChange_.calls, 2, 'fastQualityChange_ was called twice'); + assert.equal(pc.fastQualityChange_.calls, 1, 'fastQualityChange_ was called once'); } ); diff --git a/test/segment-loader.test.js b/test/segment-loader.test.js index 0d90b2259..24fd672c7 100644 --- a/test/segment-loader.test.js +++ b/test/segment-loader.test.js @@ -10,8 +10,7 @@ import { getTroublesomeSegmentDurationMessage, getSyncSegmentCandidate, segmentInfoString, - shouldFixBadTimelineChanges, - fixBadTimelineChange + shouldFixBadTimelineChanges } from '../src/segment-loader'; import mp4probe from 'mux.js/lib/mp4/probe'; import { @@ -515,37 +514,6 @@ QUnit.test('shouldFixBadTimelineChange returns false when timelineChangeControll assert.notOk(shouldFixBadTimelineChanges(timelineChangeController), 'should not fix a timeline change with no timelineChangeController'); }); -QUnit.module('fixBadTimelineChange'); - -QUnit.test('fixBadTimelineChange calls pause, resetEverything and load on a segmentLoader', function(assert) { - let pauseCalls = 0; - let resetEverythingCalls = 0; - let loadCalls = 0; - let mockSegmentLoader = { - pause() { - pauseCalls++; - }, - resetEverything() { - resetEverythingCalls++; - }, - load() { - loadCalls++; - } - }; - - fixBadTimelineChange(mockSegmentLoader); - assert.equal(pauseCalls, 1, 'calls pause once'); - assert.equal(resetEverythingCalls, 1, 'calls resetEverything once'); - assert.equal(loadCalls, 1, 'calls load once'); - - // early return if undefined. call counts remain the same. - mockSegmentLoader = undefined; - fixBadTimelineChange(mockSegmentLoader); - assert.equal(pauseCalls, 1, 'calls pause once'); - assert.equal(resetEverythingCalls, 1, 'calls resetEverything once'); - assert.equal(loadCalls, 1, 'calls load once'); -}); - QUnit.module('safeBackBufferTrimTime'); QUnit.test('uses 30s before playhead when seekable start is 0', function(assert) { @@ -1668,17 +1636,11 @@ QUnit.module('SegmentLoader', function(hooks) { loader = new SegmentLoader(LoaderCommonSettings.call(this, { loaderType: 'audio' }), {}); - const origPause = loader.pause; const origLoad = loader.load; const origResetEverything = loader.resetEverything; - let pauseCalls = 0; let loadCalls = 0; let resetEverythingCalls = 0; - loader.pause = () => { - pauseCalls++; - origPause.call(loader); - }; loader.load = () => { loadCalls++; origLoad.call(loader); @@ -1701,6 +1663,14 @@ QUnit.module('SegmentLoader', function(hooks) { } }; + let fixBadTimelineChangeCount = 0; + + loader.timelineChangeController_.trigger = (type) => { + if (type === 'fixBadTimelineChange') { + fixBadTimelineChangeCount++; + } + }; + const playlist = playlistWithDuration(20); playlist.discontinuityStarts = [1]; @@ -1716,9 +1686,9 @@ QUnit.module('SegmentLoader', function(hooks) { loader.playlist(playlist); loader.load(); this.clock.tick(1); - assert.equal(pauseCalls, 1, '1 pause call expected'); - assert.equal(loadCalls, 2, '2 load calls expected'); - assert.equal(resetEverythingCalls, 2, '1 load calls expected'); + assert.equal(loadCalls, 1, '1 load calls expected'); + assert.equal(resetEverythingCalls, 1, '1 load calls expected'); + assert.equal(fixBadTimelineChangeCount, 1, '1 fixBadTimelineChangeCount triggered'); }); }); @@ -1727,17 +1697,11 @@ QUnit.module('SegmentLoader', function(hooks) { loader = new SegmentLoader(LoaderCommonSettings.call(this, { loaderType: 'main' }), {}); - const origPause = loader.pause; const origLoad = loader.load; const origResetEverything = loader.resetEverything; - let pauseCalls = 0; let loadCalls = 0; let resetEverythingCalls = 0; - loader.pause = () => { - pauseCalls++; - origPause.call(loader); - }; loader.load = () => { loadCalls++; origLoad.call(loader); @@ -1759,6 +1723,15 @@ QUnit.module('SegmentLoader', function(hooks) { }; } }; + + let fixBadTimelineChangeCount = 0; + + loader.timelineChangeController_.trigger = (type) => { + if (type === 'fixBadTimelineChange') { + fixBadTimelineChangeCount++; + } + }; + this.sourceUpdater_.ready = () => { return true; }; @@ -1783,9 +1756,10 @@ QUnit.module('SegmentLoader', function(hooks) { loader.playlist(playlist); loader.load(); this.clock.tick(1); - assert.equal(pauseCalls, 1, '1 pause call expected'); - assert.equal(loadCalls, 2, '2 load calls expected'); - assert.equal(resetEverythingCalls, 2, '1 load calls expected'); + assert.equal(loadCalls, 1, '1 load calls expected'); + assert.equal(resetEverythingCalls, 1, '1 load calls expected'); + assert.equal(fixBadTimelineChangeCount, 1, '1 fixBadTimelineChangeCount triggered'); + }); }); diff --git a/test/videojs-http-streaming.test.js b/test/videojs-http-streaming.test.js index 5fea187cc..d215269a0 100644 --- a/test/videojs-http-streaming.test.js +++ b/test/videojs-http-streaming.test.js @@ -4923,6 +4923,9 @@ QUnit.test('eme handles keystatuschange where status is output-restricted', func assert.equal(playlists[0].excludeUntil, Infinity, 'first HD playlist excluded'); assert.equal(playlists[1].excludeUntil, Infinity, 'second HD playlist excluded'); assert.equal(playlists[2].excludeUntil, undefined, 'non-HD playlist not excluded'); + + this.clock.tick(110); + assert.equal(switchMediaCalled, 1, 'switchMedia_ called once'); });