search

libjxl / libjxl

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

`--lossy-palette` and progressive mode don't mix #1177

Closed TheDecryptor closed 2 years ago

TheDecryptor commented 2 years ago

Describe the bug When encoding an image with cjxl that combines --lossy-palette with -p, the resulting image decodes incorrectly when using the -s option with djxl

To Reproduce

  1. Run cjxl <input> -m --lossy-palette --palette=0 -p <output1>
  2. Then djxl <output1> -s 2 <output2> (Or any value >1)

Expected behavior I'd expect to either get a lower quality version of the image, or a blank result like regular non-progressive lossless modular

Screenshots This is what I get with -s 8

palette jxl

Environment x86_64 Windows 11, binary from github CI JPEG XL decoder v0.7.0 b6d7ba0 [AVX2,SSE4,SSSE3,Scalar]

ziemek99 commented 2 years ago

Oh, another bug with lossy palette mixing with another incompatible functionality, like in #75 (but this time in the decoder)?

I guess we have to check what else is lossy palette incompatible with...

sboukortt commented 2 years ago

We should probably disallow the combination for now.

jonsneyers commented 2 years ago

Delta palette is also inherently non-progressive and doing Squeeze before or after doing delta palette is not going to change that. It does lead to artistic but undesired glitchy previews since errors will propagate unboundedly when the palette indices are not (yet) exact.

To resolve this, I think we have to decide which option should take precedence: progressive or using lossy palette.

Part of the issue is also that lossy palette is not tied to the distance target at all, and it's the code path where the distance is 0 that is causing trouble here — when the distance is > 0, the encoder already knows it shouldn't do any palette transforms since they will interfere with lossy squeeze. It would perhaps be better to remove/deprecate the encode option to use lossy palette, and instead make the encoder automatically do it (obviously without Squeeze) when the distance target is in a range that is suitable for lossy palette — unless progressive is enabled, in which case it has to do Squeeze instead.