Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v2 0/3] JXL Parser and FATE tests
@ 2023-06-22  0:43 Leo Izen
  2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser Leo Izen
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Leo Izen @ 2023-06-22  0:43 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Leo Izen

This test relies on FATE samples that haven't been uploaded yet. To test, unzip
the following zipfile[1] in the FATE_SAMPLES directory, placing the test images in jxl/.

[1]: https://buzo.us/y.zip

sha256sum: 43a2eeb0dfdf471b47a9fdfb1653974fa156ceceb776891cc137569a8ebf0e75
signature: https://buzo.us/R.asc

Leo Izen (3):
  avcodec/jpegxl_parser: add JPEG XL parser
  avformat/jpegxl: remove jpegxl_probe, instead call
    avcodec/jpegxl_parse
  fate/jpegxl_anim: add demuxer fate test for jpegxl_anim

 libavcodec/Makefile                           |   2 +
 libavcodec/jpegxl.h                           | 104 +++++++++
 .../jpegxl_parse.c                            |  21 +-
 .../jpegxl_parse.h                            | 203 +++++++++++++-----
 libavcodec/jpegxl_parser.c                    | 165 ++++++++++++++
 libavcodec/parsers.c                          |   1 +
 libavcodec/version.h                          |   2 +-
 libavformat/Makefile                          |   4 +-
 libavformat/img2dec.c                         |   4 +-
 libavformat/jpegxl_anim_dec.c                 | 111 ++--------
 libavformat/jpegxl_parse.c                    |  22 ++
 tests/Makefile                                |   1 +
 tests/fate/jxl.mak                            |  16 ++
 tests/ref/fate/jxl-anim-demux-belgium         |   6 +
 tests/ref/fate/jxl-anim-demux-icos4d          |   6 +
 tests/ref/fate/jxl-anim-demux-lenna256        |   6 +
 tests/ref/fate/jxl-anim-demux-newton          |   6 +
 17 files changed, 505 insertions(+), 175 deletions(-)
 create mode 100644 libavcodec/jpegxl.h
 rename libavformat/jpegxl_probe.h => libavcodec/jpegxl_parse.c (55%)
 rename libavformat/jpegxl_probe.c => libavcodec/jpegxl_parse.h (68%)
 create mode 100644 libavcodec/jpegxl_parser.c
 create mode 100644 libavformat/jpegxl_parse.c
 create mode 100644 tests/fate/jxl.mak
 create mode 100644 tests/ref/fate/jxl-anim-demux-belgium
 create mode 100644 tests/ref/fate/jxl-anim-demux-icos4d
 create mode 100644 tests/ref/fate/jxl-anim-demux-lenna256
 create mode 100644 tests/ref/fate/jxl-anim-demux-newton

-- 
2.41.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser
  2023-06-22  0:43 [FFmpeg-devel] [PATCH v2 0/3] JXL Parser and FATE tests Leo Izen
