Skip to content

Commit

Permalink
[Proposal] Add to config most compat switches
Browse files Browse the repository at this point in the history
Based on #1510, the idea behind this proposal is to add to our config
properties allowing to force toggles we for now only enable for
specific devices.

The goal is to simplify the debugging of issues seen on specific devices
(which is the huge majority of them), by just having to update the
config in the corresponding application (as proposed by #1510).

So for example let's say that we encounter a new device where relying on
the same `MediaKeys` instance for multiple contents may fail after a
time (bug encountered on LG's WebOS, on some Panasonic TVs, and now, in
issue #1464, on Philips's TitanOS), we could just initially tell
application people to try setting that experimental flag.

If it fixes the issue, we will add a supplementary device check inside
the corresponding compat function.

This seems faster and less bothersome to me than having to create
special builds of the RxPlayer, and there our role could just be to
redirect the developer seeing the issue to the right config option
(as opposed to having to build a player, then link that player to the
application, making sure that their CI like us etc.).
  • Loading branch information
peaBerberian committed Nov 15, 2024
1 parent 0a66b5c commit e8a1824
Show file tree
Hide file tree
Showing 21 changed files with 161 additions and 33 deletions.
12 changes: 4 additions & 8 deletions src/compat/__tests__/is_seeking_approximate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,15 @@ describe("isSeekingApproximate", () => {
vi.doMock("../browser_detection", () => {
return { isTizen: true };
});
const shouldAppendBufferAfterPadding = (
await vi.importActual("../is_seeking_approximate")
).default;
expect(shouldAppendBufferAfterPadding).toBe(true);
const isSeekingApproximate = (await import("../is_seeking_approximate")).default;

Check failure on line 12 in src/compat/__tests__/is_seeking_approximate.test.ts

View workflow job for this annotation

GitHub Actions / typechecking_and_linting (20.x)

Dynamic imports are only supported when the '--module' flag is set to 'es2020', 'es2022', 'esnext', 'commonjs', 'amd', 'system', 'umd', 'node16', or 'nodenext'.
expect(isSeekingApproximate()).toBe(true);
});

it("should be false if not on tizen", async () => {
vi.doMock("../browser_detection", () => {
return { isTizen: false };
});
const shouldAppendBufferAfterPadding = (
await vi.importActual("../is_seeking_approximate")
).default;
expect(shouldAppendBufferAfterPadding).toBe(false);
const isSeekingApproximate = (await import("../is_seeking_approximate")).default;

Check failure on line 20 in src/compat/__tests__/is_seeking_approximate.test.ts

View workflow job for this annotation

GitHub Actions / typechecking_and_linting (20.x)

Dynamic imports are only supported when the '--module' flag is set to 'es2020', 'es2022', 'esnext', 'commonjs', 'amd', 'system', 'umd', 'node16', or 'nodenext'.
expect(isSeekingApproximate()).toBe(false);
});
});
5 changes: 5 additions & 0 deletions src/compat/can_rely_on_request_media_key_system_access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import config from "../config";
import { isEdgeChromium } from "./browser_detection";

