From e63c6ce77a5ffd99c34dc5651ef7708105a0b105 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Sun, 2 Feb 2025 17:52:39 +0100 Subject: [PATCH] Do not communicate needlessly the media element to multiple modules While rebasing our Proof-of-concept for content preloading, I noticed that multiple modules asked for the `HTMLMediaElement` to be provided to them as argument despite the fact that most of them also rely on a `PlaybackObserver` which fulfill most of its roles. I decided to just remove that unneeded dependency for them. A longer-term end goal might be to make `PlaybackObserver` the preferred interface to the `HTMLMediaElement` in general (and perhaps rename it), a bit like the `SourceBufferInterface` is for `SourceBuffer` objects. We're not too far from this. --- src/main_thread/api/public_api.ts | 1 - src/main_thread/api/utils.ts | 4 +--- src/main_thread/init/directfile_content_initializer.ts | 2 +- .../init/media_source_content_initializer.ts | 8 ++------ .../init/multi_thread_content_initializer.ts | 8 ++------ src/main_thread/init/utils/get_loaded_reference.ts | 9 +++------ .../stream_events_emitter/stream_events_emitter.ts | 10 +++------- 7 files changed, 12 insertions(+), 30 deletions(-) diff --git a/src/main_thread/api/public_api.ts b/src/main_thread/api/public_api.ts index 00cc94bc86..d1057c4b52 100644 --- a/src/main_thread/api/public_api.ts +++ b/src/main_thread/api/public_api.ts @@ -1241,7 +1241,6 @@ class Player extends EventEmitter { seekEventsCanceller = new TaskCanceller(); seekEventsCanceller.linkToSignal(currentContentCanceller.signal); emitSeekEvents( - videoElement, playbackObserver, () => this.trigger("seeking", null), () => this.trigger("seeked", null), diff --git a/src/main_thread/api/utils.ts b/src/main_thread/api/utils.ts index b9a18166ba..b02f69273b 100644 --- a/src/main_thread/api/utils.ts +++ b/src/main_thread/api/utils.ts @@ -30,7 +30,6 @@ import type { CancellationSignal } from "../../utils/task_canceller"; import type { ContentInitializer, IStallingSituation } from "../init"; /** - * @param {HTMLMediaElement} mediaElement * @param {Object} playbackObserver - Observes playback conditions on * `mediaElement`. * @param {function} onSeeking - Callback called when a seeking operation starts @@ -41,13 +40,12 @@ import type { ContentInitializer, IStallingSituation } from "../init"; * remove all listeners this function has registered. */ export function emitSeekEvents( - mediaElement: IMediaElement | null, playbackObserver: IReadOnlyPlaybackObserver, onSeeking: () => void, onSeeked: () => void, cancelSignal: CancellationSignal, ): void { - if (cancelSignal.isCancelled() || mediaElement === null) { + if (cancelSignal.isCancelled()) { return; } diff --git a/src/main_thread/init/directfile_content_initializer.ts b/src/main_thread/init/directfile_content_initializer.ts index 2d20cb3dd7..f6ae44eaf4 100644 --- a/src/main_thread/init/directfile_content_initializer.ts +++ b/src/main_thread/init/directfile_content_initializer.ts @@ -225,7 +225,7 @@ export default class DirectFileContentInitializer extends ContentInitializer { cancelSignal, ) .autoPlayResult.then(() => - getLoadedReference(playbackObserver, mediaElement, true, cancelSignal).onUpdate( + getLoadedReference(playbackObserver, true, cancelSignal).onUpdate( (isLoaded, stopListening) => { if (isLoaded) { stopListening(); diff --git a/src/main_thread/init/media_source_content_initializer.ts b/src/main_thread/init/media_source_content_initializer.ts index 81767de45c..1e6d52a919 100644 --- a/src/main_thread/init/media_source_content_initializer.ts +++ b/src/main_thread/init/media_source_content_initializer.ts @@ -617,11 +617,7 @@ export default class MediaSourceContentInitializer extends ContentInitializer { (isPerformed, stopListening) => { if (isPerformed) { stopListening(); - const streamEventsEmitter = new StreamEventsEmitter( - manifest, - mediaElement, - playbackObserver, - ); + const streamEventsEmitter = new StreamEventsEmitter(manifest, playbackObserver); manifest.addEventListener( "manifestUpdate", () => { @@ -748,7 +744,7 @@ export default class MediaSourceContentInitializer extends ContentInitializer { */ autoPlayResult .then(() => { - getLoadedReference(playbackObserver, mediaElement, false, cancelSignal).onUpdate( + getLoadedReference(playbackObserver, false, cancelSignal).onUpdate( (isLoaded, stopListening) => { if (isLoaded) { stopListening(); diff --git a/src/main_thread/init/multi_thread_content_initializer.ts b/src/main_thread/init/multi_thread_content_initializer.ts index 89ca5a1e56..1876206cf4 100644 --- a/src/main_thread/init/multi_thread_content_initializer.ts +++ b/src/main_thread/init/multi_thread_content_initializer.ts @@ -1563,11 +1563,7 @@ export default class MultiThreadContentInitializer extends ContentInitializer { (isPerformed, stopListening) => { if (isPerformed) { stopListening(); - const streamEventsEmitter = new StreamEventsEmitter( - manifest, - mediaElement, - playbackObserver, - ); + const streamEventsEmitter = new StreamEventsEmitter(manifest, playbackObserver); currentContentInfo.streamEventsEmitter = streamEventsEmitter; streamEventsEmitter.addEventListener( "event", @@ -1625,7 +1621,7 @@ export default class MultiThreadContentInitializer extends ContentInitializer { */ autoPlayResult .then(() => { - getLoadedReference(playbackObserver, mediaElement, false, cancelSignal).onUpdate( + getLoadedReference(playbackObserver, false, cancelSignal).onUpdate( (isLoaded, stopListening) => { if (isLoaded) { stopListening(); diff --git a/src/main_thread/init/utils/get_loaded_reference.ts b/src/main_thread/init/utils/get_loaded_reference.ts index b423a4ae0d..42efbf2592 100644 --- a/src/main_thread/init/utils/get_loaded_reference.ts +++ b/src/main_thread/init/utils/get_loaded_reference.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import type { IMediaElement } from "../../../compat/browser_compatibility_types"; import shouldValidateMetadata from "../../../compat/should_validate_metadata"; import shouldWaitForDataBeforeLoaded from "../../../compat/should_wait_for_data_before_loaded"; import shouldWaitForHaveEnoughData from "../../../compat/should_wait_for_have_enough_data"; @@ -31,14 +30,12 @@ import TaskCanceller from "../../../utils/task_canceller"; * Returns an `IReadOnlySharedReference` that switches to `true` once the * content is considered loaded (i.e. once it can begin to be played). * @param {Object} playbackObserver - * @param {HTMLMediaElement} mediaElement * @param {boolean} isDirectfile - `true` if this is a directfile content * @param {Object} cancelSignal * @returns {Object} */ export default function getLoadedReference( playbackObserver: IReadOnlyPlaybackObserver, - mediaElement: IMediaElement, isDirectfile: boolean, cancelSignal: CancellationSignal, ): IReadOnlySharedReference { @@ -58,10 +55,10 @@ export default function getLoadedReference( if (!shouldWaitForDataBeforeLoaded(isDirectfile)) { // The duration is NaN if no media data is available, // which means media is not loaded yet. - if (isNaN(mediaElement.duration)) { + if (isNaN(observation.duration)) { return; } - if (mediaElement.duration > 0) { + if (observation.duration > 0) { isLoaded.setValue(true); listenCanceller.cancel(); return; @@ -71,7 +68,7 @@ export default function getLoadedReference( const minReadyState = shouldWaitForHaveEnoughData() ? 4 : 3; if (observation.readyState >= minReadyState) { if (observation.currentRange !== null || observation.ended) { - if (!shouldValidateMetadata() || mediaElement.duration > 0) { + if (!shouldValidateMetadata() || observation.duration > 0) { isLoaded.setValue(true); listenCanceller.cancel(); return; diff --git a/src/main_thread/init/utils/stream_events_emitter/stream_events_emitter.ts b/src/main_thread/init/utils/stream_events_emitter/stream_events_emitter.ts index fcc60715a7..e7f59a2c40 100644 --- a/src/main_thread/init/utils/stream_events_emitter/stream_events_emitter.ts +++ b/src/main_thread/init/utils/stream_events_emitter/stream_events_emitter.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import type { IMediaElement } from "../../../../compat/browser_compatibility_types"; import config from "../../../../config"; import type { IManifestMetadata } from "../../../../manifest"; import { SeekingState } from "../../../../playback_observer"; @@ -44,7 +43,6 @@ interface IStreamEventsEmitterEvent { */ export default class StreamEventsEmitter extends EventEmitter { private _manifest: IManifestMetadata; - private _mediaElement: IMediaElement; private _playbackObserver: IReadOnlyPlaybackObserver; private _scheduledEventsRef: SharedReference< Array @@ -57,17 +55,14 @@ export default class StreamEventsEmitter extends EventEmitter, ) { super(); this._manifest = manifest; - this._mediaElement = mediaElement; this._playbackObserver = playbackObserver; this._canceller = null; this._scheduledEventsRef = new SharedReference< @@ -87,7 +82,6 @@ export default class StreamEventsEmitter extends EventEmitter