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.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel