From f7d4ea6eec40c89ca6f5b85f7c1837b3c580dd0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Thu, 19 Oct 2023 19:41:27 +0300 Subject: [PATCH] audioaggregator: Make access to the pad list thread-safe while mixing When mixing every single buffer the object lock is shortly released and acquired again. In the meantime the pad list can become invalid because a pad was removed or added, and equally the current pad might as well have been finalized in the meantime. To get around that, take a snapshot of all sinkpads before mixing and work with that list of pads. Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/3052 Part-of: --- .../gst-libs/gst/audio/gstaudioaggregator.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioaggregator.c b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioaggregator.c index 84c800d637..d967a3295d 100644 --- a/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioaggregator.c +++ b/subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioaggregator.c @@ -2224,6 +2224,8 @@ gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout) GstElement *element; GstAudioAggregator *aagg; GList *iter; + GstPad **sinkpads; + guint n_sinkpads, i; GstFlowReturn ret; GstBuffer *outbuf = NULL; gint64 next_offset; @@ -2471,9 +2473,17 @@ gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout) } GST_OBJECT_LOCK (agg); - for (iter = element->sinkpads; iter; iter = iter->next) { - GstAudioAggregatorPad *pad = (GstAudioAggregatorPad *) iter->data; - GstAggregatorPad *aggpad = (GstAggregatorPad *) iter->data; + + // mix_buffer() will shortly release the object lock so we need to + // ensure that the pad list stays valid. + n_sinkpads = element->numsinkpads; + sinkpads = g_newa (GstPad *, n_sinkpads + 1); + for (i = 0, iter = element->sinkpads; iter; i++, iter = iter->next) + sinkpads[i] = gst_object_ref (iter->data); + + for (i = 0; i < n_sinkpads; i++) { + GstAudioAggregatorPad *pad = (GstAudioAggregatorPad *) sinkpads[i]; + GstAggregatorPad *aggpad = (GstAggregatorPad *) sinkpads[i]; if (gst_aggregator_pad_is_inactive (aggpad)) continue; @@ -2505,6 +2515,9 @@ gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout) } GST_OBJECT_UNLOCK (agg); + for (i = 0; i < n_sinkpads; i++) + gst_object_unref (sinkpads[i]); + if (dropped) { /* We dropped a buffer, retry */ GST_LOG_OBJECT (aagg, "A pad dropped a buffer, wait for the next one");