Skip to content

Conversation

@nulano
Copy link
Contributor

@nulano nulano commented Oct 20, 2020

Fixes #4991
Fixes #3946

Adds support decoding subsampled RGB and YCbCr JPEG2000 images.
I left the grayscale formats as-is as I was a bit confused by the code and did not find test files for these formats.

Test images are from https://github.com/uclouvain/openjpeg-data, but these are huge and have an unusual license, so they are in python-pillow/pillow-depends#35. subsampling_1.jp2 and zoo1.jp2 are YCbCr 4:2:0 and subsampling_2.jp2 and zoo2.jp2 are something like RGB 2:2:0 (2x2 subsampling for all 3 channels). This list is based on this OpenJPEG test.

Until the pillow-depends PR is merged, see successful test in another branch: https://github.com/nulano/Pillow/runs/1279251514?check_suite_focus=true#step:10:425

All four images have color_space = OPJ_CLRSPC_UNSPECIFIED, and based on uclouvain/openjpeg#570 I think this may be a bug, but I did not look into fixing this.

@hugovk
Copy link
Member

hugovk commented Oct 25, 2020

python-pillow/pillow-depends#35 merged, CI restarted.

@hackermd
Copy link

hackermd commented Jan 1, 2021

@nulano I tested the nulano:jp2-decode-subsample branch on the image provided in #5165. It works. The image is decoded successfully and the colors are correct. However, I encountered a few compiler warnings when building from source. Will add a brief review to this PR.

@hackermd
Copy link

hackermd commented Jan 9, 2021

@nulano I am getting the following compiler warnings with clang (version 12) on Mac OSX:

src/_imagingft.c:1087:28: warning: assigning to 'unsigned char *' from 'char *' converts between
      pointers to integer types with different sign [-Wpointer-sign]
                    target = im->image[yy] + xx * 4;
                           ^ ~~~~~~~~~~~~~~~~~~~~~~