@ 2023-06-22  0:43 ` Leo Izen
  2023-06-22  0:59   ` James Almer
  2023-06-22 14:19   ` James Almer
  2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 2/3] avformat/jpegxl: remove jpegxl_probe, instead call avcodec/jpegxl_parse Leo Izen
  2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 3/3] fate/jpegxl_anim: add demuxer fate test for jpegxl_anim Leo Izen
  2 siblings, 2 replies; 8+ messages in thread
From: Leo Izen @ 2023-06-22  0:43 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Leo Izen

Add a parser to libavcodec for AV_CODEC_ID_JPEGXL. It doesn't find the
end of the stream in order to packetize the codec, but it does look at
the headers to set preliminary information like dimensions and pixel
format.

Note that much of this code is duplicated from avformat/jpegxl_probe.c,
but that code will be removed and call this instead in the next commit.

Signed-off-by: <leo.izen@gmail.com>
---
 libavcodec/Makefile        |   2 +
 libavcodec/jpegxl.h        | 104 ++++++++
 libavcodec/jpegxl_parse.c  |  22 ++
 libavcodec/jpegxl_parse.h  | 497 +++++++++++++++++++++++++++++++++++++
 libavcodec/jpegxl_parser.c | 165 ++++++++++++
 libavcodec/parsers.c       |   1 +
 libavcodec/version.h       |   2 +-
 7 files changed, 792 insertions(+), 1 deletion(-)
 create mode 100644 libavcodec/jpegxl.h
 create mode 100644 libavcodec/jpegxl_parse.c
 create mode 100644 libavcodec/jpegxl_parse.h
 create mode 100644 libavcodec/jpegxl_parser.c

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 0e4d27f37b..6f97f925aa 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1055,6 +1055,7 @@ STLIBOBJS-$(CONFIG_AVFORMAT)           += to_upper4.o
 STLIBOBJS-$(CONFIG_ISO_MEDIA)          += mpegaudiotabs.o
 STLIBOBJS-$(CONFIG_FLV_MUXER)          += mpeg4audio_sample_rates.o
 STLIBOBJS-$(CONFIG_HLS_DEMUXER)        += ac3_channel_layout_tab.o
+STLIBOBJS-$(CONFIG_JPEGXL_ANIM_DEMUXER)+= jpegxl_parse.o
 STLIBOBJS-$(CONFIG_MATROSKA_DEMUXER)   += mpeg4audio_sample_rates.o
 STLIBOBJS-$(CONFIG_MOV_DEMUXER)        += ac3_channel_layout_tab.o
 STLIBOBJS-$(CONFIG_MXF_MUXER)          += golomb.o
@@ -1184,6 +1185,7 @@ OBJS-$(CONFIG_HEVC_PARSER)             += hevc_parser.o hevc_data.o
 OBJS-$(CONFIG_HDR_PARSER)              += hdr_parser.o
 OBJS-$(CONFIG_IPU_PARSER)              += ipu_parser.o
 OBJS-$(CONFIG_JPEG2000_PARSER)         += jpeg2000_parser.o
+OBJS-$(CONFIG_JPEGXL_PARSER)           += jpegxl_parser.o jpegxl_parse.o
 OBJS-$(CONFIG_MISC4_PARSER)            += misc4_parser.o
 OBJS-$(CONFIG_MJPEG_PARSER)            += mjpeg_parser.o
 OBJS-$(CONFIG_MLP_PARSER)              += mlp_parse.o mlp_parser.o mlp.o
diff --git a/libavcodec/jpegxl.h b/libavcodec/jpegxl.h
new file mode 100644
index 0000000000..ceaf4b6522
--- /dev/null
+++ b/libavcodec/jpegxl.h
@@ -0,0 +1,104 @@
+/*
+ * JPEG XL Common Header Definitions
+ * Copyright (c) 2023 Leo Izen <leo.izen@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
+ */
+
+#ifndef AVCODEC_JPEGXL_H
+#define AVCODEC_JPEGXL_H
+
+#include <stdint.h>
+
+#include "libavutil/rational.h"
+
+#define FF_JPEGXL_CODESTREAM_SIGNATURE_LE 0x0aff
+#define FF_JPEGXL_CONTAINER_SIGNATURE_LE 0x204c584a0c000000
+
+typedef enum FFJXLExtraChannelType {
+    FF_JPEGXL_CT_ALPHA = 0,
+    FF_JPEGXL_CT_DEPTH,
+    FF_JPEGXL_CT_SPOT_COLOR,
+    FF_JPEGXL_CT_SELECTION_MASK,
+    FF_JPEGXL_CT_BLACK,
+    FF_JPEGXL_CT_CFA,
+    FF_JPEGXL_CT_THERMAL,
+    FF_JPEGXL_CT_NON_OPTIONAL = 15,
+    FF_JPEGXL_CT_OPTIONAL
+} FFJXLExtraChannelType;
+
+typedef enum FFJXLColorSpace {
+    FF_JPEGXL_CS_RGB = 0,
+    FF_JPEGXL_CS_GRAY,
+    FF_JPEGXL_CS_XYB,
+    FF_JPEGXL_CS_UNKNOWN
+} FFJXLColorSpace;
+
+typedef enum FFJXLWhitePoint {
+    FF_JPEGXL_WP_D65 = 1,
+    FF_JPEGXL_WP_CUSTOM,
+    FF_JPEGXL_WP_E = 10,
+    FF_JPEGXL_WP_DCI = 11
+} FFJXLWhitePoint;
+
+typedef enum FFJXLPrimaries {
+    FF_JPEGXL_PR_SRGB = 1,
+    FF_JPEGXL_PR_CUSTOM,
+    FF_JPEGXL_PR_2100 = 9,
+    FF_JPEGXL_PR_P3 = 11,
+} FFJXLPrimaries;
+
+typedef enum FFJXLTransferCharacteristic {
+    FF_JPEGXL_TR_BT709 = 1,
+    FF_JPEGXL_TR_UNKNOWN,
+    FF_JPEGXL_TR_LINEAR = 8,
+    FF_JPEGXL_TR_SRGB = 13,
+    FF_JPEGXL_TR_PQ = 16,
+    FF_JPEGXL_TR_DCI,
+    FF_JPEGXL_TR_HLG,
+    FF_JPEGXL_TR_GAMMA = 1 << 24,
+} FFJXLTransferCharacteristic;
+
+typedef struct FFJXLMetadata {
+    uint32_t width;
+    uint32_t height;
+    int bit_depth;
+    int have_alpha;
+    /*
+     * offset, in bits, of the animation header
+     * zero if not animated
+     */
+    int animation_offset;
+    AVRational timebase;
+    FFJXLColorSpace csp;
+    FFJXLWhitePoint wp;
+    FFJXLPrimaries primaries;
+    FFJXLTransferCharacteristic trc;
+} FFJXLMetadata;
+
+/*
+ * copies as much of the codestream into the buffer as possible
+ * pass a shorter buflen to request less
+ * returns the number of bytes consumed from input, may be greater than input_len
+ * if the input doesn't end on an ISOBMFF-box boundary
+ */
+int ff_jpegxl_collect_codestream_header(const uint8_t *input_buffer, int input_len,
+                                        uint8_t *buffer, int buflen, int *copied);
+
+int ff_jpegxl_parse_codestream_header(const uint8_t *buf, int buflen, int validate_level, FFJXLMetadata *meta);
+
+#endif /* AVCODEC_JPEGXL_H */
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@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"
diff --git a/libavcodec/jpegxl_parse.h b/libavcodec/jpegxl_parse.h
new file mode 100644
index 0000000000..3b7ecff07a
--- /dev/null
+++ b/libavcodec/jpegxl_parse.h
@@ -0,0 +1,497 @@
+/*
+ * JPEG XL Header Parser
+ * Copyright (c) 2023 Leo Izen <leo.izen@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
+ */
+
+#ifndef AVCODEC_JPEGXL_PARSE_H
+#define AVCODEC_JPEGXL_PARSE_H
+
+#include <stdint.h>
+
+#include "bytestream.h"
+#define UNCHECKED_BITSTREAM_READER 0
+#define BITSTREAM_READER_LE
+#include "get_bits.h"
+#include "jpegxl.h"
+
+/* read a U32(c_i + u(u_i)) */
+static av_always_inline uint32_t jxl_u32(GetBitContext *gb,
+                        uint32_t c0, uint32_t c1, uint32_t c2, uint32_t c3,
+                        uint32_t u0, uint32_t u1, uint32_t u2, uint32_t u3)
+{
+    const uint32_t constants[4] = {c0, c1, c2, c3};
+    const uint32_t ubits    [4] = {u0, u1, u2, u3};
+    uint32_t ret, choice = get_bits(gb, 2);
+
+    ret = constants[choice];
+    if (ubits[choice])
+        ret += get_bits_long(gb, ubits[choice]);
+
+    return ret;
+}
+
+static av_always_inline uint32_t jxl_enum(GetBitContext *gb)
+{
+    return jxl_u32(gb, 0, 1, 2, 18, 0, 0, 4, 6);
+}
+
+/* read a U64() */
+static uint64_t jpegxl_u64(GetBitContext *gb)
+{
+    uint64_t shift = 12, ret;
+
+    switch (get_bits(gb, 2)) {
+    case 1:
+        ret = 1 + get_bits(gb, 4);
+        break;
+    case 2:
+        ret = 17 + get_bits(gb, 8);
+        break;
+    case 3:
+        ret = get_bits(gb, 12);
+        while (get_bits1(gb)) {
+            if (shift < 60) {
+                ret |= (uint64_t)get_bits(gb, 8) << shift;
+                shift += 8;
+            } else {
+                ret |= (uint64_t)get_bits(gb, 4) << shift;
+                break;
+            }
+        }
+        break;
+    default:
+        ret = 0;
+    }
+
+    return ret;
+}
+
+static uint32_t jpegxl_width_from_ratio(uint32_t height, int ratio)
+{
+    uint64_t height64 = height; /* avoid integer overflow */
+    switch (ratio) {
+    case 1:
+        return height;
+    case 2:
+        return (uint32_t)((height64 * 12) / 10);
+    case 3:
+        return (uint32_t)((height64 * 4) / 3);
+    case 4:
+        return (uint32_t)((height64 * 3) / 2);
+    case 5:
+        return (uint32_t)((height64 * 16) / 9);
+    case 6:
+        return (uint32_t)((height64 * 5) / 4);
+    case 7:
+        return (uint32_t)(height64 * 2);
+    default:
+        break;
+    }
+
+    return 0; /* manual width */
+}
+
+/**
+ * validate a Jpeg XL Size Header
+ * @return >= 0 upon valid size, < 0 upon invalid size found
+ */
+static int jpegxl_read_size_header(GetBitContext *gb, FFJXLMetadata *meta)
+{
+    uint32_t width, height;
+
+    if (get_bits1(gb)) {
+        /* small size header */
+        height = (get_bits(gb, 5) + 1) << 3;
+        width = jpegxl_width_from_ratio(height, get_bits(gb, 3));
+        if (!width)
+            width = (get_bits(gb, 5) + 1) << 3;
+    } else {
+        /* large size header */
+        height = 1 + jxl_u32(gb, 0, 0, 0, 0, 9, 13, 18, 30);
+        width = jpegxl_width_from_ratio(height, get_bits(gb, 3));
+        if (!width)
+            width = 1 + jxl_u32(gb, 0, 0, 0, 0, 9, 13, 18, 30);
+    }
+    if (width > (1 << 18) || height > (1 << 18)
+        || (width >> 4) * (height >> 4) > (1 << 20))
+        return -1;
+
+    if (meta) {
+        meta->width = width;
+        meta->height = height;
+    }
+
+    return 0;
+}
+
+/**
+ * validate a Jpeg XL Preview Header
+ * @return >= 0 upon valid size, < 0 upon invalid size found
+ */
+static int jpegxl_read_preview_header(GetBitContext *gb)
+{
+    uint32_t width, height;
+
+    if (get_bits1(gb)) {
+        /* coded height and width divided by eight */
+        height = jxl_u32(gb, 16, 32, 1, 33, 0, 0, 5, 9) << 3;
+        width = jpegxl_width_from_ratio(height, get_bits(gb, 3));
+        if (!width)
+            width = jxl_u32(gb, 16, 32, 1, 33, 0, 0, 5, 9) << 3;
+    } else {
+        /* full height and width coded */
+        height = jxl_u32(gb, 1, 65, 321, 1345, 6, 8, 10, 12);
+        width = jpegxl_width_from_ratio(height, get_bits(gb, 3));
+        if (!width)
+            width = jxl_u32(gb, 1, 65, 321, 1345, 6, 8, 10, 12);
+    }
+    if (width > 4096 || height > 4096)
+        return -1;
+
+    return 0;
+}
+
+/**
+ * get a Jpeg XL BitDepth Header. These cannot be invalid.
+ */
+static void jpegxl_get_bit_depth(GetBitContext *gb, FFJXLMetadata *meta)
+{
+    int bit_depth;
+    if (get_bits1(gb)) {
+        /* float samples */
+        bit_depth = jxl_u32(gb, 32, 16, 24, 1, 0, 0, 0, 6); /* mantissa */
+        skip_bits_long(gb, 4); /* exponent */
+    } else {
+        /* integer samples */
+        bit_depth = jxl_u32(gb, 8, 10, 12, 1, 0, 0, 0, 6);
+    }
+    if (meta)
+        meta->bit_depth = bit_depth;
+}
+
+/**
+ * validate a Jpeg XL Extra Channel Info bundle
+ * @return >= 0 upon valid, < 0 upon invalid
+ */
+static int jpegxl_read_extra_channel_info(GetBitContext *gb, int validate_level, FFJXLMetadata *meta)
+{
+    int all_default = get_bits1(gb);
+    uint32_t type, name_len = 0;
+
+    if (!all_default) {
+        type = jxl_enum(gb);
+        if (type > 63)
+            return -1; /* enum types cannot be 64+ */
+        if (type == FF_JPEGXL_CT_BLACK && validate_level)
+            return -1;
+        jpegxl_get_bit_depth(gb, NULL);
+        jxl_u32(gb, 0, 3, 4, 1, 0, 0, 0, 3); /* dim-shift */
+        /* max of name_len is 1071 = 48 + 2^10 - 1 */
+        name_len = jxl_u32(gb, 0, 0, 16, 48, 0, 4, 5, 10);
+    } else {
+        type = FF_JPEGXL_CT_ALPHA;
+    }
+
+    /* skip over the name */
+    skip_bits_long(gb, 8 * name_len);
+
+    if (!all_default && type == FF_JPEGXL_CT_ALPHA)
+        skip_bits1(gb);
+
+    if (type == FF_JPEGXL_CT_SPOT_COLOR)
+        skip_bits_long(gb, 16 * 4);
+
+    if (type == FF_JPEGXL_CT_CFA)
+        jxl_u32(gb, 1, 0, 3, 19, 0, 2, 4, 8);
+
+    if (meta && type == FF_JPEGXL_CT_ALPHA)
+        meta->have_alpha = 1;
+
+    return 0;
+}
+
+int ff_jpegxl_parse_codestream_header(const uint8_t *buf, int buflen, int validate_level, FFJXLMetadata *meta)
+{
+    GetBitContext gbi, *gb = &gbi;
+    int all_default, extra_fields = 0;
+    int xyb_encoded = 1, have_icc_profile = 0;
+    int animation_offset = 0;
+    AVRational tb;
+    uint32_t num_extra_channels;
+    uint64_t extensions;
+    int ret;
+
+    ret = init_get_bits8(gb, buf, buflen);
+    if (ret < 0)
+        return ret;
+
+    if (get_bits_long(gb, 16) != FF_JPEGXL_CODESTREAM_SIGNATURE_LE)
+        return -1;
+
+    ret = jpegxl_read_size_header(gb, meta);
+    if (ret < 0 && validate_level)
+        return ret;
+
+    all_default = get_bits1(gb);
+    if (!all_default)
+        extra_fields = get_bits1(gb);
+
+    if (extra_fields) {
+        int orientation = get_bits(gb, 3);
+        if (orientation > 3 && meta)
+            FFSWAP(uint32_t, meta->width, meta->height);
+
+        /*
+         * intrinstic size
+         * any size header here is valid, but as it
+         * is variable length we have to read it
+         */
+        if (get_bits1(gb))
+            jpegxl_read_size_header(gb, NULL);
+
+        /* preview header */
+        if (get_bits1(gb)) {
+            ret = jpegxl_read_preview_header(gb);
+            if (ret < 0)
+                return ret;
+        }
+
+        /* animation header */
+        if (get_bits1(gb)) {
+            animation_offset = get_bits_count(gb);
+            tb.den = jxl_u32(gb, 100, 1000, 1, 1, 0, 0, 10, 30);
+            tb.num = jxl_u32(gb, 1, 1001, 1, 1, 0, 0, 8, 10);
+            jxl_u32(gb, 0, 0, 0, 0, 0, 3, 16, 32);
+            skip_bits_long(gb, 1);
+        }
+    }
+
+    if (animation_offset && meta) {
+        meta->animation_offset = animation_offset;
+        meta->timebase = tb;
+    }
+
+    if (get_bits_left(gb) < 1)
+        return AVERROR_INVALIDDATA;
+
+    if (!all_default) {
+        jpegxl_get_bit_depth(gb, meta);
+
+        /* modular_16bit_buffers must equal 1 */
+        if (!get_bits1(gb) && validate_level)
+            return -1;
+
+        num_extra_channels = jxl_u32(gb, 0, 1, 2, 1, 0, 0, 4, 12);
+        if (num_extra_channels > 4 && validate_level)
+            return -1;
+        for (uint32_t i = 0; i < num_extra_channels; i++) {
+            ret = jpegxl_read_extra_channel_info(gb, validate_level, meta);
+            if (ret < 0)
+                return ret;
+            if (get_bits_left(gb) < 1)
+                return AVERROR_INVALIDDATA;
+        }
+
+        xyb_encoded = get_bits1(gb);
+
+        /* color encoding bundle */
+        if (!get_bits1(gb)) {
+            uint32_t color_space;
+            have_icc_profile = get_bits1(gb);
+            color_space = jxl_enum(gb);
+            if (color_space > 63)
+                return -1;
+            if (meta)
+                meta->csp = color_space;
+            if (!have_icc_profile) {
+                if (color_space != FF_JPEGXL_CS_XYB) {
+                    uint32_t white_point = jxl_enum(gb);
+                    if (white_point > 63)
+                        return -1;
+                    if (meta)
+                        meta->wp = white_point;
+                    if (white_point == FF_JPEGXL_WP_CUSTOM) {
+                        /* ux and uy values */
+                        jxl_u32(gb, 0, 524288, 1048576, 2097152, 19, 19, 20, 21);
+                        jxl_u32(gb, 0, 524288, 1048576, 2097152, 19, 19, 20, 21);
+                    }
+                    if (color_space != FF_JPEGXL_CS_GRAY) {
+                        /* primaries */
+                        uint32_t primaries = jxl_enum(gb);
+                        if (primaries > 63)
+                            return -1;
+                        if (meta)
+                            meta->primaries = primaries;
+                        if (primaries == FF_JPEGXL_PR_CUSTOM) {
+                            /* ux/uy values for r,g,b */
+                            for (int i = 0; i < 6; i++) {
+                                jxl_u32(gb, 0, 524288, 1048576, 2097152, 19, 19, 20, 21);
+                                if (get_bits_left(gb) < 1)
+                                    return AVERROR_INVALIDDATA;
+                            }
+                        }
+                    }
+                }
+
+                /* transfer characteristics */
+                if (get_bits1(gb)) {
+                    /* gamma */
+                    uint32_t trc = get_bits(gb, 24);
+                    if (meta)
+                        meta->trc = trc;
+                } else {
+                    /* transfer function */
+                    uint32_t trc = jxl_enum(gb);
+                    if (trc > 63)
+                        return -1;
+                    if (meta)
+                        meta->trc = trc + (1U << 24);
+                }
+
+                /* rendering intent */
+                if (jxl_enum(gb) > 63)
+                    return -1;
+            }
+        }
+
+        /* tone mapping bundle */
+        if (extra_fields && !get_bits1(gb))
+            skip_bits_long(gb, 16 + 16 + 1 + 16);
+
+        extensions = jpegxl_u64(gb);
+        if (get_bits_left(gb) < 1)
+            return AVERROR_INVALIDDATA;
+        if (extensions) {
+            for (int i = 0; i < 64; i++) {
+                if (extensions & (UINT64_C(1) << i))
+                    jpegxl_u64(gb);
+                if (get_bits_left(gb) < 1)
+                    return AVERROR_INVALIDDATA;
+            }
+        }
+    }
+
+    if (meta) {
+        if (!meta->bit_depth)
+            meta->bit_depth = 8;
+        if (!meta->csp)
+            meta->csp = FF_JPEGXL_CS_RGB;
+        if (!meta->wp)
+            meta->wp = FF_JPEGXL_WP_D65;
+        if (!meta->primaries)
+            meta->primaries = FF_JPEGXL_PR_SRGB;
+        if (!meta->trc)
+            meta->trc = FF_JPEGXL_TR_SRGB + (1U << 24);
+    }
+
+    /* default transform */
+    if (!get_bits1(gb)) {
+        /* opsin inverse matrix */
+        if (xyb_encoded && !get_bits1(gb))
+            skip_bits_long(gb, 16 * 16);
+        /* cw_mask and default weights */
+        if (get_bits1(gb))
+            skip_bits_long(gb, 16 * 15);
+        if (get_bits1(gb))
+            skip_bits_long(gb, 16 * 55);
+        if (get_bits1(gb))
+            skip_bits_long(gb, 16 * 210);
+    }
+
+    if (!have_icc_profile) {
+        int bits_remaining = 7 - (get_bits_count(gb) - 1) % 8;
+        if (bits_remaining && get_bits(gb, bits_remaining))
+            return -1;
+    }
+
+    if (get_bits_left(gb) < 0)
+        return -1;
+
+    return 0;
+}
+
+/*
+ * copies as much of the codestream into the buffer as possible
+ * pass a shorter buflen to request less
+ * returns the number of bytes consumed from input, may be greater than input_len
+ * if the input doesn't end on an ISOBMFF-box boundary
+ */
+int ff_jpegxl_collect_codestream_header(const uint8_t *input_buffer, int input_len,
+                                        uint8_t *buffer, int buflen, int *copied)
+{
+    GetByteContext gb;
+    *copied = 0;
+    bytestream2_init(&gb, input_buffer, input_len);
+
+    while (1) {
+        uint64_t size;
+        uint32_t tag;
+        int head_size = 8;
+
+        if (bytestream2_get_bytes_left(&gb) < 16)
+            break;
+
+        size = bytestream2_get_be32(&gb);
+        if (size == 1) {
+            size = bytestream2_get_be64(&gb);
+            head_size = 16;
+        }
+        /* invalid ISOBMFF size */
+        if (size && size <= head_size)
+            return AVERROR_INVALIDDATA;
+        if (size)
+            size -= head_size;
+
+        tag = bytestream2_get_le32(&gb);
+        if (tag == MKTAG('j', 'x', 'l', 'p')) {
+            if (bytestream2_get_bytes_left(&gb) < 4)
+                break;
+            bytestream2_skip(&gb, 4);
+            if (size) {
+                if (size <= 4)
+                    return AVERROR_INVALIDDATA;
+                size -= 4;
+            }
+        }
+        /*
+         * size = 0 means "until EOF". this is legal but uncommon
+         * here we just set it to the remaining size of the probe buffer
+         */
+        if (!size)
+            size = bytestream2_get_bytes_left(&gb);
+
+        if (tag == MKTAG('j', 'x', 'l', 'c') || tag == MKTAG('j', 'x', 'l', 'p')) {
+            if (size > buflen - *copied)
+                size = buflen - *copied;
+            /*
+             * arbitrary chunking of the payload makes this memcpy hard to avoid
+             * in practice this will only be performed one or two times at most
+             */
+            *copied += bytestream2_get_buffer(&gb, buffer + *copied, size);
+        } else {
+            bytestream2_skip(&gb, size);
+        }
+        if (bytestream2_get_bytes_left(&gb) <= 0 || *copied >= buflen)
+            break;
+    }
+
+    return bytestream2_tell(&gb);
+}
+
+#endif /* AVCODEC_JPEGXL_PARSE_H */
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@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;
+    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;
+            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;
+    }
+
+    ctx->pc.frame_start_found = 1;
+
+    ret = ff_combine_frame(&ctx->pc, END_NOT_FOUND, &buf, &buf_size);
+    if (ret < 0)
+        return buf_size;
+
+    *poutbuf = buf;
+    *poutbuf_size = buf_size;
+
+    return END_NOT_FOUND;
+}
+
+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,
+};
diff --git a/libavcodec/parsers.c b/libavcodec/parsers.c
index 2c077ec3ae..aa2d0b0122 100644
--- a/libavcodec/parsers.c
+++ b/libavcodec/parsers.c
@@ -55,6 +55,7 @@ extern const AVCodecParser ff_hevc_parser;
 extern const AVCodecParser ff_hdr_parser;
 extern const AVCodecParser ff_ipu_parser;
 extern const AVCodecParser ff_jpeg2000_parser;
+extern const AVCodecParser ff_jpegxl_parser;
 extern const AVCodecParser ff_misc4_parser;
 extern const AVCodecParser ff_mjpeg_parser;
 extern const AVCodecParser ff_mlp_parser;
diff --git a/libavcodec/version.h b/libavcodec/version.h
index da6f3a84ac..9411511e04 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@
 
 #include "version_major.h"
 
-#define LIBAVCODEC_VERSION_MINOR  21
+#define LIBAVCODEC_VERSION_MINOR  22
 #define LIBAVCODEC_VERSION_MICRO 100
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
-- 
2.41.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH v2 2/3] avformat/jpegxl: remove jpegxl_probe, instead call avcodec/jpegxl_parse
  2023-06-22  0:43 [FFmpeg-devel] [PATCH v2 0/3] JXL Parser and FATE tests Leo Izen
  2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser Leo Izen
@ 2023-06-22  0:43 ` Leo Izen
  2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 3/3] fate/jpegxl_anim: add demuxer fate test for jpegxl_anim Leo Izen
  2 siblings, 0 replies; 8+ messages in thread
