Skip to content

Commit b24f4eb

Browse files
committed
Fix occurences of MEDIA_TIME_NOT_FOUND on initial fallback
We've recently seen `MEDIA_TIME_NOT_FOUND` errors in a Canal+ application. It's one of the errors that are part of our API that should actually be never sent to an application - unless there's an RxPlayer bug somewhere (it is documented as such in our API documentation). The issue --------- Turns out they were encountering a very specific race condition after a chain of events: 1. The application relied on a multi-threaded RxPlayer and played an encrypted content with some non-decipherable qualities. They also have all conditions met to allow our cache of `MediaKeySession` (which allows to reuse already-loaded decryption keys). 2. They then loaded another content, with a completely different media position that does not map to anything in the previous content (it shouldn't matter, but it will be important later) 3. The application switches again to the first content, without calling `stop` in between (which is OK and even more performant). In that situation, the following happen just after the third step: 4. We start to initialize everything for that last content. Since recently (#1607), that initialization step includes the initial polling of media metrics such as the position, the playbackRate etc. Before that work, this polling was done in a later step. 5. We stop the previous content. We do this after initializing the next content on purpose. "Stopping" a content (setting `mediaElement.src = ""`, typically) is a synchronous/blocking operations in JS that can actually take a lot of time - like hundred of ms on lower-end devices. To improve performance, we thus "initialize" the next content before stopping the previous one, as the former includes some parallelizable operations (network requests, `postMessage` to our Worker etc.) that can still take place while the "stop operation" is blocking the JS main thread. 6. The RxPlayer core (running in a WebWorker here) initialize everything, fetches the Manifest etc. Here the content is already known and its decryption keys are still "cached" locally, so we directly know that some qualities in the Manifest are not decipherable and decide to "fallback" from the higher qualities. 7. The fallback mechanism reads the last polled media metrics. If we reached that logic too fast, we will actually read the initially-polled metrics (3 steps earlier). This should be OK, but because we last polled the media metrics _BEFORE_ stopping the previous content **AND** no metrics has been emitted since then, the playback position in those metrics is actually the last position reached in the previous content. 8. The RxPlayer checks that wanted position, see that it doesn't make sense in the current content, and triggers the `MEDIA_TIME_NOT_FOUND` error. The fixes --------- The crux of this issue is both that: 1. we poll media metrics before stopping the previous content and, 2. we do not emit new metrics immediately when the initial position to seek to is known The fix I ended up choosing is very far from being the most straightforward one though :D. It's a solution I already PoCed with my [preload work](#1646) which I found elegant in terms of code architecture. Basically the `PlaybackObserver` (the class doing the polling) can now be "headless" initially: without a media element. In that case, default media metrics are considered (position `0`, paused etc.) When the media element is considered "ready", it can be attached to it, in which case "real" polling will be performed. This ensure that no polling of media metrics linked to a previous content is going on. It wasn't done with this issue in mind, but I found that it also made sense there: before stopping the previous content, we want to start our metrics-polling module (the `PlaybackObserver`) but are not yet ready to attach the media element as it is still technically playing the previous content. Once the previous content is stopped, we can now begin to actually link the media element to it to enable actual polling. Moreover, I now decide to emit those metrics right when we know what the initial position to seek to will be (as those metrics include this data point as a "wanted position). This makes sure that the RxPlayer core always has the more up-to-date information. Lastly, the already-merged #1755 fix should also have fixed that issue - as the initial position would have been known at some point by the RxPlayer Core anyway. This makes it far from a hotfix (there's a lot of lines) but I like this solution.
1 parent 444a3c2 commit b24f4eb

2 files changed

Lines changed: 180 additions & 74 deletions

File tree

src/main_thread/api/public_api.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1164,7 +1164,7 @@ class Player extends EventEmitter<IPublicAPIEvent> {
11641164
}
11651165

11661166
/** Global "playback observer" which will emit playback conditions */
1167-
const playbackObserver = new MediaElementPlaybackObserver(videoElement, {
1167+
const playbackObserver = new MediaElementPlaybackObserver({
11681168
withMediaSource: !isDirectFile,
11691169
lowLatencyMode,
11701170
});
@@ -1287,6 +1287,8 @@ class Player extends EventEmitter<IPublicAPIEvent> {
12871287
// content.
12881288
this.stop();
12891289

1290+
playbackObserver.attachMediaElement(videoElement);
1291+
12901292
// Update the RxPlayer's state at the right events
12911293
const playerStateRef = constructPlayerStateReference(
12921294
initializer,

0 commit comments

Comments
 (0)