xvimagesink: error out on buffer size sanity check failure.

If sanity checks on the buffer size allocated by XvShmCreateImage() fail,
call on g_set_error(), rather than just logging a warning, as this
failure is fatal.

Add a sanity check on buffer size when the video format is RGB. This adds to
existing checks on various YUV pixel formats.

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

3 files changed, 56 insertions, 42 deletions

diff --git a/sys/xvimage/xvimageallocator.c b/sys/xvimage/xvimageallocator.c
index 35821836f..c5be6db1d 100644
--- a/sys/xvimage/xvimageallocator.c
+++ b/sys/xvimage/xvimageallocator.c
@@ -55,6 +55,7 @@ struct _GstXvImageMemory
   GstMemory parent;
 
   gint im_format;
+  GstVideoInfo info;
   GstVideoRectangle crop;
 
   XvImage *xvimage;
@@ -229,6 +230,7 @@ gst_xvimage_memory_share (GstXvImageMemory * mem, gssize offset, gsize size)
       &mem->parent, mem->parent.maxsize, mem->parent.align,
       mem->parent.offset + offset, size);
 
+  sub->info = mem->info;
   sub->im_format = mem->im_format;
   sub->crop = mem->crop;
   sub->xvimage = mem->xvimage;
@@ -256,8 +258,8 @@ gst_xvimage_memory_copy (GstMemory * gmem, gssize offset, gsize size)
 
   copy = (GstXvImageMemory *)
       gst_xvimage_allocator_alloc (GST_XVIMAGE_ALLOCATOR_CAST (gmem->allocator),
-      mem->im_format, mem->xvimage->width, mem->xvimage->height, &mem->crop,
-      NULL);
+      mem->im_format, &mem->info, mem->xvimage->width,
+      mem->xvimage->height, &mem->crop, NULL);
 
   memcpy (copy->xvimage->data + copy->parent.offset,
       mem->xvimage->data + mem->parent.offset, mem->xvimage->data_size);
