[FFmpeg-devel] [PATCH] submit_thread should also lock the mutex that guards the source avctx (prev_thread->avctx) when calling update_context_from_thread.

[Clément Bœsch]

On Mon, Feb 29, 2016 at 11:47:14AM -0800, Wan-Teh Chang wrote:
> On Fri, Feb 26, 2016 at 5:04 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> >
> > To be a little bit more explicit, it should be relatively easy to
> > accomplish this by changing the position of qscale in the relevant struct,
> > and only copy that half of the struct (where entries are obviously grouped)
> > where we actually care about the copied value.
> >
> > See e.g. the copy_fields macro [1] on how we do partial-struct copies.
> >
> > Ronald
> >
> > [1]
> > http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/h264_slice.c;h=9a5bc3f63fb5c6aae3747a03657b8dc821c7d750;hb=HEAD#l427
> 
> Hi Ronald,
> 
> Thanks a lot for the hints. As soon as I had access to a computer last
> Saturday, I implemented your suggestion. Although the data race on
> |qscale| is gone, ThreadSantizer warned about the next data race of
> this kind. I fixed that using the same approach, and then
> ThreadSantizer warned about the next data race of this kind.
> 
> That made me realize the fix for this class of data race is going to
> big, and I should do this work in the ffmpeg HEAD rather than the old
> version I'm using. I followed the instructions James Almer gave
> earlier in this email thread:
> 
> ./configure --samples=fate-suite/ --toolchain=clang-tsan --disable-stripping
> make fate-h264 THREADS=4
> 
> (Note: James suggested --toolchain=gcc-tsan. That didn't work for me
> for some reason. I tried --toolchain=clang-tsan instead and it
> worked.)
> 
> ThreadSanitizer reported the same class of data races in the h264
> decoder with multiple threads. To understand the code and propose a
> proper fix, I am afraid that I will need to live and breathe this code
> for several days because the structs involved (H264Context,
> H264SliceContext, etc.) have a lot of members.
> 
> So, I am wondering if you could run fate-h264 under tsan and take a
> look at the data races in the h264 decoding code. I think you will be
> able to fix the data races much faster than I can. If you don't have a
> lot of time, I'll be happy to implement a solution if you can outline
> it after your investigation. Thanks.
> 

BTW, thanks a lot for working on this. You can also play with valgrind
(tool helgrind or drd) if it's supported on your system. You won't need a
special build to use it (although for fate, you'll need something like
--target-exec="valgrind --tool=helgrind" or get the command with make
fate-foobar V=1 and prepend valgrind).

Note: according to Ronald it's actually very close to TSAN.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160229/1fa40312/attachment.sig>