[FFmpeg-devel] tsan warning about a data race in libavcodec/h264_direct.c

[Ronald S. Bultje]

Hi Wan-Teh,

On Wed, Jul 19, 2017 at 2:31 PM, Wan-Teh Chang <wtc-at-google.com at ffmpeg.org
> wrote:

> Hi Ronald,
>
> Thank you for the reply.
>
> On Wed, Jul 19, 2017 at 8:56 AM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
> > Hi,
> >
> > On Wed, Jul 19, 2017 at 12:26 AM, Wan-Teh Chang <
> > wtc-at-google.com at ffmpeg.org> wrote:
> >
> >> WARNING: ThreadSanitizer: data race (pid=116081)
> >>   Read of size 4 at 0x7b7000020118 by thread T3 (mutexes: write M2239):
> >>     #0 pred_temp_direct_motion ffmpeg/libavcodec/h264_direct.c:505:9
> >> (ffmpeg+0x1159c60)
> >>
> > [..]
> >
> >>   Previous write of size 4 at 0x7b7000020118 by thread T1 (mutexes:
> >> write M2234):
> >>     #0 ff_h264_decode_mb_cabac ffmpeg/libavcodec/h264_cabac.c:2386:31
> >> (ffmpeg+0x1475e0a)
> >
> > I believe this is temporal motion vector prediction (so MV from frame 1
> is
> > used as a MV predictor in frame 2). The easy solution here is to wait for
> > the frame to have completed reconstruction of that block (which fills in
> > that mb_type) before accessing the predictor.
>
> In libavcodec/h264_direct.c, there is already an
> await_reference_mb_row() call before the read of
> sl->ref_list[1][0].parent->mb_type[mb_xy] at line 505:
>
> 487 static void pred_temp_direct_motion(const H264Context *const h,
> H264SliceCon    text *sl,
> 488                                     int *mb_type)
> 489 {
> ...
> 501
> 502     await_reference_mb_row(h, &sl->ref_list[1][0],
> 503                            sl->mb_y + !!IS_INTERLACED(*mb_type));
> 504
> 505     if (IS_INTERLACED(sl->ref_list[1][0].parent->mb_type[mb_xy]))
> { // AFL/A    FR/FR/FL -> AFL/FL
>
> This seems like the wait you suggested, but I guess it is not?


Yes, but clearly it's not doing the correct thing. :-). The ""fun"" in
these type of issues is to figure out why not. ;-).

Ronald