On 11/17/2017 4:16 PM, Michael Niedermayer wrote: > 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 >> <mich...@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@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 <nlewy...@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.
Yes, he should use C11 atomics. It's been mentioned to everyone that submitted code using the old lavu wrappers, including you some days ago. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
