Hi Mark,
thanks for your careful review.
It may need time to fix them all, but I will change them one by one.

thanks

On Tue, Dec 22, 2020 at 6:53 AM Mark Thompson <s...@jkqxz.net> wrote:

> On 21/12/2020 06:07, Nuo Mi wrote:
> > ---
> >   libavcodec/Makefile                   |    1 +
> >   libavcodec/cbs.c                      |    6 +
> >   libavcodec/cbs_h2645.c                |  337 ++++++
> >   libavcodec/cbs_h266.h                 |  711 ++++++++++++
> >   libavcodec/cbs_h266_syntax_template.c | 1493 +++++++++++++++++++++++++
> >   libavcodec/cbs_internal.h             |    1 +
> >   6 files changed, 2549 insertions(+)
> >   create mode 100644 libavcodec/cbs_h266.h
> >   create mode 100644 libavcodec/cbs_h266_syntax_template.c
> >
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index 450781886d..4045c002b7 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -73,6 +73,7 @@ OBJS-$(CONFIG_CBS)                     += cbs.o
> >   OBJS-$(CONFIG_CBS_AV1)                 += cbs_av1.o
> >   OBJS-$(CONFIG_CBS_H264)                += cbs_h2645.o h2645_parse.o
> >   OBJS-$(CONFIG_CBS_H265)                += cbs_h2645.o h2645_parse.o
> > +OBJS-$(CONFIG_CBS_H266)                += cbs_h2645.o h2645_parse.o
> >   OBJS-$(CONFIG_CBS_JPEG)                += cbs_jpeg.o
> >   OBJS-$(CONFIG_CBS_MPEG2)               += cbs_mpeg2.o
> >   OBJS-$(CONFIG_CBS_VP9)                 += cbs_vp9.o
> > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> > index f98531e131..2b4f3b2bea 100644
> > --- a/libavcodec/cbs.c
> > +++ b/libavcodec/cbs.c
> > @@ -38,6 +38,9 @@ static const CodedBitstreamType *cbs_type_table[] = {
> >   #if CONFIG_CBS_H265
> >       &ff_cbs_type_h265,
> >   #endif
> > +#if CONFIG_CBS_H266
> > +    &ff_cbs_type_h266,
> > +#endif
> >   #if CONFIG_CBS_JPEG
> >       &ff_cbs_type_jpeg,
> >   #endif
> > @@ -59,6 +62,9 @@ const enum AVCodecID ff_cbs_all_codec_ids[] = {
> >   #if CONFIG_CBS_H265
> >       AV_CODEC_ID_H265,
> >   #endif
> > +#if CONFIG_CBS_H266
> > +    AV_CODEC_ID_H266,
> > +#endif
> >   #if CONFIG_CBS_JPEG
> >       AV_CODEC_ID_MJPEG,
> >   #endif
> > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
> > index 434322492c..0b884c1ed0 100644
> > --- a/libavcodec/cbs_h2645.c
> > +++ b/libavcodec/cbs_h2645.c
> > @@ -24,11 +24,13 @@
> >   #include "cbs_internal.h"
> >   #include "cbs_h264.h"
> >   #include "cbs_h265.h"
> > +#include "cbs_h266.h"
> >   #include "h264.h"
> >   #include "h264_sei.h"
> >   #include "h2645_parse.h"
> >   #include "hevc.h"
> >   #include "hevc_sei.h"
> > +#include "h266.h"
> >
> >
> >   static int cbs_read_ue_golomb(CodedBitstreamContext *ctx,
> GetBitContext *gbc,
> > @@ -256,6 +258,7 @@ static int
> cbs_h265_payload_extension_present(GetBitContext *gbc, uint32_t paylo
> >   #define FUNC_NAME(rw, codec, name) cbs_ ## codec ## _ ## rw ## _ ##
> name
> >   #define FUNC_H264(rw, name) FUNC_NAME(rw, h264, name)
> >   #define FUNC_H265(rw, name) FUNC_NAME(rw, h265, name)
> > +#define FUNC_H266(rw, name) FUNC_NAME(rw, h266, name)
> >
> >   #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs,
> __VA_ARGS__ }) : NULL)
> >
> > @@ -364,6 +367,10 @@ static int
> cbs_h2645_read_more_rbsp_data(GetBitContext *gbc)
> >   #include "cbs_h265_syntax_template.c"
> >   #undef FUNC
> >
> > +#define FUNC(name) FUNC_H266(READWRITE, name)
> > +#include "cbs_h266_syntax_template.c"
> > +#undef FUNC
> > +
> >   #undef READ
> >   #undef READWRITE
> >   #undef RWContext
>
> Please include the second time for write support as well.  (With only read
> this is incredibly hard to test.)
>
I will implement the medata bsf after we got a decent passrate for
conformance.
decoder md5 bit match is a sort of prove for the logical.

>
> > @@ -679,6 +686,9 @@ cbs_h2645_replace_ps(4, PPS, pps,
> pic_parameter_set_id)
> >   cbs_h2645_replace_ps(5, VPS, vps, vps_video_parameter_set_id)
> >   cbs_h2645_replace_ps(5, SPS, sps, sps_seq_parameter_set_id)
> >   cbs_h2645_replace_ps(5, PPS, pps, pps_pic_parameter_set_id)
> > +cbs_h2645_replace_ps(6, VPS, vps, vps_video_parameter_set_id)
> > +cbs_h2645_replace_ps(6, SPS, sps, sps_seq_parameter_set_id)
> > +cbs_h2645_replace_ps(6, PPS, pps, pps_pic_parameter_set_id)
> >
> >   static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx,
> >                                     CodedBitstreamUnit *unit)
> > @@ -920,6 +930,117 @@ static int
> cbs_h265_read_nal_unit(CodedBitstreamContext *ctx,
> >       return 0;
> >   }
> >
> > +static int cbs_h266_read_nal_unit(CodedBitstreamContext *ctx,
> > +                                  CodedBitstreamUnit *unit)
> > +{
> > +    GetBitContext gbc;
> > +    int err;
> > +
> > +    err = init_get_bits(&gbc, unit->data, 8 * unit->data_size);
>
> init_get_bits8()
>
> > +    if (err < 0)
> > +        return err;
> > +
> > +    err = ff_cbs_alloc_unit_content2(ctx, unit);
> > +    if (err < 0)
> > +        return err;
> > +
> > +    switch (unit->type) {
> > +    case H266_NAL_VPS:
> > +        {
> > +            H266RawVPS *vps = unit->content;
> > +
> > +            err = cbs_h266_read_vps(ctx, &gbc, vps);
>
> This isn't implemented, so don't include it here so it returns ENOSYS.
>
> > +            if (err < 0)
> > +                return err;
> > +
> > +            err = cbs_h266_replace_vps(ctx, unit);
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
> > +    case H266_NAL_SPS:
> > +        {
> > +            H266RawSPS *sps = unit->content;
> > +
> > +            err = cbs_h266_read_sps(ctx, &gbc, sps);
> > +            if (err < 0)
> > +                return err;
> > +
> > +            err = cbs_h266_replace_sps(ctx, unit);
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
> > +
> > +    case H266_NAL_PPS:
> > +        {
> > +            H266RawPPS *pps = unit->content;
> > +
> > +            err = cbs_h266_read_pps(ctx, &gbc, pps);
> > +            if (err < 0)
> > +                return err;
> > +
> > +            err = cbs_h266_replace_pps(ctx, unit);
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
> > +
> > +    case H266_NAL_TRAIL:
> > +    case H266_NAL_STSA:
> > +    case H266_NAL_RADL:
>
> Missing case H266_NUT_RASL.
>
> > +    case H266_NAL_IDR_W_RADL:
> > +    case H266_NAL_IDR_N_LP:
> > +    case H266_NAL_CRA_NUT:
> > +    case H266_NAL_GDR_NUT:
> > +        {
> > +            H266RawSlice *slice = unit->content;
> > +            int pos, len;
> > +
> > +            err = cbs_h266_read_slice_header(ctx, &gbc, &slice->header);
> > +            if (err < 0)
> > +                return err;
> > +
> > +            if (!cbs_h2645_read_more_rbsp_data(&gbc))
> > +                return AVERROR_INVALIDDATA;
> > +
> > +            pos = get_bits_count(&gbc);
> > +            len = unit->data_size;
> > +
> > +            slice->data_size = len - pos / 8;
> > +            slice->data_ref  = av_buffer_ref(unit->data_ref);
> > +            if (!slice->data_ref)
> > +                return AVERROR(ENOMEM);
> > +            slice->data = unit->data + pos / 8;
> > +            slice->data_bit_start = pos % 8;
> > +        }
> > +        break;
> > +
> > +    case H266_NAL_AUD:
> > +        {
> > +            err = cbs_h266_read_aud(ctx, &gbc, unit->content);
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
> > +
> > +    case H266_NAL_PREFIX_SEI:
> > +    case H266_NAL_SUFFIX_SEI:
> > +        {
> > +            err = cbs_h266_read_sei(ctx, &gbc, unit->content,
> > +                                    unit->type == H266_NAL_PREFIX_SEI);
> > +
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
>
> Don't include SEI at all given that you haven't implemented it; just let
> it return ENOSYS.
>
I will add this very soon. If I removed it here, the compiler will report
no one uses the function, I need to remove the function definition too.



>
> > +
> > +    default:
> > +        return AVERROR(ENOSYS);
> > +    }
> > +    return 0;
> > +}
> > +
> >   static int cbs_h2645_write_slice_data(CodedBitstreamContext *ctx,
> >                                         PutBitContext *pbc, const
> uint8_t *data,
> >                                         size_t data_size, int
> data_bit_start)
> > @@ -1207,6 +1328,120 @@ static int
> cbs_h265_write_nal_unit(CodedBitstreamContext *ctx,
> >       return 0;
> >   }
> >
> > +static int cbs_h266_write_nal_unit(CodedBitstreamContext *ctx,
> > +                                   CodedBitstreamUnit *unit,
> > +                                   PutBitContext *pbc)
> > +{
> > +    printf("TODO: cbs_h266_write_nal_unit");
>
> Yes.
>
> > +#if 0
> > +    int err;
> > +
> > +    switch (unit->type) {
> > +    case HEVC_NAL_VPS:
> > +        {
> > +            H265RawVPS *vps = unit->content;
> > +
> > +            err = cbs_h265_write_vps(ctx, pbc, vps);
> > +            if (err < 0)
> > +                return err;
> > +
> > +            err = cbs_h265_replace_vps(ctx, unit);
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
> > +
> > +    case HEVC_NAL_SPS:
> > +        {
> > +            H265RawSPS *sps = unit->content;
> > +
> > +            err = cbs_h265_write_sps(ctx, pbc, sps);
> > +            if (err < 0)
> > +                return err;
> > +
> > +            err = cbs_h265_replace_sps(ctx, unit);
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
> > +
> > +    case HEVC_NAL_PPS:
> > +        {
> > +            H265RawPPS *pps = unit->content;
> > +
> > +            err = cbs_h265_write_pps(ctx, pbc, pps);
> > +            if (err < 0)
> > +                return err;
> > +
> > +            err = cbs_h265_replace_pps(ctx, unit);
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
> > +
> > +    case HEVC_NAL_TRAIL_N:
> > +    case HEVC_NAL_TRAIL_R:
> > +    case HEVC_NAL_TSA_N:
> > +    case HEVC_NAL_TSA_R:
> > +    case HEVC_NAL_STSA_N:
> > +    case HEVC_NAL_STSA_R:
> > +    case HEVC_NAL_RADL_N:
> > +    case HEVC_NAL_RADL_R:
> > +    case HEVC_NAL_RASL_N:
> > +    case HEVC_NAL_RASL_R:
> > +    case HEVC_NAL_BLA_W_LP:
> > +    case HEVC_NAL_BLA_W_RADL:
> > +    case HEVC_NAL_BLA_N_LP:
> > +    case HEVC_NAL_IDR_W_RADL:
> > +    case HEVC_NAL_IDR_N_LP:
> > +    case HEVC_NAL_CRA_NUT:
> > +        {
> > +            H265RawSlice *slice = unit->content;
> > +
> > +            err = cbs_h265_write_slice_segment_header(ctx, pbc,
> &slice->header);
> > +            if (err < 0)
> > +                return err;
> > +
> > +            if (slice->data) {
> > +                err = cbs_h2645_write_slice_data(ctx, pbc, slice->data,
> > +                                                 slice->data_size,
> > +                                                 slice->data_bit_start);
> > +                if (err < 0)
> > +                    return err;
> > +            } else {
> > +                // No slice data - that was just the header.
> > +            }
> > +        }
> > +        break;
> > +
> > +    case HEVC_NAL_AUD:
> > +        {
> > +            err = cbs_h265_write_aud(ctx, pbc, unit->content);
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
> > +
> > +    case HEVC_NAL_SEI_PREFIX:
> > +    case HEVC_NAL_SEI_SUFFIX:
> > +        {
> > +            err = cbs_h265_write_sei(ctx, pbc, unit->content,
> > +                                     unit->type == HEVC_NAL_SEI_PREFIX);
> > +
> > +            if (err < 0)
> > +                return err;
> > +        }
> > +        break;
> > +
> > +    default:
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Write unimplemented for "
> > +               "NAL unit type %"PRIu32".\n", unit->type);
> > +        return AVERROR_PATCHWELCOME;
> > +    }
> > +#endif
> > +    return 0;
> > +}
> > +
> >   static int cbs_h2645_assemble_fragment(CodedBitstreamContext *ctx,
> >                                          CodedBitstreamFragment *frag)
> >   {
> > @@ -1362,6 +1597,43 @@ static void cbs_h265_close(CodedBitstreamContext
> *ctx)
> >           av_buffer_unref(&h265->pps_ref[i]);
> >   }
> >
> > +static void cbs_h266_flush(CodedBitstreamContext *ctx)
> > +{
> > +    CodedBitstreamH266Context *h266 = ctx->priv_data;
> > +
> > +    for (int i = 0; i < FF_ARRAY_ELEMS(h266->vps); i++) {
> > +        av_buffer_unref(&h266->vps_ref[i]);
> > +        h266->vps[i] = NULL;
> > +    }
> > +    for (int i = 0; i < FF_ARRAY_ELEMS(h266->sps); i++) {
> > +        av_buffer_unref(&h266->sps_ref[i]);
> > +        h266->sps[i] = NULL;
> > +    }
> > +    for (int i = 0; i < FF_ARRAY_ELEMS(h266->pps); i++) {
> > +        av_buffer_unref(&h266->pps_ref[i]);
> > +        h266->pps[i] = NULL;
> > +    }
> > +
> > +    h266->active_vps = NULL;
> > +    h266->active_sps = NULL;
> > +    h266->active_pps = NULL;
> > +}
> > +
> > +static void cbs_h266_close(CodedBitstreamContext *ctx)
> > +{
> > +    CodedBitstreamH266Context *h266 = ctx->priv_data;
> > +    int i;
> > +
> > +    ff_h2645_packet_uninit(&h266->common.read_packet);
> > +
> > +    for (i = 0; i < FF_ARRAY_ELEMS(h266->vps); i++)
> > +        av_buffer_unref(&h266->vps_ref[i]);
> > +    for (i = 0; i < FF_ARRAY_ELEMS(h266->sps); i++)
> > +        av_buffer_unref(&h266->sps_ref[i]);
> > +    for (i = 0; i < FF_ARRAY_ELEMS(h266->pps); i++)
> > +        av_buffer_unref(&h266->pps_ref[i]);
>
> These loops look like exactly what flush does.
>
sorry, I do not fully understand this, can you explain more?
thanks


>
> > +}
> > +
> >   static void cbs_h264_free_sei_payload(H264RawSEIPayload *payload)
> >   {
> >       switch (payload->payload_type) {
> > @@ -1506,6 +1778,55 @@ static const CodedBitstreamUnitTypeDescriptor
> cbs_h265_unit_types[] = {
> >       CBS_UNIT_TYPE_END_OF_LIST
> >   };
> >
> > +static void cbs_h266_free_sei(void *opaque, uint8_t *content)
> > +{
> > +}
> > +
> > +static const CodedBitstreamUnitTypeDescriptor cbs_h266_unit_types[] = {
> > +    CBS_UNIT_TYPE_INTERNAL_REF(H266_NAL_VPS, H266RawVPS,
> extension_data.data),
> > +    CBS_UNIT_TYPE_INTERNAL_REF(H266_NAL_SPS, H266RawSPS,
> extension_data.data),
> > +    CBS_UNIT_TYPE_INTERNAL_REF(H266_NAL_PPS, H266RawPPS,
> extension_data.data),
> > +
> > +    CBS_UNIT_TYPE_POD(H266_NAL_AUD, H266RawAUD),
> > +
> > +    {
> > +        // Slices of non-IRAP pictures.
> > +        .nb_unit_types         = CBS_UNIT_TYPE_RANGE,
> > +        .unit_type_range_start = H266_NAL_TRAIL,
> > +        .unit_type_range_end   = H266_NAL_RASL,
>
> Using the list form would probably be clearer - there isn't the
> proliferation of extra types like H.265.
>
> > +
> > +        .content_type   = CBS_CONTENT_TYPE_INTERNAL_REFS,
> > +        .content_size   = sizeof(H266RawSlice),
> > +        .nb_ref_offsets = 1,
> > +        .ref_offsets    = { offsetof(H266RawSlice, data) },
> > +    },
> > +
> > +    {
> > +        // Slices of IRAP pictures.
> > +        .nb_unit_types         = CBS_UNIT_TYPE_RANGE,
> > +        .unit_type_range_start = H266_NAL_IDR_W_RADL,
> > +        .unit_type_range_end   = H266_NAL_GDR_NUT,
>
> Likewise here.
>
> > +
> > +        .content_type   = CBS_CONTENT_TYPE_INTERNAL_REFS,
> > +        .content_size   = sizeof(H266RawSlice),
> > +        .nb_ref_offsets = 1,
> > +        .ref_offsets    = { offsetof(H266RawSlice, data) },
> > +    },
> > +
> > +    {
> > +        .nb_unit_types  = 2,
> > +        .unit_types     = {
> > +            H266_NAL_PREFIX_SEI,
> > +            H266_NAL_SUFFIX_SEI
> > +        },
> > +        .content_type   = CBS_CONTENT_TYPE_COMPLEX,
> > +        .content_size   = sizeof(H266RawSEI),
> > +        .content_free   = &cbs_h266_free_sei,
> > +    },
>
> Don't include these if you didn't implement them.
>
> > +
> > +    CBS_UNIT_TYPE_END_OF_LIST
> > +};
> > +
> >   const CodedBitstreamType ff_cbs_type_h264 = {
> >       .codec_id          = AV_CODEC_ID_H264,
> >
> > @@ -1538,6 +1859,22 @@ const CodedBitstreamType ff_cbs_type_h265 = {
> >       .close             = &cbs_h265_close,
> >   };
> >
> > +const CodedBitstreamType ff_cbs_type_h266 = {
> > +    .codec_id          = AV_CODEC_ID_VVC,
> > +
> > +    .priv_data_size    = sizeof(CodedBitstreamH266Context),
> > +
> > +    .unit_types        = cbs_h266_unit_types,
> > +
> > +    .split_fragment    = &cbs_h2645_split_fragment,
> > +    .read_unit         = &cbs_h266_read_nal_unit,
> > +    .write_unit        = &cbs_h266_write_nal_unit,
> > +    .assemble_fragment = &cbs_h2645_assemble_fragment,
> > +
> > +    .flush             = &cbs_h266_flush,
> > +    .close             = &cbs_h266_close,
> > +};
> > +
> >   int ff_cbs_h264_add_sei_message(CodedBitstreamFragment *au,
> >                                   H264RawSEIPayload *payload)
> >   {
> > diff --git a/libavcodec/cbs_h266.h b/libavcodec/cbs_h266.h
> > new file mode 100644
> > index 0000000000..91d6ef4f9e
> > --- /dev/null
> > +++ b/libavcodec/cbs_h266.h
> > @@ -0,0 +1,711 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> > + */
> > +
> > +#ifndef AVCODEC_CBS_H266_H
> > +#define AVCODEC_CBS_H266_H
> > +
> > +#include <stddef.h>
> > +#include <stdint.h>
> > +
> > +#include "cbs_h2645.h"
> > +#include "h266.h"
> > +
> > +enum {
> > +    // This limit is arbitrary - it is sufficient for one message of
> each
> > +    // type plus some repeats, and will therefore easily cover all sane
> > +    // streams.  However, it is possible to make technically-valid
> streams
> > +    // for which it will fail (for example, by including a large number
> of
> > +    // user-data-unregistered messages).
> > +    H266_MAX_SEI_PAYLOADS = 64,
> > +};
> > +
>
> In this file, please horizontally align things to improve readability.  In
> particular, the starts of all normal variables and all the matching arrays
> would help quite a bit.
>
Just for plain type, not for "H266GeneralConstraintsInfo
general_constraints_info",  right?


> > +typedef struct H266RawNALUnitHeader {
> > +    uint8_t nuh_layer_id;
> > +    uint8_t nal_unit_type;
> > +    uint8_t nuh_temporal_id_plus1;
> > +} H266RawNALUnitHeader;
> > +
> > +typedef struct H266GeneralConstraintsInfo {
> > +    uint8_t gci_present_flag;
> > +} H266GeneralConstraintsInfo;
> > +
> > +typedef struct H266RawProfileTierLevel {
> > +    uint8_t general_profile_idc;
> > +    uint8_t general_tier_flag;
> > +    uint8_t general_level_idc;
> > +    uint8_t ptl_frame_only_constraint_flag;
> > +    uint8_t ptl_multilayer_enabled_flag;
> > +    H266GeneralConstraintsInfo general_constraints_info;
> > +    uint8_t ptl_sublayer_level_present_flag[H266_MAX_SUB_LAYERS - 1];
>
> That constant should be renamed to H266_MAX_SUBLAYERS, the standard never
> uses a space between them.
>
this wried to me too. just copied from hevc. I will change it

>
> > +    uint8_t sublayer_level_idc[H266_MAX_SUB_LAYERS - 1];
> > +    uint8_t ptl_num_sub_profiles;
> > +    uint32_t general_sub_profile_idc[256];
>
> I think add an H266_MAX_SUB_PROFILES for this size.
>
> > +} H266RawProfileTierLevel;
> > +
> > +typedef struct H266RawExtensionData {
> > +    uint8_t     *data;
> > +    AVBufferRef *data_ref;
> > +    size_t       bit_length;
> > +} H266RawExtensionData;
> > +
> > +typedef struct H266DpbParameters {
> > +    uint8_t dpb_max_dec_pic_buffering_minus1[H266_MAX_SUB_LAYERS];
> > +    uint8_t dpb_max_num_reorder_pics[H266_MAX_SUB_LAYERS];
> > +    uint8_t dpb_max_latency_increase_plus1[H266_MAX_SUB_LAYERS];
> > +} H266DpbParameters;
> > +
> > +typedef struct H266RawVPS {
> > +    H266RawNALUnitHeader nal_unit_header;
> > +
> > +    uint8_t vps_video_parameter_set_id;
> > +
> > +    uint8_t vps_max_layers_minus1;
> > +    uint8_t vps_max_sublayers_minus1;
> > +    /*TODO add more*/
> > +    H266RawExtensionData extension_data;
> > +} H266RawVPS;
> > +//
> > +typedef struct H266RefPicListStruct {
> > +    uint8_t num_ref_entries;
> > +    uint8_t ltrp_in_header_flag;
> > +    uint8_t inter_layer_ref_pic_flag[H266_MAX_DPB_SIZE + 13];
> > +    uint8_t st_ref_pic_flag[H266_MAX_DPB_SIZE + 13];
> > +    uint8_t abs_delta_poc_st[H266_MAX_DPB_SIZE + 13];
> > +    uint8_t strp_entry_sign_flag[H266_MAX_DPB_SIZE + 13];
> > +    uint8_t rpls_poc_lsb_lt[H266_MAX_DPB_SIZE + 13];
> > +    uint8_t ilrp_idx[H266_MAX_DPB_SIZE + 13];
> > +
> > +    //calculated values;
> > +    uint8_t num_ltrp_entries;
>
> Derived values really shouldn't be in the bitstream struct if it can be
> avoided.  In this case, it's calculated and used inline so you don't need
> to store it here.
>
some value is difficult to cal, just like this one. you can check the code
for details.

>
> > +} H266RefPicListStruct;
> > +
> > +typedef struct H266RefPicLists {
> > +    uint8_t rpl_sps_flag[2];
> > +    uint8_t rpl_idx[2];
> > +    H266RefPicListStruct rpl_ref_list[2];
> > +    uint16_t poc_lsb_lt[2][H266_MAX_DPB_SIZE + 13];
> > +    uint8_t delta_poc_msb_cycle_present_flag[2][H266_MAX_DPB_SIZE + 13];
> > +    uint16_t delta_poc_msb_cycle_lt[2][H266_MAX_DPB_SIZE + 13];
> > +} H266RefPicLists;
> > +
> > +typedef struct H266RawGeneralTimingHrdParameters {
> > +    uint32_t num_units_in_tick;
> > +    uint32_t time_scale;
> > +    uint8_t general_nal_hrd_params_present_flag;
> > +    uint8_t general_vcl_hrd_params_present_flag;
> > +    uint8_t general_same_pic_timing_in_all_ols_flag;
> > +    uint8_t general_du_hrd_params_present_flag;
> > +    uint8_t tick_divisor_minus2;
> > +    uint8_t bit_rate_scale;
> > +    uint8_t cpb_size_scale;
> > +    uint8_t cpb_size_du_scale;
> > +    uint8_t hrd_cpb_cnt_minus1;
> > +} H266RawGeneralTimingHrdParameters;
> > +
> > +typedef struct H266RawSubLayerHRDParameters {
> > +    uint32_t bit_rate_value_minus1[H266_MAX_CPB_CNT];
> > +    uint32_t cpb_size_value_minus1[H266_MAX_CPB_CNT];
> > +    uint32_t cpb_size_du_value_minus1[H266_MAX_CPB_CNT];
> > +    uint32_t bit_rate_du_value_minus1[H266_MAX_CPB_CNT];
> > +    uint8_t cbr_flag[H266_MAX_CPB_CNT];
> > +} H266RawSubLayerHRDParameters;
> > +
> > +typedef struct H266RawOlsTimingHrdParameters {
> > +    uint8_t fixed_pic_rate_general_flag[H266_MAX_SUB_LAYERS];
> > +    uint8_t fixed_pic_rate_within_cvs_flag[H266_MAX_SUB_LAYERS];
> > +    uint16_t elemental_duration_in_tc_minus1[H266_MAX_SUB_LAYERS];
> > +    uint8_t low_delay_hrd_flag[H266_MAX_SUB_LAYERS];
> > +    H266RawSubLayerHRDParameters
> nal_sub_layer_hrd_parameters[H266_MAX_SUB_LAYERS];
> > +    H266RawSubLayerHRDParameters
> vcl_sub_layer_hrd_parameters[H266_MAX_SUB_LAYERS];
> > +} H266RawOlsTimingHrdParameters;
> > +
> > +typedef struct H266RawVUI {
> > +    uint8_t vui_progressive_source_flag;
> > +    uint8_t vui_interlaced_source_flag;
> > +    uint8_t vui_non_packed_constraint_flag;
> > +    uint8_t vui_non_projected_constraint_flag;
> > +
> > +    uint8_t vui_aspect_ratio_info_present_flag;
> > +    uint8_t vui_aspect_ratio_constant_flag;
> > +    uint8_t vui_aspect_ratio_idc;
> > +
> > +    uint16_t vui_sar_width;
> > +    uint16_t vui_sar_height;;
> > +
> > +    uint8_t vui_overscan_info_present_flag;
> > +    uint8_t vui_overscan_appropriate_flag;
> > +
> > +    uint8_t vui_colour_description_present_flag;
> > +    uint8_t vui_colour_primaries;
> > +
> > +    uint8_t vui_transfer_characteristics;
> > +    uint8_t vui_matrix_coeffs;
> > +    uint8_t vui_full_range_flag;
> > +
> > +    uint8_t vui_chroma_loc_info_present_flag;
> > +    uint8_t vui_chroma_sample_loc_type_frame;
> > +    uint8_t vui_chroma_sample_loc_type_top_field;
> > +    uint8_t vui_chroma_sample_loc_type_bottom_field;
> > +} H266RawVUI;
> > +
> > +typedef struct H266RawSPS {
> > +    H266RawNALUnitHeader nal_unit_header;
> > +
> > +    uint8_t sps_seq_parameter_set_id;
> > +    uint8_t sps_video_parameter_set_id;
> > +    uint8_t sps_max_sublayers_minus1;
> > +    uint8_t sps_chroma_format_idc;
> > +    uint8_t sps_log2_ctu_size_minus5;
> > +    uint8_t sps_ptl_dpb_hrd_params_present_flag;
> > +    H266RawProfileTierLevel profile_tier_level;
> > +    uint8_t sps_gdr_enabled_flag;
> > +    uint8_t sps_ref_pic_resampling_enabled_flag;
> > +    uint8_t sps_res_change_in_clvs_allowed_flag;
> > +
> > +    uint16_t sps_pic_width_max_in_luma_samples;
> > +    uint16_t sps_pic_height_max_in_luma_samples;
> > +
> > +    uint8_t sps_conformance_window_flag;
> > +    uint16_t sps_conf_win_left_offset;
> > +    uint16_t sps_conf_win_right_offset;
> > +    uint16_t sps_conf_win_top_offset;
> > +    uint16_t sps_conf_win_bottom_offset;
> > +
> > +    uint8_t sps_subpic_info_present_flag;
> > +    uint8_t sps_num_subpics_minus1;
> > +    uint8_t sps_independent_subpics_flag;
> > +    uint8_t sps_subpic_same_size_flag;
> > +    uint8_t sps_subpic_id_len_minus1;
> > +    uint8_t sps_subpic_id_mapping_explicitly_signalled_flag;
> > +    uint8_t sps_subpic_id_mapping_present_flag;
> > +
> > +
> > +    uint8_t sps_bitdepth_minus8;
> > +    uint8_t sps_entropy_coding_sync_enabled_flag;
> > +    uint8_t sps_entry_point_offsets_present_flag;
> > +
> > +    uint8_t sps_log2_max_pic_order_cnt_lsb_minus4;
> > +    uint8_t sps_poc_msb_cycle_flag;
> > +    uint8_t sps_poc_msb_cycle_len_minus1;
> > +
> > +    uint8_t sps_num_extra_ph_bytes;
> > +    uint8_t sps_extra_ph_bit_present_flag[16];
> > +    uint8_t num_extra_ph_bits;
> > +
> > +    uint8_t sps_num_extra_sh_bytes;
> > +    uint8_t sps_extra_sh_bit_present_flag[16];
> > +    uint8_t num_extra_sh_bits;
> > +
> > +    uint8_t sps_sublayer_dpb_params_flag;
> > +    H266DpbParameters sps_dpb_params;
> > +
> > +    uint8_t sps_log2_min_luma_coding_block_size_minus2;
> > +    uint8_t sps_partition_constraints_override_enabled_flag;
> > +    uint8_t sps_log2_diff_min_qt_min_cb_intra_slice_luma;
> > +    uint8_t sps_max_mtt_hierarchy_depth_intra_slice_luma;
> > +    uint8_t sps_log2_diff_max_bt_min_qt_intra_slice_luma;
> > +    uint8_t sps_log2_diff_max_tt_min_qt_intra_slice_luma;
> > +
> > +    uint8_t sps_qtbtt_dual_tree_intra_flag;
> > +    uint8_t sps_log2_diff_min_qt_min_cb_intra_slice_chroma;
> > +    uint8_t sps_max_mtt_hierarchy_depth_intra_slice_chroma;
> > +    uint8_t sps_log2_diff_max_bt_min_qt_intra_slice_chroma;
> > +    uint8_t sps_log2_diff_max_tt_min_qt_intra_slice_chroma;
> > +
> > +    uint8_t sps_log2_diff_min_qt_min_cb_inter_slice;
> > +    uint8_t sps_max_mtt_hierarchy_depth_inter_slice;
> > +    uint8_t sps_log2_diff_max_bt_min_qt_inter_slice;
> > +    uint8_t sps_log2_diff_max_tt_min_qt_inter_slice;
> > +
> > +    uint8_t sps_max_luma_transform_size_64_flag;
> > +
> > +    uint8_t sps_transform_skip_enabled_flag;
> > +    uint8_t sps_log2_transform_skip_max_size_minus2;
> > +    uint8_t sps_bdpcm_enabled_flag;
> > +
> > +    uint8_t sps_mts_enabled_flag;
> > +    uint8_t sps_explicit_mts_intra_enabled_flag;
> > +    uint8_t sps_explicit_mts_inter_enabled_flag;
> > +
> > +    uint8_t sps_lfnst_enabled_flag;
> > +
> > +    uint8_t sps_joint_cbcr_enabled_flag;
> > +    uint8_t sps_same_qp_table_for_chroma_flag;
> > +
> > +    int8_t sps_qp_table_start_minus26[H266_MAX_PLANES];
> > +    uint8_t sps_num_points_in_qp_table_minus1[H266_MAX_PLANES];
> > +    uint8_t
> sps_delta_qp_in_val_minus1[H266_MAX_PLANES][H266_MAX_POINTS_IN_QP_TABLE];
> > +    uint8_t
> sps_delta_qp_diff_val[H266_MAX_PLANES][H266_MAX_POINTS_IN_QP_TABLE];
> > +
> > +    uint8_t sps_sao_enabled_flag;
> > +    uint8_t sps_alf_enabled_flag;
> > +    uint8_t sps_ccalf_enabled_flag;
> > +    uint8_t sps_lmcs_enabled_flag;
> > +    uint8_t sps_weighted_pred_flag;
> > +    uint8_t sps_weighted_bipred_flag;
> > +    uint8_t sps_long_term_ref_pics_flag;
> > +    uint8_t sps_inter_layer_prediction_enabled_flag;
> > +    uint8_t sps_idr_rpl_present_flag;
> > +    uint8_t sps_rpl1_same_as_rpl0_flag;
> > +
> > +    uint8_t sps_num_ref_pic_lists[2];
> > +    H266RefPicListStruct
> sps_ref_pic_list_struct[2][H266_MAX_REF_PIC_LISTS];
> > +
> > +    uint8_t sps_ref_wraparound_enabled_flag;
> > +    uint8_t sps_temporal_mvp_enabled_flag;
> > +    uint8_t sps_sbtmvp_enabled_flag;
> > +    uint8_t sps_amvr_enabled_flag;
> > +    uint8_t sps_bdof_enabled_flag;
> > +    uint8_t sps_bdof_control_present_in_ph_flag;
> > +    uint8_t sps_smvd_enabled_flag;
> > +    uint8_t sps_dmvr_enabled_flag;
> > +    uint8_t sps_dmvr_control_present_in_ph_flag;
> > +    uint8_t sps_mmvd_enabled_flag;
> > +    uint8_t sps_mmvd_fullpel_only_enabled_flag;
> > +    uint8_t sps_six_minus_max_num_merge_cand;
> > +    uint8_t sps_sbt_enabled_flag;
> > +    uint8_t sps_affine_enabled_flag;
> > +    uint8_t sps_five_minus_max_num_subblock_merge_cand;
> > +    uint8_t sps_6param_affine_enabled_flag;
> > +    uint8_t sps_affine_amvr_enabled_flag;
> > +    uint8_t sps_affine_prof_enabled_flag;
> > +    uint8_t sps_prof_control_present_in_ph_flag;
> > +    uint8_t sps_bcw_enabled_flag;
> > +    uint8_t sps_ciip_enabled_flag;
> > +    uint8_t sps_gpm_enabled_flag;
> > +    uint8_t sps_max_num_merge_cand_minus_max_num_gpm_cand;
> > +    uint8_t sps_log2_parallel_merge_level_minus2;
> > +    uint8_t sps_isp_enabled_flag;
> > +    uint8_t sps_mrl_enabled_flag;
> > +    uint8_t sps_mip_enabled_flag;
> > +    uint8_t sps_cclm_enabled_flag;
> > +    uint8_t sps_chroma_horizontal_collocated_flag;
> > +    uint8_t sps_chroma_vertical_collocated_flag;
> > +    uint8_t sps_palette_enabled_flag;
> > +    uint8_t sps_act_enabled_flag;
> > +    uint8_t sps_min_qp_prime_ts;
> > +    uint8_t sps_ibc_enabled_flag;
> > +    uint8_t sps_six_minus_max_num_ibc_merge_cand;
> > +    uint8_t sps_ladf_enabled_flag;
> > +    uint8_t sps_num_ladf_intervals_minus2;
> > +    int8_t sps_ladf_lowest_interval_qp_offset;
> > +    int8_t sps_ladf_qp_offset[4];
> > +    uint8_t sps_ladf_delta_threshold_minus1[4];
> > +
> > +    uint8_t sps_explicit_scaling_list_enabled_flag;
> > +    uint8_t sps_scaling_matrix_for_lfnst_disabled_flag;
> > +    uint8_t
> sps_scaling_matrix_for_alternative_colour_space_disabled_flag;
> > +    uint8_t sps_scaling_matrix_designated_colour_space_flag;
> > +    uint8_t sps_dep_quant_enabled_flag;
> > +    uint8_t sps_sign_data_hiding_enabled_flag;
> > +
> > +    uint8_t sps_virtual_boundaries_enabled_flag;
> > +    uint8_t sps_virtual_boundaries_present_flag;
> > +    uint8_t sps_num_ver_virtual_boundaries;
> > +    uint16_t sps_virtual_boundary_pos_x_minus1[3];
> > +    uint8_t sps_num_hor_virtual_boundaries;
> > +    uint16_t sps_virtual_boundary_pos_y_minus1[3];
> > +
> > +    uint8_t sps_timing_hrd_params_present_flag;
> > +    uint8_t sps_sublayer_cpb_params_present_flag;
> > +    H266RawGeneralTimingHrdParameters sps_general_timing_hrd_parameters;
> > +    H266RawOlsTimingHrdParameters sps_ols_timing_hrd_parameters;
> > +
> > +    uint8_t sps_field_seq_flag;
> > +    uint8_t sps_vui_parameters_present_flag;
> > +    uint16_t sps_vui_payload_size_minus1;
> > +    H266RawVUI vui;
> > +
> > +    uint8_t sps_extension_flag;
> > +
> > +    H266RawExtensionData extension_data;
> > +
> > +    //caculated values
> > +    uint8_t qp_bd_offset;
> > +    uint8_t min_cb_log2_size_y;
> > +} H266RawSPS;
> > +
> > +typedef struct H266RawPPS {
> > +    H266RawNALUnitHeader nal_unit_header;
> > +
> > +    uint8_t pps_pic_parameter_set_id;
> > +    uint8_t pps_seq_parameter_set_id;
> > +    uint8_t pps_mixed_nalu_types_in_pic_flag;
> > +    uint16_t pps_pic_width_in_luma_samples;
> > +    uint16_t pps_pic_height_in_luma_samples;
> > +
> > +    uint8_t pps_conformance_window_flag;
> > +    uint16_t pps_conf_win_left_offset;
> > +    uint16_t pps_conf_win_right_offset;
> > +    uint16_t pps_conf_win_top_offset;
> > +    uint16_t pps_conf_win_bottom_offset;
> > +
> > +    uint8_t pps_scaling_window_explicit_signalling_flag;
> > +    int16_t pps_scaling_win_left_offset;
> > +    int16_t pps_scaling_win_right_offset;
> > +    int16_t pps_scaling_win_top_offset;
> > +    int16_t pps_scaling_win_bottom_offset;
> > +
> > +    uint8_t pps_output_flag_present_flag;
> > +    uint8_t pps_no_pic_partition_flag;
> > +    uint8_t pps_subpic_id_mapping_present_flag;
> > +    /* TODO: pps_subpic_id[ i ] */
> > +    uint8_t pps_log2_ctu_size_minus5;
> > +    uint8_t pps_num_exp_tile_columns_minus1;
> > +    uint8_t pps_num_exp_tile_rows_minus1;
> > +    uint16_t pps_tile_column_width_minus1[H266_MAX_TILE_COLUMNS];
> > +    uint16_t pps_tile_row_height_minus1[H266_MAX_TILE_ROWS];
> > +
> > +    uint8_t pps_loop_filter_across_tiles_enabled_flag;
> > +    uint8_t pps_rect_slice_flag;
> > +    uint8_t pps_single_slice_per_subpic_flag;
> > +    uint16_t pps_num_slices_in_pic_minus1;
> > +    uint8_t pps_tile_idx_delta_present_flag;
> > +
> > +    /* TODO:
> > +    uint16_t pps_slice_width_in_tiles_minus1[H266_MAX_TILE_COLUMNS];
> > +    uint16_t pps_slice_height_in_tiles_minus1[H266_MAX_TILE_ROWS];
> > +    */
> > +    uint8_t pps_loop_filter_across_slices_enabled_flag;
> > +    uint8_t pps_cabac_init_present_flag;
> > +    uint8_t pps_num_ref_idx_default_active_minus1[2];
> > +    uint8_t pps_rpl1_idx_present_flag;
> > +    uint8_t pps_weighted_pred_flag;
> > +    uint8_t pps_weighted_bipred_flag;
> > +    uint8_t pps_ref_wraparound_enabled_flag;
> > +    uint8_t pps_pic_width_minus_wraparound_offset;
> > +    int8_t pps_init_qp_minus26;
> > +    uint8_t pps_cu_qp_delta_enabled_flag;
> > +    uint8_t pps_chroma_tool_offsets_present_flag;
> > +    uint8_t pps_cb_qp_offset;
> > +    uint8_t pps_cr_qp_offset;
> > +    uint8_t pps_joint_cbcr_qp_offset_present_flag;
> > +    uint8_t pps_joint_cbcr_qp_offset_value;
> > +    uint8_t pps_slice_chroma_qp_offsets_present_flag;
> > +    uint8_t pps_cu_chroma_qp_offset_list_enabled_flag;
> > +    uint8_t pps_chroma_qp_offset_list_len_minus1;
> > +    uint8_t pps_cb_qp_offset_list[6];
> > +    uint8_t pps_cr_qp_offset_list[6];
> > +    uint8_t pps_joint_cbcr_qp_offset_list[6];
> > +    uint8_t pps_deblocking_filter_control_present_flag;
> > +    uint8_t pps_deblocking_filter_override_enabled_flag;
> > +    uint8_t pps_deblocking_filter_disabled_flag;
> > +    uint8_t pps_dbf_info_in_ph_flag;
> > +
> > +    uint8_t pps_luma_beta_offset_div2;
> > +    uint8_t pps_luma_tc_offset_div2;
> > +    uint8_t pps_cb_beta_offset_div2;
> > +    uint8_t pps_cb_tc_offset_div2;
> > +    uint8_t pps_cr_beta_offset_div2;
> > +    uint8_t pps_cr_tc_offset_div2;
> > +
> > +    uint8_t pps_rpl_info_in_ph_flag;
> > +    uint8_t pps_sao_info_in_ph_flag;
> > +    uint8_t pps_alf_info_in_ph_flag;
> > +    uint8_t pps_wp_info_in_ph_flag;
> > +    uint8_t pps_qp_delta_info_in_ph_flag;
> > +
> > +    uint8_t pps_picture_header_extension_present_flag;
> > +    uint8_t pps_slice_header_extension_present_flag;
> > +    uint8_t pps_extension_flag;
> > +    H266RawExtensionData extension_data;
> > +
> > +    //calculated value;
> > +    uint16_t num_tile_columns;
> > +    uint16_t num_tile_rows;
> > +    uint16_t num_tiles_in_pic;
> > +} H266RawPPS;
> > +
> > +typedef struct H266RawAUD {
> > +    H266RawNALUnitHeader nal_unit_header;
> > +    uint8_t aud_irap_or_gdr_flag;
> > +    uint8_t aud_pic_type;
> > +} H266RawAUD;
> > +
> > +typedef struct  H266RawPH {
> > +    uint8_t  ph_gdr_or_irap_pic_flag;
> > +    uint8_t  ph_non_ref_pic_flag;
> > +    uint8_t  ph_gdr_pic_flag;
> > +    uint8_t  ph_inter_slice_allowed_flag;
> > +    uint8_t  ph_intra_slice_allowed_flag;
> > +    uint8_t  ph_pic_parameter_set_id;
> > +    uint16_t ph_pic_order_cnt_lsb;
> > +    uint8_t  ph_recovery_poc_cnt;
> > +    uint8_t  ph_extra_bit[16];
> > +    uint8_t  ph_poc_msb_cycle_present_flag;
> > +    uint8_t  ph_poc_msb_cycle_val;
> > +    uint8_t  ph_alf_enabled_flag;
> > +    uint8_t  ph_num_alf_aps_ids_luma;
> > +    //uint8_t ph_alf_aps_id_luma[i]
> > +    uint8_t  ph_alf_cb_enabled_flag;
> > +    uint8_t  ph_alf_cr_enabled_flag;
> > +    uint8_t  ph_alf_aps_id_chroma;
> > +    uint8_t  ph_alf_cc_cb_enabled_flag;
> > +    uint8_t  ph_alf_cc_cb_aps_id;
> > +    uint8_t  ph_alf_cc_cr_enabled_flag;
> > +    uint8_t  ph_alf_cc_cr_aps_id;
> > +    uint8_t  ph_lmcs_enabled_flag;
> > +    uint8_t  ph_lmcs_aps_id;
> > +    uint8_t  ph_chroma_residual_scale_flag;
> > +    uint8_t  ph_explicit_scaling_list_enabled_flag;
> > +    uint8_t  ph_scaling_list_aps_id;
> > +    uint8_t  ph_virtual_boundaries_present_flag;
> > +    uint8_t  ph_num_ver_virtual_boundaries;
> > +    //ph_virtual_boundary_pos_x_minus1[ i ]
> > +    //ph_num_hor_virtual_boundaries
> > +    //ph_virtual_boundary_pos_y_minus1[ i ]
> > +
> > +    uint8_t ph_pic_output_flag;
> > +    H266RefPicLists ph_ref_pic_lists;
> > +
> > +    uint8_t ph_partition_constraints_override_flag;
> > +
> > +    uint8_t ph_log2_diff_min_qt_min_cb_intra_slice_luma;
> > +    uint8_t ph_max_mtt_hierarchy_depth_intra_slice_luma;
> > +    uint8_t ph_log2_diff_max_bt_min_qt_intra_slice_luma;
> > +    uint8_t ph_log2_diff_max_tt_min_qt_intra_slice_luma;
> > +    uint8_t ph_log2_diff_min_qt_min_cb_intra_slice_chroma;
> > +
> > +    uint8_t ph_max_mtt_hierarchy_depth_intra_slice_chroma;
> > +    uint8_t ph_log2_diff_max_bt_min_qt_intra_slice_chroma;
> > +    uint8_t ph_log2_diff_max_tt_min_qt_intra_slice_chroma;
> > +
> > +    uint8_t ph_cu_qp_delta_subdiv_intra_slice;
> > +    uint8_t ph_cu_chroma_qp_offset_subdiv_intra_slice;
> > +
> > +    uint8_t ph_log2_diff_min_qt_min_cb_inter_slice;
> > +    uint8_t ph_max_mtt_hierarchy_depth_inter_slice;
> > +    uint8_t ph_log2_diff_max_bt_min_qt_inter_slice;
> > +    uint8_t ph_log2_diff_max_tt_min_qt_inter_slice;
> > +    uint8_t ph_cu_qp_delta_subdiv_inter_slice;
> > +    uint8_t ph_cu_chroma_qp_offset_subdiv_inter_slice;
> > +
> > +    uint8_t ph_temporal_mvp_enabled_flag;
> > +    uint8_t ph_collocated_from_l0_flag;
> > +    uint8_t ph_collocated_ref_idx;
> > +    uint8_t ph_mmvd_fullpel_only_flag;
> > +    uint8_t ph_mvd_l1_zero_flag;
> > +    uint8_t ph_bdof_disabled_flag;
> > +    uint8_t ph_dmvr_disabled_flag;
> > +    uint8_t ph_prof_disabled_flag;
> > +    //pred_weight_table( );
> > +    uint8_t ph_qp_delta;
> > +    uint8_t ph_joint_cbcr_sign_flag;
> > +    uint8_t ph_sao_luma_enabled_flag;
> > +    uint8_t ph_sao_chroma_enabled_flag;
> > +    uint8_t ph_deblocking_params_present_flag;
> > +    uint8_t ph_deblocking_filter_disabled_flag;
> > +    uint8_t ph_luma_beta_offset_div2;
> > +    uint8_t ph_luma_tc_offset_div2;
> > +    uint8_t ph_cb_beta_offset_div2;
> > +    uint8_t ph_cb_tc_offset_div2;
> > +    uint8_t ph_cr_beta_offset_div2;
> > +    uint8_t ph_cr_tc_offset_div2;
> > +    uint8_t ph_extension_length;
> > +} H266RawPH;
> > +
> > +typedef struct  H266RawSliceHeader {
> > +    H266RawNALUnitHeader nal_unit_header;
> > +    uint8_t sh_picture_header_in_slice_header_flag;
> > +    H266RawPH sh_picture_header;
> > +
> > +    uint16_t sh_subpic_id;
> > +    uint16_t sh_slice_address;
> > +    uint8_t  sh_extra_bit[16];
> > +    uint8_t  sh_num_tiles_in_slice_minus1;
> > +    uint8_t  sh_slice_type;
> > +    uint8_t  sh_no_output_of_prior_pics_flag;
> > +
> > +    uint8_t  sh_alf_enabled_flag;
> > +    uint8_t  sh_num_alf_aps_ids_luma;
> > +    uint8_t  sh_alf_aps_id_luma[8];
> > +    uint8_t  sh_alf_cb_enabled_flag;
> > +    uint8_t  sh_alf_cr_enabled_flag;
> > +    uint8_t  sh_alf_aps_id_chroma;
> > +    uint8_t  sh_alf_cc_cb_enabled_flag;
> > +    uint8_t  sh_alf_cc_cb_aps_id;
> > +    uint8_t  sh_alf_cc_cr_enabled_flag;
> > +    uint8_t  sh_alf_cc_cr_aps_id;
> > +
> > +    uint8_t  sh_lmcs_used_flag;
> > +    uint8_t  sh_explicit_scaling_list_used_flag;
> > +
> > +    H266RefPicLists sh_ref_pic_lists;
> > +
> > +    uint8_t sh_num_ref_idx_active_override_flag;
> > +    uint8_t sh_num_ref_idx_active_minus1[2];
> > +    uint8_t sh_cabac_init_flag;
> > +    uint8_t sh_collocated_from_l0_flag;
> > +    uint8_t sh_collocated_ref_idx;
> > +
> > +    int8_t  sh_qp_delta;
> > +    int8_t  sh_cb_qp_offset;
> > +    int8_t  sh_cr_qp_offset;
> > +    int8_t  sh_joint_cbcr_qp_offset;
> > +    uint8_t sh_cu_chroma_qp_offset_enabled_flag;
> > +
> > +    uint8_t sh_sao_luma_used_flag;
> > +    uint8_t sh_sao_chroma_used_flag;
> > +
> > +    uint8_t sh_deblocking_params_present_flag;
> > +    uint8_t sh_deblocking_filter_disabled_flag;
> > +    int8_t  sh_luma_beta_offset_div2;
> > +    int8_t  sh_luma_tc_offset_div2;
> > +    int8_t  sh_cb_beta_offset_div2;
> > +    int8_t  sh_cb_tc_offset_div2;
> > +    int8_t  sh_cr_beta_offset_div2;
> > +    int8_t  sh_cr_tc_offset_div2;
> > +    uint8_t sh_dep_quant_used_flag;
> > +
> > +    uint8_t  sh_sign_data_hiding_used_flag;
> > +    uint8_t  sh_ts_residual_coding_disabled_flag;
> > +    uint16_t sh_slice_header_extension_length;
> > +
> > +    uint8_t  sh_entry_offset_len_minus1;
> > +    uint32_t sh_entry_point_offset_minus1[H266_MAX_ENTRY_POINT_OFFSETS];
> > +
> > +    //calclatecd values;
> > +    uint16_t num_entry_point_offsets;
> > +} H266RawSliceHeader;
> > +
> > +
> > +typedef struct H266RawSlice {
> > +    H266RawSliceHeader header;
> > +
> > +    uint8_t     *data;
> > +    AVBufferRef *data_ref;
> > +    size_t       data_size;
> > +    int          data_bit_start;
> > +} H266RawSlice;
> > +
> > +
> > +typedef struct H266RawSEIBufferingPeriod {
> > +    uint8_t  bp_seq_parameter_set_id;
> > +} H266RawSEIBufferingPeriod;
> > +
> > +typedef struct H266RawSEIPicTiming {
> > +} H266RawSEIPicTiming;
> > +
> > +typedef struct H266RawSEIPanScanRect {
> > +} H266RawSEIPanScanRect;
> > +
> > +typedef struct H266RawSEIUserDataRegistered {
> > +
> > +} H266RawSEIUserDataRegistered;
> > +
> > +typedef struct H266RawSEIUserDataUnregistered {
> > +} H266RawSEIUserDataUnregistered;
> > +
> > +typedef struct H266RawSEIRecoveryPoint {
> > +} H266RawSEIRecoveryPoint;
> > +
> > +typedef struct H266RawSEIDisplayOrientation {
> > +} H266RawSEIDisplayOrientation;
> > +
> > +typedef struct H266RawSEIActiveParameterSets {
> > +} H266RawSEIActiveParameterSets;
> > +
> > +typedef struct H266RawSEIDecodedPictureHash {
> > +} H266RawSEIDecodedPictureHash;
> > +
> > +typedef struct H266RawSEITimeCode {
> > +} H266RawSEITimeCode;
> > +
> > +typedef struct H266RawSEIMasteringDisplayColourVolume {
> > +} H266RawSEIMasteringDisplayColourVolume;
> > +
> > +typedef struct H266RawSEIContentLightLevelInfo {
> > +    uint16_t max_content_light_level;
> > +    uint16_t max_pic_average_light_level;
> > +} H266RawSEIContentLightLevelInfo;
> > +
> > +typedef struct H266RawSEIAlternativeTransferCharacteristics {
> > +    uint8_t preferred_transfer_characteristics;
> > +} H266RawSEIAlternativeTransferCharacteristics;
> > +
> > +typedef struct H266RawSEIAlphaChannelInfo {
> > +    uint8_t  alpha_channel_cancel_flag;
> > +    uint8_t  alpha_channel_use_idc;
> > +    uint8_t  alpha_channel_bit_depth_minus8;
> > +    uint16_t alpha_transparent_value;
> > +    uint16_t alpha_opaque_value;
> > +    uint8_t  alpha_channel_incr_flag;
> > +    uint8_t  alpha_channel_clip_flag;
> > +    uint8_t  alpha_channel_clip_type_flag;
> > +} H266RawSEIAlphaChannelInfo;
> > +
> > +typedef struct H266RawSEIPayload {
> > +    uint32_t payload_type;
> > +    uint32_t payload_size;
> > +    union {
> > +        H266RawSEIBufferingPeriod buffering_period;
> > +        H266RawSEIPicTiming pic_timing;
> > +        H266RawSEIPanScanRect pan_scan_rect;
> > +        H266RawSEIUserDataRegistered user_data_registered;
> > +        H266RawSEIUserDataUnregistered user_data_unregistered;
> > +        H266RawSEIRecoveryPoint recovery_point;
> > +        H266RawSEIDisplayOrientation display_orientation;
> > +        H266RawSEIActiveParameterSets active_parameter_sets;
> > +        H266RawSEIDecodedPictureHash decoded_picture_hash;
> > +        H266RawSEITimeCode time_code;
> > +        H266RawSEIMasteringDisplayColourVolume mastering_display;
> > +        H266RawSEIContentLightLevelInfo content_light_level;
> > +        H266RawSEIAlternativeTransferCharacteristics
> > +            alternative_transfer_characteristics;
> > +        H266RawSEIAlphaChannelInfo alpha_channel_info;
> > +        struct {
> > +            uint8_t     *data;
> > +            AVBufferRef *data_ref;
> > +            size_t       data_length;
> > +        } other;
> > +    } payload;
> > +    H266RawExtensionData extension_data;
> > +} H266RawSEIPayload;
> > +
> > +typedef struct H266RawSEI {
> > +    H266RawNALUnitHeader nal_unit_header;
> > +
> > +    H266RawSEIPayload payload[H266_MAX_SEI_PAYLOADS];
> > +    uint8_t payload_count;
> > +} H266RawSEI;
> > +
> > +typedef struct CodedBitstreamH266Context {
> > +    // Reader/writer context in common with the H.264 implementation.
> > +    CodedBitstreamH2645Context common;
> > +
> > +    // All currently available parameter sets.  These are updated when
> > +    // any parameter set NAL unit is read/written with this context.
> > +    AVBufferRef *vps_ref[H266_MAX_VPS_COUNT];
> > +    AVBufferRef *sps_ref[H266_MAX_SPS_COUNT];
> > +    AVBufferRef *pps_ref[H266_MAX_PPS_COUNT];
> > +    H266RawVPS *vps[H266_MAX_VPS_COUNT];
> > +    H266RawSPS *sps[H266_MAX_SPS_COUNT];
> > +    H266RawPPS *pps[H266_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.
> > +    const H266RawVPS *active_vps;
> > +    const H266RawSPS *active_sps;
> > +    const H266RawPPS *active_pps;
> > +    const H266RawPH *active_ph;
> > +} CodedBitstreamH266Context;
> > +
> > +
> > +#endif /* AVCODEC_CBS_H266_H */
> > diff --git a/libavcodec/cbs_h266_syntax_template.c
> b/libavcodec/cbs_h266_syntax_template.c
> > new file mode 100644
> > index 0000000000..19fb1f8c27
> > --- /dev/null
> > +++ b/libavcodec/cbs_h266_syntax_template.c
> > @@ -0,0 +1,1493 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> > + */
> > +
> > +static int FUNC(rbsp_trailing_bits)(CodedBitstreamContext *ctx,
> RWContext *rw)
> > +{
> > +    int err;
> > +
> > +    fixed(1, rbsp_stop_one_bit, 1);
> > +    while (byte_alignment(rw) != 0)
> > +        fixed(1, rbsp_alignment_zero_bit, 0);
> > +    return 0;
> > +}
> > +
> > +static int FUNC(nal_unit_header)(CodedBitstreamContext *ctx, RWContext
> *rw,
> > +                                 H266RawNALUnitHeader *current,
> > +                                 int expected_nal_unit_type)
> > +{
> > +    int err;
> > +
> > +    fixed(1, forbidden_zero_bit, 0);
> > +    fixed(1, nuh_reserved_zero_bit, 0);
> > +
> > +    u(6, nuh_layer_id,          0, 55);
> > +
> > +    if (expected_nal_unit_type >= 0)
> > +        u(5, nal_unit_type, expected_nal_unit_type,
> > +                            expected_nal_unit_type);
> > +    else
> > +        ub(5, nal_unit_type);
> > +
> > +    u(3, nuh_temporal_id_plus1, 1,  7);
> > +    return 0;
> > +}
> > +
> > +static int FUNC(byte_alignment)(CodedBitstreamContext *ctx, RWContext
> *rw)
> > +{
> > +    int err;
> > +
> > +    fixed(1, alignment_bit_equal_to_one, 1);
> > +    while (byte_alignment(rw) != 0)
> > +        fixed(1, alignment_bit_equal_to_zero, 0);
> > +    return 0;
> > +}
> > +
> > +static int FUNC(general_constraints_info)(CodedBitstreamContext *ctx,
> RWContext *rw,
> > +                                    H266GeneralConstraintsInfo *current)
> > +{
> > +    int err;
> > +
> > +    flag(gci_present_flag);
> > +    if (current->gci_present_flag) {
> > +        return AVERROR_PATCHWELCOME;
> > +    }
> > +    while (byte_alignment(rw))
> > +        fixed(1, bit_equal_to_zero, 0);
> > +    return 0;
> > +}
> > +
> > +static int FUNC(profile_tier_level)(CodedBitstreamContext *ctx,
> RWContext *rw,
> > +                                    H266RawProfileTierLevel *current,
> > +                                    int profile_tire_present_flag,
>
> tire?
>
> > +                                    int max_num_sub_layers_minus1)
> > +{
> > +    int err, i;
> > +
> > +    if (profile_tire_present_flag) {
> > +        ub(7, general_profile_idc);
> > +        flag(general_tier_flag);
> > +    }
> > +    ub(8, general_level_idc);
> > +    flag(ptl_frame_only_constraint_flag);
> > +    flag(ptl_multilayer_enabled_flag);
> > +    if (profile_tire_present_flag) {
> > +        CHECK(FUNC(general_constraints_info)(ctx, rw,
> &current->general_constraints_info));
> > +    }
> > +    for( i = max_num_sub_layers_minus1 - 1; i >= 0; i--)
> > +        flags(ptl_sublayer_level_present_flag[i], 1, i);
> > +    while (byte_alignment(rw))
> > +        fixed(1, ptl_reserved_zero_bit, 0);
> > +    for( i = max_num_sub_layers_minus1 - 1; i >= 0; i-- )
>
> The styling in this is inconsistent.  Put a space after a keyword, but not
> around parentheses (also in many more places below).
>
yeah, sorry, just copied from spec. will clean up them all using search and
replace.

>
> > +        if(current->ptl_sublayer_level_present_flag[i])
> > +            ubs(8, sublayer_level_idc[i], 1, i);
> > +    if (profile_tire_present_flag) {
> > +        ub(8, ptl_num_sub_profiles);
> > +        for( i = 0; i < current->ptl_num_sub_profiles; i++ )
> > +            ubs(32, general_sub_profile_idc[i], 1, i);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int FUNC(vui_parameters)(CodedBitstreamContext *ctx, RWContext
> *rw,
> > +                                H266RawVUI *current)
> > +{
> > +    int err;
> > +
> > +    flag(vui_progressive_source_flag);
> > +    flag(vui_interlaced_source_flag);
> > +    flag(vui_non_packed_constraint_flag);
> > +    flag(vui_non_projected_constraint_flag);
> > +    flag(vui_aspect_ratio_info_present_flag);
> > +    if(current->vui_aspect_ratio_info_present_flag ) {
> > +        flag(vui_aspect_ratio_constant_flag);
> > +        ub(8, vui_aspect_ratio_idc);
> > +        if(current->vui_aspect_ratio_idc == 255 ) {
> > +            ub(16, vui_sar_width);
> > +            ub(16, vui_sar_height);
> > +        }
> > +    } else {
> > +        infer(vui_aspect_ratio_constant_flag, 0);
> > +        infer(vui_aspect_ratio_idc, 0);
> > +
> > +    }
> > +    flag(vui_overscan_info_present_flag);
> > +    if (current->vui_overscan_info_present_flag)
> > +        flag(vui_overscan_appropriate_flag);
> > +    flag(vui_colour_description_present_flag);
> > +    if(current->vui_colour_description_present_flag ) {
> > +        ub(8, vui_colour_primaries);
> > +        ub(8, vui_transfer_characteristics);
> > +        ub(8, vui_matrix_coeffs);
> > +        flag(vui_full_range_flag);
> > +    } else {
> > +        infer(vui_colour_primaries, 2);
> > +        infer(vui_transfer_characteristics, 2);
> > +        infer(vui_matrix_coeffs, 2);
> > +        infer(vui_full_range_flag, 0);
> > +    }
> > +    flag(vui_chroma_loc_info_present_flag);
> > +    if(current->vui_chroma_loc_info_present_flag ) {
> > +        if(current->vui_progressive_source_flag &&
> !current->vui_interlaced_source_flag) {
> > +            ue(vui_chroma_sample_loc_type_frame, 0, 6);
> > +        }
> > +        else {
> > +            ue(vui_chroma_sample_loc_type_top_field, 0, 6);
> > +            ue(vui_chroma_sample_loc_type_bottom_field,  0, 6);
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int FUNC(vui_payload)(CodedBitstreamContext *ctx, RWContext *rw,
> > +                                H266RawVUI *current, uint16_t
> vui_payload_size)
> > +{
> > +    int err;
> > +
> > +#ifdef READ
> > +    int start_pos, end_pos, left;
> > +
> > +    start_pos = get_bits_count(rw);
> > +    CHECK(FUNC(vui_parameters)(ctx, rw, current));
> > +    end_pos = get_bits_count(rw);
> > +
> > +    left = vui_payload_size * 8 - (end_pos - start_pos);
> > +    skip_bits(rw, left);
>
> This needs to record and regenerate the extension data to avoid breaking
> passthrough.  (Like other extension data cases.)
>
> > +#else
> > +    return AVERROR_PATCHWELCOME;
> > +#endif
> > +
> > +
> > +    return 0;
> > +}
> > +
> > +static int FUNC(vps)(CodedBitstreamContext *ctx, RWContext *rw,
> > +                     H266RawVPS *current)
> > +{
> > +    HEADER("Video Parameter Set");
> > +    av_log(ctx->log_ctx, AV_LOG_ERROR, "vps is not supported yet\n");
> > +
> > +    return AVERROR_PATCHWELCOME;
> > +}
> > +
> > +static int FUNC(dpb_parameters)(CodedBitstreamContext *ctx, RWContext
> *rw,
> > +                     H266DpbParameters *current, uint8_t
> max_sublayers_minus1, uint8_t sublayer_info_flag)
> > +{
> > +    int err, i;
> > +    for(i = ( sublayer_info_flag ? 0 : max_sublayers_minus1 ); i <=
> max_sublayers_minus1; i++ ) {
> > +        ues(dpb_max_dec_pic_buffering_minus1[i], 0, H266_MAX_DPB_SIZE -
> 1, 1, i);
> > +        ues(dpb_max_num_reorder_pics[i], 0,
> current->dpb_max_dec_pic_buffering_minus1[i], 1, i);
> > +        ues(dpb_max_latency_increase_plus1[i],  0, UINT32_MAX - 1, 1,
> i);
>
> Split lines to keep this to 80 characters.  Aligning the matching fields
> would be nice too.
>
>      for (int i = sublayer_info_flag ? 0 : max_sublayers_minus1;
>           i <= max_sublayers_minus1; i++) {
>          ues(dpb_max_dec_pic_buffering_minus1[i],
>              0, H266_MAX_DPB_SIZE - 1,                        1, i);
>          ues(dpb_max_num_reorder_pics[i],
>              0, current->dpb_max_dec_pic_buffering_minus1[i], 1, i);
>          ues(dpb_max_latency_increase_plus1[i],
>              0, UINT32_MAX - 1,                               1, i);
>      }
>
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int FUNC(ref_pic_list_struct)(CodedBitstreamContext *ctx,
> RWContext *rw,
> > +                     H266RefPicListStruct *current, uint8_t list_idx,
> uint8_t rpls_idx,
> > +                     const H266RawSPS* const sps)
>
> * is part of the declarator, not the type.  We generally don't put
> redundant const on variables, either.
>
> > +{
> > +
> > +    int err, i, j;
> > +    ue(num_ref_entries, 0, H266_MAX_DPB_SIZE + 13);
>
> These fields all have more subscripts than you've included here.  Is there
> a sensible way to make the trace output match the standard?
>
yeah, i tried to pass more subscripts, like flags(ltrp_in_header_flag, 2,
i, j), but the flags report we have missed [].


> > +    if(sps->sps_long_term_ref_pics_flag && rpls_idx <
> sps->sps_num_ref_pic_lists[list_idx] && current->num_ref_entries > 0)
> > +        flag(ltrp_in_header_flag);
> > +    if (sps->sps_long_term_ref_pics_flag && rpls_idx ==
> sps->sps_num_ref_pic_lists[list_idx])
> > +        infer(ltrp_in_header_flag, 1);
> > +    current->num_ltrp_entries = 0;
> > +    for( i = 0, j = 0; i < current->num_ref_entries; i++) {
> > +        if(sps->sps_inter_layer_prediction_enabled_flag )
> > +            flags(inter_layer_ref_pic_flag[i], 1, i);
> > +        else
> > +            infer(inter_layer_ref_pic_flag[i], 0);
> > +
> > +        if(!current->inter_layer_ref_pic_flag[i] ) {
> > +            if(sps->sps_long_term_ref_pics_flag )
> > +                flags(st_ref_pic_flag[i],  1, i);
> > +            else
> > +                infer(st_ref_pic_flag[i], 1);
> > +            if(current->st_ref_pic_flag[i]) {
> > +                int abs_delta_poc_st;
> > +                ues(abs_delta_poc_st[i], 0, MAX_UINT_BITS(15), 1, i);
> > +                if( (sps->sps_weighted_pred_flag ||
> sps->sps_weighted_bipred_flag ) && i != 0 )
> > +                    abs_delta_poc_st = current->abs_delta_poc_st[i];
> > +                else
> > +                    abs_delta_poc_st = current->abs_delta_poc_st[i] + 1;
> > +                if (abs_delta_poc_st > 0)
> > +                    flags(strp_entry_sign_flag[i], 1, i);
> > +            } else {
> > +                if(!current->ltrp_in_header_flag) {
> > +                    ubs(sps->sps_log2_max_pic_order_cnt_lsb_minus4 + 4,
> rpls_poc_lsb_lt[j], 1, j);
> > +                    j++;
> > +                }
> > +                current->num_ltrp_entries++;
> > +            }
> > +        } else {
> > +            //todo: check range when vps parser is ready.
> > +            ues(ilrp_idx[i], 0, MAX_UINT_BITS(8), 1, i);
> > +        }
> > +
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int FUNC(ref_pic_lists)(CodedBitstreamContext *ctx, RWContext
> *rw,
> > +                H266RefPicLists *current)
> > +{
> > +    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;
> > +    for (i = 0; i < 2; i++) {
> > +        if (sps->sps_num_ref_pic_lists[i] > 0 && (i == 0 || (i == 1 &&
> pps->pps_rpl1_idx_present_flag))) {
> > +            flags(rpl_sps_flag[i], 1, i);
> > +        } else {
> > +            if (!sps->sps_num_ref_pic_lists[i]) {
> > +                infer(rpl_sps_flag[i], 0);
> > +            } else {
> > +                if (!pps->pps_rpl1_idx_present_flag && i == 1)
> > +                    infer(rpl_sps_flag[1], current->rpl_sps_flag[0]);
> > +            }
> > +        }
> > +        if (current->rpl_sps_flag[i]) {
> > +            if(sps->sps_num_ref_pic_lists[i] > 1 && (i == 0 || ( i == 1
> && pps->pps_rpl1_idx_present_flag))) {
> > +                us(ceil(log2(sps->sps_num_ref_pic_lists[i])),
> rpl_idx[i], 0, sps->sps_num_ref_pic_lists[i] - 1, 1, i);
>
> Don't use floating point arithmetic.  There is av_log2() for integers.
>
> > +            } else if (sps->sps_num_ref_pic_lists[i] == 1) {
> > +                infer(rpl_idx[i], 0);
> > +            } else if (i == 1 && !pps->pps_rpl1_idx_present_flag){
> > +                infer(rpl_idx[1], current->rpl_idx[0]);
> > +            } else {
> > +                //how to handle this? or never happpend?
>
> This sort of thing needs to be resolved.
>
> I think the other conditions are a cover?
>
the spec is not so clear to me here, and the reference code did not
strictly follow the spec.
We can only resolve this after we found a real clip.

>
> > +                av_log(ctx->log_ctx, AV_LOG_ERROR, "can't infer the
> rpl_idx[i]\n");
> > +                return AVERROR_PATCHWELCOME;
> > +            }
> > +            memcpy(&current->rpl_ref_list[i],
> &sps->sps_ref_pic_list_struct[i][current->rpl_idx[i]],
> sizeof(current->rpl_ref_list[i]));
> > +        } else {
> > +            CHECK(FUNC(ref_pic_list_struct)(ctx, rw,
> &current->rpl_ref_list[i], i, sps->sps_num_ref_pic_lists[i], sps));
> > +        }
> > +        ref_list = &current->rpl_ref_list[i];
> > +
> > +        for( j = 0; j < ref_list->num_ltrp_entries; j++ ) {
> > +            if(ref_list->ltrp_in_header_flag)
> > +                ubs(sps->sps_log2_max_pic_order_cnt_lsb_minus4 + 4,
> poc_lsb_lt[i][j], 2, i, j);
> > +            flags(delta_poc_msb_cycle_present_flag[i][j], 2, i, j);
> > +            if(current->delta_poc_msb_cycle_present_flag[i][j])
> > +                ues(delta_poc_msb_cycle_lt[i][j], 0, MAX_UINT_BITS(32 -
> sps->sps_log2_max_pic_order_cnt_lsb_minus4 - 4), 2, i, j);
> > +        }
> > +    }
> > +    return 0;
> > +
> > +}
> > +
> > +static int FUNC(general_timing_hrd_parameters)(CodedBitstreamContext
> *ctx, RWContext *rw,
> > +                     H266RawGeneralTimingHrdParameters *current)
> > +{
> > +    int err;
> > +    ub(32, num_units_in_tick);
> > +    u(32, time_scale, 1, MAX_UINT_BITS(32));
> > +    flag(general_nal_hrd_params_present_flag);
> > +    flag(general_vcl_hrd_params_present_flag);
> > +
> > +    if(current->general_nal_hrd_params_present_flag ||
> current->general_vcl_hrd_params_present_flag ) {
> > +        flag(general_same_pic_timing_in_all_ols_flag);
> > +        flag(general_du_hrd_params_present_flag);
> > +        if(current->general_du_hrd_params_present_flag)
> > +            ub(8, tick_divisor_minus2);
> > +        ub(4, bit_rate_scale);
> > +        ub(4, cpb_size_scale);
> > +        if(current->general_du_hrd_params_present_flag)
> > +            ub(4, cpb_size_du_scale);
> > +        ue(hrd_cpb_cnt_minus1, 0, 31);
> > +    } else {
> > +        //infer general_same_pic_timing_in_all_ols_flag?
>
> Looks like it should be, though the standard doesn't actually state it the
> way it does with general_du_hrd_params_present_flag.
>
ok, i will infer it.

>
> > +        infer(general_du_hrd_params_present_flag, 0);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int FUNC(sublayer_hrd_parameters)(CodedBitstreamContext *ctx,
> RWContext *rw,
> > +                     H266RawSubLayerHRDParameters* current, int
> sublayer_id, const H266RawGeneralTimingHrdParameters* const general)
> > +{
> > +    int err, i;
> > +    for (i = 0; i <= general->hrd_cpb_cnt_minus1; i++) {
> > +        ues(bit_rate_value_minus1[i], 0, UINT32_MAX - 1, 2,
> sublayer_id, i);
> > +        ues(cpb_size_value_minus1[i], 0, UINT32_MAX - 1, 2,
> sublayer_id, i);
> > +        if (general->general_du_hrd_params_present_flag) {
> > +            ues(cpb_size_du_value_minus1[i], 0, UINT32_MAX - 1, 2,
> sublayer_id, i);
> > +            ues(bit_rate_du_value_minus1[i], 0, UINT32_MAX - 1, 2,
> sublayer_id, i);
> > +        }
> > +        flags(cbr_flag[i], 2, sublayer_id, i);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int FUNC(ols_timing_hrd_parameters)(CodedBitstreamContext *ctx,
> RWContext *rw,
> > +                     H266RawOlsTimingHrdParameters* current, uint8_t
> first_sublayer, uint8_t max_sublayers_minus1, const
> H266RawGeneralTimingHrdParameters * general)
> > +{
> > +    int err, i;
> > +    for( i = first_sublayer; i <= max_sublayers_minus1; i++ ) {
> > +        flags(fixed_pic_rate_general_flag[i], 1, i);
> > +        if (!current->fixed_pic_rate_general_flag[i])
> > +            flags(fixed_pic_rate_within_cvs_flag[i], 1, i);
> > +        else
> > +            infer(fixed_pic_rate_within_cvs_flag[i], 1);
> > +        if(current->fixed_pic_rate_within_cvs_flag[i]) {
> > +            ues(elemental_duration_in_tc_minus1[i], 0, 2047, 1, i);
> > +            infer(low_delay_hrd_flag[i], 0);
> > +        } else if((general->general_nal_hrd_params_present_flag ||
> general->general_vcl_hrd_params_present_flag)
> > +            && general->hrd_cpb_cnt_minus1 == 0 ) {
> > +            flags(low_delay_hrd_flag[i], 1, i);
> > +        } else {
> > +            infer(low_delay_hrd_flag[i], 0);
> > +        }
> > +        if(general->general_nal_hrd_params_present_flag)
> > +            CHECK(FUNC(sublayer_hrd_parameters)(ctx, rw,
> &current->nal_sub_layer_hrd_parameters[i], i, general));
> > +        if(general->general_vcl_hrd_params_present_flag)
> > +            CHECK(FUNC(sublayer_hrd_parameters)(ctx, rw,
> &current->nal_sub_layer_hrd_parameters[i], i, general));
> > +    }
> > +    return 0;
> > +}
> > +
> > +static uint8_t get_extra_bits(const uint8_t* present_flags, uint8_t
> extra_bytes)
> > +{
> > +    uint8_t num_extra_bits = 0;
> > +    int i;
> > +    for (i = 0; i < extra_bytes * 8; i++) {
> > +        if (present_flags[i])
> > +            num_extra_bits++;
> > +    }
> > +    return num_extra_bits;
> > +}
> > +
> > +static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw,
> > +                     H266RawSPS *current)
> > +{
> > +    CodedBitstreamH266Context *h266 = ctx->priv_data;
> > +    const H266RawVPS *vps;
> > +    int err, i, j;
> > +    unsigned int ctb_log2_size_y, min_cb_log2_size_y,
> > +                 min_qt_log2_size_intra_y, min_qt_log2_size_inter_y,
> > +                 ctb_size_y, max_num_merge_cand;
> > +
> > +    HEADER("Sequence Parameter Set");
> > +
> > +    CHECK(FUNC(nal_unit_header)(ctx, rw, &current->nal_unit_header,
> H266_NAL_SPS));
> > +
> > +    ub(4, sps_seq_parameter_set_id);
> > +    ub(4, sps_video_parameter_set_id);
> > +    h266->active_vps = vps =
> h266->vps[current->sps_video_parameter_set_id];
>
> Not if sps_video_parameter_set_id == 0.
>
> Should we be making an implied VPS in that case?
>
yes, I think you are right

>
> > +
> > +    u(3, sps_max_sublayers_minus1, 0, H266_MAX_SUB_LAYERS - 1);
> > +    u(2, sps_chroma_format_idc, 0, 3);
> > +    u(2, sps_log2_ctu_size_minus5, 0, 3);
> > +    ctb_log2_size_y = current->sps_log2_ctu_size_minus5 + 5;
> > +    ctb_size_y = 1 << ctb_log2_size_y;
> > +
> > +    flag(sps_ptl_dpb_hrd_params_present_flag);
> > +    if (current->sps_ptl_dpb_hrd_params_present_flag) {
> > +        CHECK(FUNC(profile_tier_level)(ctx, rw,
> &current->profile_tier_level,
> > +                                    1,
> current->sps_max_sublayers_minus1));
> > +    }
> > +    flag(sps_gdr_enabled_flag);
> > +    flag(sps_ref_pic_resampling_enabled_flag);
> > +    if(current->sps_ref_pic_resampling_enabled_flag)
> > +        flag(sps_res_change_in_clvs_allowed_flag);
> > +    else
> > +        infer(sps_res_change_in_clvs_allowed_flag, 0);
> > +
> > +    ue(sps_pic_width_max_in_luma_samples,  1, H266_MAX_WIDTH);
> > +    ue(sps_pic_height_max_in_luma_samples, 1, H266_MAX_HEIGHT);
> > +
> > +    flag(sps_conformance_window_flag);
> > +    if (current->sps_conformance_window_flag) {
> > +        ue(sps_conf_win_left_offset,    0,
> current->sps_pic_width_max_in_luma_samples);
> > +        ue(sps_conf_win_right_offset,   0,
> current->sps_pic_width_max_in_luma_samples);
> > +        ue(sps_conf_win_top_offset,     0,
> current->sps_pic_height_max_in_luma_samples);
> > +        ue(sps_conf_win_bottom_offset,   0,
> current->sps_pic_height_max_in_luma_samples);
>
> These bounds aren't right, because the value is post-subsampling.  Divide
> by SubWidthC.
>
> > +    } else {
> > +        infer(sps_conf_win_left_offset,     0);
> > +        infer(sps_conf_win_right_offset,    0);
> > +        infer(sps_conf_win_top_offset,      0);
> > +        infer(sps_conf_win_bottom_offset,    0);
> > +    }
> > +    flag(sps_subpic_info_present_flag);
> > +    if (current->sps_subpic_info_present_flag) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR, "subpic is not supported
> yet\n");
> > +        return AVERROR_PATCHWELCOME;
> > +    } else {
> > +        infer(sps_num_subpics_minus1, 0);
> > +        infer(sps_independent_subpics_flag, 1);
> > +        infer(sps_subpic_same_size_flag, 0);
> > +        //?sps_subpic_id_len_minus1;
>
> Doesn't matter, because subpic_id fields don't appear if no subpics.
>
> > +        infer(sps_subpic_id_mapping_explicitly_signalled_flag, 0);
> > +
> > +    }
> > +
> > +    ue(sps_bitdepth_minus8,   0, 2);
> > +    current->qp_bd_offset = 6 * current->sps_bitdepth_minus8;
>
> QpBdOffset isn't a bitstream value and is only used in this function, so
> put it on the stack.
>
no it will use by picture header and slice header, I can put it in stack if
you perfered.

>
> > +
> > +    flag(sps_entropy_coding_sync_enabled_flag);
> > +    flag(sps_entry_point_offsets_present_flag);
> > +
> > +    u(4, sps_log2_max_pic_order_cnt_lsb_minus4, 0, 12);
> > +    flag(sps_poc_msb_cycle_flag);
> > +    if (current->sps_poc_msb_cycle_flag)
> > +        ue(sps_poc_msb_cycle_len_minus1, 0, 32 -
> current->sps_log2_max_pic_order_cnt_lsb_minus4 - 5);
> > +
> > +    u(2, sps_num_extra_ph_bytes, 0, 2);
> > +    for (i = 0; i < (current->sps_num_extra_ph_bytes * 8 ); i++) {
> > +        flags(sps_extra_ph_bit_present_flag[i], 1, i);
> > +    }
> > +    current->num_extra_ph_bits =
> get_extra_bits(current->sps_extra_ph_bit_present_flag,
> current->sps_num_extra_ph_bytes);
> > +
> > +    u(2, sps_num_extra_sh_bytes, 0, 0);
> > +    for (i = 0; i < (current->sps_num_extra_sh_bytes * 8 ); i++) {
> > +        flags(sps_extra_sh_bit_present_flag[i], 1, i);
> > +    }
> > +    current->num_extra_sh_bits =
> get_extra_bits(current->sps_extra_sh_bit_present_flag,
> current->sps_num_extra_sh_bytes);
>
> Don't include the derived NumExtra[PS]hBits fields in the raw SPS
> structure.  They can trivially be recalculated when used, so I think do it
> there.
>
sure

>
> > +
> > +    if (current->sps_ptl_dpb_hrd_params_present_flag) {
> > +        if (current->sps_max_sublayers_minus1 > 0)
> > +            flag(sps_sublayer_dpb_params_flag);
> > +        else
> > +            infer(sps_sublayer_dpb_params_flag, 0);
> > +        CHECK(FUNC(dpb_parameters)(ctx, rw, &current->sps_dpb_params,
> current->sps_max_sublayers_minus1,
> > +                                current->sps_sublayer_dpb_params_flag));
> > +    }
> > +
> > +    ue(sps_log2_min_luma_coding_block_size_minus2, 0, FFMIN(4,
> current->sps_log2_ctu_size_minus5 + 3));
> > +    min_cb_log2_size_y = current->min_cb_log2_size_y =
> current->sps_log2_min_luma_coding_block_size_minus2 + 2;
> > +
> > +    flag(sps_partition_constraints_override_enabled_flag);
> > +
> > +    ue(sps_log2_diff_min_qt_min_cb_intra_slice_luma, 0, FFMIN(6,
> ctb_log2_size_y) - min_cb_log2_size_y);
> > +    min_qt_log2_size_intra_y =
> current->sps_log2_diff_min_qt_min_cb_intra_slice_luma + min_cb_log2_size_y;
> > +
> > +    ue(sps_max_mtt_hierarchy_depth_intra_slice_luma, 0, 2 *
> (ctb_log2_size_y - min_cb_log2_size_y));
> > +
> > +    if (current->sps_max_mtt_hierarchy_depth_intra_slice_luma) {
>
> Try to use the same styling as the standard as far as possible, so != 0 if
> it isn't a flag.
>
> > +        ue(sps_log2_diff_max_bt_min_qt_intra_slice_luma, 0,
> ctb_log2_size_y - min_qt_log2_size_intra_y);
> > +        ue(sps_log2_diff_max_tt_min_qt_intra_slice_luma, 0, FFMIN(6,
> ctb_log2_size_y) - min_qt_log2_size_intra_y);
> > +    } else {
> > +        infer(sps_log2_diff_max_bt_min_qt_intra_slice_luma, 0);
> > +        infer(sps_log2_diff_max_tt_min_qt_intra_slice_luma, 0);
> > +    }
> > +
> > +    if (current->sps_chroma_format_idc) {
> > +        flag(sps_qtbtt_dual_tree_intra_flag);
> > +    } else {
> > +        infer(sps_qtbtt_dual_tree_intra_flag, 0);
> > +    }
> > +
> > +    if(current->sps_qtbtt_dual_tree_intra_flag) {
> > +        ue(sps_log2_diff_min_qt_min_cb_intra_slice_chroma, 0, FFMIN(6,
> ctb_log2_size_y) - min_cb_log2_size_y);
> > +        ue(sps_max_mtt_hierarchy_depth_intra_slice_chroma, 0, 2 *
> (ctb_log2_size_y - min_cb_log2_size_y));
> > +        if(current->sps_max_mtt_hierarchy_depth_intra_slice_chroma) {
> > +            unsigned int min_qt_log2_size_intra_c =
> current->sps_log2_diff_min_qt_min_cb_intra_slice_chroma +
> min_cb_log2_size_y;
> > +            ue(sps_log2_diff_max_bt_min_qt_intra_slice_chroma, 0,
> FFMIN(6, ctb_log2_size_y) - min_qt_log2_size_intra_c);
> > +            ue(sps_log2_diff_max_tt_min_qt_intra_slice_chroma, 0,
> FFMIN(6, ctb_log2_size_y) - min_qt_log2_size_intra_c);
> > +        }
> > +    } else {
> > +        infer(sps_log2_diff_min_qt_min_cb_intra_slice_chroma, 0);
> > +        infer(sps_max_mtt_hierarchy_depth_intra_slice_chroma, 0);
> > +    }
> > +    if (!current->sps_max_mtt_hierarchy_depth_intra_slice_chroma) {
> > +        infer(sps_log2_diff_max_bt_min_qt_intra_slice_chroma, 0);
> > +        infer(sps_log2_diff_max_tt_min_qt_intra_slice_chroma, 0);
> > +    }
> > +
> > +    ue(sps_log2_diff_min_qt_min_cb_inter_slice, 0, FFMIN(6,
> ctb_log2_size_y) - min_cb_log2_size_y);
> > +    min_qt_log2_size_inter_y =
> current->sps_log2_diff_min_qt_min_cb_inter_slice + min_cb_log2_size_y;
> > +
> > +    ue(sps_max_mtt_hierarchy_depth_inter_slice, 0, 2 * (ctb_log2_size_y
> - min_cb_log2_size_y));
> > +    if(current->sps_max_mtt_hierarchy_depth_inter_slice) {
> > +        ue(sps_log2_diff_max_bt_min_qt_inter_slice, 0, ctb_log2_size_y
> - min_qt_log2_size_inter_y);
> > +        ue(sps_log2_diff_max_tt_min_qt_inter_slice, 0, FFMIN(6,
> ctb_log2_size_y) - min_qt_log2_size_inter_y);
> > +    } else {
> > +        infer(sps_log2_diff_max_bt_min_qt_inter_slice, 0);
> > +        infer(sps_log2_diff_max_tt_min_qt_inter_slice, 0);
> > +    }
> > +
> > +    if (ctb_size_y > 32)
> > +        flag(sps_max_luma_transform_size_64_flag);
> > +    else
> > +        infer(sps_max_luma_transform_size_64_flag, 0);
> > +
> > +    flag(sps_transform_skip_enabled_flag);
> > +    if (current->sps_transform_skip_enabled_flag) {
> > +        ue(sps_log2_transform_skip_max_size_minus2, 0, 3);
> > +        flag(sps_bdpcm_enabled_flag);
> > +    }
> > +
> > +    ...
>
> I haven't read through all of this in detail.  It needs at least one other
> person to check through and compare against the standard.
>
sure, I will update according to your comment, send out the patch and
continue work on conformance passrate.


>
> > +
> > +    if(current->sps_ptl_dpb_hrd_params_present_flag) {
> > +        flag(sps_timing_hrd_params_present_flag);
> > +        if (current->sps_timing_hrd_params_present_flag) {
> > +            uint8_t first_sublayer;
> > +            CHECK(FUNC(general_timing_hrd_parameters)(ctx, rw,
> &current->sps_general_timing_hrd_parameters));
> > +            if(current->sps_max_sublayers_minus1 > 0)
> > +                flag(sps_sublayer_cpb_params_present_flag);
> > +            else
> > +                infer(sps_sublayer_cpb_params_present_flag, 0);
> > +            first_sublayer =
> current->sps_sublayer_cpb_params_present_flag ? 0 :
> current->sps_max_sublayers_minus1;
> > +            CHECK(FUNC(ols_timing_hrd_parameters)(ctx, rw,
> &current->sps_ols_timing_hrd_parameters, first_sublayer,
> current->sps_max_sublayers_minus1,
> &current->sps_general_timing_hrd_parameters));
> > +        }
> > +    }
> > +
> > +    flag(sps_field_seq_flag);
> > +    flag(sps_vui_parameters_present_flag);
> > +    if (current->sps_vui_parameters_present_flag) {
> > +        ue(sps_vui_payload_size_minus1, 0, 1023);
> > +        while (!byte_alignment(rw))
> > +            fixed(1, sps_vui_alignment_zero_bit, 0);
> > +        CHECK(FUNC(vui_payload)(ctx, rw, &current->vui,
> current->sps_vui_payload_size_minus1 + 1));
> > +    }
> > +
> > +    flag(sps_extension_flag);
> > +    if (current->sps_extension_flag) {
> > +        return AVERROR_PATCHWELCOME;
>
> Also wanted for passthrough.
>
> > +    }
> > +    CHECK(FUNC(rbsp_trailing_bits)(ctx, rw));
> > +
> > +    return 0;
> > +}
> > +
> > +static const uint8_t h266_sub_width_c[] = {
> > +    1, 2, 2, 1
> > +};
> > +
> > +static const uint8_t h266_sub_height_c[] = {
> > +    1, 2, 1, 1
> > +};
>
> You can't put fixed tables here, because the file is included twice.
>
> These can be inside the function below.
>
> sure, good suggestion.

> > +
> > +static int get_num_tile(const uint16_t* pps_tile_size_minus1, uint16_t
> num_exp_tile_minus1, unsigned int pic_size_in_ctbs_y, unsigned int
> exp_tile_size)
> > +{
> > +    uint16_t uniform_size = pps_tile_size_minus1[num_exp_tile_minus1] +
> 1;
> > +    unsigned int left = pic_size_in_ctbs_y - exp_tile_size;
> > +    return num_exp_tile_minus1 + 1 + (left + uniform_size - 1) /
> uniform_size;
> > +}
>
> Likewise functions.
>
> The derived tile values don't want to go in the bitstream value structure,
> anyway.  They can be calculated locally here.
>

> > +
> > +static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext *rw,
> > +                     H266RawPPS *current)
> > +{
> > +
> > +    CodedBitstreamH266Context *h266 = ctx->priv_data;
> > +    const H266RawSPS *sps;
> > +    int err, i;
> > +    unsigned int min_cb_size_y, divisor, ctb_size_y,
> > +            pic_width_in_ctbs_y, pic_height_in_ctbs_y;
> > +    uint8_t sub_width_c, sub_height_c;
> > +
> > +    HEADER("Picture Parameter Set");
> > +
> > +    CHECK(FUNC(nal_unit_header)(ctx, rw, &current->nal_unit_header,
> H266_NAL_PPS));
> > +
> > +    ub(6, pps_pic_parameter_set_id);
> > +    ub(4, pps_seq_parameter_set_id);
> > +    sps = h266->sps[current->pps_seq_parameter_set_id];
> > +    if (!sps) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n",
> > +               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, 1,
> sps->sps_pic_width_max_in_luma_samples);
> > +    ue(pps_pic_height_in_luma_samples, 1,
> sps->sps_pic_height_max_in_luma_samples);
> > +
> > +    min_cb_size_y = 1 <<
> (sps->sps_log2_min_luma_coding_block_size_minus2 + 2);
> > +    divisor = FFMAX(min_cb_size_y, 8);
> > +    if (current->pps_pic_width_in_luma_samples  % divisor ||
> > +        current->pps_pic_height_in_luma_samples % divisor) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid dimensions: %ux%u
> not divisible "
> > +               "by %u, MinCbSizeY = %u.\n",
> current->pps_pic_width_in_luma_samples,
> > +               current->pps_pic_height_in_luma_samples, divisor,
> min_cb_size_y);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    if (!sps->sps_res_change_in_clvs_allowed_flag &&
> > +        (current->pps_pic_width_in_luma_samples !=
> sps->sps_pic_width_max_in_luma_samples ||
> > +        current->pps_pic_height_in_luma_samples !=
> sps->sps_pic_height_max_in_luma_samples)) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Resoltuion change is not
> allowed, in max resolution (%ux%u) mismatched with pps(%ux%u).\n",
> > +            sps->sps_pic_width_max_in_luma_samples,
> > +            sps->sps_pic_height_max_in_luma_samples,
> > +            current->pps_pic_width_in_luma_samples,
> > +            current->pps_pic_height_in_luma_samples);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    ctb_size_y = 1 << (sps->sps_log2_ctu_size_minus5 + 5);
> > +    if (sps->sps_ref_wraparound_enabled_flag) {
> > +        if ((ctb_size_y / min_cb_size_y + 1) >
> (current->pps_pic_width_in_luma_samples / min_cb_size_y - 1)) {
> > +            av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid width(%u),
> ctb_size_y = %u, min_cb_size_y = %u.\n",
> > +                current->pps_pic_width_in_luma_samples,
> > +                ctb_size_y, min_cb_size_y);
> > +            return AVERROR_INVALIDDATA;
> > +        }
> > +    }
> > +
> > +    flag(pps_conformance_window_flag);
> > +    if (current->pps_pic_width_in_luma_samples ==
> sps->sps_pic_width_max_in_luma_samples &&
> > +        current->pps_pic_height_in_luma_samples ==
> sps->sps_pic_height_max_in_luma_samples &&
> > +        current->pps_conformance_window_flag) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Conformance window flag
> should not true.\n");
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    sub_width_c = h266_sub_width_c[sps->sps_chroma_format_idc];
> > +    sub_height_c = h266_sub_height_c[sps->sps_chroma_format_idc];
> > +    if (current->pps_conformance_window_flag) {
> > +        ue(pps_conf_win_left_offset, 0,
> current->pps_pic_width_in_luma_samples);
> > +        ue(pps_conf_win_right_offset, 0,
> current->pps_pic_width_in_luma_samples);
> > +        ue(pps_conf_win_top_offset, 0,
> current->pps_pic_height_in_luma_samples);
> > +        ue(pps_conf_win_bottom_offset, 0,
> current->pps_pic_height_in_luma_samples);
> > +        if (sub_width_c * (current->pps_conf_win_left_offset +
> current->pps_conf_win_right_offset) >=
> current->pps_pic_width_in_luma_samples ||
> > +            sub_height_c * (current->pps_conf_win_top_offset +
> current->pps_conf_win_bottom_offset) >=
> current->pps_pic_height_in_luma_samples) {
> > +            av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid pps conformance
> window: (%u, %u, %u, %u), resolution is %ux%u, sub wxh is %ux%u.\n",
> > +                current->pps_conf_win_left_offset,
> current->pps_conf_win_right_offset,
> > +                current->pps_conf_win_top_offset,
> current->pps_conf_win_bottom_offset,
> > +                current->pps_pic_width_in_luma_samples,
> current->pps_pic_height_in_luma_samples,
> > +                sub_width_c, sub_height_c);
> > +            return AVERROR_INVALIDDATA;
> > +        }
> > +    } else {
> > +        if (current->pps_pic_width_in_luma_samples ==
> sps->sps_pic_width_max_in_luma_samples &&
> > +            current->pps_pic_height_in_luma_samples ==
> sps->sps_pic_height_max_in_luma_samples) {
> > +            infer(pps_conf_win_left_offset,
> sps->sps_conf_win_left_offset);
> > +            infer(pps_conf_win_right_offset,
> sps->sps_conf_win_right_offset);
> > +            infer(pps_conf_win_top_offset,
> sps->sps_conf_win_top_offset);
> > +            infer(pps_conf_win_bottom_offset,
> sps->sps_conf_win_bottom_offset);
> > +        } else {
> > +            infer(pps_conf_win_left_offset, 0);
> > +            infer(pps_conf_win_right_offset, 0);
> > +            infer(pps_conf_win_top_offset, 0);
> > +            infer(pps_conf_win_bottom_offset, 0);
> > +        }
> > +
> > +    }
> > +
> > +    flag(pps_scaling_window_explicit_signalling_flag);
> > +    if (!sps->sps_ref_pic_resampling_enabled_flag &&
> current->pps_scaling_window_explicit_signalling_flag) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid data:
> sps_ref_pic_resampling_enabled_flag is false, but
> pps_scaling_window_explicit_signalling_flag is true.\n");
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    if (current->pps_scaling_window_explicit_signalling_flag) {
> > +        se(pps_scaling_win_left_offset,
> -current->pps_pic_width_in_luma_samples * 15 / sub_width_c,
> current->pps_pic_width_in_luma_samples /  sub_width_c);
> > +        se(pps_scaling_win_right_offset,
> -current->pps_pic_width_in_luma_samples * 15 / sub_width_c,
> current->pps_pic_width_in_luma_samples /  sub_width_c);
> > +        se(pps_scaling_win_top_offset,
> -current->pps_pic_height_in_luma_samples * 15 / sub_height_c,
> current->pps_pic_height_in_luma_samples /  sub_height_c);
> > +        se(pps_scaling_win_bottom_offset,
> -current->pps_pic_height_in_luma_samples * 15 /sub_height_c,
> current->pps_pic_height_in_luma_samples /  sub_height_c);
> > +    } else {
> > +        infer(pps_scaling_win_left_offset,
> current->pps_conf_win_left_offset);
> > +        infer(pps_scaling_win_right_offset,
> current->pps_conf_win_right_offset);
> > +        infer(pps_scaling_win_top_offset,
> current->pps_conf_win_top_offset);
> > +        infer(pps_scaling_win_bottom_offset,
> current->pps_conf_win_bottom_offset);
> > +    }
> > +
> > +    flag(pps_output_flag_present_flag);
> > +    flag(pps_no_pic_partition_flag);
> > +    flag(pps_subpic_id_mapping_present_flag);
> > +
> > +    if(current->pps_subpic_id_mapping_present_flag ) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR,
> "pps_subpic_id_mapping_present_flag is not supported.\n");
> > +        return AVERROR_PATCHWELCOME;
> > +    }
> > +
> > +    if(!current->pps_no_pic_partition_flag ) {
> > +        unsigned int exp_tile_width = 0, exp_tile_height = 0;
> > +        pic_width_in_ctbs_y =
> ceil(current->pps_pic_width_in_luma_samples / (double)ctb_size_y);
> > +        pic_height_in_ctbs_y =
> ceil(current->pps_pic_height_in_luma_samples / (double)ctb_size_y);
>
> Don't use floating point.  (int)ceil((double)a/b) == (a + b - 1) / b
>
> > +        u(2, pps_log2_ctu_size_minus5, sps->sps_log2_ctu_size_minus5,
> sps->sps_log2_ctu_size_minus5);
> > +
> > +        ue(pps_num_exp_tile_columns_minus1, 0,
> FFMIN(pic_width_in_ctbs_y - 1, H266_MAX_TILE_COLUMNS));
> > +        ue(pps_num_exp_tile_rows_minus1, 0, FFMIN(pic_height_in_ctbs_y
> - 1, H266_MAX_TILE_ROWS));
> > +
> > +        for( i = 0; i <= current->pps_num_exp_tile_columns_minus1; i++
> ) {
> > +            ues(pps_tile_column_width_minus1[i], 0, pic_width_in_ctbs_y
> - 1, 1, i);
> > +            exp_tile_width += current->pps_tile_column_width_minus1[i]
> + 1;
> > +        }
> > +        current->num_tile_columns =
> get_num_tile(current->pps_tile_column_width_minus1,
> current->pps_num_exp_tile_columns_minus1, pic_width_in_ctbs_y,
> exp_tile_width);
> > +
> > +        for( i = 0; i <= current->pps_num_exp_tile_rows_minus1; i++ ) {
> > +            ues(pps_tile_row_height_minus1[i], 0, pic_height_in_ctbs_y
> - 1, 1, i);
> > +            exp_tile_height += current->pps_tile_row_height_minus1[i] +
> 1;
> > +        }
> > +        current->num_tile_rows =
> get_num_tile(current->pps_tile_row_height_minus1,
> current->pps_num_exp_tile_rows_minus1, pic_height_in_ctbs_y,
> exp_tile_height);
> > +
> > +        current->num_tiles_in_pic = current->num_tile_columns *
> current->num_tile_rows;
> > +        if (current->num_tiles_in_pic > 1) {
> > +            flag(pps_loop_filter_across_tiles_enabled_flag);
> > +            flag(pps_rect_slice_flag);
> > +        } else {
> > +            infer(pps_loop_filter_across_tiles_enabled_flag, 0);
> > +            infer(pps_rect_slice_flag, 1);
> > +        }
> > +        if(current->pps_rect_slice_flag)
> > +            flag(pps_single_slice_per_subpic_flag);
> > +        else
> > +            infer(pps_single_slice_per_subpic_flag, 1);
> > +        if(current->pps_rect_slice_flag &&
> !current->pps_single_slice_per_subpic_flag) {
> > +            ue(pps_num_slices_in_pic_minus1, 0, H266_MAX_SLICES);
> > +            if(current->pps_num_slices_in_pic_minus1 > 1 )
> > +                flag(pps_tile_idx_delta_present_flag);
> > +            else
> > +                infer(pps_tile_idx_delta_present_flag, 0);
> > +            return AVERROR_PATCHWELCOME;
> > +        }
> > +        if( !current->pps_rect_slice_flag ||
> current->pps_single_slice_per_subpic_flag ||
> current->pps_num_slices_in_pic_minus1 > 0 )
> > +            flag(pps_loop_filter_across_slices_enabled_flag);
> > +        else
> > +            infer(pps_loop_filter_across_slices_enabled_flag, 0);
> > +    } else {
> > +        infer(pps_num_exp_tile_columns_minus1, 0);
> > +        infer(pps_num_exp_tile_rows_minus1, 0);
> > +        infer(num_tile_columns, 1);
> > +        infer(num_tile_rows, 1);
> > +        infer(num_tiles_in_pic, 1);
> > +   }
> > +    flag(pps_cabac_init_present_flag);
> > +    for( i = 0; i < 2; i++ )
> > +        ues(pps_num_ref_idx_default_active_minus1[i], 0, 14, 1, i);
> > +    flag(pps_rpl1_idx_present_flag);
> > +    flag(pps_weighted_pred_flag);
> > +    flag(pps_weighted_bipred_flag);
> > +    flag(pps_ref_wraparound_enabled_flag);
> > +    if(current->pps_ref_wraparound_enabled_flag)
> > +        ue(pps_pic_width_minus_wraparound_offset, 0,
> (current->pps_pic_width_in_luma_samples / min_cb_size_y) - (ctb_size_y /
> min_cb_size_y) - 2);
> > +
> > +    se(pps_init_qp_minus26, -(26 + sps->qp_bd_offset), 37);
> > +    flag(pps_cu_qp_delta_enabled_flag);
> > +    flag(pps_chroma_tool_offsets_present_flag);
> > +    if(current->pps_chroma_tool_offsets_present_flag) {
> > +        se(pps_cb_qp_offset, -12, 12);
> > +        se(pps_cr_qp_offset, -12, 12);
> > +        flag(pps_joint_cbcr_qp_offset_present_flag);
> > +        if (current->pps_joint_cbcr_qp_offset_present_flag)
> > +            se(pps_joint_cbcr_qp_offset_value, -12, 12);
> > +        else
> > +            infer(pps_joint_cbcr_qp_offset_value, 0);
> > +        flag(pps_slice_chroma_qp_offsets_present_flag);
> > +        flag(pps_cu_chroma_qp_offset_list_enabled_flag);
> > +        if (current->pps_cu_chroma_qp_offset_list_enabled_flag) {
> > +            return AVERROR_PATCHWELCOME;
> > +        }
> > +    } else {
> > +        infer(pps_cb_qp_offset, 0);
> > +        infer(pps_cr_qp_offset, 0);
> > +        infer(pps_joint_cbcr_qp_offset_present_flag, 0);
> > +        infer(pps_joint_cbcr_qp_offset_value, 0);
> > +        infer(pps_slice_chroma_qp_offsets_present_flag, 0);
> > +        infer(pps_cu_chroma_qp_offset_list_enabled_flag, 0);
> > +    }
> > +    flag(pps_deblocking_filter_control_present_flag);
> > +    if (current->pps_deblocking_filter_control_present_flag) {
> > +        return AVERROR_PATCHWELCOME;
> > +    }
> > +    if(!current->pps_no_pic_partition_flag) {
> > +        return AVERROR_PATCHWELCOME;
> > +    }
> > +    flag(pps_picture_header_extension_present_flag);
> > +    flag(pps_slice_header_extension_present_flag);
> > +    flag(pps_extension_flag);
> > +    if(current->pps_extension_flag) {
> > +        return AVERROR_PATCHWELCOME;
> > +    }
> > +    CHECK(FUNC(rbsp_trailing_bits)(ctx, rw));
> > +    return 0;
> > +}
> > +
> > +static int FUNC(aud)(CodedBitstreamContext *ctx, RWContext *rw,
> > +                     H266RawAUD *current)
> > +{
> > +    int err;
> > +
> > +    HEADER("Access Unit Delimiter");
> > +
> > +    CHECK(FUNC(nal_unit_header)(ctx, rw, &current->nal_unit_header,
> HEVC_NAL_AUD));
> > +
> > +    flag(aud_irap_or_gdr_flag);
> > +    ub(3, aud_pic_type);
>
> Range is 0 .. 2.
>
> > +
> > +    CHECK(FUNC(rbsp_trailing_bits)(ctx, rw));
> > +    return 0;
> > +}
> > +
> > +static int FUNC(picture_header)(CodedBitstreamContext *ctx, RWContext
> *rw,
> > +                                      H266RawPH *current)
> > +{
> > +    CodedBitstreamH266Context *h266 = ctx->priv_data;
> > +    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;
> > +
> > +    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, 63);
>
> H266_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",
> > +               pps->pps_seq_parameter_set_id);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    sps = h266->sps[pps->pps_seq_parameter_set_id];
> > +    if (!pps) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR, "sps id %d not available.\n",
> > +               pps->pps_seq_parameter_set_id);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +    h266->active_sps = sps;
> > +    h266->active_pps = pps;
> > +
> > +    ub(sps->sps_log2_max_pic_order_cnt_lsb_minus4 + 4,
> ph_pic_order_cnt_lsb);
> > +    if(current->ph_gdr_pic_flag)
> > +        ub(sps->sps_log2_max_pic_order_cnt_lsb_minus4 + 4,
> ph_recovery_poc_cnt);
> > +
> > +    ...
> > +
> > +    if (pps->pps_qp_delta_info_in_ph_flag)
> > +        se(ph_qp_delta, -sps->qp_bd_offset - (26 +
> pps->pps_init_qp_minus26), 63 - (26 + pps->pps_init_qp_minus26));
> > +
> > +    if (sps->sps_joint_cbcr_enabled_flag)
> > +        flag(ph_joint_cbcr_sign_flag);
> > +    else
> > +        infer(ph_joint_cbcr_sign_flag, 0);
> > +    if (sps->sps_sao_enabled_flag && pps->pps_sao_info_in_ph_flag ) {
> > +        flag(ph_sao_luma_enabled_flag);
> > +        if (sps->sps_chroma_format_idc)
> > +            flag(ph_sao_chroma_enabled_flag);
> > +        else
> > +            infer(ph_sao_chroma_enabled_flag, 0);
> > +    } else {
> > +        infer(ph_sao_luma_enabled_flag, 0);
> > +        infer(ph_sao_chroma_enabled_flag, 0);
> > +    }
> > +
> > +    if(pps->pps_dbf_info_in_ph_flag) {
> > +        return AVERROR_PATCHWELCOME;
> > +    }
> > +    if(pps->pps_picture_header_extension_present_flag) {
> > +        return AVERROR_PATCHWELCOME;
> > +    }
> > +    h266->active_ph = current;
> > +    return 0;
> > +}
> > +
> > +static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw,
> > +                                      H266RawSliceHeader *current)
> > +{
> > +    CodedBitstreamH266Context *h266 = ctx->priv_data;
> > +    const H266RawSPS *sps;
> > +    const H266RawPPS *pps;
> > +    const H266RawPH  *ph;
> > +    const H266RefPicLists* ref_pic_lists;
> > +    int err, i;
> > +    uint8_t nal_unit_type;
> > +
> > +    HEADER("Slice Header");
> > +
> > +    CHECK(FUNC(nal_unit_header)(ctx, rw, &current->nal_unit_header,
> -1));
> > +
> > +    flag(sh_picture_header_in_slice_header_flag);
> > +    if (current->sh_picture_header_in_slice_header_flag){
> > +        CHECK(FUNC(picture_header)(ctx, rw, &h266->ph));
>
> That's not going to work.  The picture header needs to be a substructure
> of slice header to regenerate correctly.
>
> To fill in other slices, a copy might be needed.  I'm not sure what the
> best way of doing that is - maybe the other slices should fill their own
> picture header with the original one as if inferred?
>
This should be safe, according to 7.4.2.4.4. "When a VCL NAL unit has
sh_picture_header_in_slice_header_flag equal to 1 or is the first VCL NAL
unit that follows a PH NAL unit, the VCL NAL unit is the first VCL NAL unit
of a picture.”
Only the first slice will have sh_picture_header_in_slice_header_flag
equals to true.


> > +    }
> > +    sps = h266->active_sps;
> > +    pps = h266->active_pps;
> > +    ph  = &h266->ph;
> > +
> > +    if (!sps || !pps) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR, "no sps(%p) or pps(%p).\n",
> sps, pps);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    if(sps->sps_subpic_info_present_flag)
> > +        ub(sps->sps_subpic_id_len_minus1 + 1, sh_subpic_id);
> > +    if (pps->pps_rect_slice_flag)
> > +        return AVERROR_PATCHWELCOME;
> > +    if((!pps->pps_rect_slice_flag && pps->num_tiles_in_pic > 1)) {
> > +        unsigned int bits, max;
> > +        if (!pps->pps_rect_slice_flag) {
> > +            bits = ceil(log2(pps->num_tiles_in_pic));
>
> Floating point.
>
> > +            max = pps->num_tiles_in_pic - 1;
> > +        } else {
> > +            return AVERROR_PATCHWELCOME;
> > +        }
> > +        u(bits, sh_slice_address, 0, max);
> > +    } else {
> > +        infer(sh_slice_address, 0);
> > +    }
> > +
> > +    ...
> > +    if(pps->pps_slice_header_extension_present_flag) {
> > +        return AVERROR_PATCHWELCOME;
> > +    }
> > +    if(sps->sps_entry_point_offsets_present_flag)  {
> > +        unsigned int num_entry_points = 0;
> > +        if (pps->pps_rect_slice_flag) {
> > +            return AVERROR_PATCHWELCOME;
> > +        } else {
> > +            unsigned int tile_idx;
> > +            for (tile_idx = current->sh_slice_address; tile_idx <=
> current->sh_slice_address + current->sh_num_tiles_in_slice_minus1;
> tile_idx++) {
> > +                unsigned int tile_y, height;
> > +                tile_y = tile_idx / pps->num_tile_rows;
> > +                height = pps->pps_tile_row_height_minus1[FFMIN(tile_y,
> pps->pps_num_exp_tile_rows_minus1)] + 1;
> > +                num_entry_points +=
> (sps->sps_entropy_coding_sync_enabled_flag ? height : 1);
> > +            }
> > +            num_entry_points--;
> > +        }
> > +        infer(num_entry_point_offsets, num_entry_points);
> > +        if (num_entry_points > H266_MAX_ENTRY_POINT_OFFSETS) {
> > +            av_log(ctx->log_ctx, AV_LOG_ERROR, "Too many entry points: "
> > +               "%"PRIu16".\n", current->num_entry_point_offsets);
> > +            return AVERROR_PATCHWELCOME;
> > +        }
> > +        if (num_entry_points > 0) {
> > +            ue(sh_entry_offset_len_minus1, 0, 31);
> > +            for (i = 0; i < num_entry_points; i++) {
> > +                ubs(current->sh_entry_offset_len_minus1,
> sh_entry_point_offset_minus1[i], 1, i);
> > +            }
> > +        }
> > +    }
> > +    CHECK(FUNC(byte_alignment)(ctx, rw));
> > +
> > +    return 0;
> > +}
> > +
> > +static int FUNC(sei)(CodedBitstreamContext *ctx, RWContext *rw,
> > +                     H266RawSEI *current, int prefix)
> > +{
> > +    return 0;
> > +}
>
> - Mark
> _______________________________________________
> 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".

Reply via email to