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.
canalplus · Feb 3, 2025 · e63c6ce · e63c6ce
1 parent 8a74e2b
commit e63c6ce
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 30 deletions.
diff --git a/src/main_thread/api/public_api.ts b/src/main_thread/api/public_api.ts
@@ -1241,7 +1241,6 @@ class Player extends EventEmitter<IPublicAPIEvent> {
           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
@@ -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<IPlaybackObservation>,
   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
@@ -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
@@ -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
@@ -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
@@ -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<IPlaybackObservation>,
-  mediaElement: IMediaElement,
   isDirectfile: boolean,
   cancelSignal: CancellationSignal,
 ): IReadOnlySharedReference<boolean> {
@@ -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
@@ -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<IStreamEventsEmitterEvent> {
   private _manifest: IManifestMetadata;
-  private _mediaElement: IMediaElement;
   private _playbackObserver: IReadOnlyPlaybackObserver<IPlaybackObservation>;
   private _scheduledEventsRef: SharedReference<
     Array<IStreamEventPayload | INonFiniteStreamEventPayload>
@@ -57,17 +55,14 @@ export default class StreamEventsEmitter extends EventEmitter<IStreamEventsEmitt
 
   /**
    * @param {Object} manifest
-   * @param {HTMLMediaElement} mediaElement
    * @param {Object} playbackObserver
    */
   constructor(
     manifest: IManifestMetadata,
-    mediaElement: IMediaElement,
     playbackObserver: IReadOnlyPlaybackObserver<IPlaybackObservation>,
   ) {
     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<IStreamEventsEmitt
 
     const cancelSignal = this._canceller.signal;
     const playbackObserver = this._playbackObserver;
-    const mediaElement = this._mediaElement;
 
     let isPollingEvents = false;
     let cancelCurrentPolling = new TaskCanceller();
@@ -137,7 +131,9 @@ export default class StreamEventsEmitter extends EventEmitter<IStreamEventsEmitt
 
         function constructObservation() {
           const lastObservation = playbackObserver.getReference().getValue();
-          const currentTime = mediaElement.currentTime;
+          const currentTime =
+            playbackObserver.getCurrentTime() ??
+            playbackObserver.getReference().getValue().position.getPolled();
           const isSeeking = lastObservation.seeking !== SeekingState.None;
           return { currentTime, isSeeking };
         }