video: replace our own refcounting with libavutil's

mpv had refcounted frames before libav*, so we were not using
libavutil's facilities. Change this and drop our own code.

Since AVFrames are not actually refcounted, and only the image data
they reference, the semantics change a bit. This affects mainly
mp_image_pool, which was operating on whole images instead of buffers.
While we could work on AVBufferRefs instead (and use AVBufferPool),
this doesn't work for use with hardware decoding, which doesn't
map cleanly to FFmpeg's reference counting. But it worked out. One
weird consequence is that we still need our custom image data
allocation function (for normal image data), because AVFrame's uses
multiple buffers.

There also seems to be a timing-dependent problem with vaapi (the
pool appears to be "leaking" surfaces). I don't know if this is a new
problem, or whether the code changes just happened to cause it more
often. Raising the number of reserved surfaces seemed to fix it, but
since it appears to be timing dependent, and I couldn't find anything
wrong with the code, I'm just going to assume it's not a new bug.

4 files changed, 123 insertions, 150 deletions

diff --git a/video/decode/vd_lavc.c b/video/decode/vd_lavc.c
index 8320001e6b..125141b298 100644
--- a/video/decode/vd_lavc.c
+++ b/video/decode/vd_lavc.c
@@ -547,12 +547,6 @@ static enum AVPixelFormat get_format_hwdec(struct AVCodecContext *avctx,
     return AV_PIX_FMT_NONE;
 }
 
-static void free_mpi(void *opaque, uint8_t *data)
-{
-    struct mp_image *mpi = opaque;
-    talloc_free(mpi);
-}
-
 static int get_buffer2_hwdec(AVCodecContext *avctx, AVFrame *pic, int flags)
 {
     struct dec_video *vd = avctx->opaque;
@@ -591,9 +585,12 @@ static int get_buffer2_hwdec(AVCodecContext *avctx, AVFrame *pic, int flags)
     if (!mpi)
         return -1;
 
-    for (int i = 0; i < 4; i++)
+    for (int i = 0; i < 4; i++) {
         pic->data[i] = mpi->planes[i];
-    pic->buf[0] = av_buffer_create(NULL, 0, free_mpi, mpi, 0);
+        pic->buf[i] = mpi->bufs[i];
+        mpi->bufs[i] = NULL;
+    }
+    talloc_free(mpi);
 
     return 0;
 }
diff --git a/video/mp_image.c b/video/mp_image.c
index 3816e297ad..30488e66c2 100644
--- a/video/mp_image.c
+++ b/video/mp_image.c
@@ -38,75 +38,10 @@
 
 #include "video/filter/vf.h"
 
