Re: [FFmpeg-devel] [PATCH v3] avcodec: fix atomics usage for h264/mpeg error_count

2018-03-14 Thread Michael Niedermayer 
On Wed, Mar 14, 2018 at 08:11:35AM +0100, wm4 wrote:
> On Tue, 13 Mar 2018 19:07:33 +0100
> Michael Niedermayer  wrote:
> 
> > On Tue, Mar 13, 2018 at 04:02:57PM +, Aman Gupta wrote:
> > > On Tue, Mar 13, 2018 at 4:39 AM Michael Niedermayer 
> > > 
> > > wrote:
> > >   
> > > > On Mon, Mar 12, 2018 at 06:49:19PM -0700, Aman Gupta wrote:  
> > > > > From: Aman Gupta 
> > > > >
> > > > > ---
> > > > >  libavcodec/h264_slice.c|  5 +++--
> > > > >  libavcodec/mpeg12dec.c | 12 +++-
> > > > >  libavcodec/mpegvideo_enc.c |  3 ++-
> > > > >  3 files changed, 12 insertions(+), 8 deletions(-)  
> > > >
> > > > Iam a little bit confused, you write about "fixing" but not what this is
> > > > fixing and it appears there are several distinct types of changes  
> > > 
> > > 
> > > The type of error_count is already atomic_int. I don't know when that was
> > > changed.
> > > 
> > > The fix here is that these files do not compile on FreeBSD because of
> > > errors such as:
> > > 
> > > [2018-03-11 18:26:55.078686] [freebsd-x86_64] libavcodec/mpegvideo_enc.c:
> > > In function 'merge_context_after_encode':
> > > 
> > > [2018-03-11 18:26:55.081607] [freebsd-x86_64]
> > > libavcodec/mpegvideo_enc.c:3578:33: error: invalid operands to binary +
> > > (have 'atomic_int' and 'atomic_int')
> > > 
> > > 
> > > [2018-03-11 16:43:23.543072] [freebsd-x86_64] CClibavcodec/mpeg12dec.o
> > > 
> > > [2018-03-11 16:43:23.768864] [freebsd-x86_64] libavcodec/mpeg12dec.c: In
> > > function 'slice_decode_thread':
> > >   
> > 
> > > [2018-03-11 16:43:23.769713] [freebsd-x86_64]
> > > libavcodec/mpeg12dec.c:1996:23: error: incompatible types when assigning 
> > > to
> > > type 'atomic_int' from type 'int'  
> > 
> > what compiler is this ?
> > 6.5.16.1 Simple assignment
> > Constraints
> > One of the following shall hold: 114)
> > — the left operand has atomic, qualified, or unqualified arithmetic 
> > type, and the right has
> > arithmetic type;
> > 
> > Please someone correct me here if iam wrong but
> > To me this sounds like doing normal arithmetic operations between atomic and
> > non atomic arithmetic types is allowed.
> 
> It's true that C11 atomics allow direct access, but we also have
> some atomic wrappers that won't work correctly then. I haven't checked
> why this one fails, but the gcc compat wrappers use non-atomic types for
> the emulation, so this patch is justified and necessary.

The wrappers should be fixed if they do not work.
If they cannot be fixed and cannot implement the C11 spec correctly they 
should not use the standard C11 types but choose differnt function and type
names. And document these limitations and differences in that new API


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] avcodec: fix atomics usage for h264/mpeg error_count

2018-03-14 Thread wm4 
On Tue, 13 Mar 2018 19:07:33 +0100
Michael Niedermayer  wrote:

