diff mbox

[FFmpeg-devel,1/2] error_resilience: remove avpriv_atomic usage

Message ID 20171125003345.26015-1-atomnuker@gmail.com
State Superseded
Headers show

Commit Message

Rostislav Pehlivanov Nov. 25, 2017, 12:33 a.m. UTC 
Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com>
---
 libavcodec/error_resilience.c | 12 ++++++------
 libavcodec/error_resilience.h |  3 ++-
 2 files changed, 8 insertions(+), 7 deletions(-)

Comments

James Almer Nov. 25, 2017, 1:18 a.m. UTC | #1 
On 11/24/2017 9:33 PM, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com>
> ---
>  libavcodec/error_resilience.c | 12 ++++++------
>  libavcodec/error_resilience.h |  3 ++-
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
> index 0c7f29d171..6c7b4207af 100644
> --- a/libavcodec/error_resilience.c
> +++ b/libavcodec/error_resilience.c
> @@ -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);
>          }
>      }
>  }
> 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 <stdint.h>
> +#include <stdatomic.h>
>  
>  #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;
> 

This is missing the initialization of error_count in ff_er_frame_start().

I'm not sure if you can use atomic_init() there or should instead use
atomic_store(), as the former is supposedly not thread safe and i don't
know how and when ff_er_frame_start() is called.
Michael Niedermayer Nov. 25, 2017, 3:08 a.m. UTC | #2 
On Fri, Nov 24, 2017 at 10:18:06PM -0300, James Almer wrote:
> On 11/24/2017 9:33 PM, Rostislav Pehlivanov wrote:
> > Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com>
> > ---
> >  libavcodec/error_resilience.c | 12 ++++++------
> >  libavcodec/error_resilience.h |  3 ++-
> >  2 files changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
> > index 0c7f29d171..6c7b4207af 100644
> > --- a/libavcodec/error_resilience.c
> > +++ b/libavcodec/error_resilience.c
> > @@ -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);
> >          }
> >      }
> >  }
> > 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 <stdint.h>
> > +#include <stdatomic.h>
> >  
> >  #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;
> > 
> 
> This is missing the initialization of error_count in ff_er_frame_start().
> 
> I'm not sure if you can use atomic_init() there or should instead use
> atomic_store(), as the former is supposedly not thread safe and i don't
> know how and when ff_er_frame_start() is called.

the frame start / end stuff should not run in paralell with anything
else from the current frame

[...]
diff mbox

Patch

diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
index 0c7f29d171..6c7b4207af 100644
--- a/libavcodec/error_resilience.c
+++ b/libavcodec/error_resilience.c
@@ -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);
         }
     }
 }
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 <stdint.h>
+#include <stdatomic.h>
 
 #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;