Vips jp2ksave creates grayscale JPEG2000 image from SRGB sources

[felixbuenemann]

There's a bug in the JPEG2000 saver in Vips 8.13.0, that causes converted images to be grayscale, even though they have 3 bands.

Example:

vips jp2ksave sample.jpg sample.jp2

I tested this with both Vips 8.13.0 and OpenJPEG 2.5.0 on macOS/arm64 and Linux/aarch64, with the same result.

[kleisauke]

I could not reproduce this using OpenJPEG 2.4.0 and libvips 8.13.0 on my Fedora PC. Are you using the macOS preview app? If so, this could be a duplicate of libvips/ruby-vips#345. A handy way to check is to convert the JPEG2000 image back to PNG, e.g:

$ vips copy sample.jp2 x.png

[felixbuenemann]

The JPEG2000 image shows as grayscale in any app on my Mac: Preview, Safari, Pixelmator.

You are right, converting the file back to jpg gives the correct colors.

But if I convert the file back using ImageMagick, I get completely wrong colors. Looking at the channels the red channel looks ok, but green and blue have wrong brightness and look inverted.

Opening the Vips created JPEG2000 in Affinity Photo on the Mac show the same wrong colors, as the JPEG created from it using ImageMagick.

However if I compress from the same source image using opj_compress from OpenJPEG the problem does not occur.
Also converting that JPEG2000 back to JPEG using ImageMagick works fine.

So something appears to be wrong with the file.

[jcupitt]

Could you try an image with an odd number of columns?

[felixbuenemann]

@jcupitt Yes, the bug doesn't happen with odd widths. Eg. 1023x1023px OK, 1024x1024px buggy.

[felixbuenemann]

Here's an example what it looks like after converting back from JPEG200 to JPEG using ImageMagick:

Source 1023x1023px:

Source 1024x1024px:

[felixbuenemann]

I noticed that converting the even width JPEG200 to PNG using opj_decompress from the OpenJPEG CLI tools also works fine.

[felixbuenemann]

I noticed some difference when dumping the files with opj_dump:

1023x1023px created by Vips:

Image info {
         x0=0, y0=0
         x1=1023, y1=1023
         numcomps=3
                 component 0 {
                 dx=1, dy=1
                 prec=8
                 sgnd=0
        }
                 component 1 {
                 dx=1, dy=1
                 prec=8
                 sgnd=0
        }
                 component 2 {
                 dx=1, dy=1
                 prec=8
                 sgnd=0
        }
}

1024x1024px created by Vips:

Image info {
         x0=0, y0=0
         x1=1024, y1=1024
         numcomps=3
                 component 0 {
                 dx=1, dy=1
                 prec=8
                 sgnd=0
        }
                 component 1 {
                 dx=2, dy=2
                 prec=8
                 sgnd=0
        }
                 component 2 {
                 dx=2, dy=2
                 prec=8
                 sgnd=0
        }
}

1024x1024px created by ImageMagick:

Image info {
         x0=0, y0=0
         x1=1024, y1=1024
         numcomps=3
                 component 0 {
                 dx=1, dy=1
                 prec=8
                 sgnd=0
        }
                 component 1 {
                 dx=1, dy=1
                 prec=8
                 sgnd=0
        }
                 component 2 {
                 dx=1, dy=1
                 prec=8
                 sgnd=0
        }
}

Note that for the files that work fine, all components have dx=1, dy=1, but the problematic one has dx-2, dy=2 for component 1 and 2.

[felixbuenemann]

@jcupitt The problem seems to be related to chroma subsampling, if I use vips jp2ksave Earth1024.jpg Earth1024_no_ss.j2k --subsample-mode off the image looks fine in all viewers.

I noticed that the default for subsample-mode is auto, which enables subsampling only if quality is < 90% and Xsize and Ysize are evenly divisible by 2. This means auto mode enables subsampling for our even width image and disables it for the odd width image.

See: https://github.com/libvips/libvips/blob/master/libvips/foreign/jp2ksave.c#L793