From: Leo Izen @ 2023-06-22  0:43 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Leo Izen

This prevents code duplication in the source form by calling the parse
code that was moved to avcodec last commit. The code will be duplicated
in binary form for shared builds (it's not that large), but for source
code it will only exist in one location now.

Signed-off-by: <leo.izen@gmail.com>
---
 libavformat/Makefile                          |   4 +-
 libavformat/img2dec.c                         |   4 +-
 libavformat/jpegxl_anim_dec.c                 | 111 +----
 .../{jpegxl_probe.h => jpegxl_parse.c}        |  21 +-
 libavformat/jpegxl_probe.c                    | 412 ------------------
 5 files changed, 25 insertions(+), 527 deletions(-)
 rename libavformat/{jpegxl_probe.h => jpegxl_parse.c} (55%)
 delete mode 100644 libavformat/jpegxl_probe.c

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 2b90b788c0..ce0755e493 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -283,7 +283,7 @@ OBJS-$(CONFIG_IMAGE_HDR_PIPE_DEMUXER)     += img2dec.o img2.o
 OBJS-$(CONFIG_IMAGE_J2K_PIPE_DEMUXER)     += img2dec.o img2.o
 OBJS-$(CONFIG_IMAGE_JPEG_PIPE_DEMUXER)    += img2dec.o img2.o
 OBJS-$(CONFIG_IMAGE_JPEGLS_PIPE_DEMUXER)  += img2dec.o img2.o
-OBJS-$(CONFIG_IMAGE_JPEGXL_PIPE_DEMUXER)  += img2dec.o img2.o jpegxl_probe.o
+OBJS-$(CONFIG_IMAGE_JPEGXL_PIPE_DEMUXER)  += img2dec.o img2.o jpegxl_parse.o
 OBJS-$(CONFIG_IMAGE_PAM_PIPE_DEMUXER)     += img2dec.o img2.o
 OBJS-$(CONFIG_IMAGE_PBM_PIPE_DEMUXER)     += img2dec.o img2.o
 OBJS-$(CONFIG_IMAGE_PCX_PIPE_DEMUXER)     += img2dec.o img2.o
@@ -320,7 +320,7 @@ OBJS-$(CONFIG_IVF_MUXER)                 += ivfenc.o
 OBJS-$(CONFIG_IVR_DEMUXER)               += rmdec.o rm.o rmsipr.o
 OBJS-$(CONFIG_JACOSUB_DEMUXER)           += jacosubdec.o subtitles.o
 OBJS-$(CONFIG_JACOSUB_MUXER)             += jacosubenc.o rawenc.o
-OBJS-$(CONFIG_JPEGXL_ANIM_DEMUXER)       += jpegxl_anim_dec.o jpegxl_probe.o
+OBJS-$(CONFIG_JPEGXL_ANIM_DEMUXER)       += jpegxl_anim_dec.o jpegxl_parse.o
 OBJS-$(CONFIG_JV_DEMUXER)                += jvdec.o
 OBJS-$(CONFIG_KUX_DEMUXER)               += flvdec.o
 OBJS-$(CONFIG_KVAG_DEMUXER)              += kvag.o
diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
index b986d3a502..316b179ffa 100644
--- a/libavformat/img2dec.c
+++ b/libavformat/img2dec.c
@@ -36,7 +36,7 @@
 #include "avio_internal.h"
 #include "internal.h"
 #include "img2.h"
-#include "jpegxl_probe.h"
+#include "libavcodec/jpegxl.h"
 #include "libavcodec/mjpeg.h"
 #include "libavcodec/vbn.h"
 #include "libavcodec/xwd.h"
@@ -850,7 +850,7 @@ static int jpegxl_probe(const AVProbeData *p)
     if (AV_RL16(b) != FF_JPEGXL_CODESTREAM_SIGNATURE_LE)
         return 0;
 #if CONFIG_IMAGE_JPEGXL_PIPE_DEMUXER
-    if (ff_jpegxl_verify_codestream_header(p->buf, p->buf_size, 1) >= 0)
+    if (ff_jpegxl_parse_codestream_header(p->buf, p->buf_size, 1, NULL) >= 0)
         return AVPROBE_SCORE_MAX - 2;
 #endif
     return 0;
diff --git a/libavformat/jpegxl_anim_dec.c b/libavformat/jpegxl_anim_dec.c
index 956b56c1d8..634fca9389 100644
--- a/libavformat/jpegxl_anim_dec.c
+++ b/libavformat/jpegxl_anim_dec.c
@@ -28,96 +28,27 @@
 #include <stdint.h>
 #include <string.h>
 
-#include "libavcodec/bytestream.h"
-#define BITSTREAM_READER_LE
-#include "libavcodec/get_bits.h"
-
+#include "libavcodec/jpegxl.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/opt.h"
 
 #include "avformat.h"
 #include "internal.h"
-#include "jpegxl_probe.h"
 
 typedef struct JXLAnimDemuxContext {
     AVBufferRef *initial;
 } JXLAnimDemuxContext;
 
-/*
- * copies as much of the codestream into the buffer as possible
- * pass a shorter buflen to request less
- * returns the number of bytes consumed from input, may be greater than input_len
- * if the input doesn't end on an ISOBMFF-box boundary
- */
-static int jpegxl_collect_codestream_header(const uint8_t *input_buffer, int input_len,
-                                            uint8_t *buffer, int buflen, int *copied) {
-    GetByteContext gb;
-    *copied = 0;
-    bytestream2_init(&gb, input_buffer, input_len);
-
-    while (1) {
-        uint64_t size;
-        uint32_t tag;
-        int head_size = 8;
-
-        if (bytestream2_get_bytes_left(&gb) < 16)
-            break;
-
-        size = bytestream2_get_be32(&gb);
-        if (size == 1) {
-            size = bytestream2_get_be64(&gb);
-            head_size = 16;
-        }
-        /* invalid ISOBMFF size */
-        if (size && size <= head_size)
-            return AVERROR_INVALIDDATA;
-        if (size)
-            size -= head_size;
-
-        tag = bytestream2_get_le32(&gb);
-        if (tag == MKTAG('j', 'x', 'l', 'p')) {
-            if (bytestream2_get_bytes_left(&gb) < 4)
-                break;
-            bytestream2_skip(&gb, 4);
-            if (size) {
-                if (size <= 4)
-                    return AVERROR_INVALIDDATA;
-                size -= 4;
-            }
-        }
-        /*
-         * size = 0 means "until EOF". this is legal but uncommon
-         * here we just set it to the remaining size of the probe buffer
-         */
-        if (!size)
-            size = bytestream2_get_bytes_left(&gb);
-
-        if (tag == MKTAG('j', 'x', 'l', 'c') || tag == MKTAG('j', 'x', 'l', 'p')) {
-            if (size > buflen - *copied)
-                size = buflen - *copied;
-            /*
-             * arbitrary chunking of the payload makes this memcpy hard to avoid
-             * in practice this will only be performed one or two times at most
-             */
-            *copied += bytestream2_get_buffer(&gb, buffer + *copied, size);
-        } else {
-            bytestream2_skip(&gb, size);
-        }
-        if (bytestream2_get_bytes_left(&gb) <= 0 || *copied >= buflen)
-            break;
-    }
-
-    return bytestream2_tell(&gb);
-}
-
 static int jpegxl_anim_probe(const AVProbeData *p)
 {
     uint8_t buffer[4096 + AV_INPUT_BUFFER_PADDING_SIZE];
-    int copied;
+    int copied, ret;
+    FFJXLMetadata meta = { 0 };
 
     /* this is a raw codestream */
     if (AV_RL16(p->buf) == FF_JPEGXL_CODESTREAM_SIGNATURE_LE) {
-        if (ff_jpegxl_verify_codestream_header(p->buf, p->buf_size, 1) >= 1)
+        ret = ff_jpegxl_parse_codestream_header(p->buf, p->buf_size, 1, &meta);
+        if (ret >= 0 && meta.animation_offset > 0)
             return AVPROBE_SCORE_MAX;
 
         return 0;
@@ -127,10 +58,13 @@ static int jpegxl_anim_probe(const AVProbeData *p)
     if (AV_RL64(p->buf) != FF_JPEGXL_CONTAINER_SIGNATURE_LE)
         return 0;
 
-    if (jpegxl_collect_codestream_header(p->buf, p->buf_size, buffer, sizeof(buffer) - AV_INPUT_BUFFER_PADDING_SIZE, &copied) <= 0 || copied <= 0)
+    if (ff_jpegxl_collect_codestream_header(p->buf, p->buf_size, buffer,
+            sizeof(buffer) - AV_INPUT_BUFFER_PADDING_SIZE, &copied) <= 0
+            || copied <= 0)
         return 0;
 
-    if (ff_jpegxl_verify_codestream_header(buffer, copied, 0) >= 1)
+    ret = ff_jpegxl_parse_codestream_header(buffer, copied, 0, &meta);
+    if (ret >= 0 && meta.animation_offset > 0)
         return AVPROBE_SCORE_MAX;
 
     return 0;
@@ -141,13 +75,10 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
     JXLAnimDemuxContext *ctx = s->priv_data;
     AVIOContext *pb = s->pb;
     AVStream *st;
-    int offset = 0;
     uint8_t head[256 + AV_INPUT_BUFFER_PADDING_SIZE];
     const int sizeofhead = sizeof(head) - AV_INPUT_BUFFER_PADDING_SIZE;
-    int headsize = 0;
-    int ctrl;
-    AVRational tb;
-    GetBitContext gbi, *gb = &gbi;
+    int headsize = 0, ret;
+    FFJXLMetadata meta = { 0 };
 
     uint64_t sig16 = avio_rl16(pb);
     if (sig16 == FF_JPEGXL_CODESTREAM_SIGNATURE_LE) {
@@ -183,7 +114,7 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
                 if (av_buffer_realloc(&ctx->initial, ctx->initial->size + read) < 0)
                     return AVERROR(ENOMEM);
             }
-            jpegxl_collect_codestream_header(buf, read, head + headsize, sizeofhead - headsize, &copied);
+            ff_jpegxl_collect_codestream_header(buf, read, head + headsize, sizeofhead - headsize, &copied);
             memcpy(ctx->initial->data + (ctx->initial->size - read), buf, read);
             headsize += copied;
             if (headsize >= sizeofhead || read < sizeof(buf))
@@ -191,12 +122,9 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
         }
     }
     /* offset in bits of the animation header */
-    offset = ff_jpegxl_verify_codestream_header(head, headsize, 0);
-    if (offset <= 0)
-        return AVERROR_INVALIDDATA;
-    if (init_get_bits8(gb, head, headsize) < 0)
+    ret = ff_jpegxl_parse_codestream_header(head, headsize, 0, &meta);
+    if (ret < 0 || meta.animation_offset <= 0)
         return AVERROR_INVALIDDATA;
-    skip_bits_long(gb, offset);
 
     st = avformat_new_stream(s, NULL);
     if (!st)
@@ -204,11 +132,8 @@ static int jpegxl_anim_read_header(AVFormatContext *s)
 
     st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
     st->codecpar->codec_id   = AV_CODEC_ID_JPEGXL;
-    ctrl = get_bits(gb, 2);
-    tb.den = (const uint32_t[]){100, 1000, 1, 1}[ctrl] + get_bits_long(gb, (const uint32_t[]){0, 0, 10, 30}[ctrl]);
-    ctrl = get_bits(gb, 2);
-    tb.num = (const uint32_t[]){1, 1001, 1, 1}[ctrl] + get_bits_long(gb, (const uint32_t[]){0, 0, 8, 10}[ctrl]);
-    avpriv_set_pts_info(st, 1, tb.num, tb.den);
+    avpriv_set_pts_info(st, 1, meta.timebase.num, meta.timebase.den);
+    ffstream(st)->need_parsing = AVSTREAM_PARSE_HEADERS;
 
     return 0;
 }
