search

harvard-lts / kakadu-vips

Kakadu JP2 reader and writer for libvips
Apache License 2.0
3 stars 0 forks source link

Invalid image after calling `icc_transform` #16

Closed scossu closed 3 months ago

scossu commented 4 months ago 
>>> from imgconv.api.image import kdu_convert
DEBUG:pyvips:Binary module load failed: No module named '_libvips'
DEBUG:pyvips:Falling back to ABI mode
DEBUG:pyvips:Loaded lib <cffi.api._make_ffi_library.<locals>.FFILibrary object at 0x78b47dbeb110>
DEBUG:pyvips:Inited libvips
>>> with open("/data/498108846.jp2", "rb") as fh:
...     img = Image.kakaduload_buffer(fh.read())
... 
>>> img
<pyvips.Image 5891x7651 ushort, 3 bands, rgb16>
>>> from os import path
>>> path.isfile(OUTPUT_ICC_FILE)
True
>>> norm = img.icc_transform(OUTPUT_ICC_FILE)
DEBUG:pyvips.error:Error  image: no property named `icc_transform'

>>> norm
<pyvips.Image 5891x7651 ushort, 3 bands, rgb16>
>>> data = norm.kakadusave_buffer(options="", rate=3)
DEBUG:pyvips.error:Error  image: no property named `kakadusave_buffer'

<interpreter crashes>

I'm using ubuntu:noble Docker image with libvips 8.15.1.

I'm not sure if this is related to kakaduload, but I don't think I have seen the ABI compatibility warnings before.

jcupitt commented 4 months ago

Hi @scossu, I'd guess you've built libvips without lcms2 support.

scossu commented 4 months ago

It all looks good to me: 

$ vips --vips-config | grep lcms
ICC profile support with lcms: true

$ apt info liblcms2-2
Package: liblcms2-2
Version: 2.14-2build1
Priority: optional
Section: libs
Source: lcms2
Origin: Ubuntu
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@li$ apt info liblcms2-2
Package: liblcms2-2
Version: 2.14-2build1
Priority: optional
Section: libs
Source: lcms2
Origin: Ubuntu
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
Original-Maintainer: Thomas Weber <tweber@debian.org>
Bugs: https://bugs.launchpad.net/ubuntu/+filebug
Installed-Size: 430 kB
Depends: libc6 (>= 2.29)
Suggests: liblcms2-utils
Homepage: http://www.littlecms.com/
Task: ubuntu-desktop-minimal, ubuntu-desktop, ubuntu-wsl, print-server, ubuntu-desktop-raspi, kubuntu-desktop, xubuntu-minimal, xubuntu-desktop, lubuntu-desktop, ubuntustudio-desktop-core, ubuntustudio-desktop, ubuntukylin-desktop, ubuntukylin-desktop-minimal, ubuntu-mate-core, ubuntu-mate-desktop, ubuntu-budgie-desktop-minimal, ubuntu-budgie-desktop, ubuntu-budgie-desktop-raspi, ubuntu-unity-desktop, edubuntu-desktop-gnome-minimal, edubuntu-desktop-gnome-raspi, ubuntucinnamon-desktop-minimal, ubuntucinnamon-desktop-raspi
Download-Size: 161 kB
APT-Manual-Installed: no
APT-Sources: http://archive.ubuntu.com/ubuntu noble/main amd64 Packages
Description: Little CMS 2 color management librarysts.ubuntu.com>
Original-Maintainer: Thomas Weber <tweber@debian.org>
Bugs: https://bugs.launchpad.net/ubuntu/+filebug
Installed-Size: 430 kB
Depends: libc6 (>= 2.29)
Suggests: liblcms2-utils
Homepage: http://www.littlecms.com/
Task: ubuntu-desktop-minimal, ubuntu-desktop, ubuntu-wsl, print-server, ubuntu-desktop-raspi, kubuntu-desktop, xubuntu-minimal, xubuntu-desktop, lubuntu-desktop, ubuntustudio-desktop-core, ubuntustudio-desktop, ubuntukylin-desktop, ubuntukylin-desktop-minimal, ubuntu-mate-core, ubuntu-mate-desktop, ubuntu-budgie-desktop-minimal, ubuntu-budgie-desktop, ubuntu-budgie-desktop-raspi, ubuntu-unity-desktop, edubuntu-desktop-gnome-minimal, edubuntu-desktop-gnome-raspi, ubuntucinnamon-desktop-minimal, ubuntucinnamon-desktop-raspi
Download-Size: 161 kB
APT-Manual-Installed: no
APT-Sources: http://archive.ubuntu.com/ubuntu noble/main amd64 Packages
Description: Little CMS 2 color management library

