jxlsave produces broken images with libjxl 0.7rc

[kleisauke]

Reproducer:

$ curl -LO https://jpegxl.info/logo.jxl
$ vips avg logo.jxl
109.279851
$ vips copy logo.jxl x.jxl
$ vips avg x.jxl
jxlload: error JxlDecoderProcessInput

Here's an simple Dockerfile to reproduce this:

Dockerfile

# Build with:
# docker build -t vips-jxl-0.7 .
# docker run --rm -it --entrypoint bash vips-jxl-0.7
# curl -LO https://jpegxl.info/logo.jxl
# vips avg logo.jxl
# vips copy logo.jxl x.jxl
# vips avg x.jxl
FROM fedora:36

RUN \
  dnf update -y && \
  # Build dependencies
  dnf install -y \
    cmake \
    curl \
    gcc-c++ \
    git \
    meson \
    ninja-build \
    python3 \
    && \
  # Runtime dependencies
  dnf install -y --setopt=tsflags=nodocs \
    brotli-devel \
    expat-devel \
    glib2-devel \
    lcms2-devel \
    libexif-devel \
    libjpeg-devel \
    libpng-devel \
    libtiff-devel \
    libwebp-devel \
    orc-devel

# highway
RUN \
  curl -Ls https://github.com/google/highway/archive/refs/tags/1.0.0.tar.gz | tar xzC . && \
  cd highway-1.0.0/ && \
  cmake -S . -B _build -GNinja \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=/usr \
    -DBUILD_TESTING=FALSE \
    -DHWY_ENABLE_CONTRIB=FALSE \
    -DHWY_ENABLE_EXAMPLES=FALSE \
    && \
  cmake --build _build -- -j$(nproc) && \
  cmake --install _build

# libjxl
RUN \
  curl -Ls https://github.com/libjxl/libjxl/archive/refs/tags/v0.7rc.tar.gz | tar xzC . && \
  cd libjxl-0.7rc/ && \
  cmake -S . -B _build -GNinja \
    -DCMAKE_BUILD_TYPE=Release \
    -DCMAKE_INSTALL_PREFIX=/usr \
    -DBUILD_TESTING=FALSE \
    -DJPEGXL_ENABLE_SJPEG=FALSE \
    -DJPEGXL_ENABLE_OPENEXR=FALSE \
    -DJPEGXL_ENABLE_SKCMS=FALSE \
    -DJPEGXL_FORCE_SYSTEM_BROTLI=TRUE \
    -DJPEGXL_FORCE_SYSTEM_LCMS2=TRUE \
    -DJPEGXL_FORCE_SYSTEM_HWY=TRUE \
    && \
  cmake --build _build -- -j$(nproc) && \
  cmake --install _build

# libvips
RUN \
  git clone -b master --single-branch https://github.com/libvips/libvips.git vips-master && \
  cd vips-master && \
  meson build --prefix=/usr --buildtype=release -Dintrospection=false && \
  meson compile -C build && \
  meson install -C build

[jcupitt]

Confirmed, I see failure on ubuntu as well. There are a 8 deprecation warnings during libvips build, and a series of errors on jxl decompress and recompress.

Shall I have a stab at updating libvips for libjxl.07, Kleis? Or do you have this in hand?

[kleisauke]

I won't be able to look at this until next weekend, so feel free to investigate/fix this earlier than that.

[kleisauke]

Actually, I found a very simple fix, I'm working on a PR now.

[jcupitt]

Oh nice!

Do you think we should still support 0.6? I support 8.13 ought to not change dependencies too radically during its lifetime, so maybe we should only remove 0.6 support in 8.14.

[jcupitt]

Encode and decode now work, but I still see:

./lib/jxl/decode.cc:1640: already set input, use JxlDecoderReleaseInput first
WARNING: CPU supports 1a00 but software requires 2000000000000000

I expect you saw.

The first one looks like a simple fix, but the second one is a bit puzzling to me.

[kleisauke]

I think it's fine to remove support for libjxl <= 0.6 in libvips 8.14, assuming that libjxl 0.7 will be shortly released.

[kleisauke]

The highway warning should be fixed with commit google/highway@9b6b473, which didn't make it into the 1.0.0 release, see: google/highway#882 (comment)

I'm not sure about that first warning (or is it an error?), I haven't encountered that. Did you build libjxl in release mode?

[jcupitt]

No, debug mode.

[jcupitt]

It's here:

$ gdb vips
Reading symbols from vips...
(gdb) break decode.cc:1640
No source file named decode.cc.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (decode.cc:1640) pending.
(gdb) run copy logo.jxl x.png
Starting program: /home/john/vips/bin/vips copy logo.jxl x.png

Thread 35 "pool-vips" hit Breakpoint 1, JxlDecoderSetInput (dec=0x5555556a4410, data=0x55555569e9b0 "\377\n:\037\001\221\b\006\001", size=117) at /home/john/GIT/libjxl/lib/jxl/decode.cc:1640
1640	    return JXL_API_ERROR("already set input, use JxlDecoderReleaseInput first");
(gdb) where
#0  JxlDecoderSetInput(JxlDecoder*, uint8_t const*, size_t)
    (dec=0x5555556a4410, data=0x55555569e9b0 "\377\n:\037\001\221\b\006\001", size=117) at /home/john/GIT/libjxl/lib/jxl/decode.cc:1640
#1  0x00007ffff1c07b37 in vips_foreign_load_jxl_load (load=0x55555569e800)
    at ../libvips/foreign/jxlload.c:629
#2  0x00007ffff7abcbd4 in vips_foreign_load_start
    (out=0x5555556a8010, a=0x0, b=0x55555569e800)

So this line:

https://github.com/libvips/libvips/blob/8.13/libvips/foreign/jxlload.c#L629

When it rewinds the input after a header read.

[jcupitt]

... looks like the obvious:

        /* We have to reset the reader ... we can't reply onb the read point
         * being left just after the header.
         */
        if( vips_source_rewind( jxl->source ) )
                return( -1 );

        size_t bytes_remaining = JxlDecoderReleaseInput( jxl->decoder );
        if( vips_foreign_load_jxl_fill_input( jxl, bytes_remaining ) )
                return( -1 );
        JxlDecoderSetInput( jxl->decoder,
                jxl->input_buffer, jxl->bytes_in_buffer );

Fixes it.

[kleisauke]

Ah, it looks like that it came from:
https://github.com/libjxl/libjxl/blob/bb96fb06f3cb7ebe2871f97f8ebba4ae1e582c49/lib/jxl/decode.cc#L1629

The JXL_API_ERROR definition will only printf when JXL_DEBUG_ON_ERROR is defined, see:
https://github.com/libjxl/libjxl/blob/bb96fb06f3cb7ebe2871f97f8ebba4ae1e582c49/lib/jxl/decode.cc#L70-L73

Which explains why I didn't saw that, since JXL_DEBUG_ON_ERROR is 0 in release mode.

[jcupitt]

Ah! Makes sense. Shall I patch 8.13?

[kleisauke]

Feel free to patch 8.13!

[jcupitt]

It was a bit more complex than I'd hoped :(

I ported to master and removed support for old libjxl as well.

[kleisauke]

PR #3170 should simplify that a bit. :)