Skip to content

Commit

Permalink
fix: null check return value of selectPlaylist (#781)
Browse files Browse the repository at this point in the history
This is #779 but against master.
  • Loading branch information
gkatsev authored Mar 24, 2020
1 parent 005d769 commit 858236f
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 14 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ test/dist/

# 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

.eslintcache
.yo-rc.json
62 changes: 49 additions & 13 deletions src/master-playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,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 @@ -432,19 +473,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 @@ -1458,6 +1458,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;
const 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 858236f

Please sign in to comment.