rawvideoparse: Compute plane offsets & strides if no custom ones are set

This is useful to ensure that the offsets and strides are computed if
only width, height, format etc. in the property config are set.

https://bugzilla.gnome.org/show_bug.cgi?id=769797

gst/rawparse/gstrawvideoparse.c

@@ -34,9 +34,13 @@
  * modified by using the width, height, pixel-aspect-ratio, framerate, interlaced,
  * top-field-first, plane-strides, plane-offsets, and frame-stride properties.
  *
- * If the properties configuration is used, be sure to set valid plane stride
- * offsets and values, otherwise the produced frames will not have a correct size.
- * Merely setting the format is not enough.
+ * If the properties configuration is used, plane strides and offsets will be
+ * computed by using gst_video_info_set_format(). This can be overridden by passing
+ * GValueArrays to the plane-offsets and plane-strides properties. When this is
+ * done, these custom offsets and strides are used later even if new width,
+ * height, format etc. property values might be set. To switch back to computed
+ * plane strides & offsets, pass NULL to one or both of the plane-offset and
+ * plane-array properties.
  *
  * The frame stride property is useful in cases where there is extra data between
  * the frames (for example, trailing metadata, or headers). The parser calculates 
@@ -116,7 +120,7 @@ enum
 
 
 #define GST_RAW_VIDEO_PARSE_CAPS \
-	GST_VIDEO_CAPS_MAKE(GST_VIDEO_FORMATS_ALL) "; "
+        GST_VIDEO_CAPS_MAKE(GST_VIDEO_FORMATS_ALL) "; "
 
 
 static GstStaticPadTemplate static_sink_template =