What baffles me is that the image is created anyways, but it's probably corrupted because calling kakadusave crashes the process.

jcupitt commented 4 months ago

Could you have two libvips binaries on your system, with the command-line and python seeing different ones? I'm only guessing of course :(

jcupitt commented 4 months ago

Could you share the dockerfile that produces this error?

scossu commented 4 months ago

This is the base image: 

FROM --platform=$TARGETPLATFORM ubuntu:noble
ARG TARGETPLATFORM
ARG BUILDPLATFORM
RUN echo "I am running on $BUILDPLATFORM, building for $TARGETPLATFORM"

ENV TZ=America/New_York
ENV PATH=/usr/local/bin:${PATH}
ENV LD_IBRARY_PATH=/usr/local/lib
ENV CPATH=/usr/local/include
ENV KDU_VERSION="v8_4_1-02108C"

# Working source root.
ARG _workroot=/usr/local/src
ARG KDU_SRC=${_workroot}/kakadu/${KDU_VERSION}
ARG KVIPS_SRC=${_workroot}/kakadu-vips

WORKDIR ${KVIPS_SRC}
RUN apt update && \
    apt install -y tzdata build-essential pkg-config libvips-dev \
    libvips-tools liblcms2-dev

WORKDIR ${_workroot}

# Copy and compile Kakadu codec.
COPY ./make_wrap.sh ./
RUN chmod u+x ./make_wrap.sh
WORKDIR ${KDU_SRC}
COPY ./kakadu/${KDU_VERSION} ${KDU_SRC}

WORKDIR ${KDU_SRC}/coresys/make
RUN ${_workroot}/make_wrap.sh init ${TARGETPLATFORM}
RUN CXXFLAGS="-DFBC_ENABLED" ${_workroot}/make_wrap.sh -f MAKEFILE

WORKDIR ${KDU_SRC}/managed/make
RUN CXXFLAGS="-DFBC_ENABLED" ${_workroot}/make_wrap.sh -f MAKEFILE all_but_jni