@@ -265,7 +190,7 @@ const AVInputFormat ff_jpegxl_anim_demuxer = {
     .read_packet    = jpegxl_anim_read_packet,
     .read_close     = jpegxl_anim_close,
     .flags_internal = FF_FMT_INIT_CLEANUP,
-    .flags          = AVFMT_GENERIC_INDEX,
+    .flags          = AVFMT_GENERIC_INDEX | AVFMT_NOTIMESTAMPS,
     .mime_type      = "image/jxl",
     .extensions     = "jxl",
 };
diff --git a/libavformat/jpegxl_probe.h b/libavformat/jpegxl_parse.c
similarity index 55%
rename from libavformat/jpegxl_probe.h
rename to libavformat/jpegxl_parse.c
index 496445fbce..821d329c5c 100644
--- a/libavformat/jpegxl_probe.h
+++ b/libavformat/jpegxl_parse.c
@@ -1,6 +1,6 @@
 /*
- * Jpeg XL header verification
- * Copyright (c) 2022 Leo Izen <leo.izen@gmail.com>
+ * JPEG XL Header Parser
+ * Copyright (c) 2023 Leo Izen <leo.izen@gmail.com>
  *
  * This file is part of FFmpeg.
  *
@@ -19,19 +19,4 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-#ifndef AVFORMAT_JPEGXL_PROBE_H
-#define AVFORMAT_JPEGXL_PROBE_H
-
-#include <stdint.h>
-
-#define FF_JPEGXL_CODESTREAM_SIGNATURE_LE 0x0aff
-#define FF_JPEGXL_CONTAINER_SIGNATURE_LE 0x204c584a0c000000
-
-/**
- * @brief verify that a codestream header is valid
- * @return Negative upon error, 0 upon verifying that the codestream is not animated,
- *         and 1 upon verifying that it is animated
- */
-int ff_jpegxl_verify_codestream_header(const uint8_t *buf, int buflen, int validate_level);
-
-#endif /* AVFORMAT_JPEGXL_PROBE_H */
+ #include "libavcodec/jpegxl_parse.h"
diff --git a/libavformat/jpegxl_probe.c b/libavformat/jpegxl_probe.c
deleted file mode 100644
index 88492cb772..0000000000
--- a/libavformat/jpegxl_probe.c
+++ /dev/null
@@ -1,412 +0,0 @@
-/*
- * Jpeg XL header verification
- * Copyright (c) 2022 Leo Izen <leo.izen@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_probe.h"
-
-#define UNCHECKED_BITSTREAM_READER 0
-#define BITSTREAM_READER_LE
-#include "libavcodec/get_bits.h"
-
-enum JpegXLExtraChannelType {
-    FF_JPEGXL_CT_ALPHA = 0,
-    FF_JPEGXL_CT_DEPTH,
-    FF_JPEGXL_CT_SPOT_COLOR,
-    FF_JPEGXL_CT_SELECTION_MASK,
-    FF_JPEGXL_CT_BLACK,
-    FF_JPEGXL_CT_CFA,
-    FF_JPEGXL_CT_THERMAL,
-    FF_JPEGXL_CT_NON_OPTIONAL = 15,
-    FF_JPEGXL_CT_OPTIONAL
-};
-
-enum JpegXLColorSpace {
-    FF_JPEGXL_CS_RGB = 0,
-    FF_JPEGXL_CS_GRAY,
-    FF_JPEGXL_CS_XYB,
-    FF_JPEGXL_CS_UNKNOWN
-};
-
-enum JpegXLWhitePoint {
-    FF_JPEGXL_WP_D65 = 1,
-    FF_JPEGXL_WP_CUSTOM,
-    FF_JPEGXL_WP_E = 10,
-    FF_JPEGXL_WP_DCI = 11
-};
-
-enum JpegXLPrimaries {
-    FF_JPEGXL_PR_SRGB = 1,
-    FF_JPEGXL_PR_CUSTOM,
-    FF_JPEGXL_PR_2100 = 9,
-    FF_JPEGXL_PR_P3 = 11,
-};
-
-/* read a U32(c_i + u(u_i)) */
-static av_always_inline uint32_t jxl_u32(GetBitContext *gb,
-                        uint32_t c0, uint32_t c1, uint32_t c2, uint32_t c3,
-                        uint32_t u0, uint32_t u1, uint32_t u2, uint32_t u3)
-{
-    const uint32_t constants[4] = {c0, c1, c2, c3};
-    const uint32_t ubits    [4] = {u0, u1, u2, u3};
-    uint32_t ret, choice = get_bits(gb, 2);
-
-    ret = constants[choice];
-    if (ubits[choice])
-        ret += get_bits_long(gb, ubits[choice]);
-
-    return ret;
-}
-
-static av_always_inline uint32_t jxl_enum(GetBitContext *gb)
-{
-    return jxl_u32(gb, 0, 1, 2, 18, 0, 0, 4, 6);
-}
-
-/* read a U64() */
-static uint64_t jpegxl_u64(GetBitContext *gb)
-{
-    uint64_t shift = 12, ret;
-
-    switch (get_bits(gb, 2)) {
-    case 0:
-        ret = 0;
-        break;
-    case 1:
-        ret = 1 + get_bits(gb, 4);
-        break;
-    case 2:
-        ret = 17 + get_bits(gb, 8);
-        break;
-    case 3:
-        ret = get_bits(gb, 12);
-        while (get_bits1(gb)) {
-            if (shift < 60) {
-                ret |= (uint64_t)get_bits(gb, 8) << shift;
-                shift += 8;
-            } else {
-                ret |= (uint64_t)get_bits(gb, 4) << shift;
-                break;
-            }
-        }
-        break;
-    }
-
-    return ret;
-}
-
-static uint32_t jpegxl_width_from_ratio(uint32_t height, int ratio)
-{
-    uint64_t height64 = height; /* avoid integer overflow */
-    switch (ratio) {
-    case 1:
-        return height;
-    case 2:
-        return (uint32_t)((height64 * 12) / 10);
-    case 3:
-        return (uint32_t)((height64 * 4) / 3);
-    case 4:
-        return (uint32_t)((height64 * 3) / 2);
-    case 5:
-        return (uint32_t)((height64 * 16) / 9);
-    case 6:
-        return (uint32_t)((height64 * 5) / 4);
-    case 7:
-        return (uint32_t)(height64 * 2);
-    default:
-        break;
-    }
-
-    return 0; /* manual width */
-}
-
-/**
- * validate a Jpeg XL Size Header
- * @return >= 0 upon valid size, < 0 upon invalid size found
- */
-static int jpegxl_read_size_header(GetBitContext *gb)
-{
-    uint32_t width, height;
-
-    if (get_bits1(gb)) {
-        /* small size header */
-        height = (get_bits(gb, 5) + 1) << 3;
-        width = jpegxl_width_from_ratio(height, get_bits(gb, 3));
-        if (!width)
-            width = (get_bits(gb, 5) + 1) << 3;
-    } else {
-        /* large size header */
-        height = 1 + jxl_u32(gb, 0, 0, 0, 0, 9, 13, 18, 30);
-        width = jpegxl_width_from_ratio(height, get_bits(gb, 3));
-        if (!width)
-            width = 1 + jxl_u32(gb, 0, 0, 0, 0, 9, 13, 18, 30);
-    }
-    if (width > (1 << 18) || height > (1 << 18)
-        || (width >> 4) * (height >> 4) > (1 << 20))
-        return -1;
-
-    return 0;
-}
-
-/**
- * validate a Jpeg XL Preview Header
- * @return >= 0 upon valid size, < 0 upon invalid size found
- */
-static int jpegxl_read_preview_header(GetBitContext *gb)
-{
-    uint32_t width, height;
-
-    if (get_bits1(gb)) {
-        /* coded height and width divided by eight */
-        height = jxl_u32(gb, 16, 32, 1, 33, 0, 0, 5, 9) << 3;
-        width = jpegxl_width_from_ratio(height, get_bits(gb, 3));
-        if (!width)
-            width = jxl_u32(gb, 16, 32, 1, 33, 0, 0, 5, 9) << 3;
-    } else {
-        /* full height and width coded */
-        height = jxl_u32(gb, 1, 65, 321, 1345, 6, 8, 10, 12);
-        width = jpegxl_width_from_ratio(height, get_bits(gb, 3));
-        if (!width)
-            width = jxl_u32(gb, 1, 65, 321, 1345, 6, 8, 10, 12);
-    }
-    if (width > 4096 || height > 4096)
-        return -1;
-
-    return 0;
-}
-
-/**
- * skip a Jpeg XL BitDepth Header. These cannot be invalid.
- */
-static void jpegxl_skip_bit_depth(GetBitContext *gb)
-{
-    if (get_bits1(gb)) {
-        /* float samples */
-        jxl_u32(gb, 32, 16, 24, 1, 0, 0, 0, 6); /* mantissa */
-        skip_bits_long(gb, 4); /* exponent */
-    } else {
-        /* integer samples */
-        jxl_u32(gb, 8, 10, 12, 1, 0, 0, 0, 6);
-    }
-}
-
-/**
- * validate a Jpeg XL Extra Channel Info bundle
- * @return >= 0 upon valid, < 0 upon invalid
- */
-static int jpegxl_read_extra_channel_info(GetBitContext *gb, int validate_level)
-{
-    int all_default = get_bits1(gb);
-    uint32_t type, name_len = 0;
-
-    if (!all_default) {
-        type = jxl_enum(gb);
-        if (type > 63)
-            return -1; /* enum types cannot be 64+ */
-        if (type == FF_JPEGXL_CT_BLACK && validate_level)
-            return -1;
-        jpegxl_skip_bit_depth(gb);
-        jxl_u32(gb, 0, 3, 4, 1, 0, 0, 0, 3); /* dim-shift */
-        /* max of name_len is 1071 = 48 + 2^10 - 1 */
-        name_len = jxl_u32(gb, 0, 0, 16, 48, 0, 4, 5, 10);
-    } else {
-        type = FF_JPEGXL_CT_ALPHA;
-    }
-
-    /* skip over the name */
-    skip_bits_long(gb, 8 * name_len);
-
-    if (!all_default && type == FF_JPEGXL_CT_ALPHA)
-        skip_bits1(gb);
-
-    if (type == FF_JPEGXL_CT_SPOT_COLOR)
-        skip_bits_long(gb, 16 * 4);
-
-    if (type == FF_JPEGXL_CT_CFA)
-        jxl_u32(gb, 1, 0, 3, 19, 0, 2, 4, 8);
-
-    return 0;
-}
-
-int ff_jpegxl_verify_codestream_header(const uint8_t *buf, int buflen, int validate_level)
-{
-    GetBitContext gbi, *gb = &gbi;
-    int all_default, extra_fields = 0;
-    int xyb_encoded = 1, have_icc_profile = 0;
-    int animation_offset = 0;
-    uint32_t num_extra_channels;
-    uint64_t extensions;
-    int ret;
-
-    ret = init_get_bits8(gb, buf, buflen);
-    if (ret < 0)
-        return ret;
-
-    if (get_bits_long(gb, 16) != FF_JPEGXL_CODESTREAM_SIGNATURE_LE)
-        return -1;
-
-    if ((ret = jpegxl_read_size_header(gb)) < 0 && validate_level)
-        return ret;
-
-    all_default = get_bits1(gb);
-    if (!all_default)
-        extra_fields = get_bits1(gb);
-
-    if (extra_fields) {
-        skip_bits_long(gb, 3); /* orientation */
-
-        /*
-         * intrinstic size
-         * any size header here is valid, but as it
-         * is variable length we have to read it
-         */
-        if (get_bits1(gb))
-            jpegxl_read_size_header(gb);
-
-        /* preview header */
-        if (get_bits1(gb)) {
-            ret = jpegxl_read_preview_header(gb);
-            if (ret < 0)
-                return ret;
-        }
-
-        /* animation header */
-        if (get_bits1(gb)) {
-            animation_offset = get_bits_count(gb);
-            jxl_u32(gb, 100, 1000, 1, 1, 0, 0, 10, 30);
-            jxl_u32(gb, 1, 1001, 1, 1, 0, 0, 8, 10);
-            jxl_u32(gb, 0, 0, 0, 0, 0, 3, 16, 32);
-            skip_bits_long(gb, 1);
-        }
-    }
-    if (get_bits_left(gb) < 1)
-        return AVERROR_INVALIDDATA;
-
-    if (!all_default) {
-        jpegxl_skip_bit_depth(gb);
-
-        /* modular_16bit_buffers must equal 1 */
-        if (!get_bits1(gb) && validate_level)
-            return -1;
-
-        num_extra_channels = jxl_u32(gb, 0, 1, 2, 1, 0, 0, 4, 12);
-        if (num_extra_channels > 4 && validate_level)
-            return -1;
-        for (uint32_t i = 0; i < num_extra_channels; i++) {
-            ret = jpegxl_read_extra_channel_info(gb, validate_level);
-            if (ret < 0)
-                return ret;
-            if (get_bits_left(gb) < 1)
-                return AVERROR_INVALIDDATA;
-        }
-
-        xyb_encoded = get_bits1(gb);
-
-        /* color encoding bundle */
-        if (!get_bits1(gb)) {
-            uint32_t color_space;
-            have_icc_profile = get_bits1(gb);
-            color_space = jxl_enum(gb);
-            if (color_space > 63)
-                return -1;
-
-            if (!have_icc_profile) {
-                if (color_space != FF_JPEGXL_CS_XYB) {
-                    uint32_t white_point = jxl_enum(gb);
-                    if (white_point > 63)
-                        return -1;
-                    if (white_point == FF_JPEGXL_WP_CUSTOM) {
-                        /* ux and uy values */
-                        jxl_u32(gb, 0, 524288, 1048576, 2097152, 19, 19, 20, 21);
-                        jxl_u32(gb, 0, 524288, 1048576, 2097152, 19, 19, 20, 21);
-                    }
-                    if (color_space != FF_JPEGXL_CS_GRAY) {
-                        /* primaries */
-                        uint32_t primaries = jxl_enum(gb);
-                        if (primaries > 63)
-                            return -1;
-                        if (primaries == FF_JPEGXL_PR_CUSTOM) {
-                            /* ux/uy values for r,g,b */
-                            for (int i = 0; i < 6; i++) {
-                                jxl_u32(gb, 0, 524288, 1048576, 2097152, 19, 19, 20, 21);
-                                if (get_bits_left(gb) < 1)
-                                    return AVERROR_INVALIDDATA;
-                            }
-                        }
-                    }
-                }
-
-                /* transfer characteristics */
-                if (get_bits1(gb)) {
-                    /* gamma */
-                    skip_bits_long(gb, 24);
-                } else {
-                    /* transfer function */
-                    if (jxl_enum(gb) > 63)
-                        return -1;
-                }
-
-                /* rendering intent */
-                if (jxl_enum(gb) > 63)
-                    return -1;
-            }
-        }
-
-        /* tone mapping bundle */
-        if (extra_fields && !get_bits1(gb))
-            skip_bits_long(gb, 16 + 16 + 1 + 16);
-
-        extensions = jpegxl_u64(gb);
-        if (get_bits_left(gb) < 1)
-            return AVERROR_INVALIDDATA;
-        if (extensions) {
-            for (int i = 0; i < 64; i++) {
-                if (extensions & (UINT64_C(1) << i))
-                    jpegxl_u64(gb);
-                if (get_bits_left(gb) < 1)
-                    return AVERROR_INVALIDDATA;
-            }
-        }
-    }
-
-    /* default transform */
-    if (!get_bits1(gb)) {
-        /* opsin inverse matrix */
-        if (xyb_encoded && !get_bits1(gb))
-            skip_bits_long(gb, 16 * 16);
-        /* cw_mask and default weights */
-        if (get_bits1(gb))
-            skip_bits_long(gb, 16 * 15);
-        if (get_bits1(gb))
-            skip_bits_long(gb, 16 * 55);
-        if (get_bits1(gb))
-            skip_bits_long(gb, 16 * 210);
-    }
-
-    if (!have_icc_profile) {
-        int bits_remaining = 7 - (get_bits_count(gb) - 1) % 8;
-        if (bits_remaining && get_bits(gb, bits_remaining))
-            return -1;
-    }
-
-    if (get_bits_left(gb) < 0)
-        return -1;
-
-    return animation_offset;
-}
-- 
2.41.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH v2 3/3] fate/jpegxl_anim: add demuxer fate test for jpegxl_anim
  2023-06-22  0:43 [FFmpeg-devel] [PATCH v2 0/3] JXL Parser and FATE tests Leo Izen
  2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser Leo Izen
  2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 2/3] avformat/jpegxl: remove jpegxl_probe, instead call avcodec/jpegxl_parse Leo Izen
