[FFmpeg-devel] [PATCH 1/2] avutil/buffer: always fetch the pool atomically.

[Michael Niedermayer]

On Tue, May 27, 2014 at 08:32:46AM +0200, Clément Bœsch wrote:
> On Tue, May 27, 2014 at 04:14:41AM +0200, Michael Niedermayer wrote:
> > On Mon, May 26, 2014 at 09:18:58PM +0200, Clément Bœsch wrote:
> > > ---
> > > So, after the recent threads patches, valgrind DRD (helgrind and tsan didn't
> > > went through yet) went from 1199 to 1215 passing tests (on a total of 1937)
> > > after the thread changes. It's better but it's far from being perfect. I had a
> > > quick look to some random entries, and it was complaining in that place.
> > > 
> > > I'm not convince this is an appropriate change since I'm not familiar with that
> > > stuff, but it might help. Please comment.
> > 
> > can you explain the bug this fixes ?
> 
> It's supposed to deal with that (picked randomly on helgrind instance here):
> 
> ==27841== Possible data race during read of size 8 at 0x76B1D40 by thread #3
> ==27841== Locks held: 2, at addresses 0x767B230 0x767B570
> ==27841==    at 0xC7EC86: av_buffer_pool_get (buffer.c:346)
> ==27841==    by 0x97CE81: avcodec_default_get_buffer2 (utils.c:627)
> ==27841==    by 0x97D91E: ff_get_buffer (utils.c:1008)
> ==27841==    by 0x8DC585: ff_thread_get_buffer (pthread_frame.c:786)
> ==27841==    by 0xB1B5A1: alac_decode_frame (alac.c:296)
> ==27841==    by 0x8DA8AC: frame_worker_thread (pthread_frame.c:156)
> ==27841==    by 0x4C2D836: ??? (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so)
> ==27841==    by 0x5D33123: start_thread (in /usr/lib/libpthread-2.19.so)
> ==27841==    by 0x69614BC: clone (in /usr/lib/libc-2.19.so)
> ==27841== 
> ==27841== This conflicts with a previous write of size 8 by thread #1
> ==27841== Locks held: none
> ==27841==    at 0xC7E447: pool_release_buffer (buffer.c:279)
> ==27841==    by 0xC7E6CE: av_buffer_unref (buffer.c:115)
> ==27841==    by 0xC861F3: av_frame_unref (frame.c:364)
> ==27841==    by 0xC86328: av_frame_free (frame.c:128)
> ==27841==    by 0x4E20D2: filter_frame (af_aresample.c:217)
> ==27841==    by 0x48C772: ff_filter_frame_framed (avfilter.c:1081)
> ==27841==    by 0x48E888: ff_filter_frame (avfilter.c:1161)
> ==27841==    by 0x48C772: ff_filter_frame_framed (avfilter.c:1081)
> ==27841== 
> 
> ...but I need to check if it actually does the trick.

hmm, ok but is there a real bug ?
can you give an example of a sequence of events that cause the code
to misbehave ?


> 
> > if theres no bug, what speed effect does this have ?
> 
> I didn't test: do you see a relevant bench method to test this?

i would put START/STOP_TIMER over the functions or loops using
the changed code


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140527/c05f3ccd/attachment.asc>