Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Allow seeking through seekTo before the HTMLMediaElement is ready #1607

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

peaBerberian
Copy link
Collaborator

@peaBerberian peaBerberian commented Dec 19, 2024

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 PR 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 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.

    Note that technically, we could still have an application directly updating the HTMLMediaElement.prototype.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.

@peaBerberian peaBerberian force-pushed the misc/seek-to-before-initial branch 2 times, most recently from 1a7a15d to f477269 Compare December 19, 2024 17:19
@peaBerberian peaBerberian added API Relative to the RxPlayer's API proposal This Pull Request or Issue is only a proposal for a change with the expectation of a debate on it Performance checks Performance tests are run on this issue / PR labels Dec 19, 2024
@peaBerberian peaBerberian force-pushed the misc/seek-to-before-initial branch 2 times, most recently from 8bb2583 to 6862827 Compare December 19, 2024 18:48
@peaBerberian peaBerberian added Performance checks Performance tests are run on this issue / PR and removed Performance checks Performance tests are run on this issue / PR labels Dec 19, 2024
@peaBerberian peaBerberian force-pushed the misc/seek-to-before-initial branch from 6862827 to 0988a00 Compare December 20, 2024 09:35
peaBerberian added a commit that referenced this pull request Dec 20, 2024
We have "performance" tests which allow to ensure that we're not having
what we could call "performance regressions", such as a higher delay to
have a content loaded from a `loadVideo` call.

I wanted to run them yesterday for #1607, but they hanged indefinitely.

Turns out that our static server script, that we use here just to host
and serve our testing scripts returned a promise that was never resolved.

_(Note: we don't rely on external test runners nor on test frameworks
for our performance tests, we manually detect and launch local
chrome + firefox binaries on a local server serving our bundled
scripts (themselves importing the RxPlayer), and at runtime those
scripts send results to another "result" server when they're done)_

It was never resolved because we never properly handled the case in that
script were we wanted to launch an HTTP server but not an HTTPS one for
the same resource(s). This is now fixed.

Hopeful that performance tests now work correctly, it's been a long time
since I last ran them.
peaBerberian added a commit that referenced this pull request Dec 20, 2024
This commit builds on #1607 to allow users of the demo to seek in the
content even as it is still loading.

In real use cases, a final user might rely on e.g. seeking thumbnails to
already know where to seek to without having to wait for initial
segments to be loaded.
This is a functionality we do see with other media players, YouTube also
does this for example.

To make the seeking bar more visible, I lightly changed its CSS rules.

Also, I conditioned the appearance of the "ProgressBar" based on if the
minimum and maximum seekable positions are known, as else, seeking in
that bar is not going to lead to a precize seek. This makes the content
not seekable until the Manifest has been loaded.
peaBerberian added a commit that referenced this pull request Dec 20, 2024
This commit builds on #1607 to allow users of the demo to seek in the
content even as it is still loading.

In real use cases, a final user might rely on e.g. seeking thumbnails to
already know where to seek to without having to wait for initial
segments to be loaded.
This is a functionality we do see with other media players, YouTube also
does this for example.

To make the seeking bar more visible, I lightly changed its CSS rules.

Also, I conditioned the appearance of the "ProgressBar" based on if the
minimum and maximum seekable positions are known, as else, seeking in
that bar is not going to lead to a precize seek. This makes the content
not seekable until the Manifest has been loaded.
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](#1600 (comment))
     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.
This commit builds on #1607 to allow users of the demo to seek in the
content even as it is still loading.

In real use cases, a final user might rely on e.g. seeking thumbnails to
already know where to seek to without having to wait for initial
segments to be loaded.
This is a functionality we do see with other media players, YouTube also
does this for example.

To make the seeking bar more visible, I lightly changed its CSS rules.

Also, I conditioned the appearance of the "ProgressBar" based on if the
minimum and maximum seekable positions are known, as else, seeking in
that bar is not going to lead to a precize seek. This makes the content
not seekable until the Manifest has been loaded.
@peaBerberian peaBerberian force-pushed the misc/seek-to-before-initial branch from 5e0767c to 6c6d7c0 Compare December 23, 2024 10:20
@peaBerberian peaBerberian added Performance checks Performance tests are run on this issue / PR and removed Performance checks Performance tests are run on this issue / PR labels Dec 23, 2024
@peaBerberian peaBerberian added this to the 4.3.0 milestone Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Relative to the RxPlayer's API Performance checks Performance tests are run on this issue / PR proposal This Pull Request or Issue is only a proposal for a change with the expectation of a debate on it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant