[FFmpeg-devel] [PATCH v4 1/2] avcodec/libjxc: add JPEG XL decoding via libjxl

[James Almer]

On 11/2/2021 8:51 AM, Leo Izen wrote:
> diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
> index ab265ec584..853bbd4240 100644
> --- a/libavcodec/codec_id.h
> +++ b/libavcodec/codec_id.h
> @@ -59,6 +59,7 @@ enum AVCodecID {
>       AV_CODEC_ID_LJPEG,
>       AV_CODEC_ID_SP5X,
>       AV_CODEC_ID_JPEGLS,
> +    AV_CODEC_ID_JPEGXL,
>       AV_CODEC_ID_MPEG4,
>       AV_CODEC_ID_RAWVIDEO,
>       AV_CODEC_ID_MSMPEG4V1,

New entries must not change the value of other entries. You need to add 
it at the end of the list, or in this specific enum case, at the end of 
the section before the big offset jump, so immediately after 
AV_CODEC_ID_GEM.

>      AV_CODEC_ID_MSMPEG4V1,
> diff --git a/libavcodec/libjxl.c b/libavcodec/libjxl.c
> new file mode 100644
> index 0000000000..6e7f69c66a
> --- /dev/null
> +++ b/libavcodec/libjxl.c
> @@ -0,0 +1,72 @@
> +/*
> + * JPEG XL de/encoding via libjxl, common support implementation
> + * Copyright (c) 2021 Leo Izen <leo.izen at gmail.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * JPEG XL via libjxl common support implementation
> + */
> +
> +#include "libavutil/cpu.h"
> +#include "libavutil/mem.h"
> +
> +#include <jxl/memory_manager.h>
> +#include "libjxl.h"
> +
> +size_t ff_libjxl_get_threadcount(int threads)
> +{
> +    if (threads > FF_LIBJXL_THREADS_MAX)
> +        return FF_LIBJXL_THREADS_MAX;
> +    if (threads <= 0)
> +        return av_cpu_count();
> +    if (threads == 1)
> +        return 0;
> +    return threads;
> +}
> +
> +/**
> + * Wrapper around av_malloc used as a jpegxl_alloc_func.
> + *
> + * @param  opaque opaque pointer for jpegxl_alloc_func, always ignored
> + * @param  size Size in bytes for the memory block to be allocated
> + * @return Pointer to the allocated block, or `NULL` if it cannot be allocated
> + */
> +static void *ff_libjxl_av_malloc(void *opaque, size_t size)

If it's static then it doesn't need the ff_ prefix.

> +{
> +    return av_malloc(size);
> +}
> +
> +/**
> + * Wrapper around av_free used as a jpegxl_free_func.
> + *
> + * @param opaque  opaque pointer for jpegxl_free_func, always ignored
> + * @param address Pointer to the allocated block, to free. `NULL` permitted as a no-op.
> + */
> +static void ff_libjxl_av_free(void *opaque, void *address)

Same.

> +static const AVCodecDefault libjxl_decode_defaults[] = {
> +    { "threads",             "0" },

avctx->threads should be set to 0 by default since you set 
FF_CODEC_CAP_AUTO_THREADS, so no need for this.

> +    { NULL },
> +};
> +
> +#define OFFSET(x) offsetof(LibJxlDecodeContext, x)
> +#define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
> +
> +static const AVOption libjxl_decode_options[] = {
> +    { "threads",               "Number of worker threads",             OFFSET(ff_threads),          AV_OPT_TYPE_INT,   { .i64 = 0 }, 0, FF_LIBJXL_THREADS_MAX, VD, "threads" },
> +    { "auto",                  "automatic",                            0,                           AV_OPT_TYPE_CONST, { .i64 = 0 }, 0, FF_LIBJXL_THREADS_MAX, VD, "threads" },

Why are you adding a custom avoption named the same way as the global 
one? Just use avctx->threads.

> +    { NULL },
> +};