-static pthread_mutex_t refcount_mutex = PTHREAD_MUTEX_INITIALIZER;
-#define refcount_lock() pthread_mutex_lock(&refcount_mutex)
-#define refcount_unlock() pthread_mutex_unlock(&refcount_mutex)
-
-struct m_refcount {
-    void *arg;
-    // free() is called if refcount reaches 0.
-    void (*free)(void *arg);
-    bool (*ext_is_unique)(void *arg);
-    // Native refcount (there may be additional references if .ext_* are set)
-    int refcount;
-};
-
-// Only for checking API usage
-static void m_refcount_destructor(void *ptr)
-{
-    struct m_refcount *ref = ptr;
-    assert(ref->refcount == 0);
-}
-
-// Starts out with refcount==1, caller can set .arg and .free and .ext_*
-static struct m_refcount *m_refcount_new(void)
-{
-    struct m_refcount *ref = talloc_ptrtype(NULL, ref);
-    *ref = (struct m_refcount) { .refcount = 1 };
-    talloc_set_destructor(ref, m_refcount_destructor);
-    return ref;
-}
-
-static void m_refcount_ref(struct m_refcount *ref)
-{
-    refcount_lock();
-    ref->refcount++;
-    refcount_unlock();
-}
-
-static void m_refcount_unref(struct m_refcount *ref)
-{
-    bool dead;
-    refcount_lock();
-    assert(ref->refcount > 0);
-    ref->refcount--;
-    dead = ref->refcount == 0;
-    refcount_unlock();
-
-    if (dead) {
-        if (ref->free)
-            ref->free(ref->arg);
-        talloc_free(ref);
-    }
-}
-
-static bool m_refcount_is_unique(struct m_refcount *ref)
-{
-    bool nonunique;
-    refcount_lock();
-    nonunique = ref->refcount > 1;
-    refcount_unlock();
-
-    if (nonunique)
-        return false;
-    if (ref->ext_is_unique)
-        return ref->ext_is_unique(ref->arg); // referenced only by us
-    return true;
-}
-
 static bool mp_image_alloc_planes(struct mp_image *mpi)
 {
     assert(!mpi->planes[0]);
+    assert(!mpi->bufs[0]);
 
     if (!mp_image_params_valid(&mpi->params) || mpi->fmt.flags & MP_IMGFLAG_HWACCEL)
         return false;
@@ -129,10 +64,12 @@ static bool mp_image_alloc_planes(struct mp_image *mpi)
     for (int n = 0; n < MP_MAX_PLANES; n++)
         sum += plane_size[n];
 
-    uint8_t *data = av_malloc(FFMAX(sum, 1));
-    if (!data)
+    // Note: mp_image_pool assumes this creates only 1 AVBufferRef.
+    mpi->bufs[0] = av_buffer_alloc(FFMAX(sum, 1));
+    if (!mpi->bufs[0])
         return false;
 
+    uint8_t *data = mpi->bufs[0]->data;
     for (int n = 0; n < MP_MAX_PLANES; n++) {
         mpi->planes[n] = plane_size[n] ? data : NULL;
         data += plane_size[n];
@@ -153,7 +90,8 @@ void mp_image_setfmt(struct mp_image *mpi, int out_fmt)
 static void mp_image_destructor(void *ptr)
 {
     mp_image_t *mpi = ptr;
-    m_refcount_unref(mpi->refcount);
+    for (int p = 0; p < MP_MAX_PLANES; p++)
+        av_buffer_unref(&mpi->bufs[p]);
 }
 
 int mp_chroma_div_up(int size, int shift)
@@ -194,7 +132,6 @@ struct mp_image *mp_image_alloc(int imgfmt, int w, int h)
 {
     struct mp_image *mpi = talloc_zero(NULL, struct mp_image);
     talloc_set_destructor(mpi, mp_image_destructor);
-    mpi->refcount = m_refcount_new();
 
     mp_image_set_size(mpi, w, h);
     mp_image_setfmt(mpi, imgfmt);
@@ -202,8 +139,6 @@ struct mp_image *mp_image_alloc(int imgfmt, int w, int h)
         talloc_free(mpi);
         return NULL;
     }
-    mpi->refcount->free = av_free;
-    mpi->refcount->arg = mpi->planes[0];
     return mpi;
 }
 
@@ -223,7 +158,7 @@ struct mp_image *mp_image_new_copy(struct mp_image *img)
 void mp_image_steal_data(struct mp_image *dst, struct mp_image *src)
 {
     assert(dst->imgfmt == src->imgfmt && dst->w == src->w && dst->h == src->h);
-    assert(dst->refcount && src->refcount);
+    assert(dst->bufs[0] && src->bufs[0]);
 
     for (int p = 0; p < MP_MAX_PLANES; p++) {
         dst->planes[p] = src->planes[p];
@@ -231,9 +166,11 @@ void mp_image_steal_data(struct mp_image *dst, struct mp_image *src)
     }
     mp_image_copy_attributes(dst, src);
 
-    m_refcount_unref(dst->refcount);
-    dst->refcount = src->refcount;
-    talloc_set_destructor(src, NULL);
+    for (int p = 0; p < MP_MAX_PLANES; p++) {
+        av_buffer_unref(&dst->bufs[p]);
+        dst->bufs[p] = src->bufs[p];
+        src->bufs[p] = NULL;
+    }
     talloc_free(src);
 }
 
@@ -244,34 +181,54 @@ struct mp_image *mp_image_new_ref(struct mp_image *img)
     if (!img)
         return NULL;
 
-    if (!img->refcount)
+    if (!img->bufs[0])
         return mp_image_new_copy(img);
 
     struct mp_image *new = talloc_ptrtype(NULL, new);
     talloc_set_destructor(new, mp_image_destructor);
     *new = *img;
 
-    m_refcount_ref(new->refcount);
-    return new;
+    bool fail = false;
+    for (int p = 0; p < MP_MAX_PLANES; p++) {
+        if (new->bufs[p]) {
+            new->bufs[p] = av_buffer_ref(new->bufs[p]);
+            if (!new->bufs[p])
+                fail = true;
+        }
+    }
+
+    if (!fail)
+        return new;
+
+    // Do this after _all_ bufs were changed; we don't want it to free bufs
+    // from the original image if this fails.
+    talloc_free(new);
+    return NULL;
 }
 
