Skip to content

Commit

Permalink
fix: audio segment on incorrect timeline (#1530)
Browse files Browse the repository at this point in the history
  • Loading branch information
wseymour15 authored Aug 12, 2024
1 parent d23539a commit 876ed8c
Show file tree
Hide file tree
Showing 4 changed files with 181 additions and 8 deletions.
22 changes: 22 additions & 0 deletions src/playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
75 changes: 69 additions & 6 deletions src/segment-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -698,6 +751,8 @@ export default class SegmentLoader extends videojs.EventTarget {
this.sourceUpdater_.on('ready', () => {
if (this.hasEnoughInfoToAppend_()) {
this.processCallQueue_();
} else {
checkAndFixTimelines(this);
}
});

Expand All @@ -712,6 +767,8 @@ export default class SegmentLoader extends videojs.EventTarget {
this.timelineChangeController_.on('pendingtimelinechange', () => {
if (this.hasEnoughInfoToAppend_()) {
this.processCallQueue_();
} else {
checkAndFixTimelines(this);
}
});
}
Expand All @@ -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);
}
});
}
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down Expand Up @@ -2156,9 +2221,6 @@ Fetch At Buffer: ${this.fetchAtBuffer_}
audioDisabled: this.audioDisabled_
})
) {
if (shouldFixBadTimelineChanges(this.timelineChangeController_)) {
fixBadTimelineChange(this);
}
return false;
}

Expand Down Expand Up @@ -2219,9 +2281,6 @@ Fetch At Buffer: ${this.fetchAtBuffer_}
audioDisabled: this.audioDisabled_
})
) {
if (shouldFixBadTimelineChanges(this.timelineChangeController_)) {
fixBadTimelineChange(this);
}
return false;
}

Expand All @@ -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;
}
Expand Down Expand Up @@ -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.
Expand Down
46 changes: 46 additions & 0 deletions test/playlist-controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
46 changes: 44 additions & 2 deletions test/segment-loader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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, {
Expand Down

0 comments on commit 876ed8c

Please sign in to comment.