On Fri, Nov 17, 2017 at 05:10:17PM -0300, James Almer wrote: > On 11/17/2017 4:20 PM, James Almer wrote: > > 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. > > To expand on this, I'm aware that the entire error resilience feature > needs to be ported to C11 atomics. My point is that, much like i said to > you in that patch some days ago, it should be ported before adding new > code that will ultimately make the port harder.
Yes, i did not yet had time to update the patch [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Does the universe only have a finite lifespan? No, its going to go on forever, its just that you wont like living in it. -- Hiranya Peiri
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
