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: bad timeline changes #1526

Merged
merged 10 commits into from
Jul 22, 2024
34 changes: 34 additions & 0 deletions src/segment-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,33 @@ export const shouldWaitForTimelineChange = ({
return false;
};

export const shouldFixBadTimelineChanges = (timelineChangeController) => {
if (!timelineChangeController) {
return false;
}
const pendingAudioTimelineChange = timelineChangeController.pendingTimelineChange({ type: 'audio' });
const pendingMainTimelineChange = timelineChangeController.pendingTimelineChange({ type: 'main' });
const hasPendingTimelineChanges = pendingAudioTimelineChange && pendingMainTimelineChange;
const differentPendingChanges = hasPendingTimelineChanges && pendingAudioTimelineChange.to !== pendingMainTimelineChange.to;
const isNotInitialPendingTimelineChange = hasPendingTimelineChanges && pendingAudioTimelineChange.from !== -1 && pendingMainTimelineChange.from !== -1;

if (isNotInitialPendingTimelineChange && differentPendingChanges) {
return true;
}

return false;
};

export const fixBadTimelineChange = (segmentLoader) => {
if (!segmentLoader) {
return;
}
segmentLoader.pause();
segmentLoader.abort_();
segmentLoader.resetEverything();
dzianis-dashkevich marked this conversation as resolved.
Show resolved Hide resolved
segmentLoader.load();
};

export const mediaDuration = (timingInfos) => {
let maxDuration = 0;

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

Expand Down Expand Up @@ -2180,6 +2210,7 @@ Fetch At Buffer: ${this.fetchAtBuffer_}
return false;
}

// we need to allow an append here even if we're moving to different timelines.
if (
shouldWaitForTimelineChange({
timelineChangeController: this.timelineChangeController_,
Expand All @@ -2189,6 +2220,9 @@ Fetch At Buffer: ${this.fetchAtBuffer_}
audioDisabled: this.audioDisabled_
})
) {
if (shouldFixBadTimelineChanges(this.timelineChangeController_)) {
fixBadTimelineChange(this);
}
return false;
}

Expand Down
214 changes: 213 additions & 1 deletion test/segment-loader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import {
mediaDuration,
getTroublesomeSegmentDurationMessage,
getSyncSegmentCandidate,
segmentInfoString
segmentInfoString,
shouldFixBadTimelineChanges,
fixBadTimelineChange
} from '../src/segment-loader';
import mp4probe from 'mux.js/lib/mp4/probe';
import {
Expand Down Expand Up @@ -465,6 +467,90 @@ QUnit.test('main loader does not wait if pending audio timeline change matches s
);
});

QUnit.module('shouldFixBadTimelineChange');

QUnit.test('shouldFixBadTimelineChange returns true when timelines are both changing to different timelines', function(assert) {
const timelineChangeController = {
pendingTimelineChange({ type }) {
if (type === 'audio') {
return { from: 1, to: 2 };
} else if (type === 'main') {
return { from: 2, to: 1 };
}
}
};

assert.ok(shouldFixBadTimelineChanges(timelineChangeController), 'should fix a bad timeline change');
});

QUnit.test('shouldFixBadTimelineChange returns false when only one timeline has a pending change', function(assert) {
const timelineChangeController = {
pendingTimelineChange({ type }) {
if (type === 'audio') {
return { from: 1, to: 2 };
}
}
};

assert.notOk(shouldFixBadTimelineChanges(timelineChangeController), 'should not fix a timeline change');
});

QUnit.test('shouldFixBadTimelineChange returns false when both timelines are changing to the same value', function(assert) {
const timelineChangeController = {
pendingTimelineChange({ type }) {
if (type === 'audio') {
return { from: 1, to: 2 };
} else if (type === 'main') {
return { from: 1, to: 2 };
}
}
};

assert.notOk(shouldFixBadTimelineChanges(timelineChangeController), 'should not fix a good timeline change');
});

QUnit.test('shouldFixBadTimelineChange returns false when timelineChangeController is undefined', function(assert) {
const timelineChangeController = undefined;

assert.notOk(shouldFixBadTimelineChanges(timelineChangeController), 'should not fix a timeline change with no timelineChangeController');
});

QUnit.module('fixBadTimelineChange');

