GNOME Bugzilla – Bug 680796
[0.11] Crash because of misaligned frame data
Last modified: 2012-09-11 13:03:30 UTC
I get a crash when using a libav decoder in my pipeline and when direct rendering is enabled. In this case the output frame buffers are allocated by a downstream buffer pool. The crash happens in the put_pixels16_sse2() function in libavcodec/x86/dsputil_mmx.c:455. It happens because this function assumes that the output data is aligned on 16 bytes boundaries, and it is not the case. I can confirm that changing the movdqa instructions in this function to movdqu fixes the problem. The problem should be fixed in gst-ffmepg by forcing the plane data of the output buffers allocated from the buffer pool to be 16-bytes aligned.
0.10 or git master (0.11/1.0) ? Should be fixed in 0.11.
This happens with git 0.11. The alignment meta adds padding to the frame buffer (top 16, right 16, left 16, bottom 32), however the buffer address passed to put_pixels16_sse2 is not 16 bits aligned. This happens on x86 BTW, not amd64.
Loosely related to bug #680386
commit fe0a8d005d837703cf9b6649a183c75511f2f2bb Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Wed Aug 8 15:59:59 2012 +0200 avviddec: Properly align and pad buffers for libav https://bugzilla.gnome.org/show_bug.cgi?id=680796
Still crashes for me, I guess the buffer is now aligned however the start of the lines in the frames might not be.
The start of the lines should be, that's what GstVideoAlign is used for. The width and height of the frames is set to what libav wants, but maybe it tells us something wrong here :) What exactly is your testcase that crashes?
GstVideoAlign is not used yet AFAIK
It is and it's also implemented properly in the x*imagesink buffer pools and GstVideoBufferPool (bug #680386 happened because GstVideoAlign wasn't properly handled in vp8enc).
(In reply to comment #8) > It is and it's also implemented properly in the x*imagesink buffer pools and > GstVideoBufferPool (bug #680386 happened because GstVideoAlign wasn't properly > handled in vp8enc). The stride alignment is simply ignored in x*imagesink.
Actually you're right, seems to be ignored in GstVideoBufferPool too. But all other fields are used correctly
Also ignored in GstVideoPool, I'll see if I can fix it.
There might also be some other problems, from the logs I see the following alignment values: libav gstffmpegviddec.c:1497:gst_ffmpegviddec_decide_allocation:<avdec_mpeg4-0> aligned dimension 640x480 -> 672x512 padding t:16 l:16 r:16 b:16, stride_align 7:7:7:7 default gstvideopool.c:136:gst_video_info_align: plane 0: hedge 16 vedge 16 align 7 stride 672 default gstvideopool.c:136:gst_video_info_align: plane 1: hedge 8 vedge 8 align 7 stride 336 default gstvideopool.c:136:gst_video_info_align: plane 2: hedge 8 vedge 8 align 7 stride 336 The 7 stride align should be 15 for sse2 instructions to work. Also on plane 1 and 2 hedge is 8 so that probably is where the misaligned access is. The start address of plane 1 will be 672x512+8 in this case, which is wrong.
put_pixels16_sse2() is only used on the Y plane. The only place where It could be misaligned would be in the imagesink when XSHM is disabled. Please test with the following patch: commit 9b3849db1ccf4f6b985d8fd1eeb0913ae44bc6be Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Fri Aug 10 16:58:47 2012 +0200 x11: fix alignment in non-XSHM case Align the allocated memory to 16 bytes. When doing XSHM we are already aligned to a page boundary but without, we use plain g_malloc, which could allocate aligned on 8 bytes only. See https://bugzilla.gnome.org/show_bug.cgi?id=680796
This patch won't help since I'm using a custom sink for my platform. The buffers that make libav crash in put_pixels16_sse2 are allocated by the video pool from the videoconvert element. I will provide more info as soon as I can.
This patch seems to fix the problem, I haven't been able to reproduce the crash since I applied it: commit 4b62101f44843643a4b371c35e699fa06288de7d Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Fri Aug 10 17:03:26 2012 viddec: use the right pointers Use the plane pointers and strides. Improve some debug
Sorry I was wrong, this commit does not help. I've investigated and found out that the misaligned frame buffer is allocated by _default_mem_copy, which apparently does not respect alignment of the source memory. I am still trying to find out why the buffer is copied in the first place. The pipeline is allocated by playbin but the relevant part is the following: avdev_mpeg4 ! streamsynchronizer ! queue ! videoconvert The buffer allocated from the videoconvert buffer pool is already locked when libav tries to map the frame, and I don't understand how it is possible. The stack trace to the _default_mem_copy call is the following:
+ Trace 230665
Do you have any idea why the frame buffer is being copied ? Thanks
Some more commits: commit f56e1222dae16efe0ee6f60c86d18d117c0df28c Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Mon Aug 20 10:16:59 2012 +0200 videopool: improve alignment Check the alignment of the strides in gst_video_info_align and increase the padding on the frame until the strides are aligned.
commit 0e4d2814fdc8b4370749af107a08c25466427440 Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Mon Aug 20 11:31:51 2012 +0200 allocator: make a copy with the same alignment When making a copy of the memory allocated from the default memory allocator, make sure the new copy has the same alignment as the original memory. See https://bugzilla.gnome.org/show_bug.cgi?id=680796
I think the copy is made in streamsynchronizer when it tries to make the buffer writable to unset the DISCONT flag.
(In reply to comment #19) > I think the copy is made in streamsynchronizer when it tries to make the buffer > writable to unset the DISCONT flag. This (gst_buffer_copy()) shouldn't copy the GstMemory though if I'm not mistaken.
(In reply to comment #20) > (In reply to comment #19) > > I think the copy is made in streamsynchronizer when it tries to make the buffer > > writable to unset the DISCONT flag. > > This (gst_buffer_copy()) shouldn't copy the GstMemory though if I'm not > mistaken. The memory is marked as NO_SHARE and is thus copied. We should likely put the X info together with the GstMemory instead of setting it as buffer metadata to fix this.
Aren't the buffers allocated by the videoconvert element ? I don't know if X image sink supports I420.
At least the crash does not happen anymore now.
What's up with this? Is anything missing?
It works fine for me now, thanks.