[FFmpeg-devel] [PATCH v12 2/4] avcodec/libjxl: add Jpeg XL decoding via libjxl

[Leo Izen]

On 4/5/22 06:50, Anton Khirnov wrote:
> Quoting Leo Izen (2022-04-02 22:12:08)
>> +        case JXL_DEC_COLOR_ENCODING:
>> +            av_log(avctx, AV_LOG_DEBUG, "COLOR_ENCODING event emitted\n");
>> +            jret = JxlDecoderGetICCProfileSize(ctx->decoder, &ctx->jxl_pixfmt, JXL_COLOR_PROFILE_TARGET_ORIGINAL, &ctx->iccp_len);
> Does iccp_len need to be a context variable? Seems to me it's only used
> in this block.
Probably not, given that ctx->iccp is an AvBufferRef.
>> +            }
>> +            continue;
>> +        case JXL_DEC_FRAME:
>> +        case JXL_DEC_NEED_IMAGE_OUT_BUFFER:
>> +            /*
>> +             * We don't do this at basic info time
>> +             * because it will happen again when we
>> +             * rewind anyway
>> +             */
>> +            av_log(avctx, AV_LOG_DEBUG, "%s event emitted\n", jret == JXL_DEC_FRAME ? "FRAME" : "NEED_IMAGE_OUT_BUFFER");
>> +            ret = ff_get_buffer(avctx, frame, 0);
> Is it absolutely guaranteed that this will always happen before
> JXL_DEC_FULL_IMAGE in the same libjxl_decode_frame() invocation?
>
> Can it happen that you get JXL_DEC_NEED_IMAGE_OUT_BUFFER/JXL_DEC_FRAME,
> then JXL_DEC_NEED_MORE_INPUT, which causes you to return, then
> JXL_DEC_FULL_IMAGE on the next libjxl_decode_frame() call?
I don't see why it couldn't, but what I don't understand is why this is 
necessarily a problem.
>> +        case JXL_DEC_SUCCESS:
>> +            av_log(avctx, AV_LOG_DEBUG, "SUCCESS event emitted\n");
>> +            /*
>> +             * The file has finished decoding
>> +             * reset the decoder to let us
>> +             * reuse it again for the next image
>> +             */
>> +            JxlDecoderReset(ctx->decoder);
>> +            libjxl_init_jxl_decoder(avctx);
>> +            buf = avpkt->data;
>> +            remaining = avpkt->size;
> Why reset buf? The same data is not going to be decoded again, is it?

Decoding a single image will never fire the JXL_DEC_SUCCESS call since 
we return before then. If you decode an image2 sequence (e.g. 
input-%d.jxl), JXL_DEC_SUCCESS is fired before the start of the next 
frame. When this event is fired, the contents of avpkt->data and 
avpkt->size are the already the next frame's data, so this is indeed 
necessary.