videoaggregator: Guarantee that the output format is supported

In the case `videoaggregator` is set as allowing format conversions,
and as we convert only on the sinkpads, we should ensure that the
chosen format is usable by the subclass. This in turns implies
that the format is usable on the srcpad.

When doing conversion *any* format can be used on the sinkpads, and this
is the only way that we can avoid race conditions during renegotiations
so we can not change that fact, we just need to ensure that the chosen
intermediary format is usable, which was not actually ensured before
that patch.

Fixes https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/issues/834

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

6 files changed, 167 insertions, 31 deletions

diff --git a/docs/plugins/gst_plugins_cache.json b/docs/plugins/gst_plugins_cache.json
index 4522b64fa..f14001092 100644
--- a/docs/plugins/gst_plugins_cache.json
+++ b/docs/plugins/gst_plugins_cache.json
@@ -1766,7 +1766,7 @@
                 "long-name": "Compositor",
                 "pad-templates": {
                     "sink_%%u": {
-                        "caps": "video/x-raw:\n         format: { AYUV, VUYA, BGRA, ARGB, RGBA, ABGR, Y444, Y42B, YUY2, UYVY, YVYU, I420, YV12, NV12, NV21, Y41B, RGB, BGR, xRGB, xBGR, RGBx, BGRx }\n          width: [ 1, 2147483647 ]\n         height: [ 1, 2147483647 ]\n      framerate: [ 0/1, 2147483647/1 ]\n",
+                        "caps": "video/x-raw:\n         format: { AYUV64, ARGB64, GBRA_12LE, GBRA_12BE, Y412_LE, Y412_BE, A444_10LE, GBRA_10LE, A444_10BE, GBRA_10BE, A422_10LE, A422_10BE, A420_10LE, A420_10BE, RGB10A2_LE, BGR10A2_LE, Y410, GBRA, ABGR, VUYA, BGRA, AYUV, ARGB, RGBA, A420, Y444_16LE, Y444_16BE, v216, P016_LE, P016_BE, Y444_12LE, GBR_12LE, Y444_12BE, GBR_12BE, I422_12LE, I422_12BE, Y212_LE, Y212_BE, I420_12LE, I420_12BE, P012_LE, P012_BE, Y444_10LE, GBR_10LE, Y444_10BE, GBR_10BE, r210, I422_10LE, I422_10BE, NV16_10LE32, Y210, v210, UYVP, I420_10LE, I420_10BE, P010_10LE, NV12_10LE32, NV12_10LE40, P010_10BE, Y444, GBR, NV24, xBGR, BGRx, xRGB, RGBx, BGR, IYU2, v308, RGB, Y42B, NV61, NV16, VYUY, UYVY, YVYU, YUY2, I420, YV12, NV21, NV12, NV12_64Z32, NV12_4L4, NV12_32L32, Y41B, IYU1, YVU9, YUV9, RGB16, BGR16, RGB15, BGR15, RGB8P, GRAY16_LE, GRAY16_BE, GRAY10_LE32, GRAY8 }\n          width: [ 1, 2147483647 ]\n         height: [ 1, 2147483647 ]\n      framerate: [ 0/1, 2147483647/1 ]\n",
                         "direction": "sink",
                         "presence": "request",
                         "type": "GstCompositorPad"
diff --git a/gst-libs/gst/video/gstvideoaggregator.c b/gst-libs/gst/video/gstvideoaggregator.c
index 564a4a4c8..84fa6a446 100644
--- a/gst-libs/gst/video/gstvideoaggregator.c
+++ b/gst-libs/gst/video/gstvideoaggregator.c
@@ -751,6 +751,10 @@ struct _GstVideoAggregatorPrivate
   GstCaps *current_caps;
 
   gboolean live;
+
+  /* The (ordered) list of #GstVideoFormatInfo supported by the aggregation
+     method (from the srcpad template caps). */
+  GPtrArray *supported_formats;
 };
 
 /* Can't use the G_DEFINE_TYPE macros because we need the
@@ -792,6 +796,35 @@ gst_video_aggregator_get_instance_private (GstVideoAggregator * self)
   return (G_STRUCT_MEMBER_P (self, video_aggregator_private_offset));
 }
 
+static gboolean
+gst_video_aggregator_supports_format (GstVideoAggregator * vagg,
+    GstVideoFormat format)
+{
+  gint i;
+
+  for (i = 0; i < vagg->priv->supported_formats->len; i++) {
+    GstVideoFormatInfo *format_info = vagg->priv->supported_formats->pdata[i];
+
+    if (GST_VIDEO_FORMAT_INFO_FORMAT (format_info) == format)
+      return TRUE;
+  }
+
+  return FALSE;
+}
+
+static GstCaps *
+gst_video_aggregator_get_possible_caps_for_info (GstVideoInfo * info)
+{
+  GstStructure *s;
+  GstCaps *possible_caps = gst_video_info_to_caps (info);
+
+  s = gst_caps_get_structure (possible_caps, 0);
+  gst_structure_remove_fields (s, "width", "height", "framerate",
+      "pixel-aspect-ratio", "interlace-mode", NULL);
+
+  return possible_caps;
+}
+
 static void
 gst_video_aggregator_find_best_format (GstVideoAggregator * vagg,
     GstCaps * downstream_caps, GstVideoInfo * best_info,
@@ -801,13 +834,12 @@ gst_video_aggregator_find_best_format (GstVideoAggregator * vagg,
   GstCaps *possible_caps;
   GstVideoAggregatorPad *pad;
   gboolean need_alpha = FALSE;
-  gint best_format_number = 0;
+  gint best_format_number = 0, i;
   GHashTable *formats_table = g_hash_table_new (g_direct_hash, g_direct_equal);
 
   GST_OBJECT_LOCK (vagg);
   for (tmp = GST_ELEMENT (vagg)->sinkpads; tmp; tmp = tmp->next) {
-    GstStructure *s;
-    gint format_number;
+    gint format_number = 0;
 
     pad = tmp->data;
 
@@ -825,28 +857,29 @@ gst_video_aggregator_find_best_format (GstVideoAggregator * vagg,
     if (GST_VIDEO_INFO_FORMAT (&pad->info) == GST_VIDEO_FORMAT_UNKNOWN)
       continue;
 
-    possible_caps = gst_video_info_to_caps (&pad->info);
-
-    s = gst_caps_get_structure (possible_caps, 0);
-    gst_structure_remove_fields (s, "width", "height", "framerate",
-        "pixel-aspect-ratio", "interlace-mode", NULL);
-
     /* Can downstream accept this format ? */
-    if (!gst_caps_can_intersect (downstream_caps, possible_caps)) {
-      gst_caps_unref (possible_caps);
-      continue;
+    if (!GST_IS_VIDEO_AGGREGATOR_CONVERT_PAD (pad)) {
+      possible_caps =
+          gst_video_aggregator_get_possible_caps_for_info (&pad->info);
+      if (!gst_caps_can_intersect (downstream_caps, possible_caps)) {
+        gst_caps_unref (possible_caps);
+        continue;
+      }
     }
 
-    gst_caps_unref (possible_caps);
+    /* If the format is supported, consider it very high weight */
+    if (gst_video_aggregator_supports_format (vagg,
+            GST_VIDEO_INFO_FORMAT (&pad->info))) {
+      format_number =
+          GPOINTER_TO_INT (g_hash_table_lookup (formats_table,
+              GINT_TO_POINTER (GST_VIDEO_INFO_FORMAT (&pad->info))));
 
-    format_number =
-        GPOINTER_TO_INT (g_hash_table_lookup (formats_table,
-            GINT_TO_POINTER (GST_VIDEO_INFO_FORMAT (&pad->info))));
-    format_number += pad->info.width * pad->info.height;
+      format_number += pad->info.width * pad->info.height;
 
-    g_hash_table_replace (formats_table,
-        GINT_TO_POINTER (GST_VIDEO_INFO_FORMAT (&pad->info)),
-        GINT_TO_POINTER (format_number));
+      g_hash_table_replace (formats_table,
+          GINT_TO_POINTER (GST_VIDEO_INFO_FORMAT (&pad->info)),
+          GINT_TO_POINTER (format_number));
+    }
 
     /* If that pad is the first with alpha, set it as the new best format */
     if (!need_alpha && (pad->priv->needs_alpha
@@ -872,6 +905,41 @@ gst_video_aggregator_find_best_format (GstVideoAggregator * vagg,
   GST_OBJECT_UNLOCK (vagg);
 
   g_hash_table_unref (formats_table);
+
+  if (gst_video_aggregator_supports_format (vagg,
+          GST_VIDEO_INFO_FORMAT (best_info))) {
+    possible_caps = gst_video_aggregator_get_possible_caps_for_info (best_info);
+    if (gst_caps_can_intersect (downstream_caps, possible_caps)) {
+      gst_caps_unref (possible_caps);
+      return;
+    }
+    gst_caps_unref (possible_caps);
+  }
+
+  for (i = 0; i < vagg->priv->supported_formats->len; i++) {
+    GstVideoFormatInfo *format_info = vagg->priv->supported_formats->pdata[i];
+
+    if ((! !GST_VIDEO_FORMAT_INFO_HAS_ALPHA (format_info)) == (! !need_alpha)) {
+      gst_video_info_set_format (best_info, format_info->format,
+          best_info->width, best_info->height);
+      possible_caps =
+          gst_video_aggregator_get_possible_caps_for_info (best_info);
+
+      if (gst_caps_can_intersect (downstream_caps, possible_caps)) {
+        GST_INFO_OBJECT (vagg, "Using supported caps: %" GST_PTR_FORMAT,
+            possible_caps);
+        gst_caps_unref (possible_caps);
+
+        return;
+      }
+
+      gst_caps_unref (possible_caps);
+    }
+  }
+
+  GST_WARNING_OBJECT (vagg, "Nothing compatible with %" GST_PTR_FORMAT,
+      downstream_caps);
+  gst_video_info_init (best_info);
 }
 
 static GstCaps *
@@ -2581,6 +2649,7 @@ gst_video_aggregator_finalize (GObject * o)
   GstVideoAggregator *vagg = GST_VIDEO_AGGREGATOR (o);
 
   g_mutex_clear (&vagg->priv->lock);
+  g_ptr_array_unref (vagg->priv->supported_formats);
 
   G_OBJECT_CLASS (gst_video_aggregator_parent_class)->finalize (o);
 }
@@ -2674,12 +2743,48 @@ static void
 gst_video_aggregator_init (GstVideoAggregator * vagg,
     GstVideoAggregatorClass * klass)
 {
-  vagg->priv = gst_video_aggregator_get_instance_private (vagg);
+  GstCaps *src_template;
+  GstPadTemplate *pad_template;
 
+  vagg->priv = gst_video_aggregator_get_instance_private (vagg);
   vagg->priv->current_caps = NULL;
 
   g_mutex_init (&vagg->priv->lock);
 
   /* initialize variables */
   gst_video_aggregator_reset (vagg);
+
+  /* Finding all supported formats */
+  vagg->priv->supported_formats = g_ptr_array_new ();
+  pad_template =
+      gst_element_class_get_pad_template (GST_ELEMENT_CLASS (klass), "src");
+  src_template = gst_pad_template_get_caps (pad_template);
+  for (gint i = 0; i < gst_caps_get_size (src_template); i++) {
+    const GValue *v =
+        gst_structure_get_value (gst_caps_get_structure (src_template, i),
+        "format");
+
+    if (G_VALUE_HOLDS_STRING (v)) {
+      GstVideoFormat f = gst_video_format_from_string (g_value_get_string (v));
+      GstVideoFormatInfo *format_info =
+          (GstVideoFormatInfo *) gst_video_format_get_info (f);
+      g_ptr_array_add (vagg->priv->supported_formats, format_info);
+      continue;
+    }
+
+    if (GST_VALUE_HOLDS_LIST (v)) {
+      gint j;
+
+      for (j = 0; j < gst_value_list_get_size (v); j++) {
+        const GValue *v1 = gst_value_list_get_value (v, j);
+        GstVideoFormat f =
+            gst_video_format_from_string (g_value_get_string (v1));
+        GstVideoFormatInfo *format_info =
+            (GstVideoFormatInfo *) gst_video_format_get_info (f);
+        g_ptr_array_add (vagg->priv->supported_formats, format_info);
+      }
+    }
+  }
+
+  gst_caps_unref (src_template);
 }
diff --git a/gst/compositor/compositor.c b/gst/compositor/compositor.c
index 665d8bc68..9c2e0ee46 100644
--- a/gst/compositor/compositor.c
+++ b/gst/compositor/compositor.c
@@ -116,7 +116,7 @@ static GstStaticPadTemplate src_factory = GST_STATIC_PAD_TEMPLATE ("src",
 static GstStaticPadTemplate sink_factory = GST_STATIC_PAD_TEMPLATE ("sink_%u",
     GST_PAD_SINK,
     GST_PAD_REQUEST,
-    GST_STATIC_CAPS (GST_VIDEO_CAPS_MAKE (FORMATS))
+    GST_STATIC_CAPS (GST_VIDEO_CAPS_MAKE (GST_VIDEO_FORMATS_ALL))
     );
 
 static void gst_compositor_child_proxy_init (gpointer g_iface,
diff --git a/tests/check/elements/compositor.c b/tests/check/elements/compositor.c
index 940059bbd..36acd80ce 100644
--- a/tests/check/elements/compositor.c
+++ b/tests/check/elements/compositor.c
@@ -47,19 +47,37 @@ static GMainLoop *main_loop;
 static GstCaps *
 _compositor_get_all_supported_caps (void)
 {
-  return gst_caps_from_string (GST_VIDEO_CAPS_MAKE
-      (" { AYUV, VUYA, BGRA, ARGB, RGBA, ABGR, Y444, Y42B, YUY2, UYVY, "
-          "   YVYU, I420, YV12, NV12, NV21, Y41B, RGB, BGR, xRGB, xBGR, "
-          "   RGBx, BGRx } "));
+  return gst_caps_from_string (GST_VIDEO_CAPS_MAKE (GST_VIDEO_FORMATS_ALL));
 }
 
 static GstCaps *
 _compositor_get_non_alpha_supported_caps (void)
 {
-  return gst_caps_from_string (GST_VIDEO_CAPS_MAKE
-      (" { Y444, Y42B, YUY2, UYVY, "
-          "   YVYU, I420, YV12, NV12, NV21, Y41B, RGB, BGR, xRGB, xBGR, "
-          "   RGBx, BGRx } "));
+  gint j;
+  GValue all_formats = G_VALUE_INIT;
+  GValue nonalpha_formats = G_VALUE_INIT;
+  GstCaps *all_caps = _compositor_get_all_supported_caps ();
+
+  g_value_init (&all_formats, GST_TYPE_LIST);
+  g_value_init (&nonalpha_formats, GST_TYPE_LIST);
+  gst_value_deserialize (&all_formats, GST_VIDEO_FORMATS_ALL);
+
+  for (j = 0; j < gst_value_list_get_size (&all_formats); j++) {
+    const GValue *v1 = gst_value_list_get_value (&all_formats, j);
+    GstVideoFormat f = gst_video_format_from_string (g_value_get_string (v1));
+    GstVideoFormatInfo *format_info =
+        (GstVideoFormatInfo *) gst_video_format_get_info (f);
+    if (!GST_VIDEO_FORMAT_INFO_HAS_ALPHA (format_info))
+      gst_value_list_append_value (&nonalpha_formats, v1);
+  }
+
+  gst_structure_set_value (gst_caps_get_structure (all_caps, 0), "format",
+      &nonalpha_formats);
+
+  g_value_unset (&all_formats);
+  g_value_unset (&nonalpha_formats);
+
+  return all_caps;
 }
 
 /* make sure downstream gets a CAPS event before buffers are sent */
diff --git a/tests/validate/compositor/renogotiate_failing_unsupported_src_format.validatetest b/tests/validate/compositor/renogotiate_failing_unsupported_src_format.validatetest
new file mode 100644
index 000000000..3434ee57a
--- /dev/null
+++ b/tests/validate/compositor/renogotiate_failing_unsupported_src_format.validatetest
@@ -0,0 +1,12 @@
+meta,
+    args = {
+        "videotestsrc num-buffers=20 ! capsfilter caps=\"video/x-raw,format=I420\" ! capssetter name=cs ! compositor name=c ! fakesink sync=true\
+         videotestsrc num-buffers=20 ! capsfilter caps=\"video/x-raw,format=I420\" ! c.",
+    },
+    handles-states=true
+
+play;
+crank-clock, repeat=10;
+set-properties, cs::caps="video/x-raw,format=ARGB64"
+crank-clock, repeat=11;
+play
diff --git a/tests/validate/meson.build b/tests/validate/meson.build
index c336df4f8..776073c07 100644
--- a/tests/validate/meson.build
+++ b/tests/validate/meson.build
@@ -17,6 +17,7 @@ tests = [
     'videorate/rate_0_5_with_decoder',
     'videorate/rate_2_0',
     'videorate/rate_2_0_with_decoder',
+    'compositor/renogotiate_failing_unsupported_src_format',
 ]
 
 env = environment()