libjxl icon indicating copy to clipboard operation
libjxl copied to clipboard

djxl PNG Conversion Acting Up

Open gianni-rosato opened this issue 3 years ago • 38 comments

Describe the bug When converting a particular JXL image to PNG, the PNG has noticeable color issues with patches of discoloration.

To Reproduce Download this

djxl theimage.jxl out.png

Compare the png to the JXL.

Expected behavior The PNG would be visually identical to the JXL, or at least close enough where I wouldn't see obvious discoloration or where the PNG is earning horrible ssimu2 scores.

Screenshots If applicable, add screenshots or example input/output images to help explain your problem.

Environment

  • OS: Linux
  • Compiler version: gcc (GCC) 12.2.1 20230201
  • CPU type: x86_64
  • cjxl/djxl version string: djxl v0.9.0 13caf6f8 [AVX2,SSE4,SSSE3,SSE2]

Additional context The problem persists with other versions of cjxl/djxl when using the source PNG (which is almost 100mb).

gianni-rosato avatar Mar 11 '23 05:03 gianni-rosato

Notably, the JXL file has an attached ICC profile and is XYB encoded. djxl is outputting a PNG in Linear Light rather than in sRGB.

Traneptora avatar Mar 11 '23 13:03 Traneptora

If you want the source PNG, let me know

gianni-rosato avatar Mar 11 '23 21:03 gianni-rosato

It is the fact that djxl uses RGB_D65_SRG_Rel_Lin profile there.

If the JXL and decoded PNG look significantly visually different, that's a problem of non-implemented (or not working) color management support of the viewer application.

The functionality of djxl is not technically wrong.

Color profile used to decode can be changed:

djxl --color_space=RGB_D65_SRG_Rel_SRG imagetest15-Q70.jxl out2.png

novomesk avatar Mar 13 '23 12:03 novomesk

When inputting the weird PNG into ssimulacra2, it doesn't understand it & gives it a terrible score

gianni-rosato avatar Mar 13 '23 22:03 gianni-rosato

Did you try the "keep original" color space option when exporting from darktable? The JPEG XL file will be considerably larger, but it'll preserve the profile. Otherwise, it discards it when encoding and only decodes to linear RGB (unless you're doing lossless, i.e. quality 100 as well when you get out what you put in).

kmilos avatar Mar 14 '23 08:03 kmilos

Did you try the "keep original" color space option when exporting from darktable? The JPEG XL file will be considerably larger, but it'll preserve the profile. Otherwise, it discards it when encoding and only decodes to linear RGB (unless you're doing lossless, i.e. quality 100 as well when you get out what you put in).

To clarify: it does not discard it when encoding. The ICC file is preserved in the .jxl file (as indicated by jxlinfo). It’s just that the decoder, which we didn’t want to have depend on an external CMS, does not implement converting back to an arbitrary ICC profile, and instead defaults to converting back to linear sRGB in such instances, which I agree is a suboptimal default for 8-bit integer output. (It would be acceptable for OpenEXR output, for example, as this uses 16- or 32-bit floating point.)

sboukortt avatar Mar 27 '23 14:03 sboukortt

To clarify: it does not discard it when encoding. The ICC file is preserved in the .jxl file (as indicated by jxlinfo).

I didn't mean the ICC profile, but the actual color space of the encoded and decoded pixel data, i.e. avoiding the XYB path through the use_original option ("keep original" in darktable).

The sRGB conversion is I guess an ok-ish compromise for now, but I haven't checked what is actually tagged as a profile in the resulting PNG, is the original input ICC replaced w/ an sRGB one?

kmilos avatar Mar 27 '23 18:03 kmilos

There are three things that would fix this, and I think we need to do all of them:

  1. Make cjxl recognize Display P3 and use the Enum representation for it rather than a generic icc profile. This would help for compression, and it circumvents the djxl bug.
  2. Make djxl default to writing a 16-bit png if the image is in xyb colorspace.
  3. Make libjxl decode do full color management by default (make it a special build flag to build without dependency on external CMS).

jonsneyers avatar Mar 27 '23 18:03 jonsneyers

I have had better experiences with 15 bit than with 16 bit. It produces a better SSIMULACRA2 value

dominikhlbg avatar Mar 27 '23 19:03 dominikhlbg

