After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 757290 - gdkpixbufoverlay crash
gdkpixbufoverlay crash
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.7.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-10-29 09:20 UTC by Nicola
Modified: 2015-12-29 08:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test image that cause the crash (150 bytes, image/png)
2015-10-29 09:20 UTC, Nicola
Details

Description Nicola 2015-10-29 09:20:11 UTC
Created attachment 314372 [details]
test image that cause the crash

please test this pipeline with the attached png

videotestsrc ! gdkpixbufoverlay location=/tmp/nera1.png overlay-width=10 overlay-height=10 ! xvimagesink

you'll get this crash

  • #0 ??
  • #1 video_orc_resample_bilinear_u32
    at tmp-orc.c line 11744
  • #2 gst_video_blend_scale_linear_RGBA
    at video-blend.c line 218
  • #3 gst_video_overlay_composition_blend
    at video-overlay-composition.c line 489
  • #4 gst_gdk_pixbuf_overlay_transform_frame_ip
    at gstgdkpixbufoverlay.c line 634
  • #5 gst_video_filter_transform_ip
    at gstvideofilter.c line 320
  • #6 default_generate_output
    at gstbasetransform.c line 2175
  • #7 gst_base_transform_chain
    at gstbasetransform.c line 2333
  • #8 gst_pad_chain_data_unchecked
    at gstpad.c line 4096
  • #9 gst_pad_push_data
    at gstpad.c line 4348
  • #10 gst_pad_push
    at gstpad.c line 4467
  • #11 gst_base_src_loop
    at gstbasesrc.c line 2845
  • #12 gst_task_func
    at gsttask.c line 331
  • #13 ??
    from /usr/lib/libglib-2.0.so.0
  • #14 ??
    from /usr/lib/libglib-2.0.so.0
  • #15 start_thread
    from /usr/lib/libpthread.so.0
  • #16 clone
    from /usr/lib/libc.so.6

using an overlay image of different size make the pipeline work with no issue
Comment 1 Reynaldo H. Verdejo Pinochet 2015-11-09 00:35:38 UTC
special casing source dims on width or height == 1 goes around the problem
for me but I'm not sure about the maths here. I understand its building up
a 16.16 fixed point from the quotients but why the multiple -1s? shouldn't
it just be numerator << 16 / denominator? Additionally, is there any documentation for video_orc_resample_bilinear_u32() and why aren't we
using GstVideoScaler here?

index 8dc65c5..138a314 100644
--- a/gst-libs/gst/video/video-blend.c
+++ b/gst-libs/gst/video/video-blend.c
@@ -186,12 +186,12 @@ gst_video_blend_scale_linear_RGBA (GstVideoInfo * src, GstBuffer * src_buffer,
   gst_video_frame_map (&src_frame, src, src_buffer, GST_MAP_READ);
   gst_video_frame_map (&dest_frame, dest, *dest_buffer, GST_MAP_WRITE);
 
-  if (dest_height == 1)
+  if (dest_height == 1 || src->height == 1)
     y_increment = 0;
   else
     y_increment = ((src->height - 1) << 16) / (dest_height - 1) - 1;
 
-  if (dest_width == 1)
+  if (dest_width == 1 || src->width == 1)
     x_increment = 0;
   else
     x_increment = ((src->width - 1) << 16) / (dest_width - 1) - 1;
Comment 2 Nicolas Dufresne (ndufresne) 2015-11-09 01:01:11 UTC
> why aren't we using GstVideoScaler here?

This API precedes the video convert and scale API. It should be ported of course.
  
> index 8dc65c5..138a314 100644
> --- a/gst-libs/gst/video/video-blend.c
> +++ b/gst-libs/gst/video/video-blend.c
> @@ -186,12 +186,12 @@ gst_video_blend_scale_linear_RGBA (GstVideoInfo * src,
> GstBuffer * src_buffer,
>    gst_video_frame_map (&src_frame, src, src_buffer, GST_MAP_READ);
>    gst_video_frame_map (&dest_frame, dest, *dest_buffer, GST_MAP_WRITE);
>  
> -  if (dest_height == 1)
> +  if (dest_height == 1 || src->height == 1)
>      y_increment = 0;
>    else
>      y_increment = ((src->height - 1) << 16) / (dest_height - 1) - 1;
>  
> -  if (dest_width == 1)
> +  if (dest_width == 1 || src->width == 1)
>      x_increment = 0;
>    else
>      x_increment = ((src->width - 1) << 16) / (dest_width - 1) - 1;

That make sense to me regardless what this code is about. It protects against having -1 here (which would go badly).
Comment 3 Reynaldo H. Verdejo Pinochet 2015-12-28 22:38:34 UTC
Thanks for taking a look Nic. Confirmed with Sebastian on
IRC and I'm pushing the proposed fix and calling it done:

commit e61f5b21385e5b0f5bce594477d757b841330d6e
Author: Reynaldo H. Verdejo Pinochet <reynaldo@osg.samsung.com>
Date:   Thu Nov 12 14:01:03 2015 -0800

    videoblend: special case 1x1 src dims on increment computation
    
    Fix crash with 1x1 overlay pixmap
    
    [..]