Re: [FFmpeg-devel] [PATCH v3 3/3] error_resilience: remove avpriv_atomic usage

2017-11-26 Thread Michael Niedermayer
On Sat, Nov 25, 2017 at 05:01:57PM +, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov 
> ---
>  libavcodec/error_resilience.c | 20 ++--
>  libavcodec/error_resilience.h |  3 ++-
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
> index 0c7f29d171..8f172beca6 100644
> --- a/libavcodec/error_resilience.c
> +++ b/libavcodec/error_resilience.c
> @@ -807,7 +807,7 @@ void ff_er_frame_start(ERContext *s)
>  
>  memset(s->error_status_table, ER_MB_ERROR | VP_START | ER_MB_END,
> s->mb_stride * s->mb_height * sizeof(uint8_t));
> -s->error_count= 3 * s->mb_num;
> +atomic_init(&s->error_count, 3 * s->mb_num);
>  s->error_occurred = 0;
>  }
>  
> @@ -852,20 +852,20 @@ void ff_er_add_slice(ERContext *s, int startx, int 
> starty,
>  mask &= ~VP_START;
>  if (status & (ER_AC_ERROR | ER_AC_END)) {
>  mask   &= ~(ER_AC_ERROR | ER_AC_END);
> -avpriv_atomic_int_add_and_fetch(&s->error_count, start_i - end_i - 
> 1);
> +atomic_fetch_add(&s->error_count, start_i - end_i - 1);
>  }
>  if (status & (ER_DC_ERROR | ER_DC_END)) {
>  mask   &= ~(ER_DC_ERROR | ER_DC_END);
> -avpriv_atomic_int_add_and_fetch(&s->error_count, start_i - end_i - 
> 1);
> +atomic_fetch_add(&s->error_count, start_i - end_i - 1);
>  }
>  if (status & (ER_MV_ERROR | ER_MV_END)) {
>  mask   &= ~(ER_MV_ERROR | ER_MV_END);
> -avpriv_atomic_int_add_and_fetch(&s->error_count, start_i - end_i - 
> 1);
> +atomic_fetch_add(&s->error_count, start_i - end_i - 1);
>  }
>  
>  if (status & ER_MB_ERROR) {
>  s->error_occurred = 1;
> -avpriv_atomic_int_set(&s->error_count, INT_MAX);
> +atomic_store(&s->error_count, INT_MAX);
>  }
>  
>  if (mask == ~0x7F) {
> @@ -878,7 +878,7 @@ void ff_er_add_slice(ERContext *s, int startx, int starty,
>  }
>  
>  if (end_i == s->mb_num)
> -avpriv_atomic_int_set(&s->error_count, INT_MAX);
> +atomic_store(&s->error_count, INT_MAX);
>  else {
>  s->error_status_table[end_xy] &= mask;
>  s->error_status_table[end_xy] |= status;
> @@ -893,7 +893,7 @@ void ff_er_add_slice(ERContext *s, int startx, int starty,
>  prev_status &= ~ VP_START;
>  if (prev_status != (ER_MV_END | ER_DC_END | ER_AC_END)) {
>  s->error_occurred = 1;
> -avpriv_atomic_int_set(&s->error_count, INT_MAX);
> +atomic_store(&s->error_count, INT_MAX);
>  }
>  }
>  }
> @@ -910,10 +910,10 @@ void ff_er_frame_end(ERContext *s)
>  
>  /* We do not support ER of field pictures yet,
>   * though it should not crash if enabled. */
> -if (!s->avctx->error_concealment || s->error_count == 0||
> +if (!s->avctx->error_concealment || !atomic_load(&s->error_count)  ||
>  s->avctx->lowres   ||
>  !er_supported(s)   ||
> -s->error_count == 3 * s->mb_width *
> +atomic_load(&s->error_count) == 3 * s->mb_width *
>(s->avctx->skip_top + s->avctx->skip_bottom)) {
>  return;
>  }
> @@ -927,7 +927,7 @@ void ff_er_frame_end(ERContext *s)
>  if (   mb_x == s->mb_width
>  && s->avctx->codec_id == AV_CODEC_ID_MPEG2VIDEO
>  && (FFALIGN(s->avctx->height, 16)&16)
> -&& s->error_count == 3 * s->mb_width * (s->avctx->skip_top + 
> s->avctx->skip_bottom + 1)
> +&& atomic_load(&s->error_count) == 3 * s->mb_width * 
> (s->avctx->skip_top + s->avctx->skip_bottom + 1)
>  ) {
>  av_log(s->avctx, AV_LOG_DEBUG, "ignoring last missing slice\n");
>  return;

looking at this again , I suspect these can use some more
lax memory ordering

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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras


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


Re: [FFmpeg-devel] [PATCH v3 3/3] error_resilience: remove avpriv_atomic usage

2017-11-25 Thread Rostislav Pehlivanov
On 26 November 2017 at 01:58, Michael Niedermayer 
wrote:

> On Sat, Nov 25, 2017 at 05:01:57PM +, Rostislav Pehlivanov wrote:
> > Signed-off-by: Rostislav Pehlivanov 
> > ---
> >  libavcodec/error_resilience.c | 20 ++--
> >  libavcodec/error_resilience.h |  3 ++-
> >  2 files changed, 12 insertions(+), 11 deletions(-)
>
> LGTM
>
> thx
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I have never wished to cater to the crowd; for what I know they do not
> approve, and what they approve I do not know. -- Epicurus
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
pushed, thanks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v3 3/3] error_resilience: remove avpriv_atomic usage

2017-11-25 Thread Michael Niedermayer
On Sat, Nov 25, 2017 at 05:01:57PM +, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov 
> ---
>  libavcodec/error_resilience.c | 20 ++--
>  libavcodec/error_resilience.h |  3 ++-
>  2 files changed, 12 insertions(+), 11 deletions(-)

LGTM

thx

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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


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


[FFmpeg-devel] [PATCH v3 3/3] error_resilience: remove avpriv_atomic usage

2017-11-25 Thread Rostislav Pehlivanov
Signed-off-by: Rostislav Pehlivanov 
---
 libavcodec/error_resilience.c | 20 ++--
 libavcodec/error_resilience.h |  3 ++-
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
index 0c7f29d171..8f172beca6 100644
--- a/libavcodec/error_resilience.c
+++ b/libavcodec/error_resilience.c
@@ -807,7 +807,7 @@ void ff_er_frame_start(ERContext *s)
 
 memset(s->error_status_table, ER_MB_ERROR | VP_START | ER_MB_END,
s->mb_stride * s->mb_height * sizeof(uint8_t));
-s->error_count= 3 * s->mb_num;
+atomic_init(&s->error_count, 3 * s->mb_num);
 s->error_occurred = 0;
 }
 
@@ -852,20 +852,20 @@ void ff_er_add_slice(ERContext *s, int startx, int starty,
 mask &= ~VP_START;
 if (status & (ER_AC_ERROR | ER_AC_END)) {
 mask   &= ~(ER_AC_ERROR | ER_AC_END);
-avpriv_atomic_int_add_and_fetch(&s->error_count, start_i - end_i - 1);
+atomic_fetch_add(&s->error_count, start_i - end_i - 1);
 }
 if (status & (ER_DC_ERROR | ER_DC_END)) {
 mask   &= ~(ER_DC_ERROR | ER_DC_END);
-avpriv_atomic_int_add_and_fetch(&s->error_count, start_i - end_i - 1);
+atomic_fetch_add(&s->error_count, start_i - end_i - 1);
 }
 if (status & (ER_MV_ERROR | ER_MV_END)) {
 mask   &= ~(ER_MV_ERROR | ER_MV_END);
-avpriv_atomic_int_add_and_fetch(&s->error_count, start_i - end_i - 1);
+atomic_fetch_add(&s->error_count, start_i - end_i - 1);
 }
 
 if (status & ER_MB_ERROR) {
 s->error_occurred = 1;
-avpriv_atomic_int_set(&s->error_count, INT_MAX);
+atomic_store(&s->error_count, INT_MAX);
 }
 
 if (mask == ~0x7F) {
@@ -878,7 +878,7 @@ void ff_er_add_slice(ERContext *s, int startx, int starty,
 }
 
 if (end_i == s->mb_num)
-avpriv_atomic_int_set(&s->error_count, INT_MAX);
+atomic_store(&s->error_count, INT_MAX);
 else {
 s->error_status_table[end_xy] &= mask;
 s->error_status_table[end_xy] |= status;
@@ -893,7 +893,7 @@ void ff_er_add_slice(ERContext *s, int startx, int starty,
 prev_status &= ~ VP_START;
 if (prev_status != (ER_MV_END | ER_DC_END | ER_AC_END)) {
 s->error_occurred = 1;
-avpriv_atomic_int_set(&s->error_count, INT_MAX);
+atomic_store(&s->error_count, INT_MAX);
 }
 }
 }
@@ -910,10 +910,10 @@ void ff_er_frame_end(ERContext *s)
 
 /* We do not support ER of field pictures yet,
  * though it should not crash if enabled. */
-if (!s->avctx->error_concealment || s->error_count == 0||
+if (!s->avctx->error_concealment || !atomic_load(&s->error_count)  ||
 s->avctx->lowres   ||
 !er_supported(s)   ||
-s->error_count == 3 * s->mb_width *
+atomic_load(&s->error_count) == 3 * s->mb_width *
   (s->avctx->skip_top + s->avctx->skip_bottom)) {
 return;
 }
@@ -927,7 +927,7 @@ void ff_er_frame_end(ERContext *s)
 if (   mb_x == s->mb_width
 && s->avctx->codec_id == AV_CODEC_ID_MPEG2VIDEO
 && (FFALIGN(s->avctx->height, 16)&16)
-&& s->error_count == 3 * s->mb_width * (s->avctx->skip_top + 
s->avctx->skip_bottom + 1)
+&& atomic_load(&s->error_count) == 3 * s->mb_width * 
(s->avctx->skip_top + s->avctx->skip_bottom + 1)
 ) {
 av_log(s->avctx, AV_LOG_DEBUG, "ignoring last missing slice\n");
 return;
diff --git a/libavcodec/error_resilience.h b/libavcodec/error_resilience.h
index 27c2008694..664a765659 100644
--- a/libavcodec/error_resilience.h
+++ b/libavcodec/error_resilience.h
@@ -20,6 +20,7 @@
 #define AVCODEC_ERROR_RESILIENCE_H
 
 #include 
+#include 
 
 #include "avcodec.h"
 #include "me_cmp.h"
@@ -60,7 +61,7 @@ typedef struct ERContext {
 ptrdiff_t mb_stride;
 ptrdiff_t b8_stride;
 
-volatile int error_count;
+atomic_int error_count;
 int error_occurred;
 uint8_t *error_status_table;
 uint8_t *er_temp_buffer;
-- 
2.15.0.417.g466bffb3ac

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