[FFmpeg-devel] [PATCH] Fix threaded mpeg*video encoding

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Thu, Jul 01, 2010 at 03:48:23PM +0100, M?ns Rullg?rd wrote:
>> M?ns Rullg?rd <mans at mansr.com> writes:
>> 
>> > Michael Niedermayer <michaelni at gmx.at> writes:
>> >
>> >> On Thu, Jul 01, 2010 at 12:26:25PM +0100, M?ns Rullg?rd wrote:
>> >>> Michael Niedermayer <michaelni at gmx.at> writes:
>> >>> 
>> >>> > On Thu, Jul 01, 2010 at 04:01:24AM +0100, M?ns Rullg?rd wrote:
>> >>> >> Michael Niedermayer <michaelni at gmx.at> writes:
>> >>> >> 
>> >>> >> > On Wed, Jun 30, 2010 at 03:49:34PM +0100, Mans Rullgard wrote:
>> >>> >> >> This allocates per-thread copies of some buffers which are updated
>> >>> >> >> concurrently from the encoding threads.
>> >>> >> >
>> >>> >> > is this fixing a bug?
>> >>> >> > if so please elaborate what and where. My guess would be toward
>> >>> >> > ac_val being the one causing the issues.
>> >>> >> 
>> >>> >> The threaded regression tests are failing randomly with pthreads enabled:
>> >>> >> 
>> >>> >> http://fate.multimedia.cx/index.php?test_result=66882641
>> >>> >> http://fate.multimedia.cx/index.php?test_result=66852547
>> >>> >> http://fate.multimedia.cx/index.php?test_result=66876769
>> >>> >> 
>> >>> >> Those are just a few examples.  Valgrind complained about most of
>> >>> >> those buffers, particularly ac_val and dc_val, and I add a couple
>> >>> >> since they were written close to the places valgrind identified.  Race
>> >>> >> detection is never exact.
>> >>> >
>> >>> > could you try to just change ac_val and see if this makes the issue
>> >>> > go away?
>> >>> > for ac_val i found ff_mpeg4_clean_buffers() as possible reason for the
>> >>> > race, for the others i am not aware of code that can lead to a race
>> >>> 
>> >>> So OK to commit just the ac_val parts and see what happens?
>> >>
>> >> ok
>> >
>> > Done.  Let's keep an eye on FATE.
>> 
>> We still have some failures:
>> 
>> http://fate.multimedia.cx/index.php?build_record=251859
>> http://fate.multimedia.cx/index.php?build_record=251951
>> http://fate.multimedia.cx/index.php?build_record=251969
>
> have you confirmed that your whole patch is fixining them?

No.  It seems to happen much more frequently on BSD machines for some
reason.  Probably something to do with scheduling.  I've only managed
to reproduce it a couple of times on any of my systems.

Valgrind was explicitly complaining about both reads and writes in
dc_val.  I don't know how reliable its detection is.

> cbp_table and pred_dir_table seem decoder only

It's quite possible those are fine.

> coded_block stuff seems msmpeg4

Do we want msmpeg4 encoding to be broken?

-- 
M?ns Rullg?rd
mans at mansr.com