Message ID | 20171125170157.32154-2-atomnuker@gmail.com |
---|---|
State | Superseded |
Headers | show |
On Sat, Nov 25, 2017 at 05:01:56PM +0000, Rostislav Pehlivanov wrote: [...] > -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; why _Bool instead of atomic_bool?
On 11/25/2017 3:07 PM, Clément Bœsch wrote: > On Sat, Nov 25, 2017 at 05:01:56PM +0000, Rostislav Pehlivanov wrote: > [...] >> -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; > > why _Bool instead of atomic_bool? See http://en.cppreference.com/w/c/atomic/atomic_compare_exchange The variable passed as argument for *expected needs to be of the non-atomic type of the variable passed for *obj.
On 11/25/2017 2:01 PM, Rostislav Pehlivanov wrote: > Also makes it more robust than using volatiles. > > Signed-off-by: Rostislav Pehlivanov <atomnuker@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)) >
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)); 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)); atomic_fetch_add(&entangled_thread_counter, -1); if (lockmgr_cb) { if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE))
Also makes it more robust than using volatiles. Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com> --- libavcodec/internal.h | 1 - libavcodec/utils.c | 12 ++++++------ 2 files changed, 6 insertions(+), 7 deletions(-)