There are three things that would fix this, and I think we need to do all of them: 2. Make djxl default to writing a 16-bit png if the image is in xyb colorspace.

I would disagree with always decoding XYB to 16-bit PNGs. I think it should only do this if there's an attached ICC profile and no enum values for it, or if the enum values suggest it should. If there's enum values tagged for an SDR gamut & transfer (e.g. sRGB) and the file is tagged as 8-bit, then I think djxl should output 8-bit PNGs.

Traneptora avatar Mar 27 '23 19:03 Traneptora

If i have an 8-bit PNG as input which i encode with jxl , then decode again but then i don't get 8-bit again because ssimulacra2 generates minus values, then something else is not working well?

dominikhlbg avatar Mar 27 '23 19:03 dominikhlbg

If writing to an integer format, we should also not use sRGB primaries as it could cause gamut clipping (or worse). Which primaries to use is not obvious but P3 and Rec. 2020 would probably both be safer choices. (Not perfect but we don’t need to have found “perfect” to implement an improvement.)

Today, I started writing some logic in that direction but haven’t gotten it to work yet. What I was going for was along the lines of:

If the color space is described by an ICC profile rather than enums, and no color space hint was provided on the command line, then request the output in a color space with:

  • white point: D65;
  • transfer function: linear if floating point output, otherwise sRGB;
  • primaries: sRGB (Rec. 709) if floating point output, otherwise P3 if 8-bit output or Rec. 2020 if 16-bit output;
  • rendering intent: relative.

Any thoughts on that?

Make cjxl recognize Display P3 and use the Enum representation for it rather than a generic icc profile. This would help for compression, and it circumvents the djxl bug.

Is that what the embedded profile is here? I’m surprised we don’t recognize it.

sboukortt avatar Mar 27 '23 20:03 sboukortt

Make cjxl recognize Display P3 and use the Enum representation for it rather than a generic icc profile. This would help for compression, and it circumvents the djxl bug.

Is that what the embedded profile is here? I’m surprised we don’t recognize it.

It seems to be Adobe RGB:

$ cd-iccdump profile.icc
icc:
Header:
  Size          = 888 bytes
  Version       = 2.4
  Profile Kind  = display-device
  Colorspace    = rgb
  Conn. Space   = xyz
  Date, Time    = 2023-03-09, 22:19:49
  Flags         = Embedded profile, Use anywhere
  Dev. Attrbts  = reflective, glossy
  Rndrng Intnt  = perceptual
  Creator       = lcms
  Profile ID    = (null)
tag 00:
  sig   'desc' [0x64657363]
  size  160
  type  'desc' [0x64657363]
Text:
  **_**:        Adobe RGB (compatible) [92 bytes]

tag 01:
  sig   'cprt' [0x63707274]
  size  22
  type  'text' [0x74657874]
Text:
  **_**:        Public Domain [56 bytes]

tag 02:
  sig   'wtpt' [0x77747074]
  size  20
  type  'XYZ ' [0x58595a20]
XYZ:
  X:0.964203 Y:1.000000 Z:0.824905

tag 03:
  sig   'chad' [0x63686164]
  size  44
  type  'sf32' [0x73663332]

tag 04:
  sig   'rXYZ' [0x7258595a]
  size  20
  type  'XYZ ' [0x58595a20]
XYZ:
  X:0.609741 Y:0.311111 Z:0.019470

tag 05:
  sig   'bXYZ' [0x6258595a]
  size  20
  type  'XYZ ' [0x58595a20]
XYZ:
  X:0.149185 Y:0.063217 Z:0.744553

tag 06:
  sig   'gXYZ' [0x6758595a]
  size  20
  type  'XYZ ' [0x58595a20]
XYZ:
  X:0.205276 Y:0.625671 Z:0.060867

tag 07:
  sig   'rTRC' [0x72545243]
  size  14
  type  'curv' [0x63757276]
Curve:
  Curve is gamma of 2.199219

tag 08:
  sig   'gTRC' [0x67545243]
  link  'rTRC' [0x72545243]
tag 09:
  sig   'bTRC' [0x62545243]
  link  'gTRC' [0x67545243]
tag 10:
  sig   'chrm' [0x6368726d]
  size  36
  type  'chrm' [0x6368726d]

tag 11:
  sig   'dmdd' [0x646d6464]
  size  120
  type  'desc' [0x64657363]
