syoyo / tinydng

Header-only Tiny DNG/TIFF loader and writer in C++
MIT License
146 stars 30 forks source link

[BUG] Overexposed 16 bit whitelevel images cause LJ92_ERROR_CORRUPT #37

Closed eszdman closed 9 months ago

eszdman commented 9 months ago

Overexposed dng's with LJPEG and whiteLevel of 65535 cause an error LJ92_ERROR_CORRUPT

You can see my fork of the supposed solution to the problem

Dng data for testing test dng.tar.gz

syoyo commented 9 months ago

👀

Could you please post github (permanent) link to your fix?

eszdman commented 9 months ago

👀

Could you please post github (permanent) link to your fix?

Multiple changes starting from https://github.com/eszdman/tinydng/blob/release/tiny_dng_loader.h#L1231

syoyo commented 9 months ago

Thanks!

syoyo commented 9 months ago

Printing value here https://github.com/syoyo/tinydng/blob/13c613cf351e4c0b4188f362f4284cd7fadb71da/tiny_dng_loader.h#L1234 gives:

Px, diff = 27961, -32232
left = -4271

Possible fix are:

syoyo commented 9 months ago

@eszdman Do you see any pixel diff with your fix left = (u16)(left); compared to other DNG loader?(e.g. RawTherapee)

eszdman commented 9 months ago

@eszdman Do you see any pixel diff with your fix left = (u16)(left); compared to other DNG loader?(e.g. RawTherapee)

I don't see any, comparing to clamped version(I tried it and have artifacts around lamp)

eszdman commented 9 months ago
  • wrap around left value left = (u16)(left % 65536); as done in original lj92_decode

Type casts also doing this wrap, it's not necessary

syoyo commented 9 months ago

I am reading lj92 spec...

https://www.w3.org/Graphics/JPEG/itu-t81.pdf

And p.134 describes

For simplicity of implementation, the divide by 2 in the prediction selections 5 and 6 of Table H.1 is done by an arithmetic-right-shift of the integer values. The difference between the prediction value and the input is calculated modulo 2^16. In the decoder the difference is decoded and added, modulo 2^16, to the prediction

so we add left = (u16)(left % 65536) (or cast to u16) for each prediction(left) calculation, and no value clamping required.

syoyo commented 9 months ago

FYI I use left = (u16)(left % 65536); for the readability.