> On Tue, Mar 13, 2018 at 04:02:57PM +, Aman Gupta wrote:
> > On Tue, Mar 13, 2018 at 4:39 AM Michael Niedermayer 
> > wrote:
> >   
> > > On Mon, Mar 12, 2018 at 06:49:19PM -0700, Aman Gupta wrote:  
> > > > From: Aman Gupta 
> > > >
> > > > ---
> > > >  libavcodec/h264_slice.c|  5 +++--
> > > >  libavcodec/mpeg12dec.c | 12 +++-
> > > >  libavcodec/mpegvideo_enc.c |  3 ++-
> > > >  3 files changed, 12 insertions(+), 8 deletions(-)  
> > >
> > > Iam a little bit confused, you write about "fixing" but not what this is
> > > fixing and it appears there are several distinct types of changes  
> > 
> > 
> > The type of error_count is already atomic_int. I don't know when that was
> > changed.
> > 
> > The fix here is that these files do not compile on FreeBSD because of
> > errors such as:
> > 
> > [2018-03-11 18:26:55.078686] [freebsd-x86_64] libavcodec/mpegvideo_enc.c:
> > In function 'merge_context_after_encode':
> > 
> > [2018-03-11 18:26:55.081607] [freebsd-x86_64]
> > libavcodec/mpegvideo_enc.c:3578:33: error: invalid operands to binary +
> > (have 'atomic_int' and 'atomic_int')
> > 
> > 
> > [2018-03-11 16:43:23.543072] [freebsd-x86_64] CClibavcodec/mpeg12dec.o
> > 
> > [2018-03-11 16:43:23.768864] [freebsd-x86_64] libavcodec/mpeg12dec.c: In
> > function 'slice_decode_thread':
> >   
> 
> > [2018-03-11 16:43:23.769713] [freebsd-x86_64]
> > libavcodec/mpeg12dec.c:1996:23: error: incompatible types when assigning to
> > type 'atomic_int' from type 'int'  
> 
> what compiler is this ?
> 6.5.16.1 Simple assignment
> Constraints
> One of the following shall hold: 114)
> — the left operand has atomic, qualified, or unqualified arithmetic type, 
> and the right has
> arithmetic type;
> 
> Please someone correct me here if iam wrong but
> To me this sounds like doing normal arithmetic operations between atomic and
> non atomic arithmetic types is allowed.

It's true that C11 atomics allow direct access, but we also have
some atomic wrappers that won't work correctly then. I haven't checked
why this one fails, but the gcc compat wrappers use non-atomic types for
the emulation, so this patch is justified and necessary.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] avcodec: fix atomics usage for h264/mpeg error_count

2018-03-13 Thread Aman Gupta 
On Tue, Mar 13, 2018 at 11:43 AM, Aman Gupta  wrote:

>
> On Tue, Mar 13, 2018 at 11:34 AM Michael Niedermayer
>  wrote:
>
>> On Tue, Mar 13, 2018 at 04:02:57PM +, Aman Gupta wrote:
>> > On Tue, Mar 13, 2018 at 4:39 AM Michael Niedermayer
>> 
>> > wrote:
>> >
>> > > On Mon, Mar 12, 2018 at 06:49:19PM -0700, Aman Gupta wrote:
>> > > > From: Aman Gupta 
>> > > >
>> > > > ---
>> > > >  libavcodec/h264_slice.c|  5 +++--
>> > > >  libavcodec/mpeg12dec.c | 12 +++-
>> > > >  libavcodec/mpegvideo_enc.c |  3 ++-
>> > > >  3 files changed, 12 insertions(+), 8 deletions(-)
>> > >
>> > > Iam a little bit confused, you write about "fixing" but not what this
>> is
>> > > fixing and it appears there are several distinct types of changes
>> >
>> >
>> > The type of error_count is already atomic_int. I don't know when that
>> was
>> > changed.
>> >
>> > The fix here is that these files do not compile on FreeBSD because of
>> > errors such as:
>> >
>> > [2018-03-11 18:26:55.078686] [freebsd-x86_64]
>> libavcodec/mpegvideo_enc.c:
>> > In function 'merge_context_after_encode':
>> >
>> > [2018-03-11 18:26:55.081607] [freebsd-x86_64]
>> > libavcodec/mpegvideo_enc.c:3578:33: error: invalid operands to binary +
>> > (have 'atomic_int' and 'atomic_int')
>> >
>> >
>> > [2018-03-11 16:43:23.543072] [freebsd-x86_64] CC
>> libavcodec/mpeg12dec.o
>> >
>> > [2018-03-11 16:43:23.768864] [freebsd-x86_64] libavcodec/mpeg12dec.c: In
>> > function 'slice_decode_thread':
>> >
>>
>> > [2018-03-11 16:43:23.769713] [freebsd-x86_64]
>> > libavcodec/mpeg12dec.c:1996:23: error: incompatible types when
>> assigning to
>> > type 'atomic_int' from type 'int'
>>
>> what compiler is this ?
>
>
> Target: x86_64-portbld-freebsd10.1
> gcc version 4.8.5 (FreeBSD Ports Collection)
>

I ran `pkg update` and `pkg install gcc` and it upgraded me to:

gcc version 6.4.0 (FreeBSD Ports Collection)

Now I don't see these errors anymore. So you can consider this patchset
abandoned.