Text:
  **_**:        Adobe RGB [40 bytes]

tag 12:
  sig   'dmnd' [0x646d6e64]
  size  120
  type  'desc' [0x64657363]
Text:
  **_**:        darktable [40 bytes]

It makes sense that we currently don’t recognize it as we make no attempt to recognize pure gamma curves. Perhaps it would make sense to either try a few predefined values (including the Adobe one), or figure out the gamma value and check the fit. It won’t solve the issue of existing .jxl files that already have such an ICC profile, nor the encoding of images with a more exotic profile, but it would at least reduce the impact of the issue.

sboukortt avatar Mar 28 '23 13:03 sboukortt

Here some images with the bug http://libwebpjs.hohenlimburg.org/avif-jxl-bench/charts_avif-jpegxl-bad.html

dominikhlbg avatar Mar 28 '23 13:03 dominikhlbg

It makes sense that we currently don’t recognize it as we make no attempt to recognize pure gamma curves.

Why not? It seems very doable if there's a curv tag rather than a gamma LUT. Adobe RGB can be represented with gamma TRC and custom primaries, so I feel like we should do that.

Traneptora avatar Mar 28 '23 16:03 Traneptora

The best answer I can give to “why not” is pretty much “because we have not implemented it so far”, but I have just started giving this a try (#2341). I have it working with skcms, now I just need to implement the lcms2 version. (Or I suppose we could already merge the skcms part, but something makes me uneasy about not having feature parity between the two.)

sboukortt avatar Mar 29 '23 11:03 sboukortt

#2341 has now been merged. It doesn’t solve the issue with this specific .jxl file, but at least, if the same source file is compressed to JPEG XL again, it should create a file without the issue.

sboukortt avatar Mar 30 '23 14:03 sboukortt

I'm a beginner at this, sorry if this info isn't relevant.

One more image that decodes to something that does not look like the original, but a dark and dull looking PNG. I used djxl v0.9.0 96d76a3 from: https://artifacts.lucaversari.it/libjxl/libjxl/2023-10-17T11%3A15%3A45Z_56949f8a9772c0139803c25e46bd8ccf8ef93f7e/

Sample image:

  • Found here: https://people.csail.mit.edu/ericchan/hdr/hdr-jxl.php
  • JXL: https://people.csail.mit.edu/ericchan/hdr/jxl_images/20140606_102418_IMGP0297.jxl
  • Reference JPEG image / how it should look, i think: https://people.csail.mit.edu/ericchan/hdr/jxl_images/20140606_102418_IMGP0297.jpg

Reference JPEG image:

Click to expand

sample_jpeg

First tried djxl in.jxl out.png --bits_per_sample=8 (with or without --bits_per_sample=8 made no visual difference), it created a dark and dull result.

Then opened original JXL in GIMP to look for metadata, it said Exif.Photo.ColorSpace: sRGB. Then tried djxl in.jxl out.png --color_space=RGB_D65_SRG_Per_SRG --bits_per_sample=8, it looked good compared to the reference JPG file, and how GIMP decodes the JXL.

Result from both commands in a screenshot:

Click to expand

No color space defined on the left, RGB_D65_SRG_Per_SRG on the right.

image

I also tried jxlinfo in.jxl, it said nothing about sRGB:

JPEG XL file format container (ISO/IEC 18181-2)
JPEG XL image, 1638x2048, lossy, 16-bit RGB
Color space: RGB, D65, Rec.2100 primaries, PQ transfer function, rendering intent: Perceptual
Uncompressed Exif metadata: 818 bytes
Brotli-compressed XML  metadata: 5664 compressed bytes
unknown box: type: "hrgm" size: 147802
layer: full image size, name: "main"

o-l-a-v avatar Oct 17 '23 14:10 o-l-a-v

Two images that v0.9.1 still have problems with.

Both djxl input.jxl output.png and djxl input.jxl output.png --bits_per_sample=8 results in an output with washed out colors.

image

I think this is relevant to why colors looks off in Thorium (1) and Cromite (2) too.

  1. https://github.com/Alex313031/thorium-libjxl/issues/18
  2. https://github.com/uazo/cromite/issues/752

GIMP | Cromite v121.0.6167.101 | Thorium v120.0.6099.235

Screenshot 2024-01-26 122810

tree


Some other apps that had the same problem but now have solved it:

  • Paint.NET with pdn-jpegxl: https://github.com/0xC0000054/pdn-jpegxl/issues/1
  • XnSoft XnView MP: https://forum.xnview.com/viewtopic.php?f=104&t=46279

o-l-a-v avatar Jan 26 '24 11:01 o-l-a-v

Do note that this may be an error with the actual PNG viewer. djxl input.jxl output.png for the Jungle image results in a PNG file that has cICP tags setting the color space information. If your viewer doesn't cICP, then the image will look like this: test-libjxl-8bit

Traneptora avatar Jan 26 '24 20:01 Traneptora

Even then, we used to also generate a fallback ICC chunk. That we don’t anymore is a regression, apparently introduced by #2635. I’ll have a closer look next week.

sboukortt avatar Jan 26 '24 20:01 sboukortt

Likewise, the image on the right looks exactly like what you'd get if you interpret the BT2100/PQ png that was output as though it were sRGB. You can test this by using a PNG editor (such as TweakPNG or umbrielpng) to strip the cICP chunk and replace it with an sRGB chunk.

Traneptora avatar Jan 26 '24 20:01 Traneptora

Even then, we used to also generate a fallback ICC chunk. That we don’t anymore is a regression, apparently introduced by #2635. I’ll have a closer look next week.

Though when you do be careful with sRGB, because the PNG spec isn't clear what happens if you have both an sRGB chunk and an iCCP chunk. It recommends against it, but it doesn't forbid it, or explain what happens. Contrast with cICP which it says explicitly overrides iCCP when present. IMO fallback ICC profile is not needed in the sRGB case anyway.

Traneptora avatar Jan 26 '24 20:01 Traneptora

PNG spec isn't clear what happens if you have both an sRGB chunk and an iCCP chunk. It recommends against it, but it doesn't forbid it

~~I think it does forbid it.~~ But that doesn't mean it doesn't happen, or if various implementations deal with it consistently.

Edit: I take that back, sorry, just re-read it.

kmilos avatar Jan 27 '24 14:01 kmilos

IMO fallback ICC profile is not needed in the sRGB case anyway.

Right, in the sRGB case, we didn’t, only in the cICP-but-not-sRGB case.

Part of the motivation was to provide some fallback tone mapping (#1890).

sboukortt avatar Jan 27 '24 14:01 sboukortt

Do note that this may be an error with the actual PNG viewer. djxl input.jxl output.png for the Jungle image results in a PNG file that has cICP tags setting the color space information. If your viewer doesn't cICP, then the image will look like this:

Darktable is said to support cICP since 4.2.0: https://www.darktable.org/2022/12/darktable-4.2.0-released/

With Darktable v4.6.0, opening the PNG results in a very dark looking image:

image

I tried the latest versions of IrfanView, XnView MP, Windows Photos app (less washed out than the others actually), GIMP, Krita, ImageGlass, JPEGView. All view this image like the example you posted. Decoding/transcoding to JPG djxl input.jxl output.jpg produces visually the same output as to PNG.

Do we have any example of a program that can properly view PNG with cICP?

It doesn't make sense to decode to some exotic output that no current image viewers and editors can view properly IMO.

Edit: Including --icc_out=output.icc made programs supporting external ICC look correct, so having ICC as fallback for programs that does not yet support cICP sounds good? As sboukortt wrote.

o-l-a-v avatar Jan 28 '24 10:01 o-l-a-v

With Darktable v4.6.0, opening the PNG results in a very dark looking image:

If this is a PQ or HLG transfer curve image, darktable does not do any tone mapping, and just pulls the image down so max stays at 1.0 (which makes an HDR image dark obviously). Use the exposure module to push it back up (+6.33 EV or so for PQ content so 1.0 is no longer 10000 nits but more like 120 nits).

P.S. darktable is not a general purpose "viewer", but a scene-referred "raw developer", so one cannot/does not expect an already rendered final result upon import (especially not for HDR content) - exposure, tone mapping and highlight reconstruction are explicit operations/choices made by the artist.

kmilos avatar Jan 28 '24 12:01 kmilos

Do we have any example of a program that can properly view PNG with cICP?

Chromium and FFmpeg both support cICP chunks.

Traneptora avatar Jan 28 '24 16:01 Traneptora