diff mbox

[FFmpeg-devel,1/7] avcodec/mediacodec: convert to stdatomic

Message ID 20170322233412.6952-2-jamrial@gmail.com
State Accepted
Headers show

Commit Message

James Almer March 22, 2017, 11:34 p.m. UTC 
Signed-off-by: James Almer <jamrial@gmail.com>
---
Untested.

 libavcodec/mediacodec.c           |  5 ++---
 libavcodec/mediacodecdec.c        |  1 -
 libavcodec/mediacodecdec_common.c | 14 ++++++--------
 libavcodec/mediacodecdec_common.h |  5 +++--
 4 files changed, 11 insertions(+), 14 deletions(-)

Comments

wm4 March 23, 2017, 6:19 a.m. UTC | #1 
On Wed, 22 Mar 2017 20:34:06 -0300
James Almer <jamrial@gmail.com> wrote:

> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> Untested.
> 
>  libavcodec/mediacodec.c           |  5 ++---
>  libavcodec/mediacodecdec.c        |  1 -
>  libavcodec/mediacodecdec_common.c | 14 ++++++--------
>  libavcodec/mediacodecdec_common.h |  5 +++--
>  4 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/libavcodec/mediacodec.c b/libavcodec/mediacodec.c
> index 4ad5921bc2..91f725621d 100644
> --- a/libavcodec/mediacodec.c
> +++ b/libavcodec/mediacodec.c
> @@ -31,7 +31,6 @@
>  #include <jni.h>
>  
>  #include "libavcodec/avcodec.h"
> -#include "libavutil/atomic.h"
>  #include "libavutil/mem.h"
>  
>  #include "ffjni.h"
> @@ -90,9 +89,9 @@ void av_mediacodec_default_free(AVCodecContext *avctx)
>  int av_mediacodec_release_buffer(AVMediaCodecBuffer *buffer, int render)
>  {
>      MediaCodecDecContext *ctx = buffer->ctx;
> -    int released = avpriv_atomic_int_add_and_fetch(&buffer->released, 1);
> +    atomic_int released = atomic_fetch_add(&buffer->released, 1);
>  
> -    if (released == 1) {
> +    if (!released) {

I think the return value of atomic_fetch_add is the native type, i.e.
int. With real C11 atomics it's allowed to access atomics like normal
variables (and the access will still be atomic), but I don't know if
all of our atomic emulations follow this.

>          return ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, render);
>      }
>  
> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
> index 2e645caafd..3ada3fa698 100644
> --- a/libavcodec/mediacodecdec.c
> +++ b/libavcodec/mediacodecdec.c
> @@ -29,7 +29,6 @@
>  #include "libavutil/opt.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/pixfmt.h"
> -#include "libavutil/atomic.h"
>  
>  #include "avcodec.h"
>  #include "h264_parse.h"
> diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c
> index dfc7f5514a..87f032876d 100644
> --- a/libavcodec/mediacodecdec_common.c
> +++ b/libavcodec/mediacodecdec_common.c
> @@ -23,7 +23,6 @@
>  #include <string.h>
>  #include <sys/types.h>
>  
> -#include "libavutil/atomic.h"
>  #include "libavutil/common.h"
>  #include "libavutil/mem.h"
>  #include "libavutil/log.h"
> @@ -143,7 +142,7 @@ static enum AVPixelFormat mcdec_map_color_format(AVCodecContext *avctx,
>  
>  static void ff_mediacodec_dec_ref(MediaCodecDecContext *s)
>  {
> -    avpriv_atomic_int_add_and_fetch(&s->refcount, 1);
> +    atomic_fetch_add(&s->refcount, 1);
>  }
>  
>  static void ff_mediacodec_dec_unref(MediaCodecDecContext *s)
> @@ -151,7 +150,7 @@ static void ff_mediacodec_dec_unref(MediaCodecDecContext *s)
>      if (!s)
>          return;
>  
> -    if (!avpriv_atomic_int_add_and_fetch(&s->refcount, -1)) {
> +    if (atomic_fetch_sub(&s->refcount, 1) == 1) {
>          if (s->codec) {
>              ff_AMediaCodec_delete(s->codec);
>              s->codec = NULL;
> @@ -176,9 +175,8 @@ static void mediacodec_buffer_release(void *opaque, uint8_t *data)
>  {
>      AVMediaCodecBuffer *buffer = opaque;
>      MediaCodecDecContext *ctx = buffer->ctx;
> -    int released = avpriv_atomic_int_get(&buffer->released);
>  
> -    if (!released) {
> +    if (!atomic_load(&buffer->released)) {
>          ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, 0);
>      }
>  
> @@ -221,7 +219,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          goto fail;
>      }
>  
> -    buffer->released = 0;
> +    atomic_init(&buffer->released, 0);
>  
>      frame->buf[0] = av_buffer_create(NULL,
>                                       0,
> @@ -465,7 +463,7 @@ int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s,
>          AV_PIX_FMT_NONE,
>      };
>  
> -    s->refcount = 1;
> +    atomic_init(&s->refcount, 1);
>  
>      pix_fmt = ff_get_format(avctx, pix_fmts);
>      if (pix_fmt == AV_PIX_FMT_MEDIACODEC) {
> @@ -725,7 +723,7 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
>  
>  int ff_mediacodec_dec_flush(AVCodecContext *avctx, MediaCodecDecContext *s)
>  {
> -    if (!s->surface || avpriv_atomic_int_get(&s->refcount) == 1) {
> +    if (!s->surface || atomic_load(&s->refcount) == 1) {
>          int ret;
>  
>          /* No frames (holding a reference to the codec) are retained by the
> diff --git a/libavcodec/mediacodecdec_common.h b/libavcodec/mediacodecdec_common.h
> index c00c2e89f3..10f38277b5 100644
> --- a/libavcodec/mediacodecdec_common.h
> +++ b/libavcodec/mediacodecdec_common.h
> @@ -24,6 +24,7 @@
>  #define AVCODEC_MEDIACODECDEC_COMMON_H
>  
>  #include <stdint.h>
> +#include <stdatomic.h>
>  #include <sys/types.h>
>  
>  #include "libavutil/frame.h"
> @@ -34,7 +35,7 @@
>  
>  typedef struct MediaCodecDecContext {
>  
> -    volatile int refcount;
> +    atomic_int refcount;
>  
>      char *codec_name;
>  
> @@ -88,7 +89,7 @@ typedef struct MediaCodecBuffer {
>      MediaCodecDecContext *ctx;
>      ssize_t index;
>      int64_t pts;
> -    volatile int released;
> +    atomic_int released;
>  
>  } MediaCodecBuffer;
>  

Rest looks ok.
James Almer March 23, 2017, 2:50 p.m. UTC | #2 
On 3/23/2017 3:19 AM, wm4 wrote:
> On Wed, 22 Mar 2017 20:34:06 -0300
> James Almer <jamrial@gmail.com> wrote:
> 
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> Untested.
>>
>>  libavcodec/mediacodec.c           |  5 ++---
>>  libavcodec/mediacodecdec.c        |  1 -
>>  libavcodec/mediacodecdec_common.c | 14 ++++++--------
>>  libavcodec/mediacodecdec_common.h |  5 +++--
>>  4 files changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/libavcodec/mediacodec.c b/libavcodec/mediacodec.c
>> index 4ad5921bc2..91f725621d 100644
>> --- a/libavcodec/mediacodec.c
>> +++ b/libavcodec/mediacodec.c
>> @@ -31,7 +31,6 @@
>>  #include <jni.h>
>>  
>>  #include "libavcodec/avcodec.h"
>> -#include "libavutil/atomic.h"
>>  #include "libavutil/mem.h"
>>  
>>  #include "ffjni.h"
>> @@ -90,9 +89,9 @@ void av_mediacodec_default_free(AVCodecContext *avctx)
>>  int av_mediacodec_release_buffer(AVMediaCodecBuffer *buffer, int render)
>>  {
>>      MediaCodecDecContext *ctx = buffer->ctx;
>> -    int released = avpriv_atomic_int_add_and_fetch(&buffer->released, 1);
>> +    atomic_int released = atomic_fetch_add(&buffer->released, 1);
>>  
>> -    if (released == 1) {
>> +    if (!released) {
> 
> I think the return value of atomic_fetch_add is the native type, i.e.
> int. With real C11 atomics it's allowed to access atomics like normal
> variables (and the access will still be atomic), but I don't know if
> all of our atomic emulations follow this.
> 

Actually, the return value isn't meant to be atomic. released should
remain int like in a similar call below in mediacodecdec_common.c

>>          return ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, render);
>>      }
>>  
>> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
>> index 2e645caafd..3ada3fa698 100644
>> --- a/libavcodec/mediacodecdec.c
>> +++ b/libavcodec/mediacodecdec.c
>> @@ -29,7 +29,6 @@
>>  #include "libavutil/opt.h"
>>  #include "libavutil/intreadwrite.h"
>>  #include "libavutil/pixfmt.h"
>> -#include "libavutil/atomic.h"
>>  
>>  #include "avcodec.h"
>>  #include "h264_parse.h"
>> diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c
>> index dfc7f5514a..87f032876d 100644
>> --- a/libavcodec/mediacodecdec_common.c
>> +++ b/libavcodec/mediacodecdec_common.c
>> @@ -23,7 +23,6 @@
>>  #include <string.h>
>>  #include <sys/types.h>
>>  
>> -#include "libavutil/atomic.h"
>>  #include "libavutil/common.h"
>>  #include "libavutil/mem.h"
>>  #include "libavutil/log.h"
>> @@ -143,7 +142,7 @@ static enum AVPixelFormat mcdec_map_color_format(AVCodecContext *avctx,
>>  
>>  static void ff_mediacodec_dec_ref(MediaCodecDecContext *s)
>>  {
>> -    avpriv_atomic_int_add_and_fetch(&s->refcount, 1);
>> +    atomic_fetch_add(&s->refcount, 1);
>>  }
>>  
>>  static void ff_mediacodec_dec_unref(MediaCodecDecContext *s)
>> @@ -151,7 +150,7 @@ static void ff_mediacodec_dec_unref(MediaCodecDecContext *s)
>>      if (!s)
>>          return;
>>  
>> -    if (!avpriv_atomic_int_add_and_fetch(&s->refcount, -1)) {
>> +    if (atomic_fetch_sub(&s->refcount, 1) == 1) {
>>          if (s->codec) {
>>              ff_AMediaCodec_delete(s->codec);
>>              s->codec = NULL;
>> @@ -176,9 +175,8 @@ static void mediacodec_buffer_release(void *opaque, uint8_t *data)
>>  {
>>      AVMediaCodecBuffer *buffer = opaque;
>>      MediaCodecDecContext *ctx = buffer->ctx;
>> -    int released = avpriv_atomic_int_get(&buffer->released);
>>  
>> -    if (!released) {
>> +    if (!atomic_load(&buffer->released)) {
>>          ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, 0);
>>      }
>>  
>> @@ -221,7 +219,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>          goto fail;
>>      }
>>  
>> -    buffer->released = 0;
>> +    atomic_init(&buffer->released, 0);
>>  
>>      frame->buf[0] = av_buffer_create(NULL,
>>                                       0,
>> @@ -465,7 +463,7 @@ int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s,
>>          AV_PIX_FMT_NONE,
>>      };
>>  
>> -    s->refcount = 1;
>> +    atomic_init(&s->refcount, 1);
>>  
>>      pix_fmt = ff_get_format(avctx, pix_fmts);
>>      if (pix_fmt == AV_PIX_FMT_MEDIACODEC) {
>> @@ -725,7 +723,7 @@ int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
>>  
>>  int ff_mediacodec_dec_flush(AVCodecContext *avctx, MediaCodecDecContext *s)
>>  {
>> -    if (!s->surface || avpriv_atomic_int_get(&s->refcount) == 1) {
>> +    if (!s->surface || atomic_load(&s->refcount) == 1) {
>>          int ret;
>>  
>>          /* No frames (holding a reference to the codec) are retained by the
>> diff --git a/libavcodec/mediacodecdec_common.h b/libavcodec/mediacodecdec_common.h
>> index c00c2e89f3..10f38277b5 100644
>> --- a/libavcodec/mediacodecdec_common.h
>> +++ b/libavcodec/mediacodecdec_common.h
>> @@ -24,6 +24,7 @@
>>  #define AVCODEC_MEDIACODECDEC_COMMON_H
>>  
>>  #include <stdint.h>
>> +#include <stdatomic.h>
>>  #include <sys/types.h>
>>  
>>  #include "libavutil/frame.h"
>> @@ -34,7 +35,7 @@
>>  
>>  typedef struct MediaCodecDecContext {
>>  
>> -    volatile int refcount;
>> +    atomic_int refcount;
>>  
>>      char *codec_name;
>>  
>> @@ -88,7 +89,7 @@ typedef struct MediaCodecBuffer {
>>      MediaCodecDecContext *ctx;
>>      ssize_t index;
>>      int64_t pts;
>> -    volatile int released;
>> +    atomic_int released;
>>  
>>  } MediaCodecBuffer;
>>  
> 
> Rest looks ok.

Pushed, thanks.
diff mbox

Patch

diff --git a/libavcodec/mediacodec.c b/libavcodec/mediacodec.c
index 4ad5921bc2..91f725621d 100644
--- a/libavcodec/mediacodec.c
+++ b/libavcodec/mediacodec.c
@@ -31,7 +31,6 @@ 
 #include <jni.h>
 
 #include "libavcodec/avcodec.h"
-#include "libavutil/atomic.h"
 #include "libavutil/mem.h"
 
 #include "ffjni.h"
@@ -90,9 +89,9 @@  void av_mediacodec_default_free(AVCodecContext *avctx)
 int av_mediacodec_release_buffer(AVMediaCodecBuffer *buffer, int render)
 {
     MediaCodecDecContext *ctx = buffer->ctx;
-    int released = avpriv_atomic_int_add_and_fetch(&buffer->released, 1);
+    atomic_int released = atomic_fetch_add(&buffer->released, 1);
 
-    if (released == 1) {
+    if (!released) {
         return ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, render);
     }
 
diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
index 2e645caafd..3ada3fa698 100644
--- a/libavcodec/mediacodecdec.c
+++ b/libavcodec/mediacodecdec.c
@@ -29,7 +29,6 @@ 
 #include "libavutil/opt.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/pixfmt.h"
-#include "libavutil/atomic.h"
 
 #include "avcodec.h"
 #include "h264_parse.h"
diff --git a/libavcodec/mediacodecdec_common.c b/libavcodec/mediacodecdec_common.c
index dfc7f5514a..87f032876d 100644
--- a/libavcodec/mediacodecdec_common.c
+++ b/libavcodec/mediacodecdec_common.c
@@ -23,7 +23,6 @@ 
 #include <string.h>
 #include <sys/types.h>
 
-#include "libavutil/atomic.h"
 #include "libavutil/common.h"
 #include "libavutil/mem.h"
 #include "libavutil/log.h"
@@ -143,7 +142,7 @@  static enum AVPixelFormat mcdec_map_color_format(AVCodecContext *avctx,
 
 static void ff_mediacodec_dec_ref(MediaCodecDecContext *s)
 {
-    avpriv_atomic_int_add_and_fetch(&s->refcount, 1);
+    atomic_fetch_add(&s->refcount, 1);
 }
 
 static void ff_mediacodec_dec_unref(MediaCodecDecContext *s)
@@ -151,7 +150,7 @@  static void ff_mediacodec_dec_unref(MediaCodecDecContext *s)
     if (!s)
         return;
 
-    if (!avpriv_atomic_int_add_and_fetch(&s->refcount, -1)) {
+    if (atomic_fetch_sub(&s->refcount, 1) == 1) {
         if (s->codec) {
             ff_AMediaCodec_delete(s->codec);
             s->codec = NULL;
@@ -176,9 +175,8 @@  static void mediacodec_buffer_release(void *opaque, uint8_t *data)
 {
     AVMediaCodecBuffer *buffer = opaque;
     MediaCodecDecContext *ctx = buffer->ctx;
-    int released = avpriv_atomic_int_get(&buffer->released);
 
-    if (!released) {
+    if (!atomic_load(&buffer->released)) {
         ff_AMediaCodec_releaseOutputBuffer(ctx->codec, buffer->index, 0);
     }
 
@@ -221,7 +219,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
         goto fail;
     }
 
-    buffer->released = 0;
+    atomic_init(&buffer->released, 0);
 
     frame->buf[0] = av_buffer_create(NULL,
                                      0,
@@ -465,7 +463,7 @@  int ff_mediacodec_dec_init(AVCodecContext *avctx, MediaCodecDecContext *s,
         AV_PIX_FMT_NONE,
     };
 
-    s->refcount = 1;
+    atomic_init(&s->refcount, 1);
 
     pix_fmt = ff_get_format(avctx, pix_fmts);
     if (pix_fmt == AV_PIX_FMT_MEDIACODEC) {
@@ -725,7 +723,7 @@  int ff_mediacodec_dec_decode(AVCodecContext *avctx, MediaCodecDecContext *s,
 
 int ff_mediacodec_dec_flush(AVCodecContext *avctx, MediaCodecDecContext *s)
 {
-    if (!s->surface || avpriv_atomic_int_get(&s->refcount) == 1) {
+    if (!s->surface || atomic_load(&s->refcount) == 1) {
         int ret;
 
         /* No frames (holding a reference to the codec) are retained by the
diff --git a/libavcodec/mediacodecdec_common.h b/libavcodec/mediacodecdec_common.h
index c00c2e89f3..10f38277b5 100644
--- a/libavcodec/mediacodecdec_common.h
+++ b/libavcodec/mediacodecdec_common.h
@@ -24,6 +24,7 @@ 
 #define AVCODEC_MEDIACODECDEC_COMMON_H
 
 #include <stdint.h>
+#include <stdatomic.h>
 #include <sys/types.h>
 
 #include "libavutil/frame.h"
@@ -34,7 +35,7 @@ 
 
 typedef struct MediaCodecDecContext {
 
-    volatile int refcount;
+    atomic_int refcount;
 
     char *codec_name;
 
@@ -88,7 +89,7 @@  typedef struct MediaCodecBuffer {
     MediaCodecDecContext *ctx;
     ssize_t index;
     int64_t pts;
-    volatile int released;
+    atomic_int released;
 
 } MediaCodecBuffer;