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/909>

diff --git a/docs/plugins/gst_plugins_cache.json b/docs/plugins/gst_plugins_cache.json
index 387700a..b083b2c 100644 (file)
--- 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 023543a..7c59afb 100644 (file)
--- 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 *
@@ -2576,6 +2644,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);
 }
@@ -2669,12 +2738,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 2b5c9a2..3687507 100644 (file)
--- 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 940059b..36acd80 100644 (file)
--- 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 (file)
index 0000000..3434ee5
--- /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 c336df4..776073c 100644 (file)
--- 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()