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

[Wan-Teh Chang]

On Fri, Feb 26, 2016 at 12:44 PM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
> Hi,
>
> On Fri, Feb 26, 2016 at 3:26 PM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
>
>>
>> So doesn't this patch remove all concurrency by making the decode() of
>> thread N-1 finish before decode() of thread N can start? More practically,
>> what is the output of time ffmpeg -threads N -i large-h264-file -f null -
>> with N being 1 and 2 before and after your patch?
>>
>
> I actually tested this, just for fun:
>
> before:
> $ for t in 1 2; do time ffmpeg -threads $t -i tos3k-vp9-b5000.webm -f null
> -v 0 -nostats -vframes 500 -; done
>
> real 0m6.732s
> user 0m6.643s
> sys 0m0.064s
>
> real 0m4.124s <<< note how this is significantly smaller than 6.732
> user 0m6.566s
> sys 0m0.114s
>
> after:
> $ for t in 1 2; do time ffmpeg -threads $t -i tos3k-vp9-b5000.webm -f null
> -v 0 -nostats -vframes 500 -; done
>
> real 0m6.808s
> user 0m6.541s
> sys 0m0.071s
>
> real 0m7.001s <<< note how this is no longer significantly smaller than
> 6.808
> user 0m6.945s
> sys 0m0.108s
>
> That seems like a pretty major malfunction of MT performance to me...

Hi Ronald,

Thank you very much for reviewing and testing my patch. I will study
your description of the code, analyze the data race reported by
ThreadSanitizer again, and report back.

Among the four data races I reported yesterday, this data race is the
most difficult to analyze because the code that has the data race is
outside libavcodec/pthread_frame.c, in the h264 decoder I remember.

Wan-Teh Chang