[FFmpeg-devel,v3,2/5] lavc: replace and deprecate the lock manager

Submitted by wm4 on Dec. 23, 2017, 11:51 p.m.

Details

Message ID 20171223235120.31578-2-nfxjfg@googlemail.com
State New
Headers show

Commit Message

wm4 Dec. 23, 2017, 11:51 p.m.
Use static mutexes instead of requiring a lock manager. The behavior
should be roughly the same before and after this change for API users
which did not set the lock manager at all (except that a minor memory
leak disappears).
---
 doc/APIchanges       |   5 +++
 libavcodec/avcodec.h |   8 +++-
 libavcodec/utils.c   | 107 +++++----------------------------------------------
 libavcodec/version.h |   5 ++-
 libavutil/thread.h   |   2 +
 5 files changed, 28 insertions(+), 99 deletions(-)

Patch hide | download patch | download mbox

diff --git a/doc/APIchanges b/doc/APIchanges
index da444ffb7c..df1391d83a 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,11 @@  libavutil:     2017-10-21
 
 API changes, most recent first:
 
+2017-xx-xx - xxxxxxx - lavc 58.9.100 - avcodec.h
+  Deprecate av_lockmgr_register(). You need to build FFmpeg with threading
+  support enabled to get basic thread-safety (which is the default build
+  configuration).
+
 2017-xx-xx - xxxxxxx - lavc 58.8.100 - avcodec.h
   The MediaCodec decoders now support AVCodecContext.hw_device_ctx.
 
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index ce089b7c4a..a9182a9e3d 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -5930,10 +5930,11 @@  attribute_deprecated
 AVHWAccel *av_hwaccel_next(const AVHWAccel *hwaccel);
 #endif
 
-
+#if FF_API_LOCKMGR
 /**
  * Lock operation used by lockmgr
  */
+attribute_deprecated
 enum AVLockOp {
   AV_LOCK_CREATE,  ///< Create a mutex
   AV_LOCK_OBTAIN,  ///< Lock the mutex
@@ -5963,8 +5964,13 @@  enum AVLockOp {
  *           mechanism (i.e. do not use a single static object to
  *           implement your lock manager). If cb is set to NULL the
  *           lockmgr will be unregistered.
+ *
+ * @deprecated This function does nothing, and always returns 0. Be sure to
+ *             build with thread support to get basic thread safety.
  */
+attribute_deprecated
 int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op));
