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

[James Almer]

On 1/25/2021 11:15 AM, Nuo Mi wrote:
> +static int FUNC(ref_pic_list_struct)(CodedBitstreamContext *ctx, RWContext *rw,
> +                                     H266RefPicListStruct *current,
> +                                     uint8_t list_idx, uint8_t rpls_idx,
> +                                     const H266RawSPS *sps)
> +{
> +    CodedBitstreamH266Context *h266 = ctx->priv_data;
> +    int err, i, j, general_layer_idx = -1, num_direct_ref_layers = 0;
> +    const H266RawVPS *vps = h266->vps[sps->sps_video_parameter_set_id];
> +
> +    if (!vps) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR,
> +               "VPS id %d not available.\n", sps->sps_video_parameter_set_id);
> +        return AVERROR_INVALIDDATA;

Wont this break all the samples that lack a VPS?

(I thought for that matter that all those samples having this field but 
then no VPS at all was odd).

[...]

> +static int FUNC(picture_header)(CodedBitstreamContext *ctx, RWContext *rw,
> +                                H266RawPH *current)
> +{
> +    CodedBitstreamH266Context *h266 = ctx->priv_data;
> +    const H266RawVPS *vps;
> +    const H266RawSPS *sps;
> +    const H266RawPPS *pps;
> +    int err, i;
> +    unsigned int ctb_log2_size_y, min_cb_log2_size_y,
> +        min_qt_log2_size_intra_y, min_qt_log2_size_inter_y;
> +    uint8_t qp_bd_offset;
> +
> +    flag(ph_gdr_or_irap_pic_flag);
> +    flag(ph_non_ref_pic_flag);
> +    if (current->ph_gdr_or_irap_pic_flag)
> +        flag(ph_gdr_pic_flag);
> +    else
> +        infer(ph_gdr_pic_flag, 0);
> +    flag(ph_inter_slice_allowed_flag);
> +    if (current->ph_inter_slice_allowed_flag)
> +        flag(ph_intra_slice_allowed_flag);
> +    else
> +        infer(ph_intra_slice_allowed_flag, 1);
> +    ue(ph_pic_parameter_set_id, 0, VVC_MAX_PPS_COUNT - 1);
> +    pps = h266->pps[current->ph_pic_parameter_set_id];
> +    if (!pps) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "PPS id %d not available.\n",
> +               current->ph_pic_parameter_set_id);
> +        return AVERROR_INVALIDDATA;
> +    }
> +    sps = h266->sps[pps->pps_seq_parameter_set_id];
> +    if (!sps) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n",
> +               pps->pps_seq_parameter_set_id);
> +        return AVERROR_INVALIDDATA;
> +    }
> +    vps = h266->vps[sps->sps_video_parameter_set_id];
> +    if (!vps) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR, "VPS id %d not available.\n",
> +               sps->sps_video_parameter_set_id);
> +        return AVERROR_INVALIDDATA;

Ditto. But in this case the VPS is not used at all by the 
picture_header. 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.