Skip to content

Commit

Permalink
Improve NO_PLAYABLE_REPRESENTATION behavior for the v4
Browse files Browse the repository at this point in the history
In the v3.x.x, the impossibility to play any `Representation` from a
chosen track due to non-usable keys, lead to an error with the
`NO_PLAYABLE_REPRESENTATION` error code .

This seemed sensible at the time, but we're now encountering use cases
where it would be more sensible to change the current track instead, to
one that perhaps has decipherable Representation(s).

The main example would be to have a separate video track linked to
to another dynamic range (e.g. an HDR and a SDR track) each with different
security policies (tracks with higher dynamic range would have more drastic
security policies for example). Here, I would guess that an application
would prefer that by default we switch to the SDR video track if no
`Representation` in the HDR one is decipherable, instead of just
stopping playback with a `NO_PLAYABLE_REPRESENTATION` error.

This is sadly not something we can do easilty in a `v3.x.x` because it
would imply breaking changes (such as switching to the `RELOADING`
state during the track change without a supplementary API), but that's
definitely something we would want to implement in the v4.

Because an application might still want to be notified or even stop
playback by itself when the chosen track has no playable Representation,
I added the `no-playable-Representation` `reason` to the `trackUpdate`
event, which indicates that the current track for any Period of the
current content was updated due to this situation.

The `NO_PLAYABLE_REPRESENTATION` error is still thrown, only now it is
when no `Representation` of all tracks for the given type are
decipherable.
  • Loading branch information
peaBerberian committed Sep 26, 2023
1 parent 0a4298d commit bdb0dce
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 64 deletions.
3 changes: 3 additions & 0 deletions doc/api/Player_Errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,9 @@ As described in the corresponding code's documentation, A aupplementary
`trackInfo` property may be set on `MEDIA_ERROR` depending on its `code`
property.

Note that even if the code may be linked to a `trackInfo` property, that
property may well also be unset.

That `trackInfo` describes, when it makes sense, the characteristics of the track
linked to an error. For example, you may want to know which video track led to a
`BUFFER_APPEND_ERROR` and thus might be linked to corrupted segments.
Expand Down
4 changes: 4 additions & 0 deletions doc/api/Player_Events.md
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,10 @@ The payload for this event is an object with the following properties:
- `"missing"` the previously-chosen track was missing from the content's
refreshed Manifest.

- `"no-playable-representation"`: the previously-chosen track had none of
its `Representation` playable, most likely because of decipherability
issues and thus the RxPlayer decided to switch to a new track.

