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

refactor: clean up parameters of excludePlaylist #1304

Merged
merged 3 commits into from
Aug 19, 2022
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
2 changes: 1 addition & 1 deletion src/media-groups.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ export const onError = {
// Default track encountered an error. All we can do now is exclude the current
// rendition and hope another will switch audio groups
excludePlaylist({
message: 'Problem encountered loading the default audio track.'
error: { message: 'Problem encountered loading the default audio track.' }
});
return;
}
Expand Down
5 changes: 3 additions & 2 deletions src/playback-watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,9 @@ export default class PlaybackWatcher {
// TODO: should we exclude audio tracks rather than main tracks
// when type is audio?
pc.excludePlaylist({
message: `Excessive ${type} segment downloading detected.`
}, Infinity);
error: { message: `Excessive ${type} segment downloading detected.` },
playlistExclusionDuration: Infinity
});
}

/**
Expand Down
73 changes: 46 additions & 27 deletions src/playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,9 @@ export class PlaylistController extends videojs.EventTarget {
});

this.mainPlaylistLoader_.on('error', () => {
this.excludePlaylist(this.mainPlaylistLoader_.error);
const error = this.mainPlaylistLoader_.error;

this.excludePlaylist({ playlistToExclude: error.playlist, error });
});

this.mainPlaylistLoader_.on('mediachanging', () => {
Expand Down Expand Up @@ -644,8 +646,10 @@ export class PlaylistController extends videojs.EventTarget {
// one is updating (and give the player a chance to re-adjust to the
// safe live point).
this.excludePlaylist({
message: 'Playlist no longer updating.',
reason: 'playlist-unchanged'
error: {
message: 'Playlist no longer updating.',
reason: 'playlist-unchanged'
}
});
// useful for monitoring QoS
this.tech_.trigger('playliststuck');
Expand Down Expand Up @@ -778,7 +782,9 @@ export class PlaylistController extends videojs.EventTarget {
}

this.mainSegmentLoader_.on('error', () => {
this.excludePlaylist(this.mainSegmentLoader_.error());
const error = this.mainSegmentLoader_.error();

this.excludePlaylist({ playlistToExclude: error.playlist, error });
});

this.mainSegmentLoader_.on('appenderror', () => {
Expand Down Expand Up @@ -816,9 +822,12 @@ export class PlaylistController extends videojs.EventTarget {
this.delegateLoaders_('all', ['abort']);

this.excludePlaylist({
message: 'Aborted early because there isn\'t enough bandwidth to complete the ' +
'request without rebuffering.'
}, ABORT_EARLY_EXCLUSION_SECONDS);
error: {
message: 'Aborted early because there isn\'t enough bandwidth to complete ' +
'the request without rebuffering.'
},
playlistExclusionDuration: ABORT_EARLY_EXCLUSION_SECONDS
});
});

const updateCodecs = () => {
Expand Down Expand Up @@ -1122,25 +1131,31 @@ export class PlaylistController extends videojs.EventTarget {
}

/**
* Exclude a playlist when an error occurs for a set amount of time
* making it unavailable for selection by the rendition selection algorithm
* and then forces a new playlist (rendition) selection.
* Exclude a playlist for a set amount of time, making it unavailable for selection by
* the rendition selection algorithm, then force a new playlist (rendition) selection.
*
* @param {Object=} error an optional error that may include the playlist
* to exclude
* @param {number=} playlistExclusionDuration an optional number of seconds to exclude the
* playlist
* @param {Object=} playlistToExclude
* the playlist to exclude, defaults to the currently selected playlist
* @param {Object=} error
* an optional error
* @param {number=} playlistExclusionDuration
* an optional number of seconds to exclude the playlist
*/
excludePlaylist(error = {}, playlistExclusionDuration) {
excludePlaylist({
playlistToExclude = this.mainPlaylistLoader_.media(),
error = {},
playlistExclusionDuration
}) {

// If the `error` was generated by the playlist loader, it will contain
// the playlist we were trying to load (but failed) and that should be
// excluded instead of the currently selected playlist which is likely
// out-of-date in this scenario
const playlistToExclude = error.playlist || this.mainPlaylistLoader_.media();
playlistToExclude = playlistToExclude || this.mainPlaylistLoader_.media();

playlistExclusionDuration = playlistExclusionDuration ||
error.playlistExclusionDuration ||
this.playlistExclusionDuration;
error.playlistExclusionDuration ||
this.playlistExclusionDuration;

// If there is no current playlist, then an error occurred while we were
// trying to load the main OR while we were disposing of the tech
Expand Down Expand Up @@ -1694,8 +1709,8 @@ export class PlaylistController extends videojs.EventTarget {
// no codecs, no playback.
if (!codecs.audio && !codecs.video) {
this.excludePlaylist({
playlist: this.media(),
message: 'Could not determine codecs for playlist.',
playlistToExclude: this.media(),
error: { message: 'Could not determine codecs for playlist.' },
playlistExclusionDuration: Infinity
});
return;
Expand Down Expand Up @@ -1746,9 +1761,11 @@ export class PlaylistController extends videojs.EventTarget {
}, '') + '.';

this.excludePlaylist({
playlist: this.media(),
internal: true,
message,
playlistToExclude: this.media(),
error: {
internal: true,
message
},
playlistExclusionDuration: Infinity
});
return;
Expand All @@ -1771,10 +1788,12 @@ export class PlaylistController extends videojs.EventTarget {

if (switchMessages.length) {
this.excludePlaylist({
playlist: this.media(),
message: `Codec switching not supported: ${switchMessages.join(', ')}.`,
playlistExclusionDuration: Infinity,
internal: true
playlistToExclude: this.media(),
error: {
message: `Codec switching not supported: ${switchMessages.join(', ')}.`,
internal: true
},
playlistExclusionDuration: Infinity
});
return;
}
Expand Down
7 changes: 5 additions & 2 deletions src/videojs-http-streaming.js
Original file line number Diff line number Diff line change
Expand Up @@ -1019,8 +1019,11 @@ class VhsHandler extends Component {
this.player_.tech_.on('keystatuschange', (e) => {
if (e.status === 'output-restricted') {
this.playlistController_.excludePlaylist({
playlist: this.playlistController_.media(),
message: `DRM keystatus changed to ${e.status}. Playlist will fail to play. Check for HDCP content.`,
playlistToExclude: this.playlistController_.media(),
error: {
message: `DRM keystatus changed to ${e.status}. Playlist will fail to play. ` +
'Check for HDCP content.'
},
playlistExclusionDuration: Infinity
});
}
Expand Down
92 changes: 56 additions & 36 deletions test/playlist-controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4753,7 +4753,7 @@ QUnit.module('PlaylistController codecs', {
this.pc = this.playlistController;

this.exclusionList = [];
this.pc.excludePlaylist = (playlist) => this.exclusionList.push(playlist);
this.pc.excludePlaylist = (options) => this.exclusionList.push(options);

this.contentSetup = (options) => {
const {
Expand Down Expand Up @@ -4988,8 +4988,8 @@ QUnit.test('excludes playlist without detected audio/video', function(assert) {

assert.deepEqual(this.exclusionList, [{
playlistExclusionDuration: Infinity,
message: 'Could not determine codecs for playlist.',
playlist: {attributes: {}}
playlistToExclude: { attributes: {} },
error: { message: 'Could not determine codecs for playlist.' }
}], 'excluded playlist');
assert.deepEqual(codecs, void 0, 'no codecs returned');
});
Expand All @@ -5009,9 +5009,11 @@ QUnit.test('excludes unsupported muxer codecs for ts', function(assert) {

assert.deepEqual(this.exclusionList, [{
playlistExclusionDuration: Infinity,
playlist: {attributes: {}},
internal: true,
message: 'muxer does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
playlistToExclude: {attributes: {}},
error: {
internal: true,
message: 'muxer does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
}
}], 'excluded playlist');
assert.deepEqual(codecs, void 0, 'codecs returned');
});
Expand All @@ -5035,9 +5037,11 @@ QUnit.test('excludes unsupported browser codecs for muxed fmp4', function(assert

assert.deepEqual(this.exclusionList, [{
playlistExclusionDuration: Infinity,
playlist: {attributes: {}},
internal: true,
message: 'browser does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
playlistToExclude: {attributes: {}},
error: {
internal: true,
message: 'browser does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
}
}], 'excluded playlist');
assert.deepEqual(codecs, void 0, 'codecs returned');
});
Expand All @@ -5058,9 +5062,11 @@ QUnit.test('excludes unsupported muxer codecs for muxed ts', function(assert) {

assert.deepEqual(this.exclusionList, [{
playlistExclusionDuration: Infinity,
playlist: {attributes: {}},
internal: true,
message: 'muxer does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
playlistToExclude: {attributes: {}},
error: {
internal: true,
message: 'muxer does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
}
}], 'excluded playlist');
assert.deepEqual(codecs, void 0, 'codecs returned');
});
Expand All @@ -5083,9 +5089,11 @@ QUnit.test('excludes unsupported browser codecs for fmp4', function(assert) {

assert.deepEqual(this.exclusionList, [{
playlistExclusionDuration: Infinity,
playlist: {attributes: {}},
internal: true,
message: 'browser does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
playlistToExclude: {attributes: {}},
error: {
internal: true,
message: 'browser does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
}
}], 'excluded playlist');
assert.deepEqual(codecs, void 0, 'codecs returned');
});
Expand Down Expand Up @@ -5113,9 +5121,12 @@ QUnit.test('excludes unsupported codecs video ts, audio fmp4', function(assert)

assert.deepEqual(this.exclusionList, [{
playlistExclusionDuration: Infinity,
playlist: {attributes: {AUDIO: 'low-quality'}},
internal: true,
message: 'muxer does not support codec(s): "hvc1.2.4.L123.B0", browser does not support codec(s): "ac-3".'
playlistToExclude: {attributes: {AUDIO: 'low-quality'}},
error: {
internal: true,
message: 'muxer does not support codec(s): "hvc1.2.4.L123.B0", ' +
'browser does not support codec(s): "ac-3".'
}
}], 'excluded playlist');
assert.deepEqual(codecs, void 0, 'codecs returned');
});
Expand Down Expand Up @@ -5143,9 +5154,12 @@ QUnit.test('excludes unsupported codecs video fmp4, audio ts', function(assert)

assert.deepEqual(this.exclusionList, [{
playlistExclusionDuration: Infinity,
playlist: {attributes: {AUDIO: 'low-quality'}},
internal: true,
message: 'browser does not support codec(s): "hvc1.2.4.L123.B0", muxer does not support codec(s): "ac-3".'
playlistToExclude: {attributes: {AUDIO: 'low-quality'}},
error: {
internal: true,
message: 'browser does not support codec(s): "hvc1.2.4.L123.B0", ' +
'muxer does not support codec(s): "ac-3".'
}
}], 'excluded playlist');
assert.deepEqual(codecs, void 0, 'codecs returned');
});
Expand Down Expand Up @@ -5173,9 +5187,11 @@ QUnit.test('excludes all of audio group on unsupported audio', function(assert)

assert.deepEqual(this.exclusionList, [{
playlistExclusionDuration: Infinity,
playlist: {attributes: {AUDIO: 'low-quality'}, id: 'bar'},
internal: true,
message: 'muxer does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
playlistToExclude: {attributes: {AUDIO: 'low-quality'}, id: 'bar'},
error: {
internal: true,
message: 'muxer does not support codec(s): "hvc1.2.4.L123.B0,ac-3".'
}
}], 'excluded playlist');
assert.deepEqual(codecs, void 0, 'codecs returned');
assert.equal(this.main.playlists[2].id, 'foo', 'playlist 3 is the one we added');
Expand Down Expand Up @@ -5218,9 +5234,11 @@ QUnit.test('excludes on codec switch if codec switching not supported', function

assert.deepEqual(this.exclusionList, [{
playlistExclusionDuration: Infinity,
playlist: {attributes: {AUDIO: 'low-quality'}},
internal: true,
message: 'Codec switching not supported: "avc1.4c400d" -> "hvc1.2.4.L123.B0", "mp4a.40.2" -> "ac-3".'
playlistToExclude: {attributes: {AUDIO: 'low-quality'}},
error: {
internal: true,
message: 'Codec switching not supported: "avc1.4c400d" -> "hvc1.2.4.L123.B0", "mp4a.40.2" -> "ac-3".'
}
}], 'excluded playlist');
assert.deepEqual(codecs, void 0, 'codecs returned');
});
Expand Down Expand Up @@ -5429,8 +5447,8 @@ QUnit.module('PlaylistController - exclusion behavior', {
assert.equal(this.pc.media(), this.pc.main().playlists[0], 'selected first playlist');

this.pc.excludePlaylist({
internal: true,
playlist: this.pc.main().playlists[0],
playlistToExclude: this.pc.main().playlists[0],
error: { internal: true },
playlistExclusionDuration: Infinity
});

Expand Down Expand Up @@ -6012,7 +6030,7 @@ QUnit.test("don't exclude only playlist unless it was excluded forever", functio

mpl.load = (delay) => (shouldDelay = delay);

pc.excludePlaylist();
pc.excludePlaylist({});

assert.notOk('excludeUntil' in playlist, 'playlist was not excluded since excludeDuration was finite');
assert.ok(shouldDelay, 'we delay retry since it is the final rendition');
Expand Down Expand Up @@ -6048,7 +6066,9 @@ QUnit.test("don't exclude only playlist unless it was excluded forever", functio
});

// exclude forever
pc.excludePlaylist({}, Infinity);
pc.excludePlaylist({
playlistExclusionDuration: Infinity
});

assert.ok('excludeUntil' in playlist, 'playlist was excluded');
assert.notOk(shouldDelay, 'value was not changed');
Expand Down Expand Up @@ -6084,7 +6104,7 @@ QUnit.test('switch playlists if current playlist gets excluded and re-include if

mpl.load = (delay) => (shouldDelay = delay);

pc.excludePlaylist();
pc.excludePlaylist({});

assert.ok('excludeUntil' in playlist, 'playlist was excluded since there is another playlist');
assert.notOk(shouldDelay, 'we do not delay retry since it is not the final rendition');
Expand All @@ -6096,7 +6116,7 @@ QUnit.test('switch playlists if current playlist gets excluded and re-include if
this.standardXHRResponse(this.requests.shift());
playlist2 = mpl.main.playlists[1];

pc.excludePlaylist();
pc.excludePlaylist({});

assert.ok('excludeUntil' in playlist2, 'playlist2 was excluded');
assert.notOk('excludeUntil' in playlist, 'playlist was re-included');
Expand Down Expand Up @@ -6133,13 +6153,13 @@ QUnit.test('Playlist is excluded indefinitely if number of playlistErrors_ excee

assert.equal(playlist.playlistErrors_, 0, 'playlistErrors_ starts at zero');

pc.excludePlaylist();
pc.excludePlaylist({});

assert.ok('excludeUntil' in playlist, 'playlist was excluded');
assert.equal(playlist.playlistErrors_, 1, 'we incremented playlistErrors_');
assert.notEqual(playlist.excludeUntil, Infinity, 'The playlist was not excluded indefinitely');

pc.excludePlaylist();
pc.excludePlaylist({});

assert.equal(playlist.playlistErrors_, 2, 'we incremented playlistErrors_');
assert.equal(playlist.excludeUntil, Infinity, 'The playlist was excluded indefinitely');
Expand Down Expand Up @@ -6179,7 +6199,7 @@ QUnit.test('should delay loading of new playlist if lastRequest was less than ha
};
playlist2.lastRequest = Date.now() - 1000;

pc.excludePlaylist();
pc.excludePlaylist({});

assert.ok('excludeUntil' in playlist, 'playlist was excluded since there is another playlist');
assert.ok(shouldDelay, 'we delay retry since second rendition was loaded less than half target duration ago');
Expand Down
Loading