libjxl / libjxl

JPEG XL image format reference implementation
BSD 3-Clause "New" or "Revised" License
2.71k stars 260 forks source link

PNG to JPEG XL conversion using default (-d 1) parameters is causing wrong colors #1579

Open Funkmiser opened 2 years ago

Funkmiser commented 2 years ago

Wrong colors images.zip The wrong colors zip is around 290MB which is why I had to use Google Drive for the upload. The jxl file is one using the default parameters for cjxl.exe

Describe the bug I used the release version 0.6.1 x64 cjxl.exe to convert a png to a jxl file. This all used default parameters which I believe is distance 1 (-d 1) because it is a png file. I was getting crazy super dark color results viewing the jxl file using the ImageGlass viewer so I used the reference decoder djxl.exe to convert the jxl to a jpg to see what libjxl viewed the image as. The result is an image that seems to be slightly dark or slightly more blue than the regular version as seen below in the screenshots. What's weird about this is that later on I tried converting the png to jxl using cjxl.exe with a distance of 0 (-d 0) and when viewing the jxl file and using djxl.exe to get a jpg file they both looked exactly like the original. I then tested using a distance of 2 and it looks wrong like the distance of 1 does.

Screenshots (These are screenshots of the ImageGlass viewer displaying the images because the images are so big) Screenshot 2022-07-06 021237 regular This is the regular version

Screenshot 2022-07-06 021123 blue This is what shows up after converting the jxl back to a jpg using djxl.exe, the image viewer I am using (ImageGlass) uses the Magick.Net decoder which shows even darker colors than above when viewing the jxl file. This is why I am showing the jpg I got back from djxl because I assume that would be what a reference decoder should look like. I have created an issue for Magick.Net for the decoding issues https://github.com/dlemstra/Magick.NET/issues/1212

Environment

mo271 commented 2 years ago

Thanks for the bug report, @Funkmiser! Could you try to confirm that the bug is still present with a more recent version, for example with the jxl-x64-windows-static artifact from here: https://github.com/libjxl/libjxl/actions/runs/2616802303 ?

jendalinda commented 2 years ago

The jxl file seems to be invalid. djxl 0.7.0 is unable to decode it. When opened in IrfanView 4.60, it looks the same as the PNG image, though.

Funkmiser commented 2 years ago

The jxl being invalid is an odd one, I downloaded the zip and unzipped it and both the v0.6.1 djxl.exe and the artifacts windows x64 djxl.exe that was linked in the other comment both correctly decoded the jxl to a jpg file showing the incorrect colors. When checked using Irfanview it actually shows a darker image than djxl decodes it into. Confusingly enough, that darker image happens when using Irfanview to view both the png and the jxl, with the jpg created from djxl.exe being even darker, so Irfanview seems to view it correctly. This is with an Irfanview I had just installed and downloaded and activated the Jpeg XL plugin with.

Screenshot 2022-07-06 092043 The Irfanview view

That's all with the jxl from the zip file, using the windows x64 cjxl.exe from the artifacts section that @mo271 linked still shows the same behavior when creating the jxl file using -d 1, -d 0 still shows correctly when viewing the jxl directly and when using either djxl.exe to decode to jpg.

Atari 5200 - Super Software Poster 2 d1 newer.zip This is a jxl created using the newer artifacts cjxl and explicitly using -d 1

Screenshot 2022-07-06 093045 newer d1 all stacked Here you can see a stack containing the original png and the newer djxl jpg in ImageGlass and then the new -d 1 jxl shown in Irfanview with a comparison of the newer djxl.exe jpg again just to show it's darker. This is to point out that Irfanview is actually decoding this correctly(?) compared to if you open the png using Irfanview, but for whatever reason the image is darker than ImageGlass.

Funkmiser commented 2 years ago

Yet another addendum to this, the newer djxl.exe seems to decode both jxls (the older and newer versions) into a wrong colored jpg but still different from the v0.6.1 jpg. The v0.6.1 jpg was more blue while the newer decode is darker.

Screenshot 2022-07-06 095644 stack djxl comparison Here's a stack of the original png, the older djxl decode (more blue), and the newer djxl decode (more dark/grey)

jendalinda commented 2 years ago

That's interesting. I've tried djxl builds v0.7.0 ab775e2 and v0.7.0 21da8c2. These are failing to decode the JXL file.

Funkmiser commented 2 years ago

I used git to checkout the ab775e2 commit and built it using Msys2. The djxl.exe from that does in fact throw an error when trying to write pixels to an image.

