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

[James Almer]

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.

> 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;