src/_imaging.c:3928:26: warning: comparison of integers of different signs: 'int' and 'unsigned long'
      [-Wsign-compare]
    else if ( blocks_max > SIZE_MAX/sizeof(ImagingDefaultArena.blocks_pool[0])) {
              ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/libImaging/Resample.c:211:17: warning: comparison of integers of different signs: 'int' and
      'unsigned long' [-Wsign-compare]
    if (outSize > INT_MAX / (ksize * sizeof(double))) {
        ~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/libImaging/Draw.c:1633:31: warning: comparison of integers of different signs: 'int' and
      'unsigned long' [-Wsign-compare]
            if (outline->size > INT_MAX / sizeof(Edge)) {
                ~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~

src/libImaging/GetBBox.c:201:25: warning: assigning to 'unsigned char *' from 'char *' converts between
      pointers to integer types with different sign [-Wpointer-sign]
                  pixel = im->image[y] + x * sizeof(v);
                        ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/libImaging/RankFilter.c:75:14: warning: comparison of integers of different signs: 'int' and
      'unsigned long' [-Wsign-compare]
        size > INT_MAX / (size * sizeof(FLOAT32))) {
        ~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/libImaging/Jpeg2KDecode.c:777:41: warning: comparison of integers of different signs: 'unsigned int'
      and 'int' [-Wsign-compare]
            || tile_info.x1 - image->x0 > im->xsize
               ~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~

src/libImaging/Jpeg2KDecode.c:778:41: warning: comparison of integers of different signs: 'unsigned int'
      and 'int' [-Wsign-compare]
            || tile_info.y1 - image->y0 > im->ysize) {
               ~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~
$ /usr/bin/cc --version
Apple clang version 12.0.0 (clang-1200.0.32.2)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

When I try to compile it with clang (version 11) on Linux (Ubuntu 20.04), compilation fails:

docker build -t pillow:test . -f-<<EOF
FROM ubuntu:focal AS base

ENV LC_ALL=C.UTF-8 \
    LANG=C.UTF-8 \
    DEBIAN_FRONTEND=noninteractive \
    DEBCONF_NONINTERACTIVE_SEEN=true

RUN apt-get update && \
    apt-get install -y --no-install-suggests --no-install-recommends \
        build-essential \
        ca-certificates \
        clang-11 \
        cmake \
        curl \
        dumb-init \
        git \
        liblcms2-dev \
        libfreetype6-dev \
        libfribidi-dev \
        libharfbuzz-dev \
        libjpeg-turbo8-dev \
        libopenjp2-7-dev \
        libpng-dev \
        libtiff-dev \
        libwebp-dev \
        libxcb1-dev \
        libxcb-icccm4 \
        libxcb-image0 \
        libxcb-keysyms1 \
        libxcb-render-util0 \
        libxkbcommon-x11-0 \
        netpbm \
        libzstd-dev \
        lldb-11 \
        lld-11 \
        python3-dev \
        python3-pip \
        python3-setuptools \
        python3-tk \
        s3cmd \
        tcl8.6-dev \
        tk8.6-dev \
        unzip \
        zlib1g-dev && \
    apt-get clean && rm -rf /var/lib/apt/lists/*

WORKDIR /usr/local/share/

RUN git clone https://github.com/nulano/Pillow && \
    cd Pillow && \
    git checkout 2341c6b93306088652787335ef94ebbae6aba348 && \
    python3 -m pip install -U pip setuptools && \
    CC=clang python3 setup.py build_ext --debug install

ENTRYPOINT ["/usr/bin/dumb-init", "--", "python3"]
EOF
$ /usr/bin/cc --version
Ubuntu clang version 11.0.0-2~ubuntu20.04.1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

@nulano
Copy link
Contributor Author

nulano commented Jan 9, 2021

@hackermd Thank you for testing. All of those warnings are unrelated to this PR and should now be fixed by the merge @radarhere did just above your comment. You didn't provide a log for the Linux Docker build, but I'm guessing it uses -Wall and is failing due to the (now resolved) warnings above. If you still get warnings or compile errors after this merge, I would suggest opening a new issue.

@hackermd
Copy link

@nulano the build is already based on 2341c6b

Here is the relevant part of the docker build log:

#7 29.25 running build_ext
#7 29.29 Package libtiff-5 was not found in the pkg-config search path.
#7 29.29 Perhaps you should add the directory containing `libtiff-5.pc'
#7 29.29 to the PKG_CONFIG_PATH environment variable
#7 29.29 No package 'libtiff-5' found
#7 29.34 Package libimagequant was not found in the pkg-config search path.
#7 29.34 Perhaps you should add the directory containing `libimagequant.pc'
#7 29.34 to the PKG_CONFIG_PATH environment variable
#7 29.34 No package 'libimagequant' found
#7 29.36 Looking for `libjpeg` using pkg-config.
#7 29.36 Appending path /usr/local/share/Pillow
#7 29.36 Looking for `libopenjp2` using pkg-config.
#7 29.36 Appending path /usr/local/share/Pillow
#7 29.36 Appending path /usr/include/openjpeg-2.3
#7 29.36 Looking for `libtiff-5` using pkg-config.
#7 29.36 Looking for `libtiff-4` using pkg-config.
#7 29.36 Appending path /usr/include/x86_64-linux-gnu
#7 29.36 Looking for `zlib` using pkg-config.
#7 29.36 Looking for `freetype2` using pkg-config.
#7 29.36 Looking for `lcms2` using pkg-config.
#7 29.36 Looking for `libimagequant` using pkg-config.
#7 29.36 Appending path /usr/lib
#7 29.36 Appending path /usr/include
#7 29.36 Appending path /usr/lib/x86_64-linux-gnu
#7 29.36 Appending path /usr/local/lib
#7 29.36 Appending path /usr/local/include
#7 29.36 Looking for zlib
#7 29.36 Checking for include file zlib.h in /usr/local/share/Pillow
#7 29.36 Checking for include file zlib.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file zlib.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file zlib.h in /usr/include
#7 29.36 Found zlib.h
#7 29.36 Found library z at /usr/lib/x86_64-linux-gnu/libz.so
#7 29.36 Looking for jpeg
#7 29.36 Checking for include file jpeglib.h in /usr/local/share/Pillow
#7 29.36 Checking for include file jpeglib.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file jpeglib.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file jpeglib.h in /usr/include
#7 29.36 Found jpeglib.h
#7 29.36 Found library jpeg at /usr/lib/x86_64-linux-gnu/libjpeg.so
#7 29.36 Looking for jpeg2000
#7 29.36 Checking for openjpeg-#.# in /usr/local/share/Pillow
#7 29.36 Checking for openjpeg-#.# in /usr/include/openjpeg-2.3
#7 29.36 Checking for openjpeg-#.# in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for openjpeg-#.# in /usr/include
#7 29.36 Found openjpeg.h in /usr/include/openjpeg-2.1
#7 29.36 Best openjpeg version (2, 1) so far in /usr/include/openjpeg-2.1
#7 29.36 Found openjpeg.h in /usr/include/openjpeg-2.3
#7 29.36 Best openjpeg version (2, 3) so far in /usr/include/openjpeg-2.3
#7 29.36 Checking for openjpeg-#.# in /usr/local/include
#7 29.36 Checking for openjpeg-#.# in /usr/include/python3.8
#7 29.36 Found library openjp2 at /usr/lib/x86_64-linux-gnu/libopenjp2.so
#7 29.36 Looking for imagequant
#7 29.36 Checking for include file libimagequant.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file libimagequant.h in /usr/local/share/Pillow
#7 29.36 Checking for include file libimagequant.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file libimagequant.h in /usr/include
#7 29.36 Checking for include file libimagequant.h in /usr/local/include
#7 29.36 Checking for include file libimagequant.h in /usr/include/python3.8
#7 29.36 Looking for tiff
#7 29.36 Checking for include file tiff.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file tiff.h in /usr/local/share/Pillow
#7 29.36 Checking for include file tiff.h in /usr/include/x86_64-linux-gnu
#7 29.36 Found tiff.h
#7 29.36 Found library tiff at /usr/lib/x86_64-linux-gnu/libtiff.so
#7 29.36 Looking for freetype
#7 29.36 Found library freetype at /usr/lib/x86_64-linux-gnu/libfreetype.so
#7 29.36 Checking for include file ft2build.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file ft2build.h in /usr/include/openjpeg-2.3/freetype2
#7 29.36 Checking for include file ft2build.h in /usr/local/share/Pillow
#7 29.36 Checking for include file ft2build.h in /usr/local/share/Pillow/freetype2
#7 29.36 Checking for include file ft2build.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file ft2build.h in /usr/include/x86_64-linux-gnu/freetype2
#7 29.36 Checking for include file ft2build.h in /usr/include
#7 29.36 Checking for include file ft2build.h in /usr/include/freetype2
#7 29.36 Found ft2build.h in /usr/include/freetype2
#7 29.36 Inserting path /usr/include/freetype2
#7 29.36 Looking for lcms
#7 29.36 Checking for include file lcms2.h in /usr/include/freetype2
#7 29.36 Checking for include file lcms2.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file lcms2.h in /usr/local/share/Pillow
#7 29.36 Checking for include file lcms2.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file lcms2.h in /usr/include
#7 29.36 Found lcms2.h
#7 29.36 Found library lcms2 at /usr/lib/x86_64-linux-gnu/liblcms2.so
#7 29.36 Looking for webp
#7 29.36 Checking for include file webp/encode.h in /usr/include/freetype2
#7 29.36 Checking for include file webp/encode.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file webp/encode.h in /usr/local/share/Pillow
#7 29.36 Checking for include file webp/encode.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file webp/encode.h in /usr/include
#7 29.36 Found webp/encode.h
#7 29.36 Checking for include file webp/decode.h in /usr/include/freetype2
#7 29.36 Checking for include file webp/decode.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file webp/decode.h in /usr/local/share/Pillow
#7 29.36 Checking for include file webp/decode.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file webp/decode.h in /usr/include
#7 29.36 Found webp/decode.h
#7 29.36 Found library webp at /usr/lib/x86_64-linux-gnu/libwebp.so
#7 29.36 Looking for webpmux
#7 29.36 Checking for include file webp/mux.h in /usr/include/freetype2
#7 29.36 Checking for include file webp/mux.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file webp/mux.h in /usr/local/share/Pillow
#7 29.36 Checking for include file webp/mux.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file webp/mux.h in /usr/include
#7 29.36 Found webp/mux.h
#7 29.36 Checking for include file webp/demux.h in /usr/include/freetype2
#7 29.36 Checking for include file webp/demux.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file webp/demux.h in /usr/local/share/Pillow
#7 29.36 Checking for include file webp/demux.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file webp/demux.h in /usr/include
#7 29.36 Found webp/demux.h
#7 29.36 Found library webpmux at /usr/lib/x86_64-linux-gnu/libwebpmux.so
#7 29.36 Found library webpdemux at /usr/lib/x86_64-linux-gnu/libwebpdemux.so
#7 29.36 Couldn't find library libwebpmux in ['/usr/local/share/Pillow', '/usr/lib', '/usr/lib/x86_64-linux-gnu', '/usr/local/lib']
#7 29.36 Looking for xcb
#7 29.36 Checking for include file xcb/xcb.h in /usr/include/freetype2
#7 29.36 Checking for include file xcb/xcb.h in /usr/include/openjpeg-2.3
#7 29.36 Checking for include file xcb/xcb.h in /usr/local/share/Pillow
#7 29.36 Checking for include file xcb/xcb.h in /usr/include/x86_64-linux-gnu
#7 29.36 Checking for include file xcb/xcb.h in /usr/include
#7 29.36 Found xcb/xcb.h
#7 29.36 Found library xcb at /usr/lib/x86_64-linux-gnu/libxcb.so
#7 29.36 building 'PIL._imaging' extension
#7 29.36 creating build
#7 29.37 building 'PIL._imagingcms' extension
#7 29.37 building 'PIL._imagingft' extension
#7 29.37 creating build/temp.linux-x86_64-3.8
#7 29.37 creating build/temp.linux-x86_64-3.8
#7 29.37 creating build/temp.linux-x86_64-3.8
#7 29.37 creating build/temp.linux-x86_64-3.8/src
#7 29.37 clang -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -g -I/usr/include/freetype2 -I/usr/include/openjpeg-2.3 -I/usr/local/share/Pillow -I/usr/include/x86_64-linux-gnu -I/usr/include -I/usr/local/include -I/usr/include/python3.8 -c src/_imagingcms.c -o build/temp.linux-x86_64-3.8/src/_imagingcms.o
#7 29.37 building 'PIL._webp' extension
#7 29.37 creating build/temp.linux-x86_64-3.8/src
#7 29.38 creating build/temp.linux-x86_64-3.8/src
#7 29.38 clang -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -g -I/usr/include/freetype2 -I/usr/include/openjpeg-2.3 -I/usr/local/share/Pillow -I/usr/include/x86_64-linux-gnu -I/usr/include -I/usr/local/include -I/usr/include/python3.8 -c src/_imagingft.c -o build/temp.linux-x86_64-3.8/src/_imagingft.o
#7 29.38 creating build/temp.linux-x86_64-3.8/src/libImaging
#7 29.38 clang -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -g -DHAVE_WEBPMUX -I/usr/include/freetype2 -I/usr/include/openjpeg-2.3 -I/usr/local/share/Pillow -I/usr/include/x86_64-linux-gnu -I/usr/include -I/usr/local/include -I/usr/include/python3.8 -c src/_webp.c -o build/temp.linux-x86_64-3.8/src/_webp.o
#7 29.38 building 'PIL._imagingtk' extension
#7 29.39 building 'PIL._imagingmath' extension
#7 29.39 clang -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -g -I/usr/include/freetype2 -I/usr/include/openjpeg-2.3 -I/usr/local/share/Pillow -I/usr/include/x86_64-linux-gnu -I/usr/include -I/usr/local/include -I/usr/include/python3.8 -c src/_imagingmath.c -o build/temp.linux-x86_64-3.8/src/_imagingmath.o
#7 29.39 creating build/temp.linux-x86_64-3.8/src/Tk
#7 29.39 clang -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -g -I/usr/include/freetype2 -I/usr/include/openjpeg-2.3 -I/usr/local/share/Pillow -I/usr/include/x86_64-linux-gnu -I/usr/include -I/usr/local/include -I/usr/include/python3.8 -c src/_imagingtk.c -o build/temp.linux-x86_64-3.8/src/_imagingtk.o
#7 29.39 clang -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -g -DHAVE_LIBJPEG -DHAVE_OPENJPEG -DHAVE_LIBZ -DHAVE_LIBTIFF -DHAVE_XCB -DPILLOW_VERSION="8.1.0.dev0" -I/usr/include/freetype2 -I/usr/include/openjpeg-2.3 -I/usr/local/share/Pillow -I/usr/include/x86_64-linux-gnu -I/usr/include -I/usr/local/include -I/usr/include/python3.8 -c src/_imaging.c -o build/temp.linux-x86_64-3.8/src/_imaging.o
#7 29.40 building 'PIL._imagingmorph' extension
#7 29.40 clang -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -g -I/usr/include/freetype2 -I/usr/include/openjpeg-2.3 -I/usr/local/share/Pillow -I/usr/include/x86_64-linux-gnu -I/usr/include -I/usr/local/include -I/usr/include/python3.8 -c src/_imagingmorph.c -o build/temp.linux-x86_64-3.8/src/_imagingmorph.o
#7 29.41 error: command 'clang' failed with exit status 1

@nulano
Copy link
Contributor Author

nulano commented Jan 11, 2021

I don't see an error in that output, I think there might still be something missing. I also don't see why this PR should cause any compile issues, have you tried compiling the master branch?

@hackermd
Copy link

I don't see an error in that output, I think there might still be something missing. I also don't see why this PR should cause any compile issues, have you tried compiling the master branch?

I am getting the same issue when trying to compile the master branch. Do I need to configure anything else for using clang to compile PIL on Ubuntu? I am fine with using GCC.

@nulano
Copy link
Contributor Author

nulano commented Mar 29, 2021

Is there anything blocking this?

@wiredfool
Copy link
Member

I don't think so. There are a few uncovered lines, but we'd need to find specific j2k files to hit them.

@wiredfool
Copy link
Member

This does interact with an upcoming security patch. It's a pretty trivial merge fix, but it changes the error from the test cases.

@wiredfool
Copy link
Member

I think I'd prefer to merge this just after the release, so that we don't potentially trip up a security issue.

@hugovk hugovk added this to the 8.3.0 milestone Mar 31, 2021
@radarhere radarhere requested a review from wiredfool April 10, 2021 11:14
@wiredfool
Copy link
Member

I was ok with this before the release, except for the potential conflict with the security patch. Lets get it merged and fix things up if necessary.

@hugovk hugovk merged commit 197673b into python-pillow:master Apr 17, 2021
@nulano nulano deleted the jp2-decode-subsample branch July 10, 2021 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OSError: broken data stream when reading image file JP2 Truncated/corrupted jpeg2000 image

5 participants