From 4e83e894dfd2bf0e52e826dd42a93f0bed61ab4f Mon Sep 17 00:00:00 2001 From: Duncan Palmer Date: Thu, 7 Jul 2016 22:27:15 +1000 Subject: [PATCH] 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 --- sys/xvimage/xvimageallocator.c | 95 ++++++++++++++++++++++++------------------ sys/xvimage/xvimageallocator.h | 1 + sys/xvimage/xvimagepool.c | 2 +- 3 files changed, 56 insertions(+), 42 deletions(-) diff --git a/sys/xvimage/xvimageallocator.c b/sys/xvimage/xvimageallocator.c index 3582183..c5be6db 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 c5fb9c8..b47539d 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 235a9f9..7203105 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); -- 2.7.4