GNOME Bugzilla – Bug 694299
crash in put_pixels16_sse2() with SVQ1 video
Last modified: 2013-07-18 14:25:54 UTC
Downstream at launchpad: https://bugs.launchpad.net/ubuntu/+source/totem/+bug/1115070 Crashed while Nautilus opened a big Download folder: Related to: https://bugs.launchpad.net/bugs/404522 and https://bugs.launchpad.net/bugs/420930 ThreadStackTrace: https://launchpadlibrarian.net/131873894/ThreadStacktrace.txt StacktraceTop: https://launchpadlibrarian.net/131873892/StacktraceTop.txt StacktraceSource: https://launchpadlibrarian.net/131873890/StacktraceSource.txt Stacktrace: https://launchpadlibrarian.net/131873888/Stacktrace.txt
It's crashing in libav: av_lzo1x_decode() Can you please make the file that caused the crash available?
I am the original submitter @launchpad/Ubuntu Here: http://www.noiraude.net/bug/levent_t1.mov Is the file that was processed (according to the ThreadStackTrace.txt) during the crash (as you requested): Regards.
Created attachment 237168 [details] track of totem in gdb with the crashing file pasted into it
the crashing file works ok in VLC, but make crash with same error Banshee
Also crashes with gst-launch-1.0. Looks like a libav bug.
+ Trace 231545
Does not work with git master + internal libav 9.1 either.
I have the same problem for this video: http://absolut.zogzog.org/share/samples/mov/Stereophonics%20-%20Lying.mov It crashes in put_pixels16_sse2 with a general protection ip error, so it probably is an alignment bug.
Did anybody reproduce this with avplay or any other software using libav? Also did anybody forward this to the libav bugtracker?
The video I posted above indeed crashes with avplay (0.8.5). I'll try with 0.9
Crashes with latest git master (using libav 9.4) but doesn't crash with avplay from the Debian 9.4 packages.
*** Bug 700965 has been marked as a duplicate of this bug. ***
This does not seem to be libav's fault: This works fine: gst-launch-1.0 file:///media/data/contents/levent_t1.mov ! qtdemux ! avdec_svq1 ! fakesink avdec_svq1-0.GstPad:src: caps = video/x-raw, format=(string)YUV9, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive, colorimetry=(string)bt601, framerate=(fraction)24/1 This segfaults gst-launch-1.0 file:///media/data/contents/levent_t1.mov ! qtdemux ! avdec_svq1 ! videoconvert ! xvimagesink avdec_svq1-0.GstPad:src: caps = video/x-raw, format=(string)YUV9, width=(int)320, height=(int)240, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive, colorimetry=(string)bt601, framerate=(fraction)24/1 (obviously running it in valgrind doesn't reproduce the issue .. .*sigh*)
This seems to be a bug in the direct rendering path. Works: gst-launch file:///media/data/contents/levent_t1.mov ! qtdemux ! avdec_svq1 direct-rendering=false ! videoconvert ! 'video/x-raw, format=NV12' ! fakesink Crashes: gst-launch file:///media/data/contents/levent_t1.mov ! qtdemux ! avdec_svq1 direct-rendering=true ! videoconvert ! 'video/x-raw, format=NV12' ! fakesink
Created attachment 249433 [details] [review] avviddec: match avplay to compute plane offsets for picture This patch fixes the issue for me. The new padding values make the video frame info match avplay exactly (plane offsets and sizes). I'm not sure if's libav or videoconvert that does not cope with the non-0 left and top padding values properly.
Unfortunately this makes other codecs crash...
Comment on attachment 249433 [details] [review] avviddec: match avplay to compute plane offsets for picture seems to need work accodring to comment #15.
Where is the relevant code for that in libav?
Libav allocates output buffer in update_frame_pool in libavcodec/utils.c. I've attached a small patch that prints out the plane offsets and sizes for reference.
Created attachment 249476 [details] [review] print output frame info in libav
Ok, so in any case it's probably a good idea to completely replicate that code (after your patch that's not the case yet, right?). If it still crashes then, we'll have to understand why :)
The new offsets do match the libav values, so I'm not sure why some other codecs crash now. Here are some examples: * h264 yuv420p, 848x480 video: libav: frame 848x480 aligned 848x482 stride align 16-16-16 with edge 880x514 strides for width 880: 880-440-440 strides for width 896: 896-448-448 data[0] = 0 size=460544 data[1] = 460544 size=115136 data[2] = 575680 size=115136 gstlibav: <avdec_h264-0> aligned dimension 848x480 -> 880x514 padding t:0 l:0 r:32 b:34, stride_align 15:15:15:15 gst_buffer_add_video_meta_full: plane 0, offset 0, stride 896 gst_buffer_add_video_meta_full: plane 1, offset 460544, stride 448 gst_buffer_add_video_meta_full: plane 2, offset 575680, stride 448 * mpeg4, yuv420p, 640x480 video: libav: frame 640x480 aligned 640x480 stride align 16-16-16 with edge 672x512 strides for width 672: 672-336-336 data[0] = 0 size=344064 data[1] = 344064 size=86016 data[2] = 430080 size=86016 gstlibav: <avdec_mpeg4-0> aligned dimension 640x480 -> 672x512 padding t:0 l:0 r:32 b:32, stride_align 15:15:15:15 gst_buffer_add_video_meta_full: plane 0, offset 0, stride 672 gst_buffer_add_video_meta_full: plane 1, offset 344064, stride 336 gst_buffer_add_video_meta_full: plane 2, offset 430080, stride 336 * svq1, yuv410p, 320x240 video: libav: frame 320x240 aligned 320x256 stride align 16-16-16 with edge 352x288 strides for width 352: 352-88-88 strides for width 384: 384-96-96 data[0] = 0 size=110592 data[1] = 110592 size=6912 data[2] = 117504 size=6912 gstlibav: <avdec_svq1-0> aligned dimension 320x240 -> 352x288 padding t:0 l:0 r:32 b:48, stride_align 15:15:15:15 gst_buffer_add_video_meta_full: plane 0, offset 0, stride 384 gst_buffer_add_video_meta_full: plane 1, offset 110592, stride 96 gst_buffer_add_video_meta_full: plane 2, offset 117504, stride 96
After some more investigation it turns out the gstlibav code is right, libav does need padding on top and left of the planes. The padding offset are computed in video_get_buffer in libavcodec/utils.c, and in the crash is due to the fact that the UV plane offsets are not computed correctly in gst_video_info_align. The offsets are not aligned to the stride_align values, which is why the SIMD instructions crash with a general protection error. For the levent_1.mov video, libav uses the following offsets for each plane: offset[0] = 6160 offset[1] = 400 offset[2] = 400 Whereas gstreamer uses the following values: plane 0: comp: 0, hedge 16 vedge 16 align 15 stride 384 offset 6160 plane 1: comp: 1, hedge 4 vedge 4 align 15 stride 96 offset 388 plane 2: comp: 2, hedge 4 vedge 4 align 15 stride 96 offset 388
Are you planning to write a patch for calculating the UV plane offsets correctly?
Yes I'm working on it
Created attachment 249501 [details] [review] video: respect stride alignment when calculating planes offsets This patch for gst-plugins-base makes all plane start offsets aligned on the plane stride align value. The patch by itself is right, however it makes some videos crash when decoding them with libav. This is because libav needs even more padding at the very end of each plane, in some SIMD optimized functions. See https://trac.ffmpeg.org/ticket/2645 and http://git.videolan.org/?p=ffmpeg.git;a=commit;h=175e916fa20b7887bdb29809817985e481ae0888. This is a patch from ffmpeg 2.0, even in the latest libav there is an out-of-bounds write in some cases because the padding is not big enough. I'm not sure how to add this small padding at the end of each plane with the current API in gstreamer.
What exactly does libav require there? Some padding after each plane or each plane start aligned?
Both. The plane size should have 16 + stride_align bytes padding added at the end for some SIMD optimized functions to work, from the links I've posted above. The plane start offset must then be aligned otherwise some SIMD instructions will trigger a general protection error. The simplest way to work around this would be to increase the align.padding_bottom value by 1 in gstavviddec.c. This would alloc one more line than necessary in each plane.
Yes, let's just do that then
Yes, this is correct, after vertical and horizontal padding, it should add some more to align to the requested alignment, like what libav does. for the extra-extra padding, I would indeed just add more padding at the bottom. You could theoretically also configure the extra padding in the params of the allocator that you set with gst_buffer_pool_config_set_allocator() but that seems more trouble.
Created attachment 249515 [details] [review] avviddec: increase bottom padding for output frames
Review of attachment 249501 [details] [review]: Needs work ::: gst-libs/gst/video/video-info.c @@ +722,3 @@ gint width, height; gint padded_width, padded_height; + gint i, j, n_planes; I can't see where j is used. @@ +785,3 @@ (hedge * GST_VIDEO_FORMAT_INFO_PSTRIDE (vinfo, comp)); + + offset = (offset + align->stride_align[i]) & ~align->stride_align[i]; you're overwriting offset again. Which one do you want ?
commit 28e1dadbfaa403679e69f8173d1aa2c7500fd556 Author: Arnaud Vrac <avrac@freebox.fr> Date: Thu Jul 18 14:13:33 2013 +0200 video: respect stride alignment when calculating planes offsets https://bugzilla.gnome.org/show_bug.cgi?id=694299 commit 4a2054c6aad18db3c1e9bc359d4ba4de855b036d Author: Arnaud Vrac <avrac@freebox.fr> Date: Thu Jul 18 16:11:16 2013 +0200 avviddec: increase bottom padding for output frames libav can write slightly after the plane end in some SIMD optimized functions. The extra padding value needs to be at least 16+stride_align for each plane, so just increase the bottom padding value for the output frame. https://bugzilla.gnome.org/show_bug.cgi?id=694299