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

[James Almer]

On 3/23/2017 3:15 PM, Matthieu Bouron wrote:
> Le 23 mars 2017 12:35 AM, "James Almer" <jamrial at gmail.com> a écrit :
> 
> Signed-off-by: James Almer <jamrial at 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) {
> 
> 
> Hello,
> 
> The underlying buffer is meant to be released once. Unless i miss something
> there, you could potentially release a later buffer using the same index or
> release an invalid buffer. A mediacodec buffer can only be released once.
> Once it is released it goes back to the decoder pool and it is not usable
> anymore unless it bas been returned by the decoder again. The buffer itself
> is represented by an index. Can you bring the original check back ?
> (released == 1)

C11 atomics only has fetch_add functions, not add_fetch like the previous
implementation.

add_fetch(&dst, 1) adds 1 to dst, then returns the new dst value.
fetch_add(&dst, 1) adds 1 to dst, then returns the old dst value.

This means that for us to check if the buffer was released once, while for
add_fetch we had to check if the return value was 1, for fetch_add we need
to check if it's zero.

> 
> I dont have the time to review and test the entire patch (nor the machine,
> i m writing from my phone) as i am on holidays but i ll be back on monday
> (and i ll test the patch).
> 
> Matthieu
> [...]
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>