QUnit.test('fixBadTimelineChange calls pause, resetEverything and load on a segmentLoader', function(assert) {
let pauseCalls = 0;
let resetEverythingCalls = 0;
let loadCalls = 0;
let abortCalls = 0;
let mockSegmentLoader = {
pause() {
pauseCalls++;
},
abort_() {
abortCalls++;
},
resetEverything() {
resetEverythingCalls++;
},
load() {
loadCalls++;
}
};

fixBadTimelineChange(mockSegmentLoader);
assert.equal(pauseCalls, 1, 'calls pause once');
assert.equal(resetEverythingCalls, 1, 'calls resetEverything once');
assert.equal(loadCalls, 1, 'calls load once');

// early return if undefined. call counts remain the same.
mockSegmentLoader = undefined;
fixBadTimelineChange(mockSegmentLoader);
assert.equal(pauseCalls, 1, 'calls pause once');
assert.equal(resetEverythingCalls, 1, 'calls resetEverything once');
assert.equal(loadCalls, 1, 'calls load once');
assert.equal(abortCalls, 1, 'calls abort once');
});

QUnit.module('safeBackBufferTrimTime');

QUnit.test('uses 30s before playhead when seekable start is 0', function(assert) {
Expand Down Expand Up @@ -1582,6 +1668,132 @@ QUnit.module('SegmentLoader', function(hooks) {
});
});

QUnit.test('hasEnoughInfoToLoad_ calls fixBadTimelineChange', function(assert) {
loader.dispose();
loader = new SegmentLoader(LoaderCommonSettings.call(this, {
loaderType: 'audio'
}), {});
const origPause = loader.pause;
const origLoad = loader.load;
const origResetEverything = loader.resetEverything;
let pauseCalls = 0;
let loadCalls = 0;
let resetEverythingCalls = 0;

loader.pause = () => {
pauseCalls++;
origPause.call(loader);
};
loader.load = () => {
loadCalls++;
origLoad.call(loader);
};
loader.resetEverything = () => {
resetEverythingCalls++;
origResetEverything.call(loader);
};
loader.timelineChangeController_.pendingTimelineChange = ({ type }) => {
if (type === 'audio') {
return {
from: 3,
to: 2
};
} else if (type === 'main') {
return {
from: 0,
to: 1
};
}
};

const playlist = playlistWithDuration(20);

playlist.discontinuityStarts = [1];
loader.getCurrentMediaInfo_ = () => {
return {
hasVideo: true,
hasAudio: false,
isMuxed: false
};
};

return this.setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => {
loader.playlist(playlist);
loader.load();
this.clock.tick(1);
assert.equal(pauseCalls, 1, '1 pause call expected');
assert.equal(loadCalls, 2, '2 load calls expected');
assert.equal(resetEverythingCalls, 2, '1 load calls expected');
});
});

QUnit.test('hasEnoughInfoToAppend_ calls fixBadTimelineChange', function(assert) {
loader.dispose();
loader = new SegmentLoader(LoaderCommonSettings.call(this, {
loaderType: 'main'
}), {});
const origPause = loader.pause;
const origLoad = loader.load;
const origResetEverything = loader.resetEverything;
let pauseCalls = 0;
let loadCalls = 0;
let resetEverythingCalls = 0;

loader.pause = () => {
pauseCalls++;
origPause.call(loader);
};
loader.load = () => {
loadCalls++;
origLoad.call(loader);
};
loader.resetEverything = () => {
resetEverythingCalls++;
origResetEverything.call(loader);
};
loader.timelineChangeController_.pendingTimelineChange = ({ type }) => {
if (type === 'audio') {
return {
from: 3,
to: 2
};
} else if (type === 'main') {
return {
from: 0,
to: 1
};
}
};
this.sourceUpdater_.ready = () => {
return true;
};

const playlist = playlistWithDuration(20);

playlist.discontinuityStarts = [1];
loader.getCurrentMediaInfo_ = () => {
return {
hasVideo: true,
hasAudio: false,
isMuxed: false
};
};
loader.pendingSegment_ = {
foo: 'bar',
videoTimingInfo: 1
};
loader.audioDisabled_ = true;

return this.setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => {
loader.playlist(playlist);
loader.load();
this.clock.tick(1);
assert.equal(pauseCalls, 1, '1 pause call expected');
assert.equal(loadCalls, 2, '2 load calls expected');
assert.equal(resetEverythingCalls, 2, '1 load calls expected');
});
});

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