Potential problem with compressed high ISO CR3

[guigri]

darktable 4.6.0 uses LibRaw and displays compressed CR3 images in very poor quality under certain circumstances.
This was observed on compressed RAWs created with a Canon EOS R6 Mark II with high ISO (25.600) values.
These images have clear stair-stepping effects and severe loss of detail.

In a certain way, the problem can't be a bug because the EOS R6 Mark II is not yet officially supported by LibRaw. However, as I understand it, the lack of support is more of a color management issue and less of a problem that CR3 cannot be processed in general. Also, I've only noticed these issues with high ISO images so far. You should at least keep an eye on the issue while making LibRaw officially available for the EOS R6 Mark II.

darktable screenshot of a compressed CR3 (CRAW) taken with EOS R6 Mark II:

darktable screenshot of an uncompressed CR3 with the exact same camera settings otherwise:

The problem has already been discussed in detail at darktable and it seems to be an issue with LibRaw. The problem was also identified with RawTherapy, which also uses LibRaw.
See previous discussion at darktable with lots of information and examples linked there:
darktable-org/darktable#15955.

If you convert the CR3 images to DNG, then the DNG images are displayed normally by darktable.
The CR3 images also look as they should in Canon Digital Photos Professional.

[LibRaw]

Here is 1st sample from mentioned thread, rendered image + RAW histogram (RawDigger):

Here is same file converted by Adobe DNG converter and also opened in RawDigger:

I do not see any difference in rendering and (more important) in the RAW histogram.

And, yes, EOR R6 m2 is NOT supported by actual LibRaw 0.21.2 so there are no guarantees that the metadata is retrieved correctly (screenshots above are from RawDigger w/ EOS R6 m2 support).

The next time you report a problem, please compile everything into one complete report. Don’t suggest that we go to another forum, follow another link from there (with a password) and download something from there. Thank you in advance.

[cytrinox]

I can confirm that there is some sort of problem.
There is an shl overflow during inverse quantization located here: https://github.com/LibRaw/LibRaw/blob/21368133a94fbc35f594112a737d36f7ce65c7c0/src/decoders/crx.cpp#L2035C77-L2035C77
and in dnglab located here: https://github.com/dnglab/dnglab/blob/9211212ed68a82494adc58aab4fc20badf004ad6/rawler/src/decompressors/crx/iquant.rs#L143

quantVal is too big (in case of the file mentioned in this issue: always 36 or 40) so there is an shift overflow.

I've not tested it with libraw itself but I can reproduce all this problems with dnglab/rawler. If I filter out too big shift values and assume 1 in such cases, the image output looks "okay".

Maybe this issue (in libraw context like darktable issues) can only be triggered by some compiler or code optimizations which has an impact on the output of the shl operator?

Same problem for the issue reported here: https://discuss.pixls.us/t/cr3-import-into-darktable-4-6-0/41172/7
There is again a problematic quantVal of 36 (40 is missing).

[cytrinox]

Here are some more results:

Problematic line(s) are:

*qStepTbl = q_step_tbl[quantVal % 6] * (1 << (quantVal / 6 + 26));

For quantVal = 36 or quantVal = 40, there is an overflow because it results in: 1 << 32 which is 0 and thus the whole qstep value is 0 as it is multiplied against 0.
Image when allowing 0 as multiplier:

When replacing shift left by rotate left, multiplier is 1 and image looks correct:

And same issue for the image linked in this issue when multiplier is 0:

@LibRaw Please excuse that I've not using libraw directly but debugging the problem with dnglab was so much easier for me 😄
Can you confirm that using 0 or 1 as multipliers triggers the same problem in RawDigger?
I've no better idea than bit-rotate, but it seems to fix the problem on my side. Please let me know if you find a better fix.

PS: in libraw the relevant line is used 3 times (for each image level 1..3).

Edit: CR3 file for first example: https://discuss.pixls.us/uploads/short-url/6hX4kSJfZLX1VTer9DpteiQPRph.CR3

[LibRaw]

Thank you for your investigation.

Here is sample file provided, rendered with RawDigger (actual version w/ current LibRaw)

We'll investigate different compiler behavior in nearest future

[LibRaw]

Investigated via debugger, disassembly and Intel Software manual

1. Visual Studio (we use) generates shl eax, cl instruction here (note: 32-bit register used, not 64)
2. Quote from Intel Software Manual (Vol 2B, page 4-593):

The destination operand can be a register or a memory location. The count operand can be an immediate value or
the CL register. The count is masked to 5 bits (or 6 bits with a 64-bit operand). The count range is limited to 0 to
31 (or 63 with a 64-bit operand).

So, for 32-bit code shl eax,cl(=0x20) is equal to NOP :)

Conclusion: 0x1f mask should be added to the shift value to allow accurate 64-bit execution.

[cytrinox]

If I understand correctly, RawDigger is compiled as 32 bit app and the shift becomes a NOP on 32 bit targets. But on 64 bit it should still be a 32 bit operand (quantVal), so why is it zero in this case?

BTW, I assume the code can be simplified because a * (1 << b) is equal to a << b.

*qStepTbl = q_step_tbl[quantVal % 6]  << ((quantVal / 6 + 26) & 0x1f);

To be honest - I've no idea why such big values should be used in this IQ step. In this branch, the shift bits is always >=26.
I've checked my sample file repository yesterday and this if-branch is never triggered except for the two provided CR3 files in this issue. I will check all my R5 raws (>10.000 files) in the next few days.

[LibRaw]

RawDigger is compiled as 64-bit app.
But the code generated for shift operates with 32-bit eax.

[cytrinox]

FYI: cytrinox/dnglab@726584f

[LibRaw]

FYI: cytrinox/dnglab@726584f

Do I understand correctly that wrapping_shl for 32-bit type is val << (shift & 0x1f) ?

[cytrinox]

Yes.

wrapping_shl() Panic-free bitwise shift-left; yields self << mask(rhs), where mask removes any high-order bits of rhs that would cause the shift to exceed the bitwidth of the type.

[LibRaw]

Although we're still unable to reproduce the problem with any compiler we use, this patch will fix it even if (wrong) 64-bit shift generated: e231b01