[FFmpeg-devel] [PATCH v3 2/3] libavcodec/utils.c: simplify avcodec locking with atomics

[James Almer]

On 11/25/2017 2:01 PM, Rostislav Pehlivanov wrote:
> Also makes it more robust than using volatiles.
> 
> Signed-off-by: Rostislav Pehlivanov <atomnuker at gmail.com>
> ---
>  libavcodec/internal.h |  1 -
>  libavcodec/utils.c    | 12 ++++++------
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index d3310b6afe..1c54966f37 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -246,7 +246,6 @@ int ff_init_buffer_info(AVCodecContext *s, AVFrame *frame);
>  
>  void ff_color_frame(AVFrame *frame, const int color[4]);
>  
> -extern volatile int ff_avcodec_locked;
>  int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec);
>  int ff_unlock_avcodec(const AVCodec *codec);
>  
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 3a0f3c11f5..96bc9ff4a4 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -114,7 +114,7 @@ static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = NULL;
>  #endif
>  
>  
> -volatile int ff_avcodec_locked;
> +static atomic_bool ff_avcodec_locked;
>  static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0);
>  static void *codec_mutex;
>  static void *avformat_mutex;
> @@ -1937,6 +1937,7 @@ int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op))
>  
>  int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
>  {
> +    _Bool exp = 1;
>      if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init)
>          return 0;
>  
> @@ -1952,22 +1953,21 @@ int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
>                 atomic_load(&entangled_thread_counter));
>          if (!lockmgr_cb)
>              av_log(log_ctx, AV_LOG_ERROR, "No lock manager is set, please see av_lockmgr_register()\n");
> -        ff_avcodec_locked = 1;
> +        atomic_store(&ff_avcodec_locked, 1);
>          ff_unlock_avcodec(codec);
>          return AVERROR(EINVAL);
>      }
> -    av_assert0(!ff_avcodec_locked);
> -    ff_avcodec_locked = 1;
> +    av_assert0(atomic_compare_exchange_strong(&ff_avcodec_locked, &exp, 1));

_Bool atomic_compare_exchange_strong( volatile A* obj,
                                      C* expected, C desired );

"Atomically compares the value pointed to by obj with the value pointed
to by expected, and if those are equal, replaces the former with desired
(performs read-modify-write operation).
Return value: The result of the comparison: true if *obj was equal to
*exp, false otherwise."

exp should be 0. You need to assert that ff_avcodec_locked == 0, then
set it to 1.

>      return 0;
>  }
>  
>  int ff_unlock_avcodec(const AVCodec *codec)
>  {
> +    _Bool exp = 0;
>      if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init)
>          return 0;
>  
> -    av_assert0(ff_avcodec_locked);
> -    ff_avcodec_locked = 0;
> +    av_assert0(atomic_compare_exchange_strong(&ff_avcodec_locked, &exp, 0));

And here exp should be 1.

>      atomic_fetch_add(&entangled_thread_counter, -1);
>      if (lockmgr_cb) {
>          if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE))
>