-// Return a reference counted reference to img. is_unique us used to connect to
-// an external refcounting API. It is assumed that the new object
-// has an initial reference to that external API. If free is given, that is
-// called after the last unref. All function pointers are optional.
-// On allocation failure, unref the frame and return NULL.
-static struct mp_image *mp_image_new_external_ref(struct mp_image *img, void *arg,
-                                                  bool (*is_unique)(void *arg),
-                                                  void (*free)(void *arg))
+struct free_args {
+    void *arg;
+    void (*free)(void *arg);
+};
+
+static void call_free(void *opaque, uint8_t *data)
+{
+    struct free_args *args = opaque;
+    args->free(args->arg);
+    talloc_free(args);
+}
+
+// Create a new mp_image based on img, but don't set any buffers.
+// Using this is only valid until the original img is unreferenced (including
+// implicit unreferencing of the data by mp_image_make_writeable()), unless
+// a new reference is set.
+struct mp_image *mp_image_new_dummy_ref(struct mp_image *img)
 {
     struct mp_image *new = talloc_ptrtype(NULL, new);
     talloc_set_destructor(new, mp_image_destructor);
     *new = *img;
-
-    new->refcount = m_refcount_new();
-    new->refcount->ext_is_unique = is_unique;
-    new->refcount->free = free;
-    new->refcount->arg = arg;
+    for (int p = 0; p < MP_MAX_PLANES; p++)
+        new->bufs[p] = NULL;
     return new;
 }
 
@@ -279,17 +236,36 @@ static struct mp_image *mp_image_new_external_ref(struct mp_image *img, void *ar
 // 0, call free(free_arg). The data passed by img must not be free'd before
 // that. The new reference will be writeable.
 // On allocation failure, unref the frame and return NULL.
+// This is only used for hw decoding; this is important, because libav* expects
+// all plane data to be accounted for by AVBufferRefs.
 struct mp_image *mp_image_new_custom_ref(struct mp_image *img, void *free_arg,
                                          void (*free)(void *arg))
 {
-    return mp_image_new_external_ref(img, free_arg, NULL, free);
+    assert(!img->bufs[0]);
+
+    struct mp_image *new = mp_image_new_dummy_ref(img);
+
+    struct free_args *args = talloc_ptrtype(NULL, args);
+    *args = (struct free_args){free_arg, free};
+    new->bufs[0] = av_buffer_create(NULL, 0, call_free, args,
+                                    AV_BUFFER_FLAG_READONLY);
+    if (new->bufs[0])
+        return new;
+    talloc_free(new);
+    return NULL;
 }
 
 bool mp_image_is_writeable(struct mp_image *img)
 {
-    if (!img->refcount)
+    if (!img->bufs[0])
         return true; // not ref-counted => always considered writeable
-    return m_refcount_is_unique(img->refcount);
+    for (int p = 0; p < MP_MAX_PLANES; p++) {
+        if (!img->bufs[p])
+            break;
+        if (!av_buffer_is_writable(img->bufs[p]))
+            return false;
+    }
+    return true;
 }
 
 // Make the image data referenced by img writeable. This allocates new data
@@ -663,40 +639,20 @@ void mp_image_copy_fields_to_av_frame(struct AVFrame *dst,
     dst->color_range = mp_csp_levels_to_avcol_range(src->params.colorlevels);
 }
 
-static void frame_free(void *p)
-{
-    AVFrame *frame = p;
-    av_frame_free(&frame);
-}
-
-static bool frame_is_unique(void *p)
-{
-    AVFrame *frame = p;
-    return av_frame_is_writable(frame);
-}
-
 // Create a new mp_image reference to av_frame.
 struct mp_image *mp_image_from_av_frame(struct AVFrame *av_frame)
 {
-    AVFrame *new_ref = av_frame_clone(av_frame);
-    if (!new_ref)
-        return NULL;
     struct mp_image t = {0};
-    mp_image_copy_fields_from_av_frame(&t, new_ref);
-    return mp_image_new_external_ref(&t, new_ref, frame_is_unique, frame_free);
-}
-
-static void free_img(void *opaque, uint8_t *data)
-{
-    struct mp_image *img = opaque;
-    talloc_free(img);
+    mp_image_copy_fields_from_av_frame(&t, av_frame);
+    for (int p = 0; p < MP_MAX_PLANES; p++)
+        t.bufs[p] = av_frame->buf[p];
+    return mp_image_new_ref(&t);
 }
 
 // Convert the mp_image reference to a AVFrame reference.
 // Warning: img is unreferenced (i.e. free'd). This is asymmetric to
-//          mp_image_from_av_frame(). It's done this way to allow marking the
-//          resulting AVFrame as writeable if img is the only reference (in
-//          other words, it's an optimization).
+//          mp_image_from_av_frame(). It was done as some sort of optimization,
+//          but now these semantics are pointless.
 // On failure, img is only unreffed.
 struct AVFrame *mp_image_to_av_frame_and_unref(struct mp_image *img)
 {
@@ -710,22 +666,9 @@ struct AVFrame *mp_image_to_av_frame_and_unref(struct mp_image *img)
         return NULL;
     }
     mp_image_copy_fields_to_av_frame(frame, new_ref);
-    // Caveat: if img has shared references, and all other references disappear
-    //         at a later point, the AVFrame will still be read-only.
-    int flags = 0;
-    if (!mp_image_is_writeable(new_ref))
-        flags |= AV_BUFFER_FLAG_READONLY;
-    for (int n = 0; n < new_ref->num_planes; n++) {
-        // Make it so that the actual image data is freed only if _all_ buffers
-        // are unreferenced.
-        struct mp_image *dummy_ref = mp_image_new_ref(new_ref);
-        if (!dummy_ref)
-            abort(); // out of memory (for the ref, not real image data)
-        void *ptr = new_ref->planes[n];
-        size_t size = new_ref->stride[n] * new_ref->h;
-        frame->buf[n] = av_buffer_create(ptr, size, free_img, dummy_ref, flags);
-        if (!frame->buf[n])
-            abort();
+    for (int p = 0; p < MP_MAX_PLANES; p++) {
+        frame->buf[p] = new_ref->bufs[p];
+        new_ref->bufs[p] = NULL;
     }
     talloc_free(new_ref);
     return frame;
diff --git a/video/mp_image.h b/video/mp_image.h
index b0110c1376..f5759205f4 100644
--- a/video/mp_image.h
+++ b/video/mp_image.h
@@ -90,10 +90,16 @@ typedef struct mp_image {
 
     /* only inside filter chain */
     double pts;
-    /* memory management */
-    struct m_refcount *refcount;
     /* for private use */
     void* priv;
+
+    // Reference-counted data references.
+    // These do not necessarily map directly to planes[]. They can have
+    // different order or count. There shouldn't be more buffers than planes.
+    // If bufs[n] is NULL, bufs[n+1] must also be NULL.
+    // All mp_* functions manage this automatically; do not mess with it.
+    // (See also AVFrame.buf.)
+    struct AVBufferRef *bufs[MP_MAX_PLANES];
 } mp_image_t;
 
 int mp_chroma_div_up(int size, int shift);
@@ -120,6 +126,7 @@ int mp_image_plane_h(struct mp_image *mpi, int plane);
 void mp_image_setfmt(mp_image_t* mpi, int out_fmt);
 void mp_image_steal_data(struct mp_image *dst, struct mp_image *src);
 
+struct mp_image *mp_image_new_dummy_ref(struct mp_image *img);
 struct mp_image *mp_image_new_custom_ref(struct mp_image *img, void *arg,
                                          void (*free)(void *arg));
 
diff --git a/video/mp_image_pool.c b/video/mp_image_pool.c
index 173c018ec3..5992c631f9 100644
--- a/video/mp_image_pool.c
+++ b/video/mp_image_pool.c
@@ -22,6 +22,8 @@
 #include <pthread.h>
 #include <assert.h>
 
+#include <libavutil/buffer.h>
+
 #include "talloc.h"
 
 #include "common/common.h"
@@ -94,9 +96,9 @@ void mp_image_pool_clear(struct mp_image_pool *pool)
 
 // This is the only function that is allowed to run in a different thread.
 // (Consider passing an image to another thread, which frees it.)
-static void unref_image(void *ptr)
+static void unref_image(void *opaque, uint8_t *data)
 {
-    struct mp_image *img = ptr;
+    struct mp_image *img = opaque;
     struct image_flags *it = img->priv;
     bool alive;
     pool_lock();
@@ -135,11 +137,31 @@ struct mp_image *mp_image_pool_get_no_alloc(struct mp_image_pool *pool, int fmt,
     pool_unlock();
     if (!new)
         return NULL;
+
+    // Reference the new image. Since mp_image_pool is not declared thread-safe,
+    // and unreffing images from other threads does not allocate new images,
+    // no synchronization is required here.
+    for (int p = 0; p < MP_MAX_PLANES; p++)
+        assert(!!new->bufs[p] == !p); // only 1 AVBufferRef
+
+    struct mp_image *ref = mp_image_new_dummy_ref(new);
+
+    // This assumes the buffer is at this point exclusively owned by us: we
+    // can't track whether the buffer is unique otherwise.
+    // (av_buffer_is_writable() checks the refcount of the new buffer only.)
+    int flags = av_buffer_is_writable(new->bufs[0]) ? 0 : AV_BUFFER_FLAG_READONLY;
+    ref->bufs[0] = av_buffer_create(new->bufs[0]->data, new->bufs[0]->size,
+                                    unref_image, new, flags);
+    if (!ref->bufs[0]) {
+        talloc_free(ref);
+        return NULL;
+    }
+
     struct image_flags *it = new->priv;
     assert(!it->referenced && it->pool_alive);
     it->referenced = true;
     it->order = ++pool->lru_counter;
-    return mp_image_new_custom_ref(new, new, unref_image);
+    return ref;
 }
 
 // Return a new image of given format/size. The only difference to
@@ -204,6 +226,10 @@ bool mp_image_pool_make_writeable(struct mp_image_pool *pool,
     return true;
 }
 
+// Call cb(cb_data, fmt, w, h) to allocate an image. Note that the resulting
+// image must use only 1 AVBufferRef. The returned image must also be owned
+// exclusively by the image pool, otherwise mp_image_is_writeable() will not
+// work due to FFmpeg restrictions.
 void mp_image_pool_set_allocator(struct mp_image_pool *pool,
                                  mp_image_allocator cb, void  *cb_data)
 {