Re: [FFmpeg-devel] [PATCH v2 12/13] lavc/vp9dec: use cbs_vp9 to parse the frame header

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

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?

- Andreas

_______________________________________________
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".