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

[Leo Izen]

On 4/15/22 07:08, Andreas Rheinhardt wrote:
> Leo Izen:
>
>> +            return avpkt->size - remaining;
> If this decoder is supposed to produce multiple frames from one packet
> of input, it needs to use the receive_frame-callback. For video
> decoders, it is only checked whether the return value is >= 0 or not
> (see lines 451-456 in decode.c).
It isn't, because I've chosen not to support animated JXL at this time. 
As far as I'm aware an animated JXL file will just decode to its first 
frame with this decoder.
>> +        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);
> You are only resetting the decoder on success. What happens if any of
> the errors happened? Would the decoder need to be reset before decoding
> the next picture?
This was already brought up by Anton. The JXL_DEC_SUCCESS event isn't 
fired until the start of the next packet (if there is one). It is the 
"finished decoding" event which we never actually get to for single 
images because JXL_DEC_FULL_IMAGE is always fired first. If this is 
fired it means we're in an image2 sequence and this is the next frame, 
at which point the data will have been swapped out. Resetting the 
decoder is necessary here to make the library re-read the header info 
and not whine. I agree that it's a bit confusing and I should probably 
add a block comment explaining this.
>> +            buf = avpkt->data;
>> +            remaining = avpkt->size;
>> +            continue;
> Didn't you already feed this data to the decoder? This somehow looks
> like a recipe for an infinite loop.
No, see up.

- Leo Izen (thebombzen)