From 876ed8c2bd55b8cc01bfb90541863ede9bffcebd Mon Sep 17 00:00:00 2001 From: Walter Seymour Date: Mon, 12 Aug 2024 11:00:21 -0500 Subject: [PATCH] fix: audio segment on incorrect timeline (#1530) --- src/playlist-controller.js | 22 ++++++++++ src/segment-loader.js | 75 +++++++++++++++++++++++++++++--- test/playlist-controller.test.js | 46 ++++++++++++++++++++ test/segment-loader.test.js | 46 +++++++++++++++++++- 4 files changed, 181 insertions(+), 8 deletions(-) diff --git a/src/playlist-controller.js b/src/playlist-controller.js index 46708c829..0a61ad8bc 100644 --- a/src/playlist-controller.js +++ b/src/playlist-controller.js @@ -929,6 +929,28 @@ 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. + if (this.sourceType_ === 'dash') { + this.timelineChangeController_.on('audioTimelineBehind', () => { + 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..b98321d6a 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -423,15 +423,68 @@ 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. + * + * @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; + + return hasPendingTimelineChanges && pendingAudioTimelineChange.to < pendingMainTimelineChange.to; +}; + +/** + * 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 + */ +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_)) { + // Audio being behind should only happen on DASH sources. + if (segmentLoader.sourceType_ === 'dash' && isAudioTimelineBehind(segmentLoader)) { + segmentLoader.timelineChangeController_.trigger('audioTimelineBehind'); + return; + } + + fixBadTimelineChange(segmentLoader); + } +}; + export const mediaDuration = (timingInfos) => { let maxDuration = 0; @@ -698,6 +751,8 @@ export default class SegmentLoader extends videojs.EventTarget { this.sourceUpdater_.on('ready', () => { if (this.hasEnoughInfoToAppend_()) { this.processCallQueue_(); + } else { + checkAndFixTimelines(this); } }); @@ -712,6 +767,8 @@ export default class SegmentLoader extends videojs.EventTarget { this.timelineChangeController_.on('pendingtimelinechange', () => { if (this.hasEnoughInfoToAppend_()) { this.processCallQueue_(); + } else { + checkAndFixTimelines(this); } }); } @@ -723,9 +780,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); } }); } @@ -1961,6 +2022,8 @@ Fetch At Buffer: ${this.fetchAtBuffer_} // check if any calls were waiting on the track info if (this.hasEnoughInfoToAppend_()) { this.processCallQueue_(); + } else { + checkAndFixTimelines(this); } } @@ -1981,6 +2044,8 @@ Fetch At Buffer: ${this.fetchAtBuffer_} // check if any calls were waiting on the timing info if (this.hasEnoughInfoToAppend_()) { this.processCallQueue_(); + } else { + checkAndFixTimelines(this); } } @@ -2156,9 +2221,6 @@ Fetch At Buffer: ${this.fetchAtBuffer_} audioDisabled: this.audioDisabled_ }) ) { - if (shouldFixBadTimelineChanges(this.timelineChangeController_)) { - fixBadTimelineChange(this); - } return false; } @@ -2219,9 +2281,6 @@ Fetch At Buffer: ${this.fetchAtBuffer_} audioDisabled: this.audioDisabled_ }) ) { - if (shouldFixBadTimelineChanges(this.timelineChangeController_)) { - fixBadTimelineChange(this); - } return false; } @@ -2238,6 +2297,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; } @@ -2627,6 +2688,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. 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..dafdbd506 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,48 @@ QUnit.module('SegmentLoader', function(hooks) { }); }); + QUnit.test('triggers event when DASH audio timeline is behind main', function(assert) { + loader.dispose(); + loader = new SegmentLoader(LoaderCommonSettings.call(this, { + loaderType: 'audio', + sourceType: 'dash' + }), {}); + + 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, {