[FFmpeg-devel] [PATCH v2 1/1] avcodec/libjxlenc: Add libjxl_animated encoder

[Leo Izen]

On 12/15/23 11:40, Zsolt Vadász via ffmpeg-devel wrote:
> On Friday, December 15th, 2023 at 12:20 AM, Leo Izen <leo.izen at gmail.com> wrote:
> 
> 
>>> + AVFrame *last;
>>
>>
>> I don't see the purpose of keeping this here.
>>
> 
> The name is misleading, I should have named it `previous`, since it always contains the previous frame.
> I did it this way so I could call JxlEncoderCloseInput when the callback received NULL.

This isn't needed to call JxlEncoderCloseInput, as it takes one 
argument, which is JxlEncoder *.


>>> +
>>> + if(!ctx->last && !avctx->internal->draining) { > + ctx->last = av_frame_clone(frame);
>>> + *got_packet = 0;
>>> + return AVERROR(EAGAIN);
>>
>> It looks like you're trying to emit one packet per image, rather than
>> one packet per encoded frame. This is fine, but you should not be
>> calling av_frame_clone, and there's no reason to use
>> avctx->internal->draining here. If you are doing this, you also have no
>> reason to cache ctx->last at all.
> 
> It's the opposite, I'm trying to emit a packet for each frame of the animation.

Libjxl provides no promise of doing this meaningfully, by the way. You 
may end up with arbitrary subdivisions, not subdivisions on frame 
boundaries.

>>> +const FFCodec ff_libjxl_animated_encoder = {
>>> + .p.name = "libjxl_animated",
>>> + CODEC_LONG_NAME("libjxl Animated JPEG XL"),
>>> + .p.type = AVMEDIA_TYPE_VIDEO,
>>> + .p.id = AV_CODEC_ID_JPEGXL,
>>> + .priv_data_size = sizeof(LibJxlEncodeContext),
>>> + .init = libjxl_animated_encode_init,
>>> + FF_CODEC_ENCODE_CB(libjxl_animated_encode_frame),
>>> + .close = libjxl_encode_close,
>>> + .p.capabilities = AV_CODEC_CAP_OTHER_THREADS |
>>> + AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE |
>>> + AV_CODEC_CAP_DELAY,
>>> + .caps_internal = FF_CODEC_CAP_NOT_INIT_THREADSAFE |
>>> + FF_CODEC_CAP_AUTO_THREADS | FF_CODEC_CAP_INIT_CLEANUP |
>>> + FF_CODEC_CAP_ICC_PROFILES | FF_CODEC_CAP_EOF_FLUSH,
>>
>>
>> Why FF_CODEC_CAP_EOF_FLUSH?
> 
> So the encoder receives a NULL after the last frame has been submitted,
> so JxlEncoderCloseInput can be called and the final frame properly emitted.

Ah, that makes sense, thanks.