Aman


>
>
>
>> 6.5.16.1 Simple assignment
>> Constraints
>> One of the following shall hold: 114)
>> — the left operand has atomic, qualified, or unqualified arithmetic
>> type, and the right has
>> arithmetic type;
>>
>> Please someone correct me here if iam wrong but
>> To me this sounds like doing normal arithmetic operations between atomic
>> and
>> non atomic arithmetic types is allowed.
>>
>>
>>
>> [...]
>>
>> --
>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Those who are best at talking, realize last or never when they are wrong.
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] avcodec: fix atomics usage for h264/mpeg error_count

2018-03-13 Thread Aman Gupta 
On Tue, Mar 13, 2018 at 11:34 AM Michael Niedermayer 
wrote:

> On Tue, Mar 13, 2018 at 04:02:57PM +, Aman Gupta wrote:
> > On Tue, Mar 13, 2018 at 4:39 AM Michael Niedermayer
> 
> > wrote:
> >
> > > On Mon, Mar 12, 2018 at 06:49:19PM -0700, Aman Gupta wrote:
> > > > From: Aman Gupta 
> > > >
> > > > ---
> > > >  libavcodec/h264_slice.c|  5 +++--
> > > >  libavcodec/mpeg12dec.c | 12 +++-
> > > >  libavcodec/mpegvideo_enc.c |  3 ++-
> > > >  3 files changed, 12 insertions(+), 8 deletions(-)
> > >
> > > Iam a little bit confused, you write about "fixing" but not what this
> is
> > > fixing and it appears there are several distinct types of changes
> >
> >
> > The type of error_count is already atomic_int. I don't know when that was
> > changed.
> >
> > The fix here is that these files do not compile on FreeBSD because of
> > errors such as:
> >
> > [2018-03-11 18:26:55.078686] [freebsd-x86_64] libavcodec/mpegvideo_enc.c:
> > In function 'merge_context_after_encode':
> >
> > [2018-03-11 18:26:55.081607] [freebsd-x86_64]
> > libavcodec/mpegvideo_enc.c:3578:33: error: invalid operands to binary +
> > (have 'atomic_int' and 'atomic_int')
> >
> >
> > [2018-03-11 16:43:23.543072] [freebsd-x86_64] CC
> libavcodec/mpeg12dec.o
> >
> > [2018-03-11 16:43:23.768864] [freebsd-x86_64] libavcodec/mpeg12dec.c: In
> > function 'slice_decode_thread':
> >
>
> > [2018-03-11 16:43:23.769713] [freebsd-x86_64]
> > libavcodec/mpeg12dec.c:1996:23: error: incompatible types when assigning
> to
> > type 'atomic_int' from type 'int'
>
> what compiler is this ?


Target: x86_64-portbld-freebsd10.1
gcc version 4.8.5 (FreeBSD Ports Collection)



> 6.5.16.1 Simple assignment
> Constraints
> One of the following shall hold: 114)
> — the left operand has atomic, qualified, or unqualified arithmetic
> type, and the right has
> arithmetic type;
>
> Please someone correct me here if iam wrong but
> To me this sounds like doing normal arithmetic operations between atomic
> and
> non atomic arithmetic types is allowed.
>
>
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who are best at talking, realize last or never when they are wrong.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] avcodec: fix atomics usage for h264/mpeg error_count

2018-03-13 Thread Michael Niedermayer 
On Tue, Mar 13, 2018 at 04:02:57PM +, Aman Gupta wrote:
> On Tue, Mar 13, 2018 at 4:39 AM Michael Niedermayer 
> wrote:
> 
> > On Mon, Mar 12, 2018 at 06:49:19PM -0700, Aman Gupta wrote:
> > > From: Aman Gupta 
> > >
> > > ---
> > >  libavcodec/h264_slice.c|  5 +++--
> > >  libavcodec/mpeg12dec.c | 12 +++-
> > >  libavcodec/mpegvideo_enc.c |  3 ++-
> > >  3 files changed, 12 insertions(+), 8 deletions(-)
> >
> > Iam a little bit confused, you write about "fixing" but not what this is
> > fixing and it appears there are several distinct types of changes
> 
> 
> The type of error_count is already atomic_int. I don't know when that was
> changed.
> 
> The fix here is that these files do not compile on FreeBSD because of
> errors such as:
> 
> [2018-03-11 18:26:55.078686] [freebsd-x86_64] libavcodec/mpegvideo_enc.c:
> In function 'merge_context_after_encode':
> 
> [2018-03-11 18:26:55.081607] [freebsd-x86_64]
> libavcodec/mpegvideo_enc.c:3578:33: error: invalid operands to binary +
> (have 'atomic_int' and 'atomic_int')
> 
> 
> [2018-03-11 16:43:23.543072] [freebsd-x86_64] CClibavcodec/mpeg12dec.o
> 
> [2018-03-11 16:43:23.768864] [freebsd-x86_64] libavcodec/mpeg12dec.c: In
> function 'slice_decode_thread':
> 

