Skip to content

Commit

Permalink
refactor: rename 'blacklist' to 'exclude' (#1274)
Browse files Browse the repository at this point in the history 
* remove blacklist in comments and property names
* rename blacklist in tests
* rename excludeCurrentPlaylist to excludePlaylist
  • Loading branch information
@harisha-swaminathan
harisha-swaminathan authored Jun 23, 2022
1 parent f85c153 commit d79d783
Show file tree
Hide file tree
Showing 17 changed files with 298 additions and 299 deletions.
12 changes: 6 additions & 6 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Video.js Compatibility: 6.0, 7.0
- [useCueTags](#usecuetags)
- [parse708captions](#parse708captions)
- [overrideNative](#overridenative)
- [blacklistDuration](#blacklistduration)
- [playlistExclusionDuration](#playlistexclusionduration)
- [maxPlaylistRetries](#maxplaylistretries)
- [bandwidth](#bandwidth)
- [useBandwidthFromLocalStorage](#usebandwidthfromlocalstorage)
Expand Down Expand Up @@ -346,13 +346,13 @@ var player = videojs('playerId', {

Since MSE playback may be desirable on all browsers with some native support other than Safari, `overrideNative: !videojs.browser.IS_SAFARI` could be used.

##### blacklistDuration
##### playlistExclusionDuration
* Type: `number`
* can be used as an initialization option

When the `blacklistDuration` property is set to a time duration in seconds,
if a playlist is blacklisted, it will be blacklisted for a period of that
customized duration. This enables the blacklist duration to be configured
When the `playlistExclusionDuration` property is set to a time duration in seconds,
if a playlist is excluded, it will be excluded for a period of that
customized duration. This enables the exclusion duration to be configured
by the user.

##### maxPlaylistRetries
Expand Down Expand Up @@ -761,7 +761,7 @@ Each of the following usage events are fired per use:
| vhs-audio-change | a user selected an alternate audio stream |
| vhs-rendition-disabled | a rendition was disabled |
| vhs-rendition-enabled | a rendition was enabled |
| vhs-rendition-blacklisted | a rendition was blacklisted |
| vhs-rendition-excluded| a rendition was excluded |
| vhs-timestamp-offset | a timestamp offset was set in HLS (can identify discontinuities) |
| vhs-unknown-waiting | the player stopped for an unknown reason and we seeked to current time try to address it |
| vhs-live-resync | playback fell off the back of a live playlist and we resynced to the live point |
Expand Down
2 changes: 1 addition & 1 deletion docs/a-walk-through-vhs.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,4 +247,4 @@ The video can start playing as soon as there's enough audio and video (for muxed

But once `SegmentLoader` does finish, it starts the process again, looking for new content.

There are other modules, and other functions of the code (e.g., blacklisting logic, ABR, etc.), but this is the most critical path of VHS, the one that allows video to play in the browser.
There are other modules, and other functions of the code (e.g., excluding logic, ABR, etc.), but this is the most critical path of VHS, the one that allows video to play in the browser.
2 changes: 1 addition & 1 deletion src/dash-playlist-loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ export default class DashPlaylistLoader extends EventTarget {
response: '',
playlist,
internal: true,
blacklistDuration: Infinity,
playlistExclusionDuration: Infinity,
// MEDIA_ERR_NETWORK
code: 2
}, request);
Expand Down
115 changes: 57 additions & 58 deletions src/master-playlist-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { codecsForPlaylist, unwrapCodecList, codecCount } from './util/codecs.js
import { createMediaTypes, setupMediaGroups } from './media-groups';
import logger from './util/logger';

const ABORT_EARLY_BLACKLIST_SECONDS = 60 * 2;
const ABORT_EARLY_EXCLUSION_SECONDS = 60 * 2;

let Vhs;

Expand Down Expand Up @@ -155,7 +155,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
bandwidth,
externVhs,
useCueTags,
blacklistDuration,
playlistExclusionDuration,
enableLowInitialPlaylist,
sourceType,
cacheEncryptionKeys,
Expand Down Expand Up @@ -183,7 +183,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
this.vhs_ = tech.vhs;
this.sourceType_ = sourceType;
this.useCueTags_ = useCueTags;
this.blacklistDuration = blacklistDuration;
this.playlistExclusionDuration = playlistExclusionDuration;
this.maxPlaylistRetries = maxPlaylistRetries;
this.enableLowInitialPlaylist = enableLowInitialPlaylist;

Expand Down Expand Up @@ -525,7 +525,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
vhs: this.vhs_,
master: this.master(),
mediaTypes: this.mediaTypes_,
blacklistCurrentPlaylist: this.blacklistCurrentPlaylist.bind(this)
excludePlaylist: this.excludePlaylist.bind(this)
});

this.triggerPresenceUsage_(this.master(), media);
Expand Down Expand Up @@ -592,7 +592,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
});

this.masterPlaylistLoader_.on('error', () => {
this.blacklistCurrentPlaylist(this.masterPlaylistLoader_.error);
this.excludePlaylist(this.masterPlaylistLoader_.error);
});

this.masterPlaylistLoader_.on('mediachanging', () => {
Expand Down Expand Up @@ -640,10 +640,10 @@ export class MasterPlaylistController extends videojs.EventTarget {

if (playlistOutdated) {
// Playlist has stopped updating and we're stuck at its end. Try to
// blacklist it and switch to another playlist in the hope that that
// exclude it and switch to another playlist in the hope that that
// one is updating (and give the player a chance to re-adjust to the
// safe live point).
this.blacklistCurrentPlaylist({
this.excludePlaylist({
message: 'Playlist no longer updating.',
reason: 'playlist-unchanged'
});
Expand Down Expand Up @@ -778,7 +778,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
}

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

this.mainSegmentLoader_.on('appenderror', () => {
Expand Down Expand Up @@ -815,10 +815,10 @@ export class MasterPlaylistController extends videojs.EventTarget {

this.delegateLoaders_('all', ['abort']);

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

const updateCodecs = () => {
Expand Down Expand Up @@ -1122,29 +1122,29 @@ export class MasterPlaylistController extends videojs.EventTarget {
}

/**
* Blacklists a playlist when an error occurs for a set amount of time
* 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.
*
* @param {Object=} error an optional error that may include the playlist
* to blacklist
* @param {number=} blacklistDuration an optional number of seconds to blacklist the
* to exclude
* @param {number=} playlistExclusionDuration an optional number of seconds to exclude the
* playlist
*/
blacklistCurrentPlaylist(error = {}, blacklistDuration) {
excludePlaylist(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
// blacklisted instead of the currently selected playlist which is likely
// excluded instead of the currently selected playlist which is likely
// out-of-date in this scenario
const currentPlaylist = error.playlist || this.masterPlaylistLoader_.media();
const playlistToExclude = error.playlist || this.masterPlaylistLoader_.media();

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

// If there is no current playlist, then an error occurred while we were
// trying to load the master OR while we were disposing of the tech
if (!currentPlaylist) {
if (!playlistToExclude) {
this.error = error;

if (this.mediaSource.readyState !== 'open') {
Expand All @@ -1156,16 +1156,16 @@ export class MasterPlaylistController extends videojs.EventTarget {
return;
}

currentPlaylist.playlistErrors_++;
playlistToExclude.playlistErrors_++;

const playlists = this.masterPlaylistLoader_.master.playlists;
const enabledPlaylists = playlists.filter(isEnabled);
const isFinalRendition = enabledPlaylists.length === 1 && enabledPlaylists[0] === currentPlaylist;
const isFinalRendition = enabledPlaylists.length === 1 && enabledPlaylists[0] === playlistToExclude;

// Don't blacklist the only playlist unless it was blacklisted
// Don't exclude the only playlist unless it was excluded
// forever
if (playlists.length === 1 && blacklistDuration !== Infinity) {
videojs.log.warn(`Problem encountered with playlist ${currentPlaylist.id}. ` +
if (playlists.length === 1 && playlistExclusionDuration !== Infinity) {
videojs.log.warn(`Problem encountered with playlist ${playlistToExclude.id}. ` +
'Trying again since it is the only playlist.');

this.tech_.trigger('retryplaylist');
Expand All @@ -1174,15 +1174,15 @@ export class MasterPlaylistController extends videojs.EventTarget {
}

if (isFinalRendition) {
// Since we're on the final non-blacklisted playlist, and we're about to blacklist
// Since we're on the final non-excluded playlist, and we're about to exclude
// it, instead of erring the player or retrying this playlist, clear out the current
// blacklist. This allows other playlists to be attempted in case any have been
// exclusion list. This allows other playlists to be attempted in case any have been
// fixed.
let reincluded = false;

playlists.forEach((playlist) => {
// skip current playlist which is about to be blacklisted
if (playlist === currentPlaylist) {
// skip current playlist which is about to be excluded
if (playlist === playlistToExclude) {
return;
}
const excludeUntil = playlist.excludeUntil;
Expand All @@ -1204,28 +1204,27 @@ export class MasterPlaylistController extends videojs.EventTarget {
}
}

// Blacklist this playlist
// Exclude this playlist
let excludeUntil;

if (currentPlaylist.playlistErrors_ > this.maxPlaylistRetries) {
if (playlistToExclude.playlistErrors_ > this.maxPlaylistRetries) {
excludeUntil = Infinity;
} else {
excludeUntil = Date.now() + (blacklistDuration * 1000);
excludeUntil = Date.now() + (playlistExclusionDuration * 1000);
}

currentPlaylist.excludeUntil = excludeUntil;
playlistToExclude.excludeUntil = excludeUntil;

if (error.reason) {
currentPlaylist.lastExcludeReason_ = error.reason;
playlistToExclude.lastExcludeReason_ = error.reason;
}
this.tech_.trigger('blacklistplaylist');
this.tech_.trigger({type: 'usage', name: 'vhs-rendition-blacklisted'});
this.tech_.trigger('excludeplaylist');
this.tech_.trigger({type: 'usage', name: 'vhs-rendition-excluded'});

// TODO: should we select a new playlist if this blacklist wasn't for the currentPlaylist?
// Would be something like media().id !=== currentPlaylist.id and we would need something
// like `pendingMedia` in playlist loaders to check against that too. This will prevent us
// from loading a new playlist on any blacklist.
// Select a new playlist
// TODO: only load a new playlist if we're excluding the current playlist
// If this function was called with a playlist that's not the current active playlist
// (e.g., media().id !== playlistToExclude.id),
// then a new playlist should not be selected and loaded, as there's nothing wrong with the current playlist.
const nextPlaylist = this.selectPlaylist();

if (!nextPlaylist) {
Expand All @@ -1237,16 +1236,16 @@ export class MasterPlaylistController extends videojs.EventTarget {
const logFn = error.internal ? this.logger_ : videojs.log.warn;
const errorMessage = error.message ? (' ' + error.message) : '';

logFn(`${(error.internal ? 'Internal problem' : 'Problem')} encountered with playlist ${currentPlaylist.id}.` +
logFn(`${(error.internal ? 'Internal problem' : 'Problem')} encountered with playlist ${playlistToExclude.id}.` +
`${errorMessage} Switching to playlist ${nextPlaylist.id}.`);

// if audio group changed reset audio loaders
if (nextPlaylist.attributes.AUDIO !== currentPlaylist.attributes.AUDIO) {
if (nextPlaylist.attributes.AUDIO !== playlistToExclude.attributes.AUDIO) {
this.delegateLoaders_('audio', ['abort', 'pause']);
}

// if subtitle group changed reset subtitle loaders
if (nextPlaylist.attributes.SUBTITLES !== currentPlaylist.attributes.SUBTITLES) {
if (nextPlaylist.attributes.SUBTITLES !== playlistToExclude.attributes.SUBTITLES) {
this.delegateLoaders_('subtitle', ['abort', 'pause']);
}

Expand Down Expand Up @@ -1694,10 +1693,10 @@ export class MasterPlaylistController extends videojs.EventTarget {

// no codecs, no playback.
if (!codecs.audio && !codecs.video) {
this.blacklistCurrentPlaylist({
this.excludePlaylist({
playlist: this.media(),
message: 'Could not determine codecs for playlist.',
blacklistDuration: Infinity
playlistExclusionDuration: Infinity
});
return;
}
Expand Down Expand Up @@ -1733,7 +1732,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
this.logger_(`excluding audio group ${audioGroup} as ${unsupportedAudio} does not support codec(s): "${codecs.audio}"`);
}

// if we have any unsupported codecs blacklist this playlist.
// if we have any unsupported codecs exclude this playlist.
if (Object.keys(unsupportedCodecs).length) {
const message = Object.keys(unsupportedCodecs).reduce((acc, supporter) => {

Expand All @@ -1746,11 +1745,11 @@ export class MasterPlaylistController extends videojs.EventTarget {
return acc;
}, '') + '.';

this.blacklistCurrentPlaylist({
this.excludePlaylist({
playlist: this.media(),
internal: true,
message,
blacklistDuration: Infinity
playlistExclusionDuration: Infinity
});
return;
}
Expand All @@ -1771,10 +1770,10 @@ export class MasterPlaylistController extends videojs.EventTarget {
});

if (switchMessages.length) {
this.blacklistCurrentPlaylist({
this.excludePlaylist({
playlist: this.media(),
message: `Codec switching not supported: ${switchMessages.join(', ')}.`,
blacklistDuration: Infinity,
playlistExclusionDuration: Infinity,
internal: true
});
return;
Expand Down Expand Up @@ -1861,7 +1860,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
}

/**
* Blacklist playlists that are known to be codec or
* Exclude playlists that are known to be codec or
* stream-incompatible with the SourceBuffer configuration. For
* instance, Media Source Extensions would cause the video element to
* stall waiting for video data if you switched from a variant with
Expand Down Expand Up @@ -1892,7 +1891,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
}

ids.push(variant.id);
const blacklistReasons = [];
const exclusionReasons = [];

// get codecs from the playlist for this variant
const variantCodecs = codecsForPlaylist(this.masterPlaylistLoader_.master, variant);
Expand All @@ -1908,7 +1907,7 @@ export class MasterPlaylistController extends videojs.EventTarget {
// old media source and creating a new one, but it will take some work.
// The number of streams cannot change
if (variantCodecCount !== codecCount_) {
blacklistReasons.push(`codec count "${variantCodecCount}" !== "${codecCount_}"`);
exclusionReasons.push(`codec count "${variantCodecCount}" !== "${codecCount_}"`);
}

// only exclude playlists by codec change, if codecs cannot switch
Expand All @@ -1919,18 +1918,18 @@ export class MasterPlaylistController extends videojs.EventTarget {

// the video codec cannot change
if (variantVideoDetails && videoDetails && variantVideoDetails.type.toLowerCase() !== videoDetails.type.toLowerCase()) {
blacklistReasons.push(`video codec "${variantVideoDetails.type}" !== "${videoDetails.type}"`);
exclusionReasons.push(`video codec "${variantVideoDetails.type}" !== "${videoDetails.type}"`);
}

// the audio codec cannot change
if (variantAudioDetails && audioDetails && variantAudioDetails.type.toLowerCase() !== audioDetails.type.toLowerCase()) {
blacklistReasons.push(`audio codec "${variantAudioDetails.type}" !== "${audioDetails.type}"`);
exclusionReasons.push(`audio codec "${variantAudioDetails.type}" !== "${audioDetails.type}"`);
}
}

if (blacklistReasons.length) {
if (exclusionReasons.length) {
variant.excludeUntil = Infinity;
this.logger_(`blacklisting ${variant.id}: ${blacklistReasons.join(' && ')}`);
this.logger_(`excluding ${variant.id}: ${exclusionReasons.join(' && ')}`);
}
});
}
Expand Down
Loading

0 comments on commit d79d783

Please sign in to comment.