From a3f8f036fee404fc5c7349a47962f23c9d7c593e Mon Sep 17 00:00:00 2001 From: Johan Sternerup Date: Tue, 16 Apr 2024 14:30:24 +0200 Subject: [PATCH] gstbasesrc: Do not hold LIVE_LOCK while sending events An application that triggers a state transition from PLAYING to PAUSED needs to acquire the LIVE_LOCK. Consequently the LIVE_LOCK must not be taken while pushing anything on the pads because this operation might get blocked by something that cannot be unblocked without the application being able to proceed with the state transitions for other elements in the pipeline. This commit extends the previous behaviour where the live lock was released before pushing buffers (indirectly through the unlock before subclass->create) to now also include unlocking before pushing events. The issue was discovered in a case for WebRTC where the application tried to shut down a pipeline but an event originating from a video source element (based on basesrc) was in the process of being pushed down the pipeline when it got stuck on the STREAM_LOCK for the pad after the rtpgccbwe element. This lock in turn was held by the rtcpgccbwe element as it was in the process of pushing data down the pipeline but was stuck on the blocking probes installed on dtlssrtpenc to prevent data from flowing before dtls keys had been negotiated. What should have happened here is that the blocking probes should be removed, but that can only happen if the application may continue driving the state transitions. Part-of: --- subprojects/gstreamer/libs/gst/base/gstbasesrc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/subprojects/gstreamer/libs/gst/base/gstbasesrc.c b/subprojects/gstreamer/libs/gst/base/gstbasesrc.c index a840b4ac24..6f033ada83 100644 --- a/subprojects/gstreamer/libs/gst/base/gstbasesrc.c +++ b/subprojects/gstreamer/libs/gst/base/gstbasesrc.c @@ -2885,6 +2885,7 @@ gst_base_src_loop (GstPad * pad) gboolean eos; guint blocksize; GList *pending_events = NULL, *tmp; + GstEvent *seg_event = NULL; eos = FALSE; @@ -2973,14 +2974,15 @@ gst_base_src_loop (GstPad * pad) /* push events to close/start our segment before we push the buffer. */ if (G_UNLIKELY (src->priv->segment_pending)) { - GstEvent *seg_event = gst_event_new_segment (&src->segment); + /* generate the event but do not send until outside of live_lock */ + seg_event = gst_event_new_segment (&src->segment); gst_event_set_seqnum (seg_event, src->priv->segment_seqnum); src->priv->segment_seqnum = gst_util_seqnum_next (); - gst_pad_push_event (pad, seg_event); src->priv->segment_pending = FALSE; } + /* collect any pending events */ if (g_atomic_int_get (&src->priv->have_events)) { GST_OBJECT_LOCK (src); /* take the events */ @@ -2989,8 +2991,13 @@ gst_base_src_loop (GstPad * pad) g_atomic_int_set (&src->priv->have_events, FALSE); GST_OBJECT_UNLOCK (src); } + GST_LIVE_UNLOCK (src); - /* Push out pending events if any */ + /* now outside the live_lock we can push the segment event */ + if (G_UNLIKELY (seg_event)) + gst_pad_push_event (pad, seg_event); + + /* and the pending events if any */ if (G_UNLIKELY (pending_events != NULL)) { for (tmp = pending_events; tmp; tmp = g_list_next (tmp)) { GstEvent *ev = (GstEvent *) tmp->data; @@ -3070,7 +3077,6 @@ gst_base_src_loop (GstPad * pad) GST_BUFFER_FLAG_SET (buf, GST_BUFFER_FLAG_DISCONT); src->priv->discont = FALSE; } - GST_LIVE_UNLOCK (src); /* push buffer or buffer list */ if (src->priv->pending_bufferlist != NULL) {