[FFmpeg-devel] [PATCH] lavc/utils: remove unnecessary locking

[Hendrik Leppkes]

On Fri, Dec 8, 2017 at 11:27 AM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Fri, Dec 08, 2017 at 09:49:25AM +0100, Hendrik Leppkes wrote:
>> On Fri, Dec 8, 2017 at 6:09 AM, Rostislav Pehlivanov
>> <atomnuker at gmail.com> wrote:
>> > Its already done by lockmgr.
>> >
>> > Signed-off-by: Rostislav Pehlivanov <atomnuker at gmail.com>
>> > ---
>> >  libavcodec/utils.c | 6 ------
>> >  1 file changed, 6 deletions(-)
>> >
>> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> > index baf09119fe..796d24dcbb 100644
>> > --- a/libavcodec/utils.c
>> > +++ b/libavcodec/utils.c
>> > @@ -115,7 +115,6 @@ static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = NULL;
>> >  #endif
>> >
>> >
>> > -static atomic_bool ff_avcodec_locked;
>> >  static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0);
>> >  static void *codec_mutex;
>> >  static void *avformat_mutex;
>> > @@ -1943,7 +1942,6 @@ int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op))
>> >
>> >  int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
>> >  {
>> > -    _Bool exp = 0;
>> >      if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init)
>> >          return 0;
>> >
>> > @@ -1959,21 +1957,17 @@ 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");
>> > -        atomic_store(&ff_avcodec_locked, 1);
>> >          ff_unlock_avcodec(codec);
>> >          return AVERROR(EINVAL);
>> >      }
>> > -    av_assert0(atomic_compare_exchange_strong(&ff_avcodec_locked, &exp, 1));
>> >      return 0;
>> >  }
>> >
>> >  int ff_unlock_avcodec(const AVCodec *codec)
>> >  {
>> > -    _Bool exp = 1;
>> >      if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init)
>> >          return 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))
>> > --
>> > 2.15.1.424.g9478a66081
>> >
>>
>> These variables never performed any locking, they only existed as a
>> sanity check that lock/unlock is always called in pairs. If that is
>> really required is up for discussion.
>
> They shuld be usefull to detect some bugs related to locking
>

Either this patch or a simple revert of the original commit which
introduced the atomics (the code didn't even use any atomics before,
and probably doesn't need to because of the lock manager) should be
pushed soon. MSVC builds remain broken otherwise.
Since Michael sees some use for these checks, I'll push a revert in a
day or two if noone objects.

- Hendrik