From a80b2bb4084ca648aaf7fb0bd9f42ae3706eb617 Mon Sep 17 00:00:00 2001 From: Paul Berberian Date: Thu, 19 Dec 2024 17:41:31 +0100 Subject: [PATCH] Allow seeking through `seekTo` before the HTMLMediaElement is ready As #1600 signalled, there's currently no practical way to update the initial position (configured through the `startAt` `loadVideo` option) until `startAt` has actually been applied. More generally, it's not possible to perform a `seekTo` call until the initial seek has been performed on the content (the `seekTo` call will just be ignored). This commit proposes the following code updates to improve on that situation: 1. Tangentially related, we now schedule a JavaScript micro-task right at `ContentTimeBoundariesObserver`'s instantiation so the caller can catch `events` that were previously synchronously sent (I'm thinking here of the warnings for the `MEDIA_TIME_BEFORE_MANIFEST` and the `MEDIA_TIME_AFTER_MANIFEST` codes). Without this, we would wait for the next playback observation, which could happen a whole second later. 2. I noticed that the position indicated through the `startAt` `loadVideo` option was bounded so it's inside the `[minimumPosition, maximumPosition]` range (as obtained from the Manifest). As [stated here](https://github.com/canalplus/rx-player/issues/1600#issuecomment-2554545611) I personally think this is suboptimal. In my opinion, we should let the initial position go outside the range of the Manifest and let the application do its thing based on `MEDIA_TIME_BEFORE_MANIFEST` / `MEDIA_TIME_AFTER_MANIFEST` events. As to not totally change the behavior, I've only done so for dynamic contents (contents for which segments are added or removed, like live and contents that are being downloaded locally). VoD contents continue to have the previous behavior for now. 3. I added a "seek-blocking" mechanism to the `MediaElementPlaybackObserver` and made all seek operations, including the one from `seekTo` go through it. The idea is that when it is blocked (as it is initially), we'll delay any seek operation (by marking it as a "pending seek") and only seek to the last one of those once the `unblockSeeking` method is called (when the RxPlayer considers that the initial seek should be done). I also had to add `getPendingSeekInformation` method, sadly. I feel that we could do without it if we're smarter about things, but I wanted to avoid changing too much code here. I also thought about reworking the initial seek so it is completely handled by the `MediaElementPlaybackObserver` - as it could make everything simpler - but I chose for now to implement that less disruptive "seek-blocking" mechanism for now. Note that technically, we could still have an application directly updating the `HTMLMediaElement.property.currentTime` property, or even a web feature such as picture-in-picture doing that. I ensured that this eventuality did not break anything. Still, there will be a preference for pending seeks performed through the `MediaElementPlaybackObserver` over such "HTML5 seeks" performed during that time (if there is a "pending seek", we will apply it regardless of if an "HTML5 seek" happened since then). I'm now unsure if the `getPosition` or `getCurrentBufferGap` API should now return that planned position. I did nothing for those yet. --- doc/api/Basic_Methods/seekTo.md | 7 +- .../content_time_boundaries_observer.ts | 53 ++++---- src/main_thread/api/public_api.ts | 41 +++--- .../init/utils/get_initial_time.ts | 8 +- .../init/utils/initial_seek_and_play.ts | 18 ++- .../media_element_playback_observer.ts | 119 +++++++++++++++--- .../scenarios/loadVideo_options.test.js | 22 ++++ 7 files changed, 206 insertions(+), 62 deletions(-) diff --git a/doc/api/Basic_Methods/seekTo.md b/doc/api/Basic_Methods/seekTo.md index d7a7f96d20..ded08eba13 100644 --- a/doc/api/Basic_Methods/seekTo.md +++ b/doc/api/Basic_Methods/seekTo.md @@ -8,8 +8,8 @@ The argument can be an object with a single `Number` property, either: - `relative`: seek relatively to the current position -- `position`: seek to the given absolute position (equivalent to - `player.getVideoElement().currentTime = newPosition`) +- `position`: seek to the given absolute position (equivalent to what you would give to + `player.getVideoElement().currentTime) - `wallClockTime`: seek to the given wallClock position, as returned by `getWallClockTime`. @@ -17,8 +17,7 @@ The argument can be an object with a single `Number` property, either: The argument can also just be a `Number` property, which will have the same effect than the `position` property (absolute position). -Seeking should only be done when a content is loaded (i.e. the player isn't in the -`STOPPED`, `LOADING` or `RELOADING` state). +Seeking cannot be done when the content is `STOPPED`. The seek operation will start as soon as possible, in almost every cases directly after this method is called. diff --git a/src/core/main/common/content_time_boundaries_observer.ts b/src/core/main/common/content_time_boundaries_observer.ts index d8f6f55eeb..f425eaf151 100644 --- a/src/core/main/common/content_time_boundaries_observer.ts +++ b/src/core/main/common/content_time_boundaries_observer.ts @@ -29,6 +29,7 @@ import type { IReadOnlyPlaybackObserver } from "../../../playback_observer"; import type { IPlayerError } from "../../../public_types"; import EventEmitter from "../../../utils/event_emitter"; import isNullOrUndefined from "../../../utils/is_null_or_undefined"; +import queueMicrotask from "../../../utils/queue_microtask"; import SortedList from "../../../utils/sorted_list"; import TaskCanceller from "../../../utils/task_canceller"; @@ -94,29 +95,35 @@ export default class ContentTimeBoundariesObserver extends EventEmitter { - const wantedPosition = position.getWanted(); - if (wantedPosition < manifest.getMinimumSafePosition()) { - const warning = new MediaError( - "MEDIA_TIME_BEFORE_MANIFEST", - "The current position is behind the " + - "earliest time announced in the Manifest.", - ); - this.trigger("warning", warning); - } else if ( - wantedPosition > maximumPositionCalculator.getMaximumAvailablePosition() - ) { - const warning = new MediaError( - "MEDIA_TIME_AFTER_MANIFEST", - "The current position is after the latest " + - "time announced in the Manifest.", - ); - this.trigger("warning", warning); - } - }, - { includeLastObservation: true, clearSignal: cancelSignal }, - ); + + // As the following code may send events synchronously, which would not be + // catchable as a caller could not have called `addEventListener` yet, + // we schedule it in a micro-task + queueMicrotask(() => { + playbackObserver.listen( + ({ position }) => { + const wantedPosition = position.getWanted(); + if (wantedPosition < manifest.getMinimumSafePosition()) { + const warning = new MediaError( + "MEDIA_TIME_BEFORE_MANIFEST", + "The current position is behind the " + + "earliest time announced in the Manifest.", + ); + this.trigger("warning", warning); + } else if ( + wantedPosition > maximumPositionCalculator.getMaximumAvailablePosition() + ) { + const warning = new MediaError( + "MEDIA_TIME_AFTER_MANIFEST", + "The current position is after the latest " + + "time announced in the Manifest.", + ); + this.trigger("warning", warning); + } + }, + { includeLastObservation: true, clearSignal: cancelSignal }, + ); + }); manifest.addEventListener( "manifestUpdate", diff --git a/src/main_thread/api/public_api.ts b/src/main_thread/api/public_api.ts index 57c820db34..1ce49f4354 100644 --- a/src/main_thread/api/public_api.ts +++ b/src/main_thread/api/public_api.ts @@ -1014,10 +1014,27 @@ class Player extends EventEmitter { }); } + /** Global "playback observer" which will emit playback conditions */ + const playbackObserver = new MediaElementPlaybackObserver(videoElement, { + withMediaSource: !isDirectFile, + lowLatencyMode, + }); + + /* + * We want to block seeking operations until we know the media element is + * ready for it. + */ + playbackObserver.blockSeeking(); + + currentContentCanceller.signal.register(() => { + playbackObserver.stop(); + }); + /** Future `this._priv_contentInfos` related to this content. */ const contentInfos: IPublicApiContentInfos = { contentId: generateContentId(), originalUrl: url, + playbackObserver, currentContentCanceller, defaultAudioTrackSwitchingMode, initializer, @@ -1108,16 +1125,6 @@ class Player extends EventEmitter { // content. this.stop(); - /** Global "playback observer" which will emit playback conditions */ - const playbackObserver = new MediaElementPlaybackObserver(videoElement, { - withMediaSource: !isDirectFile, - lowLatencyMode, - }); - - currentContentCanceller.signal.register(() => { - playbackObserver.stop(); - }); - // Update the RxPlayer's state at the right events const playerStateRef = constructPlayerStateReference( initializer, @@ -1677,10 +1684,6 @@ class Player extends EventEmitter { } const { isDirectFile, manifest } = this._priv_contentInfos; - if (!isDirectFile && manifest === null) { - throw new Error("player: the content did not load yet"); - } - let positionWanted: number | undefined; if (typeof time === "number") { @@ -1700,7 +1703,11 @@ class Player extends EventEmitter { } else if (!isNullOrUndefined(timeObj.wallClockTime)) { if (manifest !== null) { positionWanted = timeObj.wallClockTime - (manifest.availabilityStartTime ?? 0); - } else if (isDirectFile && this.videoElement !== null) { + } else if (!isDirectFile) { + throw new Error( + "Cannot seek: wallClockTime asked but Manifest not yet loaded.", + ); + } else if (this.videoElement !== null) { const startDate = getStartDate(this.videoElement); if (startDate !== undefined) { positionWanted = timeObj.wallClockTime - startDate; @@ -1722,7 +1729,7 @@ class Player extends EventEmitter { throw new Error("invalid time given"); } log.info("API: API Seek to", positionWanted); - this.videoElement.currentTime = positionWanted; + this._priv_contentInfos.playbackObserver.setCurrentTime(positionWanted, false); return positionWanted; } @@ -3376,6 +3383,8 @@ interface IPublicApiContentInfos { originalUrl: string | undefined; /** `ContentInitializer` used to load the content. */ initializer: ContentInitializer; + /** interface emitting regularly playback observations. */ + playbackObserver: MediaElementPlaybackObserver; /** TaskCanceller triggered when it's time to stop the current content. */ currentContentCanceller: TaskCanceller; /** The default behavior to adopt when switching the audio track. */ diff --git a/src/main_thread/init/utils/get_initial_time.ts b/src/main_thread/init/utils/get_initial_time.ts index 2eacaf6afc..36156654bb 100644 --- a/src/main_thread/init/utils/get_initial_time.ts +++ b/src/main_thread/init/utils/get_initial_time.ts @@ -94,13 +94,19 @@ export default function getInitialTime( const min = getMinimumSafePosition(manifest); const max = getMaximumSafePosition(manifest); if (!isNullOrUndefined(startAt.position)) { - log.debug("Init: using startAt.minimumPosition"); + log.debug("Init: using startAt.position"); + if (manifest.isDynamic) { + return startAt.position; + } return Math.max(Math.min(startAt.position, max), min); } else if (!isNullOrUndefined(startAt.wallClockTime)) { log.debug("Init: using startAt.wallClockTime"); const ast = manifest.availabilityStartTime === undefined ? 0 : manifest.availabilityStartTime; const position = startAt.wallClockTime - ast; + if (manifest.isDynamic) { + return position; + } return Math.max(Math.min(position, max), min); } else if (!isNullOrUndefined(startAt.fromFirstPosition)) { log.debug("Init: using startAt.fromFirstPosition"); 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 7d4919b290..5fd0a3adf3 100644 --- a/src/main_thread/init/utils/initial_seek_and_play.ts +++ b/src/main_thread/init/utils/initial_seek_and_play.ts @@ -93,8 +93,20 @@ export default function performInitialSeekAndPlay( let hasAskedForInitialSeek = false; const performInitialSeek = (initialSeekTime: number) => { - playbackObserver.setCurrentTime(initialSeekTime); + const pendingSeek = playbackObserver.getPendingSeekInformation(); + + /* + * NOTE: The user might have asked for a seek before the media element + * was ready, in which case we want the seek to be at the user's wanted + * position instead. + * If multiple internal seeks were asked however, we want to keep the + * last one. + */ + if (pendingSeek === null || pendingSeek.isInternal) { + playbackObserver.setCurrentTime(initialSeekTime); + } hasAskedForInitialSeek = true; + playbackObserver.unblockSeeking(); }; // `startTime` defined as a function might depend on metadata to make its @@ -109,6 +121,8 @@ export default function performInitialSeekAndPlay( typeof startTime === "number" ? startTime : startTime(); if (initiallySeekedTime !== 0 && initiallySeekedTime !== undefined) { performInitialSeek(initiallySeekedTime); + } else { + playbackObserver.unblockSeeking(); } waitForSeekable(); } else { @@ -141,6 +155,8 @@ export default function performInitialSeekAndPlay( performInitialSeek(initiallySeekedTime); }, 0); } + } else { + playbackObserver.unblockSeeking(); } waitForSeekable(); } diff --git a/src/playback_observer/media_element_playback_observer.ts b/src/playback_observer/media_element_playback_observer.ts index ab98c2ca7d..d0af56a4e5 100644 --- a/src/playback_observer/media_element_playback_observer.ts +++ b/src/playback_observer/media_element_playback_observer.ts @@ -80,12 +80,13 @@ export default class PlaybackObserver { private _lowLatencyMode: boolean; /** - * If set, position which could not yet be seeked to as the HTMLMediaElement - * had a readyState of `0`. - * This position should be seeked to as soon as the HTMLMediaElement is able - * to handle it. + * If set, position which could not yet be seeked to due to either seeking + * operations being blocked or due to the HTMLMediaElement having a + * `readyState` of `0`. + * This position should be seeked to as soon as none of those conditions are + * met. */ - private _pendingSeek: number | null; + private _pendingSeek: IPendingSeekInformation | null; /** * The RxPlayer usually wants to differientate when a seek was sourced from @@ -131,6 +132,13 @@ export default class PlaybackObserver { */ private _expectedSeekingPosition: number | null; + /** + * If `true` seek operations asked through the + * `MediaElementPlaybackObserver` will not be performed now but after the + * `unblockSeeking` method is called. + */ + private _isSeekBlocked: boolean; + /** * Create a new `PlaybackObserver`, which allows to produce new "playback * observations" on various media events and intervals. @@ -150,12 +158,13 @@ export default class PlaybackObserver { this._observationRef = this._createSharedReference(); this._expectedSeekingPosition = null; this._pendingSeek = null; + this._isSeekBlocked = false; const onLoadedMetadata = () => { - if (this._pendingSeek !== null) { - const positionToSeekTo = this._pendingSeek; + if (this._pendingSeek !== null && !this._isSeekBlocked) { + const { position: positionToSeekTo, isInternal } = this._pendingSeek; this._pendingSeek = null; - this._actuallySetCurrentTime(positionToSeekTo); + this._actuallySetCurrentTime(positionToSeekTo, isInternal); } }; mediaElement.addEventListener("loadedmetadata", onLoadedMetadata); @@ -205,6 +214,69 @@ export default class PlaybackObserver { return this._mediaElement.paused; } + /** + * Prevent seeking operations from being performed from inside the + * `MediaElementPlaybackObserver` until the `unblockSeeking` method is called. + * + * You might want to call this method when you want to ensure that the next + * seek operation on the media element happens at a specific, controlled, + * point in time. + */ + public blockSeeking(): void { + this._isSeekBlocked = true; + } + + /** + * Remove seeking block created by the `blockSeeking` method if it was called. + * + * If a seek operation was requested while the block was active, the + * `MediaElementPlaybackObserver` will seek at the last seeked position as + * soon as possible (either right now, or when the `readyState` of the + * `HTMLMediaElement` will have at least reached the `"HAVE_METADATA"` state). + */ + public unblockSeeking(): void { + if (this._isSeekBlocked) { + this._isSeekBlocked = false; + if (this._pendingSeek !== null && this._mediaElement.readyState >= 1) { + const { position: positionToSeekTo, isInternal } = this._pendingSeek; + this._pendingSeek = null; + this._actuallySetCurrentTime(positionToSeekTo, isInternal); + } + } + } + + /** + * Returns `true` if seeking operations are currently blocked due to a call to + * `blockSeeking` that was not yet undone by a call to `unblockSeeking`. + * @returns {boolean} - `true` if seeking operations are blocked currently. + */ + public isSeekingBlocked(): boolean { + return this._isSeekBlocked; + } + + /** + * Seek operations, as performed by the `setCurrentTime` method, might be not + * yet performed due to either of those reasons: + * + * - Seek operations are blocked due to a call to the `blockSeeking` method. + * + * - The `HTMLMediaElement`'s `readyState` property has not yet reached the + * `"HAVE_METADATA"` state. + * + * Under any of those two conditions, this method will return the position + * that is planned to be seeked to as soon as both conditions are not met + * anymore. + * + * If seeks are possible right now, no seek should be "pending" and as such + * this method will return `null`. + * + * @returns {Object|null} - If a seek is planned, the position to seek to. + * `null` otherwise. + */ + public getPendingSeekInformation(): IPendingSeekInformation | null { + return this._pendingSeek; + } + /** * Update the current position (seek) on the `HTMLMediaElement`, by giving a * new position in seconds. @@ -214,14 +286,18 @@ export default class PlaybackObserver { * observation than regular seeks (which most likely comes from the outside, * e.g. the user). * @param {number} time + * @param {boolean} [isInternal=true] - If `false`, the seek was performed by + * the user. */ - public setCurrentTime(time: number): void { - if (this._mediaElement.readyState >= 1) { - this._actuallySetCurrentTime(time); + public setCurrentTime(time: number, isInternal: boolean = true): void { + if (!this._isSeekBlocked && this._mediaElement.readyState >= 1) { + this._actuallySetCurrentTime(time, isInternal); } else { + this._pendingSeek = { position: time, isInternal }; this._internalSeeksIncoming = []; - this._pendingSeek = time; - this._generateObservationForEvent("manual"); + if (!this._isSeekBlocked) { + this._generateObservationForEvent("manual"); + } } } @@ -303,9 +379,11 @@ export default class PlaybackObserver { return generateReadOnlyObserver(this, transform, this._canceller.signal); } - private _actuallySetCurrentTime(time: number): void { - log.info("API: Seeking internally", time); - this._internalSeeksIncoming.push(time); + private _actuallySetCurrentTime(time: number, isInternal: boolean): void { + log.info("API: Actually seeking", time, isInternal); + if (isInternal) { + this._internalSeeksIncoming.push(time); + } this._mediaElement.currentTime = time; } @@ -385,7 +463,7 @@ export default class PlaybackObserver { let isInternalSeeking = false; /** If set, the position for which we plan to seek to as soon as possible. */ - let pendingPosition: number | null = this._pendingSeek; + let pendingPosition: number | null = this._pendingSeek?.position ?? null; /** Initially-polled playback observation, before adjustments. */ const mediaTimings = getMediaInfos(this._mediaElement); @@ -945,3 +1023,10 @@ function getInitialObservation(mediaElement: IMediaElement): IPlaybackObservatio currentRange: null, }); } + +interface IPendingSeekInformation { + /** Position to seek to. */ + position: number; + /** If `true`, the seek was performed by the RxPlayer's internal logic. */ + isInternal: boolean; +} diff --git a/tests/integration/scenarios/loadVideo_options.test.js b/tests/integration/scenarios/loadVideo_options.test.js index 97c931cc57..2025f30750 100644 --- a/tests/integration/scenarios/loadVideo_options.test.js +++ b/tests/integration/scenarios/loadVideo_options.test.js @@ -172,6 +172,28 @@ function runLoadVideoOptionsTests({ multithread } = {}) { }); }); + it("should allow seeking over startAt while content is still loading", async function () { + const startAt = 10; + player.loadVideo({ + transport: manifestInfos.transport, + url: manifestInfos.url, + autoPlay: false, + startAt: { position: startAt }, + }); + // In a microtask just to ensure asynchronicity without the content + // being loaded. + Promise.resolve().then(() => { + player.seekTo(20); + }); + await waitForLoadedStateAfterLoadVideo(player); + expect(player.getPlayerState()).to.equal("LOADED"); + const initialPosition = player.getPosition(); + expect(initialPosition).to.be.closeTo(20, 0.5); + await checkAfterSleepWithBackoff(null, () => { + expect(player.getPosition()).to.equal(initialPosition); + }); + }); + it("should seek at the right position if startAt.wallClockTime is set", async function () { const startAt = 10; player.loadVideo({