[FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser
James Almer
jamrial at gmail.com
Thu Jun 22 03:59:35 EEST 2023
On 6/21/2023 9:43 PM, Leo Izen wrote:
> diff --git a/libavcodec/jpegxl_parse.c b/libavcodec/jpegxl_parse.c
> new file mode 100644
> index 0000000000..be360acb08
> --- /dev/null
> +++ b/libavcodec/jpegxl_parse.c
> @@ -0,0 +1,22 @@
> +/*
> + * JPEG XL Header Parser
> + * Copyright (c) 2023 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
> + */
> +
> +#include "jpegxl_parse.h"
This is a really weird way to achieve code sharing between libraries.
What you named jpegxl_parse.h should be jpegxl_parse.c instead, and
jpegxl_parse.h should only have the prototypes for
ff_jpegxl_collect_codestream_header() and
ff_jpegxl_parse_codestream_header(), plus the definition of
FFJXLMetadata. The enums can stay in jpegxl.h, and they for that matter
don't need the FF_ prefix.
> diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
> new file mode 100644
> index 0000000000..096b22be0f
> --- /dev/null
> +++ b/libavcodec/jpegxl_parser.c
> @@ -0,0 +1,165 @@
> +/*
> + * JPEG XL Codec Parser
> + * Copyright (c) 2023 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
> + */
> +
> +#include "libavutil/intreadwrite.h"
> +
> +#include "codec_id.h"
> +#include "jpegxl.h"
> +#include "parser.h"
> +
> +typedef struct JXLParseContext {
> + ParseContext pc;
You don't need this. You're not assembling a packet.
> + FFJXLMetadata meta;
> + int parsed_header;
> +} JXLParseContext;
> +
> +static int jpegxl_parse(AVCodecParserContext *s, AVCodecContext *avctx,
> + const uint8_t **poutbuf, int *poutbuf_size,
> + const uint8_t *buf, int buf_size)
> +{
> + JXLParseContext *ctx = s->priv_data;
> + int ret;
> +
> + *poutbuf_size = 0;
> + *poutbuf = NULL;
> +
> + if (!ctx->parsed_header) {
> + if (AV_RL64(buf) == FF_JPEGXL_CONTAINER_SIGNATURE_LE) {
> + int copied;
> + uint8_t codestream_header[4096];
> + ret = ff_jpegxl_collect_codestream_header(buf, buf_size, codestream_header,
> + sizeof(codestream_header), &copied);
> + if (ret < 0)
> + return ret;
> + /* copied may be larger than the bufsize if stuff was skipped */
> + copied = FFMIN(copied, sizeof(codestream_header));
> + ret = ff_jpegxl_parse_codestream_header(codestream_header, copied, 0, &ctx->meta);
> + if (ret < 0)
> + return ret;
> + } else {
> + ret = ff_jpegxl_parse_codestream_header(buf, buf_size, 1, &ctx->meta);
> + if (ret < 0)
> + return ret;
> + }
> + avctx->width = ctx->meta.width;
> + avctx->height = ctx->meta.height;
> +
> + switch (ctx->meta.csp) {
> + case FF_JPEGXL_CS_RGB:
> + case FF_JPEGXL_CS_XYB:
> + avctx->colorspace = AVCOL_SPC_RGB;
> + break;
> + default:
> + avctx->colorspace = AVCOL_SPC_UNSPECIFIED;
> + }
> +
> + if (ctx->meta.wp == FF_JPEGXL_WP_D65) {
> + switch (ctx->meta.primaries) {
> + case FF_JPEGXL_PR_SRGB:
> + avctx->color_primaries = AVCOL_PRI_BT709;
> + break;
> + case FF_JPEGXL_PR_P3:
> + avctx->color_primaries = AVCOL_PRI_SMPTE432;
> + break;
> + case FF_JPEGXL_PR_2100:
> + avctx->color_primaries = AVCOL_PRI_BT2020;
> + break;
> + default:
> + avctx->color_primaries = AVCOL_PRI_UNSPECIFIED;
> + }
> + } else if (ctx->meta.wp == FF_JPEGXL_WP_DCI && ctx->meta.primaries == FF_JPEGXL_PR_P3) {
> + avctx->color_primaries = AVCOL_PRI_SMPTE431;
> + } else {
> + avctx->color_primaries = AVCOL_PRI_UNSPECIFIED;
> + }
> +
> + if (ctx->meta.trc > FF_JPEGXL_TR_GAMMA) {
> + FFJXLTransferCharacteristic trc = ctx->meta.trc - FF_JPEGXL_TR_GAMMA;
> + switch (trc) {
> + case FF_JPEGXL_TR_BT709:
> + avctx->color_trc = AVCOL_TRC_BT709;
> + break;
> + case FF_JPEGXL_TR_LINEAR:
> + avctx->color_trc = AVCOL_TRC_LINEAR;
> + break;
> + case FF_JPEGXL_TR_SRGB:
> + avctx->color_trc = AVCOL_TRC_IEC61966_2_1;
> + break;
> + case FF_JPEGXL_TR_PQ:
> + avctx->color_trc = AVCOL_TRC_SMPTEST2084;
> + break;
> + case FF_JPEGXL_TR_DCI:
> + avctx->color_trc = AVCOL_TRC_SMPTE428;
> + break;
> + case FF_JPEGXL_TR_HLG:
> + avctx->color_trc = AVCOL_TRC_ARIB_STD_B67;
> + break;
> + default:
> + avctx->color_trc = AVCOL_TRC_UNSPECIFIED;
> + }
> + } else if (ctx->meta.trc > 0) {
> + if (ctx->meta.trc > 45355 && ctx->meta.trc < 45555)
> + avctx->color_trc = AVCOL_TRC_GAMMA22;
> + else if (ctx->meta.trc > 35614 && ctx->meta.trc < 35814)
> + avctx->color_trc = AVCOL_TRC_GAMMA28;
> + else
> + avctx->color_trc = AVCOL_TRC_UNSPECIFIED;
> + } else {
> + avctx->color_trc = AVCOL_TRC_UNSPECIFIED;
> + }
> +
> + if (ctx->meta.csp == FF_JPEGXL_CS_GRAY) {
> + if (ctx->meta.bit_depth <= 8)
> + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_YA8 : AV_PIX_FMT_GRAY8;.
Set s->format, not avctx->pix_fmt. The format in an AVCodecContext is
set by a decoder, which may not match what the parser sets.
> + else if (ctx->meta.bit_depth <= 16)
> + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_YA16 : AV_PIX_FMT_GRAY16;
> + else
> + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_NONE : AV_PIX_FMT_GRAYF32;
> + } else {
> + if (ctx->meta.bit_depth <= 8)
> + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_RGBA : AV_PIX_FMT_RGB24;
> + else if (ctx->meta.bit_depth <= 16)
> + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_RGBA64 : AV_PIX_FMT_RGB48;
> + else
> + avctx->pix_fmt = ctx->meta.have_alpha ? AV_PIX_FMT_RGBAF32 : AV_PIX_FMT_RGBF32;
> + }
> +
> + ctx->parsed_header = 1;
So this is done once for the entire stream?
> + }
> +
> + ctx->pc.frame_start_found = 1;
Unnecessary.
> +
> + ret = ff_combine_frame(&ctx->pc, END_NOT_FOUND, &buf, &buf_size);
Also unnecessary.
> + if (ret < 0)
> + return buf_size;
> +
> + *poutbuf = buf;
> + *poutbuf_size = buf_size;
> +
> + return END_NOT_FOUND;
???
The process succeeded. Return 0.
> +}
> +
> +const AVCodecParser ff_jpegxl_parser = {
> + .codec_ids = { AV_CODEC_ID_JPEGXL },
> + .priv_data_size = sizeof(JXLParseContext),
> + .parser_parse = jpegxl_parse,
> + .parser_close = ff_parse_close,
> +};
More information about the ffmpeg-devel
mailing list