audioaggregator: Make access to the pad list thread-safe while mixing
authorSebastian Dröge <sebastian@centricular.com>
Thu, 19 Oct 2023 16:41:27 +0000 (19:41 +0300)
committerTim-Philipp Müller <tim@centricular.com>
Wed, 25 Oct 2023 13:58:06 +0000 (14:58 +0100)
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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5553>

subprojects/gst-plugins-base/gst-libs/gst/audio/gstaudioaggregator.c

index 84c800d..d967a32 100644 (file)
@@ -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");