| Message ID | 20230629224510.10393-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2,1/2] avcodec/cbs_h2645: fix parsing and storing Picture Header references in the context | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Fri, Jun 30, 2023 at 6:45 AM James Almer <jamrial@gmail.com> wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/cbs_h2645.c | 35 ++++++++++++++++----------- > libavcodec/cbs_h266.h | 17 +++++++------ > libavcodec/cbs_h266_syntax_template.c | 17 ++++++------- > libavcodec/h266_metadata_bsf.c | 13 +++++----- > libavcodec/vvc_parser.c | 10 ++++---- > 5 files changed, 50 insertions(+), 42 deletions(-) > > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c > index cdd7901518..68ccf6a7eb 100644 > --- a/libavcodec/cbs_h2645.c > +++ b/libavcodec/cbs_h2645.c > @@ -525,12 +525,6 @@ static int > cbs_h2645_split_fragment(CodedBitstreamContext *ctx, > if (frag->data_size == 0) > return 0; > > - if (codec_id == AV_CODEC_ID_VVC) { > - //we deactive picture header here to avoid reuse previous au's ph. > - CodedBitstreamH266Context *h266 = ctx->priv_data; > - h266->priv.ph = NULL; > - } > - > if (header && frag->data[0] && codec_id == AV_CODEC_ID_H264) { > // AVCC header. > size_t size, start, end; > @@ -793,19 +787,20 @@ cbs_h266_replace_ps(6, SPS, sps, > sps_seq_parameter_set_id) > cbs_h266_replace_ps(6, PPS, pps, pps_pic_parameter_set_id) > > static int cbs_h266_replace_ph(CodedBitstreamContext *ctx, > - CodedBitstreamUnit *unit) > + CodedBitstreamUnit *unit, > + H266RawPictureHeader *ph) > { > CodedBitstreamH266Context *h266 = ctx->priv_data; > int err; > > - h266->priv.ph = NULL; > err = ff_cbs_make_unit_refcounted(ctx, unit); > if (err < 0) > return err; > - err = av_buffer_replace(&h266->priv.ph_ref, unit->content_ref); > + av_assert0(unit->content_ref); > + err = av_buffer_replace(&h266->ph_ref, unit->content_ref); > if (err < 0) > return err; > - h266->priv.ph = (H266RawPH*)h266->priv.ph_ref->data; > + h266->ph = ph; > return 0; > } > > @@ -1111,7 +1106,7 @@ static int > cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, > err = cbs_h266_read_ph(ctx, &gbc, ph); > if (err < 0) > return err; > - err = cbs_h266_replace_ph(ctx, unit); > + err = cbs_h266_replace_ph(ctx, unit, &ph->ph_picture_header); > if (err < 0) > return err; > } > @@ -1139,6 +1134,12 @@ static int > cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, > pos = get_bits_count(&gbc); > len = unit->data_size; > > + if (slice->header.sh_picture_header_in_slice_header_flag) { > + err = cbs_h266_replace_ph(ctx, unit, > &slice->header.sh_picture_header); > + if (err < 0) > + return err; > + } > + > slice->data_size = len - pos / 8; > slice->data_ref = av_buffer_ref(unit->data_ref); > if (!slice->data_ref) > @@ -1640,7 +1641,7 @@ static int > cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h266_replace_ph(ctx, unit); > + err = cbs_h266_replace_ph(ctx, unit, &ph->ph_picture_header); > if (err < 0) > return err; > } > @@ -1661,6 +1662,12 @@ static int > cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > + if (slice->header.sh_picture_header_in_slice_header_flag) { > + err = cbs_h266_replace_ph(ctx, unit, > &slice->header.sh_picture_header); > + if (err < 0) > + return err; > + } > + > if (slice->data) { > err = cbs_h2645_write_slice_data(ctx, pbc, slice->data, > slice->data_size, > @@ -1884,8 +1891,8 @@ static void cbs_h266_flush(CodedBitstreamContext > *ctx) > av_buffer_unref(&h266->pps_ref[i]); > h266->pps[i] = NULL; > } > - av_buffer_unref(&h266->priv.ph_ref); > - h266->priv.ph = NULL; > + av_buffer_unref(&h266->ph_ref); > + h266->ph = NULL; > } > > static void cbs_h266_close(CodedBitstreamContext *ctx) > diff --git a/libavcodec/cbs_h266.h b/libavcodec/cbs_h266.h > index 03dfd4a954..54590748c3 100644 > --- a/libavcodec/cbs_h266.h > +++ b/libavcodec/cbs_h266.h > @@ -581,8 +581,7 @@ typedef struct H266RawPredWeightTable { > int16_t delta_chroma_offset_l1[15][2]; > } H266RawPredWeightTable; > > -typedef struct H266RawPH { > - H266RawNALUnitHeader nal_unit_header; > +typedef struct H266RawPictureHeader { > uint8_t ph_gdr_or_irap_pic_flag; > uint8_t ph_non_ref_pic_flag; > uint8_t ph_gdr_pic_flag; > @@ -670,12 +669,17 @@ typedef struct H266RawPH { > > uint8_t ph_extension_length; > uint8_t ph_extension_data_byte[256]; > +} H266RawPictureHeader; > + > +typedef struct H266RawPH { > + H266RawNALUnitHeader nal_unit_header; > + H266RawPictureHeader ph_picture_header; > } H266RawPH; > > typedef struct H266RawSliceHeader { > H266RawNALUnitHeader nal_unit_header; > uint8_t sh_picture_header_in_slice_header_flag; > - H266RawPH sh_picture_header; > + H266RawPictureHeader sh_picture_header; > > uint16_t sh_subpic_id; > uint16_t sh_slice_address; > @@ -770,14 +774,11 @@ typedef struct CodedBitstreamH266Context { > AVBufferRef *vps_ref[VVC_MAX_VPS_COUNT]; > AVBufferRef *sps_ref[VVC_MAX_SPS_COUNT]; > AVBufferRef *pps_ref[VVC_MAX_PPS_COUNT]; > + AVBufferRef *ph_ref; > H266RawVPS *vps[VVC_MAX_SPS_COUNT]; > H266RawSPS *sps[VVC_MAX_SPS_COUNT]; > H266RawPPS *pps[VVC_MAX_PPS_COUNT]; > - > - struct { > - AVBufferRef *ph_ref; > - H266RawPH *ph; > - } priv; > + H266RawPictureHeader *ph; > } CodedBitstreamH266Context; > > #endif /* AVCODEC_CBS_H266_H */ > diff --git a/libavcodec/cbs_h266_syntax_template.c > b/libavcodec/cbs_h266_syntax_template.c > index 06f9f29e08..6d826eba49 100644 > --- a/libavcodec/cbs_h266_syntax_template.c > +++ b/libavcodec/cbs_h266_syntax_template.c > @@ -2231,8 +2231,8 @@ static int FUNC(pred_weight_table) > (CodedBitstreamContext *ctx, RWContext *rw, > return 0; > } > > -static int FUNC(picture_header) (CodedBitstreamContext *ctx, RWContext > *rw, > - H266RawPH *current){ > +static int FUNC(picture_header_structure)(CodedBitstreamContext *ctx, > RWContext *rw, > + H266RawPictureHeader *current) { > CodedBitstreamH266Context *h266 = ctx->priv_data; > const H266RawVPS *vps; > const H266RawSPS *sps; > @@ -2651,7 +2651,7 @@ static int FUNC(ph) (CodedBitstreamContext *ctx, > RWContext *rw, > HEADER("Picture Header"); > > CHECK(FUNC(nal_unit_header) (ctx, rw, ¤t->nal_unit_header, > VVC_PH_NUT)); > - CHECK(FUNC(picture_header) (ctx, rw, current)); > + CHECK(FUNC(picture_header_structure) (ctx, rw, > ¤t->ph_picture_header)); > CHECK(FUNC(rbsp_trailing_bits) (ctx, rw)); > return 0; > } > @@ -2662,7 +2662,7 @@ static int FUNC(slice_header) (CodedBitstreamContext > *ctx, RWContext *rw, > CodedBitstreamH266Context *h266 = ctx->priv_data; > const H266RawSPS *sps; > const H266RawPPS *pps; > - const H266RawPH *ph; > + const H266RawPictureHeader *ph; > const H266RefPicLists *ref_pic_lists; > int err, i; > uint8_t nal_unit_type, qp_bd_offset; > @@ -2675,12 +2675,11 @@ static int FUNC(slice_header) > (CodedBitstreamContext *ctx, RWContext *rw, > > flag(sh_picture_header_in_slice_header_flag); > if (current->sh_picture_header_in_slice_header_flag) { > - CHECK(FUNC(picture_header) (ctx, rw, > ¤t->sh_picture_header)); > + //7.4.8 if sh_picture_header_in_slice_header_flag is true, we do > not have a PH NAL unit > + CHECK(FUNC(picture_header_structure) (ctx, rw, > ¤t->sh_picture_header)); > ph = ¤t->sh_picture_header; > - //7.4.8 if sh_picture_header_in_slice_header_flag is true, we do > not have PH NAL unit > - h266->priv.ph = NULL; > } else { > - ph = h266->priv.ph; > + ph = h266->ph; > Based on the following items in the spec, all slices will have the same picture header. Maybe we can remove sh_picture_header and just keep h266->ph. 1. The PH syntax structure contains information that is common for all slices of the current picture. 2. It is a requirement of bitstream conformance that the value of sh_picture_header_in_slice_header_flag shall be the same in all coded slices in a CLVS. 3. When sh_picture_header_in_slice_header_flag is equal to 1 for a coded slice, it is a requirement of bitstream conformance that no NAL unit with nal_unit_type equal to PH_NUT shall be present in the CLVS. > if (!ph) { > av_log(ctx->log_ctx, AV_LOG_ERROR, > "Picture header not available.\n"); > @@ -2822,7 +2821,7 @@ static int FUNC(slice_header) (CodedBitstreamContext > *ctx, RWContext *rw, > (ctx, rw, sps, pps, ¤t->sh_ref_pic_lists)); > ref_pic_lists = ¤t->sh_ref_pic_lists; > } else { > - ref_pic_lists = &h266->priv.ph->ph_ref_pic_lists; > + ref_pic_lists = &ph->ph_ref_pic_lists; > } > if ((current->sh_slice_type != VVC_SLICE_TYPE_I && > ref_pic_lists->rpl_ref_list[0].num_ref_entries > 1) || > diff --git a/libavcodec/h266_metadata_bsf.c > b/libavcodec/h266_metadata_bsf.c > index 18fa1a5c9c..924eeeaf35 100644 > --- a/libavcodec/h266_metadata_bsf.c > +++ b/libavcodec/h266_metadata_bsf.c > @@ -48,7 +48,7 @@ static int h266_metadata_update_fragment(AVBSFContext > *bsf, AVPacket *pkt, > ff_cbs_delete_unit(pu, 0); > } else if ( pkt && ctx->aud == BSF_ELEMENT_INSERT) { > const H266RawSlice *first_slice = NULL; > - const H266RawPH *ph = NULL; > + const H266RawPictureHeader *picture_header = NULL; > H266RawAUD *aud = &ctx->aud_nal; > int pic_type = 0, temporal_id = 8, layer_id = 0; > for (i = 0; i < pu->nb_units; i++) { > @@ -58,7 +58,8 @@ static int h266_metadata_update_fragment(AVBSFContext > *bsf, AVPacket *pkt, > if (nal->nuh_temporal_id_plus1 < temporal_id + 1) > temporal_id = nal->nuh_temporal_id_plus1 - 1; > if ( nal->nal_unit_type == VVC_PH_NUT ) { > - ph = pu->units[i].content; > + const H266RawPH *ph = pu->units[i].content; > + picture_header = &ph->ph_picture_header; > } else if (IS_H266_SLICE(nal->nal_unit_type)) { > const H266RawSlice *slice = pu->units[i].content; > layer_id = nal->nuh_layer_id; > @@ -72,13 +73,13 @@ static int h266_metadata_update_fragment(AVBSFContext > *bsf, AVPacket *pkt, > first_slice = slice; > if (first_slice->header. > sh_picture_header_in_slice_header_flag) > - ph = &first_slice->header.sh_picture_header; > - else if (!ph) > + picture_header = > &first_slice->header.sh_picture_header; > + else if (!picture_header) > break; > } > } > } > - if (!ph) { > + if (!picture_header) { > av_log(bsf, AV_LOG_ERROR, "no avaliable picture header"); > return AVERROR_INVALIDDATA; > } > @@ -89,7 +90,7 @@ static int h266_metadata_update_fragment(AVBSFContext > *bsf, AVPacket *pkt, > .nuh_temporal_id_plus1 = temporal_id + 1, > }; > aud->aud_pic_type = pic_type; > - aud->aud_irap_or_gdr_flag = ph->ph_gdr_or_irap_pic_flag; > + aud->aud_irap_or_gdr_flag = > picture_header->ph_gdr_or_irap_pic_flag; > > err = ff_cbs_insert_unit_content(pu, 0, VVC_AUD_NUT, aud, NULL); > if (err < 0) { > diff --git a/libavcodec/vvc_parser.c b/libavcodec/vvc_parser.c > index 69696eef57..19ca601bdb 100644 > --- a/libavcodec/vvc_parser.c > +++ b/libavcodec/vvc_parser.c > @@ -31,7 +31,7 @@ > typedef struct PuInfo { > const H266RawPPS *pps; > const H266RawSPS *sps; > - const H266RawPH *ph; > + const H266RawPictureHeader *ph; > const H266RawSlice *slice; > int pic_type; > } PuInfo; > @@ -155,7 +155,6 @@ static void set_parser_ctx(AVCodecParserContext *s, > AVCodecContext *avctx, > }; > const H266RawSPS *sps = pu->sps; > const H266RawPPS *pps = pu->pps; > - //const H266RawPH *ph = pu->ph; > const H266RawNALUnitHeader *nal = &pu->slice->header.nal_unit_header; > > s->pict_type = pu->pic_type; > @@ -205,7 +204,7 @@ static void set_parser_ctx(AVCodecParserContext *s, > AVCodecContext *avctx, > //We follow the VTM. > static void get_slice_poc(VVCParserContext *s, int *poc, > const H266RawSPS *sps, > - const H266RawPH *ph, > + const H266RawPictureHeader *ph, > const H266RawSliceHeader *slice, void *log_ctx) > { > int poc_msb, max_poc_lsb, poc_lsb; > @@ -251,7 +250,7 @@ static int is_au_start(VVCParserContext *s, const > PuInfo *pu, void *log_ctx) > AuDetector *d = &s->au_detector; > const H266RawSPS *sps = pu->sps; > const H266RawNALUnitHeader *nal = &pu->slice->header.nal_unit_header; > - const H266RawPH *ph = pu->ph; > + const H266RawPictureHeader *ph = pu->ph; > const H266RawSlice *slice = pu->slice; > int ret, poc, nut; > > @@ -282,7 +281,8 @@ static int get_pu_info(PuInfo *info, const > CodedBitstreamH266Context *h266, > if (!nal) > continue; > if ( nal->nal_unit_type == VVC_PH_NUT ) { > - info->ph = pu->units[i].content; > + const H266RawPH *ph = pu->units[i].content; > + info->ph = &ph->ph_picture_header; > } else if (IS_H266_SLICE(nal->nal_unit_type)) { > info->slice = pu->units[i].content; > if > (info->slice->header.sh_picture_header_in_slice_header_flag) > -- > 2.41.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 7/1/2023 8:51 AM, Nuo Mi wrote: > On Fri, Jun 30, 2023 at 6:45 AM James Almer<jamrial@gmail.com> wrote: > >> Signed-off-by: James Almer<jamrial@gmail.com> >> --- >> libavcodec/cbs_h2645.c | 35 ++++++++++++++++----------- >> libavcodec/cbs_h266.h | 17 +++++++------ >> libavcodec/cbs_h266_syntax_template.c | 17 ++++++------- >> libavcodec/h266_metadata_bsf.c | 13 +++++----- >> libavcodec/vvc_parser.c | 10 ++++---- >> 5 files changed, 50 insertions(+), 42 deletions(-) >> >> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c >> index cdd7901518..68ccf6a7eb 100644 >> --- a/libavcodec/cbs_h2645.c >> +++ b/libavcodec/cbs_h2645.c >> @@ -525,12 +525,6 @@ static int >> cbs_h2645_split_fragment(CodedBitstreamContext *ctx, >> if (frag->data_size == 0) >> return 0; >> >> - if (codec_id == AV_CODEC_ID_VVC) { >> - //we deactive picture header here to avoid reuse previous au's ph. >> - CodedBitstreamH266Context *h266 = ctx->priv_data; >> - h266->priv.ph = NULL; >> - } >> - >> if (header && frag->data[0] && codec_id == AV_CODEC_ID_H264) { >> // AVCC header. >> size_t size, start, end; >> @@ -793,19 +787,20 @@ cbs_h266_replace_ps(6, SPS, sps, >> sps_seq_parameter_set_id) >> cbs_h266_replace_ps(6, PPS, pps, pps_pic_parameter_set_id) >> >> static int cbs_h266_replace_ph(CodedBitstreamContext *ctx, >> - CodedBitstreamUnit *unit) >> + CodedBitstreamUnit *unit, >> + H266RawPictureHeader *ph) >> { >> CodedBitstreamH266Context *h266 = ctx->priv_data; >> int err; >> >> - h266->priv.ph = NULL; >> err = ff_cbs_make_unit_refcounted(ctx, unit); >> if (err < 0) >> return err; >> - err = av_buffer_replace(&h266->priv.ph_ref, unit->content_ref); >> + av_assert0(unit->content_ref); >> + err = av_buffer_replace(&h266->ph_ref, unit->content_ref); >> if (err < 0) >> return err; >> - h266->priv.ph = (H266RawPH*)h266->priv.ph_ref->data; >> + h266->ph = ph; >> return 0; >> } >> >> @@ -1111,7 +1106,7 @@ static int >> cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, >> err = cbs_h266_read_ph(ctx, &gbc, ph); >> if (err < 0) >> return err; >> - err = cbs_h266_replace_ph(ctx, unit); >> + err = cbs_h266_replace_ph(ctx, unit, &ph->ph_picture_header); >> if (err < 0) >> return err; >> } >> @@ -1139,6 +1134,12 @@ static int >> cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, >> pos = get_bits_count(&gbc); >> len = unit->data_size; >> >> + if (slice->header.sh_picture_header_in_slice_header_flag) { >> + err = cbs_h266_replace_ph(ctx, unit, >> &slice->header.sh_picture_header); >> + if (err < 0) >> + return err; >> + } >> + >> slice->data_size = len - pos / 8; >> slice->data_ref = av_buffer_ref(unit->data_ref); >> if (!slice->data_ref) >> @@ -1640,7 +1641,7 @@ static int >> cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h266_replace_ph(ctx, unit); >> + err = cbs_h266_replace_ph(ctx, unit, &ph->ph_picture_header); >> if (err < 0) >> return err; >> } >> @@ -1661,6 +1662,12 @@ static int >> cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> + if (slice->header.sh_picture_header_in_slice_header_flag) { >> + err = cbs_h266_replace_ph(ctx, unit, >> &slice->header.sh_picture_header); >> + if (err < 0) >> + return err; >> + } >> + >> if (slice->data) { >> err = cbs_h2645_write_slice_data(ctx, pbc, slice->data, >> slice->data_size, >> @@ -1884,8 +1891,8 @@ static void cbs_h266_flush(CodedBitstreamContext >> *ctx) >> av_buffer_unref(&h266->pps_ref[i]); >> h266->pps[i] = NULL; >> } >> - av_buffer_unref(&h266->priv.ph_ref); >> - h266->priv.ph = NULL; >> + av_buffer_unref(&h266->ph_ref); >> + h266->ph = NULL; >> } >> >> static void cbs_h266_close(CodedBitstreamContext *ctx) >> diff --git a/libavcodec/cbs_h266.h b/libavcodec/cbs_h266.h >> index 03dfd4a954..54590748c3 100644 >> --- a/libavcodec/cbs_h266.h >> +++ b/libavcodec/cbs_h266.h >> @@ -581,8 +581,7 @@ typedef struct H266RawPredWeightTable { >> int16_t delta_chroma_offset_l1[15][2]; >> } H266RawPredWeightTable; >> >> -typedef struct H266RawPH { >> - H266RawNALUnitHeader nal_unit_header; >> +typedef struct H266RawPictureHeader { >> uint8_t ph_gdr_or_irap_pic_flag; >> uint8_t ph_non_ref_pic_flag; >> uint8_t ph_gdr_pic_flag; >> @@ -670,12 +669,17 @@ typedef struct H266RawPH { >> >> uint8_t ph_extension_length; >> uint8_t ph_extension_data_byte[256]; >> +} H266RawPictureHeader; >> + >> +typedef struct H266RawPH { >> + H266RawNALUnitHeader nal_unit_header; >> + H266RawPictureHeader ph_picture_header; >> } H266RawPH; >> >> typedef struct H266RawSliceHeader { >> H266RawNALUnitHeader nal_unit_header; >> uint8_t sh_picture_header_in_slice_header_flag; >> - H266RawPH sh_picture_header; >> + H266RawPictureHeader sh_picture_header; >> >> uint16_t sh_subpic_id; >> uint16_t sh_slice_address; >> @@ -770,14 +774,11 @@ typedef struct CodedBitstreamH266Context { >> AVBufferRef *vps_ref[VVC_MAX_VPS_COUNT]; >> AVBufferRef *sps_ref[VVC_MAX_SPS_COUNT]; >> AVBufferRef *pps_ref[VVC_MAX_PPS_COUNT]; >> + AVBufferRef *ph_ref; >> H266RawVPS *vps[VVC_MAX_SPS_COUNT]; >> H266RawSPS *sps[VVC_MAX_SPS_COUNT]; >> H266RawPPS *pps[VVC_MAX_PPS_COUNT]; >> - >> - struct { >> - AVBufferRef *ph_ref; >> - H266RawPH *ph; >> - } priv; >> + H266RawPictureHeader *ph; >> } CodedBitstreamH266Context; >> >> #endif /* AVCODEC_CBS_H266_H */ >> diff --git a/libavcodec/cbs_h266_syntax_template.c >> b/libavcodec/cbs_h266_syntax_template.c >> index 06f9f29e08..6d826eba49 100644 >> --- a/libavcodec/cbs_h266_syntax_template.c >> +++ b/libavcodec/cbs_h266_syntax_template.c >> @@ -2231,8 +2231,8 @@ static int FUNC(pred_weight_table) >> (CodedBitstreamContext *ctx, RWContext *rw, >> return 0; >> } >> >> -static int FUNC(picture_header) (CodedBitstreamContext *ctx, RWContext >> *rw, >> - H266RawPH *current){ >> +static int FUNC(picture_header_structure)(CodedBitstreamContext *ctx, >> RWContext *rw, >> + H266RawPictureHeader *current) { >> CodedBitstreamH266Context *h266 = ctx->priv_data; >> const H266RawVPS *vps; >> const H266RawSPS *sps; >> @@ -2651,7 +2651,7 @@ static int FUNC(ph) (CodedBitstreamContext *ctx, >> RWContext *rw, >> HEADER("Picture Header"); >> >> CHECK(FUNC(nal_unit_header) (ctx, rw, ¤t->nal_unit_header, >> VVC_PH_NUT)); >> - CHECK(FUNC(picture_header) (ctx, rw, current)); >> + CHECK(FUNC(picture_header_structure) (ctx, rw, >> ¤t->ph_picture_header)); >> CHECK(FUNC(rbsp_trailing_bits) (ctx, rw)); >> return 0; >> } >> @@ -2662,7 +2662,7 @@ static int FUNC(slice_header) (CodedBitstreamContext >> *ctx, RWContext *rw, >> CodedBitstreamH266Context *h266 = ctx->priv_data; >> const H266RawSPS *sps; >> const H266RawPPS *pps; >> - const H266RawPH *ph; >> + const H266RawPictureHeader *ph; >> const H266RefPicLists *ref_pic_lists; >> int err, i; >> uint8_t nal_unit_type, qp_bd_offset; >> @@ -2675,12 +2675,11 @@ static int FUNC(slice_header) >> (CodedBitstreamContext *ctx, RWContext *rw, >> >> flag(sh_picture_header_in_slice_header_flag); >> if (current->sh_picture_header_in_slice_header_flag) { >> - CHECK(FUNC(picture_header) (ctx, rw, >> ¤t->sh_picture_header)); >> + //7.4.8 if sh_picture_header_in_slice_header_flag is true, we do >> not have a PH NAL unit >> + CHECK(FUNC(picture_header_structure) (ctx, rw, >> ¤t->sh_picture_header)); >> ph = ¤t->sh_picture_header; >> - //7.4.8 if sh_picture_header_in_slice_header_flag is true, we do >> not have PH NAL unit >> - h266->priv.ph = NULL; >> } else { >> - ph = h266->priv.ph; >> + ph = h266->ph; >> > Based on the following items in the spec, all slices will have the same > picture header. Maybe we can remove sh_picture_header and just keep > h266->ph. > > 1. The PH syntax structure contains information that is common for all > slices of the current picture. > 2. It is a requirement of bitstream conformance that the value of > sh_picture_header_in_slice_header_flag shall be the same in all coded > slices in a CLVS. > 3. When sh_picture_header_in_slice_header_flag is equal to 1 for a coded > slice, it is a requirement of bitstream conformance that no NAL unit with > nal_unit_type equal to PH_NUT shall be present in the CLVS. CodedBitstreamH266Context holds the state of an hypothetical decoder after the last unit fed to CBS was parsed. If you were to feed it two PUs in a row, h266->ph will be a pointer to the H266RawPictureHeader relevant to the second PU (Either picture_header from the last PH NALU, or picture_header from the last Slice NALU). We can't store values read from the bitstream there, since if they are overwritten, then they will be unavailable to callers. It's only meant to store pointers and references to values and structs stored in units within the fragment, or derived values.
On Sun, Jul 2, 2023 at 7:32 AM James Almer <jamrial@gmail.com> wrote: > On 7/1/2023 8:51 AM, Nuo Mi wrote: > > On Fri, Jun 30, 2023 at 6:45 AM James Almer<jamrial@gmail.com> wrote: > > > >> Signed-off-by: James Almer<jamrial@gmail.com> > >> --- > >> libavcodec/cbs_h2645.c | 35 ++++++++++++++++----------- > >> libavcodec/cbs_h266.h | 17 +++++++------ > >> libavcodec/cbs_h266_syntax_template.c | 17 ++++++------- > >> libavcodec/h266_metadata_bsf.c | 13 +++++----- > >> libavcodec/vvc_parser.c | 10 ++++---- > >> 5 files changed, 50 insertions(+), 42 deletions(-) > >> > >> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c > >> index cdd7901518..68ccf6a7eb 100644 > >> --- a/libavcodec/cbs_h2645.c > >> +++ b/libavcodec/cbs_h2645.c > >> @@ -525,12 +525,6 @@ static int > >> cbs_h2645_split_fragment(CodedBitstreamContext *ctx, > >> if (frag->data_size == 0) > >> return 0; > >> > >> - if (codec_id == AV_CODEC_ID_VVC) { > >> - //we deactive picture header here to avoid reuse previous au's > ph. > >> - CodedBitstreamH266Context *h266 = ctx->priv_data; > >> - h266->priv.ph = NULL; > >> - } > >> - > >> if (header && frag->data[0] && codec_id == AV_CODEC_ID_H264) { > >> // AVCC header. > >> size_t size, start, end; > >> @@ -793,19 +787,20 @@ cbs_h266_replace_ps(6, SPS, sps, > >> sps_seq_parameter_set_id) > >> cbs_h266_replace_ps(6, PPS, pps, pps_pic_parameter_set_id) > >> > >> static int cbs_h266_replace_ph(CodedBitstreamContext *ctx, > >> - CodedBitstreamUnit *unit) > >> + CodedBitstreamUnit *unit, > >> + H266RawPictureHeader *ph) > >> { > >> CodedBitstreamH266Context *h266 = ctx->priv_data; > >> int err; > >> > >> - h266->priv.ph = NULL; > >> err = ff_cbs_make_unit_refcounted(ctx, unit); > >> if (err < 0) > >> return err; > >> - err = av_buffer_replace(&h266->priv.ph_ref, unit->content_ref); > >> + av_assert0(unit->content_ref); > >> + err = av_buffer_replace(&h266->ph_ref, unit->content_ref); > >> if (err < 0) > >> return err; > >> - h266->priv.ph = (H266RawPH*)h266->priv.ph_ref->data; > >> + h266->ph = ph; > >> return 0; > >> } > >> > >> @@ -1111,7 +1106,7 @@ static int > >> cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, > >> err = cbs_h266_read_ph(ctx, &gbc, ph); > >> if (err < 0) > >> return err; > >> - err = cbs_h266_replace_ph(ctx, unit); > >> + err = cbs_h266_replace_ph(ctx, unit, > &ph->ph_picture_header); > >> if (err < 0) > >> return err; > >> } > >> @@ -1139,6 +1134,12 @@ static int > >> cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, > >> pos = get_bits_count(&gbc); > >> len = unit->data_size; > >> > >> + if (slice->header.sh_picture_header_in_slice_header_flag) { > >> + err = cbs_h266_replace_ph(ctx, unit, > >> &slice->header.sh_picture_header); > >> + if (err < 0) > >> + return err; > >> + } > >> + > >> slice->data_size = len - pos / 8; > >> slice->data_ref = av_buffer_ref(unit->data_ref); > >> if (!slice->data_ref) > >> @@ -1640,7 +1641,7 @@ static int > >> cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, > >> if (err < 0) > >> return err; > >> > >> - err = cbs_h266_replace_ph(ctx, unit); > >> + err = cbs_h266_replace_ph(ctx, unit, > &ph->ph_picture_header); > >> if (err < 0) > >> return err; > >> } > >> @@ -1661,6 +1662,12 @@ static int > >> cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, > >> if (err < 0) > >> return err; > >> > >> + if (slice->header.sh_picture_header_in_slice_header_flag) { > >> + err = cbs_h266_replace_ph(ctx, unit, > >> &slice->header.sh_picture_header); > >> + if (err < 0) > >> + return err; > >> + } > >> + > >> if (slice->data) { > >> err = cbs_h2645_write_slice_data(ctx, pbc, > slice->data, > >> slice->data_size, > >> @@ -1884,8 +1891,8 @@ static void cbs_h266_flush(CodedBitstreamContext > >> *ctx) > >> av_buffer_unref(&h266->pps_ref[i]); > >> h266->pps[i] = NULL; > >> } > >> - av_buffer_unref(&h266->priv.ph_ref); > >> - h266->priv.ph = NULL; > >> + av_buffer_unref(&h266->ph_ref); > >> + h266->ph = NULL; > >> } > >> > >> static void cbs_h266_close(CodedBitstreamContext *ctx) > >> diff --git a/libavcodec/cbs_h266.h b/libavcodec/cbs_h266.h > >> index 03dfd4a954..54590748c3 100644 > >> --- a/libavcodec/cbs_h266.h > >> +++ b/libavcodec/cbs_h266.h > >> @@ -581,8 +581,7 @@ typedef struct H266RawPredWeightTable { > >> int16_t delta_chroma_offset_l1[15][2]; > >> } H266RawPredWeightTable; > >> > >> -typedef struct H266RawPH { > >> - H266RawNALUnitHeader nal_unit_header; > >> +typedef struct H266RawPictureHeader { > >> uint8_t ph_gdr_or_irap_pic_flag; > >> uint8_t ph_non_ref_pic_flag; > >> uint8_t ph_gdr_pic_flag; > >> @@ -670,12 +669,17 @@ typedef struct H266RawPH { > >> > >> uint8_t ph_extension_length; > >> uint8_t ph_extension_data_byte[256]; > >> +} H266RawPictureHeader; > >> + > >> +typedef struct H266RawPH { > >> + H266RawNALUnitHeader nal_unit_header; > >> + H266RawPictureHeader ph_picture_header; > >> } H266RawPH; > >> > >> typedef struct H266RawSliceHeader { > >> H266RawNALUnitHeader nal_unit_header; > >> uint8_t sh_picture_header_in_slice_header_flag; > >> - H266RawPH sh_picture_header; > >> + H266RawPictureHeader sh_picture_header; > >> > >> uint16_t sh_subpic_id; > >> uint16_t sh_slice_address; > >> @@ -770,14 +774,11 @@ typedef struct CodedBitstreamH266Context { > >> AVBufferRef *vps_ref[VVC_MAX_VPS_COUNT]; > >> AVBufferRef *sps_ref[VVC_MAX_SPS_COUNT]; > >> AVBufferRef *pps_ref[VVC_MAX_PPS_COUNT]; > >> + AVBufferRef *ph_ref; > >> H266RawVPS *vps[VVC_MAX_SPS_COUNT]; > >> H266RawSPS *sps[VVC_MAX_SPS_COUNT]; > >> H266RawPPS *pps[VVC_MAX_PPS_COUNT]; > >> - > >> - struct { > >> - AVBufferRef *ph_ref; > >> - H266RawPH *ph; > >> - } priv; > >> + H266RawPictureHeader *ph; > >> } CodedBitstreamH266Context; > >> > >> #endif /* AVCODEC_CBS_H266_H */ > >> diff --git a/libavcodec/cbs_h266_syntax_template.c > >> b/libavcodec/cbs_h266_syntax_template.c > >> index 06f9f29e08..6d826eba49 100644 > >> --- a/libavcodec/cbs_h266_syntax_template.c > >> +++ b/libavcodec/cbs_h266_syntax_template.c > >> @@ -2231,8 +2231,8 @@ static int FUNC(pred_weight_table) > >> (CodedBitstreamContext *ctx, RWContext *rw, > >> return 0; > >> } > >> > >> -static int FUNC(picture_header) (CodedBitstreamContext *ctx, RWContext > >> *rw, > >> - H266RawPH *current){ > >> +static int FUNC(picture_header_structure)(CodedBitstreamContext *ctx, > >> RWContext *rw, > >> + H266RawPictureHeader > *current) { > >> CodedBitstreamH266Context *h266 = ctx->priv_data; > >> const H266RawVPS *vps; > >> const H266RawSPS *sps; > >> @@ -2651,7 +2651,7 @@ static int FUNC(ph) (CodedBitstreamContext *ctx, > >> RWContext *rw, > >> HEADER("Picture Header"); > >> > >> CHECK(FUNC(nal_unit_header) (ctx, rw, ¤t->nal_unit_header, > >> VVC_PH_NUT)); > >> - CHECK(FUNC(picture_header) (ctx, rw, current)); > >> + CHECK(FUNC(picture_header_structure) (ctx, rw, > >> ¤t->ph_picture_header)); > >> CHECK(FUNC(rbsp_trailing_bits) (ctx, rw)); > >> return 0; > >> } > >> @@ -2662,7 +2662,7 @@ static int FUNC(slice_header) > (CodedBitstreamContext > >> *ctx, RWContext *rw, > >> CodedBitstreamH266Context *h266 = ctx->priv_data; > >> const H266RawSPS *sps; > >> const H266RawPPS *pps; > >> - const H266RawPH *ph; > >> + const H266RawPictureHeader *ph; > >> const H266RefPicLists *ref_pic_lists; > >> int err, i; > >> uint8_t nal_unit_type, qp_bd_offset; > >> @@ -2675,12 +2675,11 @@ static int FUNC(slice_header) > >> (CodedBitstreamContext *ctx, RWContext *rw, > >> > >> flag(sh_picture_header_in_slice_header_flag); > >> if (current->sh_picture_header_in_slice_header_flag) { > >> - CHECK(FUNC(picture_header) (ctx, rw, > >> ¤t->sh_picture_header)); > >> + //7.4.8 if sh_picture_header_in_slice_header_flag is true, we > do > >> not have a PH NAL unit > >> + CHECK(FUNC(picture_header_structure) (ctx, rw, > >> ¤t->sh_picture_header)); > >> ph = ¤t->sh_picture_header; > >> - //7.4.8 if sh_picture_header_in_slice_header_flag is true, we > do > >> not have PH NAL unit > >> - h266->priv.ph = NULL; > >> } else { > >> - ph = h266->priv.ph; > >> + ph = h266->ph; > >> > > Based on the following items in the spec, all slices will have the same > > picture header. Maybe we can remove sh_picture_header and just keep > > h266->ph. > > > > 1. The PH syntax structure contains information that is common for all > > slices of the current picture. > > 2. It is a requirement of bitstream conformance that the value of > > sh_picture_header_in_slice_header_flag shall be the same in all coded > > slices in a CLVS. > > 3. When sh_picture_header_in_slice_header_flag is equal to 1 for a coded > > slice, it is a requirement of bitstream conformance that no NAL unit with > > nal_unit_type equal to PH_NUT shall be present in the CLVS. > > CodedBitstreamH266Context holds the state of an hypothetical decoder > after the last unit fed to CBS was parsed. If you were to feed it two > PUs in a row, h266->ph will be a pointer to the H266RawPictureHeader > relevant to the second PU (Either picture_header from the last PH NALU, > or picture_header from the last Slice NALU). > > We can't store values read from the bitstream there, since if they are > overwritten, then they will be unavailable to callers. It's only meant > to store pointers and references to values and structs stored in units > within the fragment, or derived values. > All slices in one picture will have the same picture header content. if sh_picture_header_in_slice_header_flag == 0, we only have one PH NAL. so we will not overwrite. if sh_picture_header_in_slice_header_flag == 1, we only need to parse the PH in the first slice, then use memcpy to do a sanity check for later slices. BTW, do we need constify the raw vps, sps, pps and ph in CodedBitstreamH266Context
On 7/1/2023 10:06 PM, Nuo Mi wrote: > On Sun, Jul 2, 2023 at 7:32 AM James Almer <jamrial@gmail.com> wrote: > >> On 7/1/2023 8:51 AM, Nuo Mi wrote: >>> On Fri, Jun 30, 2023 at 6:45 AM James Almer<jamrial@gmail.com> wrote: >>> >>>> Signed-off-by: James Almer<jamrial@gmail.com> >>>> --- >>>> libavcodec/cbs_h2645.c | 35 ++++++++++++++++----------- >>>> libavcodec/cbs_h266.h | 17 +++++++------ >>>> libavcodec/cbs_h266_syntax_template.c | 17 ++++++------- >>>> libavcodec/h266_metadata_bsf.c | 13 +++++----- >>>> libavcodec/vvc_parser.c | 10 ++++---- >>>> 5 files changed, 50 insertions(+), 42 deletions(-) >>>> >>>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c >>>> index cdd7901518..68ccf6a7eb 100644 >>>> --- a/libavcodec/cbs_h2645.c >>>> +++ b/libavcodec/cbs_h2645.c >>>> @@ -525,12 +525,6 @@ static int >>>> cbs_h2645_split_fragment(CodedBitstreamContext *ctx, >>>> if (frag->data_size == 0) >>>> return 0; >>>> >>>> - if (codec_id == AV_CODEC_ID_VVC) { >>>> - //we deactive picture header here to avoid reuse previous au's >> ph. >>>> - CodedBitstreamH266Context *h266 = ctx->priv_data; >>>> - h266->priv.ph = NULL; >>>> - } >>>> - >>>> if (header && frag->data[0] && codec_id == AV_CODEC_ID_H264) { >>>> // AVCC header. >>>> size_t size, start, end; >>>> @@ -793,19 +787,20 @@ cbs_h266_replace_ps(6, SPS, sps, >>>> sps_seq_parameter_set_id) >>>> cbs_h266_replace_ps(6, PPS, pps, pps_pic_parameter_set_id) >>>> >>>> static int cbs_h266_replace_ph(CodedBitstreamContext *ctx, >>>> - CodedBitstreamUnit *unit) >>>> + CodedBitstreamUnit *unit, >>>> + H266RawPictureHeader *ph) >>>> { >>>> CodedBitstreamH266Context *h266 = ctx->priv_data; >>>> int err; >>>> >>>> - h266->priv.ph = NULL; >>>> err = ff_cbs_make_unit_refcounted(ctx, unit); >>>> if (err < 0) >>>> return err; >>>> - err = av_buffer_replace(&h266->priv.ph_ref, unit->content_ref); >>>> + av_assert0(unit->content_ref); >>>> + err = av_buffer_replace(&h266->ph_ref, unit->content_ref); >>>> if (err < 0) >>>> return err; >>>> - h266->priv.ph = (H266RawPH*)h266->priv.ph_ref->data; >>>> + h266->ph = ph; >>>> return 0; >>>> } >>>> >>>> @@ -1111,7 +1106,7 @@ static int >>>> cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, >>>> err = cbs_h266_read_ph(ctx, &gbc, ph); >>>> if (err < 0) >>>> return err; >>>> - err = cbs_h266_replace_ph(ctx, unit); >>>> + err = cbs_h266_replace_ph(ctx, unit, >> &ph->ph_picture_header); >>>> if (err < 0) >>>> return err; >>>> } >>>> @@ -1139,6 +1134,12 @@ static int >>>> cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, >>>> pos = get_bits_count(&gbc); >>>> len = unit->data_size; >>>> >>>> + if (slice->header.sh_picture_header_in_slice_header_flag) { >>>> + err = cbs_h266_replace_ph(ctx, unit, >>>> &slice->header.sh_picture_header); >>>> + if (err < 0) >>>> + return err; >>>> + } >>>> + >>>> slice->data_size = len - pos / 8; >>>> slice->data_ref = av_buffer_ref(unit->data_ref); >>>> if (!slice->data_ref) >>>> @@ -1640,7 +1641,7 @@ static int >>>> cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, >>>> if (err < 0) >>>> return err; >>>> >>>> - err = cbs_h266_replace_ph(ctx, unit); >>>> + err = cbs_h266_replace_ph(ctx, unit, >> &ph->ph_picture_header); >>>> if (err < 0) >>>> return err; >>>> } >>>> @@ -1661,6 +1662,12 @@ static int >>>> cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, >>>> if (err < 0) >>>> return err; >>>> >>>> + if (slice->header.sh_picture_header_in_slice_header_flag) { >>>> + err = cbs_h266_replace_ph(ctx, unit, >>>> &slice->header.sh_picture_header); >>>> + if (err < 0) >>>> + return err; >>>> + } >>>> + >>>> if (slice->data) { >>>> err = cbs_h2645_write_slice_data(ctx, pbc, >> slice->data, >>>> slice->data_size, >>>> @@ -1884,8 +1891,8 @@ static void cbs_h266_flush(CodedBitstreamContext >>>> *ctx) >>>> av_buffer_unref(&h266->pps_ref[i]); >>>> h266->pps[i] = NULL; >>>> } >>>> - av_buffer_unref(&h266->priv.ph_ref); >>>> - h266->priv.ph = NULL; >>>> + av_buffer_unref(&h266->ph_ref); >>>> + h266->ph = NULL; >>>> } >>>> >>>> static void cbs_h266_close(CodedBitstreamContext *ctx) >>>> diff --git a/libavcodec/cbs_h266.h b/libavcodec/cbs_h266.h >>>> index 03dfd4a954..54590748c3 100644 >>>> --- a/libavcodec/cbs_h266.h >>>> +++ b/libavcodec/cbs_h266.h >>>> @@ -581,8 +581,7 @@ typedef struct H266RawPredWeightTable { >>>> int16_t delta_chroma_offset_l1[15][2]; >>>> } H266RawPredWeightTable; >>>> >>>> -typedef struct H266RawPH { >>>> - H266RawNALUnitHeader nal_unit_header; >>>> +typedef struct H266RawPictureHeader { >>>> uint8_t ph_gdr_or_irap_pic_flag; >>>> uint8_t ph_non_ref_pic_flag; >>>> uint8_t ph_gdr_pic_flag; >>>> @@ -670,12 +669,17 @@ typedef struct H266RawPH { >>>> >>>> uint8_t ph_extension_length; >>>> uint8_t ph_extension_data_byte[256]; >>>> +} H266RawPictureHeader; >>>> + >>>> +typedef struct H266RawPH { >>>> + H266RawNALUnitHeader nal_unit_header; >>>> + H266RawPictureHeader ph_picture_header; >>>> } H266RawPH; >>>> >>>> typedef struct H266RawSliceHeader { >>>> H266RawNALUnitHeader nal_unit_header; >>>> uint8_t sh_picture_header_in_slice_header_flag; >>>> - H266RawPH sh_picture_header; >>>> + H266RawPictureHeader sh_picture_header; >>>> >>>> uint16_t sh_subpic_id; >>>> uint16_t sh_slice_address; >>>> @@ -770,14 +774,11 @@ typedef struct CodedBitstreamH266Context { >>>> AVBufferRef *vps_ref[VVC_MAX_VPS_COUNT]; >>>> AVBufferRef *sps_ref[VVC_MAX_SPS_COUNT]; >>>> AVBufferRef *pps_ref[VVC_MAX_PPS_COUNT]; >>>> + AVBufferRef *ph_ref; >>>> H266RawVPS *vps[VVC_MAX_SPS_COUNT]; >>>> H266RawSPS *sps[VVC_MAX_SPS_COUNT]; >>>> H266RawPPS *pps[VVC_MAX_PPS_COUNT]; >>>> - >>>> - struct { >>>> - AVBufferRef *ph_ref; >>>> - H266RawPH *ph; >>>> - } priv; >>>> + H266RawPictureHeader *ph; >>>> } CodedBitstreamH266Context; >>>> >>>> #endif /* AVCODEC_CBS_H266_H */ >>>> diff --git a/libavcodec/cbs_h266_syntax_template.c >>>> b/libavcodec/cbs_h266_syntax_template.c >>>> index 06f9f29e08..6d826eba49 100644 >>>> --- a/libavcodec/cbs_h266_syntax_template.c >>>> +++ b/libavcodec/cbs_h266_syntax_template.c >>>> @@ -2231,8 +2231,8 @@ static int FUNC(pred_weight_table) >>>> (CodedBitstreamContext *ctx, RWContext *rw, >>>> return 0; >>>> } >>>> >>>> -static int FUNC(picture_header) (CodedBitstreamContext *ctx, RWContext >>>> *rw, >>>> - H266RawPH *current){ >>>> +static int FUNC(picture_header_structure)(CodedBitstreamContext *ctx, >>>> RWContext *rw, >>>> + H266RawPictureHeader >> *current) { >>>> CodedBitstreamH266Context *h266 = ctx->priv_data; >>>> const H266RawVPS *vps; >>>> const H266RawSPS *sps; >>>> @@ -2651,7 +2651,7 @@ static int FUNC(ph) (CodedBitstreamContext *ctx, >>>> RWContext *rw, >>>> HEADER("Picture Header"); >>>> >>>> CHECK(FUNC(nal_unit_header) (ctx, rw, ¤t->nal_unit_header, >>>> VVC_PH_NUT)); >>>> - CHECK(FUNC(picture_header) (ctx, rw, current)); >>>> + CHECK(FUNC(picture_header_structure) (ctx, rw, >>>> ¤t->ph_picture_header)); >>>> CHECK(FUNC(rbsp_trailing_bits) (ctx, rw)); >>>> return 0; >>>> } >>>> @@ -2662,7 +2662,7 @@ static int FUNC(slice_header) >> (CodedBitstreamContext >>>> *ctx, RWContext *rw, >>>> CodedBitstreamH266Context *h266 = ctx->priv_data; >>>> const H266RawSPS *sps; >>>> const H266RawPPS *pps; >>>> - const H266RawPH *ph; >>>> + const H266RawPictureHeader *ph; >>>> const H266RefPicLists *ref_pic_lists; >>>> int err, i; >>>> uint8_t nal_unit_type, qp_bd_offset; >>>> @@ -2675,12 +2675,11 @@ static int FUNC(slice_header) >>>> (CodedBitstreamContext *ctx, RWContext *rw, >>>> >>>> flag(sh_picture_header_in_slice_header_flag); >>>> if (current->sh_picture_header_in_slice_header_flag) { >>>> - CHECK(FUNC(picture_header) (ctx, rw, >>>> ¤t->sh_picture_header)); >>>> + //7.4.8 if sh_picture_header_in_slice_header_flag is true, we >> do >>>> not have a PH NAL unit >>>> + CHECK(FUNC(picture_header_structure) (ctx, rw, >>>> ¤t->sh_picture_header)); >>>> ph = ¤t->sh_picture_header; >>>> - //7.4.8 if sh_picture_header_in_slice_header_flag is true, we >> do >>>> not have PH NAL unit >>>> - h266->priv.ph = NULL; >>>> } else { >>>> - ph = h266->priv.ph; >>>> + ph = h266->ph; >>>> >>> Based on the following items in the spec, all slices will have the same >>> picture header. Maybe we can remove sh_picture_header and just keep >>> h266->ph. >>> >>> 1. The PH syntax structure contains information that is common for all >>> slices of the current picture. >>> 2. It is a requirement of bitstream conformance that the value of >>> sh_picture_header_in_slice_header_flag shall be the same in all coded >>> slices in a CLVS. >>> 3. When sh_picture_header_in_slice_header_flag is equal to 1 for a coded >>> slice, it is a requirement of bitstream conformance that no NAL unit with >>> nal_unit_type equal to PH_NUT shall be present in the CLVS. >> >> CodedBitstreamH266Context holds the state of an hypothetical decoder >> after the last unit fed to CBS was parsed. If you were to feed it two >> PUs in a row, h266->ph will be a pointer to the H266RawPictureHeader >> relevant to the second PU (Either picture_header from the last PH NALU, >> or picture_header from the last Slice NALU). >> >> We can't store values read from the bitstream there, since if they are >> overwritten, then they will be unavailable to callers. It's only meant >> to store pointers and references to values and structs stored in units >> within the fragment, or derived values. >> > All slices in one picture will have the same picture header content. > if sh_picture_header_in_slice_header_flag == 0, we only have one PH NAL. > so we will not overwrite. > if sh_picture_header_in_slice_header_flag == 1, we only need to parse the > PH in the first slice, then use memcpy to do a sanity check for later > slices. We need to parse the entire slice header in all cases, picture header included if present, to advance the getbitcontext and reach the following fields. > > BTW, do we need constify the raw vps, sps, pps and ph > in CodedBitstreamH266Context Yeah, probably a good idea.
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c index cdd7901518..68ccf6a7eb 100644 --- a/libavcodec/cbs_h2645.c +++ b/libavcodec/cbs_h2645.c @@ -525,12 +525,6 @@ static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx, if (frag->data_size == 0) return 0; - if (codec_id == AV_CODEC_ID_VVC) { - //we deactive picture header here to avoid reuse previous au's ph. - CodedBitstreamH266Context *h266 = ctx->priv_data; - h266->priv.ph = NULL; - } - if (header && frag->data[0] && codec_id == AV_CODEC_ID_H264) { // AVCC header. size_t size, start, end; @@ -793,19 +787,20 @@ cbs_h266_replace_ps(6, SPS, sps, sps_seq_parameter_set_id) cbs_h266_replace_ps(6, PPS, pps, pps_pic_parameter_set_id) static int cbs_h266_replace_ph(CodedBitstreamContext *ctx, - CodedBitstreamUnit *unit) + CodedBitstreamUnit *unit, + H266RawPictureHeader *ph) { CodedBitstreamH266Context *h266 = ctx->priv_data; int err; - h266->priv.ph = NULL; err = ff_cbs_make_unit_refcounted(ctx, unit); if (err < 0) return err; - err = av_buffer_replace(&h266->priv.ph_ref, unit->content_ref); + av_assert0(unit->content_ref); + err = av_buffer_replace(&h266->ph_ref, unit->content_ref); if (err < 0) return err; - h266->priv.ph = (H266RawPH*)h266->priv.ph_ref->data; + h266->ph = ph; return 0; } @@ -1111,7 +1106,7 @@ static int cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, err = cbs_h266_read_ph(ctx, &gbc, ph); if (err < 0) return err; - err = cbs_h266_replace_ph(ctx, unit); + err = cbs_h266_replace_ph(ctx, unit, &ph->ph_picture_header); if (err < 0) return err; } @@ -1139,6 +1134,12 @@ static int cbs_h266_read_nal_unit(CodedBitstreamContext *ctx, pos = get_bits_count(&gbc); len = unit->data_size; + if (slice->header.sh_picture_header_in_slice_header_flag) { + err = cbs_h266_replace_ph(ctx, unit, &slice->header.sh_picture_header); + if (err < 0) + return err; + } + slice->data_size = len - pos / 8; slice->data_ref = av_buffer_ref(unit->data_ref); if (!slice->data_ref) @@ -1640,7 +1641,7 @@ static int cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h266_replace_ph(ctx, unit); + err = cbs_h266_replace_ph(ctx, unit, &ph->ph_picture_header); if (err < 0) return err; } @@ -1661,6 +1662,12 @@ static int cbs_h266_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; + if (slice->header.sh_picture_header_in_slice_header_flag) { + err = cbs_h266_replace_ph(ctx, unit, &slice->header.sh_picture_header); + if (err < 0) + return err; + } + if (slice->data) { err = cbs_h2645_write_slice_data(ctx, pbc, slice->data, slice->data_size, @@ -1884,8 +1891,8 @@ static void cbs_h266_flush(CodedBitstreamContext *ctx) av_buffer_unref(&h266->pps_ref[i]); h266->pps[i] = NULL; } - av_buffer_unref(&h266->priv.ph_ref); - h266->priv.ph = NULL; + av_buffer_unref(&h266->ph_ref); + h266->ph = NULL; } static void cbs_h266_close(CodedBitstreamContext *ctx) diff --git a/libavcodec/cbs_h266.h b/libavcodec/cbs_h266.h index 03dfd4a954..54590748c3 100644 --- a/libavcodec/cbs_h266.h +++ b/libavcodec/cbs_h266.h @@ -581,8 +581,7 @@ typedef struct H266RawPredWeightTable { int16_t delta_chroma_offset_l1[15][2]; } H266RawPredWeightTable; -typedef struct H266RawPH { - H266RawNALUnitHeader nal_unit_header; +typedef struct H266RawPictureHeader { uint8_t ph_gdr_or_irap_pic_flag; uint8_t ph_non_ref_pic_flag; uint8_t ph_gdr_pic_flag; @@ -670,12 +669,17 @@ typedef struct H266RawPH { uint8_t ph_extension_length; uint8_t ph_extension_data_byte[256]; +} H266RawPictureHeader; + +typedef struct H266RawPH { + H266RawNALUnitHeader nal_unit_header; + H266RawPictureHeader ph_picture_header; } H266RawPH; typedef struct H266RawSliceHeader { H266RawNALUnitHeader nal_unit_header; uint8_t sh_picture_header_in_slice_header_flag; - H266RawPH sh_picture_header; + H266RawPictureHeader sh_picture_header; uint16_t sh_subpic_id; uint16_t sh_slice_address; @@ -770,14 +774,11 @@ typedef struct CodedBitstreamH266Context { AVBufferRef *vps_ref[VVC_MAX_VPS_COUNT]; AVBufferRef *sps_ref[VVC_MAX_SPS_COUNT]; AVBufferRef *pps_ref[VVC_MAX_PPS_COUNT]; + AVBufferRef *ph_ref; H266RawVPS *vps[VVC_MAX_SPS_COUNT]; H266RawSPS *sps[VVC_MAX_SPS_COUNT]; H266RawPPS *pps[VVC_MAX_PPS_COUNT]; - - struct { - AVBufferRef *ph_ref; - H266RawPH *ph; - } priv; + H266RawPictureHeader *ph; } CodedBitstreamH266Context; #endif /* AVCODEC_CBS_H266_H */ diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c index 06f9f29e08..6d826eba49 100644 --- a/libavcodec/cbs_h266_syntax_template.c +++ b/libavcodec/cbs_h266_syntax_template.c @@ -2231,8 +2231,8 @@ static int FUNC(pred_weight_table) (CodedBitstreamContext *ctx, RWContext *rw, return 0; } -static int FUNC(picture_header) (CodedBitstreamContext *ctx, RWContext *rw, - H266RawPH *current){ +static int FUNC(picture_header_structure)(CodedBitstreamContext *ctx, RWContext *rw, + H266RawPictureHeader *current) { CodedBitstreamH266Context *h266 = ctx->priv_data; const H266RawVPS *vps; const H266RawSPS *sps; @@ -2651,7 +2651,7 @@ static int FUNC(ph) (CodedBitstreamContext *ctx, RWContext *rw, HEADER("Picture Header"); CHECK(FUNC(nal_unit_header) (ctx, rw, ¤t->nal_unit_header, VVC_PH_NUT)); - CHECK(FUNC(picture_header) (ctx, rw, current)); + CHECK(FUNC(picture_header_structure) (ctx, rw, ¤t->ph_picture_header)); CHECK(FUNC(rbsp_trailing_bits) (ctx, rw)); return 0; } @@ -2662,7 +2662,7 @@ static int FUNC(slice_header) (CodedBitstreamContext *ctx, RWContext *rw, CodedBitstreamH266Context *h266 = ctx->priv_data; const H266RawSPS *sps; const H266RawPPS *pps; - const H266RawPH *ph; + const H266RawPictureHeader *ph; const H266RefPicLists *ref_pic_lists; int err, i; uint8_t nal_unit_type, qp_bd_offset; @@ -2675,12 +2675,11 @@ static int FUNC(slice_header) (CodedBitstreamContext *ctx, RWContext *rw, flag(sh_picture_header_in_slice_header_flag); if (current->sh_picture_header_in_slice_header_flag) { - CHECK(FUNC(picture_header) (ctx, rw, ¤t->sh_picture_header)); + //7.4.8 if sh_picture_header_in_slice_header_flag is true, we do not have a PH NAL unit + CHECK(FUNC(picture_header_structure) (ctx, rw, ¤t->sh_picture_header)); ph = ¤t->sh_picture_header; - //7.4.8 if sh_picture_header_in_slice_header_flag is true, we do not have PH NAL unit - h266->priv.ph = NULL; } else { - ph = h266->priv.ph; + ph = h266->ph; if (!ph) { av_log(ctx->log_ctx, AV_LOG_ERROR, "Picture header not available.\n"); @@ -2822,7 +2821,7 @@ static int FUNC(slice_header) (CodedBitstreamContext *ctx, RWContext *rw, (ctx, rw, sps, pps, ¤t->sh_ref_pic_lists)); ref_pic_lists = ¤t->sh_ref_pic_lists; } else { - ref_pic_lists = &h266->priv.ph->ph_ref_pic_lists; + ref_pic_lists = &ph->ph_ref_pic_lists; } if ((current->sh_slice_type != VVC_SLICE_TYPE_I && ref_pic_lists->rpl_ref_list[0].num_ref_entries > 1) || diff --git a/libavcodec/h266_metadata_bsf.c b/libavcodec/h266_metadata_bsf.c index 18fa1a5c9c..924eeeaf35 100644 --- a/libavcodec/h266_metadata_bsf.c +++ b/libavcodec/h266_metadata_bsf.c @@ -48,7 +48,7 @@ static int h266_metadata_update_fragment(AVBSFContext *bsf, AVPacket *pkt, ff_cbs_delete_unit(pu, 0); } else if ( pkt && ctx->aud == BSF_ELEMENT_INSERT) { const H266RawSlice *first_slice = NULL; - const H266RawPH *ph = NULL; + const H266RawPictureHeader *picture_header = NULL; H266RawAUD *aud = &ctx->aud_nal; int pic_type = 0, temporal_id = 8, layer_id = 0; for (i = 0; i < pu->nb_units; i++) { @@ -58,7 +58,8 @@ static int h266_metadata_update_fragment(AVBSFContext *bsf, AVPacket *pkt, if (nal->nuh_temporal_id_plus1 < temporal_id + 1) temporal_id = nal->nuh_temporal_id_plus1 - 1; if ( nal->nal_unit_type == VVC_PH_NUT ) { - ph = pu->units[i].content; + const H266RawPH *ph = pu->units[i].content; + picture_header = &ph->ph_picture_header; } else if (IS_H266_SLICE(nal->nal_unit_type)) { const H266RawSlice *slice = pu->units[i].content; layer_id = nal->nuh_layer_id; @@ -72,13 +73,13 @@ static int h266_metadata_update_fragment(AVBSFContext *bsf, AVPacket *pkt, first_slice = slice; if (first_slice->header. sh_picture_header_in_slice_header_flag) - ph = &first_slice->header.sh_picture_header; - else if (!ph) + picture_header = &first_slice->header.sh_picture_header; + else if (!picture_header) break; } } } - if (!ph) { + if (!picture_header) { av_log(bsf, AV_LOG_ERROR, "no avaliable picture header"); return AVERROR_INVALIDDATA; } @@ -89,7 +90,7 @@ static int h266_metadata_update_fragment(AVBSFContext *bsf, AVPacket *pkt, .nuh_temporal_id_plus1 = temporal_id + 1, }; aud->aud_pic_type = pic_type; - aud->aud_irap_or_gdr_flag = ph->ph_gdr_or_irap_pic_flag; + aud->aud_irap_or_gdr_flag = picture_header->ph_gdr_or_irap_pic_flag; err = ff_cbs_insert_unit_content(pu, 0, VVC_AUD_NUT, aud, NULL); if (err < 0) { diff --git a/libavcodec/vvc_parser.c b/libavcodec/vvc_parser.c index 69696eef57..19ca601bdb 100644 --- a/libavcodec/vvc_parser.c +++ b/libavcodec/vvc_parser.c @@ -31,7 +31,7 @@ typedef struct PuInfo { const H266RawPPS *pps; const H266RawSPS *sps; - const H266RawPH *ph; + const H266RawPictureHeader *ph; const H266RawSlice *slice; int pic_type; } PuInfo; @@ -155,7 +155,6 @@ static void set_parser_ctx(AVCodecParserContext *s, AVCodecContext *avctx, }; const H266RawSPS *sps = pu->sps; const H266RawPPS *pps = pu->pps; - //const H266RawPH *ph = pu->ph; const H266RawNALUnitHeader *nal = &pu->slice->header.nal_unit_header; s->pict_type = pu->pic_type; @@ -205,7 +204,7 @@ static void set_parser_ctx(AVCodecParserContext *s, AVCodecContext *avctx, //We follow the VTM. static void get_slice_poc(VVCParserContext *s, int *poc, const H266RawSPS *sps, - const H266RawPH *ph, + const H266RawPictureHeader *ph, const H266RawSliceHeader *slice, void *log_ctx) { int poc_msb, max_poc_lsb, poc_lsb; @@ -251,7 +250,7 @@ static int is_au_start(VVCParserContext *s, const PuInfo *pu, void *log_ctx) AuDetector *d = &s->au_detector; const H266RawSPS *sps = pu->sps; const H266RawNALUnitHeader *nal = &pu->slice->header.nal_unit_header; - const H266RawPH *ph = pu->ph; + const H266RawPictureHeader *ph = pu->ph; const H266RawSlice *slice = pu->slice; int ret, poc, nut; @@ -282,7 +281,8 @@ static int get_pu_info(PuInfo *info, const CodedBitstreamH266Context *h266, if (!nal) continue; if ( nal->nal_unit_type == VVC_PH_NUT ) { - info->ph = pu->units[i].content; + const H266RawPH *ph = pu->units[i].content; + info->ph = &ph->ph_picture_header; } else if (IS_H266_SLICE(nal->nal_unit_type)) { info->slice = pu->units[i].content; if (info->slice->header.sh_picture_header_in_slice_header_flag)
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/cbs_h2645.c | 35 ++++++++++++++++----------- libavcodec/cbs_h266.h | 17 +++++++------ libavcodec/cbs_h266_syntax_template.c | 17 ++++++------- libavcodec/h266_metadata_bsf.c | 13 +++++----- libavcodec/vvc_parser.c | 10 ++++---- 5 files changed, 50 insertions(+), 42 deletions(-)