Skip to content

Commit

Permalink
fix: fastQualityChange refactor (#1414)
Browse files Browse the repository at this point in the history
  • Loading branch information
@adrums86
adrums86 authored Aug 14, 2023
1 parent bf0a300 commit 4590bdd
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 91 deletions.
36 changes: 22 additions & 14 deletions src/playlist-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -919,9 +919,9 @@ export class PlaylistController extends videojs.EventTarget {

/**
* Re-tune playback quality level for the current player
* conditions. This method will perform destructive actions like removing
* already buffered content in order to readjust the currently active
* playlist quickly. This is good for manual quality changes
* conditions. This will reset the main segment loader
* and the next segment position to the currentTime.
* This is good for manual quality changes.
*
* @private
*/
Expand All @@ -930,20 +930,28 @@ export class PlaylistController extends videojs.EventTarget {
this.logger_('skipping fastQualityChange because new media is same as old');
return;
}

this.switchMedia_(media, 'fast-quality');
// Reset main segment loader properties and next segment position information.
// Don't need to reset audio as it is reset when media changes.
// We resetLoaderProperties separately here as we want to fetch init segments if
// necessary and ensure we're not in an ended state when we switch playlists.
this.resetMainLoaderReplaceSegments();
}

// Delete all buffered data to allow an immediate quality switch, then seek to give
// the browser a kick to remove any cached frames from the previous rendtion (.04 seconds
// ahead was roughly the minimum that will accomplish this across a variety of content
// in IE and Edge, but seeking in place is sufficient on all other browsers)
// Edge/IE bug: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/14600375/
// Chrome bug: https://bugs.chromium.org/p/chromium/issues/detail?id=651904
this.mainSegmentLoader_.resetEverything(() => {
this.tech_.setCurrentTime(this.tech_.currentTime());
});
/**
* Sets the replaceUntil flag on the main segment soader to the buffered end
* and resets the main segment loaders properties.
*/
resetMainLoaderReplaceSegments() {
const buffered = this.tech_.buffered();
const bufferedEnd = buffered.end(buffered.length - 1);

// don't need to reset audio as it is reset when media changes
// Set the replace segments flag to the buffered end, this forces fetchAtBuffer
// on the main loader to remain, false after the resetLoader call, until we have
// replaced all content buffered ahead of the currentTime.
this.mainSegmentLoader_.replaceSegmentsUntil = bufferedEnd;
this.mainSegmentLoader_.resetLoaderProperties();
this.mainSegmentLoader_.resetLoader();
}

/**
Expand Down
36 changes: 30 additions & 6 deletions src/segment-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,8 @@ export default class SegmentLoader extends videojs.EventTarget {

// ...for determining the fetch location
this.fetchAtBuffer_ = false;
// For comparing with currentTime when overwriting segments on fastQualityChange_ changes. Use -1 as the inactive flag.
this.replaceSegmentsUntil_ = -1;

this.logger_ = logger(`SegmentLoader[${this.loaderType_}]`);

Expand Down Expand Up @@ -1157,18 +1159,25 @@ export default class SegmentLoader extends videojs.EventTarget {
}

/**
* Delete all the buffered data and reset the SegmentLoader
*
* @param {Function} [done] an optional callback to be executed when the remove
* operation is complete
* Resets the segment loader ended and init properties.
*/
resetEverything(done) {
resetLoaderProperties() {
this.ended_ = false;
this.activeInitSegmentId_ = null;
this.appendInitSegment_ = {
audio: true,
video: true
};
}

/**
* Delete all the buffered data and reset the SegmentLoader
*
* @param {Function} [done] an optional callback to be executed when the remove
* operation is complete
*/
resetEverything(done) {
this.resetLoaderProperties();
this.resetLoader();

// remove from 0, the earliest point, to Infinity, to signify removal of everything.
Expand Down Expand Up @@ -3050,7 +3059,10 @@ export default class SegmentLoader extends videojs.EventTarget {
this.logger_(`Appended ${segmentInfoString(segmentInfo)}`);

this.addSegmentMetadataCue_(segmentInfo);
this.fetchAtBuffer_ = true;
if (this.currentTime_() >= this.replaceSegmentsUntil_) {
this.replaceSegmentsUntil_ = -1;
this.fetchAtBuffer_ = true;
}
if (this.currentTimeline_ !== segmentInfo.timeline) {
this.timelineChangeController_.lastTimelineChange({
type: this.loaderType_,
Expand Down Expand Up @@ -3204,4 +3216,16 @@ export default class SegmentLoader extends videojs.EventTarget {

this.segmentMetadataTrack_.addCue(cue);
}

/**
* Public setter for defining the private replaceSegmentsUntil_ property, which
* determines when we can return fetchAtBuffer to true if overwriting the buffer.
*
* @param {number} bufferedEnd the end of the buffered range to replace segments
* until currentTime reaches this time.
*/
set replaceSegmentsUntil(bufferedEnd) {
this.logger_(`Replacing currently buffered segments until ${bufferedEnd}`);
this.replaceSegmentsUntil_ = bufferedEnd;
}
}
1 change: 1 addition & 0 deletions src/videojs-http-streaming.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,7 @@ class VhsHandler extends Component {
);

// Clear the buffer before switching playlists, since it may already contain unplayable segments
this.playlistController_.mainSegmentLoader_.resetEverything();
this.playlistController_.fastQualityChange_();
}
});
Expand Down
81 changes: 10 additions & 71 deletions test/playlist-controller.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,8 @@ QUnit.test('resets everything for a fast quality change', function(assert) {
let resets = 0;
let removeFuncArgs = {};

this.player.tech_.buffered = () => createTimeRanges(0, 1);

this.playlistController.mediaSource.trigger('sourceopen');
// main
this.standardXHRResponse(this.requests.shift());
Expand All @@ -701,18 +703,11 @@ QUnit.test('resets everything for a fast quality change', function(assert) {
originalResync.call(segmentLoader);
};

const origResetEverything = segmentLoader.resetEverything;
const origRemove = segmentLoader.remove;
const origResetLoaderProperties = segmentLoader.resetLoaderProperties;

segmentLoader.resetEverything = () => {
segmentLoader.resetLoaderProperties = () => {
resets++;
origResetEverything.call(segmentLoader);
};

segmentLoader.remove = (start, end) => {
assert.equal(end, Infinity, 'on a remove all, end should be Infinity');

origRemove.call(segmentLoader, start, end);
origResetLoaderProperties.call(segmentLoader);
};

segmentLoader.startingMediaInfo_ = { hasVideo: true };
Expand Down Expand Up @@ -748,9 +743,7 @@ QUnit.test('resets everything for a fast quality change', function(assert) {

assert.equal(resyncs, 1, 'resynced segment loader if media is changed');

assert.equal(resets, 1, 'resetEverything called if media is changed');

assert.deepEqual(removeFuncArgs, {start: 0, end: 60}, 'remove() called with correct arguments if media is changed');
assert.equal(resets, 1, 'resetLoaderProperties called if media is changed');
});

QUnit.test('loadVttJs should be passed to the vttSegmentLoader and resolved on vttjsloaded', function(assert) {
Expand All @@ -771,55 +764,6 @@ QUnit.test('loadVttJs should be passed to the vttSegmentLoader and rejected on v
});
});

QUnit.test('seeks in place for fast quality switch on non-IE/Edge browsers', function(assert) {
let seeks = 0;

this.playlistController.mediaSource.trigger('sourceopen');
// main
this.standardXHRResponse(this.requests.shift());
// media
this.standardXHRResponse(this.requests.shift());

const segmentLoader = this.playlistController.mainSegmentLoader_;

return requestAndAppendSegment({
request: this.requests.shift(),
segmentLoader,
clock: this.clock
}).then(() => {
// media is changed
this.playlistController.selectPlaylist = () => {
const playlists = this.playlistController.main().playlists;
const currentPlaylist = this.playlistController.media();

return playlists.find((playlist) => playlist !== currentPlaylist);
};

this.player.tech_.on('seeking', function() {
seeks++;
});

const timeBeforeSwitch = this.player.currentTime();

// mock buffered values so removes are processed
segmentLoader.sourceUpdater_.audioBuffer.buffered = createTimeRanges([[0, 10]]);
segmentLoader.sourceUpdater_.videoBuffer.buffered = createTimeRanges([[0, 10]]);

this.playlistController.fastQualityChange_();
// trigger updateend to indicate the end of the remove operation
segmentLoader.sourceUpdater_.audioBuffer.trigger('updateend');
segmentLoader.sourceUpdater_.videoBuffer.trigger('updateend');
this.clock.tick(1);

assert.equal(
this.player.currentTime(),
timeBeforeSwitch,
'current time remains the same on fast quality switch'
);
assert.equal(seeks, 1, 'seek event occurs on fast quality switch');
});
});

QUnit.test('basic timeToLoadedData, mediaAppends, appendsToLoadedData stats', function(assert) {
this.player.tech_.trigger('loadstart');
this.playlistController.mediaSource.trigger('sourceopen');
Expand Down Expand Up @@ -4810,14 +4754,15 @@ QUnit.test('on error all segment and playlist loaders are paused and aborted', f

QUnit.test('can pass or select a playlist for fastQualityChange', function(assert) {
const calls = {
resetEverything: 0,
resyncLoader: 0,
media: 0,
selectPlaylist: 0
};

const pc = this.playlistController;

this.player.tech_.buffered = () => createTimeRanges(0, 1);

pc.mediaSource.trigger('sourceopen');
// main
this.standardXHRResponse(this.requests.shift());
Expand All @@ -4841,24 +4786,18 @@ QUnit.test('can pass or select a playlist for fastQualityChange', function(asser
calls.resyncLoader++;
};

pc.mainSegmentLoader_.resetEverything = () => {
calls.resetEverything++;
};

pc.fastQualityChange_(pc.main().playlists[1]);
assert.deepEqual(calls, {
resetEverything: 1,
media: 1,
selectPlaylist: 0,
resyncLoader: 0
resyncLoader: 1
}, 'calls expected function when passed a playlist');

pc.fastQualityChange_();
assert.deepEqual(calls, {
resetEverything: 2,
media: 2,
selectPlaylist: 1,
resyncLoader: 0
resyncLoader: 2
}, 'calls expected function when not passed a playlist');
});

Expand Down

0 comments on commit 4590bdd

Please sign in to comment.