diff mbox

[FFmpeg-devel] huffyuv: assign correct per-thread avctx pointer to HYuvContext::avctx.

Message ID 1491272913-92233-1-git-send-email-rsbultje@gmail.com
State Accepted
Commit 7c7e7c44a6eb68eca861e45cb2ce78f582b12c69
Headers show

Commit Message

Ronald S. Bultje April 4, 2017, 2:28 a.m. UTC
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(+)

Comments

Michael Niedermayer April 5, 2017, 7:29 p.m. UTC | #1
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


[...]
Ronald S. Bultje April 5, 2017, 7:33 p.m. UTC | #2
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 mbox

Patch

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;