@ 2023-06-22  0:43 ` Leo Izen
  2 siblings, 0 replies; 8+ messages in thread
From: Leo Izen @ 2023-06-22  0:43 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Leo Izen

Adds a fate test for the jpegxl_anim demuxer, that should allow testing
for true positives and false positives for animated jpegxl files. Note
that two of the test cases are not animated, in order to help sort out
false positives.

Signed-off-by: <leo.izen@gmail.com>
---
 tests/Makefile                         |  1 +
 tests/fate/jxl.mak                     | 16 ++++++++++++++++
 tests/ref/fate/jxl-anim-demux-belgium  |  6 ++++++
 tests/ref/fate/jxl-anim-demux-icos4d   |  6 ++++++
 tests/ref/fate/jxl-anim-demux-lenna256 |  6 ++++++
 tests/ref/fate/jxl-anim-demux-newton   |  6 ++++++
 6 files changed, 41 insertions(+)
 create mode 100644 tests/fate/jxl.mak
 create mode 100644 tests/ref/fate/jxl-anim-demux-belgium
 create mode 100644 tests/ref/fate/jxl-anim-demux-icos4d
 create mode 100644 tests/ref/fate/jxl-anim-demux-lenna256
 create mode 100644 tests/ref/fate/jxl-anim-demux-newton

diff --git a/tests/Makefile b/tests/Makefile
index e09f30a0fc..7b80762e81 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -201,6 +201,7 @@ include $(SRC_PATH)/tests/fate/image.mak
 include $(SRC_PATH)/tests/fate/imf.mak
 include $(SRC_PATH)/tests/fate/indeo.mak
 include $(SRC_PATH)/tests/fate/jpeg2000.mak
+include $(SRC_PATH)/tests/fate/jxl.mak
 include $(SRC_PATH)/tests/fate/libavcodec.mak
 include $(SRC_PATH)/tests/fate/libavdevice.mak
 include $(SRC_PATH)/tests/fate/libavformat.mak
diff --git a/tests/fate/jxl.mak b/tests/fate/jxl.mak
new file mode 100644
index 0000000000..057d3be0e1
--- /dev/null
+++ b/tests/fate/jxl.mak
@@ -0,0 +1,16 @@
+# These two are animated JXL files
+FATE_JPEGXL_ANIM_DEMUX += fate-jxl-anim-demux-newton
+fate-jxl-anim-demux-newton: CMD = framecrc -i $(TARGET_SAMPLES)/jxl/newton.jxl -c copy
+FATE_JPEGXL_ANIM_DEMUX += fate-jxl-anim-demux-icos4d
+fate-jxl-anim-demux-icos4d: CMD = framecrc -i $(TARGET_SAMPLES)/jxl/icos4d.jxl -c copy
+
+# These two are not animated JXL. They are here to check false positives.
+FATE_JPEGXL_ANIM_DEMUX += fate-jxl-anim-demux-belgium
+fate-jxl-anim-demux-belgium: CMD = framecrc -i $(TARGET_SAMPLES)/jxl/belgium.jxl -c copy
+FATE_JPEGXL_ANIM_DEMUX += fate-jxl-anim-demux-lenna256
+fate-jxl-anim-demux-lenna256: CMD = framecrc -i $(TARGET_SAMPLES)/jxl/lenna-256.jxl -c copy
+
+FATE_JPEGXL_ANIM_DEMUX += $(FATE_JPEGXL_ANIM_DEMUX-yes)
+
+FATE_SAMPLES_FFMPEG-$(call FRAMECRC, JPEGXL_ANIM) += $(FATE_JPEGXL_ANIM_DEMUX)
+fate-jxl-anim-demux: $(FATE_JPEGXL_ANIM_DEMUX)
diff --git a/tests/ref/fate/jxl-anim-demux-belgium b/tests/ref/fate/jxl-anim-demux-belgium
new file mode 100644
index 0000000000..b2fe5035ac
--- /dev/null
+++ b/tests/ref/fate/jxl-anim-demux-belgium
@@ -0,0 +1,6 @@
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: jpegxl
+#dimensions 0: 768x512
+#sar 0: 0/1
+0,          0,          0,        1,       32, 0xa2930a20
diff --git a/tests/ref/fate/jxl-anim-demux-icos4d b/tests/ref/fate/jxl-anim-demux-icos4d
new file mode 100644
index 0000000000..eff6ff1f1b
--- /dev/null
+++ b/tests/ref/fate/jxl-anim-demux-icos4d
@@ -0,0 +1,6 @@
+#tb 0: 1/1000
+#media_type 0: video
+#codec_id 0: jpegxl
+#dimensions 0: 48x48
+#sar 0: 0/1
+0,          0,          0,        0,    67898, 0x53b6516b
diff --git a/tests/ref/fate/jxl-anim-demux-lenna256 b/tests/ref/fate/jxl-anim-demux-lenna256
new file mode 100644
index 0000000000..99233d612a
--- /dev/null
+++ b/tests/ref/fate/jxl-anim-demux-lenna256
@@ -0,0 +1,6 @@
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: jpegxl
+#dimensions 0: 256x256
+#sar 0: 0/1
+0,          0,          0,        1,     8088, 0xbbfea9bd
diff --git a/tests/ref/fate/jxl-anim-demux-newton b/tests/ref/fate/jxl-anim-demux-newton
new file mode 100644
index 0000000000..6fcb85c41e
--- /dev/null
+++ b/tests/ref/fate/jxl-anim-demux-newton
@@ -0,0 +1,6 @@
+#tb 0: 1/1000
+#media_type 0: video
+#codec_id 0: jpegxl
+#dimensions 0: 128x96
+#sar 0: 0/1
+0,          0,          0,        0,    43376, 0xb2296182
-- 
2.41.0

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser
  2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser Leo Izen
@ 2023-06-22  0:59   ` James Almer
  2023-06-22  1:06     ` Leo Izen
  2023-06-22 14:19   ` James Almer
  1 sibling, 1 reply; 8+ messages in thread