@@ -340,20 +342,22 @@ gst_xvimage_allocator_peek_context (GstXvImageAllocator * allocator)
 
 GstMemory *
 gst_xvimage_allocator_alloc (GstXvImageAllocator * allocator, gint im_format,
-    gint padded_width, gint padded_height, GstVideoRectangle * crop,
-    GError ** error)
+    const GstVideoInfo * info, gint padded_width, gint padded_height,
+    GstVideoRectangle * crop, GError ** error)
 {
   int (*handler) (Display *, XErrorEvent *);
   gboolean success = FALSE;
   GstXvContext *context;
   gint align, offset;
   GstXvImageMemory *mem;
+  gint expected_size = 0;
 
   context = allocator->context;
 
   mem = g_slice_new (GstXvImageMemory);
 
   mem->im_format = im_format;
+  mem->info = *info;
 #ifdef HAVE_XSHM
   mem->SHMInfo.shmaddr = ((void *) -1);
   mem->SHMInfo.shmid = -1;
@@ -371,7 +375,6 @@ gst_xvimage_allocator_alloc (GstXvImageAllocator * allocator, gint im_format,
 
 #ifdef HAVE_XSHM
   if (context->use_xshm) {
-    int expected_size;
 
     mem->xvimage = XvShmCreateImage (context->disp,
         context->xv_port_id, im_format, NULL, padded_width, padded_height,
@@ -401,45 +404,47 @@ gst_xvimage_allocator_alloc (GstXvImageAllocator * allocator, gint im_format,
 
     /* calculate the expected size.  This is only for sanity checking the
      * number we get from X. */
-    switch (im_format) {
-      case GST_MAKE_FOURCC ('I', '4', '2', '0'):
-      case GST_MAKE_FOURCC ('Y', 'V', '1', '2'):
-      {
-        gint pitches[3];
-        gint offsets[3];
-        guint plane;
-
-        offsets[0] = 0;
-        pitches[0] = GST_ROUND_UP_4 (padded_width);
-        offsets[1] = offsets[0] + pitches[0] * GST_ROUND_UP_2 (padded_height);
-        pitches[1] = GST_ROUND_UP_8 (padded_width) / 2;
-        offsets[2] =
-            offsets[1] + pitches[1] * GST_ROUND_UP_2 (padded_height) / 2;
-        pitches[2] = GST_ROUND_UP_8 (pitches[0]) / 2;
-
-        expected_size =
-            offsets[2] + pitches[2] * GST_ROUND_UP_2 (padded_height) / 2;
-
-        for (plane = 0; plane < mem->xvimage->num_planes; plane++) {
-          GST_DEBUG_OBJECT (allocator,
-              "Plane %u has a expected pitch of %d bytes, " "offset of %d",
-              plane, pitches[plane], offsets[plane]);
+    if (GST_VIDEO_FORMAT_INFO_IS_YUV (info->finfo)) {
+      switch (GST_VIDEO_FORMAT_INFO_FORMAT (info->finfo)) {
+        case GST_VIDEO_FORMAT_I420:
+        case GST_VIDEO_FORMAT_YV12:
+        {
+          gint pitches[3];
+          gint offsets[3];
+          guint plane;
+
+          offsets[0] = 0;
+          pitches[0] = GST_ROUND_UP_4 (padded_width);
+          offsets[1] = offsets[0] + pitches[0] * GST_ROUND_UP_2 (padded_height);
+          pitches[1] = GST_ROUND_UP_8 (padded_width) / 2;
+          offsets[2] =
+              offsets[1] + pitches[1] * GST_ROUND_UP_2 (padded_height) / 2;
+          pitches[2] = GST_ROUND_UP_8 (pitches[0]) / 2;
+
+          expected_size =
+              offsets[2] + pitches[2] * GST_ROUND_UP_2 (padded_height) / 2;
+
+          for (plane = 0; plane < mem->xvimage->num_planes; plane++) {
+            GST_DEBUG_OBJECT (allocator,
+                "Plane %u has a expected pitch of %d bytes, " "offset of %d",
+                plane, pitches[plane], offsets[plane]);
+          }
+          break;
         }
-        break;
+        case GST_VIDEO_FORMAT_YUY2:
+        case GST_VIDEO_FORMAT_UYVY:
+          expected_size = padded_height * GST_ROUND_UP_4 (padded_width * 2);
+          break;
+        default:
+          break;
       }
-      case GST_MAKE_FOURCC ('Y', 'U', 'Y', '2'):
-      case GST_MAKE_FOURCC ('U', 'Y', 'V', 'Y'):
-        expected_size = padded_height * GST_ROUND_UP_4 (padded_width * 2);
-        break;
-      default:
-        expected_size = 0;
-        break;
-    }
-    if (expected_size != 0 && mem->xvimage->data_size != expected_size) {
-      GST_WARNING_OBJECT (allocator,
-          "unexpected XShm image size (got %d, expected %d)",
-          mem->xvimage->data_size, expected_size);
+    } else if (GST_VIDEO_FORMAT_INFO_IS_RGB (info->finfo) &&
+        GST_VIDEO_FORMAT_INFO_N_PLANES (info->finfo) == 1) {
+      expected_size = padded_height * GST_ROUND_UP_4 (padded_width *
+          GST_VIDEO_FORMAT_INFO_PSTRIDE (info->finfo, 0));
     }
+    if (expected_size != 0 && mem->xvimage->data_size != expected_size)
+      goto unexpected_size;
 
     /* Be verbose about our XvImage stride */
     {
@@ -523,6 +528,14 @@ beach:
   return GST_MEMORY_CAST (mem);
 
   /* ERRORS */
+unexpected_size:
+  {
+    g_mutex_unlock (&context->lock);
+    g_set_error (error, GST_RESOURCE_ERROR, GST_RESOURCE_ERROR_WRITE,
+        "unexpected XShm image size (got %d, expected %d)",
+        mem->xvimage->data_size, expected_size);
+    goto beach;
+  }
 create_failed:
   {
     g_mutex_unlock (&context->lock);
diff --git a/sys/xvimage/xvimageallocator.h b/sys/xvimage/xvimageallocator.h
index c5fb9c80d..b47539dff 100644
--- a/sys/xvimage/xvimageallocator.h
+++ b/sys/xvimage/xvimageallocator.h
@@ -44,6 +44,7 @@ GstXvContext *        gst_xvimage_allocator_peek_context (GstXvImageAllocator *
 
 GstMemory *           gst_xvimage_allocator_alloc       (GstXvImageAllocator * allocator,
                                                          gint im_format,
+                                                         const GstVideoInfo * info,
                                                          gint padded_width,
                                                          gint padded_height,
                                                          GstVideoRectangle *crop,
diff --git a/sys/xvimage/xvimagepool.c b/sys/xvimage/xvimagepool.c
index 235a9f94e..7203105aa 100644
--- a/sys/xvimage/xvimagepool.c
+++ b/sys/xvimage/xvimagepool.c
@@ -174,7 +174,7 @@ xvimage_buffer_pool_alloc (GstBufferPool * pool, GstBuffer ** buffer,
   xvimage = gst_buffer_new ();
 
   mem = gst_xvimage_allocator_alloc (xvpool->allocator, xvpool->im_format,
-      xvpool->padded_width, xvpool->padded_height, &xvpool->crop, &err);
+      info, xvpool->padded_width, xvpool->padded_height, &xvpool->crop, &err);
 
   if (mem == NULL) {
     gst_buffer_unref (xvimage);