Re: [FFmpeg-devel] [PATCH v4 4/9] avcodec: add cbs for h266/vvc

Nuo Mi Tue, 26 Jan 2021 05:36:19 -0800

On Tue, Jan 26, 2021 at 5:54 AM James Almer <jamr...@gmail.com> wrote:


> On 1/25/2021 2:12 PM, James Almer wrote:
> > Did you add this just to set active_vps? I thought the
> > idea was to remove all that since it's not a concept defined in h266.
>
> The following should get rid of active_*, but the actual fields can't be
> removed from CodedBitstreamH266Context unless we duplicate the
> cbs_h2645_replace_ps() macro to remove the relevant lines it shares with
> the other two codecs.
>
One solution  is  define a cbs_h26n_replace_ps.
cbs_h2645 _replace_ps calls  cbs_h26n_replace_ps and set active_xps to NULL.
cbs_h266_replace_ps calls the cbs_h26n_replace_ps only.
I did not use this since I think active_xps will make the caller's life
easier.
They do not need to use the id again and again to get pps, sps, vps.
If you think remove it is better, I can remove it.


> > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> > index da541407cf..2516481015 100644
> > --- a/libavcodec/cbs_h2645.c
> > +++ b/libavcodec/cbs_h2645.c
> > @@ -1690,10 +1690,6 @@ static void cbs_h266_flush(CodedBitstreamContext
> *ctx)
> >      }
> >      av_buffer_unref(&h266->ph_ref);
> >      h266->ph = NULL;
> > -
> > -    h266->active_vps = NULL;
> > -    h266->active_sps = NULL;
> > -    h266->active_pps = NULL;
> >  }
> >
> >
> > diff --git a/libavcodec/cbs_h266.h b/libavcodec/cbs_h266.h
> > index 8ce29be3fa..bf15a14d85 100644
> > --- a/libavcodec/cbs_h266.h
> > +++ b/libavcodec/cbs_h266.h
> > @@ -800,9 +800,7 @@ typedef struct CodedBitstreamH266Context {
> >      H266RawPPS  *pps[VVC_MAX_PPS_COUNT];
> >      H266RawPH   *ph;
> >
> > -    // The currently active parameter sets.  These are updated when any
> > -    // NAL unit refers to the relevant parameter set.  These pointers
> > -    // must also be present in the arrays above.
> > +    // Unused, added to keep compatibility with the
> cbs_h2645_replace_ps() macro
> >      const H266RawVPS *active_vps;
> >      const H266RawSPS *active_sps;
> >      const H266RawPPS *active_pps;
> > diff --git a/libavcodec/cbs_h266_syntax_template.c
> b/libavcodec/cbs_h266_syntax_template.c
> > index c030766c34..3e1a8be5d9 100644
> > --- a/libavcodec/cbs_h266_syntax_template.c
> > +++ b/libavcodec/cbs_h266_syntax_template.c
> > @@ -463,11 +463,9 @@ static int
> FUNC(ref_pic_list_struct)(CodedBitstreamContext *ctx, RWContext *rw,
> >  }
> >
> >  static int FUNC(ref_pic_lists)(CodedBitstreamContext *ctx, RWContext
> *rw,
> > -                               H266RefPicLists *current)
> > +                               H266RefPicLists *current,
> > +                               const H266RawSPS *sps, const H266RawPPS
> *pps)
> >  {
> > -    CodedBitstreamH266Context *h266 = ctx->priv_data;
> > -    const H266RawSPS *sps = h266->active_sps;
> > -    const H266RawPPS *pps = h266->active_pps;
> >      const H266RefPicListStruct *ref_list;
> >      int err, i, j, num_ltrp_entries;
> >      for (i = 0; i < 2; i++) {
> > @@ -974,7 +972,6 @@ static int FUNC(sps)(CodedBitstreamContext *ctx,
> RWContext *rw,
> >          vps->vps_layer_id[0] = current->nal_unit_header.nuh_layer_id;
> >          h266->vps_ref[0] = ref;
> >          h266->vps[0] = vps;
> > -        h266->active_vps = vps;
> >      }
> >
> >      u(3, sps_max_sublayers_minus1, 0, VVC_MAX_SUBLAYERS - 1);
> > @@ -1528,7 +1525,6 @@ static int FUNC(pps)(CodedBitstreamContext *ctx,
> RWContext *rw,
> >                 current->pps_seq_parameter_set_id);
> >          return AVERROR_INVALIDDATA;
> >      }
> > -    h266->active_sps = sps;
> >
> >      flag(pps_mixed_nalu_types_in_pic_flag);
> >      ue(pps_pic_width_in_luma_samples,
> > @@ -2194,9 +2190,6 @@ static int
> FUNC(picture_header)(CodedBitstreamContext *ctx, RWContext *rw,
> >                 sps->sps_video_parameter_set_id);
> >          return AVERROR_INVALIDDATA;
> >      }
> > -    h266->active_sps = sps;
> > -    h266->active_pps = pps;
> > -    h266->active_vps = vps;
> >
> >      ub(sps->sps_log2_max_pic_order_cnt_lsb_minus4 + 4,
> ph_pic_order_cnt_lsb);
> >      if (current->ph_gdr_pic_flag)
> > @@ -2292,7 +2285,7 @@ static int
> FUNC(picture_header)(CodedBitstreamContext *ctx, RWContext *rw,
> >      else
> >          infer(ph_pic_output_flag, 1);
> >      if (pps->pps_rpl_info_in_ph_flag) {
> > -        CHECK(FUNC(ref_pic_lists)(ctx, rw, &current->ph_ref_pic_lists));
> > +        CHECK(FUNC(ref_pic_lists)(ctx, rw, &current->ph_ref_pic_lists,
> sps, pps));
> >      }
> >      if (sps->sps_partition_constraints_override_enabled_flag)
> >          flag(ph_partition_constraints_override_flag);
> > @@ -2591,17 +2584,13 @@ static int
> FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw,
> >          h266->ph = (H266RawPH*)h266->ph_ref->data;
> >          memcpy(h266->ph, &current->sh_picture_header,
> sizeof(H266RawPH));
> >      }
> > -    sps = h266->active_sps;
> > -    pps = h266->active_pps;
> >      ph  = h266->ph;
> > -
> >      if (!ph) {
> >          av_log(ctx->log_ctx, AV_LOG_ERROR, "Picture header not
> available.\n");
> >          return AVERROR_INVALIDDATA;
> >      }
> > -    //sps and pps checked in picture_header(), just assert them.
> > -    av_assert0(sps);
> > -    av_assert0(pps);
> > +    pps = h266->pps[ph->ph_pic_parameter_set_id];
> > +    sps = h266->sps[pps->pps_seq_parameter_set_id];
> >
> >      if (sps->sps_subpic_info_present_flag) {
> >          ub(sps->sps_subpic_id_len_minus1 + 1, sh_subpic_id);
> > @@ -2722,7 +2711,7 @@ static int
> FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw,
> >          ((nal_unit_type != VVC_IDR_W_RADL &&
> >          nal_unit_type != VVC_IDR_N_LP) ||
> >          sps->sps_idr_rpl_present_flag)) {
> > -        CHECK(FUNC(ref_pic_lists)(ctx, rw, &current->sh_ref_pic_lists));
> > +        CHECK(FUNC(ref_pic_lists)(ctx, rw, &current->sh_ref_pic_lists,
> sps, pps));
> >          ref_pic_lists = &current->sh_ref_pic_lists;
> >      } else {
> >          ref_pic_lists = &h266->ph->ph_ref_pic_lists;
> > diff --git a/libavcodec/vvc_parser.c b/libavcodec/vvc_parser.c
> > index b160ef9426..61db91d6f0 100644
> > --- a/libavcodec/vvc_parser.c
> > +++ b/libavcodec/vvc_parser.c
> > @@ -166,15 +166,13 @@ static int parse_nal_units(AVCodecParserContext
> *ctx, const uint8_t *buf,
> >          goto end;
> >      }
> >
> > -    sps = h266->active_sps;
> > -    pps = h266->active_pps;
> >      ph = h266->ph;
> >      if (!ph) {
> >          av_log(avctx, AV_LOG_ERROR, "No picture header.\n");
> >          goto end;
> >      }
> > -    av_assert0(sps);
> > -    av_assert0(pps);
> > +    pps = h266->pps[ph->ph_pic_parameter_set_id];
> > +    sps = h266->sps[pps->pps_seq_parameter_set_id];
> >
> >      ctx->key_frame    = ph->ph_gdr_or_irap_pic_flag;
> >      ctx->coded_width  = pps->pps_pic_width_in_luma_samples;
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 4/9] avcodec: add cbs for h266/vvc

Reply via email to