[FFmpeg-devel] [PATCH] frame_thread_encoder: make task indexing deterministic.

[Michael Niedermayer]

On Fri, Mar 31, 2017 at 10:20:43AM -0400, Ronald S. Bultje wrote:
> Fixes tsan warnings in fate-utvideoenc. Example warning from
> utvideoenc_rgb_left:
> 
> WARNING: ThreadSanitizer: data race (pid=19929)
>   Read of size 8 at 0x7d840001cf18 by main thread:
>     #0 ff_thread_video_encode_frame src/libavcodec/frame_thread_encoder.c:276 (ffmpeg+0x00000136a39d)
>     #1 avcodec_encode_video2 src/libavcodec/utils.c:1986 (ffmpeg+0x000000f34a18)
> [..]
>   Previous write of size 8 at 0x7d840001cf18 by thread T14 (mutexes: write M1348):
>     #0 worker src/libavcodec/frame_thread_encoder.c:100 (ffmpeg+0x000001369964)
> 

> We prevent that by using the same mechanism as frame-mt decoding, and
> assuming that we're encoding N packets in parallel (where N=n_threads),
> and thus the first N calls to encode_video() won't produce a packet.

The whole idea behind frame_thread_encoder was that the number of
threads and the delay could be changed at runtime
ive rechecked the original commit and it said
"Compared to the decoder side, this code is able to change both the
    delay and the number of threads seamlessly during encoding. Also
    any idle thread can pick up tasks, the strict round robin in order
    limit is gone too."

Making it like frame-mt decoding doesnt sound good to me.

Though the frame_thread_encoder was/is limited by the encoding API

about the patch itself, the changes done to indexing seem not to
change anything, it makes it possible for the variables to overflow
though.

IIUC the only change your patch does is to remove the outdata check
from the quoted warning

its a while ago that i worked n this code but isnt this just "missing"
a finished_task_mutex lock over the access ?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170401/a1eb6403/attachment.sig>