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

Hi, On Mon, Feb 29, 2016 at 2:47 PM, Wan-Teh Chang wrote: > On Fri, Feb 26, 2016 at 5:04 PM, Ronald S. Bultje > wrote: > > > > To be a little bit more explicit, it should be relatively easy to > > accomplish this by changing the position of qscale in the relevant > struct, > > and only copy tha

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

On Mon, Feb 29, 2016 at 11:47:14AM -0800, Wan-Teh Chang wrote: > On Fri, Feb 26, 2016 at 5:04 PM, Ronald S. Bultje wrote: > > > > To be a little bit more explicit, it should be relatively easy to > > accomplish this by changing the position of qscale in the relevant struct, > > and only copy that

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

On Fri, Feb 26, 2016 at 5:04 PM, Ronald S. Bultje wrote: > > To be a little bit more explicit, it should be relatively easy to > accomplish this by changing the position of qscale in the relevant struct, > and only copy that half of the struct (where entries are obviously grouped) > where we actua

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

Hi James, On Thu, Feb 25, 2016 at 6:50 PM, James Almer wrote: > > Did you try running the FATE suite using threadsanitizer, or just decoded > random > videos? I remember like half the tests were failing because of data races and > most of them pointed to code changed by your patches. It would be

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

Hi, On Fri, Feb 26, 2016 at 5:49 PM, Ronald S. Bultje wrote: > Now, one could argue that maybe we shouldn't copy the value *at all* if we > don't use it anyway, which would incidentally also make tools like tsan > happy about this particular thingy. I'm not against that, if it doesn't > make the

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

Hi, On Fri, Feb 26, 2016 at 5:35 PM, Wan-Teh Chang wrote: > The data race is reported on the |qscale| member of MpegEncContext. sl->qscale is unconditionally assigned in ff_h264_decode_slice_header(), which run at the start of each frame. Therefore, qscale is (B), not (A) (it may be copied, bu

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

On Fri, Feb 26, 2016 at 1:17 PM, Ronald S. Bultje wrote: > > I'm happy to help out if you tell me which field/member tsan is complaining > about. Hi Ronald, I am using an old version of ffmpeg. Here is the ThreadSanitizer warning on the data race and the relevant source code from that ffmpeg sou

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

Hi, On Fri, Feb 26, 2016 at 4:12 PM, Wan-Teh Chang wrote: > On Fri, Feb 26, 2016 at 12:44 PM, Ronald S. Bultje > wrote: > > Hi, > > > > On Fri, Feb 26, 2016 at 3:26 PM, Ronald S. Bultje > > wrote: > > > >> > >> So doesn't this patch remove all concurrency by making the decode() of > >> thread

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

On Fri, Feb 26, 2016 at 12:44 PM, Ronald S. Bultje wrote: > Hi, > > On Fri, Feb 26, 2016 at 3:26 PM, Ronald S. Bultje > 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 th

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

Hi, On Fri, Feb 26, 2016 at 3:26 PM, Ronald S. Bultje wrote: > Hi, > > On Thu, Feb 25, 2016 at 9:32 PM, Wan-Teh Chang < > wtc-at-google@ffmpeg.org> wrote: > >> This is because the codec->decode() call in frame_worker_thread may be >> modifying that avctx at the same time. This data race is r

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

Hi, On Thu, Feb 25, 2016 at 9:32 PM, Wan-Teh Chang wrote: > This is because the codec->decode() call in frame_worker_thread may be > modifying that avctx at the same time. This data race is reported by > ThreadSanitizer. > > Although submit_thread holds two locks simultaneously, it always > acqu

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

On 2/25/2016 11:32 PM, Wan-Teh Chang wrote: > This is because the codec->decode() call in frame_worker_thread may be > modifying that avctx at the same time. This data race is reported by > ThreadSanitizer. > > Although submit_thread holds two locks simultaneously, it always > acquires them in the

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

This is because the codec->decode() call in frame_worker_thread may be modifying that avctx at the same time. This data race is reported by ThreadSanitizer. Although submit_thread holds two locks simultaneously, it always acquires them in the same order because |prev_thread| is always the array el