From e353d595078681ae80298c1a8ac9ca39835865c3 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Fri, 23 Aug 2024 11:56:45 +0200 Subject: [PATCH] [Proposal] Add to config most compat switches 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.). --- .../__tests__/is_seeking_approximate.test.ts | 8 +- ...rely_on_request_media_key_system_access.ts | 5 ++ src/compat/can_reuse_media_keys.ts | 4 +- ...can_seek_directly_after_loaded_metadata.ts | 8 +- ..._issues_with_high_media_source_duration.ts | 4 +- src/compat/is_seeking_approximate.ts | 9 ++- ...dia_element_fail_on_undecipherable_data.ts | 8 +- src/compat/should_await_set_media_keys.ts | 4 +- src/compat/should_favour_custom_safari_EME.ts | 7 +- ..._media_source_on_decipherability_update.ts | 10 ++- .../should_renew_media_key_system_access.ts | 4 +- src/compat/should_unset_media_keys.ts | 4 +- src/compat/should_validate_metadata.ts | 4 +- .../should_wait_for_data_before_loaded.ts | 4 +- .../should_wait_for_have_enough_data.ts | 4 +- src/default_config.ts | 81 +++++++++++++++++++ .../init/media_source_content_initializer.ts | 2 +- .../init/multi_thread_content_initializer.ts | 4 +- .../init/utils/initial_seek_and_play.ts | 2 +- .../media_element_playback_observer.ts | 4 +- 20 files changed, 153 insertions(+), 27 deletions(-) diff --git a/src/compat/__tests__/is_seeking_approximate.test.ts b/src/compat/__tests__/is_seeking_approximate.test.ts index e8c616ec83..cd991bccfe 100644 --- a/src/compat/__tests__/is_seeking_approximate.test.ts +++ b/src/compat/__tests__/is_seeking_approximate.test.ts @@ -15,19 +15,19 @@ describe("isSeekingApproximate", () => { vi.doMock("../browser_detection", () => { return { isTizen: true }; }); - const shouldAppendBufferAfterPadding = (await vi.importActual( + const isSeekingApproximate = (await vi.importActual( "../is_seeking_approximate", )) as any; - expect(shouldAppendBufferAfterPadding.default).toBe(true); + expect(isSeekingApproximate.default()).toBe(true); }); it("should be false if not on tizen", async () => { vi.doMock("../browser_detection", () => { return { isTizen: false }; }); - const shouldAppendBufferAfterPadding = (await vi.importActual( + const isSeekingApproximate = (await vi.importActual( "../is_seeking_approximate", )) as any; - expect(shouldAppendBufferAfterPadding.default).toBe(false); + expect(isSeekingApproximate.default()).toBe(false); }); }); diff --git a/src/compat/can_rely_on_request_media_key_system_access.ts b/src/compat/can_rely_on_request_media_key_system_access.ts index 4270923af7..4bb971152d 100644 --- a/src/compat/can_rely_on_request_media_key_system_access.ts +++ b/src/compat/can_rely_on_request_media_key_system_access.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import config from "../config"; import { isEdgeChromium } from "./browser_detection"; /** @@ -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; } diff --git a/src/compat/can_reuse_media_keys.ts b/src/compat/can_reuse_media_keys.ts index 851d776c6d..4556d2f59f 100644 --- a/src/compat/can_reuse_media_keys.ts +++ b/src/compat/can_reuse_media_keys.ts @@ -1,3 +1,4 @@ +import config from "../config"; import { isPanasonic, isWebOs } from "./browser_detection"; /** @@ -13,5 +14,6 @@ import { isPanasonic, isWebOs } from "./browser_detection"; * @returns {boolean} */ export default function canReuseMediaKeys(): boolean { - return !isWebOs && !isPanasonic; + const { FORCE_CANNOT_REUSE_MEDIA_KEYS } = config.getCurrent(); + return !(FORCE_CANNOT_REUSE_MEDIA_KEYS || isWebOs || isPanasonic); } diff --git a/src/compat/can_seek_directly_after_loaded_metadata.ts b/src/compat/can_seek_directly_after_loaded_metadata.ts index 00131c1b58..3a0d2d781e 100644 --- a/src/compat/can_seek_directly_after_loaded_metadata.ts +++ b/src/compat/can_seek_directly_after_loaded_metadata.ts @@ -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); +} diff --git a/src/compat/has_issues_with_high_media_source_duration.ts b/src/compat/has_issues_with_high_media_source_duration.ts index e3a4007289..12951f00b4 100644 --- a/src/compat/has_issues_with_high_media_source_duration.ts +++ b/src/compat/has_issues_with_high_media_source_duration.ts @@ -1,3 +1,4 @@ +import config from "../config"; import { isPlayStation5 } from "./browser_detection"; /** @@ -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; } diff --git a/src/compat/is_seeking_approximate.ts b/src/compat/is_seeking_approximate.ts index da348affd4..5e44a9cef2 100644 --- a/src/compat/is_seeking_approximate.ts +++ b/src/compat/is_seeking_approximate.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import config from "../config"; import { isTizen } from "./browser_detection"; /** @@ -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; +} diff --git a/src/compat/may_media_element_fail_on_undecipherable_data.ts b/src/compat/may_media_element_fail_on_undecipherable_data.ts index 2e5eb76259..003fce2532 100644 --- a/src/compat/may_media_element_fail_on_undecipherable_data.ts +++ b/src/compat/may_media_element_fail_on_undecipherable_data.ts @@ -1,3 +1,4 @@ +import config from "../config"; import { isPlayStation5 } from "./browser_detection"; /** @@ -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; +} diff --git a/src/compat/should_await_set_media_keys.ts b/src/compat/should_await_set_media_keys.ts index 696f72f879..31de5ab062 100644 --- a/src/compat/should_await_set_media_keys.ts +++ b/src/compat/should_await_set_media_keys.ts @@ -1,3 +1,4 @@ +import config from "../config"; import { isWebOs } from "./browser_detection"; /** @@ -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; } diff --git a/src/compat/should_favour_custom_safari_EME.ts b/src/compat/should_favour_custom_safari_EME.ts index 242d0434a0..153d8ae7b9 100644 --- a/src/compat/should_favour_custom_safari_EME.ts +++ b/src/compat/should_favour_custom_safari_EME.ts @@ -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"; @@ -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) + ); } diff --git a/src/compat/should_reload_media_source_on_decipherability_update.ts b/src/compat/should_reload_media_source_on_decipherability_update.ts index cb1cecf3d0..0c1f93226b 100644 --- a/src/compat/should_reload_media_source_on_decipherability_update.ts +++ b/src/compat/should_reload_media_source_on_decipherability_update.ts @@ -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. @@ -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 + ); } diff --git a/src/compat/should_renew_media_key_system_access.ts b/src/compat/should_renew_media_key_system_access.ts index aee6041dff..c0baaa0294 100644 --- a/src/compat/should_renew_media_key_system_access.ts +++ b/src/compat/should_renew_media_key_system_access.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import config from "../config"; import { isIE11 } from "./browser_detection"; /** @@ -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; } diff --git a/src/compat/should_unset_media_keys.ts b/src/compat/should_unset_media_keys.ts index 90d3f2d964..d31d050f3a 100644 --- a/src/compat/should_unset_media_keys.ts +++ b/src/compat/should_unset_media_keys.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import config from "../config"; import { isIE11 } from "./browser_detection"; /** @@ -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; } diff --git a/src/compat/should_validate_metadata.ts b/src/compat/should_validate_metadata.ts index 691d905ea9..a51b9dc39f 100644 --- a/src/compat/should_validate_metadata.ts +++ b/src/compat/should_validate_metadata.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import config from "../config"; import { isSamsungBrowser } from "./browser_detection"; /** @@ -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; } diff --git a/src/compat/should_wait_for_data_before_loaded.ts b/src/compat/should_wait_for_data_before_loaded.ts index 54e7ad5705..2fa8c6495b 100644 --- a/src/compat/should_wait_for_data_before_loaded.ts +++ b/src/compat/should_wait_for_data_before_loaded.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import config from "../config"; import { isSafariMobile } from "./browser_detection"; /** @@ -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; diff --git a/src/compat/should_wait_for_have_enough_data.ts b/src/compat/should_wait_for_have_enough_data.ts index 80bc63ad53..8dcd7ba0db 100644 --- a/src/compat/should_wait_for_have_enough_data.ts +++ b/src/compat/should_wait_for_have_enough_data.ts @@ -1,3 +1,4 @@ +import config from "../config"; import { isPlayStation5 } from "./browser_detection"; /** @@ -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; } diff --git a/src/default_config.ts b/src/default_config.ts index 7e1490d5c9..6ac8327a5f 100644 --- a/src/default_config.ts +++ b/src/default_config.ts @@ -1191,6 +1191,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; diff --git a/src/main_thread/init/media_source_content_initializer.ts b/src/main_thread/init/media_source_content_initializer.ts index 1ebd02ded7..8e5b036233 100644 --- a/src/main_thread/init/media_source_content_initializer.ts +++ b/src/main_thread/init/media_source_content_initializer.ts @@ -670,7 +670,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", diff --git a/src/main_thread/init/multi_thread_content_initializer.ts b/src/main_thread/init/multi_thread_content_initializer.ts index b45fa44faa..a3ed5c8fb6 100644 --- a/src/main_thread/init/multi_thread_content_initializer.ts +++ b/src/main_thread/init/multi_thread_content_initializer.ts @@ -1209,7 +1209,7 @@ export default class MultiThreadContentInitializer extends ContentInitializer { updates, ); if ( - mayMediaElementFailOnUndecipherableData && + mayMediaElementFailOnUndecipherableData() && manUpdates.some((e) => e.representation.decipherable !== true) ) { reloadMediaSource(); @@ -1237,7 +1237,7 @@ export default class MultiThreadContentInitializer extends ContentInitializer { protData, ); if ( - mayMediaElementFailOnUndecipherableData && + mayMediaElementFailOnUndecipherableData() && manUpdates.some((e) => e.representation.decipherable !== true) ) { reloadMediaSource(); diff --git a/src/main_thread/init/utils/initial_seek_and_play.ts b/src/main_thread/init/utils/initial_seek_and_play.ts index c485c20890..2f730f53c6 100644 --- a/src/main_thread/init/utils/initial_seek_and_play.ts +++ b/src/main_thread/init/utils/initial_seek_and_play.ts @@ -119,7 +119,7 @@ export default function performInitialSeekAndPlay( const initiallySeekedTime = typeof startTime === "number" ? startTime : startTime(); if (initiallySeekedTime !== 0) { - if (canSeekDirectlyAfterLoadedMetadata) { + if (canSeekDirectlyAfterLoadedMetadata()) { performInitialSeek(initiallySeekedTime); } else { setTimeout(() => { diff --git a/src/playback_observer/media_element_playback_observer.ts b/src/playback_observer/media_element_playback_observer.ts index 0dca709b49..dfdd433232 100644 --- a/src/playback_observer/media_element_playback_observer.ts +++ b/src/playback_observer/media_element_playback_observer.ts @@ -398,7 +398,7 @@ export default class PlaybackObserver { isInternalSeeking = true; tmpEvt = "internal-seeking"; const startedInternalSeekTime = this._internalSeeksIncoming.shift(); - this._expectedSeekingPosition = isSeekingApproximate + this._expectedSeekingPosition = isSeekingApproximate() ? Math.max(position, startedInternalSeekTime ?? 0) : position; } else { @@ -413,7 +413,7 @@ export default class PlaybackObserver { this._expectedSeekingPosition ?? 0, ); } else if ( - isSeekingApproximate && + isSeekingApproximate() && this._expectedSeekingPosition !== null && position < this._expectedSeekingPosition ) {