/**
Expand All @@ -35,6 +36,10 @@ import { isEdgeChromium } from "./browser_detection";
* @returns {boolean}
*/
export function canRelyOnRequestMediaKeySystemAccess(keySystem: string): boolean {
const { FORCE_CANNOT_RELY_ON_REQUEST_MEDIA_KEY_SYSTEM_ACCESS } = config.getCurrent();
if (FORCE_CANNOT_RELY_ON_REQUEST_MEDIA_KEY_SYSTEM_ACCESS) {
return false;
}
if (isEdgeChromium && keySystem.indexOf("playready") !== -1) {
return false;
}
Expand Down
10 changes: 9 additions & 1 deletion src/compat/can_reuse_media_keys.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import config from "../config";
import { isA1KStb40xx, isPanasonic, isPhilipsNetTv, isWebOs } from "./browser_detection";

/**
Expand All @@ -19,5 +20,12 @@ import { isA1KStb40xx, isPanasonic, isPhilipsNetTv, isWebOs } from "./browser_de
* @returns {boolean}
*/
export default function canReuseMediaKeys(): boolean {
return !isWebOs && !isPhilipsNetTv && !isPanasonic && !isA1KStb40xx;
const { FORCE_CANNOT_REUSE_MEDIA_KEYS } = config.getCurrent();
return !(
FORCE_CANNOT_REUSE_MEDIA_KEYS ||
isWebOs ||
isPhilipsNetTv ||
isPanasonic ||
isA1KStb40xx
);
}
8 changes: 6 additions & 2 deletions src/compat/can_seek_directly_after_loaded_metadata.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import config from "../config";
import { isSafariMobile } from "./browser_detection";

/**
* On safari mobile (version 17.1.2) seeking too early cause the video to never buffer
* media data. Using delaying mechanisms such as `setTimeout(fn, 0)` defers the seek
* to a moment at which safari should be more able to handle a seek.
* @returns {boolean}
*/
const canSeekDirectlyAfterLoadedMetadata = !isSafariMobile;
export default canSeekDirectlyAfterLoadedMetadata;
export default function canSeekDirectlyAfterLoadedMetadata(): boolean {
const { FORCE_CANNOT_SEEK_DIRECTLY_AFTER_LOADED_METADATA } = config.getCurrent();
return !(FORCE_CANNOT_SEEK_DIRECTLY_AFTER_LOADED_METADATA || isSafariMobile);
}
4 changes: 3 additions & 1 deletion src/compat/has_issues_with_high_media_source_duration.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import config from "../config";
import { isPlayStation5 } from "./browser_detection";

/**
Expand All @@ -21,7 +22,8 @@ import { isPlayStation5 } from "./browser_detection";
* @returns {boolean}
*/
export default function hasIssuesWithHighMediaSourceDuration(): boolean {
const { FORCE_HAS_ISSUES_WITH_HIGH_MEDIA_SOURCE_DURATION } = config.getCurrent();
// For now only seen on the Webkit present in the PlayStation 5, for which the
// alternative is known to work.
return isPlayStation5;
return FORCE_HAS_ISSUES_WITH_HIGH_MEDIA_SOURCE_DURATION || isPlayStation5;
}
9 changes: 6 additions & 3 deletions src/compat/is_seeking_approximate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import config from "../config";
import { isTizen } from "./browser_detection";

/**
Expand All @@ -26,7 +27,9 @@ import { isTizen } from "./browser_detection";
*
* This boolean is only `true` on the devices where this behavior has been
* observed.
* @returns {boolean}
*/
const isSeekingApproximate: boolean = isTizen;

export default isSeekingApproximate;
export default function isSeekingApproximate(): boolean {
const { FORCE_IS_SEEKING_APPROXIMATE } = config.getCurrent();
return FORCE_IS_SEEKING_APPROXIMATE || isTizen;
}
8 changes: 6 additions & 2 deletions src/compat/may_media_element_fail_on_undecipherable_data.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import config from "../config";
import { isPlayStation5 } from "./browser_detection";

/**
Expand All @@ -13,6 +14,9 @@ import { isPlayStation5 } from "./browser_detection";
*
* Consequently, we have to specifically consider platforms with that
* fail-on-undecipherable-data issue, to perform a work-around in that case.
* @returns {boolean}
*/
const mayMediaElementFailOnUndecipherableData = isPlayStation5;
export default mayMediaElementFailOnUndecipherableData;
export default function mayMediaElementFailOnUndecipherableData(): boolean {
const { FORCE_MEDIA_ELEMENT_FAIL_ON_UNDECIPHERABLE_DATA } = config.getCurrent();
return FORCE_MEDIA_ELEMENT_FAIL_ON_UNDECIPHERABLE_DATA || isPlayStation5;
}
4 changes: 3 additions & 1 deletion src/compat/should_await_set_media_keys.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import config from "../config";
import { isWebOs } from "./browser_detection";

/**
Expand All @@ -16,5 +17,6 @@ import { isWebOs } from "./browser_detection";
* @returns {boolean}
*/
export default function shouldAwaitSetMediaKeys(): boolean {
return isWebOs;
const { FORCE_SHOULD_AWAIT_SET_MEDIA_KEYS } = config.getCurrent();
return FORCE_SHOULD_AWAIT_SET_MEDIA_KEYS || isWebOs;
}
7 changes: 6 additions & 1 deletion src/compat/should_favour_custom_safari_EME.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import config from "../config";
import { isSafariDesktop, isSafariMobile } from "./browser_detection";
import { WebKitMediaKeysConstructor } from "./eme/custom_media_keys/webkit_media_keys_constructor";

Expand All @@ -25,5 +26,9 @@ import { WebKitMediaKeysConstructor } from "./eme/custom_media_keys/webkit_media
* @returns {boolean}
*/
export default function shouldFavourCustomSafariEME(): boolean {
return (isSafariDesktop || isSafariMobile) && WebKitMediaKeysConstructor !== undefined;
const { FORCE_SHOULD_FAVOUR_CUSTOM_SAFARI_EME } = config.getCurrent();
return (
FORCE_SHOULD_FAVOUR_CUSTOM_SAFARI_EME ||
((isSafariDesktop || isSafariMobile) && WebKitMediaKeysConstructor !== undefined)
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

import config from "../config";

/**
* Returns true if we have to reload the MediaSource due to an update in the
* decipherability status of some segments based on the current key sytem.
Expand All @@ -27,5 +29,11 @@
export default function shouldReloadMediaSourceOnDecipherabilityUpdate(
currentKeySystem: string | undefined,
): boolean {
return currentKeySystem === undefined || currentKeySystem.indexOf("widevine") < 0;
const { FORCE_SHOULD_RELOAD_MEDIA_SOURCE_ON_DECIPHERABILITY_UPDATE } =
config.getCurrent();
return (
FORCE_SHOULD_RELOAD_MEDIA_SOURCE_ON_DECIPHERABILITY_UPDATE ||
currentKeySystem === undefined ||
currentKeySystem.indexOf("widevine") < 0
);
}
4 changes: 3 additions & 1 deletion src/compat/should_renew_media_key_system_access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import config from "../config";
import { isIE11 } from "./browser_detection";

/**
Expand All @@ -22,5 +23,6 @@ import { isIE11 } from "./browser_detection";
* @returns {Boolean}
*/
export default function shouldRenewMediaKeySystemAccess(): boolean {
return isIE11;
const { FORCE_SHOULD_RENEW_MEDIA_KEY_SYSTEM_ACCESS } = config.getCurrent();
return FORCE_SHOULD_RENEW_MEDIA_KEY_SYSTEM_ACCESS || isIE11;
}
4 changes: 3 additions & 1 deletion src/compat/should_unset_media_keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import config from "../config";
import { isIE11 } from "./browser_detection";

/**
Expand All @@ -23,5 +24,6 @@ import { isIE11 } from "./browser_detection";
* @returns {Boolean}
*/
export default function shouldUnsetMediaKeys(): boolean {
return isIE11;
const { FORCE_SHOULD_UNSET_MEDIA_KEYS } = config.getCurrent();
return FORCE_SHOULD_UNSET_MEDIA_KEYS || isIE11;
}
4 changes: 3 additions & 1 deletion src/compat/should_validate_metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import config from "../config";
import { isSamsungBrowser } from "./browser_detection";

/**
Expand All @@ -23,5 +24,6 @@ import { isSamsungBrowser } from "./browser_detection";
* @returns {boolean}
*/
export default function shouldValidateMetadata(): boolean {
return isSamsungBrowser;
const { FORCE_SHOULD_VALIDATE_METADATA } = config.getCurrent();
return FORCE_SHOULD_VALIDATE_METADATA || isSamsungBrowser;
}
4 changes: 3 additions & 1 deletion src/compat/should_wait_for_data_before_loaded.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

import config from "../config";
import { isSafariMobile } from "./browser_detection";

/**
Expand All @@ -25,7 +26,8 @@ import { isSafariMobile } from "./browser_detection";
* @returns {Boolean}
*/
export default function shouldWaitForDataBeforeLoaded(isDirectfile: boolean): boolean {
if (isDirectfile && isSafariMobile) {
const { FORCE_DONT_WAIT_FOR_DATA_BEFORE_LOADED } = config.getCurrent();
if (FORCE_DONT_WAIT_FOR_DATA_BEFORE_LOADED || (isDirectfile && isSafariMobile)) {
return false;
} else {
return true;
Expand Down
4 changes: 3 additions & 1 deletion src/compat/should_wait_for_have_enough_data.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import config from "../config";
import { isPlayStation5 } from "./browser_detection";

/**
Expand All @@ -13,5 +14,6 @@ import { isPlayStation5 } from "./browser_detection";
* @returns {boolean}
*/
export default function shouldWaitForHaveEnoughData(): boolean {
return isPlayStation5;
const { FORCE_WAIT_FOR_HAVE_ENOUGH_DATA } = config.getCurrent();
return FORCE_WAIT_FOR_HAVE_ENOUGH_DATA || isPlayStation5;
}
81 changes: 81 additions & 0 deletions src/default_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,87 @@ const DEFAULT_CONFIG = {
* one.
*/
DEFAULT_AUDIO_TRACK_SWITCHING_MODE: "seamless" as const,

// Compatibility toggles:

/**
* If set to `true`, we'll always try to check thoroughly that a
* `MediaKeySystemAccess` can be relied on.
*/
FORCE_CANNOT_RELY_ON_REQUEST_MEDIA_KEY_SYSTEM_ACCESS: false,

/**
* If set to `true`, we'll always re-create a `MediaKeys` instance for each
* encrypted content where we need one.
*/
FORCE_CANNOT_REUSE_MEDIA_KEYS: false,

/**
* If set to `true`, we'll never seek directly after receiving the
* `loadedmetadata` event from an `HTMLMediaElement`, instead waiting a small
* amount of time before.
*/
FORCE_CANNOT_SEEK_DIRECTLY_AFTER_LOADED_METADATA: false,

/**
* If set to `true`, force work-around for devices which have issues with
* `MediaSource` objects with a high `duration` property.
*/
FORCE_HAS_ISSUES_WITH_HIGH_MEDIA_SOURCE_DURATION: false,

/**
* If set to `true`, the device might seek before what we actually tell it to,
* breaking other RxPlayer behaviors in the process.
* Setting it to `true` allows to enable work-arounds.
*/
FORCE_IS_SEEKING_APPROXIMATE: false,

/**
* If set to `true`, the device might fail directly after uncountering
* decipherable data.
*/
FORCE_MEDIA_ELEMENT_FAIL_ON_UNDECIPHERABLE_DATA: false,

/**
* If set to `true` we'll await a `MediaKeys` attachment on a given
* `HTMLMediaElement` before trying to set a new one.
*/
FORCE_SHOULD_AWAIT_SET_MEDIA_KEYS: false,

/** If set to `true` we'll rely on Safari's Webkit flavor of the EME API. */
FORCE_SHOULD_FAVOUR_CUSTOM_SAFARI_EME: false,

/**
* If `true`, we'll reload if unencrypted data is encountered close to
* the current position.
*/
FORCE_SHOULD_RELOAD_MEDIA_SOURCE_ON_DECIPHERABILITY_UPDATE: false,

/** If `true`, we'll for each content re-create a `MediaKeySystemAccess`. */
FORCE_SHOULD_RENEW_MEDIA_KEY_SYSTEM_ACCESS: false,

/** If `true`, we'll unset the `MediaKeys` at each stop. */
FORCE_SHOULD_UNSET_MEDIA_KEYS: false,

/**
* If `true`, we cannot trust that a `loadedmetadata` event from the
* `HTMLMediaElement` is sent after the browser has parsed key metadata such
* as the content's duration.
*/
FORCE_SHOULD_VALIDATE_METADATA: false,

/**
* If `true`, we have to announce the content as loaded even if no data is
* actually loaded, because that target do not preload, meaning a `play` call
* is required.
*/
FORCE_DONT_WAIT_FOR_DATA_BEFORE_LOADED: false,

/**
* If `true`, we have to wait for the `HAVE_ENOUGH_DATA` `readyState` before
* announcing the content as loaded.
*/
FORCE_WAIT_FOR_HAVE_ENOUGH_DATA: false,
};

export type IDefaultConfig = typeof DEFAULT_CONFIG;
Expand Down
2 changes: 1 addition & 1 deletion src/main_thread/init/media_source_content_initializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ export default class MediaSourceContentInitializer extends ContentInitializer {
segmentSinksStore,
);

if (mayMediaElementFailOnUndecipherableData) {
if (mayMediaElementFailOnUndecipherableData()) {
// On some devices, just reload immediately when data become undecipherable
manifest.addEventListener(
"decipherabilityUpdate",
Expand Down
4 changes: 2 additions & 2 deletions src/main_thread/init/multi_thread_content_initializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1268,7 +1268,7 @@ export default class MultiThreadContentInitializer extends ContentInitializer {
updates,
);
if (
mayMediaElementFailOnUndecipherableData &&
mayMediaElementFailOnUndecipherableData() &&
manUpdates.some((e) => e.representation.decipherable !== true)
) {
reloadMediaSource();
Expand Down Expand Up @@ -1296,7 +1296,7 @@ export default class MultiThreadContentInitializer extends ContentInitializer {
protData,
);
if (
mayMediaElementFailOnUndecipherableData &&
mayMediaElementFailOnUndecipherableData() &&
manUpdates.some((e) => e.representation.decipherable !== true)
) {
reloadMediaSource();
Expand Down
2 changes: 1 addition & 1 deletion src/main_thread/init/utils/initial_seek_and_play.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export default function performInitialSeekAndPlay(
stopListening();

if (initiallySeekedTime !== 0 && initiallySeekedTime !== undefined) {
if (canSeekDirectlyAfterLoadedMetadata) {
if (canSeekDirectlyAfterLoadedMetadata()) {
performInitialSeek(initiallySeekedTime);
} else {
setTimeout(() => {
Expand Down
Loading

0 comments on commit e8a1824

Please sign in to comment.