From: James Almer @ 2023-06-22  0:59 UTC (permalink / raw)
  To: ffmpeg-devel

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@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@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,
> +};

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser
  2023-06-22  0:59   ` James Almer
@ 2023-06-22  1:06     ` Leo Izen
  2023-06-22  1:21       ` James Almer
  0 siblings, 1 reply; 8+ messages in thread
From: Leo Izen @ 2023-06-22  1:06 UTC (permalink / raw)
  To: ffmpeg-devel

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.izen@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.izen@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-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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser
  2023-06-22  1:06     ` Leo Izen
@ 2023-06-22  1:21       ` James Almer
  0 siblings, 0 replies; 8+ messages in thread
From: James Almer @ 2023-06-22  1:21 UTC (permalink / raw)
  To: ffmpeg-devel

On 6/21/2023 10:06 PM, Leo Izen wrote:
> 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.izen@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.

Create a jpegxl_parse.c file in libavformat with the only line being

#include "libavcodec/jpegxl_parse.c"

Then add jpegxl_parse.o to SHLIBOBJS-$(CONFIG_JPEGXL_ANIM_DEMUXER) in 
libavformat/Makefile.
You can remove jpegxl_parse.o from libavcodec's STLIBOBJS list too.
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser
  2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser Leo Izen
  2023-06-22  0:59   ` James Almer
@ 2023-06-22 14:19   ` James Almer
  1 sibling, 0 replies; 8+ messages in thread
From: James Almer @ 2023-06-22 14:19 UTC (permalink / raw)
  To: ffmpeg-devel

On 6/21/2023 9:43 PM, Leo Izen wrote:
> +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;

Set s->width and s->height instead. The generic code will set the avctx 
fields if required.
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-06-22 14:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-22  0:43 [FFmpeg-devel] [PATCH v2 0/3] JXL Parser and FATE tests Leo Izen
2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 1/3] avcodec/jpegxl_parser: add JPEG XL parser Leo Izen
2023-06-22  0:59   ` James Almer
2023-06-22  1:06     ` Leo Izen
2023-06-22  1:21       ` James Almer
2023-06-22 14:19   ` James Almer
2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 2/3] avformat/jpegxl: remove jpegxl_probe, instead call avcodec/jpegxl_parse Leo Izen
2023-06-22  0:43 ` [FFmpeg-devel] [PATCH v2 3/3] fate/jpegxl_anim: add demuxer fate test for jpegxl_anim Leo Izen

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git