Skip to content

Commit ea79572

Browse files
committed
[MSE][GStreamer] Deadlock while flushing on paused when there's a non in-place transform element
https://bugs.webkit.org/show_bug.cgi?id=260067 Reviewed by Alicia Boya Garcia. There is a deadlock possible inside WebKit media src (MSE) between streaming thread handling CAPS event and pipeline flush in the main thread. This happens in case where there is a transform element in the pipeline that does transition NOT in place. Basetransform elem expects that it will allocate buffers so on CAPS change it triggers allocation negotiations (ALLOCATION query). In such case CAPS event becomes fully synchronous as basetransform does ALLOCATION query that is synchronous (serialized with data) and may block the streaming thread. If the pipeline is paused and the sink thread doesn't accept any data, this will block CAPS event until pipeline is unpaused or flushed. But flush requires a lock that streaming thread is holding (DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };) See: #1135 A way to fix this is to make webKitMediaSrcLoop() release the lock before pushing the caps event (which may take a long time to get processed) to let the main thread start the flush. Such a flush would cause the sink element to release the streaming thread and the caps event processing to finish. After the caps event has been pushed, the lock would be retaken. But streamingMembers might have changed under our feet (and it certainly will, because of the flush). We should reevaluate if the flush condition is present, and in that case abort the execution of webKitMediaSrcLoop() after having paused the streaming task of the corresponding pad. * Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp: (webKitMediaSrcLoop): Run the caps pushing code with the lock released and reevaluate the flush condition after the lock is reacquired. Canonical link: https://commits.webkit.org/267276@main
1 parent 8a606e4 commit ea79572

1 file changed

Lines changed: 15 additions & 4 deletions

File tree

Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,10 @@ static void webKitMediaSrcLoop(void* userData)
431431
// By keeping the lock we are guaranteed that a flush will not happen while we send essential events.
432432
// These events should never block downstream, so the lock should be released in little time in every
433433
// case.
434+
// There's one exception to this rule: a basetransform with not-in-place transformations (its sink thread
435+
// is decoupled from its src thread) may have to handle a CAPS event, which may trigger renegotiation and
436+
// an allocation query, which may be blocked because the pipeline sink is paused.
437+
// FIXME: re-evaluate releasing the lock before pushing other events too, especially once early flush race conditions are fixed in GStreamer.
434438

435439
if (!streamingMembers->hasPushedStreamCollectionEvent) {
436440
GST_DEBUG_OBJECT(pad, "Pushing STREAM_COLLECTION event.");
@@ -517,11 +521,18 @@ static void webKitMediaSrcLoop(void* userData)
517521

518522
if (!gst_caps_is_equal(gst_sample_get_caps(sample.get()), streamingMembers->previousCaps.get())) {
519523
// This sample needs new caps (typically because of a quality change).
520-
GST_DEBUG_OBJECT(pad, "Pushing new CAPS event: %" GST_PTR_FORMAT, gst_sample_get_caps(sample.get()));
521-
bool result = gst_pad_push_event(stream->pad.get(), gst_event_new_caps(gst_sample_get_caps(sample.get())));
522-
GST_DEBUG_OBJECT(pad, "CAPS event pushed, result = %s.", boolForPrinting(result));
523-
ASSERT(result);
524524
streamingMembers->previousCaps = gst_sample_get_caps(sample.get());
525+
// This CAPS event may block, so we release the lock and reevaluate later if there's been a flush in the meantime.
526+
streamingMembers.runUnlocked([&stream, &sample, &pad]() {
527+
GST_DEBUG_OBJECT(pad, "Pushing new CAPS event: %" GST_PTR_FORMAT, gst_sample_get_caps(sample.get()));
528+
bool result = gst_pad_push_event(stream->pad.get(), gst_event_new_caps(gst_sample_get_caps(sample.get())));
529+
GST_DEBUG_OBJECT(pad, "CAPS event pushed, result = %s.", boolForPrinting(result));
530+
ASSERT(result);
531+
});
532+
if (streamingMembers->isFlushing) {
533+
gst_pad_pause_task(pad);
534+
return;
535+
}
525536
}
526537

527538
GRefPtr<GstBuffer> buffer = gst_sample_get_buffer(sample.get());

0 commit comments

Comments
 (0)