From e6490725848af838d95d8a9f5814240818a56a18 Mon Sep 17 00:00:00 2001 From: Gustav Fahlen Date: Thu, 10 Apr 2025 11:31:28 +0200 Subject: [PATCH] gstrtspsrc: Do not emit signal 'no-more-pads' too early Due to race condition it was previously possible that gst_element_add_pad was not completed for each RTSP stream before signal 'no-more-pads' was emitted. Race condition explained: Lets say two RTSP streams are created: Video and Audio. 1. Callback new_manager_pad is called for the Video stream => stream->added=TRUE. all_added=FALSE because both streams are not yet added. Call gst_element_add_pad and emit signal 'pad-added' for Video stream. 2. Callback new_manager_pad is called for Audio stream => stream->added=TRUE. all_added=TRUE because both streams are added. Call gst_element_add_pad and emit signal 'pad-added' for Audio stream. 3. Lets say gst_element_add_pad for the audio stream completes before the video stream. Since the audio stream already has all_added==TRUE this will result in the signal 'no-more-pads' to be emitted before gst_element_add_pad for the video stream is completed. Solution is to move the logic that sets added=True and checks if all streams are added to after gst_element_add_pad. This will make sure signal 'no-more-pads' is not emitted until all code in gst_element_add_pad is completed for all streams. Part-of: --- .../gst-plugins-good/gst/rtsp/gstrtspsrc.c | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c b/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c index 87a7fda6ee..7fd3df5251 100644 --- a/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c +++ b/subprojects/gst-plugins-good/gst/rtsp/gstrtspsrc.c @@ -3910,25 +3910,6 @@ new_manager_pad (GstElement * manager, GstPad * pad, GstRTSPSrc * src) /* save SSRC */ stream->ssrc = ssrc; - - /* we'll add it later see below */ - stream->added = TRUE; - - /* check if we added all streams */ - all_added = TRUE; - for (ostreams = src->streams; ostreams; ostreams = g_list_next (ostreams)) { - GstRTSPStream *ostream = (GstRTSPStream *) ostreams->data; - - GST_DEBUG_OBJECT (src, "stream %p, container %d, added %d, setup %d", - ostream, ostream->container, ostream->added, ostream->setup); - - /* if we find a stream for which we did a setup that is not added, we - * need to wait some more */ - if (ostream->setup && !ostream->added) { - all_added = FALSE; - break; - } - } GST_RTSP_STATE_UNLOCK (src); /* create a new pad we will use to stream to */ @@ -3967,6 +3948,29 @@ new_manager_pad (GstElement * manager, GstPad * pad, GstRTSPSrc * src) gst_element_add_pad (GST_ELEMENT_CAST (src), stream->srcpad); } + GST_RTSP_STATE_LOCK (src); + /* Setting added after gst_element_add_pad will make + sure callbacks for signal 'pad-added' will always + finish before signal 'no-more-pads' is executed. */ + stream->added = TRUE; + + /* check if we added all streams */ + all_added = TRUE; + for (ostreams = src->streams; ostreams; ostreams = g_list_next (ostreams)) { + GstRTSPStream *ostream = (GstRTSPStream *) ostreams->data; + + GST_DEBUG_OBJECT (src, "stream %p, container %d, added %d, setup %d", + ostream, ostream->container, ostream->added, ostream->setup); + + /* if we find a stream for which we did a setup that is not added, we + * need to wait some more */ + if (ostream->setup && !ostream->added) { + all_added = FALSE; + break; + } + } + GST_RTSP_STATE_UNLOCK (src); + if (all_added) { GST_DEBUG_OBJECT (src, "We added all streams"); /* when we get here, all stream are added and we can fire the no-more-pads