[FFmpeg-devel] [PATCH v13 3/4] avcodec/libjxl: add Jpeg XL encoding via libjxl

[Anton Khirnov]

Quoting Leo Izen (2022-04-05 18:55:03)
> +static int libjxl_init_jxl_encoder(AVCodecContext *avctx)
> +{
> +    LibJxlEncodeContext *ctx = avctx->priv_data;
> +
> +    /* reset the encoder every frame for image2 muxer */
> +    JxlEncoderReset(ctx->encoder);
> +
> +    ctx->options = JxlEncoderFrameSettingsCreate(ctx->encoder, NULL);
> +    if (!ctx->options) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to create JxlEncoderOptions\n");
> +        return AVERROR_EXTERNAL;
> +    }
> +
> +    /* This needs to be set each time the decoder is reset */
> +    if (JxlEncoderSetParallelRunner(ctx->encoder, JxlThreadParallelRunner, ctx->runner)
> +            != JXL_ENC_SUCCESS) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to set JxlThreadParallelRunner\n");
> +        return AVERROR_EXTERNAL;
> +    }
> +
> +    /* these shouldn't fail, libjxl bug notwithstanding */
> +    if (JxlEncoderFrameSettingsSetOption(ctx->options, JXL_ENC_FRAME_SETTING_EFFORT, ctx->effort)
> +            != JXL_ENC_SUCCESS) {
> +        av_log(avctx, AV_LOG_ERROR, "Failed to set effort to: %d\n", ctx->effort);
> +        return AVERROR_EXTERNAL;
> +    }
> +
> +    /* check for negative zero, our default */
> +    if (1.0f / ctx->distance == 1.0f / -0.0f) {

IIRC division by zero is UB. Why not make the default -1.0 and then just
check whether the number is negative?

> +    /*
> +     * This buffer will be copied when the generic
> +     * code makes this packet refcounted,
> +     * so we can use the buffer again.
> +     */
> +    pkt->data = ctx->buffer;
> +    pkt->size = bytes_written;

This is very evil. Encoders should return refcounted packets and not
rely on the generic code fixing stuff up for them.

Also, pointers from av_malloc() cannot be passed to av_realloc(). You
need to allocate it with av_realloc() in the first place.

-- 
Anton Khirnov