+#endif
 
 /**
  * Get the type of the given codec.
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 873f39f9bd..eec4437693 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -67,58 +67,10 @@ 
 #include "libavutil/ffversion.h"
 const char av_codec_ffversion[] = "FFmpeg version " FFMPEG_VERSION;
 
-#if HAVE_PTHREADS || HAVE_W32THREADS || HAVE_OS2THREADS
-static int default_lockmgr_cb(void **arg, enum AVLockOp op)
-{
-    void * volatile * mutex = arg;
-    int err;
-
-    switch (op) {
-    case AV_LOCK_CREATE:
-        return 0;
-    case AV_LOCK_OBTAIN:
-        if (!*mutex) {
-            pthread_mutex_t *tmp = av_malloc(sizeof(pthread_mutex_t));
-            if (!tmp)
-                return AVERROR(ENOMEM);
-            if ((err = pthread_mutex_init(tmp, NULL))) {
-                av_free(tmp);
-                return AVERROR(err);
-            }
-            if (avpriv_atomic_ptr_cas(mutex, NULL, tmp)) {
-                pthread_mutex_destroy(tmp);
-                av_free(tmp);
-            }
-        }
-
-        if ((err = pthread_mutex_lock(*mutex)))
-            return AVERROR(err);
-
-        return 0;
-    case AV_LOCK_RELEASE:
-        if ((err = pthread_mutex_unlock(*mutex)))
-            return AVERROR(err);
-
-        return 0;
-    case AV_LOCK_DESTROY:
-        if (*mutex)
-            pthread_mutex_destroy(*mutex);
-        av_free(*mutex);
-        avpriv_atomic_ptr_cas(mutex, *mutex, NULL);
-        return 0;
-    }
-    return 1;
-}
-static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = default_lockmgr_cb;
-#else
-static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = NULL;
-#endif
-
-
 volatile int ff_avcodec_locked;
 static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0);
-static void *codec_mutex;
-static void *avformat_mutex;
+static AVMutex codec_mutex = AV_MUTEX_INITIALIZER;
+static AVMutex avformat_mutex = AV_MUTEX_INITIALIZER;
 
 void av_fast_padded_malloc(void *ptr, unsigned int *size, size_t min_size)
 {
@@ -1909,55 +1861,26 @@  void av_register_hwaccel(AVHWAccel *hwaccel)
 }
 #endif
 
+#if FF_API_LOCKMGR
 int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op))
 {
-    if (lockmgr_cb) {
-        // There is no good way to rollback a failure to destroy the
-        // mutex, so we ignore failures.
-        lockmgr_cb(&codec_mutex,    AV_LOCK_DESTROY);
-        lockmgr_cb(&avformat_mutex, AV_LOCK_DESTROY);
-        lockmgr_cb     = NULL;
-        codec_mutex    = NULL;
-        avformat_mutex = NULL;
-    }
-
-    if (cb) {
-        void *new_codec_mutex    = NULL;
-        void *new_avformat_mutex = NULL;
-        int err;
-        if (err = cb(&new_codec_mutex, AV_LOCK_CREATE)) {
-            return err > 0 ? AVERROR_UNKNOWN : err;
-        }
-        if (err = cb(&new_avformat_mutex, AV_LOCK_CREATE)) {
-            // Ignore failures to destroy the newly created mutex.
-            cb(&new_codec_mutex, AV_LOCK_DESTROY);
-            return err > 0 ? AVERROR_UNKNOWN : err;
-        }
-        lockmgr_cb     = cb;
-        codec_mutex    = new_codec_mutex;
-        avformat_mutex = new_avformat_mutex;
-    }
-
     return 0;
 }
+#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 (lockmgr_cb) {
-        if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_OBTAIN))
-            return -1;
-    }
+    if (ff_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));
-        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;
         ff_unlock_avcodec(codec);
         return AVERROR(EINVAL);
@@ -1975,30 +1898,20 @@  int ff_unlock_avcodec(const AVCodec *codec)
     av_assert0(ff_avcodec_locked);
     ff_avcodec_locked = 0;
     atomic_fetch_add(&entangled_thread_counter, -1);
-    if (lockmgr_cb) {
-        if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE))
-            return -1;
-    }
+    if (ff_mutex_unlock(&codec_mutex))
+        return -1;
 
     return 0;
 }
 
 int avpriv_lock_avformat(void)
 {
-    if (lockmgr_cb) {
-        if ((*lockmgr_cb)(&avformat_mutex, AV_LOCK_OBTAIN))
-            return -1;
-    }
-    return 0;
+    return ff_mutex_lock(&avformat_mutex) ? -1 : 0;
 }
 
 int avpriv_unlock_avformat(void)
 {
-    if (lockmgr_cb) {
-        if ((*lockmgr_cb)(&avformat_mutex, AV_LOCK_RELEASE))
-            return -1;
-    }
-    return 0;
+    return ff_mutex_unlock(&avformat_mutex) ? -1 : 0;
 }
 
 unsigned int avpriv_toupper4(unsigned int x)
diff --git a/libavcodec/version.h b/libavcodec/version.h
index d55de89797..47a15d52b8 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -28,7 +28,7 @@ 
 #include "libavutil/version.h"
 
 #define LIBAVCODEC_VERSION_MAJOR  58
-#define LIBAVCODEC_VERSION_MINOR   8
+#define LIBAVCODEC_VERSION_MINOR   9
 #define LIBAVCODEC_VERSION_MICRO 100
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
@@ -126,6 +126,9 @@ 
 #ifndef FF_API_USER_VISIBLE_AVHWACCEL
 #define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < 59)
 #endif
+#ifndef FF_API_LOCKMGR
+#define FF_API_LOCKMGR (LIBAVCODEC_VERSION_MAJOR < 59)
+#endif
 
 
 #endif /* AVCODEC_VERSION_H */
diff --git a/libavutil/thread.h b/libavutil/thread.h
index 309414aa7d..cc5272d379 100644
--- a/libavutil/thread.h
+++ b/libavutil/thread.h
@@ -134,6 +134,7 @@  static inline int strict_pthread_once(pthread_once_t *once_control, void (*init_
 #endif
 
 #define AVMutex pthread_mutex_t
+#define AV_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
 
 #define ff_mutex_init    pthread_mutex_init
 #define ff_mutex_lock    pthread_mutex_lock
@@ -148,6 +149,7 @@  static inline int strict_pthread_once(pthread_once_t *once_control, void (*init_
 #else
 
 #define AVMutex char
+#define AV_MUTEX_INITIALIZER 0
 
 static inline int ff_mutex_init(AVMutex *mutex, const void *attr){ return 0; }
 static inline int ff_mutex_lock(AVMutex *mutex){ return 0; }