[FFmpeg-devel] [PATCH v4 2/4] avcodec/jpegxl_parser: add JPEG XL parser

[James Almer]

On 6/27/2023 8:18 PM, Leo Izen wrote:
> On 6/27/23 18:58, James Almer wrote:
>> On 6/26/2023 12:49 PM, Leo Izen wrote:
>>> +/*
>>> + * copies as much of the codestream into the buffer as possible
>>> + * pass a shorter buflen to request less
>>> + * returns the number of bytes consumed from input, may be greater 
>>> than input_len
>>> + * if the input doesn't end on an ISOBMFF-box boundary
>>> + */
>>> +int ff_jpegxl_collect_codestream_header(const uint8_t *input_buffer, 
>>> int input_len,
>>> +                                        uint8_t *buffer, int buflen, 
>>> int *copied)
>>> +{
>>> +    GetByteContext gb;
>>> +    int pos = 0;
>>> +    bytestream2_init(&gb, input_buffer, input_len);
>>> +
>>> +    while (1) {
>>> +        uint64_t size;
>>> +        uint32_t tag;
>>> +        int head_size = 8;
>>> +
>>> +        if (bytestream2_get_bytes_left(&gb) < 16)
>>> +            break;
>>> +
>>> +        size = bytestream2_get_be32(&gb);
>>> +        if (size == 1) {
>>> +            size = bytestream2_get_be64(&gb);
>>> +            head_size = 16;
>>> +        }
>>> +        /* invalid ISOBMFF size */
>>> +        if (size && size <= head_size)
>>> +            return AVERROR_INVALIDDATA;
>>
>> ISOBMFF? Are you propagating container bytes as if they were part of 
>> the bitstream from image2? Why is it not being handled by the mov/mp4 
>> demuxer instead?
> 
> It's not actually ISOBMFF, it just uses the same style of box layout. 
> This is also how j2k works.

Ok.

> 
>>
>>> +    if (ctx->meta.csp == JPEGXL_CS_GRAY) {
>>> +        if (ctx->meta.bit_depth <= 8)
>>> +            avctx->pix_fmt = s->format = ctx->meta.have_alpha ? 
>>> AV_PIX_FMT_YA8 : AV_PIX_FMT_GRAY8;
>>> +        else if (ctx->meta.bit_depth <= 16)
>>> +            avctx->pix_fmt = s->format = ctx->meta.have_alpha ? 
>>> AV_PIX_FMT_YA16 : AV_PIX_FMT_GRAY16;
>>> +        else
>>> +            avctx->pix_fmt = s->format = ctx->meta.have_alpha ? 
>>> AV_PIX_FMT_NONE : AV_PIX_FMT_GRAYF32;
>>> +    } else {
>>> +        if (ctx->meta.bit_depth <= 8)
>>> +            avctx->pix_fmt = s->format = ctx->meta.have_alpha ? 
>>> AV_PIX_FMT_RGBA : AV_PIX_FMT_RGB24;
>>> +        else if (ctx->meta.bit_depth <= 16)
>>> +            avctx->pix_fmt = s->format = ctx->meta.have_alpha ? 
>>> AV_PIX_FMT_RGBA64 : AV_PIX_FMT_RGB48;
>>> +        else
>>> +            avctx->pix_fmt = s->format = ctx->meta.have_alpha ? 
>>> AV_PIX_FMT_RGBAF32 : AV_PIX_FMT_RGBF32;
>>
>> Again, don't fill avctx->pix_fmt, fill s->format.
>> _______________________________________________
> 
> If I don't fill this, the CLI will not show the pixel format without the 
> external decoder library enabled. Should I consider this an ffmpeg.c bug?

That's expected, and it's not a bug. A given decoder can and many times 
disagrees with the parser. In an scenario without a decoder, remuxing 
still works as pix_fmt is not a container level parameter.