From 787351a68e9f3cbe46c3dcf6d0d9b001bcd139b3 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Thu, 4 Aug 2022 05:18:58 +0200 Subject: [PATCH] avcodec/h264_ps: Use RefStruct API for SPS/PPS 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.) Reviewed-by: Anton Khirnov Signed-off-by: Andreas Rheinhardt --- 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 | 49 ++++++++++---------------------- libavcodec/h264dec.h | 1 - libavcodec/mediacodecdec.c | 4 +-- libavcodec/vulkan_h264.c | 4 +-- 10 files changed, 52 insertions(+), 99 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; + PPS *pps = obj; - av_buffer_unref(&pps->sps_ref); - - 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 -#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 24f4690e79..0811a025cf 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; @@ -362,26 +360,13 @@ int ff_h264_update_thread_context(AVCodecContext *dst, memcpy(h->block_offset, h1->block_offset, sizeof(h->block_offset)); // 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; - } - 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; - } + for (int i = 0; i < FF_ARRAY_ELEMS(h->ps.sps_list); i++) + ff_refstruct_replace(&h->ps.sps_list[i], h1->ps.sps_list[i]); + for (int i = 0; i < FF_ARRAY_ELEMS(h->ps.pps_list); i++) + 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; - h->ps.sps = NULL; - if (h1->ps.pps_ref) { - h->ps.pps = (const PPS*)h->ps.pps_ref->data; - h->ps.sps = h->ps.pps->sps; - } + ff_refstruct_replace(&h->ps.pps, h1->ps.pps); + h->ps.sps = h1->ps.sps; if (need_reinit || !inited) { h->width = h1->width; @@ -931,7 +916,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; @@ -1067,17 +1052,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 || @@ -1749,7 +1728,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); @@ -2136,7 +2115,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++;