[FFmpeg-devel] h264 threading fate tests

[Clément Bœsch]

On Tue, Oct 02, 2012 at 11:34:21AM +0200, Clément Bœsch wrote:
> On Mon, Oct 01, 2012 at 11:50:19AM +0200, Clément Bœsch wrote:
> > On Fri, Sep 28, 2012 at 11:07:54AM +0200, Clément Bœsch wrote:
> > > On Mon, Apr 16, 2012 at 04:43:42PM +0200, Clément Bœsch wrote:
> > > > Hi,
> > > > 
> > > > I recently setup a few fate instances to test the threading (2, 8, 16 and
> > > > auto), and regularly one of the h264 conformance test fails; look at the
> > > > yellow entries here for instance:
> > > > http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-threads-8
> > > > 
> > > > The other day, I tried to run an automated git bisect run, but
> > > > unfortunately testing the potential regression would requires to run
> > > > something like "while true; do make fate-h264 -j20 THREADS=8; done" for
> > > > around 15 minutes at least each time, and it might not even be reliable.
> > > > 
> > > > I'm not familiar at all with AVC decoding or threading in FFmpeg, but
> > > > maybe someone has an idea of what could cause this?
> > > > 
> > > > Maybe I should just open an issue in the trac?
> > > 
> > > OK so here is some more information about that race. Since the failure
> > > seems to be always the same (failing frame CRCs are identical), I dumped
> > > both outputs and made a diff. Here it is:
> > > 
> > >     http://lucy.pkh.me/race.html
> > > 
> > > Basically, we can notice some ±1 byte of difference at times. The
> > > generated outputs can be downloaded from that page.
> > > 
> > > Also, I was able the other day (purely by luck) to have a valgrind
> > > backtrace triggering the problem. The output was flooded, but here is a
> > > sample: http://pastie.org/4602183. BTW, it seems the test is using a frame
> > > based threading.
> > > 
> > > BTW, we observe only two failing tests: h264-conformance-cama2_vtc_b
> > > mostly, but at times h264-conformance-mr1_bt_a fails as well. The
> > > information I gave above are related to h264-conformance-cama2_vtc_b only.
> > > 
> > > Still anyone to look into this? :(
> > > 
> > 
> > More digging info: it seems the problem (or at least part of it) is around
> > H264Context->ref_count[2] (frame-based threading), especially when copying it
> > from the priv_data in lavc/h264.c:decode_update_thread_context():
> > 
> >     libavcodec/h264.c:1248:    copy_fields(h, h1, ref_count, list_count);
> > 
> 
> I have no idea about that specific race, but it looks like there is
> another one: simultaneous frame worker threads seems to call
> ff_init_cabac_states() (conflict around ff_h264_lps_range accesses).
> 
>   worker thread -> decode_frame -> decode_slice -> init cabac states
> 
> So maybe moving ff_init_cabac_states() only once in the init or something
> (even if not necessary) could help.
> 


The tables init by ff_init_cabac_states() seem to be used in read-only, so
I tried this:

diff --git a/libavcodec/h264.c b/libavcodec/h264.c
index fe3bac0..83bebfe 100644
--- a/libavcodec/h264.c
+++ b/libavcodec/h264.c
@@ -1113,6 +1113,8 @@ av_cold int ff_h264_decode_init(AVCodecContext *avctx)
         s->low_delay           = 0;
     }
 
+    ff_init_cabac_states();
+
     return 0;
 }
 
@@ -3535,7 +3537,6 @@ static int decode_slice(struct AVCodecContext *avctx, void 
         align_get_bits(&s->gb);
 
         /* init cabac */
-        ff_init_cabac_states();
         ff_init_cabac_decoder(&h->cabac,
                               s->gb.buffer + get_bits_count(&s->gb) / 8,
                               (get_bits_left(&s->gb) + 7) / 8);

Passes fate-h264 and seems to fix that specific h264/cabac race.

Now there is still a problem with the ref_count, and it looks like there might
be another one...

[...]

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