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>

diff --git a/docs/gst_plugins_cache.json b/docs/gst_plugins_cache.json
index d757acb..46e17d5 100644 (file)
--- 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 9ad5df7..71f36ff 100644 (file)
--- 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 4082b99..cf7588e 100644 (file)
--- 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 114aca4..b737838 100644 (file)
--- 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);