Skip to content

Commit

Permalink
[Proposal] "Remove" manifestUpdateUrl loadVideo option for the v4
Browse files Browse the repository at this point in the history
This commit undocument the `manifestUpdateUrl` API, so it is effictevely
not part of the official API anymore, even if its behavior stays for now
(renamed as `__priv_manifestUpdateUrl`).

The `manifestUpdateUrl` was an advanced DASH optimization where it was
possible to update a dynamic MPD through a shorter version of it, thus
much faster.
This was a clever solution for a real issue we had for a long time at
Canal+, but it relied on complex and easy-to-break logic to perform
updates.

Now, we prefer other solutions, which are either standard (e.g. DASH
packaging techniques to reduce the size of the MPD) or purely based on
code optimizations (WebAssembly-based parsers, advanced parsing modes) -
thus not relying on special URL like it was with this.

As It is not needed at Canal+ anymore, it is not standard in any way,
we didn't hear of any other person using this feature and as removing
it does not break playback (it only make it less efficient if the
feature was relied on before), I propose we profit from the v4 still
being in beta to remove it before an official v4.

Though this commit does not really remove the corresponding logic yet,
it only moves it as an undocumented API.
This is because we may want to still keep that logic maintained for
some months in case its need ever seems to make a comeback.
  • Loading branch information
peaBerberian committed Sep 27, 2023
1 parent 3d22953 commit 4552426
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 17 deletions.
13 changes: 13 additions & 0 deletions doc/Getting_Started/Migration_From_v3/loadVideo_Options.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@ If you still need that option for a valid use case, you are welcomed to open an
issue.


### `transportOptions.manifestUpdateUrl`

The `manifestUpdateUrl` option has been removed without replacement.

It was previously used as a non-standard DASH optimization to be able to refresh
a DASH MPD (its Manifest document) through an URL containing a shorter version
of the full DASH MPD.
As we knew, it was only used at Canal+, though we now use (and we always
preferred) more standard solutions both on the packaging-side (use of repeat
attributes) and on the RxPlayer-side (usage of WebAssembly, internal
optimizations like "unsafeMode").


### `transportOptions.aggressiveMode`

The `aggressiveMode` option has been removed without replacement.
Expand Down
4 changes: 0 additions & 4 deletions doc/reference/API_Reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,6 @@ properties, methods, events and so on.
- [`initialManifest`](../api/Loading_a_Content.md#initialmanifest):
Allows to provide an initial Manifest to speed-up the content loading

- [`manifestUpdateUrl`](../api/Loading_a_Content.md#manifestupdateurl):
Provide another URL, potentially to a shorter Manifest, used only for
Manifest updates

- [`representationFilter`](../api/Loading_a_Content.md#representationfilter):
Filter out qualities from the Manifest based on its characteristics.

Expand Down
9 changes: 2 additions & 7 deletions src/core/api/option_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ interface IParsedLoadVideoOptionsBase {
representationFilter? : IRepresentationFilter | undefined;
segmentLoader? : ISegmentLoader | undefined;
serverSyncInfos? : IServerSyncInfos | undefined;
manifestUpdateUrl? : string | undefined;
__priv_manifestUpdateUrl? : string | undefined;
__priv_patchLastSegmentInSidx? : boolean | undefined;
}

Expand Down Expand Up @@ -346,11 +346,6 @@ function parseLoadVideoOptions(
onCodecSwitch = DEFAULT_CODEC_SWITCHING_BEHAVIOR;
}

if (!isNullOrUndefined(options.manifestUpdateUrl)) {
warnOnce("`manifestUpdateUrl` API is deprecated, please open an issue if you" +
" still rely on this.");
}

if (isNullOrUndefined(options.textTrackMode)) {
textTrackMode = DEFAULT_TEXT_TRACK_MODE;
} else {
Expand Down Expand Up @@ -402,6 +397,7 @@ function parseLoadVideoOptions(
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
return { __priv_patchLastSegmentInSidx: (options as any).__priv_patchLastSegmentInSidx,
__priv_manifestUpdateUrl: (options as any).__priv_manifestUpdateUrl,
/* eslint-enable @typescript-eslint/no-explicit-any */
/* eslint-enable @typescript-eslint/no-unsafe-assignment */
/* eslint-enable @typescript-eslint/no-unsafe-member-access */
Expand All @@ -413,7 +409,6 @@ function parseLoadVideoOptions(
keySystems,
lowLatencyMode,
manifestLoader: options.manifestLoader,
manifestUpdateUrl: options.manifestUpdateUrl,
minimumManifestUpdateInterval,
requestConfig,
onCodecSwitch,
Expand Down
4 changes: 2 additions & 2 deletions src/core/api/public_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -547,11 +547,11 @@ class Player extends EventEmitter<IPublicAPIEvent> {
transport,
checkMediaSegmentIntegrity,
manifestLoader,
manifestUpdateUrl,
referenceDateTime,
representationFilter,
segmentLoader,
serverSyncInfos,
__priv_manifestUpdateUrl,
__priv_patchLastSegmentInSidx,
url } = options;

Expand Down Expand Up @@ -587,11 +587,11 @@ class Player extends EventEmitter<IPublicAPIEvent> {
const transportPipelines = transportFn({ lowLatencyMode,
checkMediaSegmentIntegrity,
manifestLoader,
manifestUpdateUrl,
referenceDateTime,
representationFilter,
segmentLoader,
serverSyncInfos,
__priv_manifestUpdateUrl,
__priv_patchLastSegmentInSidx });

/** Interface used to load and refresh the Manifest. */
Expand Down
3 changes: 0 additions & 3 deletions src/public_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,6 @@ export interface ILoadVideoOptions {
/** Custom implementation for performing Manifest requests. */
manifestLoader? : IManifestLoader;

/** Possible custom URL pointing to a shorter form of the Manifest. */
manifestUpdateUrl? : string;

/** Minimum bound for Manifest updates, in milliseconds. */
minimumManifestUpdateInterval? : number;

Expand Down
2 changes: 1 addition & 1 deletion src/transports/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -795,10 +795,10 @@ export interface ITransportOptions {
checkMediaSegmentIntegrity? : boolean | undefined;
lowLatencyMode : boolean;
manifestLoader?: IManifestLoader | undefined;
manifestUpdateUrl? : string | undefined;
referenceDateTime? : number | undefined;
representationFilter? : IRepresentationFilter | undefined;
segmentLoader? : ICustomSegmentLoader | undefined;
serverSyncInfos? : IServerSyncInfos | undefined;
__priv_manifestUpdateUrl? : string | undefined;
__priv_patchLastSegmentInSidx? : boolean | undefined;
}

0 comments on commit 4552426

Please sign in to comment.