Commit 22ba6e9
committed
Fix occurences of
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 the 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, 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 fix
-------
The crux of this issue is just that we poll media metrics before
stopping the previous content, so there was a lot of ways this issue
could be easily fixed.
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 `MediaElementPlaybackObserver` (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.
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
polling logic 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.
This makes it far from a hotfix (there's a lot of lines) but I like this
solution.MEDIA_TIME_NOT_FOUND on initial fallback1 parent 6b397f1 commit 22ba6e9
2 files changed
Lines changed: 174 additions & 72 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1164 | 1164 | | |
1165 | 1165 | | |
1166 | 1166 | | |
1167 | | - | |
| 1167 | + | |
1168 | 1168 | | |
1169 | 1169 | | |
1170 | 1170 | | |
| |||
1287 | 1287 | | |
1288 | 1288 | | |
1289 | 1289 | | |
| 1290 | + | |
| 1291 | + | |
1290 | 1292 | | |
1291 | 1293 | | |
1292 | 1294 | | |
| |||
0 commit comments