[FFmpeg-devel] [PATCH v2 03/10] avcodec: add bitstream parser for H266/VVC

[Thomas Siedel]

On Mon, 31 Oct 2022 at 21:02, James Almer <jamrial at gmail.com> wrote:

> On 10/31/2022 7:16 AM, Thomas Siedel wrote:
> > On Mon, 24 Oct 2022 at 16:38, James Almer <jamrial at gmail.com> wrote:
> >
> >> On 10/24/2022 11:06 AM, Thomas Siedel wrote:
> >>> +static int combine_au(AVCodecParserContext *ctx, AVCodecContext
> *avctx,
> >>> +                      const uint8_t **buf, int *buf_size)
> >>> +{
> >>
> >> This is being called only when you first assembled AUs from what's
> >> assumed to be raw input. When PARSER_FLAG_COMPLETE_FRAMES is set, it
> >> should also parse the AU for bitstream information.
> >>
> >
> > Yes, currently, "combine_au" is only called when
> > PARSER_FLAG_COMPLETE_FRAMES is disabled, but I do not really understand
> > what the issue is here.
>
> I'm not talking about combine_au() but parse_nal_units(), which is only
> called from within combine_au(). My comment was about bitstream parsing,
> not AU assembling. Apologies if it wasn't clear.
>
> > As far as I know, all bitstream information is parsed properly for any
> kind
> > of supported content (currently, ES, MP4, and TS are supported).
> > Would you happen to have an example of a use case where
> > PARSER_FLAG_COMPLETE_FRAMES is enabled, and the bitstream information is
> > needed or not parsed correctly from the current implementation?
>
> libavformat and its demuxers are not the only user of AVCodecParsers.
>
> >
> >   The current behavior is pretty similar to other codec
> > parser implementations like hevc, avc, and av1. Why should the vvc parser
> > code differ from the (default) behavior of other codecs?
>
> Both h264 and hevc parsers call parse_nal_units() regardless of
> PARSER_FLAG_COMPLETE_FRAMES being set or not.
>
> AVCodecParsers have two purposes. One is assembling a full packet for a
> given timestamp (access unit for these codecs, temporal unit for av1,
> the like) depending on the value of AVCodecParserContext.flags, and the
> other is parsing the full packet for bitstream information. If a parser
> does the latter, it should do it regardless of having assembled a packet
> before that or not.
> The parser doesn't know where the packet came from. It could have been a
> lavf demuxer, or it could have been from some other lavc user, because
> this API after all is from lavc and not lavf. So it must always fill the
> AVCodecParserContext struct with the parsed information if it's present
> and readable.
>
> If i set the PARSER_FLAG_COMPLETE_FRAMES flag, this parser as you wrote
> it is a no-op.
>

It seems I misunderstood your previous comment, but now it is clear. Thanks
for the explanation.

I now changed the implementation, it is included in version 3 of the patch
set.
combine_au() has been moved so that it is always called when the input
buffer is valid.
Now, this should be the same behavior as in other codecs.


> >
> >
> >>> +    VVCParserContext *s = ctx->priv_data;
> >>> +    CodedBitstreamFragment *pu = &s->picture_unit;
> >>> +    int ret;
> >>> +
> >>> +    s->cbc->log_ctx = avctx;
> >>> +
> >>> +    if (avctx->extradata_size && !s->parsed_extradata) {
> >>> +        s->parsed_extradata = 1;
> >>> +
> >>> +        if ((ret = ff_cbs_read(s->cbc, pu, avctx->extradata,
> >> avctx->extradata_size)) < 0)
> >>
> >> ff_cbs_read_extradata_from_codec()
>

Thanks for pointing this out. I am now using
ff_cbs_read_extradata_from_codec instead of ff_cbs_read.
The code has been moved into vvc_parser_parse.
In older versions, the vvc_parser_parse function returned an error with
'AVERROR_PATCHWELCOME', so
that the extradata parsing couldn´t be reached.
Now ff_cbs_read_extradata_from_codec has been moved here.