From 4e918fd91e472106ede6b4386dd73135898450d9 Mon Sep 17 00:00:00 2001 From: wseymour Date: Tue, 6 Aug 2024 13:31:34 -0500 Subject: [PATCH 1/7] fix: audio segment on incorrect timeline --- src/playlist-controller.js | 25 +++++++++++++++++++++++++ src/segment-loader.js | 5 +++++ 2 files changed, 30 insertions(+) diff --git a/src/playlist-controller.js b/src/playlist-controller.js index 46708c829..78d1b0ce9 100644 --- a/src/playlist-controller.js +++ b/src/playlist-controller.js @@ -929,6 +929,31 @@ export class PlaylistController extends videojs.EventTarget { this.onEndOfStream(); }); + // In DASH, there is the possibility of the video segment and the audio segment + // at a current time to be on different timelines. When this occurs, the player + // forwards playback to a point where these two segment types are back on the same + // timeline. This time will be just after the end of the audio segment that is on + // a previous timeline. + this.timelineChangeController_.on('audioTimelineBehind', () => { + const mainPendingTimelineChange = this.timelineChangeController_.pendingTimelineChange({ type: 'main' }); + + // Adjust the pending audio timeline to match the main timeline + this.timelineChangeController_.pendingTimelineChange({ to: mainPendingTimelineChange.to, from: mainPendingTimelineChange.from, type: 'audio' }); + + const segmentInfo = this.audioSegmentLoader_.pendingSegment_; + + if (!segmentInfo || !segmentInfo.segment || !segmentInfo.segment.syncInfo) { + return; + } + + // Update the current time to just after the faulty audio segment. + // This moves playback to a spot where both audio and video segments + // are on the same timeline. + const newTime = segmentInfo.segment.syncInfo.end + 0.01; + + this.tech_.setCurrentTime(newTime); + }); + this.mainSegmentLoader_.on('earlyabort', (event) => { // never try to early abort with the new ABR algorithm if (this.bufferBasedABR) { diff --git a/src/segment-loader.js b/src/segment-loader.js index c37e180a0..a6fd2e60c 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -413,6 +413,11 @@ export const shouldFixBadTimelineChanges = (timelineChangeController) => { const pendingAudioTimelineChange = timelineChangeController.pendingTimelineChange({ type: 'audio' }); const pendingMainTimelineChange = timelineChangeController.pendingTimelineChange({ type: 'main' }); const hasPendingTimelineChanges = pendingAudioTimelineChange && pendingMainTimelineChange; + + if (hasPendingTimelineChanges && pendingAudioTimelineChange.to < pendingMainTimelineChange.to) { + timelineChangeController.trigger('audioTimelineBehind'); + } + const differentPendingChanges = hasPendingTimelineChanges && pendingAudioTimelineChange.to !== pendingMainTimelineChange.to; const isNotInitialPendingTimelineChange = hasPendingTimelineChanges && pendingAudioTimelineChange.from !== -1 && pendingMainTimelineChange.from !== -1; From 129b89c9ca91076dd30b9e679d30cedfc4bc36bc Mon Sep 17 00:00:00 2001 From: wseymour Date: Wed, 7 Aug 2024 11:21:32 -0500 Subject: [PATCH 2/7] fix: improvements to remove conditional side effects --- src/playlist-controller.js | 30 ++++++++-------- src/segment-loader.js | 70 ++++++++++++++++++++++++++++++++------ 2 files changed, 75 insertions(+), 25 deletions(-) diff --git a/src/playlist-controller.js b/src/playlist-controller.js index 78d1b0ce9..b8c00b6d9 100644 --- a/src/playlist-controller.js +++ b/src/playlist-controller.js @@ -934,25 +934,27 @@ export class PlaylistController extends videojs.EventTarget { // forwards playback to a point where these two segment types are back on the same // timeline. This time will be just after the end of the audio segment that is on // a previous timeline. - this.timelineChangeController_.on('audioTimelineBehind', () => { - const mainPendingTimelineChange = this.timelineChangeController_.pendingTimelineChange({ type: 'main' }); + if (this.sourceType_ === 'dash') { + this.timelineChangeController_.on('audioTimelineBehind', () => { + const mainPendingTimelineChange = this.timelineChangeController_.pendingTimelineChange({ type: 'main' }); - // Adjust the pending audio timeline to match the main timeline - this.timelineChangeController_.pendingTimelineChange({ to: mainPendingTimelineChange.to, from: mainPendingTimelineChange.from, type: 'audio' }); + // Adjust the pending audio timeline to match the main timeline + this.timelineChangeController_.pendingTimelineChange({ to: mainPendingTimelineChange.to, from: mainPendingTimelineChange.from, type: 'audio' }); - const segmentInfo = this.audioSegmentLoader_.pendingSegment_; + const segmentInfo = this.audioSegmentLoader_.pendingSegment_; - if (!segmentInfo || !segmentInfo.segment || !segmentInfo.segment.syncInfo) { - return; - } + if (!segmentInfo || !segmentInfo.segment || !segmentInfo.segment.syncInfo) { + return; + } - // Update the current time to just after the faulty audio segment. - // This moves playback to a spot where both audio and video segments - // are on the same timeline. - const newTime = segmentInfo.segment.syncInfo.end + 0.01; + // Update the current time to just after the faulty audio segment. + // This moves playback to a spot where both audio and video segments + // are on the same timeline. + const newTime = segmentInfo.segment.syncInfo.end + 0.01; - this.tech_.setCurrentTime(newTime); - }); + this.tech_.setCurrentTime(newTime); + }); + } this.mainSegmentLoader_.on('earlyabort', (event) => { // never try to early abort with the new ABR algorithm diff --git a/src/segment-loader.js b/src/segment-loader.js index a6fd2e60c..5c750b443 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -413,11 +413,6 @@ export const shouldFixBadTimelineChanges = (timelineChangeController) => { const pendingAudioTimelineChange = timelineChangeController.pendingTimelineChange({ type: 'audio' }); const pendingMainTimelineChange = timelineChangeController.pendingTimelineChange({ type: 'main' }); const hasPendingTimelineChanges = pendingAudioTimelineChange && pendingMainTimelineChange; - - if (hasPendingTimelineChanges && pendingAudioTimelineChange.to < pendingMainTimelineChange.to) { - timelineChangeController.trigger('audioTimelineBehind'); - } - const differentPendingChanges = hasPendingTimelineChanges && pendingAudioTimelineChange.to !== pendingMainTimelineChange.to; const isNotInitialPendingTimelineChange = hasPendingTimelineChanges && pendingAudioTimelineChange.from !== -1 && pendingMainTimelineChange.from !== -1; @@ -428,15 +423,58 @@ export const shouldFixBadTimelineChanges = (timelineChangeController) => { return false; }; +/** + * Fixes certain bad timeline scenarios. In some cases this may be + * resetting the loaders. In others, it may force a timeline change + * when a timeline is behind. + * + * @param {SegmentLoader} segmentLoader + */ export const fixBadTimelineChange = (segmentLoader) => { if (!segmentLoader) { return; } + + const pendingAudioTimelineChange = segmentLoader.timelineChangeController_.pendingTimelineChange({ type: 'audio' }); + const pendingMainTimelineChange = segmentLoader.timelineChangeController_.pendingTimelineChange({ type: 'main' }); + const hasPendingTimelineChanges = pendingAudioTimelineChange && pendingMainTimelineChange; + + if (hasPendingTimelineChanges && pendingAudioTimelineChange.to < pendingMainTimelineChange.to) { + segmentLoader.timelineChangeController_.trigger('audioTimelineBehind'); + return; + } + segmentLoader.pause(); segmentLoader.resetEverything(); segmentLoader.load(); }; +/** + * A method to check if the player is waiting for a timeline change, and fixes + * certain scenarios where the timelines need to be updated. + * + * @param {SegmentLoader} segmentLoader + */ +export const checkAndFixTimelines = (segmentLoader) => { + const segmentInfo = segmentLoader.pendingSegment_; + + if (!segmentInfo) { + return; + } + + const waitingForTimelineChange = shouldWaitForTimelineChange({ + timelineChangeController: segmentLoader.timelineChangeController_, + currentTimeline: segmentLoader.currentTimeline_, + segmentTimeline: segmentInfo.timeline, + loaderType: segmentLoader.loaderType_, + audioDisabled: segmentLoader.audioDisabled_ + }); + + if (waitingForTimelineChange && shouldFixBadTimelineChanges(segmentLoader.timelineChangeController_)) { + fixBadTimelineChange(segmentLoader); + } +}; + export const mediaDuration = (timingInfos) => { let maxDuration = 0; @@ -703,6 +741,8 @@ export default class SegmentLoader extends videojs.EventTarget { this.sourceUpdater_.on('ready', () => { if (this.hasEnoughInfoToAppend_()) { this.processCallQueue_(); + } else { + checkAndFixTimelines(this); } }); @@ -717,6 +757,8 @@ export default class SegmentLoader extends videojs.EventTarget { this.timelineChangeController_.on('pendingtimelinechange', () => { if (this.hasEnoughInfoToAppend_()) { this.processCallQueue_(); + } else { + checkAndFixTimelines(this); } }); } @@ -728,9 +770,13 @@ export default class SegmentLoader extends videojs.EventTarget { this.trigger({type: 'timelinechange', ...metadata }); if (this.hasEnoughInfoToLoad_()) { this.processLoadQueue_(); + } else { + checkAndFixTimelines(this); } if (this.hasEnoughInfoToAppend_()) { this.processCallQueue_(); + } else { + checkAndFixTimelines(this); } }); } @@ -1966,6 +2012,8 @@ Fetch At Buffer: ${this.fetchAtBuffer_} // check if any calls were waiting on the track info if (this.hasEnoughInfoToAppend_()) { this.processCallQueue_(); + } else { + checkAndFixTimelines(this); } } @@ -1986,6 +2034,8 @@ Fetch At Buffer: ${this.fetchAtBuffer_} // check if any calls were waiting on the timing info if (this.hasEnoughInfoToAppend_()) { this.processCallQueue_(); + } else { + checkAndFixTimelines(this); } } @@ -2161,9 +2211,6 @@ Fetch At Buffer: ${this.fetchAtBuffer_} audioDisabled: this.audioDisabled_ }) ) { - if (shouldFixBadTimelineChanges(this.timelineChangeController_)) { - fixBadTimelineChange(this); - } return false; } @@ -2224,9 +2271,6 @@ Fetch At Buffer: ${this.fetchAtBuffer_} audioDisabled: this.audioDisabled_ }) ) { - if (shouldFixBadTimelineChanges(this.timelineChangeController_)) { - fixBadTimelineChange(this); - } return false; } @@ -2243,6 +2287,8 @@ Fetch At Buffer: ${this.fetchAtBuffer_} // If there's anything in the call queue, then this data came later and should be // executed after the calls currently queued. if (this.callQueue_.length || !this.hasEnoughInfoToAppend_()) { + checkAndFixTimelines(this); + this.callQueue_.push(this.handleData_.bind(this, simpleSegment, result)); return; } @@ -2632,6 +2678,8 @@ Fetch At Buffer: ${this.fetchAtBuffer_} } if (!this.hasEnoughInfoToLoad_()) { + checkAndFixTimelines(this); + this.loadQueue_.push(() => { // regenerate the audioAppendStart, timestampOffset, etc as they // may have changed since this function was added to the queue. From 4c3e8b297aa0e50e6ba1bcb3efd35a6d2e314bbd Mon Sep 17 00:00:00 2001 From: wseymour Date: Wed, 7 Aug 2024 11:36:23 -0500 Subject: [PATCH 3/7] move some functions around --- src/segment-loader.js | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/segment-loader.js b/src/segment-loader.js index 5c750b443..0d2237973 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -424,9 +424,7 @@ export const shouldFixBadTimelineChanges = (timelineChangeController) => { }; /** - * Fixes certain bad timeline scenarios. In some cases this may be - * resetting the loaders. In others, it may force a timeline change - * when a timeline is behind. + * Fixes certain bad timeline scenarios by resetting the loader. * * @param {SegmentLoader} segmentLoader */ @@ -435,18 +433,28 @@ export const fixBadTimelineChange = (segmentLoader) => { return; } + segmentLoader.pause(); + segmentLoader.resetEverything(); + segmentLoader.load(); +}; + +/** + * Check if the pending audio timeline change is behind the + * pending main timeline change. + * + * @param {SegmentLoader} segmentLoader + * @return {boolean} + */ +const isAudioTimelineBehind = (segmentLoader) => { const pendingAudioTimelineChange = segmentLoader.timelineChangeController_.pendingTimelineChange({ type: 'audio' }); const pendingMainTimelineChange = segmentLoader.timelineChangeController_.pendingTimelineChange({ type: 'main' }); const hasPendingTimelineChanges = pendingAudioTimelineChange && pendingMainTimelineChange; if (hasPendingTimelineChanges && pendingAudioTimelineChange.to < pendingMainTimelineChange.to) { - segmentLoader.timelineChangeController_.trigger('audioTimelineBehind'); - return; + return true; } - segmentLoader.pause(); - segmentLoader.resetEverything(); - segmentLoader.load(); + return false; }; /** @@ -455,7 +463,7 @@ export const fixBadTimelineChange = (segmentLoader) => { * * @param {SegmentLoader} segmentLoader */ -export const checkAndFixTimelines = (segmentLoader) => { +const checkAndFixTimelines = (segmentLoader) => { const segmentInfo = segmentLoader.pendingSegment_; if (!segmentInfo) { @@ -471,6 +479,11 @@ export const checkAndFixTimelines = (segmentLoader) => { }); if (waitingForTimelineChange && shouldFixBadTimelineChanges(segmentLoader.timelineChangeController_)) { + if (isAudioTimelineBehind) { + segmentLoader.timelineChangeController_.trigger('audioTimelineBehind'); + return; + } + fixBadTimelineChange(segmentLoader); } }; From 8ee9993df374d4a3eb86806868e518496c9b22ad Mon Sep 17 00:00:00 2001 From: wseymour Date: Wed, 7 Aug 2024 12:06:51 -0500 Subject: [PATCH 4/7] remove unneeded code --- src/playlist-controller.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/playlist-controller.js b/src/playlist-controller.js index b8c00b6d9..0a61ad8bc 100644 --- a/src/playlist-controller.js +++ b/src/playlist-controller.js @@ -936,11 +936,6 @@ export class PlaylistController extends videojs.EventTarget { // a previous timeline. if (this.sourceType_ === 'dash') { this.timelineChangeController_.on('audioTimelineBehind', () => { - const mainPendingTimelineChange = this.timelineChangeController_.pendingTimelineChange({ type: 'main' }); - - // Adjust the pending audio timeline to match the main timeline - this.timelineChangeController_.pendingTimelineChange({ to: mainPendingTimelineChange.to, from: mainPendingTimelineChange.from, type: 'audio' }); - const segmentInfo = this.audioSegmentLoader_.pendingSegment_; if (!segmentInfo || !segmentInfo.segment || !segmentInfo.segment.syncInfo) { From 2e3fe0c6f8be9da9c7b82160245705ce827fb133 Mon Sep 17 00:00:00 2001 From: wseymour Date: Wed, 7 Aug 2024 13:27:05 -0500 Subject: [PATCH 5/7] update audio timeline logic --- src/segment-loader.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/segment-loader.js b/src/segment-loader.js index 0d2237973..40debfc60 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -450,11 +450,7 @@ const isAudioTimelineBehind = (segmentLoader) => { const pendingMainTimelineChange = segmentLoader.timelineChangeController_.pendingTimelineChange({ type: 'main' }); const hasPendingTimelineChanges = pendingAudioTimelineChange && pendingMainTimelineChange; - if (hasPendingTimelineChanges && pendingAudioTimelineChange.to < pendingMainTimelineChange.to) { - return true; - } - - return false; + return hasPendingTimelineChanges && pendingAudioTimelineChange.to < pendingMainTimelineChange.to; }; /** From ee1277ccdd14cf1af4a24f88b9348ff030511008 Mon Sep 17 00:00:00 2001 From: wseymour Date: Wed, 7 Aug 2024 16:30:35 -0500 Subject: [PATCH 6/7] add tests --- src/segment-loader.js | 2 +- test/playlist-controller.test.js | 46 ++++++++++++++++++++++++++++++++ test/segment-loader.test.js | 45 +++++++++++++++++++++++++++++-- 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/src/segment-loader.js b/src/segment-loader.js index 40debfc60..877209e78 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -475,7 +475,7 @@ const checkAndFixTimelines = (segmentLoader) => { }); if (waitingForTimelineChange && shouldFixBadTimelineChanges(segmentLoader.timelineChangeController_)) { - if (isAudioTimelineBehind) { + if (isAudioTimelineBehind(segmentLoader)) { segmentLoader.timelineChangeController_.trigger('audioTimelineBehind'); return; } diff --git a/test/playlist-controller.test.js b/test/playlist-controller.test.js index a58e7aa3c..f50b8eb67 100644 --- a/test/playlist-controller.test.js +++ b/test/playlist-controller.test.js @@ -2647,6 +2647,52 @@ QUnit.test( } ); +QUnit.test( + 'setCurrentTime is not called on audioTimelineBehind when there is no pending segment', + function(assert) { + const options = { + src: 'test', + tech: this.player.tech_, + sourceType: 'dash', + player_: this.player + }; + const pc = new PlaylistController(options); + const tech = this.player.tech_; + const setCurrentTimeSpy = sinon.spy(tech, 'setCurrentTime'); + + pc.timelineChangeController_.trigger('audioTimelineBehind'); + + assert.notOk(setCurrentTimeSpy.called, 'setCurrentTime not called'); + } +); + +QUnit.test( + 'setCurrentTime to after audio segment when audioTimelineBehind is triggered', + function(assert) { + const options = { + src: 'test', + tech: this.player.tech_, + sourceType: 'dash', + player_: this.player + }; + const pc = new PlaylistController(options); + const tech = this.player.tech_; + const setCurrentTimeSpy = sinon.spy(tech, 'setCurrentTime'); + + pc.audioSegmentLoader_.pendingSegment_ = { + segment: { + syncInfo: { + end: 10 + } + } + }; + + pc.timelineChangeController_.trigger('audioTimelineBehind'); + + assert.ok(setCurrentTimeSpy.calledWith(10.01), 'sets current time to just after the end of the audio segment'); + } +); + QUnit.test('calls to update cues on new media', function(assert) { const origVhsOptions = videojs.options.vhs; diff --git a/test/segment-loader.test.js b/test/segment-loader.test.js index 3b5063935..1a27e982d 100644 --- a/test/segment-loader.test.js +++ b/test/segment-loader.test.js @@ -1663,7 +1663,7 @@ QUnit.module('SegmentLoader', function(hooks) { }); }); - QUnit.test('hasEnoughInfoToLoad_ calls fixBadTimelineChange', function(assert) { + QUnit.test('fixBadTimelineChange on load', function(assert) { loader.dispose(); loader = new SegmentLoader(LoaderCommonSettings.call(this, { loaderType: 'audio' @@ -1722,7 +1722,7 @@ QUnit.module('SegmentLoader', function(hooks) { }); }); - QUnit.test('hasEnoughInfoToAppend_ calls fixBadTimelineChange', function(assert) { + QUnit.test('fixBadTimelineChange on append', function(assert) { loader.dispose(); loader = new SegmentLoader(LoaderCommonSettings.call(this, { loaderType: 'main' @@ -1789,6 +1789,47 @@ QUnit.module('SegmentLoader', function(hooks) { }); }); + QUnit.test('triggers event when audio timeline is behind on bad timelines', function(assert) { + loader.dispose(); + loader = new SegmentLoader(LoaderCommonSettings.call(this, { + loaderType: 'audio' + }), {}); + + loader.timelineChangeController_.pendingTimelineChange = ({ type }) => { + if (type === 'audio') { + return { + from: 0, + to: 5 + }; + } else if (type === 'main') { + return { + from: 0, + to: 10 + }; + } + }; + + const triggerSpy = sinon.spy(loader.timelineChangeController_, 'trigger'); + + const playlist = playlistWithDuration(20); + + playlist.discontinuityStarts = [1]; + loader.getCurrentMediaInfo_ = () => { + return { + hasVideo: true, + hasAudio: true, + isMuxed: true + }; + }; + + return this.setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => { + loader.playlist(playlist); + loader.load(); + this.clock.tick(1); + assert.ok(triggerSpy.calledWith('audioTimelineBehind'), 'audio timeline behind event is triggered'); + }); + }); + QUnit.test('audio loader does not wait to request segment even if timestamp offset is nonzero', function(assert) { loader.dispose(); loader = new SegmentLoader(LoaderCommonSettings.call(this, { From efe1d0d7f7c07df80e2a03a306dad5855a62f55b Mon Sep 17 00:00:00 2001 From: wseymour Date: Wed, 7 Aug 2024 16:46:39 -0500 Subject: [PATCH 7/7] add additional dash check --- src/segment-loader.js | 3 ++- test/segment-loader.test.js | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/segment-loader.js b/src/segment-loader.js index 877209e78..b98321d6a 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -475,7 +475,8 @@ const checkAndFixTimelines = (segmentLoader) => { }); if (waitingForTimelineChange && shouldFixBadTimelineChanges(segmentLoader.timelineChangeController_)) { - if (isAudioTimelineBehind(segmentLoader)) { + // Audio being behind should only happen on DASH sources. + if (segmentLoader.sourceType_ === 'dash' && isAudioTimelineBehind(segmentLoader)) { segmentLoader.timelineChangeController_.trigger('audioTimelineBehind'); return; } diff --git a/test/segment-loader.test.js b/test/segment-loader.test.js index 1a27e982d..dafdbd506 100644 --- a/test/segment-loader.test.js +++ b/test/segment-loader.test.js @@ -1789,10 +1789,11 @@ QUnit.module('SegmentLoader', function(hooks) { }); }); - QUnit.test('triggers event when audio timeline is behind on bad timelines', function(assert) { + QUnit.test('triggers event when DASH audio timeline is behind main', function(assert) { loader.dispose(); loader = new SegmentLoader(LoaderCommonSettings.call(this, { - loaderType: 'audio' + loaderType: 'audio', + sourceType: 'dash' }), {}); loader.timelineChangeController_.pendingTimelineChange = ({ type }) => {