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(live): only reset playlist loader for LLHLS #1410

Merged
merged 2 commits into from
Jul 25, 2023
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
16 changes: 9 additions & 7 deletions src/segment-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -1048,13 +1048,15 @@ export default class SegmentLoader extends videojs.EventTarget {
// we must reset/resync the segment loader when we switch renditions and
// the segment loader is already synced to the previous rendition

// on playlist changes we want it to be possible to fetch
// at the buffer for vod but not for live. So we use resetLoader
// for live and resyncLoader for vod. We want this because
// if a playlist uses independent and non-independent segments/parts the
// buffer may not accurately reflect the next segment that we should try
// downloading.
if (!newPlaylist.endList) {
// We only want to reset the loader here for LLHLS playback, as resetLoader sets fetchAtBuffer_
// to false, resulting in fetching segments at currentTime and causing repeated
// same-segment requests on playlist change. This erroneously drives up the playback watcher
// stalled segment count, as re-requesting segments at the currentTime or browser cached segments
// will not change the buffer.
// Reference for LLHLS fixes: https://github.com/videojs/http-streaming/pull/1201
const isLLHLS = !newPlaylist.endList && typeof newPlaylist.partTargetDuration === 'number';

if (isLLHLS) {
this.resetLoader();
} else {
this.resyncLoader();
Expand Down
44 changes: 43 additions & 1 deletion test/loader-common.js
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ export const LoaderCommonFactory = ({
});
});

QUnit.test('live rendition switch uses resetLoader', function(assert) {
QUnit.test('live LLHLS rendition switch uses resetLoader', function(assert) {
return this.setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => {

loader.playlist(playlistWithDuration(50, {
Expand Down Expand Up @@ -896,6 +896,7 @@ export const LoaderCommonFactory = ({
});

newPlaylist.uri = 'playlist2.m3u8';
newPlaylist.partTargetDuration = 1;

loader.playlist(newPlaylist);

Expand All @@ -906,6 +907,47 @@ export const LoaderCommonFactory = ({
});
});

QUnit.test('live rendition switch uses resyncLoader', function(assert) {
return this.setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => {

loader.playlist(playlistWithDuration(50, {
mediaSequence: 0,
endList: false
}));

loader.load();
loader.mediaIndex = 0;
let resyncCalled = false;
let resetCalled = false;
const origReset = loader.resetLoader;
const origResync = loader.resyncLoader;

loader.resetLoader = function() {
resetCalled = true;
return origReset.call(loader);
};

loader.resyncLoader = function() {
resyncCalled = true;
return origResync.call(loader);
};

const newPlaylist = playlistWithDuration(50, {
mediaSequence: 0,
endList: false
});

newPlaylist.uri = 'playlist2.m3u8';

loader.playlist(newPlaylist);

assert.true(resyncCalled, 'resync was called');
assert.false(resetCalled, 'reset was not called');

return Promise.resolve();
});
});

QUnit.test('vod rendition switch uses resyncLoader', function(assert) {
return this.setupMediaSource(loader.mediaSource_, loader.sourceUpdater_).then(() => {

Expand Down