videoaggregator: Fix locking around vagg->info

Take `GST_OBJECT_LOCK` when writing `vagg->info`, so that reading in
subclasses is protected against races, as documented in the struct.

    /*< public >*/
    /* read-only, with OBJECT_LOCK */
    GstVideoInfo                  info;

`gst_video_aggregator_default_negotiated_src_caps` should take the
`GST_VIDEO_AGGREGATOR_LOCK` to avoid racing with
`gst_video_aggregator_reset` called by
`gst_video_aggregator_release_pad` of the last sinkpad. Otherwise it can
happen that `latency = gst_util_uint64_scale (...` gets called with a
zero framerate.

There doesn't seem to be any reason not to use the local `info` instead
of `vagg->info`, so do that.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/915>

diff --git a/gst-libs/gst/video/gstvideoaggregator.c b/gst-libs/gst/video/gstvideoaggregator.c
index 7c59afb..4e186e0 100644 (file)
--- a/gst-libs/gst/video/gstvideoaggregator.c
+++ b/gst-libs/gst/video/gstvideoaggregator.c
@@ -1091,12 +1091,15 @@ gst_video_aggregator_default_negotiated_src_caps (GstAggregator * agg,
 {
   GstVideoAggregator *vagg = GST_VIDEO_AGGREGATOR (agg);
   gboolean at_least_one_alpha = FALSE;
+  gboolean ret = FALSE;
   const GstVideoFormatInfo *finfo;
   GstVideoInfo info;
   GList *l;
 
   GST_INFO_OBJECT (agg->srcpad, "set src caps: %" GST_PTR_FORMAT, caps);
 
+  GST_VIDEO_AGGREGATOR_LOCK (vagg);
+
   GST_OBJECT_LOCK (vagg);
   for (l = GST_ELEMENT (vagg)->sinkpads; l; l = l->next) {
     GstVideoAggregatorPad *mpad = l->data;
@@ -1111,7 +1114,7 @@ gst_video_aggregator_default_negotiated_src_caps (GstAggregator * agg,
   GST_OBJECT_UNLOCK (vagg);
 
   if (!gst_video_info_from_caps (&info, caps))
-    return FALSE;
+    goto unlock_and_return;
 
   if (GST_VIDEO_INFO_FPS_N (&vagg->info) != GST_VIDEO_INFO_FPS_N (&info) ||
       GST_VIDEO_INFO_FPS_D (&vagg->info) != GST_VIDEO_INFO_FPS_D (&info)) {
@@ -1125,15 +1128,17 @@ gst_video_aggregator_default_negotiated_src_caps (GstAggregator * agg,
     gst_video_aggregator_reset_qos (vagg);
   }
 
+  GST_OBJECT_LOCK (vagg);
   vagg->info = info;
+  GST_OBJECT_UNLOCK (vagg);
 
-  finfo = vagg->info.finfo;
+  finfo = info.finfo;
 
   if (at_least_one_alpha && !(finfo->flags & GST_VIDEO_FORMAT_FLAG_ALPHA)) {
     GST_ELEMENT_ERROR (vagg, CORE, NEGOTIATION,
         ("At least one of the input pads contains alpha, but configured caps don't support alpha."),
         ("Either convert your inputs to not contain alpha or add a videoconvert after the aggregator"));
-    return FALSE;
+    goto unlock_and_return;
   }
 
   /* Then browse the sinks once more, setting or unsetting conversion if needed */
@@ -1148,11 +1153,15 @@ gst_video_aggregator_default_negotiated_src_caps (GstAggregator * agg,
 
     gst_aggregator_set_src_caps (agg, caps);
     latency = gst_util_uint64_scale (GST_SECOND,
-        GST_VIDEO_INFO_FPS_D (&vagg->info), GST_VIDEO_INFO_FPS_N (&vagg->info));
+        GST_VIDEO_INFO_FPS_D (&info), GST_VIDEO_INFO_FPS_N (&info));
     gst_aggregator_set_latency (agg, latency, latency);
   }
 
-  return TRUE;
+  ret = TRUE;
+
+unlock_and_return:
+  GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
+  return ret;
 }
 
 static gboolean
@@ -1484,7 +1493,10 @@ gst_video_aggregator_reset (GstVideoAggregator * vagg)
   GstAggregator *agg = GST_AGGREGATOR (vagg);
   GList *l;
 
+  GST_OBJECT_LOCK (vagg);
   gst_video_info_init (&vagg->info);
+  GST_OBJECT_UNLOCK (vagg);
+
   vagg->priv->ts_offset = 0;
   vagg->priv->nframes = 0;
   vagg->priv->live = FALSE;