On 6/21/23 20:59, James Almer wrote:
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.i...@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.


This is based on JEEB's recommendation. He provided these as examples:

https://github.com/jeeb/ffmpeg/commit/b74e47c4ff5bca998936c0d8b9a0892104a7403d
https://github.com/jeeb/ffmpeg/commit/d7a75d21635eab4f4a1efea22945933059c2e36f

How else should I do it? If I do the usual thing where I put prototypes and structs in the header file and the implementation in the C file, then I won't be able to compile them into avformat as well, as it will just grab the stubs, and the ff_funcs won't be exported from avcodec.


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.i...@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?

That's the intention.


+    }
+
+    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.

If I'm not packetizing, I wasn't sure if I could return any other value, but I can change it to 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,
+};

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to