[FFmpeg-devel] [PATCH] libavcodec/mpeg12dec: Use atomic addition for value updated in multiple threads.

[Michael Niedermayer]

On Thu, Nov 16, 2017 at 03:07:54PM -0800, Nick Lewycky wrote:
> Sorry! Let's try an attachment then.
> 
> On 16 November 2017 at 14:36, Michael Niedermayer
> <michael at niedermayer.cc> wrote:
> > On Thu, Nov 16, 2017 at 12:41:32PM -0800, Nick Lewycky wrote:
> >> I initially discovered a signed integer overflow on this line. Since
> >> this value is updated in multiple threads, I use an atomic update and
> >> as it happens atomic addition is defined to wrap around. However,
> >> there's still a potential bug in that the error_count may wrap around
> >> and equal zero again causing problems down the line.
> >>
> >> ---
> >>  libavcodec/mpeg12dec.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> >> index d5bc5f21b2..b7c3b5106e 100644
> >> --- a/libavcodec/mpeg12dec.c
> >> +++ b/libavcodec/mpeg12dec.c
> >> @@ -28,6 +28,7 @@
> >>  #define UNCHECKED_BITSTREAM_READER 1
> >>  #include <inttypes.h>
> >>
> >> +#include "libavutil/atomic.h"
> >>  #include "libavutil/attributes.h"
> >>  #include "libavutil/imgutils.h"
> >>  #include "libavutil/internal.h"
> >> @@ -2476,7 +2477,7 @@ static int decode_chunks(AVCodecContext *avctx,
> >> AVFrame *picture,
> >>                                     &s2->thread_context[0], NULL,
> >>                                     s->slice_count, sizeof(void *));
> >>                      for (i = 0; i < s->slice_count; i++)
> >> -                        s2->er.error_count +=
> >> s2->thread_context[i]->er.error_count;
> >> +
> >> avpriv_atomic_int_add_and_fetch(&s2->er.error_count,
> >> s2->thread_context[i]->er.error_count);
> >>                  }
> >
> > This patch is corrupted by newlines
> >
> > [...]
> >
> > --
> > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> >
> > Dictatorship naturally arises out of democracy, and the most aggravated
> > form of tyranny and slavery out of the most extreme liberty. -- Plato
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >

>  mpeg12dec.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> eed6baa2f9ae5b6fcd227e4b7fa0e87e9ca55b64  0001-libavcodec-mpeg12dec-Use-atomic-addition-for-value-u.patch
> From cd8ed0ee35853ae089df3d904846879ce4f00c4a Mon Sep 17 00:00:00 2001
> From: Nick Lewycky <nlewycky at google.com>
> Date: Thu, 16 Nov 2017 11:50:38 -0800
> Subject: [PATCH] libavcodec/mpeg12dec: Use atomic addition for value updated
>  in multiple threads.

LGTM, unless theres a new API for doing this, in which case the new
style should be used.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- 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/20171117/f1ea60d2/attachment.sig>