[FFmpeg-devel] [PATCH 2/2] lavc/utils.c: use C11 atomics for entangled thread handling

[James Almer]

On 11/24/2017 9:33 PM, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov <atomnuker at gmail.com>
> ---
>  libavcodec/utils.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index e50de6e89b..a3cd96ed2e 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -56,6 +56,7 @@
>  #include "version.h"
>  #include <stdlib.h>
>  #include <stdarg.h>
> +#include <stdatomic.h>
>  #include <limits.h>
>  #include <float.h>
>  #if CONFIG_ICONV
> @@ -114,7 +115,7 @@ static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = NULL;
>  
>  
>  volatile int ff_avcodec_locked;
> -static int volatile entangled_thread_counter = 0;
> +atomic_int volatile entangled_thread_counter = ATOMIC_VAR_INIT(0);

There's no need for volatile after making it atomic_int, and it should
remain static.

>  static void *codec_mutex;
>  static void *avformat_mutex;
>  
> @@ -1944,11 +1945,12 @@ int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
>              return -1;
>      }
>  
> -    if (avpriv_atomic_int_add_and_fetch(&entangled_thread_counter, 1) != 1) {
> +    atomic_fetch_add(&entangled_thread_counter, 1);
> +    if (atomic_load(&entangled_thread_counter) != 1) {

Since the c11 function returns the value before the add instead of after
it, you can simplify this by doing

if (!atomic_fetch_add(&entangled_thread_counter, 1)) {

>          av_log(log_ctx, AV_LOG_ERROR,
>                 "Insufficient thread locking. At least %d threads are "
>                 "calling avcodec_open2() at the same time right now.\n",
> -               entangled_thread_counter);
> +               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;
> @@ -1967,7 +1969,7 @@ int ff_unlock_avcodec(const AVCodec *codec)
>  
>      av_assert0(ff_avcodec_locked);
>      ff_avcodec_locked = 0;
> -    avpriv_atomic_int_add_and_fetch(&entangled_thread_counter, -1);
> +    atomic_fetch_add(&entangled_thread_counter, -1);
>      if (lockmgr_cb) {
>          if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE))
>              return -1;
> 

It might be worth for that matter testing to see if we can use laxer
memory access orders here and in ER, as by default the strictest one is
used (same as with the old avpriv_atomic functions).