Cody@Cod-Desktop MINGW64 ~/build_ab775e2/tools $ ./djxl.exe Atari\ 5200\ -\ Super\ Software\ Poster\ 2.jxl Atari\ 5200\ -\ Super\ Software\ Poster\ 2.jpg JPEG XL decoder v0.7.0 ab775e2 [AVX2,SSE4,SSSE3,Scalar] Read 9839230 compressed bytes. ./tools/box/box.cc:291: JXL_FAILURE: Cannot decode to JPEG without a JPEG reconstruction box ./tools/djxl.cc:210: JXL_FAILURE: Failed to decode JXL to JPEG Warning: could not decode losslessly to JPEG. Retrying with --pixels_to_jpeg... Decoded to pixels. ./lib/jxl/enc_color_management.cc:1076: JXL_ERROR: Failed to make RGB_0.345985;0.360691_0.711098;0.288564;0.177004;0.822111;0.158514;-0.000808_Rel_TF? usable as a color transform destination ./lib/jxl/enc_color_management.h:61: JXL_RETURN_IF_ERROR code=1: cmsdata != nullptr ./lib/jxl/enc_image_bundle.cc:109: JXL_RETURN_IF_ERROR code=1: RunOnPool( pool, 0, rect.ysize(), [&](const size_t num_threads) { return c_transform.Init(ib->c_current(), c_desired, metadata->IntensityTarget(), rect.xsize(), num_threads); }, [&](const uint32_t y, const size_t thread) { float mutable_src_buf = c_transform.BufSrc(thread); const float src_buf = mutable_src_buf; if (is_gray) { src_buf = rect.ConstPlaneRow(ib->color(), 0, y); } else if (ib->c_current().IsCMYK()) { if (!ib->HasBlack()) { ok.store(false); return; } const float JXL_RESTRICT row_in0 = rect.ConstPlaneRow(ib->color(), 0, y); const float JXL_RESTRICT row_in1 = rect.ConstPlaneRow(ib->color(), 1, y); const float JXL_RESTRICT row_in2 = rect.ConstPlaneRow(ib->color(), 2, y); const float JXL_RESTRICT row_in3 = rect.ConstRow(ib->black(), y); for (size_t x = 0; x < rect.xsize(); x++) { mutable_src_buf[4 x + 0] = row_in0[x]; mutable_src_buf[4 x + 1] = row_in1[x]; mutable_src_buf[4 x + 2] = row_in2[x]; mutable_src_buf[4 x + 3] = row_in3[x]; } } else { const float JXL_RESTRICT row_in0 = rect.ConstPlaneRow(ib->color(), 0, y); const float JXL_RESTRICT row_in1 = rect.ConstPlaneRow(ib->color(), 1, y); const float JXL_RESTRICT row_in2 = rect.ConstPlaneRow(ib->color(), 2, y); for (size_t x = 0; x < rect.xsize(); x++) { mutable_src_buf[3 x + 0] = row_in0[x]; mutable_src_buf[3 x + 1] = row_in1[x]; mutable_src_buf[3 x + 2] = row_in2[x]; } } float JXL_RESTRICT dst_buf = c_transform.BufDst(thread); if (!c_transform.Run(thread, src_buf, dst_buf)) { ok.store(false); return; } float JXL_RESTRICT row_out0 = out->PlaneRow(0, y); float JXL_RESTRICT row_out1 = out->PlaneRow(1, y); float JXL_RESTRICT row_out2 = out->PlaneRow(2, y); if (is_gray) { for (size_t x = 0; x < rect.xsize(); x++) { row_out0[x] = dst_buf[x]; row_out1[x] = dst_buf[x]; row_out2[x] = dst_buf[x]; } } else { for (size_t x = 0; x < rect.xsize(); x++) { row_out0[x] = dst_buf[3 x + 0]; row_out1[x] = dst_buf[3 x + 1]; row_out2[x] = dst_buf[3 * x + 2]; } } }, "Colorspace transform") ./lib/jxl/enc_image_bundle.cc:118: JXL_RETURN_IFERROR code=1: CopyTo(Rect(color), cdesired, cms, &color, pool) ./lib/extras/enc/jpg.cc:218: JXL_RETURN_IF_ERROR code=1: TransformIfNeeded( ib, metadata.color_encoding, GetJxlCms(), pool, &ib_store, &transformed) Failed to write decoded image.

I also built the latest commit version (JPEG XL decoder v0.7.0 711382f [AVX2,SSE4,SSSE3,Emu128]) and it seems to decode the same way the newer artifact build (ae95f45) linked above does in that the decoded jpg seems to be more grey instead of more blue like the v0.6.1 release build.

Dogway commented 2 years ago

To add to the discussion, this only happens with png 16-bit as I tested. My guess is this is a problem with XYB conversion in 16-bit. Also wanted to take the opportunity to request for an output bitdepth argument, so for example a 16-bit input can leverage higher bitdepth conversions to ultimately save as dithered 8-bit.

TheDecryptor commented 2 years ago

I think it's something with the embedded colour profile, with the base PNG image I see the dark colours in TweakPNG and Edge, but correct colours in Firefox and the GIMP.

