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

[Leo Izen <leo.izen@gmail.com>]

On 12/15/23 16:31, Zsolt Vadász via ffmpeg-devel wrote:
> On Friday, December 15th, 2023 at 10:12 PM, Leo Izen <leo.izen@gmail.com> wrote:
> 
> 
>> 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@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 *.
> 
> Indeed, but according to the docs[0]:
> "If the last frame or last box has been added, JxlEncoderCloseInput, JxlEncoderCloseFrames
> and/or JxlEncoderCloseBoxes must be called before the next JxlEncoderProcessOutput call,
> or the codestream won’t be encoded correctly."
> When the encoder eventually receives NULL, there isn't anything left to add, unless I store the previous frame,
> is there?

You can always call JxlEncoderProcessOutput at the start of the loop, 
rather than calling it after JxlEncoderAddImageFrame. (Except perhaps 
the first loop.)

If you do this, you'll process the frame that was added last loop, and 
then you can add another frame. If you receive NULL, you call 
JxlEncoderCloseInput at the start of the loop, before you call 
JxlEncoderProcessOutput. This saves you from having to store the AVFrame 
in memory. libjxl will have a copy of it in memory anyway for reference 
purposes so this allows us to reduce memory usage.


>>>>> +
>>>>> + 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.
>>
> Well that's one thing I didn't account for, a real pity. Do you recommend emitting a
> single packet instead?

You can still accomplish what you're trying to do with libjxl but you 
need to finesse it a little bit. You need to (1) disable the container 
format, as its interaction with Frames is undefined, and you need to (2) 
loop calls to JxlEncoderProcessOutput until it returns JXL_ENC_SUCCESS.

The docs specify "This encodes the frames and/or boxes added so far" so 
it will end up flushing out the remaining frames if the container format 
is disabled.


>>>>> +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.
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> [0] https://libjxl.readthedocs.io/en/latest/api_encoder.html#_CPPv423JxlEncoderProcessOutputP10JxlEncoderPP7uint8_tP6size_t
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".