From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id F28CB44B09 for ; Mon, 14 Jul 2025 05:06:50 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id EBAC968E6A2; Mon, 14 Jul 2025 08:06:46 +0300 (EEST) Received: from vidala.pars.ee (vidala.pars.ee [116.203.72.101]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id AF1ED68E262 for ; Mon, 14 Jul 2025 08:06:39 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; s=202405r; d=lynne.ee; c=relaxed/relaxed; h=From:To:Subject:Date:Message-ID; t=1752469599; bh=9Q9k1ecq6g0h+9Lgn14Al/e ndVkYw3p+TYoSA5CvP+I=; b=BP3NyL7Nj4Nh+KVjTrU8S6QmzAFeghADXZq6oMnriGdhfMX2cx kuiZyy/mDbIuO+k3gI9Qxk7s18BIn/1A9hYNHk+5zNar7FOkvgx7Cs5LXDCku93ubx0bqUvblPD qmlm907/0sFEDZ4GvhdlfBb61ciZRlareb4tbHGD46rnlAXskt7eIr2Su5o+R/l/58d6i5cxB+Q jvUjdfzLr5ZTPahEwS6px61kav3v6EQZ6H49leZ03ziLRczUKAtz2rdK5fFeACqYxMhcwC8OSIT UQXMlPJWChChKmF4kEYsOxy8BPLIrHd8yILQKFjo/HK107VjaRZqN/V/foQ4AahuCcA==; DKIM-Signature: v=1; a=ed25519-sha256; s=202405e; d=lynne.ee; c=relaxed/relaxed; h=From:To:Subject:Date:Message-ID; t=1752469599; bh=9Q9k1ecq6g0h+9Lgn14Al/e ndVkYw3p+TYoSA5CvP+I=; b=KcczdHZONzZX0N1P9iXJ0Afu0+Yj18XPDS1ag0Q5P4G55o13De uP8gS3kHjgNaeGK0pGP8IvayGTm2ZvjhuIAA==; Message-ID: Date: Mon, 14 Jul 2025 14:06:36 +0900 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20250712185128.862167-1-dev@lynne.ee> <20250712185128.862167-12-dev@lynne.ee> Content-Language: en-US From: Lynne In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH v2 12/13] lavc/vp9dec: use cbs_vp9 to parse the frame header X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 14/07/2025 03:15, Andreas Rheinhardt wrote: > Lynne: >> --- >> configure | 2 +- >> libavcodec/vp9.c | 40 ++++++++++++++++++++++++++++++++++++++++ >> libavcodec/vp9dec.h | 6 ++++++ >> libavcodec/vp9shared.h | 4 ++++ >> 4 files changed, 51 insertions(+), 1 deletion(-) >> >> diff --git a/configure b/configure >> index eeb81d7aa3..92ee54c7a6 100755 >> --- a/configure >> +++ b/configure >> @@ -3153,7 +3153,7 @@ vp6a_decoder_select="vp6_decoder" >> vp6f_decoder_select="vp6_decoder" >> vp7_decoder_select="h264pred videodsp vp8dsp" >> vp8_decoder_select="h264pred videodsp vp8dsp" >> -vp9_decoder_select="videodsp vp9_parser vp9_superframe_split_bsf" >> +vp9_decoder_select="videodsp vp9_parser cbs_vp9 vp9_superframe_split_bsf" >> vvc_decoder_select="cabac cbs_h266 golomb videodsp vvc_sei" >> wcmv_decoder_select="inflate_wrapper" >> webp_decoder_select="vp8_decoder exif" >> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c >> index 141f0941b4..a385956f4f 100644 >> --- a/libavcodec/vp9.c >> +++ b/libavcodec/vp9.c >> @@ -97,6 +97,7 @@ static void vp9_tile_data_free(VP9TileData *td) >> static void vp9_frame_unref(VP9Frame *f) >> { >> ff_progress_frame_unref(&f->tf); >> + av_refstruct_unref(&f->header_ref); >> av_refstruct_unref(&f->extradata); >> av_refstruct_unref(&f->hwaccel_picture_private); >> f->segmentation_map = NULL; >> @@ -145,6 +146,9 @@ fail: >> >> static void vp9_frame_replace(VP9Frame *dst, const VP9Frame *src) >> { >> + av_refstruct_replace(&dst->header_ref, src->header_ref); >> + dst->frame_header = src->frame_header; >> + >> ff_progress_frame_replace(&dst->tf, &src->tf); >> >> av_refstruct_replace(&dst->extradata, src->extradata); >> @@ -1255,6 +1259,11 @@ static av_cold int vp9_decode_free(AVCodecContext *avctx) >> av_freep(&s->entries); >> ff_pthread_free(s, vp9_context_offsets); >> #endif >> + >> + av_refstruct_unref(&s->header_ref); >> + ff_cbs_fragment_free(&s->current_frag); >> + ff_cbs_close(&s->cbc); >> + >> av_freep(&s->td); >> return 0; >> } >> @@ -1557,11 +1566,27 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame, >> int size = pkt->size; >> VP9Context *s = avctx->priv_data; >> int ret, i, j, ref; >> + CodedBitstreamUnit *unit; >> + VP9RawFrame *rf; >> + >> int retain_segmap_ref = s->s.frames[REF_FRAME_SEGMAP].segmentation_map && >> (!s->s.h.segmentation.enabled || !s->s.h.segmentation.update_map); >> const VP9Frame *src; >> AVFrame *f; >> >> + ret = ff_cbs_read_packet(s->cbc, &s->current_frag, pkt); >> + if (ret < 0) { >> + ff_cbs_fragment_reset(&s->current_frag); >> + av_log(avctx, AV_LOG_ERROR, "Failed to read frame header.\n"); >> + return ret; >> + } >> + >> + unit = &s->current_frag.units[0]; >> + rf = unit->content; >> + >> + av_refstruct_replace(&s->header_ref, unit->content_ref); >> + s->frame_header = &rf->header; >> + >> if ((ret = decode_frame_header(avctx, data, size, &ref)) < 0) { >> return ret; >> } else if (ret == 0) { >> @@ -1592,6 +1617,10 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame, >> vp9_frame_unref(&s->s.frames[CUR_FRAME]); >> if ((ret = vp9_frame_alloc(avctx, &s->s.frames[CUR_FRAME])) < 0) >> return ret; >> + >> + s->s.frames[CUR_FRAME].header_ref = av_refstruct_ref(s->header_ref); >> + s->s.frames[CUR_FRAME].frame_header = s->frame_header; >> + >> f = s->s.frames[CUR_FRAME].tf.f; >> if (s->s.h.keyframe) >> f->flags |= AV_FRAME_FLAG_KEY; >> @@ -1779,6 +1808,9 @@ static void vp9_decode_flush(AVCodecContext *avctx) >> for (i = 0; i < 8; i++) >> ff_progress_frame_unref(&s->s.refs[i]); >> >> + ff_cbs_fragment_reset(&s->current_frag); >> + ff_cbs_flush(s->cbc); >> + >> if (FF_HW_HAS_CB(avctx, flush)) >> FF_HW_SIMPLE_CALL(avctx, flush); >> } >> @@ -1791,6 +1823,10 @@ static av_cold int vp9_decode_init(AVCodecContext *avctx) >> s->last_bpp = 0; >> s->s.h.filter.sharpness = -1; >> >> + ret = ff_cbs_init(&s->cbc, AV_CODEC_ID_VP9, avctx); >> + if (ret < 0) >> + return ret; >> + >> #if HAVE_THREADS >> if (avctx->active_thread_type & FF_THREAD_SLICE) { >> ret = ff_pthread_init(s, vp9_context_offsets); >> @@ -1814,6 +1850,10 @@ static int vp9_decode_update_thread_context(AVCodecContext *dst, const AVCodecCo >> av_refstruct_replace(&s->frame_extradata_pool, ssrc->frame_extradata_pool); >> s->frame_extradata_pool_size = ssrc->frame_extradata_pool_size; >> >> + av_refstruct_replace(&s->header_ref, ssrc->header_ref); >> + s->frame_header = ssrc->frame_header; >> + memcpy(s->cbc->priv_data, ssrc->cbc->priv_data, sizeof(CodedBitstreamVP9Context)); >> + >> s->s.h.invisible = ssrc->s.h.invisible; >> s->s.h.keyframe = ssrc->s.h.keyframe; >> s->s.h.intraonly = ssrc->s.h.intraonly; >> diff --git a/libavcodec/vp9dec.h b/libavcodec/vp9dec.h >> index e41f47a82a..c3ad2bbcdb 100644 >> --- a/libavcodec/vp9dec.h >> +++ b/libavcodec/vp9dec.h >> @@ -38,6 +38,7 @@ >> #include "vp9dsp.h" >> #include "vp9shared.h" >> #include "vpx_rac.h" >> +#include "cbs_vp9.h" >> >> #define REF_INVALID_SCALE 0xFFFF >> >> @@ -97,6 +98,11 @@ typedef struct VP9Context { >> VP9SharedContext s; >> VP9TileData *td; >> >> + CodedBitstreamContext *cbc; >> + CodedBitstreamFragment current_frag; >> + VP9RawFrame *header_ref; ///< RefStruct reference backing frame_header >> + VP9RawFrameHeader *frame_header; >> + >> VP9DSPContext dsp; >> VideoDSPContext vdsp; >> GetBitContext gb; >> diff --git a/libavcodec/vp9shared.h b/libavcodec/vp9shared.h >> index 8a450c26a6..d2226e0072 100644 >> --- a/libavcodec/vp9shared.h >> +++ b/libavcodec/vp9shared.h >> @@ -30,6 +30,7 @@ >> #include "libavutil/mem_internal.h" >> >> #include "progressframe.h" >> +#include "cbs_vp9.h" >> #include "vp9.h" >> >> enum BlockPartition { >> @@ -63,6 +64,9 @@ typedef struct VP9mvrefPair { >> } VP9mvrefPair; >> >> typedef struct VP9Frame { >> + VP9RawFrame *header_ref; ///< RefStruct reference backing frame_header >> + VP9RawFrameHeader *frame_header; >> + >> ProgressFrame tf; >> void *extradata; ///< RefStruct reference >> uint8_t *segmentation_map; > > My expectation for a patch that uses CBS to parse the frame header is > that the other header parsing code would be removed in said patch. Why > is this not true here? What is the benefit of this patch then? It allows us to implement the vulkan_vp9 hwaccel without needing to implement parsing dozens of more fields. The plan is to gradually switch the decoder to use the CBS structs, which have different names and sometimes different definitions. I don't mind doing this first, but it would delay the vulkan_vp9 hwaccel, which users have been waiting for quite a long time, and I wanted to hear whether switching to CBS was acceptable at all. _______________________________________________ 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".