Message ID | 1491272913-92233-1-git-send-email-rsbultje@gmail.com |
---|---|
State | Accepted |
Commit | 7c7e7c44a6eb68eca861e45cb2ce78f582b12c69 |
Headers | show |
On Mon, Apr 03, 2017 at 10:28:33PM -0400, Ronald S. Bultje wrote: > Fixes the following tsan warning when running fate-vsynth_lena-ffvhuff: > > WARNING: ThreadSanitizer: data race (pid=6484) > Write of size 8 at 0x7d64000154b8 by main thread (mutexes: write M1331): > #0 update_context_from_user src/libavcodec/pthread_frame.c:331 (ffmpeg+0x000000dca887) > [..] > Previous read of size 8 at 0x7d64000154b8 by thread T2 (mutexes: write M1334): > #0 draw_slice src/libavcodec/huffyuvdec.c:857 (ffmpeg+0x000000bcc86f) > --- > libavcodec/huffyuvdec.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c > index 5572b98..979c4b9 100644 > --- a/libavcodec/huffyuvdec.c > +++ b/libavcodec/huffyuvdec.c > @@ -579,6 +579,8 @@ static av_cold int decode_init_thread_copy(AVCodecContext *avctx) > HYuvContext *s = avctx->priv_data; > int i, ret; > > + s->avctx = avctx; LGTM do you think its better in decode_init_thread_copy() or decode_frame() ? I think i would have instinctivly put it in decode_frame() but i dont see a issue with decode_init_thread_copy() thx [...]
Hi, On Wed, Apr 5, 2017 at 3:29 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Mon, Apr 03, 2017 at 10:28:33PM -0400, Ronald S. Bultje wrote: > > Fixes the following tsan warning when running fate-vsynth_lena-ffvhuff: > > > > WARNING: ThreadSanitizer: data race (pid=6484) > > Write of size 8 at 0x7d64000154b8 by main thread (mutexes: write > M1331): > > #0 update_context_from_user src/libavcodec/pthread_frame.c:331 > (ffmpeg+0x000000dca887) > > [..] > > Previous read of size 8 at 0x7d64000154b8 by thread T2 (mutexes: write > M1334): > > #0 draw_slice src/libavcodec/huffyuvdec.c:857 > (ffmpeg+0x000000bcc86f) > > --- > > libavcodec/huffyuvdec.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c > > index 5572b98..979c4b9 100644 > > --- a/libavcodec/huffyuvdec.c > > +++ b/libavcodec/huffyuvdec.c > > @@ -579,6 +579,8 @@ static av_cold int decode_init_thread_copy(AVCodecContext > *avctx) > > HYuvContext *s = avctx->priv_data; > > int i, ret; > > > > + s->avctx = avctx; > > LGTM > > do you think its better in decode_init_thread_copy() or decode_frame() ? > I think i would have instinctivly put it in decode_frame() but i dont > see a issue with decode_init_thread_copy() I see _init() as logically more correct since this data is supposed to be stable across multiple (all) frames. But in practice I don't think it would matter. Ronald
diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c index 5572b98..979c4b9 100644 --- a/libavcodec/huffyuvdec.c +++ b/libavcodec/huffyuvdec.c @@ -579,6 +579,8 @@ static av_cold int decode_init_thread_copy(AVCodecContext *avctx) HYuvContext *s = avctx->priv_data; int i, ret; + s->avctx = avctx; + if ((ret = ff_huffyuv_alloc_temp(s)) < 0) { ff_huffyuv_common_end(s); return ret;