Skip to content

Conversation

@Florent-Bouisset
Copy link
Collaborator

We were reported a bug on Microsoft Edge where the player tried to play a content with HEVC codec but the MSE API failed to addSourceBuffer in a WebWorker.
The browser was supposed to handle HEVC because the API MediaSource.isTypeSupported returned true.
But calling this same API from a WebWorker returned false.

It is not clear if it is a Microsoft Edge limitation or a browser bug.
The fix is to test the codec support in the context of the WebWorker (only if it has MSE in worker).

@github-actions
Copy link

Automated performance checks have been performed on commit fd6defdaa08bc59a89b984b4b21030db516fccef with the base branch dev.

Tests results

✅ Tests have passed.

Performance tests 1st run output

No significative change in performance for tests:

Name Mean Median
loading 19.19ms -> 19.51ms (-0.321ms, z: 0.58364) 26.25ms -> 26.25ms
seeking 21.95ms -> 16.67ms (5.284ms, z: 0.08623) 11.25ms -> 11.25ms
audio-track-reload 26.82ms -> 26.96ms (-0.138ms, z: 1.02007) 39.00ms -> 39.00ms
cold loading multithread 48.01ms -> 48.13ms (-0.117ms, z: 9.54963) 70.20ms -> 69.15ms
seeking multithread 23.32ms -> 22.66ms (0.653ms, z: 0.51842) 10.50ms -> 10.50ms
audio-track-reload multithread 26.59ms -> 26.71ms (-0.126ms, z: 1.51808) 39.00ms -> 39.15ms
hot loading multithread 16.19ms -> 16.37ms (-0.177ms, z: 0.04757) 23.55ms -> 23.55ms

If you want to skip performance checks for latter commits, add the skip-performance-checks label to this Pull Request.

@github-actions
Copy link

✅ Automated performance checks have passed on commit f542751fc89624b4652f159a5915691c40b01419 with the base branch dev.

Details

Performance tests 1st run output

No significative change in performance for tests:

Name Mean Median
loading 19.98ms -> 20.62ms (-0.643ms, z: 2.36400) 27.45ms -> 27.60ms
seeking 16.73ms -> 16.09ms (0.639ms, z: 1.60334) 11.55ms -> 11.55ms
audio-track-reload 27.63ms -> 27.72ms (-0.091ms, z: 1.66835) 40.05ms -> 40.20ms
cold loading multithread 49.40ms -> 48.96ms (0.440ms, z: 8.29995) 72.30ms -> 71.40ms
seeking multithread 25.42ms -> 25.45ms (-0.029ms, z: 1.10132) 10.50ms -> 10.65ms
audio-track-reload multithread 27.34ms -> 27.31ms (0.034ms, z: 1.09333) 40.20ms -> 40.05ms
hot loading multithread 17.21ms -> 17.28ms (-0.068ms, z: 0.43295) 25.05ms -> 25.05ms

@peaBerberian
Copy link
Collaborator

There's an issue seen by the CI

@Florent-Bouisset
Copy link
Collaborator Author

There's an issue seen by the CI

Yes my IDE didn't highlight it, I have fixed it

@github-actions
Copy link

✅ Automated performance checks have passed on commit 30c0c5b936894e8f6942272c6269925c12e70cc4 with the base branch dev.

Details

Performance tests 1st run output

No significative change in performance for tests:

Name Mean Median
loading 18.94ms -> 19.07ms (-0.130ms, z: 0.81289) 25.80ms -> 25.80ms
seeking 20.51ms -> 19.20ms (1.319ms, z: 0.49916) 11.10ms -> 11.10ms
audio-track-reload 26.38ms -> 26.40ms (-0.022ms, z: 0.59580) 38.40ms -> 38.40ms
cold loading multithread 47.43ms -> 46.93ms (0.498ms, z: 9.83808) 69.30ms -> 68.40ms
seeking multithread 30.59ms -> 19.85ms (10.745ms, z: 0.45350) 10.35ms -> 10.35ms
audio-track-reload multithread 26.22ms -> 26.18ms (0.035ms, z: 0.59477) 38.40ms -> 38.40ms
hot loading multithread 15.84ms -> 15.71ms (0.127ms, z: 2.79332) 22.95ms -> 22.80ms

@peaBerberian
Copy link
Collaborator

It's weird. The same test fails over and over (well at least twice in a row) only on linux, and linked to the VideoThumbnailLoader on this PR.
Maybe we're just unlucky?

@Florent-Bouisset
Copy link
Collaborator Author

It's weird. The same test fails over and over (well at least twice in a row) only on linux, and linked to the VideoThumbnailLoader on this PR. Maybe we're just unlucky?

Which one? I don't any test failing on the CI, it fails only on your local?

@peaBerberian
Copy link
Collaborator

It was a temporary issue it seems, it's OK now

@peaBerberian peaBerberian merged commit 6224ef3 into dev Mar 26, 2025
15 of 16 checks passed
@peaBerberian
Copy link
Collaborator

I just noticed the iconic PR number btw :p

peaBerberian added a commit that referenced this pull request Mar 26, 2025
While exchanging in that PR with @Florent-Bouisset, we noticed that the
choice of whether we rely on mse-in-worker was done by the Public API
part of the code, which then directly told our WebWorker to use that
feature without telling any other part of the code.

However, that PR brought the need to have that information inside the
`ContentInitializer` - so that it knows whether a given codec is
supported (for cases, only seen on Edge for now, where codec support is
different depending on if MSE is relied on inside a worker or on main
thread).

Even without this, I thought that our `ContentInitializer`, as a global
controlling module, should probably architecturally be in the know of
whether MSE API are called in-worker or in-main-thread.

So I propose this modification, where the information on whether
mse-in-worker should be used is both communicated to the
`ContentInitializer`, but is also set per-content (we don't need that,
but it's actually both simpler and more powerful that way so why not).

This also opens the way for an API to control this, but I did not add
one for now.
peaBerberian added a commit that referenced this pull request Mar 26, 2025
…-worker

Built on #1664 (the PR, not the other thing).

While exchanging in that PR with @Florent-Bouisset, we noticed that the
choice of whether we rely on mse-in-worker was done by the Public API
part of the code, which then directly told our WebWorker to use that
feature without telling any other part of the code.

However, that PR brought the need to have that information inside the
`ContentInitializer` - so that it knows whether a given codec is
supported (for cases, only seen on Edge for now, where codec support is
different depending on if MSE is relied on inside a worker or on main
thread).

Even without this, I thought that our `ContentInitializer`, as a global
controlling module, should probably architecturally be in the know of
whether MSE API are called in-worker or in-main-thread.

So I propose this modification, where the information on whether
mse-in-worker should be used is both communicated to the
`ContentInitializer`, but is also set per-content (we don't need that,
but it's actually both simpler and more powerful that way so why not).

This also opens the way for an API to control this, but I did not add
one for now.
@peaBerberian peaBerberian added this to the 4.3.0 milestone Mar 27, 2025
@peaBerberian peaBerberian mentioned this pull request Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants