GNOME Bugzilla – Bug 628176
[basetransform] Problems with buffer handling in inplace mode
Last modified: 2010-09-23 16:30:09 UTC
+++ This bug was initially created as a clone of Bug #628174 +++ ==812== Source and destination overlap in memcpy(0xb9846e0, 0xb9846e0, 622080) ==812== at 0x4027865: memcpy (mc_replace_strmem.c:497) ==812== by 0x4552926: gst_base_transform_handle_buffer (string3.h:52) ==812== by 0x4553489: gst_base_transform_chain (gstbasetransform.c:2308) ==812== by 0x45C31F4: gst_pad_chain_data_unchecked (gstpad.c:4182) ==812== by 0x45C3C53: gst_pad_push_data (gstpad.c:4411) ==812== by 0x7EAFB41: handle_slice (gstmpeg2dec.c:1040) ==812== by 0x7EB0511: gst_mpeg2dec_chain (gstmpeg2dec.c:1227) ==812== by 0x45C31F4: gst_pad_chain_data_unchecked (gstpad.c:4182) ==812== by 0x45C3C53: gst_pad_push_data (gstpad.c:4411) ==812== by 0x7D7F819: gst_queue_loop (gstqueue.c:1109) ==812== by 0x45F1460: gst_task_func (gsttask.c:271) ==812== by 0x45F2B36: default_func (gsttaskpool.c:68) ==812== by 0x476BD0B: ??? (in /lib/libglib-2.0.so.0.2400.1) ==812== by 0x4769DEE: ??? (in /lib/libglib-2.0.so.0.2400.1) ==812== by 0x47F696D: start_thread (pthread_create.c:300) ==812== by 0x48D7A4D: clone (clone.S:130) This is the memcpy() in gstbasetransform.c:2186: > if (inbuf != *outbuf) { > /* different buffers, copy the input to the output first, we then do an > * in-place transform on the output buffer. */ > memcpy (GST_BUFFER_DATA (*outbuf), GST_BUFFER_DATA (inbuf), > GST_BUFFER_SIZE (inbuf)); > } If the data pointers are the same but the buffer pointer are not the same, one of them must be a subbuffer of the other one and both are not writable at all, or even worse, they're completely unrelated but share the same memory.
(In reply to comment #0) > > This is the memcpy() in gstbasetransform.c:2186: > > if (inbuf != *outbuf) { > > /* different buffers, copy the input to the output first, we then do an > > * in-place transform on the output buffer. */ > > memcpy (GST_BUFFER_DATA (*outbuf), GST_BUFFER_DATA (inbuf), > > GST_BUFFER_SIZE (inbuf)); > > } > > If the data pointers are the same but the buffer pointer are not the same, one > of them must be a subbuffer of the other one and both are not writable at all, > or even worse, they're completely unrelated but share the same memory. Your reasoning would be correct if it weren't for the FIXME in gst_base_transform_prepare_output_buffer(). the problem is currently that when the prepare_output_buffer function reuses the input buffer as output buffer (which would increase the refcount of in/out), we decrease that refcount later again in order to keep the reused buffer writable. I don't quite remember how this abomination came to live but it think it was a natural result of the refcounting practices for a function like prepare_output_buffer(). It should have taken ownership of the input buffer instead of how it was designed before. So, you can end up in above situation without anything being really wrong.. I think the best thing is to simply check for the pointers as well.
commit 90d65cb446dbdaa681351a4dc0a65e77ac5fa330 Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Thu Sep 23 18:18:54 2010 +0200 basetransform: avoid useless memcpy Because of the awkward refcounting in prepare_output_buffer, we might end up with writable buffers that point to the same data. Check for those cases so that we avoid a useless memcpy and keep valgrind quiet. Fixes #628176