Skip to content

Commit

Permalink
fix: null check return value of selectPlaylist (#779)
Browse files Browse the repository at this point in the history
  • Loading branch information
gkatsev authored Mar 24, 2020
1 parent 808c3b1 commit 90a0215
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 13 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,4 @@ deploy/
# this file is generated and shouldn't be checked in
src/decrypter-worker.worker.js
src/mse/transmuxer-worker.worker.js
src/transmuxer-worker.worker.js
62 changes: 49 additions & 13 deletions src/master-playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,47 @@ const sumLoaderStat = function(stat) {
return this.audioSegmentLoader_[stat] +
this.mainSegmentLoader_[stat];
};
const shouldSwitchToMedia = function({
currentPlaylist,
nextPlaylist,
forwardBuffer,
bufferLowWaterLine,
duration,
log
}) {
// we have no other playlist to switch to
if (!nextPlaylist) {
videojs.log.warn('We received no playlist to switch to. Please check your stream.');
return false;
}

// If the playlist is live, then we want to not take low water line into account.
// This is because in LIVE, the player plays 3 segments from the end of the
// playlist, and if `BUFFER_LOW_WATER_LINE` is greater than the duration availble
// in those segments, a viewer will never experience a rendition upswitch.
if (!currentPlaylist.endList) {
return true;
}

// For the same reason as LIVE, we ignore the low water line when the VOD
// duration is below the max potential low water line
if (duration < Config.MAX_BUFFER_LOW_WATER_LINE) {
return true;
}

// we want to switch down to lower resolutions quickly to continue playback, but
if (nextPlaylist.attributes.BANDWIDTH < currentPlaylist.attributes.BANDWIDTH) {
return true;
}

// ensure we have some buffer before we switch up to prevent us running out of
// buffer while loading a higher rendition.
if (forwardBuffer >= bufferLowWaterLine) {
return true;
}

return false;
};

/**
* the master playlist controller controller all interactons
Expand Down Expand Up @@ -425,19 +466,14 @@ export class MasterPlaylistController extends videojs.EventTarget {

const bufferLowWaterLine = this.bufferLowWaterLine();

// If the playlist is live, then we want to not take low water line into account.
// This is because in LIVE, the player plays 3 segments from the end of the
// playlist, and if `BUFFER_LOW_WATER_LINE` is greater than the duration availble
// in those segments, a viewer will never experience a rendition upswitch.
if (!currentPlaylist.endList ||
// For the same reason as LIVE, we ignore the low water line when the VOD
// duration is below the max potential low water line
this.duration() < Config.MAX_BUFFER_LOW_WATER_LINE ||
// we want to switch down to lower resolutions quickly to continue playback, but
nextPlaylist.attributes.BANDWIDTH < currentPlaylist.attributes.BANDWIDTH ||
// ensure we have some buffer before we switch up to prevent us running out of
// buffer while loading a higher rendition.
forwardBuffer >= bufferLowWaterLine) {
if (shouldSwitchToMedia({
currentPlaylist,
nextPlaylist,
forwardBuffer,
bufferLowWaterLine,
duration: this.duration(),
log: this.logger_
})) {
this.masterPlaylistLoader_.media(nextPlaylist);
}

Expand Down
31 changes: 31 additions & 0 deletions test/master-playlist-controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1283,6 +1283,37 @@ QUnit.test('selects a playlist after main/combined segment downloads', function(
assert.equal(this.player.tech_.hls.stats.bandwidth, 4194304, 'default bandwidth');
});

QUnit.test('does not select a playlist after segment downloads if only one playlist', function(assert) {
const origWarn = videojs.log.warn;
let calls = 0;
let warnings = [];

videojs.log.warn = (text) => warnings.push(text);
this.masterPlaylistController.selectPlaylist = () => {
calls++;
return null;
};
this.masterPlaylistController.mediaSource.trigger('sourceopen');

// master
this.standardXHRResponse(this.requests.shift());
// media
this.standardXHRResponse(this.requests.shift());

// "downloaded" a segment
this.masterPlaylistController.mainSegmentLoader_.trigger('bandwidthupdate');
assert.strictEqual(calls, 2, 'selects after the initial segment');

assert.equal(warnings.length, 1, 'one warning logged');
assert.equal(
warnings[0],
'We received no playlist to switch to. Please check your stream.',
'we logged the correct warning'
);

videojs.log.warn = origWarn;
});

QUnit.test('re-triggers bandwidthupdate events on the tech', function(assert) {
this.masterPlaylistController.mediaSource.trigger('sourceopen');
// master
Expand Down

0 comments on commit 90a0215

Please sign in to comment.