videocrop: handle non raw caps features

Currently, videocrop, only negotiates raw caps (system memory) because
it's the type of memory it can modify. Nonetheless, it's also possible
for the element to handle non-raw caps when only adding the crop meta
is possible, in other words, when downstream buffer pools expose the
crop API.

This patch enable non-raw caps negotiation. If downstream doesn't
expose crop API and negotiated caps are featured, the negotiation
fails.

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

4 files changed, 151 insertions, 13 deletions

diff --git a/docs/gst_plugins_cache.json b/docs/gst_plugins_cache.json
index d757acb3e..46e17d53c 100644
--- a/docs/gst_plugins_cache.json
+++ b/docs/gst_plugins_cache.json
@@ -24781,12 +24781,12 @@
                 "long-name": "Crop",
                 "pad-templates": {
                     "sink": {
-                        "caps": "video/x-raw:\n         format: { RGBx, xRGB, BGRx, xBGR, RGBA, ARGB, BGRA, ABGR, RGB, BGR, AYUV, YUY2, Y444, Y42B, Y41B, YVYU, UYVY, I420, YV12, RGB16, RGB15, GRAY8, NV12, NV21, GRAY16_LE, GRAY16_BE }\n          width: [ 1, 2147483647 ]\n         height: [ 1, 2147483647 ]\n      framerate: [ 0/1, 2147483647/1 ]\n",
+                        "caps": "video/x-raw:\n         format: { RGBx, xRGB, BGRx, xBGR, RGBA, ARGB, BGRA, ABGR, RGB, BGR, AYUV, YUY2, Y444, Y42B, Y41B, YVYU, UYVY, I420, YV12, RGB16, RGB15, GRAY8, NV12, NV21, GRAY16_LE, GRAY16_BE }\n          width: [ 1, 2147483647 ]\n         height: [ 1, 2147483647 ]\n      framerate: [ 0/1, 2147483647/1 ]\n\nvideo/x-raw(ANY):\n          width: [ 1, 2147483647 ]\n         height: [ 1, 2147483647 ]\n      framerate: [ 0/1, 2147483647/1 ]\n",
                         "direction": "sink",
                         "presence": "always"
                     },
                     "src": {
-                        "caps": "video/x-raw:\n         format: { RGBx, xRGB, BGRx, xBGR, RGBA, ARGB, BGRA, ABGR, RGB, BGR, AYUV, YUY2, Y444, Y42B, Y41B, YVYU, UYVY, I420, YV12, RGB16, RGB15, GRAY8, NV12, NV21, GRAY16_LE, GRAY16_BE }\n          width: [ 1, 2147483647 ]\n         height: [ 1, 2147483647 ]\n      framerate: [ 0/1, 2147483647/1 ]\n",
+                        "caps": "video/x-raw:\n         format: { RGBx, xRGB, BGRx, xBGR, RGBA, ARGB, BGRA, ABGR, RGB, BGR, AYUV, YUY2, Y444, Y42B, Y41B, YVYU, UYVY, I420, YV12, RGB16, RGB15, GRAY8, NV12, NV21, GRAY16_LE, GRAY16_BE }\n          width: [ 1, 2147483647 ]\n         height: [ 1, 2147483647 ]\n      framerate: [ 0/1, 2147483647/1 ]\n\nvideo/x-raw(ANY):\n          width: [ 1, 2147483647 ]\n         height: [ 1, 2147483647 ]\n      framerate: [ 0/1, 2147483647/1 ]\n",
                         "direction": "src",
                         "presence": "always"
                     }
diff --git a/gst/videocrop/gstvideocrop.c b/gst/videocrop/gstvideocrop.c
index 9ad5df7d1..71f36ff4f 100644
--- a/gst/videocrop/gstvideocrop.c
+++ b/gst/videocrop/gstvideocrop.c
@@ -82,7 +82,11 @@ enum
   GST_VIDEO_CAPS_MAKE ("{ RGBx, xRGB, BGRx, xBGR, "    \
       "RGBA, ARGB, BGRA, ABGR, RGB, BGR, AYUV, YUY2, Y444, " \
       "Y42B, Y41B, YVYU, UYVY, I420, YV12, RGB16, RGB15, "  \
-      "GRAY8, NV12, NV21, GRAY16_LE, GRAY16_BE }")
+      "GRAY8, NV12, NV21, GRAY16_LE, GRAY16_BE }") "; "     \
+  "video/x-raw(ANY), "                                      \
+      "width = " GST_VIDEO_SIZE_RANGE ", "                  \
+      "height = " GST_VIDEO_SIZE_RANGE ", "                 \
+      "framerate = " GST_VIDEO_FPS_RANGE
 
 static GstStaticPadTemplate src_template = GST_STATIC_PAD_TEMPLATE ("src",
     GST_PAD_SRC,
@@ -479,10 +483,14 @@ gst_video_crop_decide_allocation (GstBaseTransform * trans, GstQuery * query)
     GST_INFO_OBJECT (crop, "we are doing in-place transform using crop meta");
     gst_base_transform_set_passthrough (GST_BASE_TRANSFORM (crop), FALSE);
     gst_base_transform_set_in_place (GST_BASE_TRANSFORM (crop), TRUE);
-  } else {
+  } else if (crop->raw_caps) {
     GST_INFO_OBJECT (crop, "we are not using passthrough");
     gst_base_transform_set_passthrough (GST_BASE_TRANSFORM (crop), FALSE);
     gst_base_transform_set_in_place (GST_BASE_TRANSFORM (crop), FALSE);
+  } else {
+    GST_ELEMENT_ERROR (crop, STREAM, WRONG_TYPE,
+        ("Dowstream doesn't support crop for non-raw caps"), (NULL));
+    return FALSE;
   }
 
   return GST_BASE_TRANSFORM_CLASS (parent_class)->decide_allocation (trans,
@@ -692,8 +700,10 @@ gst_video_crop_transform_caps (GstBaseTransform * trans,
     const GValue *v;
     GstStructure *structure, *new_structure;
     GValue w_val = G_VALUE_INIT, h_val = G_VALUE_INIT;
+    GstCapsFeatures *features;
 
     structure = gst_caps_get_structure (caps, i);
+    features = gst_caps_get_features (caps, i);
 
     v = gst_structure_get_value (structure, "width");
     if (!gst_video_crop_transform_dimension_value (v, dx, &w_val, direction,
@@ -717,9 +727,13 @@ gst_video_crop_transform_caps (GstBaseTransform * trans,
     gst_structure_set_value (new_structure, "height", &h_val);
     g_value_unset (&w_val);
     g_value_unset (&h_val);
+
     GST_LOG_OBJECT (vcrop, "transformed structure %2d: %" GST_PTR_FORMAT
-        " => %" GST_PTR_FORMAT, i, structure, new_structure);
+        " => %" GST_PTR_FORMAT "features %" GST_PTR_FORMAT, i, structure,
+        new_structure, features);
     gst_caps_append_structure (other_caps, new_structure);
+
+    gst_caps_set_features (other_caps, i, gst_caps_features_copy (features));
   }
 
   if (!gst_caps_is_empty (other_caps) && filter_caps) {
@@ -737,6 +751,7 @@ gst_video_crop_set_info (GstVideoFilter * vfilter, GstCaps * in,
     GstVideoInfo * in_info, GstCaps * out, GstVideoInfo * out_info)
 {
   GstVideoCrop *crop = GST_VIDEO_CROP (vfilter);
+  GstCapsFeatures *features;
   int dx, dy;
 
   GST_OBJECT_LOCK (crop);
@@ -786,6 +801,13 @@ gst_video_crop_set_info (GstVideoFilter * vfilter, GstCaps * in,
     GST_LOG_OBJECT (crop, "incaps = %" GST_PTR_FORMAT ", outcaps = %"
         GST_PTR_FORMAT, in, out);
 
+  features = gst_caps_get_features (in, 0);
+  crop->raw_caps = gst_caps_features_is_equal (features,
+      GST_CAPS_FEATURES_MEMORY_SYSTEM_MEMORY);
+
+  if (!crop->raw_caps)
+    goto beach;
+
   if (GST_VIDEO_INFO_IS_RGB (in_info)
       || GST_VIDEO_INFO_IS_GRAY (in_info)) {
     crop->packing = VIDEO_CROP_PIXEL_FORMAT_PACKED_SIMPLE;
@@ -822,6 +844,7 @@ gst_video_crop_set_info (GstVideoFilter * vfilter, GstCaps * in,
     }
   }
 
+beach:
   crop->in_info = *in_info;
   crop->out_info = *out_info;
 
diff --git a/gst/videocrop/gstvideocrop.h b/gst/videocrop/gstvideocrop.h
index 4082b9959..cf7588ee5 100644
--- a/gst/videocrop/gstvideocrop.h
+++ b/gst/videocrop/gstvideocrop.h
@@ -68,6 +68,8 @@ struct _GstVideoCrop
 
   VideoCropPixelFormat  packing;
   gint macro_y_off;
+
+  gboolean raw_caps;
 };
 
 struct _GstVideoCropClass
diff --git a/tests/check/elements/videocrop.c b/tests/check/elements/videocrop.c
index 114aca4e3..b7378384b 100644
--- a/tests/check/elements/videocrop.c
+++ b/tests/check/elements/videocrop.c
@@ -49,10 +49,15 @@ video_crop_get_test_caps (GstElement * videocrop)
   for (i = 0; i < gst_caps_get_size (allowed_caps); ++i) {
     GstStructure *new_structure;
     GstCaps *single_caps;
+    GstStructure *structure;
+
+    /* featured caps don't describe format: skip them */
+    structure = gst_caps_get_structure (allowed_caps, i);
+    if (!gst_structure_has_field (structure, "format"))
+      continue;
 
     single_caps = gst_caps_new_empty ();
-    new_structure =
-        gst_structure_copy (gst_caps_get_structure (allowed_caps, i));
+    new_structure = gst_structure_copy (structure);
     gst_structure_set (new_structure, "framerate", GST_TYPE_FRACTION,
         1, 1, NULL);
     gst_structure_remove_field (new_structure, "width");
@@ -177,13 +182,18 @@ handoff_cb (GstElement * sink, GstBuffer * buf, GstPad * pad,
 }
 
 static void
-videocrop_test_cropping_init_context (GstVideoCropTestContext * ctx)
+videocrop_test_cropping_init_context_full (GstVideoCropTestContext * ctx,
+    gboolean featured)
 {
   fail_unless (ctx != NULL);
 
   ctx->pipeline = gst_pipeline_new ("pipeline");
   fail_unless (ctx->pipeline != NULL);
-  ctx->src = gst_element_factory_make ("videotestsrc", "src");
+
+  if (featured)
+    ctx->src = gst_element_factory_make ("fakesrc", "src");
+  else
+    ctx->src = gst_element_factory_make ("videotestsrc", "src");
   fail_unless (ctx->src != NULL, "Failed to create videotestsrc element");
   ctx->filter = gst_element_factory_make ("capsfilter", "filter");
   fail_unless (ctx->filter != NULL, "Failed to create capsfilter element");
@@ -200,11 +210,18 @@ videocrop_test_cropping_init_context (GstVideoCropTestContext * ctx)
   gst_element_link_many (ctx->src, ctx->filter, ctx->crop, ctx->filter2,
       ctx->sink, NULL);
 
-  /* set pattern to 'red' - for our purposes it doesn't matter anyway */
-  g_object_set (ctx->src, "pattern", 4, NULL);
+  if (featured) {
+    g_object_set (ctx->src, "format", GST_FORMAT_TIME, NULL);
+  } else {
+    /* set pattern to 'red' - for our purposes it doesn't matter anyway */
+    g_object_set (ctx->src, "pattern", 4, NULL);
+  }
 
-  g_object_set (ctx->sink, "signal-handoffs", TRUE, NULL);
-  g_signal_connect (ctx->sink, "preroll-handoff", G_CALLBACK (handoff_cb), ctx);
+  if (!featured) {
+    g_object_set (ctx->sink, "signal-handoffs", TRUE, NULL);
+    g_signal_connect (ctx->sink, "preroll-handoff", G_CALLBACK (handoff_cb),
+        ctx);
+  }
 
   ctx->last_buf = NULL;
   ctx->last_caps = NULL;
@@ -213,6 +230,12 @@ videocrop_test_cropping_init_context (GstVideoCropTestContext * ctx)
 }
 
 static void
+videocrop_test_cropping_init_context (GstVideoCropTestContext * ctx)
+{
+  videocrop_test_cropping_init_context_full (ctx, FALSE);
+}
+
+static void
 videocrop_test_cropping_deinit_context (GstVideoCropTestContext * ctx)
 {
   GST_LOG ("deiniting context");
@@ -786,6 +809,94 @@ GST_START_TEST (test_caps_transform)
 
 GST_END_TEST;
 
+static GstCaps *
+get_featured_caps (void)
+{
+  GstCaps *caps;
+  GstCapsFeatures *features;
+
+  features = gst_caps_features_new ("memory:DMABuf", NULL);
+  caps = gst_caps_new_simple ("video/x-raw",
+      "format", G_TYPE_STRING, "NV12",
+      "framerate", GST_TYPE_FRACTION, 1, 1,
+      "width", G_TYPE_INT, 200, "height", G_TYPE_INT, 100, NULL);
+  gst_caps_set_features_simple (caps, features);
+
+  return caps;
+}
+
+GST_START_TEST (test_caps_transform_featured)
+{
+  GstVideoCropTestContext ctx;
+  GstBaseTransformClass *klass;
+  GstBaseTransform *crop;
+  GstCaps *caps, *adj_caps;
+
+  videocrop_test_cropping_init_context (&ctx);
+
+  caps = get_featured_caps ();
+
+  crop = GST_BASE_TRANSFORM (ctx.crop);
+  klass = GST_BASE_TRANSFORM_GET_CLASS (ctx.crop);
+  fail_unless (klass != NULL);
+
+  /* by default, it should be no cropping and hence passthrough */
+  adj_caps = klass->transform_caps (crop, GST_PAD_SRC, caps, NULL);
+  fail_unless (adj_caps != NULL);
+
+  fail_unless (gst_caps_is_equal (adj_caps, caps));
+  gst_caps_unref (adj_caps);
+
+  adj_caps = klass->transform_caps (crop, GST_PAD_SINK, caps, NULL);
+  fail_unless (adj_caps != NULL);
+  fail_unless (gst_caps_is_equal (adj_caps, caps));
+  gst_caps_unref (adj_caps);
+
+  /* make sure that's still true after changing properties back and forth */
+  g_object_set (ctx.crop, "left", 1, "right", 3, "top", 5, "bottom", 7, NULL);
+  g_object_set (ctx.crop, "left", 0, "right", 0, "top", 0, "bottom", 0, NULL);
+
+  adj_caps = klass->transform_caps (crop, GST_PAD_SRC, caps, NULL);
+  fail_unless (adj_caps != NULL);
+  fail_unless (gst_caps_is_equal (adj_caps, caps));
+  gst_caps_unref (adj_caps);
+
+  adj_caps = klass->transform_caps (crop, GST_PAD_SINK, caps, NULL);
+  fail_unless (adj_caps != NULL);
+  fail_unless (gst_caps_is_equal (adj_caps, caps));
+  gst_caps_unref (adj_caps);
+
+  gst_caps_unref (caps);
+  videocrop_test_cropping_deinit_context (&ctx);
+}
+
+GST_END_TEST;
+
+GST_START_TEST (test_passthrough_featured)
+{
+  GstStateChangeReturn state_ret;
+  GstVideoCropTestContext ctx;
+  GstCaps *caps;
+
+  videocrop_test_cropping_init_context_full (&ctx, TRUE);
+
+  g_object_set (ctx.src, "num-buffers", 1, NULL);
+
+  caps = get_featured_caps ();
+  g_object_set (ctx.filter, "caps", caps, NULL);
+  gst_caps_unref (caps);
+
+  g_object_set (ctx.crop, "left", 50, "top", 10, NULL);
+
+  state_ret = gst_element_set_state (ctx.pipeline, GST_STATE_PAUSED);
+  fail_unless (state_ret == GST_STATE_CHANGE_ASYNC,
+      "pipeline should fail on negotiation");
+
+  videocrop_test_cropping_deinit_context (&ctx);
+}
+
+GST_END_TEST;
+
 static Suite *
 videocrop_suite (void)
 {
@@ -807,7 +918,9 @@ videocrop_suite (void)
   suite_add_tcase (s, tc_chain);
   tcase_add_test (tc_chain, test_crop_to_1x1);
   tcase_add_test (tc_chain, test_caps_transform);
+  tcase_add_test (tc_chain, test_caps_transform_featured);
   tcase_add_test (tc_chain, test_passthrough);
+  tcase_add_test (tc_chain, test_passthrough_featured);
   tcase_add_test (tc_chain, test_unit_sizes);
   tcase_add_loop_test (tc_chain, test_cropping, 0, 25);