# Copy apps and libs to a standard path.
WORKDIR ${KDU_SRC}
RUN cp lib/$(cat /etc/kakadu_makefile)/* /usr/local/lib && \
    cp bin/$(cat /etc/kakadu_makefile)/* /usr/local/bin && \
    cp coresys/common/*.h apps/compressed_io/*.h apps/support/*.h /usr/local/include

# Compile kakadu-vips.
WORKDIR ${KVIPS_SRC}
COPY ./kakadu-vips/src ${KVIPS_SRC}
RUN KAKADUHOME=${KDU_SRC} KAKADU_ARCH=$(cat /etc/kakadu_makefile) make release && \
    KAKADUHOME=${KDU_SRC} KAKADU_ARCH=$(cat /etc/kakadu_makefile) make install

# Clean up build packages.
WORKDIR ${_workroot}
RUN rm -r ${KVIPS_SRC} && rm -r ${KDU_SRC} && \
    rm make_wrap.sh && \
    apt remove -y build-essential && \
    apt autoremove -y && \
    rm -rf /var/lib/apt/lists/*

the implementation basically installs Python and pyvips, it's in a separate image that has a bunch of other irrelevant things.

jcupitt commented 3 months ago

Ooop, sorry, I was distracted again.

I can't reproduce this error, but I can't give a bit of background:

>>> from imgconv.api.image import kdu_convert
DEBUG:pyvips:Binary module load failed: No module named '_libvips'
DEBUG:pyvips:Falling back to ABI mode

This is not good -- pyvips runs in two modes, ABI (a run-time binding) and API (an install-time binding).

The run-time one (ABI mode) only needs the libvips.so shared library binary to work, but it's slow, since it has to do things like building function calls in python. The install-time mode works by generating and compiling a C module to interface between libvips and python during pip install pyvips. Because function calls are then done in C, it's a lot quicker.

When you pip install pyvips, it uses pkg-config to find the libvips headers and build the module. If this fails, it falls back to ABI mode. This is what has happened here.

To fix this, make sure the libvips dev headers are available at pip install time. You'll need to usual build environment as well. 

>>> norm = img.icc_transform(OUTPUT_ICC_FILE)
DEBUG:pyvips.error:Error  image: no property named `icc_transform'

pyvips will introspect the libvips binary it finds at run-time and present the methods it discovers. This error means that the libvips binary that your python is finding has been built without lcms.

I'd check which libvips.so python is picking up, and look into how that was built.

jcupitt commented 3 months ago

I made you a tiny demo dockerfile:

FROM ubuntu:noble

RUN apt-get update \
    && apt-get install -y

# we need some extra packages to build from git master
RUN apt-get install -y \
    build-essential  \
    wget \
    meson

# stuff we need to build our own libvips ... this is a pretty random selection
# of dependencies, you'll want to adjust these
RUN apt-get install -y \
    glib-2.0-dev \
    libexpat-dev \
    librsvg2-dev \
    libpng-dev \
    libjpeg-dev \
    libexif-dev \
    liblcms2-dev

ARG VIPS_URL=https://github.com/libvips/libvips/releases/download
ARG VIPS_VERSION=8.15.1
WORKDIR /usr/local/src

RUN wget ${VIPS_URL}/v${VIPS_VERSION}/vips-${VIPS_VERSION}.tar.xz

RUN tar xf vips-${VIPS_VERSION}.tar.xz \
    && cd vips-${VIPS_VERSION} \
    && meson setup build \
    && cd build \
    && meson compile \
    && meson install

RUN apt-get install -y \
    python3-pip \
    python3-dev \
    python3-venv

RUN python3 -m venv pyvips \
    && pyvips/bin/pip install pyvips

I can run:

$ docker build -t pyvips-noble .
...
$ docker run -it --rm -v $PWD:/data pyvips-noble                                                          
root@db6955c49bbe:/usr/local/src# ./pyvips/bin/python
Python 3.12.3 (main, Apr 10 2024, 05:33:47) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pyvips
>>> pyvips.API_mode
True
>>> im = pyvips.Image.black(100, 100, bands=3).copy(interpretation='srgb')
>>> im.icc_transform('cmyk')
<pyvips.Image 100x100 uchar, 4 bands, cmyk>
>>>
scossu commented 3 months ago

I finally got to the bottom of this.

Pyvips was binding in ABI mode because I did a package cleanup after building all sources. That removed librsvg2 which was installed as a dependency of librsvg2-dev, but is needed by the vips runtime. Hence pkgconfig could not satisfy the dependency graph and pip would proceed with the ABI mode...

I'm cleaning up my Docker files now and will do further testing with the color profile transforms.

scossu commented 3 months ago

It's getting stranger: 

>>> pyvips.API_mode
True
>>> im = pyvips.Image.black(100, 100, bands=3)
>>> im
<pyvips.Image 100x100 uchar, 3 bands, multiband>
>>> im = im.copy(interpretation="srgb")
DEBUG:pyvips.error:Error  image: no property named `copy'

>>> im
<pyvips.Image 100x100 uchar, 3 bands, srgb>
>>> im.icc_transform("cmyk")
DEBUG:pyvips.error:Error  image: no property named `icc_transform'

<pyvips.Image 100x100 uchar, 4 bands, cmyk>
jcupitt commented 3 months ago

Wait, do you have debug logging turned on? Those messages are expected and harmless, eg.:

$ python
Python 3.11.6 (main, Oct  8 2023, 05:06:43) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import logging
>>> logging.basicConfig(level = logging.DEBUG)
>>> import pyvips
DEBUG:pyvips:Loaded binary module _libvips
DEBUG:pyvips:Module generated for libvips 8.16
DEBUG:pyvips:Linked to libvips 8.16
DEBUG:pyvips:Inited libvips
>>> im = pyvips.Image.black(100, 100, bands=3)
DEBUG:pyvips.voperation:VipsOperation.call: operation_name = black
...
DEBUG:pyvips.voperation:VipsOperation.call: result = <pyvips.Image 100x100 uchar, 3 bands, multiband>
>>> im = im.copy(interpretation="srgb")
DEBUG:pyvips.error:Error  image: no property named `copy'

DEBUG:pyvips.voperation:VipsOperation.call: operation_name = copy
...
>>>

When you do im.copy() it's first trying to get copy as a property (like width, for example), then if that fails, looking for an operation of that name which it can call as a function. The error message from the property search is normally suppressed, but will appear in debug logging.

scossu commented 3 months ago

Understood.

However, pyvips keeps crashing on kakadusave. Attached is a sample script reproducing the problem along with the image and ICC profile I used for testing.

kdusave_crash.tgz

scossu commented 3 months ago

Note that the sample image is 16-bit RGB and the ICC transform is downsampling to 8-bit—but even without the depth=8 option, it still crashes..

jcupitt commented 3 months ago

Oh, maybe the bit reduction is confusing it? I'll investigate. Thanks for the sample!

jcupitt commented 3 months ago

Sorry, you sample code is working fine here :( I have ubuntu noble on this PC.

Shall I try making a dockerfile for noble with all this stuff working? Or maybe you can make a dockerfile that would let me reproduces the fault?

scossu commented 3 months ago

Let me try on my host first. I'll install the latest packages and let you know how that goes.

scossu commented 3 months ago

This happens on my host too (Arch Linux, vips 8.15.1):

>>> import pyvips
>>> pyvips.API_mode
True
>>> from pyvips import Image
>>> img = Image.kakaduload("./data/498108846.jp2")
>>> img
<pyvips.Image 5891x7651 ushort, 3 bands, rgb16>
>>> from pyvips.enums import Intent, PCS
>>> norm = img.icc_transform("./data/sRGB.icc", pcs=PCS.LAB, intent=Intent.RELATIVE, black_point_compensation=True, embedded=True, depth=8)
>>> norm
<pyvips.Image 5891x7651 uchar, 3 bands, srgb>
>>> norm.kakadusave("/tmp/xx.jph")
[crash]
jcupitt commented 3 months ago

I had another thought -- are you using clang as a compiler anywhere?

C++ doesn't have an ABI, so you can't mix compilers in one executable. You have to use exactly the same compiler everywhere or you'll get mysterious crashes.

scossu commented 3 months ago

I just checked, gcc is installed in the Docker guest and clang is not (before doing any package cleanup), so I guess that the answer is no.

scossu commented 3 months ago

I also noticed that kakadusave works for the non-converted image, so it must be something with the ICC conversion.

jcupitt commented 3 months ago

Ah, I found it! kakadu is failing here:

        else if (profile_is_display)
          { KDU_ERROR(e,101); e <<
              KDU_TXT("Embedded ICC profile in JP2 colour description "
              "box specifies a display profile, but does not contain a "
              "complete set of tone reproduction curves!  This condition is "
              "not compatible with any legal ICC profile.");
          }

So it hates your ICC profile and silently kills the process.

I'll look at catching kakadu errors in a more sensible way.

We should also look into this profile you are using --- it looks very non-optimal to me at first glance. I think a simple sRGB display profile should only be a few 100 bytes, not 64kb.

scossu commented 3 months ago

Oh, that's super helpful info. We actually had trouble with Kakadu not liking our ICC profile and crashing. I'll forward this to our Imaging department and let you know.

In the meantime it would be great if you could have those messages bubble up the stack. Thanks!

jcupitt commented 3 months ago

I improved handling of kakadu exceptions with https://github.com/harvard-lts/kakadu-vips/commit/8addb12c243501fbb499b6136ebab96feb9ada53

I now see:

john@banana ~/try/kakadusave $ ./t.py 
Traceback (most recent call last):
  File "/home/john/try/kakadusave/./t.py", line 28, in <module>
    norm.kakadusave("t.jph", options=" ".join(HTJ2K_OPTS), rate=3)
  File "/home/john/vips/lib/python3.12/site-packages/pyvips/vimage.py", line 1354, in call_function
    return pyvips.Operation.call(name, self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/john/vips/lib/python3.12/site-packages/pyvips/voperation.py", line 305, in call
    raise Error('unable to call {0}'.format(operation_name))
pyvips.error.Error: unable to call kakadusave
  VipsForeignKakadu: Error in Kakadu File Format Support:
Embedded ICC profile in JP2 colour description box specifies a display profile,
but does not contain a complete set of tone reproduction curves!  This
condition is not compatible with any legal ICC profile.

So we get a sensible error message in pyvips.

I'll close, I think we're done (phew).

scossu commented 3 months ago

Thank you. I'll try with a different sRGB ICC file, and follow up if there are issues.