[FFmpeg-devel,5/5] lavc: remove weird debug code around avcodec init locking

Submitted by wm4 on Dec. 21, 2017, 10:22 p.m.

Details

Message ID 20171221222224.18577-5-nfxjfg@googlemail.com
State New
Headers show

Commit Message

wm4 Dec. 21, 2017, 10:22 p.m.
This is just a lot of complicated and confusing code that does nothing.
Also, the functions return values were checked only sometimes. Locking
shouldn't fail anyway, so remove the return values. Barely any other
pthread lock calls check the return value (including more important code
that is more likely to fail horribly if locking fails).

It could be argued that it might be helpful in some debugging
situations, or when the user built FFmpeg without thread support against
all good advice.

But there are dummy atomics too, so the atomic check won't help with
ensuring correctness absolutely. You gain very little.

Also, for debugging, you can just raise the ASSERT_LEVEL, and then
libavutil/thread.h will redefine the locking functions to explicitly
check the return values.
---
 libavcodec/internal.h |  4 ----
 libavcodec/utils.c    | 55 +++++++++++++--------------------------------------
 2 files changed, 14 insertions(+), 45 deletions(-)

Patch hide | download patch | download mbox

diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index bf58f36ad3..1c901a7cbc 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -241,10 +241,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);
-
 /**
  * Maximum size in bytes of extradata.
  * This value was chosen such that every bit of the buffer is
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 718063f1a1..7432bb78a6 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -67,8 +67,6 @@ 
 #include "libavutil/ffversion.h"
 const char av_codec_ffversion[] = "FFmpeg version " FFMPEG_VERSION;
 
-volatile int ff_avcodec_locked;
-static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0);
 static pthread_mutex_t codec_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 void av_fast_padded_malloc(void *ptr, unsigned int *size, size_t min_size)
@@ -550,6 +548,19 @@  static int64_t get_bit_rate(AVCodecContext *ctx)
     return bit_rate;
 }
 
+
+static void ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
+{
+    if (!(codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE) && codec->init)
+        pthread_mutex_lock(&codec_mutex);
+}
+
+static void ff_unlock_avcodec(const AVCodec *codec)
+{
+    if (!(codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE) && codec->init)
+        pthread_mutex_unlock(&codec_mutex);
+}
+
 int attribute_align_arg ff_codec_open2_recursive(AVCodecContext *avctx, const AVCodec *codec, AVDictionary **options)
 {
     int ret = 0;
@@ -589,9 +600,7 @@  int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
     if (options)
         av_dict_copy(&tmp, *options, 0);
 
-    ret = ff_lock_avcodec(avctx, codec);
-    if (ret < 0)
-        return ret;
+    ff_lock_avcodec(avctx, codec);
 
     avctx->internal = av_mallocz(sizeof(AVCodecInternal));
     if (!avctx->internal) {
@@ -1867,42 +1876,6 @@  int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op))
 }
 #endif
 
-int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec)
-{
-    if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init)
-        return 0;
-
-    if (pthread_mutex_lock(&codec_mutex))
-        return -1;
-
-    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",
-               atomic_load(&entangled_thread_counter));
-        ff_avcodec_locked = 1;
-        ff_unlock_avcodec(codec);
-        return AVERROR(EINVAL);
-    }
-    av_assert0(!ff_avcodec_locked);
-    ff_avcodec_locked = 1;
-    return 0;
-}
-
-int ff_unlock_avcodec(const AVCodec *codec)
-{
-    if (codec->caps_internal & FF_CODEC_CAP_INIT_THREADSAFE || !codec->init)
-        return 0;
-
-    av_assert0(ff_avcodec_locked);
-    ff_avcodec_locked = 0;
-    atomic_fetch_add(&entangled_thread_counter, -1);
-    if (pthread_mutex_unlock(&codec_mutex))
-        return -1;
-
-    return 0;
-}
-
 unsigned int avpriv_toupper4(unsigned int x)
 {
     return av_toupper(x & 0xFF) +