> [2018-03-11 16:43:23.769713] [freebsd-x86_64]
> libavcodec/mpeg12dec.c:1996:23: error: incompatible types when assigning to
> type 'atomic_int' from type 'int'

what compiler is this ?
6.5.16.1 Simple assignment
Constraints
One of the following shall hold: 114)
— the left operand has atomic, qualified, or unqualified arithmetic type, 
and the right has
arithmetic type;

Please someone correct me here if iam wrong but
To me this sounds like doing normal arithmetic operations between atomic and
non atomic arithmetic types is allowed.



[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are wrong.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] avcodec: fix atomics usage for h264/mpeg error_count

2018-03-13 Thread Aman Gupta 
On Tue, Mar 13, 2018 at 4:39 AM Michael Niedermayer 
wrote:

> On Mon, Mar 12, 2018 at 06:49:19PM -0700, Aman Gupta wrote:
> > From: Aman Gupta 
> >
> > ---
> >  libavcodec/h264_slice.c|  5 +++--
> >  libavcodec/mpeg12dec.c | 12 +++-
> >  libavcodec/mpegvideo_enc.c |  3 ++-
> >  3 files changed, 12 insertions(+), 8 deletions(-)
>
> Iam a little bit confused, you write about "fixing" but not what this is
> fixing and it appears there are several distinct types of changes


The type of error_count is already atomic_int. I don't know when that was
changed.

The fix here is that these files do not compile on FreeBSD because of
errors such as:

[2018-03-11 18:26:55.078686] [freebsd-x86_64] libavcodec/mpegvideo_enc.c:
In function 'merge_context_after_encode':

[2018-03-11 18:26:55.081607] [freebsd-x86_64]
libavcodec/mpegvideo_enc.c:3578:33: error: invalid operands to binary +
(have 'atomic_int' and 'atomic_int')


[2018-03-11 16:43:23.543072] [freebsd-x86_64] CClibavcodec/mpeg12dec.o

[2018-03-11 16:43:23.768864] [freebsd-x86_64] libavcodec/mpeg12dec.c: In
function 'slice_decode_thread':

[2018-03-11 16:43:23.769713] [freebsd-x86_64]
libavcodec/mpeg12dec.c:1996:23: error: incompatible types when assigning to
type 'atomic_int' from type 'int'

[2018-03-11 16:43:23.769808] [freebsd-x86_64]  s->er.error_count = (3 *
(s->end_mb_y - s->start_mb_y) * s->mb_width) >> field_pic;


[2018-03-11 15:40:00.186073] [freebsd-x86_64] CClibavcodec/h264_slice.o

[2018-03-11 15:40:00.335377] [freebsd-x86_64] libavcodec/h264_slice.c: In
function 'ff_h264_execute_decode_slices':

[2018-03-11 15:40:00.336554] [freebsd-x86_64]
libavcodec/h264_slice.c:2765:36: error: incompatible types when assigning
to type 'atomic_int' from type 'int'

[2018-03-11 15:40:00.336639] [freebsd-x86_64]
sl->er.error_count
= 0;

Aman


>
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Asymptotically faster algorithms should always be preferred if you have
> asymptotical amounts of data
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] avcodec: fix atomics usage for h264/mpeg error_count

2018-03-13 Thread Michael Niedermayer 
On Mon, Mar 12, 2018 at 06:49:19PM -0700, Aman Gupta wrote:
> From: Aman Gupta 
> 
> ---
>  libavcodec/h264_slice.c|  5 +++--
>  libavcodec/mpeg12dec.c | 12 +++-
>  libavcodec/mpegvideo_enc.c |  3 ++-
>  3 files changed, 12 insertions(+), 8 deletions(-)

Iam a little bit confused, you write about "fixing" but not what this is
fixing and it appears there are several distinct types of changes


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v3] avcodec: fix atomics usage for h264/mpeg error_count

