GNOME Bugzilla – Bug 663248
videoconvert: bad memory accesses in orc code for odd width buffers
Last modified: 2015-04-16 10:55:58 UTC
I've tracked them down to the ORC versions of some of the AYUV related routines when run on buffers with odd width. Since AYUV is not subsampled, I guess the issue is with the other format converted from/to. I'm not 100% sure, but I think these AYUV related routines process two pixels at a time, and the caller will pass (width+1)/2. There seems to be no way to convert an extra column alone, save custom code each time. Someone who is fluent with ORC code could maybe look at cogorc_convert_UYVY_AYUV and say if I'm correct about that two pixels thing. We might then need to either convert it to single pixel at a time, or have an alternate version to use for odd widths. Easily checked by running the pipelines/colorspace test. The crash goes with this patch (which removes the tests for these AYUV related formats (though I've not run that with valgrind, so it might be there still are more bad accesses): diff --git a/gst/colorspace/colorspace.c b/gst/colorspace/colorspace.c index bb8c58d..d3d804d 100644 --- a/gst/colorspace/colorspace.c +++ b/gst/colorspace/colorspace.c @@ -2397,7 +2397,7 @@ static const ColorspaceTransform transforms[] = { COLOR_SPEC_NONE, TRUE, convert_UYVY_I420}, {GST_VIDEO_FORMAT_UYVY, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_YUY2, COLOR_SPEC_NONE, TRUE, convert_UYVY_YUY2}, - {GST_VIDEO_FORMAT_UYVY, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_AYUV, + {GST_VIDEO_FORMAT_UYVY+88888, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_AYUV, COLOR_SPEC_NONE, TRUE, convert_UYVY_AYUV}, {GST_VIDEO_FORMAT_UYVY, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_Y42B, COLOR_SPEC_NONE, TRUE, convert_UYVY_Y42B}, @@ -2406,11 +2406,11 @@ static const ColorspaceTransform transforms[] = { {GST_VIDEO_FORMAT_AYUV, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_I420, COLOR_SPEC_NONE, TRUE, convert_AYUV_I420}, - {GST_VIDEO_FORMAT_AYUV, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_YUY2, + {GST_VIDEO_FORMAT_AYUV+88888, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_YUY2, COLOR_SPEC_NONE, TRUE, convert_AYUV_YUY2}, - {GST_VIDEO_FORMAT_AYUV, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_UYVY, + {GST_VIDEO_FORMAT_AYUV+88888, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_UYVY, COLOR_SPEC_NONE, TRUE, convert_AYUV_UYVY}, - {GST_VIDEO_FORMAT_AYUV, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_Y42B, + {GST_VIDEO_FORMAT_AYUV+8888, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_Y42B, COLOR_SPEC_NONE, TRUE, convert_AYUV_Y42B}, {GST_VIDEO_FORMAT_AYUV, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_Y444, COLOR_SPEC_NONE, TRUE, convert_AYUV_Y444}, @@ -2421,7 +2421,7 @@ static const ColorspaceTransform transforms[] = { COLOR_SPEC_NONE, TRUE, convert_Y42B_YUY2}, {GST_VIDEO_FORMAT_Y42B, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_UYVY, COLOR_SPEC_NONE, TRUE, convert_Y42B_UYVY}, - {GST_VIDEO_FORMAT_Y42B, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_AYUV, + {GST_VIDEO_FORMAT_Y42B+8888, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_AYUV, COLOR_SPEC_NONE, TRUE, convert_Y42B_AYUV}, {GST_VIDEO_FORMAT_Y42B, COLOR_SPEC_NONE, GST_VIDEO_FORMAT_Y444, COLOR_SPEC_NONE, TRUE, convert_Y42B_Y444},
Does this also/still apply to videoconvert in git master? If yes, I would suggest we move the bug to -base. If not, I would suggest we WONTFIX it.
I have seen green artifact with odd width in videoconvert, but haven't had time to dig into it. It's most likely related. Valgrind also report some bad memory access from ORC, but I was assuming the my version or ORC might be too old.
At least part of this bug (UYVY->AYUV and YUY2->AYUV) is caused by: From d69e4e7a2c4107acf9f09338c78c79306cdf79b9 Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Thu, 24 Nov 2011 11:09:20 +0100 Subject: [PATCH 1/1] videoconvert: fix width/height mismatches https://bugzilla.gnome.org/show_bug.cgi?id=663238 --- gst/videoconvert/videoconvert.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) In any case, this is not a NEEDINFO.
Let's move it to videoconvert then..
odd width/height problems are a bit everywhere, this is the status: - video-format: pack/unpack functions work in all cases - videotestsrc: uses pack/unpack functions, works in all cases - videoconvert: fastpath only when it supports odd witdh/height, other paths use pack/unpack functions and works correctly too commit 9b4e2b4b36a7e27564bf31584de8741c6365ee82 Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Fri Sep 6 12:44:10 2013 +0200 videoconvert: disable fastpath for odd width on some formats commit fae9d82515cba6d799dc83f093a7685aadb94e9e Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Fri Sep 6 12:43:27 2013 +0200 video-format: fix NV24 pack/unpack function We can't reuse the NV12 functions, we need to make new ones. commit 26d04c7582c9d01c40ce36bf99be24124e3dd344 Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Fri Sep 6 12:42:47 2013 +0200 video-format: handle odd width in more pack/unpack functions commit 2f6f0ee2149e83841e0e7cb8c69197e385db3d51 Author: Tim-Philipp Müller <tim@centricular.net> Date: Thu Sep 5 18:33:28 2013 +0100 video-format: minor pack_YVYU optimisation Re-use already calculated line offset. commit ede804041c5efa5c6e977fb94c43c669a57d1adc Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Thu Sep 5 17:46:03 2013 +0200 videotestsrc: flush pending lines on odd height commit e97f6401deb3272c03ec1fe8da0442910d5b2ecf Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Thu Sep 5 17:22:50 2013 +0200 videoconvert: add additional width/height constraints Some of the fastpath function can only work with aligned widht/height so make sure we check this as well when choosing a fastpath. Add fastpath for I420/YV12 -> BGRx
videoscale does not handle odd width correctly in many cases.
please try again with git, it's all new code.
This made me think of something from earlier: https://bugzilla.gnome.org/show_bug.cgi?id=720692 Can you try this patch to see if it helps ?
Gah, wrong bug, sorry.
For saneish values, it seems to work fine. However, for rescaling to 1x1, 16x1, or 320x1: ** (gst-launch-1.0:7272): CRITICAL **: gst_video_resampler_init: assertion 'out_size != 0' failed ** (gst-launch-1.0:7272): CRITICAL **: resampler_zip: assertion 'r1->max_taps == r2->max_taps' failed *** glibc detected *** gst-launch-1.0: munmap_chunk(): invalid pointer: 0x00007f7849417fd6 *** 320x2: A stream of: 0:00:08.963817917 7307 0x7fa708037850 ERROR default video-frame.c:147:gst_video_frame_map_id: failed to map video frame plane 2 WARNING: from element /GstPipeline:pipeline0/GstVideoScale:videoscale0: Internal GStreamer error: code not implemented. Please file a bug at http://bugzilla.gnome.org/enter_bug.cgi?product=GStreamer. Additional debug info: gstvideofilter.c(292): gst_video_filter_transform (): /GstPipeline:pipeline0/GstVideoScale:videoscale0: invalid video buffer received At 320x3, it starts working. The second one seems like a different issue, not sure about the memory corruption from the first one,
I can't see those anymore, with pipeline: gst-launch-1.0 uridecodebin uri=file://... ! videoscale ! video/x-raw,width=320,height=1 ! videoconvert ! fakesink It's also valgrind clean, including for weird widths.
Let's close it then. If it's still an issue with the current code, someone will run into it again soon enough and file a new bug.