[felixbuenemann]

@jcupitt I found that ImageMagick sets x1 and y1 differently, based on which subsampling is used:

https://github.com/ImageMagick/ImageMagick/blob/main/coders/jp2.c#L1019

But Vips just uses width and height:

https://github.com/libvips/libvips/blob/master/libvips/foreign/jp2ksave.c#L714

[felixbuenemann]

Hmm, that's not it, I tried the following patch and still grayscale:

diff --git a/libvips/foreign/jp2ksave.c b/libvips/foreign/jp2ksave.c
index 530ce61f4..9a6c4a96d 100644
--- a/libvips/foreign/jp2ksave.c
+++ b/libvips/foreign/jp2ksave.c
@@ -711,8 +711,8 @@ vips_foreign_save_jp2k_new_image( VipsImage *im,

        image = vips_opj_image_create( im->Bands, comps, color_space,
                allocate );
-       image->x1 = width;
-       image->y1 = height;
+       image->x1 = (width-1)*(subsample ? 2 : 1)+1;
+       image->y1 = (height-1)*(subsample ? 2 : 1)+1;

        /* Tag alpha channels.
         */

[felixbuenemann]

However the computation of x1 and y1 in libvips seems to be wrong, see:

https://github.com/uclouvain/openjpeg/blob/master/src/bin/jp2/convertpng.c#L212

That matches what I did in my patch above, I'm only ignoring x0 and y0, since they are hardcoded to 0 in libvips.

[felixbuenemann]

Hmm, the ImageMagick and OpenJPEG encoders linked above also both set dx and dy the same on all planes, instead of keeping it to 1 on the first plane, which seems strange to me. But even if I set dx/dy and x1/y1 like those decoders there's no difference. So the problem is likely not in the headers.

[felixbuenemann]

Oh, I just noticed vips only uses YCbCr when subsampling is enabled, so if it's disabled we use RGB:

https://github.com/libvips/libvips/blob/master/libvips/foreign/jp2ksave.c#L818

But there are different codepaths for subsample and save_as_ycc, which doesn't make sense in this case:

https://github.com/libvips/libvips/blob/master/libvips/foreign/jp2ksave.c#L482

[felixbuenemann]

Argh, I had the wrong vips binary symlinked in my header tests. So I was testing the original code all the time.

Essentially it doesn't seem to matter:

Either set dx/dy to 2 for all planes and use the more complicated calculation for x1, y1 or do it just as it currently is. If you only change x1, y1 you get a "Size mismatch between tile data and sent data." error.

In the end both variants end up with the version that looks like grayscale when subsampling is enabled, so I'm back at the beginning.

[jcupitt]

Ooof, it's been a couple of years since I looked at this, I remember it was hard to get chroma subsampling to work.

I've read the libvips jp2k load and save again and it's coming back a bit. As I recall, openjpeg, opj_compress, and opj_decompress don't support chroma subsampling directly, and don't include YCC <-> RGB conversion --- applications need to code the shrink, expand, and colourspace transform themselves. It also doesn't support chroma subsample with odd-sized image tiles.

Because of this, few openjpeg-based decoders support anything other than 4:4:4 RGB. It looks like imagemagick and the built-in macOS converters are 4:4:4 RGB only.

This means the default libvips behaviour is to write images which few people can read :-( in retrospect this looks like an error, sorry.

We can't change this in 8.13, but for 8.14 lets swap the default to 4:4:4 RGB and have the fancy subsample mode behind a switch. As a workaround, setting subsample-mode=off should write an image everyone can decode correctly. Filesize almost doubles, but of course an unreadable image is useless.

[jcupitt]

It'd be good to test the libvips chromasubsample mode against another implementation. I think we'd probably need to get a license for one of the commercial jp2k systems and see how that behaves.

Looking at the IM sources for jp2 it looks like it ought to support YCC and chroma subsample save, but it seems to crash for me:

$ convert k2.jpg -colorspace yuv x.jp2
convert: ./src/lib/openjp2/mqc.c:205: opj_mqc_init_enc: Assertion `*(mqc->bp) != 0xff' failed.
Aborted (core dumped)

That's git master IM7 and opj 2.5.

It supports chroma upsample for load, but doesn't seem to check the image colourspace, so it won't know to do YCC->RGB conversion.

[jcupitt]

... libvips 8.14 defaults jp2k save to chroma subsample off.

[felixbuenemann]

@jcupitt There seems to be a problem with the MCT (Multi-Component-Transform) header:

If I compress with opj_compress it sets mct to 1, which is RGB->YCC conversion, but if I compress with vips and subsample on, it still sets mct to 0, which is no MCT.

This explains why the image decoded by ImageMagick looks strange, it is decoded as if it were RGB even though it is actually YCbCr.

You can check this with opj_dump and look for the mct value in the codestream infro from main header under default tile.

The mct parameter maps to the tcp_mct field of the opj_cparameters_t struct.

[jcupitt]

Oh, interesting. We've gone back and forth on mct a few times, it was most recently disabled if subsampling is on.

57d8dd3

Of course I intelligently didn't make a note of why this change was made :( A workaround for some bug somewhere, I suppose.

It sounds like we should always set mct if we are saving as OPJ_CLRSPC_SYCC, is that right?

[felixbuenemann]

In opj_compress by default RGB->YCC transform is used, if there are three components or more. I can manually set it to -mct 0 (no transform) or -mct 1 (RGB->YCC). The file with mct 0 is bigger, suggesting it is RGB. I also get the corresponding value for mct when I analyze those files with obj_dump.

However, when I hardcode both places we set tcp_mct to 1, the resulting files stay grayscale and opj_dump shows mct=0, so somehow the parameter isn't set.

If I set subsample_mode off in vips and hardcode tcp_mct to either 0 or 1 I get a smaller file with tcp_mct 1 and opj_dump properly shows mct-0 or mct=1.

What I'm asking myself is:

Should Vips even be doing the RGB->YCC transform or is that handled by OpenJPEG itself, if the tcp_mct is properly set?

[jcupitt]

Huh hmmm I'm not sure either now. The openjpeg docs are rather opaque :( I didn't realize mct meant "enable RGB -> YCC conversion" until you said (if indeed that's what it does mean? looking at the header it seems to relate to enable some kind of correlation transform?).

So maybe libvips should set mct if bands >= 3, and not use OPJ_CLRSPC_SYCC?

[felixbuenemann]

I'll try asking on the OpenJPEG Google Group.

[felixbuenemann]

I ended up no asking on the group, but doing some research instead.

MCT (Multi Component Transform) is an addition to the JPEG 2000 standard that was added to help in compression of multi dimensional images, like medical imaging. You can supply a custom transformation or use the built-in MCT 1 which does an RGB to YCC transform without chroma subsampling (YCbCr 4:4:4).

This means we should only use MCT 1 on (s)RGB data.

JPEG2000 can also store YCbCr image data natively, which is what the current subsample_mode "on" or "auto" does, but from some research and own testing on macOS, those images are not compatible with all viewers.

So my proposal would be to default to subsample_mode "off" (as @jcupitt already suggested) and remove the Xsize % 2 == 0 and Ysize % 2 == 0 from the "auto" check, because it makes the behaviour very unpredictable: As a user I don't expect my images to use a different colourspace just because their width/height is even or odd.

As a side note: The obj_compress program has a -s x,y option to set subsampling mode, but it only changes the header values and is not actually implemented. That caused quite a bit of confusion during testing for me.

[jcupitt]

That sounds reasonable. I'll make a PR with a proposed revision we can test. Thanks for doing this research @felixbuenemann !

(or would you rather make a PR? I don't mind)

[felixbuenemann]

I noticed you already changed the default for subsample_mode, so I've made a PR with the change to the "auto" behaviour.

[jcupitt]

I think this is all done now, I'll close.