Though other reasons may be added in the future (for future reasons not
covered by those values), so you should expect this possibility in your
application's logic.
Expand Down
150 changes: 110 additions & 40 deletions src/core/api/public_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -694,26 +694,7 @@ class Player extends EventEmitter<IPublicAPIEvent> {

// Bind events
initializer.addEventListener("error", (error) => {
const formattedError = formatError(error, {
defaultCode: "NONE",
defaultReason: "An unknown error stopped content playback.",
});
formattedError.fatal = true;

contentInfos.currentContentCanceller.cancel();
this._priv_cleanUpCurrentContentState();
this._priv_currentError = formattedError;
log.error("API: The player stopped because of an error",
error instanceof Error ? error : "");
this._priv_setPlayerState(PLAYER_STATES.STOPPED);

// TODO This condition is here because the eventual callback called when the
// player state is updated can launch a new content, thus the error will not
// be here anymore, in which case triggering the "error" event is unwanted.
// This is very ugly though, and we should probable have a better solution
if (this._priv_currentError === formattedError) {
this.trigger("error", formattedError);
}
this._priv_onFatalError(error, contentInfos);
});
initializer.addEventListener("warning", (error) => {
const formattedError = formatError(error, {
Expand Down Expand Up @@ -2106,21 +2087,34 @@ class Player extends EventEmitter<IPublicAPIEvent> {
return; // Event for another content
}
contentInfos.manifest = manifest;
const cancelSignal = contentInfos.currentContentCanceller.signal;
this._priv_reloadingMetadata.manifest = manifest;

contentInfos.tracksStore = new TracksStore({
const tracksStore = new TracksStore({
preferTrickModeTracks: this._priv_preferTrickModeTracks,
defaultAudioTrackSwitchingMode: contentInfos.defaultAudioTrackSwitchingMode,
});
contentInfos.tracksStore.addEventListener("newAvailablePeriods", (p) => {
contentInfos.tracksStore = tracksStore;
tracksStore.addEventListener("newAvailablePeriods", (p) => {
this.trigger("newAvailablePeriods", p);
});
contentInfos.tracksStore.addEventListener("brokenRepresentationsLock", (e) => {
tracksStore.addEventListener("brokenRepresentationsLock", (e) => {
this.trigger("brokenRepresentationsLock", e);
});
contentInfos.tracksStore.addEventListener("trackUpdate", (e) => {
tracksStore.addEventListener("trackUpdate", (e) => {
this.trigger("trackUpdate", e);

const currentPeriod = this._priv_contentInfos?.currentPeriod ?? undefined;
if (e.reason === "no-playable-representation" &&
e.period.id === currentPeriod?.id)
{
this._priv_onAvailableTracksMayHaveChanged(e.trackType);
}
});
contentInfos.tracksStore.addEventListener("warning", (err) => {
this.trigger("warning", err);
});
contentInfos.tracksStore.addEventListener("error", (err) => {
this._priv_onFatalError(err, contentInfos);
});

contentInfos.tracksStore.updatePeriodList(manifest);
Expand All @@ -2131,8 +2125,8 @@ class Player extends EventEmitter<IPublicAPIEvent> {
contentInfos.tracksStore.updatePeriodList(manifest);
}
const currentPeriod = this._priv_contentInfos?.currentPeriod ?? undefined;
const tracksStore = this._priv_contentInfos?.tracksStore;
if (currentPeriod === undefined || isNullOrUndefined(tracksStore)) {
const currTracksStore = this._priv_contentInfos?.tracksStore;
if (currentPeriod === undefined || isNullOrUndefined(currTracksStore)) {
return;
}
for (const update of updates.updatedPeriods) {
Expand All @@ -2141,22 +2135,13 @@ class Player extends EventEmitter<IPublicAPIEvent> {
update.result.removedAdaptations.length > 0)
{
// We might have new (or less) tracks, send events just to be sure
const periodRef = tracksStore.getPeriodObjectFromPeriod(currentPeriod);
const periodRef = currTracksStore.getPeriodObjectFromPeriod(currentPeriod);
if (periodRef === undefined) {
return;
}
const audioTracks = tracksStore.getAvailableAudioTracks(periodRef);
this._priv_triggerEventIfNotStopped("availableAudioTracksChange",
audioTracks ?? [],
cancelSignal);
const textTracks = tracksStore.getAvailableTextTracks(periodRef);
this._priv_triggerEventIfNotStopped("availableTextTracksChange",
textTracks ?? [],
cancelSignal);
const videoTracks = tracksStore.getAvailableVideoTracks(periodRef);
this._priv_triggerEventIfNotStopped("availableVideoTracksChange",
videoTracks ?? [],
cancelSignal);
this._priv_onAvailableTracksMayHaveChanged("audio");
this._priv_onAvailableTracksMayHaveChanged("text");
this._priv_onAvailableTracksMayHaveChanged("video");
}
}
return;
Expand Down Expand Up @@ -2630,6 +2615,91 @@ class Player extends EventEmitter<IPublicAPIEvent> {
}
return cb(tracksStore, periodRef);
}

/**
* Method to call when some event lead to a high for possibility that the
* available tracks for the given type have changed.
* Send the corresponding `available*Tracks` change event with the last
* available tracks.
*
* @param {string} trackType
* @param {Object|undefined} [oPeriodRef] - optional period object used by the
* `tracksStore` API, allows to optimize the method by bypassing this step.
*/
private _priv_onAvailableTracksMayHaveChanged(
trackType : IBufferType,
oPeriodRef? : ITMPeriodObject
): void {
const contentInfos = this._priv_contentInfos;
if (contentInfos === null) {
return;
}
const { currentPeriod,
tracksStore,
currentContentCanceller } = contentInfos;
const cancelSignal = currentContentCanceller.signal;
if (isNullOrUndefined(currentPeriod) || tracksStore === null) {
return;
}
const periodRef = oPeriodRef ?? tracksStore.getPeriodObjectFromPeriod(currentPeriod);
if (periodRef === undefined) {
return;
}
switch (trackType) {
case "video":
const videoTracks = tracksStore.getAvailableVideoTracks(periodRef);
this._priv_triggerEventIfNotStopped("availableVideoTracksChange",
videoTracks ?? [],
cancelSignal);
break;
case "audio":
const audioTracks = tracksStore.getAvailableAudioTracks(periodRef);
this._priv_triggerEventIfNotStopped("availableAudioTracksChange",
audioTracks ?? [],
cancelSignal);
break;
case "text":
const textTracks = tracksStore.getAvailableTextTracks(periodRef);
this._priv_triggerEventIfNotStopped("availableTextTracksChange",
textTracks ?? [],
cancelSignal);
break;
default:
assertUnreachable(trackType);
}
}

/**
* Method to call when a fatal error lead to the stopping of the current
* content.
*
* @param {*} err - The error encountered.
* @param {Object} contentInfos - The `IPublicApiContentInfos` object linked
* to the content for which the error was received.
*/
private _priv_onFatalError(
err : unknown,
contentInfos : IPublicApiContentInfos
): void {
const formattedError = formatError(err, {
defaultCode: "NONE",
defaultReason: "An unknown error stopped content playback.",
});
formattedError.fatal = true;
contentInfos.currentContentCanceller.cancel();
this._priv_cleanUpCurrentContentState();
this._priv_currentError = formattedError;
log.error("API: The player stopped because of an error", formattedError);
this._priv_setPlayerState(PLAYER_STATES.STOPPED);

// TODO This condition is here because the eventual callback called when the
// player state is updated can launch a new content, thus the error will not
// be here anymore, in which case triggering the "error" event is unwanted.
// This is very ugly though, and we should probable have a better solution
if (this._priv_currentError === formattedError) {
this.trigger("error", formattedError);
}
}
}
Player.version = /* PLAYER_VERSION */"3.31.0";

Expand Down
52 changes: 37 additions & 15 deletions src/core/api/track_management/track_dispatcher.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { MediaError } from "../../../errors";
import Manifest, {
Adaptation,
Representation,
Expand Down Expand Up @@ -63,48 +62,69 @@ export default class TrackDispatcher extends EventEmitter<ITrackDispatcherEvent>
/** Interface allowing to clean-up resources when they are not needed anymore. */
private _canceller : TaskCanceller;

/**
* Boolean set to `true` when a track-updating method is called and to `false`
* just before it performs the actual track change to allow checking for
* re-entrancy: if the token is already reset to `false` before the
* track change is officialized, then another track update has already been
* performed in the meantime.
*/
private _updateToken: boolean;

/**
* Create a new `TrackDispatcher` by giving its Reference and an initial track
* setting.
* This constructor will update the Reference with the right preferences
* synchronously.
* @param {Object} manifest
* @param {Object} adaptationRef
* @param {Object|null} initialTrackInfo
*/
constructor(
manifest : Manifest,
adaptationRef : ISharedReference<IAdaptationChoice | null | undefined>,
initialTrackInfo : ITrackSetting | null
adaptationRef : ISharedReference<IAdaptationChoice | null | undefined>
) {
super();
this._canceller = new TaskCanceller();
this._manifest = manifest;
this._adaptationRef = adaptationRef;
this._updateToken = false;
}

/**
* @param {Object|null} initialTrackInfo
*/
public start(initialTrackInfo : ITrackSetting | null) : void {
this._updateToken = true;
if (initialTrackInfo === null) {
this._lastEmitted = initialTrackInfo;
adaptationRef.setValue(null);
this._lastEmitted = null;
this._updateToken = false;
this._adaptationRef.setValue(null);
return;
}
const reference = this._constructLockedRepresentationsReference(initialTrackInfo);
if (!this._updateToken) {
return;
}
this._lastEmitted = { adaptation: initialTrackInfo.adaptation,
switchingMode: initialTrackInfo.switchingMode,
lockedRepresentations: null };
adaptationRef.setValue({ adaptation: initialTrackInfo.adaptation,
switchingMode: initialTrackInfo.switchingMode,
representations: reference });
this._updateToken = false;
this._adaptationRef.setValue({ adaptation: initialTrackInfo.adaptation,
switchingMode: initialTrackInfo.switchingMode,
representations: reference });
}

/**
* Update the wanted track on the Reference linked to this `TrackDispatcher`.
* @param {Object|null} newTrackInfo
*/
public updateTrack(newTrackInfo : ITrackSetting | null) : void {
this._updateToken = true;
if (newTrackInfo === null) {
if (this._lastEmitted === null) {
return;
}
this._updateToken = false;
this._canceller.cancel();

// has no point but let's still create one for simplicity sake
Expand All @@ -117,7 +137,11 @@ export default class TrackDispatcher extends EventEmitter<ITrackDispatcherEvent>
this._canceller.cancel();
this._canceller = new TaskCanceller();
const reference = this._constructLockedRepresentationsReference(newTrackInfo);
if (!this._updateToken) {
return;
}
this._lastEmitted = { adaptation, switchingMode, lockedRepresentations: null };
this._updateToken = false;
this._adaptationRef.setValue({ adaptation,
switchingMode,
representations: reference });
Expand Down Expand Up @@ -176,12 +200,9 @@ export default class TrackDispatcher extends EventEmitter<ITrackDispatcherEvent>
}
}
if (playableRepresentations.length <= 0) {
const adaptationType = trackInfo.adaptation.type;
const noRepErr = new MediaError("NO_PLAYABLE_REPRESENTATION",
"No Representation in the chosen " +
adaptationType + " Adaptation can be played",
{ adaptation: trackInfo.adaptation });
throw noRepErr;
trackInfo.adaptation.isSupported = false;
self.trigger("noPlayableRepresentation", null);
return;
}

// Check if Locked Representations have changed
Expand Down Expand Up @@ -222,6 +243,7 @@ export interface ITrackDispatcherEvent {
* none of them are currently "playable".
*/
noPlayableLockedRepresentation : null;
noPlayableRepresentation: null;
}

/** Define a new Track preference given to the `TrackDispatcher`. */
Expand Down
Loading

0 comments on commit bdb0dce

Please sign in to comment.