2018-03-13 Thread Aman Gupta 
On Mon, Mar 12, 2018 at 6:49 PM, Aman Gupta  wrote:

> From: Aman Gupta 
>
> ---
>  libavcodec/h264_slice.c|  5 +++--
>  libavcodec/mpeg12dec.c | 12 +++-
>  libavcodec/mpegvideo_enc.c |  3 ++-
>  3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index 90e05ed8f1..b381397b4d 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -2762,7 +2762,7 @@ int ff_h264_execute_decode_slices(H264Context *h)
>
>  sl = >slice_ctx[i];
>  if (CONFIG_ERROR_RESILIENCE) {
> -sl->er.error_count = 0;
> +sl->er.error_count = ATOMIC_VAR_INIT(0);
>  }
>
>  /* make sure none of those slices overlap */
> @@ -2786,7 +2786,8 @@ int ff_h264_execute_decode_slices(H264Context *h)
>  h->mb_y  = sl->mb_y;
>  if (CONFIG_ERROR_RESILIENCE) {
>  for (i = 1; i < context_count; i++)
> -h->slice_ctx[0].er.error_count +=
> h->slice_ctx[i].er.error_count;
> +atomic_fetch_add(>slice_ctx[0].er.error_count,
> + atomic_load(>slice_ctx[i].
> er.error_count));
>  }
>
>  if (h->postpone_filter) {
> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index 9e076e89da..5a8570672c 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -1992,7 +1992,7 @@ static int slice_decode_thread(AVCodecContext *c,
> void *arg)
>  int mb_y= s->start_mb_y;
>  const int field_pic = s->picture_structure != PICT_FRAME;
>
> -s->er.error_count = (3 * (s->end_mb_y - s->start_mb_y) * s->mb_width)
> >> field_pic;
> +s->er.error_count = ATOMIC_VAR_INIT((3 * (s->end_mb_y -
> s->start_mb_y) * s->mb_width) >> field_pic);
>
>  for (;;) {
>  uint32_t start_code;
> @@ -2002,7 +2002,7 @@ static int slice_decode_thread(AVCodecContext *c,
> void *arg)
>  emms_c();
>  ff_dlog(c, "ret:%d resync:%d/%d mb:%d/%d ts:%d/%d ec:%d\n",
>  ret, s->resync_mb_x, s->resync_mb_y, s->mb_x, s->mb_y,
> -s->start_mb_y, s->end_mb_y, s->er.error_count);
> +s->start_mb_y, s->end_mb_y, atomic_load(>er.error_
> count));
>  if (ret < 0) {
>  if (c->err_recognition & AV_EF_EXPLODE)
>  return ret;
> @@ -2485,7 +2485,8 @@ static int decode_chunks(AVCodecContext *avctx,
> AVFrame *picture,
> >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;
> +atomic_fetch_add(>er.error_count,
> + atomic_load(>thread_
> context[i]->er.error_count));
>  }
>
>  ret = slice_end(avctx, picture);
> @@ -2499,7 +2500,7 @@ static int decode_chunks(AVCodecContext *avctx,
> AVFrame *picture,
>  }
>  s2->pict_type = 0;
>
> -if (avctx->err_recognition & AV_EF_EXPLODE &&
> s2->er.error_count)
> +if (avctx->err_recognition & AV_EF_EXPLODE &&
> atomic_load(>er.error_count))
>  return AVERROR_INVALIDDATA;
>
>  return FFMAX(0, buf_ptr - buf - s2->parse_context.last_index);
> @@ -2553,7 +2554,8 @@ static int decode_chunks(AVCodecContext *avctx,
> AVFrame *picture,
> s2->thread_context, 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;
> +atomic_fetch_add(>er.error_count,
> + atomic_load(>thread_
> context[i]->er.error_count));
>  s->slice_count = 0;
>  }
>  if (last_code == 0 || last_code == SLICE_MIN_START_CODE) {
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index 979e138b88..3447c5e87a 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -3595,7 +3595,8 @@ static void merge_context_after_encode(MpegEncContext
> *dst, MpegEncContext *src)
>  MERGE(b_count);
>  MERGE(skip_count);
>  MERGE(misc_bits);
> -MERGE(er.error_count);
> +atomic_fetch_add(>er.error_count, atomic_load(>er.error_
> count));
> +atomic_store(>er.error_count, 0);
>  MERGE(padding_bug_score);
>  MERGE(current_picture.encoding_error[0]);
>  MERGE(current_picture.encoding_error[1]);
>

There was some uncertainty on IRC about whether the usages of
atomic_store() and ATOMIC_VAR_INIT() in this patch were correct, or needed
to be swapped.

Would appreciate if someone familiar with the error

[FFmpeg-devel] [PATCH v3] avcodec: fix atomics usage for h264/mpeg error_count

2018-03-12 Thread Aman Gupta 
From: Aman Gupta 

---
 libavcodec/h264_slice.c|  5 +++--
 libavcodec/mpeg12dec.c | 12 +++-
 libavcodec/mpegvideo_enc.c |  3 ++-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 90e05ed8f1..b381397b4d 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -2762,7 +2762,7 @@ int ff_h264_execute_decode_slices(H264Context *h)
 
 sl = >slice_ctx[i];
 if (CONFIG_ERROR_RESILIENCE) {
-sl->er.error_count = 0;
+sl->er.error_count = ATOMIC_VAR_INIT(0);
 }
 
 /* make sure none of those slices overlap */
@@ -2786,7 +2786,8 @@ int ff_h264_execute_decode_slices(H264Context *h)
 h->mb_y  = sl->mb_y;
 if (CONFIG_ERROR_RESILIENCE) {
 for (i = 1; i < context_count; i++)
-h->slice_ctx[0].er.error_count += 
h->slice_ctx[i].er.error_count;
+atomic_fetch_add(>slice_ctx[0].er.error_count,
+ atomic_load(>slice_ctx[i].er.error_count));
 }
 
 if (h->postpone_filter) {
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 9e076e89da..5a8570672c 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1992,7 +1992,7 @@ static int slice_decode_thread(AVCodecContext *c, void 
*arg)
 int mb_y= s->start_mb_y;
 const int field_pic = s->picture_structure != PICT_FRAME;
 
-s->er.error_count = (3 * (s->end_mb_y - s->start_mb_y) * s->mb_width) >> 
field_pic;
+s->er.error_count = ATOMIC_VAR_INIT((3 * (s->end_mb_y - s->start_mb_y) * 
s->mb_width) >> field_pic);
 
 for (;;) {
 uint32_t start_code;
@@ -2002,7 +2002,7 @@ static int slice_decode_thread(AVCodecContext *c, void 
*arg)
 emms_c();
 ff_dlog(c, "ret:%d resync:%d/%d mb:%d/%d ts:%d/%d ec:%d\n",
 ret, s->resync_mb_x, s->resync_mb_y, s->mb_x, s->mb_y,
-s->start_mb_y, s->end_mb_y, s->er.error_count);
+s->start_mb_y, s->end_mb_y, atomic_load(>er.error_count));
 if (ret < 0) {
 if (c->err_recognition & AV_EF_EXPLODE)
 return ret;
@@ -2485,7 +2485,8 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame 
*picture,
>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;
+atomic_fetch_add(>er.error_count,
+ 
atomic_load(>thread_context[i]->er.error_count));
 }
 
 ret = slice_end(avctx, picture);
@@ -2499,7 +2500,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame 
*picture,
 }
 s2->pict_type = 0;
 
-if (avctx->err_recognition & AV_EF_EXPLODE && s2->er.error_count)
+if (avctx->err_recognition & AV_EF_EXPLODE && 
atomic_load(>er.error_count))
 return AVERROR_INVALIDDATA;
 
 return FFMAX(0, buf_ptr - buf - s2->parse_context.last_index);
@@ -2553,7 +2554,8 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame 
*picture,
s2->thread_context, 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;
+atomic_fetch_add(>er.error_count,
+ 
atomic_load(>thread_context[i]->er.error_count));
 s->slice_count = 0;
 }
 if (last_code == 0 || last_code == SLICE_MIN_START_CODE) {
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 979e138b88..3447c5e87a 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -3595,7 +3595,8 @@ static void merge_context_after_encode(MpegEncContext 
*dst, MpegEncContext *src)
 MERGE(b_count);
 MERGE(skip_count);
 MERGE(misc_bits);
-MERGE(er.error_count);
+atomic_fetch_add(>er.error_count, atomic_load(>er.error_count));
+atomic_store(>er.error_count, 0);
 MERGE(padding_bug_score);
 MERGE(current_picture.encoding_error[0]);
 MERGE(current_picture.encoding_error[1]);
-- 
2.14.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel