Bug 245135 - Holepunch missing after video source change
Summary: Holepunch missing after video source change
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Miguel Gomez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-09-13 05:46 PDT by Olivier Blin
Modified: 2022-09-16 01:30 PDT (History)
7 users (show)

See Also:


Attachments
Test case to reproduce the missing holepunch after changing video source (936 bytes, text/html)
2022-09-13 05:46 PDT, Olivier Blin
no flags Details
tentative fix (1.47 KB, patch)
2022-09-15 01:59 PDT, Miguel Gomez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Blin 2022-09-13 05:46:23 PDT
Created attachment 462314 [details]
Test case to reproduce the missing holepunch after changing video source

Since r295749 (251754@main), the holepunch feature is broken (missing) after changing the video source.
See https://bugs.webkit.org/show_bug.cgi?id=240283 for the crash fix which causes this regression.

This bug can be seen with YouTube TV, just after skipping an ad.

The holepunch contents layer is reset to NULL after changing the source, which causes the holepunch buffer not to be rendered anymore.

The new content layer from the new MediaPlayer is set from TextureMapperPlatformLayerProxyGL::activateOnCompositingThread(), but after that, TextureMapperPlatformLayerProxyGL::invalidate() is called to invalidate the old content layer from the old MediaPlayer.
This sets the content layer to NULL in the target layer, and this disables the new content layer.

This can be reproduced on a WPE build with USE_GSTREAMER_HOLEPUNCH=ON and the attached test page.
Comment 1 Miguel Gomez 2022-09-15 01:59:47 PDT
Created attachment 462353 [details]
tentative fix

The issue is quite complicated to reproduce in my system. I was able to see it only once in tens of executions. This is because it has to happen that the old player is destroyed and the new one created before there's a layerFlush, so the CoordinatedGraphicsScene can be in the situation where a layer's proxy is replaced with a new one. Most of the times there's a layerFlush between the destruction of the old player and the creation of the new one, so the problem doesn't reproduce.

Anyway, I think the fix is as simple as doing the invalidation of the old proxies before activating the new ones.

Olivier, as you seem to be able to reproduce the issue reliably, could you give a test to this patch and check whether it fixes the problem?
Comment 2 Olivier Blin 2022-09-15 04:29:27 PDT
Miguel: thank you, this fixes the issue for me.

I could reproduce the issue on my laptop, but it has been first detected on set-top boxes with less powerful CPUs.
Comment 3 Miguel Gomez 2022-09-15 06:22:26 PDT
Pull request: https://github.com/WebKit/WebKit/pull/4379
Comment 4 EWS 2022-09-16 01:30:42 PDT
Committed 254548@main (f994a1b3e3dc): <https://commits.webkit.org/254548@main>

Reviewed commits have been landed. Closing PR #4379 and removing active labels.