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

[James Almer]

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.

> 
> 
>>> +    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()
>>
>>> +            av_log(avctx, AV_LOG_WARNING, "Failed to parse
>> extradata.\n");
>>> +
>>> +        ff_cbs_fragment_reset(pu);
>>> +    }
>>> +    av_packet_unref(&s->last_au);
>>> +    ret = parse_nal_units(ctx, *buf, *buf_size, avctx);
>>> +    if (ret == 0) {
>>> +        if (s->last_au.size) {
>>> +            *buf = s->last_au.data;
>>> +            *buf_size = s->last_au.size;
>>> +        } else {
>>> +            //no output
>>> +            ret = 1;
>>> +        }
>>> +    }
>>> +    s->cbc->log_ctx = NULL;
>>> +    return ret;
>>> +}
>>> +
>>> +static int vvc_parser_parse(AVCodecParserContext *ctx, AVCodecContext
>> *avctx,
>>> +                      const uint8_t **poutbuf, int *poutbuf_size,
>>> +                      const uint8_t *buf, int buf_size)
>>> +{
>>> +    int next;
>>> +    VVCParserContext *s = ctx->priv_data;
>>> +    ParseContext *pc = &s->pc;
>>> +
>>> +    if (avctx->extradata && !s->parsed_extradata) {
>>> +        av_log(avctx, AV_LOG_INFO, "extra data is not supported
>> yet.\n");
>>> +        return AVERROR_PATCHWELCOME;
>>> +    }
>>> +
>>> +    if (ctx->flags & PARSER_FLAG_COMPLETE_FRAMES) {
>>> +        next = buf_size;
>>> +    } else {
>>> +        int ret, flush = !buf_size;
>>> +        next = find_frame_end(ctx, buf, buf_size);
>>> +        if (ff_combine_frame(pc, next, &buf, &buf_size) < 0)
>>> +            goto no_out;
>>> +        ret = combine_au(ctx, avctx, &buf, &buf_size);
>>> +        if (ret > 0 && flush) {
>>> +            buf_size = 0;
>>> +            ret = combine_au(ctx, avctx, &buf, &buf_size);
>>> +        }
>>> +        if (ret != 0) {
>>> +            buf_size = next;
>>> +            goto no_out;
>>> +        }
>>> +    }
>>> +    *poutbuf      = buf;
>>> +    *poutbuf_size = buf_size;
>>> +    return next;
>>> +no_out:
>>> +    *poutbuf      = NULL;
>>> +    *poutbuf_size = 0;
>>> +    return buf_size;
>>> +}
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".