search

darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.77k stars 1.14k forks source link

jxl - check option combinations & bit depth #17487

Open TurboGit opened 1 month ago

TurboGit commented 1 month ago

I just found out that whatever option is used for bit depth (8, 10, 12, 16, 32) the file size is the same and according to GIMP it is 32bit flow version. Is that expected ?

kmilos commented 1 month ago

Is that expected ?

For lossy encoding, yes. JPEG XL is a very different beast to other formats.

file size is the same

This is because starting point is the same - we go straight from our float output buffer, and bpp is basically just a metadata hint to the decoder, but they don't have to respect it (especially if they support float input buffers). The alternative would be to forcefully quantize our float output first, then encode it (but encoding in lossy JPEG XL is done internally in float anyway, so that's why we didn't do it that way; we could if we think explicitly introducing quantization errors is somehow a desired feature...?)

For lossess it is a different story obviously.

TurboGit commented 1 month ago

I have the Quality set to 95% so lossy indeed. So if I put to 100% the bit depth should be effective?

kmilos commented 1 month ago

So if I put to 100% the bit depth should be effective?

Mos def.

TurboGit commented 1 month ago

Mos def.

Sorry I don't parse this :)

TurboGit commented 1 month ago

Also some settings seems to be incompatible. For example 16bit / Float makes JXL to fail. Maybe there is way to improve the interface to avoid selecting incompatible options?

kmilos commented 1 month ago

Sorry I don't parse this :)

https://www.urbandictionary.com/define.php?term=mos+def 😉

For example 16bit / Float makes JXL to fail.

That should be working. At least at some point all combos were valid and working. Maybe change the title and let's track any eventual issue here? Likely depends on libjxl version and highway (libhwy) version...

TurboGit commented 1 month ago

Maybe change the title and let's track any eventual issue here?

Done.

kmilos commented 1 month ago

11/12 combos work here (4.9.0+516~g09e219c19d nightly on Windows 11 and i7-1355U; libjxl 0.10.3, highway 1.2.0)

Indeed, only lossless 16b float results in "could not export ..."

Looks like this combo was introduced recently in https://github.com/darktable-org/darktable/pull/17053 @kiwixz

kiwixz commented 1 month ago

On my system (debian 12, libjxl 0.7), lossy 16b float fails:

./lib/jxl/enc_modular.cc:288: JXL_FAILURE: 0.207843 is losing precision (mant: 54d4cd)
./lib/jxl/enc_modular.cc:288: JXL_FAILURE: 0.207843 is losing precision (mant: 54d4cd)
./lib/jxl/enc_modular.cc:288: JXL_FAILURE: 0.207843 is losing precision (mant: 54d4cd)
./lib/jxl/enc_modular.cc:288: JXL_FAILURE: 0.207843 is losing precision (mant: 54d4cd)
./lib/jxl/enc_modular.cc:590: JXL_FAILURE: Error in float to integer conversion
./lib/jxl/enc_frame.cc:1318: JXL_RETURN_IF_ERROR code=1: modular_frame_encoder->ComputeEncodingData( *frame_header, *ib.metadata(), &opsin, *extra_channels, lossy_frame_encoder.State(), cms, pool, aux_out, frame_header->encoding == FrameEncoding::kModular)
./lib/jxl/encode.cc:496: Failed to encode frame
    32.1444 [imageio_storage_disk] could not export to file: `/tmp/clown.jxl'!

And lossless 16b float crashes:

./lib/jxl/enc_modular.cc:288: JXL_FAILURE: 0.682353 is losing precision (mant: 2eaeae)
./lib/jxl/enc_modular.cc:288: JXL_FAILURE: 0.101961 is losing precision (mant: 50d0b9)
./lib/jxl/enc_modular.cc:288: JXL_FAILURE: 0.407843 is losing precision (mant: 50d0ce)
./lib/jxl/enc_modular.cc:288: JXL_FAILURE: 0.101961 is losing precision (mant: 50d0b9)
./lib/jxl/enc_modular.cc:590: JXL_FAILURE: Error in float to integer conversion
./lib/jxl/enc_frame.cc:1318: JXL_RETURN_IF_ERROR code=1: modular_frame_encoder->ComputeEncodingData( *frame_header, *ib.metadata(), &opsin, *extra_channels, lossy_frame_encoder.State(), cms, pool, aux_out, frame_header->encoding == FrameEncoding::kModular)
./lib/jxl/enc_patch_dictionary.cc:774: JXL_CHECK: EncodeFrame(cparams, patch_frame_info, state->shared.metadata, ib, &roundtrip_state, cms, pool, special_frame.get(), aux_out ? &patch_aux_out : nullptr)
zsh: illegal hardware instruction (core dumped)  ./build/bin/darktable

Do you get a better error on your end ? I see nothing in libjxl doc or code that suggests lossless 16b float is not supported; but if we can't get it working I guess we need to disable it.

kmilos commented 1 month ago

I'm only seeing the log (no time to build and run debug, sorry):

[jxl] libjxl call failed with err 1 (src/imageio/format/jxl.c#L370)

Edit: same w/ libjxl 0.11.0...

kiwixz commented 1 month ago

Opened libjxl/libjxl#3844 to see what they plan to do with this.

jonnyawsom3 commented 1 month ago

Came here from the libjxl repo and read through the related issues and PRs about lossless float JXLs. Thought I would bring up this issue too, since while I tested it with 32 bit floats, it could affect 16 bit floats too https://github.com/libjxl/libjxl/issues/3511.

As far as I'm aware we've only tested floats using PFM, so while lossless 16 bit is supported, we've never had a 32 bit input requested to be stored as lower. Should be a relatively simple fix, but we'll see what they decide on.

kmilos commented 1 month ago

@kiwixz Looks like there are two options currently:

  1. Disable fp16 lossless for the time being
  2. Do an intermediate conversion using Imath (like we do for e.g. TIFF and EXR for this case only. However, looks like there is then https://github.com/libjxl/libjxl/issues/3881
kiwixz commented 1 month ago

Nice find, it seems clear that lossless float16 is largely untested. I'll write a patch to disable it.