Skip to content

Commit

Permalink
fix: improvements to remove conditional side effects
Browse files Browse the repository at this point in the history
  • Loading branch information
wseymour15 committed Aug 7, 2024
1 parent 4e918fd commit 129b89c
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 25 deletions.
30 changes: 16 additions & 14 deletions src/playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
70 changes: 59 additions & 11 deletions src/segment-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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

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

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

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

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

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

Expand All @@ -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;
}
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 129b89c

Please sign in to comment.