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>
sailfishos-mirror · Nov 3, 2020 · d268c19 · d268c19
1 parent 660b5e4
commit d268c19
Show file tree

Hide file tree

Showing 6 changed files with 167 additions and 31 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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()