WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
211 stars 135 forks source link

[wpe-2.38] Deadlock in WebKitMediaSrc when sending initial CAPS event #1252

Closed Scony closed 6 months ago

Scony commented 8 months ago

As in title. The issue is virtually the same as https://github.com/WebPlatformForEmbedded/WPEWebKit/issues/1135 but the deadlock happens on sending initial CAPS event here: https://github.com/WebPlatformForEmbedded/WPEWebKit/blob/wpe-2.38/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp#L479

I've observed this issue on arm32 platform while running test case 34 from EME Conformance Tests from https://ytlr-cert.appspot.com/2021/main.html#1701787277015

eocanha commented 6 months ago

After trying to reproduce this issue during several days, the only I managed to get has been timeouts in the test (sometimes) and successes (sometimes). Lately only timeouts, though. In any case a deadlock as such. Then main thread was running free.

The timeouts I got are caused by the decryptor not actually decrypting buffers. This happens because it's not finding the protection GstMeta attached to them. In turn, this happens because qtdemux is detecting that "cbcs stream is not encrypted yet, not adding protection metadata" (from this code).

Also, note that, according to @calvaris, CBCS would only work on Nexus systems or similar. Specifically, the software decryptor on the Raspberry Pi doesn't support it.

At this point I'd need to now some more details about the deadlock that you found, because under the current circumstances of not being able to reproduce it, I can't work on it.

Scony commented 6 months ago

The issue is hard to reproduce - even on Comcast platforms. The deadlock started appearing at some point when timings changed after minor WPE WebKit upgrade. I'm not sure it still happens as I haven't been checking this for a while. Also, it's technically possible that this issue originates from the fact that playbin3 pipeline cannot be fully set up and ends up in bizarre state.

Regarding test case, the GstProtectionMeta is not present as the asset used by the test case is probably broken - we have ongoing discussion with YouTube team regarding that.

Anyway, since the outcome of discussion with YT may impact this issue (e.g. if they decide to fix the asset) I propose to hold any work on this issue. I'm closing it for now and will get back to it in case it's necessary.

P.S. I'm pasting my work-in-progress fix in case someone encounters this issue as well and would like to quickly proceed locally:

diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp
index df182ae06550..8d61c0ce713f 100644
--- a/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp
+++ b/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp
@@ -711,8 +711,10 @@ static void webKitMediaSrcLoop(void* userData)
         GRefPtr<GstEvent> event = adoptGRef(gst_event_new_caps(streamingMembers->pendingInitialCaps.get()));

         GST_DEBUG_OBJECT(pad, "Pushing initial CAPS event: %" GST_PTR_FORMAT, streamingMembers->pendingInitialCaps.get());
-        bool wasCapsEventSent = gst_pad_push_event(pad, event.leakRef());
-        GST_DEBUG_OBJECT(pad, "Pushed initial CAPS event, %s was returned.", boolForPrinting(wasCapsEventSent));
+        streamingMembers.runUnlocked([&event, &pad]() {
+            bool wasCapsEventSent = gst_pad_push_event(pad, event.leakRef());
+            GST_DEBUG_OBJECT(pad, "Pushed initial CAPS event, %s was returned.", boolForPrinting(wasCapsEventSent));
+        });

         streamingMembers->previousCaps = WTFMove(streamingMembers->pendingInitialCaps);
         ASSERT(!streamingMembers->pendingInitialCaps);