From: Nuo Mi <nuomi2021@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec/cbs_h2645: fix parsing and storing Picture Header references in the context
Date: Sat, 1 Jul 2023 19:51:31 +0800
Message-ID: <CAFXK13cCspQTkZtaOF+wphcZoL8GmS82Y82USFyzW+kDFg1KLQ@mail.gmail.com> (raw)
In-Reply-To: <20230629224510.10393-1-jamrial@gmail.com>
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".
>
_______________________________________________
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".
next prev parent reply other threads:[~2023-07-01 11:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-29 22:45 James Almer
2023-06-29 22:45 ` [FFmpeg-devel] [PATCH v3 2/2] fate/cbs: add VVC tests James Almer
2023-07-01 11:51 ` Nuo Mi [this message]
2023-07-01 23:32 ` [FFmpeg-devel] [PATCH v2 1/2] avcodec/cbs_h2645: fix parsing and storing Picture Header references in the context James Almer
2023-07-02 1:06 ` Nuo Mi
2023-07-02 13:01 ` James Almer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFXK13cCspQTkZtaOF+wphcZoL8GmS82Y82USFyzW+kDFg1KLQ@mail.gmail.com \
--to=nuomi2021@gmail.com \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git