[FFmpeg-devel] [PATCH 26/42] avcodec/refstruct: Allow to use a dynamic opaque

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

This is in preparation for a new ThreadFrame-API
based around RefStruct to allow to pass the AVCodecContext*
to ff_thread_release_buffer().

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Now that thread-unsafe callbacks are no more, one could
also just use av_frame_unref() instead of ff_thread_release_buffer().
But this is IMO more elegant.

 libavcodec/cbs.c       |  8 ++--
 libavcodec/refstruct.c | 83 +++++++++++++++++++++++++++++++++---------
 libavcodec/refstruct.h | 83 ++++++++++++++++++++++++++++++++++++++----
 3 files changed, 146 insertions(+), 28 deletions(-)

diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index 00c462b09d..40235ce647 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -903,11 +903,13 @@ static const CodedBitstreamUnitTypeDescriptor
 
 static void *cbs_alloc_content(const CodedBitstreamUnitTypeDescriptor *desc)
 {
+    FFRefStructUnrefCB unref_cb;
+    unref_cb.unref = desc->content_type == CBS_CONTENT_TYPE_COMPLEX
+                                            ? desc->type.complex.content_free
+                                            : cbs_default_free_unit_content;
     return ff_refstruct_alloc_ext_c(desc->content_size, 0,
                                     (FFRefStructOpaque){ .c = desc },
-                                    desc->content_type == CBS_CONTENT_TYPE_COMPLEX
-                                            ? desc->type.complex.content_free
-                                            : cbs_default_free_unit_content);
+                                    unref_cb);
 }
 
 int ff_cbs_alloc_unit_content(CodedBitstreamContext *ctx,
diff --git a/libavcodec/refstruct.c b/libavcodec/refstruct.c
index 2108ff8163..817a8a455a 100644
--- a/libavcodec/refstruct.c
+++ b/libavcodec/refstruct.c
@@ -20,6 +20,8 @@
 #include <stdint.h>
 #include <string.h>
 
+#include "config.h"
+
 #include "internal.h"
 #include "refstruct.h"
 
@@ -37,8 +39,11 @@ typedef struct RefCount {
      */
     atomic_uintptr_t  refcount;
     FFRefStructOpaque opaque;
-    void (*free_cb)(FFRefStructOpaque opaque, void *obj);
+    FFRefStructUnrefCB free_cb;
     void (*free)(void *ref);
+#if defined(ASSERT_LEVEL) && ASSERT_LEVEL >= 1
+    unsigned flags;
+#endif
 } RefCount;
 
 #if __STDC_VERSION__ >= 201112L
@@ -63,16 +68,19 @@ static void *get_userdata(void *buf)
 }
 
 static void refcount_init(RefCount *ref, FFRefStructOpaque opaque,
-                          void (*free_cb)(FFRefStructOpaque opaque, void *obj))
+                          unsigned flags, FFRefStructUnrefCB free_cb)
 {
     atomic_init(&ref->refcount, 1);
     ref->opaque  = opaque;
     ref->free_cb = free_cb;
     ref->free    = av_free;
+#if defined(ASSERT_LEVEL) && ASSERT_LEVEL >= 1
+    ref->flags = flags;
+#endif
 }
 
 void *ff_refstruct_alloc_ext_c(size_t size, unsigned flags, FFRefStructOpaque opaque,
-                               void (*free_cb)(FFRefStructOpaque opaque, void *obj))
+                               FFRefStructUnrefCB free_cb)
 {
     void *buf, *obj;
 
@@ -81,7 +89,7 @@ void *ff_refstruct_alloc_ext_c(size_t size, unsigned flags, FFRefStructOpaque op
     buf = av_malloc(size + REFCOUNT_OFFSET);
     if (!buf)
         return NULL;
-    refcount_init(buf, opaque, free_cb);
+    refcount_init(buf, opaque, flags, free_cb);
     obj = get_userdata(buf);
     if (!(flags & FF_REFSTRUCT_FLAG_NO_ZEROING))
         memset(obj, 0, size);
@@ -105,9 +113,31 @@ void ff_refstruct_unref(void *objp)
     memcpy(objp, &(void *){ NULL }, sizeof(obj));
 
     ref = get_refcount(obj);
+    av_assert1(!(ref->flags & FF_REFSTRUCT_FLAG_DYNAMIC_OPAQUE));
+    if (atomic_fetch_sub_explicit(&ref->refcount, 1, memory_order_acq_rel) == 1) {
+        if (ref->free_cb.unref)
+            ref->free_cb.unref(ref->opaque, obj);
+        ref->free(ref);
+    }
+
+    return;
+}
+
+void ff_refstruct_unref_ext_c(FFRefStructOpaque opaque, void *objp)
+{
+    void *obj;
+    RefCount *ref;
+
+    memcpy(&obj, objp, sizeof(obj));
+    if (!obj)
+        return;
+    memcpy(objp, &(void *){ NULL }, sizeof(obj));
+
+    ref = get_refcount(obj);
+    av_assert1(ref->flags & FF_REFSTRUCT_FLAG_DYNAMIC_OPAQUE);
     if (atomic_fetch_sub_explicit(&ref->refcount, 1, memory_order_acq_rel) == 1) {
-        if (ref->free_cb)
-            ref->free_cb(ref->opaque, obj);
+        if (ref->free_cb.unref_ext)
+            ref->free_cb.unref_ext(opaque, ref->opaque, obj);
         ref->free(ref);
     }
 
@@ -161,7 +191,7 @@ struct FFRefStructPool {
     size_t size;
     FFRefStructOpaque opaque;
     int  (*init_cb)(FFRefStructOpaque opaque, void *obj);
-    void (*reset_cb)(FFRefStructOpaque opaque, void *obj);
+    FFRefStructUnrefCB reset_cb;
     void (*free_entry_cb)(FFRefStructOpaque opaque, void *obj);
     void (*free_cb)(FFRefStructOpaque opaque);
 
@@ -221,14 +251,23 @@ static void pool_reset_entry(FFRefStructOpaque opaque, void *entry)
 {
     FFRefStructPool *pool = opaque.nc;
 
-    pool->reset_cb(pool->opaque, entry);
+    pool->reset_cb.unref(pool->opaque, entry);
+}
+
+static void pool_reset_entry_ext(FFRefStructOpaque opaque,
+                                 FFRefStructOpaque initial_opaque,
+                                 void *entry)
+{
+    FFRefStructPool *pool = initial_opaque.nc;
+
+    pool->reset_cb.unref_ext(opaque, pool->opaque, entry);
 }
 
-static int refstruct_pool_get_ext(void *datap, FFRefStructPool *pool)
+static int refstruct_pool_get_ext(void *objp, FFRefStructPool *pool)
 {
     void *ret = NULL;
 
-    memcpy(datap, &(void *){ NULL }, sizeof(void*));
+    memcpy(objp, &(void *){ NULL }, sizeof(void*));
 
     pthread_mutex_lock(&pool->mutex);
     av_assert1(!pool->uninited);
@@ -243,8 +282,13 @@ static int refstruct_pool_get_ext(void *datap, FFRefStructPool *pool)
 
     if (!ret) {
         RefCount *ref;
-        ret = ff_refstruct_alloc_ext(pool->size, pool->entry_flags, pool,
-                                     pool->reset_cb ? pool_reset_entry : NULL);
+#define CB_INIT(suffix) ((FFRefStructUnrefCB) { .unref ## suffix = pool->reset_cb.unref ## suffix ? \
+                                                                   pool_reset_entry ## suffix : NULL })
+        ret = ff_refstruct_alloc_ext_c(pool->size, pool->entry_flags,
+                                       (FFRefStructOpaque){ .nc = pool },
+                                       (pool->pool_flags & FF_REFSTRUCT_FLAG_DYNAMIC_OPAQUE) ?
+                                       CB_INIT(_ext) : CB_INIT());
+#undef CB_INIT
         if (!ret)
             return AVERROR(ENOMEM);
         ref = get_refcount(ret);
@@ -253,7 +297,7 @@ static int refstruct_pool_get_ext(void *datap, FFRefStructPool *pool)
             int err = pool->init_cb(pool->opaque, ret);
             if (err < 0) {
                 if (pool->pool_flags & FF_REFSTRUCT_POOL_FLAG_RESET_ON_INIT_ERROR)
-                    pool->reset_cb(pool->opaque, ret);
+                    pool->reset_cb.unref(pool->opaque, ret);
                 if (pool->pool_flags & FF_REFSTRUCT_POOL_FLAG_FREE_ON_INIT_ERROR)
                     pool->free_entry_cb(pool->opaque, ret);
                 av_free(ref);
@@ -266,7 +310,7 @@ static int refstruct_pool_get_ext(void *datap, FFRefStructPool *pool)
     if (pool->pool_flags & FF_REFSTRUCT_POOL_FLAG_ZERO_EVERY_TIME)
         memset(ret, 0, pool->size);
 
-    memcpy(datap, &ret, sizeof(ret));
+    memcpy(objp, &ret, sizeof(ret));
 
     return 0;
 }
@@ -312,7 +356,7 @@ FFRefStructPool *ff_refstruct_pool_alloc(size_t size, unsigned flags)
 FFRefStructPool *ff_refstruct_pool_alloc_ext_c(size_t size, unsigned flags,
                                                FFRefStructOpaque opaque,
                                                int  (*init_cb)(FFRefStructOpaque opaque, void *obj),
-                                               void (*reset_cb)(FFRefStructOpaque opaque, void *obj),
+                                               FFRefStructUnrefCB reset_cb,
                                                void (*free_entry_cb)(FFRefStructOpaque opaque, void *obj),
                                                void (*free_cb)(FFRefStructOpaque opaque))
 {
@@ -330,10 +374,15 @@ FFRefStructPool *ff_refstruct_pool_alloc_ext_c(size_t size, unsigned flags,
     pool->reset_cb      = reset_cb;
     pool->free_entry_cb = free_entry_cb;
     pool->free_cb       = free_cb;
-#define COMMON_FLAGS FF_REFSTRUCT_POOL_FLAG_NO_ZEROING
+#define COMMON_FLAGS (FF_REFSTRUCT_POOL_FLAG_NO_ZEROING | FF_REFSTRUCT_POOL_FLAG_DYNAMIC_OPAQUE)
     pool->entry_flags   = flags & COMMON_FLAGS;
+    // Dynamic opaque and resetting-on-init-error are incompatible
+    // (there is no dynamic opaque available in ff_refstruct_pool_get()).
+    av_assert1(!(flags & FF_REFSTRUCT_POOL_FLAG_DYNAMIC_OPAQUE &&
+                 flags & FF_REFSTRUCT_POOL_FLAG_RESET_ON_INIT_ERROR));
     // Filter out nonsense combinations to avoid checks later.
-    if (!pool->reset_cb)
+    if (flags & FF_REFSTRUCT_POOL_FLAG_RESET_ON_INIT_ERROR &&
+        !pool->reset_cb.unref)
         flags &= ~FF_REFSTRUCT_POOL_FLAG_RESET_ON_INIT_ERROR;
     if (!pool->free_entry_cb)
         flags &= ~FF_REFSTRUCT_POOL_FLAG_FREE_ON_INIT_ERROR;
diff --git a/libavcodec/refstruct.h b/libavcodec/refstruct.h
index d525be61e8..b0a750e2f7 100644
--- a/libavcodec/refstruct.h
+++ b/libavcodec/refstruct.h
@@ -53,18 +53,42 @@
  *
  * The functions provided by this API with an FFRefStructOpaque come in pairs
  * named foo_c and foo. The foo function accepts void* as opaque and is just
- * a wrapper around the foo_c function; "_c" means "(potentially) const".
+ * a wrapper around the foo_c function for the common case of a non-const
+ * opaque ("_c" means "(potentially) const"). For the allocation functions
+ * the wrappers also accept the ordinary "unref" form of FFRefStructUnrefCB;
+ * only the "_c" versions accept the full union to set the unref_ext variant.
  */
 typedef union {
     void *nc;
     const void *c;
 } FFRefStructOpaque;
 
+typedef union FFRefStructUnrefCB {
+    void (*unref)(FFRefStructOpaque opaque, void *obj);
+    void (*unref_ext)(FFRefStructOpaque dynamic_opaque,
+                      FFRefStructOpaque initial_opaque,
+                      void *obj);
+} FFRefStructUnrefCB;
+
 /**
  * If this flag is set in ff_refstruct_alloc_ext_c(), the object will not
  * be initially zeroed.
  */
 #define FF_REFSTRUCT_FLAG_NO_ZEROING (1 << 0)
+/**
+ * This flag being set indicates that the free_cb union is in
+ * the unref_ext-state.
+ * In this case unreferencing the object has to be done
+ * via ff_refstruct_unref_ext() instead of ff_refstruct_unref().
+ * Using the latter is forbidden and leads to undefined behaviour.
+ *
+ * The free_ext-callback will be called when the refcount reaches zero
+ * with the opaque given to ff_refstruct_unref_ext() passed along as
+ * the callback's dynamic_opaque parameter; the argument corresponding
+ * to the initial_opaque parameter will be the opaque provided to
+ * ff_refstruct_alloc_ext_c() and obj is the object to be unreferenced.
+ */
+#define FF_REFSTRUCT_FLAG_DYNAMIC_OPAQUE                              (1 << 1)
 
 /**
  * Allocate a refcounted object of usable size `size` managed via
@@ -76,17 +100,19 @@ typedef union {
  * @param size    Desired usable size of the returned object.
  * @param flags   A bitwise combination of FF_REFSTRUCT_FLAG_* flags.
  * @param opaque  A pointer that will be passed to the free_cb callback.
- * @param free_cb A callback for freeing this object's content
+ * @param free_cb Contains the callback for freeing this object's content
  *                when its reference count reaches zero;
  *                it must not free the object itself.
+ *                The state of free_cb is indicated by the
+ *                FF_REFSTRUCT_FLAG_DYNAMIC_OPAQUE flag.
  * @return A pointer to an object of the desired size or NULL on failure.
  */
 void *ff_refstruct_alloc_ext_c(size_t size, unsigned flags, FFRefStructOpaque opaque,
-                               void (*free_cb)(FFRefStructOpaque opaque, void *obj));
+                               FFRefStructUnrefCB free_cb);
 
 /**
  * A wrapper around ff_refstruct_alloc_ext_c() for the common case
- * of a non-const qualified opaque.
+ * of a non-const qualified and non-dynamic opaque.
  *
  * @see ff_refstruct_alloc_ext_c()
  */
@@ -95,7 +121,7 @@ void *ff_refstruct_alloc_ext(size_t size, unsigned flags, void *opaque,
                              void (*free_cb)(FFRefStructOpaque opaque, void *obj))
 {
     return ff_refstruct_alloc_ext_c(size, flags, (FFRefStructOpaque){.nc = opaque},
-                                    free_cb);
+                                    (FFRefStructUnrefCB){ .unref = free_cb });
 }
 
 /**
@@ -107,6 +133,10 @@ void *ff_refstruct_allocz(size_t size);
  * Decrement the reference count of the underlying object and automatically
  * free the object if there are no more references to it.
  *
+ * This function must not be used if the object has been created
+ * with the dynamic opaque flags (FF_REFSTRUCT_FLAG_DYNAMIC_OPAQUE
+ * or FF_REFSTRUCT_POOL_FLAG_DYNAMIC_OPAQUE for objects from pools).
+ *
  * `*objp == NULL` is legal and a no-op.
  *
  * @param objp Pointer to a pointer that is either NULL or points to an object
@@ -114,6 +144,32 @@ void *ff_refstruct_allocz(size_t size);
  */
 void ff_refstruct_unref(void *objp);
 
+/**
+ * Decrement the reference count of the underlying object and automatically
+ * free the object if there are no more references to it.
+ *
+ * This function may only be used of the object has been created
+ * with the dynamic opaque flags (FF_REFSTRUCT_FLAG_DYNAMIC_OPAQUE
+ * or FF_REFSTRUCT_POOL_FLAG_DYNAMIC_OPAQUE for objects from pools).
+ *
+ * `*objp == NULL` is legal and a no-op.
+ *
+ * @param objp Pointer to a pointer that is either NULL or points to an object
+ *             managed via this API. `*objp` is set to NULL on return.
+ */
+void ff_refstruct_unref_ext_c(FFRefStructOpaque opaque, void *objp);
+/**
+ * A wrapper around ff_refstruct_unref_ext_c() for the common case
+ * of a non-const qualified dynamic opaque.
+ *
+ * @see ff_refstruct_alloc_ext_c()
+ */
+static inline
+void ff_refstruct_unref_ext(void *opaque, void *objp)
+{
+    ff_refstruct_unref_ext_c((FFRefStructOpaque){ .nc = opaque }, objp);
+}
+
 /**
  * Create a new reference to an object managed via this API,
  * i.e. increment the reference count of the underlying object
@@ -215,6 +271,16 @@ typedef struct FFRefStructPool FFRefStructPool;
  * flag had been provided.
  */
 #define FF_REFSTRUCT_POOL_FLAG_ZERO_EVERY_TIME                       (1 << 18)
+/**
+ * This flag being set indicates that the reset_cb union is in
+ * the unref_ext-state. The semantics of FF_REFSTRUCT_FLAG_DYNAMIC_OPAQUE
+ * apply with the opaque given to ff_refstruct_alloc_ext_c()
+ * corresponding to the opaque given to ff_refstruct_pool_alloc_ext_c().
+ *
+ * This flag is incompatible with FF_REFSTRUCT_POOL_FLAG_RESET_ON_INIT_ERROR.
+ * Giving both to a pool allocation function leads to undefined behaviour.
+ */
+#define FF_REFSTRUCT_POOL_FLAG_DYNAMIC_OPAQUE FF_REFSTRUCT_FLAG_DYNAMIC_OPAQUE
 
 /**
  * Equivalent to ff_refstruct_pool_alloc(size, flags, NULL, NULL, NULL, NULL, NULL)
@@ -240,13 +306,13 @@ FFRefStructPool *ff_refstruct_pool_alloc(size_t size, unsigned flags);
 FFRefStructPool *ff_refstruct_pool_alloc_ext_c(size_t size, unsigned flags,
                                                FFRefStructOpaque opaque,
                                                int  (*init_cb)(FFRefStructOpaque opaque, void *obj),
-                                               void (*reset_cb)(FFRefStructOpaque opaque, void *obj),
+                                               FFRefStructUnrefCB reset_cb,
                                                void (*free_entry_cb)(FFRefStructOpaque opaque, void *obj),
                                                void (*free_cb)(FFRefStructOpaque opaque));
 
 /**
  * A wrapper around ff_refstruct_pool_alloc_ext_c() for the common case
- * of a non-const qualified opaque.
+ * of a non-const qualified and non-dynamic opaque.
  *
  * @see ff_refstruct_pool_alloc_ext_c()
  */
@@ -259,7 +325,8 @@ FFRefStructPool *ff_refstruct_pool_alloc_ext(size_t size, unsigned flags,
                                              void (*free_cb)(FFRefStructOpaque opaque))
 {
     return ff_refstruct_pool_alloc_ext_c(size, flags, (FFRefStructOpaque){.nc = opaque},
-                                         init_cb, reset_cb, free_entry_cb, free_cb);
+                                         init_cb, (FFRefStructUnrefCB){ .unref = reset_cb },
+                                         free_entry_cb, free_cb);
 }
 
 /**
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".