[FFmpeg-devel] [PATCH 18/42] avcodec/h264dec: Use RefStruct-pool API instead of AVBufferPool API

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

It involves less allocations and therefore has the nice property
that deriving a reference from a reference can't fail.
This allows for considerable simplifications in
ff_h264_(ref|replace)_picture().
Switching to the RefStruct API also allows to make H264Picture
smaller, because some AVBufferRef* pointers could be removed
without replacement.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/h264_picture.c | 72 +++++++++++----------------------------
 libavcodec/h264_slice.c   | 44 ++++++++++++------------
 libavcodec/h264dec.c      | 19 ++++++-----
 libavcodec/h264dec.h      | 23 ++++++-------
 4 files changed, 62 insertions(+), 96 deletions(-)

diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c
index 9353e4b445..c1df9b8d05 100644
--- a/libavcodec/h264_picture.c
+++ b/libavcodec/h264_picture.c
@@ -48,29 +48,39 @@ void ff_h264_unref_picture(H264Context *h, H264Picture *pic)
     ff_thread_release_buffer(h->avctx, pic->f_grain);
     ff_refstruct_unref(&pic->hwaccel_picture_private);
 
-    av_buffer_unref(&pic->qscale_table_buf);
-    av_buffer_unref(&pic->mb_type_buf);
+    ff_refstruct_unref(&pic->qscale_table_base);
+    ff_refstruct_unref(&pic->mb_type_base);
     ff_refstruct_unref(&pic->pps);
     for (i = 0; i < 2; i++) {
-        av_buffer_unref(&pic->motion_val_buf[i]);
-        av_buffer_unref(&pic->ref_index_buf[i]);
+        ff_refstruct_unref(&pic->motion_val_base[i]);
+        ff_refstruct_unref(&pic->ref_index[i]);
     }
-    av_buffer_unref(&pic->decode_error_flags);
+    ff_refstruct_unref(&pic->decode_error_flags);
 
     memset((uint8_t*)pic + off, 0, sizeof(*pic) - off);
 }
 
 static void h264_copy_picture_params(H264Picture *dst, const H264Picture *src)
 {
+    ff_refstruct_replace(&dst->qscale_table_base, src->qscale_table_base);
+    ff_refstruct_replace(&dst->mb_type_base,      src->mb_type_base);
     ff_refstruct_replace(&dst->pps, src->pps);
 
+    for (int i = 0; i < 2; i++) {
+        ff_refstruct_replace(&dst->motion_val_base[i], src->motion_val_base[i]);
+        ff_refstruct_replace(&dst->ref_index[i],       src->ref_index[i]);
+    }
+
+    ff_refstruct_replace(&dst->hwaccel_picture_private,
+                          src->hwaccel_picture_private);
+
+    ff_refstruct_replace(&dst->decode_error_flags, src->decode_error_flags);
+
     dst->qscale_table = src->qscale_table;
     dst->mb_type      = src->mb_type;
 
-    for (int i = 0; i < 2; i++) {
+    for (int i = 0; i < 2; i++)
         dst->motion_val[i] = src->motion_val[i];
-        dst->ref_index[i]  = src->ref_index[i];
-    }
 
     for (int i = 0; i < 2; i++)
         dst->field_poc[i] = src->field_poc[i];
@@ -96,7 +106,7 @@ static void h264_copy_picture_params(H264Picture *dst, const H264Picture *src)
 
 int ff_h264_ref_picture(H264Context *h, H264Picture *dst, H264Picture *src)
 {
-    int ret, i;
+    int ret;
 
     av_assert0(!dst->f->buf[0]);
     av_assert0(src->f->buf[0]);
@@ -113,29 +123,6 @@ int ff_h264_ref_picture(H264Context *h, H264Picture *dst, H264Picture *src)
             goto fail;
     }
 
-    dst->qscale_table_buf = av_buffer_ref(src->qscale_table_buf);
-    dst->mb_type_buf      = av_buffer_ref(src->mb_type_buf);
-    if (!dst->qscale_table_buf || !dst->mb_type_buf) {
-        ret = AVERROR(ENOMEM);
-        goto fail;
-    }
-
-    for (i = 0; i < 2; i++) {
-        dst->motion_val_buf[i] = av_buffer_ref(src->motion_val_buf[i]);
-        dst->ref_index_buf[i]  = av_buffer_ref(src->ref_index_buf[i]);
-        if (!dst->motion_val_buf[i] || !dst->ref_index_buf[i]) {
-            ret = AVERROR(ENOMEM);
-            goto fail;
-        }
-    }
-
-    ff_refstruct_replace(&dst->hwaccel_picture_private,
-                          src->hwaccel_picture_private);
-
-    ret = av_buffer_replace(&dst->decode_error_flags, src->decode_error_flags);
-    if (ret < 0)
-        goto fail;
-
     h264_copy_picture_params(dst, src);
 
     return 0;
@@ -146,7 +133,7 @@ fail:
 
 int ff_h264_replace_picture(H264Context *h, H264Picture *dst, const H264Picture *src)
 {
-    int ret, i;
+    int ret;
 
     if (!src->f || !src->f->buf[0]) {
         ff_h264_unref_picture(h, dst);
@@ -167,25 +154,6 @@ int ff_h264_replace_picture(H264Context *h, H264Picture *dst, const H264Picture
             goto fail;
     }
 
-    ret  = av_buffer_replace(&dst->qscale_table_buf, src->qscale_table_buf);
-    ret |= av_buffer_replace(&dst->mb_type_buf, src->mb_type_buf);
-    if (ret < 0)
-        goto fail;
-
-    for (i = 0; i < 2; i++) {
-        ret  = av_buffer_replace(&dst->motion_val_buf[i], src->motion_val_buf[i]);
-        ret |= av_buffer_replace(&dst->ref_index_buf[i], src->ref_index_buf[i]);
-        if (ret < 0)
-            goto fail;
-    }
-
-    ff_refstruct_replace(&dst->hwaccel_picture_private,
-                          src->hwaccel_picture_private);
-
-    ret = av_buffer_replace(&dst->decode_error_flags, src->decode_error_flags);
-    if (ret < 0)
-        goto fail;
-
     h264_copy_picture_params(dst, src);
 
     return 0;
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 6df80c9522..3e21caeda7 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -165,20 +165,19 @@ static int init_table_pools(H264Context *h)
     const int b4_stride     = h->mb_width * 4 + 1;
     const int b4_array_size = b4_stride * h->mb_height * 4;
 
-    h->qscale_table_pool = av_buffer_pool_init(big_mb_num + h->mb_stride,
-                                               av_buffer_allocz);
-    h->mb_type_pool      = av_buffer_pool_init((big_mb_num + h->mb_stride) *
-                                               sizeof(uint32_t), av_buffer_allocz);
-    h->motion_val_pool   = av_buffer_pool_init(2 * (b4_array_size + 4) *
-                                               sizeof(int16_t), av_buffer_allocz);
-    h->ref_index_pool    = av_buffer_pool_init(4 * mb_array_size, av_buffer_allocz);
+    h->qscale_table_pool = ff_refstruct_pool_alloc(big_mb_num + h->mb_stride, 0);
+    h->mb_type_pool      = ff_refstruct_pool_alloc((big_mb_num + h->mb_stride) *
+                                                   sizeof(uint32_t), 0);
+    h->motion_val_pool   = ff_refstruct_pool_alloc(2 * (b4_array_size + 4) *
+                                                   sizeof(int16_t), 0);
+    h->ref_index_pool    = ff_refstruct_pool_alloc(4 * mb_array_size, 0);
 
     if (!h->qscale_table_pool || !h->mb_type_pool || !h->motion_val_pool ||
         !h->ref_index_pool) {
-        av_buffer_pool_uninit(&h->qscale_table_pool);
-        av_buffer_pool_uninit(&h->mb_type_pool);
-        av_buffer_pool_uninit(&h->motion_val_pool);
-        av_buffer_pool_uninit(&h->ref_index_pool);
+        ff_refstruct_pool_uninit(&h->qscale_table_pool);
+        ff_refstruct_pool_uninit(&h->mb_type_pool);
+        ff_refstruct_pool_uninit(&h->motion_val_pool);
+        ff_refstruct_pool_uninit(&h->ref_index_pool);
         return AVERROR(ENOMEM);
     }
 
@@ -211,10 +210,10 @@ static int alloc_picture(H264Context *h, H264Picture *pic)
         goto fail;
 
     if (h->decode_error_flags_pool) {
-        pic->decode_error_flags = av_buffer_pool_get(h->decode_error_flags_pool);
+        pic->decode_error_flags = ff_refstruct_pool_get(h->decode_error_flags_pool);
         if (!pic->decode_error_flags)
             goto fail;
-        atomic_init((atomic_int*)pic->decode_error_flags->data, 0);
+        atomic_init(pic->decode_error_flags, 0);
     }
 
     if (CONFIG_GRAY && !h->avctx->hwaccel && h->flags & AV_CODEC_FLAG_GRAY && pic->f->data[2]) {
@@ -236,22 +235,21 @@ static int alloc_picture(H264Context *h, H264Picture *pic)
             goto fail;
     }
 
-    pic->qscale_table_buf = av_buffer_pool_get(h->qscale_table_pool);
-    pic->mb_type_buf      = av_buffer_pool_get(h->mb_type_pool);
-    if (!pic->qscale_table_buf || !pic->mb_type_buf)
+    pic->qscale_table_base = ff_refstruct_pool_get(h->qscale_table_pool);
+    pic->mb_type_base      = ff_refstruct_pool_get(h->mb_type_pool);
+    if (!pic->qscale_table_base || !pic->mb_type_base)
         goto fail;
 
-    pic->mb_type      = (uint32_t*)pic->mb_type_buf->data + 2 * h->mb_stride + 1;
-    pic->qscale_table = pic->qscale_table_buf->data + 2 * h->mb_stride + 1;
+    pic->mb_type      = pic->mb_type_base + 2 * h->mb_stride + 1;
+    pic->qscale_table = pic->qscale_table_base + 2 * h->mb_stride + 1;
 
     for (i = 0; i < 2; i++) {
-        pic->motion_val_buf[i] = av_buffer_pool_get(h->motion_val_pool);
-        pic->ref_index_buf[i]  = av_buffer_pool_get(h->ref_index_pool);
-        if (!pic->motion_val_buf[i] || !pic->ref_index_buf[i])
+        pic->motion_val_base[i] = ff_refstruct_pool_get(h->motion_val_pool);
+        pic->ref_index[i]       = ff_refstruct_pool_get(h->ref_index_pool);
+        if (!pic->motion_val_base[i] || !pic->ref_index[i])
             goto fail;
 
-        pic->motion_val[i] = (int16_t (*)[2])pic->motion_val_buf[i]->data + 4;
-        pic->ref_index[i]  = pic->ref_index_buf[i]->data;
+        pic->motion_val[i] = pic->motion_val_base[i] + 4;
     }
 
     pic->pps = ff_refstruct_ref_c(h->ps.pps);
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 796f80be8d..8578d3b346 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -51,6 +51,7 @@
 #include "mpegutils.h"
 #include "profiles.h"
 #include "rectangle.h"
+#include "refstruct.h"
 #include "thread.h"
 #include "threadframe.h"
 
@@ -151,10 +152,10 @@ void ff_h264_free_tables(H264Context *h)
     av_freep(&h->mb2b_xy);
     av_freep(&h->mb2br_xy);
 
-    av_buffer_pool_uninit(&h->qscale_table_pool);
-    av_buffer_pool_uninit(&h->mb_type_pool);
-    av_buffer_pool_uninit(&h->motion_val_pool);
-    av_buffer_pool_uninit(&h->ref_index_pool);
+    ff_refstruct_pool_uninit(&h->qscale_table_pool);
+    ff_refstruct_pool_uninit(&h->mb_type_pool);
+    ff_refstruct_pool_uninit(&h->motion_val_pool);
+    ff_refstruct_pool_uninit(&h->ref_index_pool);
 
 #if CONFIG_ERROR_RESILIENCE
     av_freep(&h->er.mb_index2xy);
@@ -308,7 +309,7 @@ static int h264_init_context(AVCodecContext *avctx, H264Context *h)
     ff_h264_sei_uninit(&h->sei);
 
     if (avctx->active_thread_type & FF_THREAD_FRAME) {
-        h->decode_error_flags_pool = av_buffer_pool_init(sizeof(atomic_int), NULL);
+        h->decode_error_flags_pool = ff_refstruct_pool_alloc(sizeof(atomic_int), 0);
         if (!h->decode_error_flags_pool)
             return AVERROR(ENOMEM);
     }
@@ -359,7 +360,7 @@ static av_cold int h264_decode_end(AVCodecContext *avctx)
 
     h->cur_pic_ptr = NULL;
 
-    av_buffer_pool_uninit(&h->decode_error_flags_pool);
+    ff_refstruct_pool_uninit(&h->decode_error_flags_pool);
 
     av_freep(&h->slice_ctx);
     h->nb_slice_ctx = 0;
@@ -749,7 +750,7 @@ static int decode_nal_units(H264Context *h, const uint8_t *buf, int buf_size)
     if ((ret < 0 || h->er.error_occurred) && h->cur_pic_ptr) {
         if (h->cur_pic_ptr->decode_error_flags) {
             /* Frame-threading in use */
-            atomic_int *decode_error = (atomic_int*)h->cur_pic_ptr->decode_error_flags->data;
+            atomic_int *decode_error = h->cur_pic_ptr->decode_error_flags;
             /* Using atomics here is not supposed to provide syncronisation;
              * they are merely used to allow to set decode_error from both
              * decoding threads in case of coded slices. */
@@ -800,7 +801,7 @@ end:
         ff_er_frame_end(&h->er, &decode_error_flags);
         if (decode_error_flags) {
             if (h->cur_pic_ptr->decode_error_flags) {
-                atomic_int *decode_error = (atomic_int*)h->cur_pic_ptr->decode_error_flags->data;
+                atomic_int *decode_error = h->cur_pic_ptr->decode_error_flags;
                 atomic_fetch_or_explicit(decode_error, decode_error_flags,
                                          memory_order_relaxed);
             } else
@@ -878,7 +879,7 @@ static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp)
         return ret;
 
     if (srcp->decode_error_flags) {
-        atomic_int *decode_error = (atomic_int*)srcp->decode_error_flags->data;
+        atomic_int *decode_error = srcp->decode_error_flags;
         /* The following is not supposed to provide synchronisation at all:
          * given that srcp has already finished decoding, decode_error
          * has already been set to its final value. */
diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
index 7436e0a54f..a330e0a24f 100644
--- a/libavcodec/h264dec.h
+++ b/libavcodec/h264dec.h
@@ -108,19 +108,18 @@ typedef struct H264Picture {
 
     AVFrame *f_grain;
 
-    AVBufferRef *qscale_table_buf;
+    int8_t *qscale_table_base;        ///< RefStruct reference
     int8_t *qscale_table;
 
-    AVBufferRef *motion_val_buf[2];
+    int16_t (*motion_val_base[2])[2]; ///< RefStruct reference
     int16_t (*motion_val[2])[2];
 
-    AVBufferRef *mb_type_buf;
+    uint32_t *mb_type_base;           ///< RefStruct reference
     uint32_t *mb_type;
 
     void *hwaccel_picture_private; ///< hardware accelerator private data
 
-    AVBufferRef *ref_index_buf[2];
-    int8_t *ref_index[2];
+    int8_t *ref_index[2];   ///< RefStruct reference
 
     int field_poc[2];       ///< top/bottom POC
     int poc;                ///< frame POC
@@ -151,8 +150,8 @@ typedef struct H264Picture {
     int mb_width, mb_height;
     int mb_stride;
 
-    /* data points to an atomic_int */
-    AVBufferRef *decode_error_flags;
+    /// RefStruct reference; its pointee is shared between decoding threads.
+    atomic_int *decode_error_flags;
 } H264Picture;
 
 typedef struct H264Ref {
@@ -546,11 +545,11 @@ typedef struct H264Context {
 
     H264SEIContext sei;
 
-    AVBufferPool *qscale_table_pool;
-    AVBufferPool *mb_type_pool;
-    AVBufferPool *motion_val_pool;
-    AVBufferPool *ref_index_pool;
-    AVBufferPool *decode_error_flags_pool;
+    struct FFRefStructPool *qscale_table_pool;
+    struct FFRefStructPool *mb_type_pool;
+    struct FFRefStructPool *motion_val_pool;
+    struct FFRefStructPool *ref_index_pool;
+    struct FFRefStructPool *decode_error_flags_pool;
     int ref2frm[MAX_SLICES][2][64];     ///< reference to frame number lists, used in the loop filter, the first 2 are for -2,-1
 } H264Context;
 
-- 
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".