Bug 245135

Summary: Holepunch missing after video source change
Product: WebKit Reporter: Olivier Blin <olivier.blin>
Component: WPE WebKitAssignee: Miguel Gomez <magomez>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, bugs-noreply, cgarcia, Hironori.Fujii, loic.yhuel, magomez, zdobersek
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Test case to reproduce the missing holepunch after changing video source
none
tentative fix none

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.