@@ -513,6 +517,17 @@ gst_raw_video_parse_set_property (GObject * object, guint prop_id,
       guint n_planes;
       guint i;
 
+      /* If no valarray is given, then disable custom
+       * plane strides & offsets and stick to the
+       * standard computed ones */
+      if (valarray == NULL) {
+        GST_DEBUG_OBJECT (raw_video_parse,
+            "custom plane strides & offsets disabled");
+        props_cfg->custom_plane_strides = FALSE;
+        gst_raw_video_parse_update_info (props_cfg);
+        break;
+      }
+
       /* Sanity check - reject empty arrays */
       if ((valarray != NULL) && (valarray->n_values == 0)) {
         GST_ELEMENT_ERROR (raw_video_parse, LIBRARY, SETTINGS,
@@ -541,6 +556,8 @@ gst_raw_video_parse_set_property (GObject * object, guint prop_id,
             props_cfg->plane_strides[i]);
       }
 
+      props_cfg->custom_plane_strides = TRUE;
+
       gst_raw_video_parse_update_info (props_cfg);
 
       if (!gst_raw_video_parse_is_using_sink_caps (raw_video_parse))
@@ -558,6 +575,17 @@ gst_raw_video_parse_set_property (GObject * object, guint prop_id,
       guint n_planes;
       guint i;
 
+      /* If no valarray is given, then disable custom
+       * plane strides & offsets and stick to the
+       * standard computed ones */
+      if (valarray == NULL) {
+        GST_DEBUG_OBJECT (raw_video_parse,
+            "custom plane strides & offsets disabled");
+        props_cfg->custom_plane_strides = FALSE;
+        gst_raw_video_parse_update_info (props_cfg);
+        break;
+      }
+
       /* Sanity check - reject empty arrays */
       if ((valarray != NULL) && (valarray->n_values == 0)) {
         GST_ELEMENT_ERROR (raw_video_parse, LIBRARY, SETTINGS,
@@ -586,6 +614,8 @@ gst_raw_video_parse_set_property (GObject * object, guint prop_id,
             i, props_cfg->plane_offsets[i]);
       }
 
+      props_cfg->custom_plane_strides = TRUE;
+
       gst_raw_video_parse_update_info (props_cfg);
 
       if (!gst_raw_video_parse_is_using_sink_caps (raw_video_parse))
@@ -1093,11 +1123,17 @@ gst_raw_video_parse_init_config (GstRawVideoParseConfig * config)
 static void
 gst_raw_video_parse_update_info (GstRawVideoParseConfig * config)
 {
-  int i;
+  guint i;
   guint n_planes;
+  guint last_plane;
   gsize last_plane_offset, last_plane_size;
   GstVideoInfo *info = &(config->info);
 
+  GST_DEBUG ("updating info with width %u height %u format %s "
+      " custom plane strides&offsets %d", config->width, config->height,
+      gst_video_format_to_string (config->format),
+      config->custom_plane_strides);
+
   gst_video_info_set_format (info, config->format, config->width,
       config->height);
 
@@ -1108,24 +1144,53 @@ gst_raw_video_parse_update_info (GstRawVideoParseConfig * config)
   GST_VIDEO_INFO_INTERLACE_MODE (info) =
       config->interlaced ? GST_VIDEO_INTERLACE_MODE_INTERLEAVED :
       GST_VIDEO_INTERLACE_MODE_PROGRESSIVE;
-  for (i = 0; i < GST_VIDEO_MAX_PLANES; ++i) {
-    GST_VIDEO_INFO_PLANE_OFFSET (info, i) = config->plane_offsets[i];
-    GST_VIDEO_INFO_PLANE_STRIDE (info, i) = config->plane_strides[i];
+
+  /* Check if there are custom plane strides & offsets that need to be preserved */
+  if (config->custom_plane_strides) {
+    /* In case there are, overwrite the offsets&strides computed by
+     * gst_video_info_set_format with the custom ones */
+    for (i = 0; i < GST_VIDEO_MAX_PLANES; ++i) {
+      GST_VIDEO_INFO_PLANE_OFFSET (info, i) = config->plane_offsets[i];
+      GST_VIDEO_INFO_PLANE_STRIDE (info, i) = config->plane_strides[i];
+    }
+  } else {
+    /* No custom planes&offsets; copy the computed ones into
+     * the plane_offsets & plane_strides arrays to ensure they
+     * are equal to the ones in the videoinfo */
+    for (i = 0; i < GST_VIDEO_MAX_PLANES; ++i) {
+      config->plane_offsets[i] = GST_VIDEO_INFO_PLANE_OFFSET (info, i);
+      config->plane_strides[i] = GST_VIDEO_INFO_PLANE_STRIDE (info, i);
+    }
   }
 
   n_planes = GST_VIDEO_INFO_N_PLANES (info);
   if (n_planes < 1)
     n_planes = 1;
 
-  last_plane_offset = GST_VIDEO_INFO_PLANE_OFFSET (info, n_planes - 1);
+  /* Figure out what plane is the physically last one. Typically, the
+   * this is the last plane in the list (= at index n_planes-1).
+   * However, this is not guaranteed, so we have to scan the offsets
+   * to find the last plane. */
+  last_plane_offset = 0;
+  last_plane = 0;
+  for (i = 0; i < n_planes; ++i) {
+    gsize plane_offset = GST_VIDEO_INFO_PLANE_OFFSET (info, i);
+    if (plane_offset >= last_plane_offset) {
+      last_plane = i;
+      last_plane_offset = plane_offset;
+    }
+  }
+  last_plane = n_planes - 1;
+
   last_plane_size =
       GST_VIDEO_INFO_PLANE_STRIDE (info,
-      n_planes - 1) * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT (info->finfo,
-      n_planes - 1, config->height);
+      last_plane) * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT (info->finfo,
+      last_plane, config->height);
 
   GST_VIDEO_INFO_SIZE (info) = last_plane_offset + last_plane_size;
 
-  GST_DEBUG ("last plane offset: %" G_GSIZE_FORMAT " last plane size: %"
+  GST_DEBUG ("last plane #%u:  offset: %" G_GSIZE_FORMAT " size: %"
       G_GSIZE_FORMAT " => frame size minus extra padding: %" G_GSIZE_FORMAT,
-      last_plane_offset, last_plane_size, GST_VIDEO_INFO_SIZE (info));
+      last_plane, last_plane_offset, last_plane_size,
+      GST_VIDEO_INFO_SIZE (info));
 }

gst/rawparse/gstrawvideoparse.h

@@ -78,6 +78,8 @@ struct _GstRawVideoParseConfig
   guint frame_stride;
 
   GstVideoInfo info;
+
+  gboolean custom_plane_strides;
 };
 
 

tests/check/elements/rawvideoparse.c

@@ -440,6 +440,117 @@ GST_START_TEST (test_push_with_no_framerate)
 
 GST_END_TEST;
 
+GST_START_TEST (test_computed_plane_strides)
+{
+  /* Test how plane strides & offsets are (re)computed if custom offsets/strides
+   * are disabled, and how they are preserved if they are enabled. */
+
+  GValueArray *plane_offsets_array;
+  GValueArray *plane_strides_array;
+  guint i;
+  guint const expected_comp_psize = TEST_WIDTH * TEST_HEIGHT;
+
+  setup_rawvideoparse (FALSE, TRUE, NULL, GST_FORMAT_BYTES);
+
+
+  /* The setup set a custom set of plane offsets and strides together with
+   * width=TEST_WIDTH and height=TEST_HEIGHT. Check that the offsets & strides
+   * are preserved even after setting new, different width & height values. */
+
+  g_object_set (G_OBJECT (rawvideoparse), "width", TEST_WIDTH * 2,
+      "height", TEST_HEIGHT * 2, NULL);
+
+  g_object_get (G_OBJECT (rawvideoparse), "plane-offsets",
+      &plane_offsets_array, "plane-strides", &plane_strides_array, NULL);
+
+  for (i = 0; i < plane_offsets_array->n_values; ++i) {
+    GValue *gvalue;
+
+    /* See setup_rawvideoparse() for how the offsets & strides are defined
+     * there. Offsets are set to plane_size*plane_index, and strides are
+     * set to the properties_ctx.plane_stride value. */
+
+    gvalue = g_value_array_get_nth (plane_offsets_array, i);
+    fail_unless (gvalue != NULL);
+    fail_unless_equals_uint64 (properties_ctx.plane_size * i,
+        g_value_get_uint (gvalue));
+
+    gvalue = g_value_array_get_nth (plane_strides_array, i);
+    fail_unless (gvalue != NULL);
+    fail_unless_equals_uint64 (properties_ctx.plane_stride,
+        g_value_get_uint (gvalue));
+  }
+
+  g_value_array_free (plane_offsets_array);
+  g_value_array_free (plane_strides_array);
+
+
+  /* Discard the custom planes&offsets, re-enabling computed values. */
+  g_object_set (G_OBJECT (rawvideoparse), "plane-offsets", (GValueArray *) NULL,
+      "plane-strides", (GValueArray *) NULL, NULL);
+
+
+  /* The strides & offsets should have been recomputed by now. Since the Y444
+   * format is used, all strides are the same, and should equal the frame width
+   * (which was set to TEST_WIDTH*2 earlier). Plane offsets should be
+   * plane_size*plane_index, with plane_size set to (TEST_WIDTH*2 * TEST_HEIGHT*2),
+   * or TEST_WIDTH*TEST_HEIGHT*4 (-> expected_comp_psize*4). */
+
+  g_object_get (G_OBJECT (rawvideoparse), "plane-offsets",
+      &plane_offsets_array, "plane-strides", &plane_strides_array, NULL);
+
+  for (i = 0; i < plane_offsets_array->n_values; ++i) {
+    GValue *gvalue;
+
+    gvalue = g_value_array_get_nth (plane_offsets_array, i);
+    fail_unless (gvalue != NULL);
+    fail_unless_equals_uint64 (expected_comp_psize * 4 * i,
+        g_value_get_uint (gvalue));
+
+    gvalue = g_value_array_get_nth (plane_strides_array, i);
+    fail_unless (gvalue != NULL);
+    fail_unless_equals_uint64 (TEST_WIDTH * 2, g_value_get_uint (gvalue));
+  }
+
+  g_value_array_free (plane_offsets_array);
+  g_value_array_free (plane_strides_array);
+
+
+  /* Again change the width & height values. width=TEST_WIDTH, height=TEST_HEIGHT.
+   * However, this time, offsets&strides are computed; the current values should
+   * not be preserved. Expected plane stride and offset values are similar to
+   * above, expect that no multiplications by 2 are present (since the TEST_WIDTH
+   * and TEST_HEIGHT values were passed without multiplying them). */
+
+  g_object_set (G_OBJECT (rawvideoparse), "width", TEST_WIDTH,
+      "height", TEST_HEIGHT, NULL);
+
+
+  g_object_get (G_OBJECT (rawvideoparse), "plane-offsets",
+      &plane_offsets_array, "plane-strides", &plane_strides_array, NULL);
+
+  for (i = 0; i < plane_offsets_array->n_values; ++i) {
+    GValue *gvalue;
+
+    gvalue = g_value_array_get_nth (plane_offsets_array, i);
+    fail_unless (gvalue != NULL);
+    fail_unless_equals_uint64 (expected_comp_psize * i,
+        g_value_get_uint (gvalue));
+
+    gvalue = g_value_array_get_nth (plane_strides_array, i);
+    fail_unless (gvalue != NULL);
+    fail_unless_equals_uint64 (TEST_WIDTH, g_value_get_uint (gvalue));
+  }
+
+  g_value_array_free (plane_offsets_array);
+  g_value_array_free (plane_strides_array);
+
+
+  cleanup_rawvideoparse ();
+}
+
+GST_END_TEST;
+
 
 static Suite *
 rawvideoparse_suite (void)
@@ -452,6 +563,7 @@ rawvideoparse_suite (void)
   tcase_add_test (tc_chain, test_push_unaligned_data_sink_caps_config);
   tcase_add_test (tc_chain, test_config_switch);
   tcase_add_test (tc_chain, test_push_with_no_framerate);
+  tcase_add_test (tc_chain, test_computed_plane_strides);
 
   return s;
 }