Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: audio segment on incorrect timeline #1530

Merged
merged 7 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading