[FFmpeg-devel] [PATCH 04/42] avcodec/h264_ps: Use RefStruct API for SPS/PPS

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

Avoids allocations and error checks for these allocations;
e.g. syncing buffers across threads can't fail any more
and needn't be checked. It also avoids having to keep
H264ParamSets.pps and H264ParamSets.pps_ref and PPS.sps
and PPS.sps_ref in sync and gets rid of casts and indirections.

(The removal of these checks and the syncing code even more
than offset the additional code for RefStruct.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/h264_parser.c   |  9 ++----
 libavcodec/h264_picture.c  | 10 +++----
 libavcodec/h264_ps.c       | 57 +++++++++++++-------------------------
 libavcodec/h264_ps.h       | 13 ++++-----
 libavcodec/h264_refs.c     |  2 +-
 libavcodec/h264_sei.c      |  2 +-
 libavcodec/h264_slice.c    | 41 ++++++++-------------------
 libavcodec/h264dec.h       |  1 -
 libavcodec/mediacodecdec.c |  4 +--
 libavcodec/vulkan_h264.c   |  4 +--
 10 files changed, 50 insertions(+), 93 deletions(-)

diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 43abc45f9c..3574010a64 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -47,6 +47,7 @@
 #include "h264data.h"
 #include "mpegutils.h"
 #include "parser.h"
+#include "refstruct.h"
 #include "startcode.h"
 
 typedef struct H264ParseContext {
@@ -373,13 +374,7 @@ static inline int parse_nal_units(AVCodecParserContext *s,
                 goto fail;
             }
 
-            av_buffer_unref(&p->ps.pps_ref);
-            p->ps.pps = NULL;
-            p->ps.sps = NULL;
-            p->ps.pps_ref = av_buffer_ref(p->ps.pps_list[pps_id]);
-            if (!p->ps.pps_ref)
-                goto fail;
-            p->ps.pps = (const PPS*)p->ps.pps_ref->data;
+            ff_refstruct_replace(&p->ps.pps, p->ps.pps_list[pps_id]);
             p->ps.sps = p->ps.pps->sps;
             sps       = p->ps.sps;
 
diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c
index 31b5e231c2..25d0d96ddb 100644
--- a/libavcodec/h264_picture.c
+++ b/libavcodec/h264_picture.c
@@ -32,6 +32,7 @@
 #include "h264dec.h"
 #include "hwaccel_internal.h"
 #include "mpegutils.h"
+#include "refstruct.h"
 #include "thread.h"
 #include "threadframe.h"
 
@@ -49,7 +50,7 @@ void ff_h264_unref_picture(H264Context *h, H264Picture *pic)
 
     av_buffer_unref(&pic->qscale_table_buf);
     av_buffer_unref(&pic->mb_type_buf);
-    av_buffer_unref(&pic->pps_buf);
+    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]);
@@ -61,9 +62,10 @@ void ff_h264_unref_picture(H264Context *h, H264Picture *pic)
 
 static void h264_copy_picture_params(H264Picture *dst, const H264Picture *src)
 {
+    ff_refstruct_replace(&dst->pps, src->pps);
+
     dst->qscale_table = src->qscale_table;
     dst->mb_type      = src->mb_type;
-    dst->pps          = src->pps;
 
     for (int i = 0; i < 2; i++) {
         dst->motion_val[i] = src->motion_val[i];
@@ -113,8 +115,7 @@ int ff_h264_ref_picture(H264Context *h, H264Picture *dst, H264Picture *src)
 
     dst->qscale_table_buf = av_buffer_ref(src->qscale_table_buf);
     dst->mb_type_buf      = av_buffer_ref(src->mb_type_buf);
-    dst->pps_buf          = av_buffer_ref(src->pps_buf);
-    if (!dst->qscale_table_buf || !dst->mb_type_buf || !dst->pps_buf) {
+    if (!dst->qscale_table_buf || !dst->mb_type_buf) {
         ret = AVERROR(ENOMEM);
         goto fail;
     }
@@ -174,7 +175,6 @@ int ff_h264_replace_picture(H264Context *h, H264Picture *dst, const H264Picture
 
     ret  = av_buffer_replace(&dst->qscale_table_buf, src->qscale_table_buf);
     ret |= av_buffer_replace(&dst->mb_type_buf, src->mb_type_buf);
-    ret |= av_buffer_replace(&dst->pps_buf, src->pps_buf);
     if (ret < 0)
         goto fail;
 
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index 53446e9aab..dcc51b96db 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -34,6 +34,7 @@
 #include "h2645_vui.h"
 #include "h264_ps.h"
 #include "golomb.h"
+#include "refstruct.h"
 
 #define MIN_LOG2_MAX_FRAME_NUM    4
 
@@ -85,7 +86,7 @@ static const int level_max_dpb_mbs[][2] = {
 
 static void remove_pps(H264ParamSets *s, int id)
 {
-    av_buffer_unref(&s->pps_list[id]);
+    ff_refstruct_unref(&s->pps_list[id]);
 }
 
 static void remove_sps(H264ParamSets *s, int id)
@@ -95,11 +96,11 @@ static void remove_sps(H264ParamSets *s, int id)
     if (s->sps_list[id]) {
         /* drop all PPS that depend on this SPS */
         for (i = 0; i < FF_ARRAY_ELEMS(s->pps_list); i++)
-            if (s->pps_list[i] && ((PPS*)s->pps_list[i]->data)->sps_id == id)
+            if (s->pps_list[i] && s->pps_list[i]->sps_id == id)
                 remove_pps(s, i);
     }
 #endif
-    av_buffer_unref(&s->sps_list[id]);
+    ff_refstruct_unref(&s->sps_list[id]);
 }
 
 static inline int decode_hrd_parameters(GetBitContext *gb, void *logctx,
@@ -271,31 +272,27 @@ void ff_h264_ps_uninit(H264ParamSets *ps)
     int i;
 
     for (i = 0; i < MAX_SPS_COUNT; i++)
-        av_buffer_unref(&ps->sps_list[i]);
+        ff_refstruct_unref(&ps->sps_list[i]);
 
     for (i = 0; i < MAX_PPS_COUNT; i++)
-        av_buffer_unref(&ps->pps_list[i]);
+        ff_refstruct_unref(&ps->pps_list[i]);
 
-    av_buffer_unref(&ps->pps_ref);
-
-    ps->pps = NULL;
+    ff_refstruct_unref(&ps->pps);
     ps->sps = NULL;
 }
 
 int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
                                      H264ParamSets *ps, int ignore_truncation)
 {
-    AVBufferRef *sps_buf;
     int profile_idc, level_idc, constraint_set_flags = 0;
     unsigned int sps_id;
     int i, log2_max_frame_num_minus4;
     SPS *sps;
     int ret;
 
-    sps_buf = av_buffer_allocz(sizeof(*sps));
-    if (!sps_buf)
+    sps = ff_refstruct_allocz(sizeof(*sps));
+    if (!sps)
         return AVERROR(ENOMEM);
-    sps = (SPS*)sps_buf->data;
 
     sps->data_size = gb->buffer_end - gb->buffer;
     if (sps->data_size > sizeof(sps->data)) {
@@ -580,17 +577,17 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
      * original one.
      * otherwise drop all PPSes that depend on it */
     if (ps->sps_list[sps_id] &&
-        !memcmp(ps->sps_list[sps_id]->data, sps_buf->data, sps_buf->size)) {
-        av_buffer_unref(&sps_buf);
+        !memcmp(ps->sps_list[sps_id], sps, sizeof(*sps))) {
+        ff_refstruct_unref(&sps);
     } else {
         remove_sps(ps, sps_id);
-        ps->sps_list[sps_id] = sps_buf;
+        ps->sps_list[sps_id] = sps;
     }
 
     return 0;
 
 fail:
-    av_buffer_unref(&sps_buf);
+    ff_refstruct_unref(&sps);
     return AVERROR_INVALIDDATA;
 }
 
@@ -689,19 +686,16 @@ static int more_rbsp_data_in_pps(const SPS *sps, void *logctx)
     return 1;
 }
 
-static void pps_free(void *opaque, uint8_t *data)
+static void pps_free(FFRefStructOpaque unused, void *obj)
 {
-    PPS *pps = (PPS*)data;
-
-    av_buffer_unref(&pps->sps_ref);
+    PPS *pps = obj;
 
-    av_freep(&data);
+    ff_refstruct_unref(&pps->sps);
 }
 
 int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avctx,
                                          H264ParamSets *ps, int bit_length)
 {
-    AVBufferRef *pps_buf;
     const SPS *sps;
     unsigned int pps_id = get_ue_golomb(gb);
     PPS *pps;
@@ -714,15 +708,9 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
         return AVERROR_INVALIDDATA;
     }
 
-    pps = av_mallocz(sizeof(*pps));
+    pps = ff_refstruct_alloc_ext(sizeof(*pps), 0, NULL, pps_free);
     if (!pps)
         return AVERROR(ENOMEM);
-    pps_buf = av_buffer_create((uint8_t*)pps, sizeof(*pps),
-                               pps_free, NULL, 0);
-    if (!pps_buf) {
-        av_freep(&pps);
-        return AVERROR(ENOMEM);
-    }
 
     pps->data_size = gb->buffer_end - gb->buffer;
     if (pps->data_size > sizeof(pps->data)) {
@@ -745,12 +733,7 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
         ret = AVERROR_INVALIDDATA;
         goto fail;
     }
-    pps->sps_ref = av_buffer_ref(ps->sps_list[pps->sps_id]);
-    if (!pps->sps_ref) {
-        ret = AVERROR(ENOMEM);
-        goto fail;
-    }
-    pps->sps = (const SPS*)pps->sps_ref->data;
+    pps->sps = ff_refstruct_ref_c(ps->sps_list[pps->sps_id]);
     sps      = pps->sps;
 
     if (sps->bit_depth_luma > 14) {
@@ -852,11 +835,11 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
     }
 
     remove_pps(ps, pps_id);
-    ps->pps_list[pps_id] = pps_buf;
+    ps->pps_list[pps_id] = pps;
 
     return 0;
 
 fail:
-    av_buffer_unref(&pps_buf);
+    ff_refstruct_unref(&pps);
     return ret;
 }
diff --git a/libavcodec/h264_ps.h b/libavcodec/h264_ps.h
index e675619635..80af4832fe 100644
--- a/libavcodec/h264_ps.h
+++ b/libavcodec/h264_ps.h
@@ -26,7 +26,6 @@
 
 #include <stdint.h>
 
-#include "libavutil/buffer.h"
 #include "libavutil/pixfmt.h"
 #include "libavutil/rational.h"
 
@@ -139,18 +138,16 @@ typedef struct PPS {
     uint32_t(*dequant4_coeff[6])[16];
     uint32_t(*dequant8_coeff[6])[64];
 
-    AVBufferRef *sps_ref;
-    const SPS   *sps;
+    const SPS   *sps; ///< RefStruct reference
 } PPS;
 
 typedef struct H264ParamSets {
-    AVBufferRef *sps_list[MAX_SPS_COUNT];
-    AVBufferRef *pps_list[MAX_PPS_COUNT];
+    const SPS *sps_list[MAX_SPS_COUNT]; ///< RefStruct references
+    const PPS *pps_list[MAX_PPS_COUNT]; ///< RefStruct references
 
-    AVBufferRef *pps_ref;
     /* currently active parameters sets */
-    const PPS *pps;
-    const SPS *sps;
+    const PPS *pps; ///< RefStruct reference
+    const SPS *sps; ///< ordinary pointer, no RefStruct reference
 
     int overread_warning_printed[2];
 } H264ParamSets;
diff --git a/libavcodec/h264_refs.c b/libavcodec/h264_refs.c
index 50bbe94917..1b24c493df 100644
--- a/libavcodec/h264_refs.c
+++ b/libavcodec/h264_refs.c
@@ -807,7 +807,7 @@ int ff_h264_execute_ref_pic_marking(H264Context *h)
 
     for (i = 0; i < FF_ARRAY_ELEMS(h->ps.pps_list); i++) {
         if (h->ps.pps_list[i]) {
-            const PPS *pps = (const PPS *)h->ps.pps_list[i]->data;
+            const PPS *pps = h->ps.pps_list[i];
             pps_ref_count[0] = FFMAX(pps_ref_count[0], pps->ref_count[0]);
             pps_ref_count[1] = FFMAX(pps_ref_count[1], pps->ref_count[1]);
         }
diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
index 668204959f..8d6dc77943 100644
--- a/libavcodec/h264_sei.c
+++ b/libavcodec/h264_sei.c
@@ -179,7 +179,7 @@ static int decode_buffering_period(H264SEIBufferingPeriod *h, GetBitContext *gb,
                "non-existing SPS %d referenced in buffering period\n", sps_id);
         return sps_id > 31 ? AVERROR_INVALIDDATA : AVERROR_PS_NOT_FOUND;
     }
-    sps = (const SPS*)ps->sps_list[sps_id]->data;
+    sps = ps->sps_list[sps_id];
 
     // NOTE: This is really so duplicated in the standard... See H.264, D.1.1
     if (sps->nal_hrd_parameters_present_flag) {
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 5657327f0c..632f5b23b2 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -44,6 +44,7 @@
 #include "mathops.h"
 #include "mpegutils.h"
 #include "rectangle.h"
+#include "refstruct.h"
 #include "thread.h"
 #include "threadframe.h"
 
@@ -254,10 +255,7 @@ static int alloc_picture(H264Context *h, H264Picture *pic)
         pic->ref_index[i]  = pic->ref_index_buf[i]->data;
     }
 
-    pic->pps_buf = av_buffer_ref(h->ps.pps_ref);
-    if (!pic->pps_buf)
-        goto fail;
-    pic->pps = (const PPS*)pic->pps_buf->data;
+    pic->pps = ff_refstruct_ref_c(h->ps.pps);
 
     pic->mb_width  = h->mb_width;
     pic->mb_height = h->mb_height;
@@ -363,25 +361,16 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
 
     // SPS/PPS
     for (i = 0; i < FF_ARRAY_ELEMS(h->ps.sps_list); i++) {
-        ret = av_buffer_replace(&h->ps.sps_list[i], h1->ps.sps_list[i]);
-        if (ret < 0)
-            return ret;
+        ff_refstruct_replace(&h->ps.sps_list[i], h1->ps.sps_list[i]);
     }
     for (i = 0; i < FF_ARRAY_ELEMS(h->ps.pps_list); i++) {
-        ret = av_buffer_replace(&h->ps.pps_list[i], h1->ps.pps_list[i]);
-        if (ret < 0)
-            return ret;
+        ff_refstruct_replace(&h->ps.pps_list[i], h1->ps.pps_list[i]);
     }
 
-    ret = av_buffer_replace(&h->ps.pps_ref, h1->ps.pps_ref);
-    if (ret < 0)
-        return ret;
-    h->ps.pps = NULL;
+    ff_refstruct_replace(&h->ps.pps, h1->ps.pps);
     h->ps.sps = NULL;
-    if (h1->ps.pps_ref) {
-        h->ps.pps = (const PPS*)h->ps.pps_ref->data;
+    if (h1->ps.pps)
         h->ps.sps = h->ps.pps->sps;
-    }
 
     if (need_reinit || !inited) {
         h->width     = h1->width;
@@ -935,7 +924,7 @@ static enum AVPixelFormat get_pixel_format(H264Context *h, int force_callback)
 /* export coded and cropped frame dimensions to AVCodecContext */
 static void init_dimensions(H264Context *h)
 {
-    const SPS *sps = (const SPS*)h->ps.sps;
+    const SPS *sps = h->ps.sps;
     int cr = sps->crop_right;
     int cl = sps->crop_left;
     int ct = sps->crop_top;
@@ -1071,17 +1060,11 @@ static int h264_init_ps(H264Context *h, const H264SliceContext *sl, int first_sl
     const SPS *sps;
     int needs_reinit = 0, must_reinit, ret;
 
-    if (first_slice) {
-        av_buffer_unref(&h->ps.pps_ref);
-        h->ps.pps = NULL;
-        h->ps.pps_ref = av_buffer_ref(h->ps.pps_list[sl->pps_id]);
-        if (!h->ps.pps_ref)
-            return AVERROR(ENOMEM);
-        h->ps.pps = (const PPS*)h->ps.pps_ref->data;
-    }
+    if (first_slice)
+        ff_refstruct_replace(&h->ps.pps, h->ps.pps_list[sl->pps_id]);
 
     if (h->ps.sps != h->ps.pps->sps) {
-        h->ps.sps = (const SPS*)h->ps.pps->sps;
+        h->ps.sps = h->ps.pps->sps;
 
         if (h->mb_width  != h->ps.sps->mb_width ||
             h->mb_height != h->ps.sps->mb_height ||
@@ -1753,7 +1736,7 @@ static int h264_slice_header_parse(const H264Context *h, H264SliceContext *sl,
                sl->pps_id);
         return AVERROR_INVALIDDATA;
     }
-    pps = (const PPS*)h->ps.pps_list[sl->pps_id]->data;
+    pps = h->ps.pps_list[sl->pps_id];
     sps = pps->sps;
 
     sl->frame_num = get_bits(&sl->gb, sps->log2_max_frame_num);
@@ -2140,7 +2123,7 @@ int ff_h264_queue_decode_slice(H264Context *h, const H2645NAL *nal)
     }
 
     if (!first_slice) {
-        const PPS *pps = (const PPS*)h->ps.pps_list[sl->pps_id]->data;
+        const PPS *pps = h->ps.pps_list[sl->pps_id];
 
         if (h->ps.pps->sps_id != pps->sps_id ||
             h->ps.pps->transform_8x8_mode != pps->transform_8x8_mode /*||
diff --git a/libavcodec/h264dec.h b/libavcodec/h264dec.h
index 322c06a19c..513856749a 100644
--- a/libavcodec/h264dec.h
+++ b/libavcodec/h264dec.h
@@ -147,7 +147,6 @@ typedef struct H264Picture {
     int sei_recovery_frame_cnt;
     int needs_fg;           ///< whether picture needs film grain synthesis (see `f_grain`)
 
-    AVBufferRef *pps_buf;
     const PPS   *pps;
 
     int mb_width, mb_height;
diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
index 44f55947be..52b3a2c1f7 100644
--- a/libavcodec/mediacodecdec.c
+++ b/libavcodec/mediacodecdec.c
@@ -146,14 +146,14 @@ static int h264_set_extradata(AVCodecContext *avctx, FFAMediaFormat *format)
 
     for (i = 0; i < MAX_PPS_COUNT; i++) {
         if (ps.pps_list[i]) {
-            pps = (const PPS*)ps.pps_list[i]->data;
+            pps = ps.pps_list[i];
             break;
         }
     }
 
     if (pps) {
         if (ps.sps_list[pps->sps_id]) {
-            sps = (const SPS*)ps.sps_list[pps->sps_id]->data;
+            sps = ps.sps_list[pps->sps_id];
         }
     }
 
diff --git a/libavcodec/vulkan_h264.c b/libavcodec/vulkan_h264.c
index 32ef32d640..cdc2c7fe30 100644
--- a/libavcodec/vulkan_h264.c
+++ b/libavcodec/vulkan_h264.c
@@ -321,7 +321,7 @@ static int vk_h264_create_params(AVCodecContext *avctx, AVBufferRef **buf)
     /* SPS list */
     for (int i = 0; i < FF_ARRAY_ELEMS(h->ps.sps_list); i++) {
         if (h->ps.sps_list[i]) {
-            const SPS *sps_l = (const SPS *)h->ps.sps_list[i]->data;
+            const SPS *sps_l = h->ps.sps_list[i];
             int idx = h264_params_info.stdSPSCount;
             set_sps(sps_l, &vksps_scaling[idx], &vksps_vui_header[idx], &vksps_vui[idx], &vksps[idx]);
             h264_params_info.stdSPSCount++;
@@ -331,7 +331,7 @@ static int vk_h264_create_params(AVCodecContext *avctx, AVBufferRef **buf)
     /* PPS list */
     for (int i = 0; i < FF_ARRAY_ELEMS(h->ps.pps_list); i++) {
         if (h->ps.pps_list[i]) {
-            const PPS *pps_l = (const PPS *)h->ps.pps_list[i]->data;
+            const PPS *pps_l = h->ps.pps_list[i];
             int idx = h264_params_info.stdPPSCount;
             set_pps(pps_l, pps_l->sps, &vkpps_scaling[idx], &vkpps[idx]);
             h264_params_info.stdPPSCount++;
-- 
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".