And when converting the PNG file to JXL and back the profile doesn't survive (I don't actually know if that's expected)), djxl attaches a linear sRGB profile to the result.

Funkmiser commented 2 years ago

The PNG was originally a TIFF that came from https://archive.org/details/Atari5200InsertsPosters that I converted using the ImageMagick command: magick mogrify -format png *.tif

The description says that an ICC color profile/file was applied to the images, I didn't really know whether color profiles or EXIF data might be at fault since the (-d 0) png to jxl conversion does work fine and it's the (-d 1) and beyond conversions that don't when viewing the jxls directly or when using djxl to convert jxl to jpg. But I don't know at what point in the pipeline of tiff->png->jxl->jpg something gets thrown off 🤷.

EDIT: Hmmm, actually it mentions without the ICC file support image viewer programs that don't support them will make the image look very dark. That matches up to my experience when using Irfanview to view both the png and jxl where they were darker than when using ImageGlass, but with Irfanview when viewing the incorrect jpg it was even darker than both the png and jxl. Is the (-d 0) cjxl parameter the only one that supports icc files or is the icc file just some extra data that is taken along for a ride through all the conversions.

jonsneyers commented 2 years ago

An ICC profile defines the color space of the pixels, including the transfer function.

By default, cjxl will preserve the ICC profile exactly when doing lossless, while when doing lossy, it will use the ICC profile to interpret the input, but store the actual data in XYB (which is an absolute color space). It will preserve the essential info from the input ICC profile (primaries and transfer curve) so when decoding back to RGB, the original space (or something very close to it) can be used, and an ICC profile can be synthesized that resembles the original ICC profile (though it will not be exactly the same: e.g. name strings and other comments will be lost, some numbers will be rounded differently, etc).

In this case, the issue might be that there could be something corrupt in the input ICC profile that causes it to be interpreted differently by cjxl than by a viewer.

Funkmiser commented 2 years ago

That's why I was doing the whole conversion chain from png->jxl->jpg using cjxl and djxl and doing comparisons between the starting png and jpg. I had tried viewing the incorrect (-d 1) jxl using ImageGlass directly (which uses the Magick.Net dlls to decode jxl) but was getting some crazy results so I was hoping the reference decoder would view the jxl file correctly.

The part that always confuses me is that a distance of 0 converts perfectly and it was only with the others (-d 1 and beyond) that problems began to show, the -d 0 jxl even shows correctly in ImageGlass. The incorrect images are only slightly darker now (as opposed to more blue with v0.6.1 djxl) so I guess it might just be rounding errors at this point if the colors profiles use stuff like that in the conversion process.

I was mainly just trying to figure out what this might be on the libjxl side so I could mention it in the Magick.Net issue I created so they could work on fixing the jxl decoding that they use.

jonsneyers commented 2 years ago

I guess the main problem is this:

./lib/jxl/enc_color_management.cc:1076: JXL_ERROR: Failed to make RGB_0.345985;0.360691_0.711098;0.288564;0.177004;0.822111;0.158514;-0.000808_Rel_TF? usable as a color transform destination

@sboukortt any idea what's going on here?

Dogway commented 2 years ago

@jonsneyers Looks like reading Adobe Wide Gamut when the profile indicates sRGB. Anyway, those coefficients are off by some digits from my references:

0.34567,0.35850
0.73469,0.26531
0.11416,0.82621
0.15664,0.01770
sboukortt commented 2 years ago

I guess the main problem is this:

./lib/jxl/enc_color_management.cc:1076: JXL_ERROR: Failed to make RGB_0.345985;0.360691_0.711098;0.288564;0.177004;0.822111;0.158514;-0.000808_Rel_TF? usable as a color transform destination

@sboukortt any idea what's going on here?

That would explain the old djxl not managing to convert back to the original profile (it is a LUT profile so skcms cannot), but it seems that there is something else going on: lcms2 and skcms interpret the profile differently and give different results when converting from it (it is brighter with lcms).

If I compress the original image with an skcms build of cjxl and then decompress to PNG (which creates a 16-bit linear sRGB PNG), it looks the same as the original in Chrome (which makes sense since Chrome uses skcms too).

If I tweak the lcms version of libjxl so that it doesn’t fall back to interpreting this input as sRGB, then compress with that and decode to PNG as above, the linear sRGB output looks like the original image in GIMP (which uses lcms), or in Chrome after conversion to linear sRGB by ImageMagick (idem).

Essentially, compressing to lossy JXL “hardcodes” the interpretation by either lcms or skcms, which in this case happen to diverge.

AlifianK commented 1 year ago

Grayscale pictures seem to have the same problem. Though when I tried opening it in ImageGlass it crashed as described in ImageGlass' issue 1381.

Artoria2e5 commented 1 year ago

I believe there was a post on r/jpegxl with a PNG containing two ICC profile segments, both from GIMP, reporting similar discoloration.