cgohlke / imagecodecs

Image transformation, compression, and decompression codecs
https://pypi.org/project/imagecodecs
BSD 3-Clause "New" or "Revised" License
117 stars 23 forks source link

lj92 decoder is broken for SSSS=16 #59

Closed esoha-nvidia closed 1 year ago

esoha-nvidia commented 1 year ago

The lj92 decoder and also the source code that it is based on are broken for the case of SSSS=16.

esoha-nvidia commented 1 year ago

I have made the fix already here. If you like, I can make one for you, too, and perhaps add a test to prove it.

cgohlke commented 1 year ago

Thanks for the report. I can apply the patch but a test or image would be useful.

cgohlke commented 1 year ago

There is a more actively maintained version of lj92 at https://github.com/ilia3101/MLV-App/blob/master/src/mlv/liblj92/lj92.c. It seems to include a fix for this issue. Can you confirm? I'll probably switch to that source.

For reference, there's yet another version at https://github.com/syoyo/tinydng/blob/master/tiny_dng_loader.h#L480

esoha-nvidia commented 1 year ago

Why do you say that it seems to include a fix? Where is that fix?

esoha-nvidia commented 1 year ago

I see it now: https://github.com/ilia3101/MLV-App/commit/170ec6436cedc4e33a1a7efa221889138281e127

Yup., that's the issue. I don't know if the fix is correct but it might be.

esoha-nvidia commented 1 year ago

How do you run tests on this repo? If you have unit testing, I can provide a file that you can use for your testing.

I tried this but it didn't work:

python3 setup.py test
cgohlke commented 1 year ago

How do you run tests on this repo?

Run python -m pytest tests after an inline build or run test_imagecodecs.py directly. Anyway, building imagecodecs is not straightforward.

I can provide a file that you can use for your testing.

Yes, please.

After applying https://github.com/ilia3101/MLV-App/pull/221 and some other changes, I am now able to read most LJPEG files I could find (not that many).

esoha-nvidia commented 1 year ago

From the URL https://www.kaggle.com/competitions/rsna-breast-cancer-detection/data you can download a large dataset which includes the file 9989/439796429.dcm which has inside of it an LJ92 file which I have saved as 439796429. (Trying to download and decode the dataset yourself will be a pain.)

I can confirm that, in my code, this file does not work with the old lj92 but it does work after applying something similar to https://github.com/ilia3101/MLV-App/pull/221 (we did it a different way but it's the same idea).

Can you confirm that your code, on the above file, is not working without the change and then working after it?

Before the change, the first few pixels were incorrectly:

23405
23411
23411
23411
23411
23411
23411
23411
23411

After the change, the first few pixels are:

0
0
0
0
0
0
0
0

Also, from where you do have other lj92 files? We'd like to try them out, too!

cgohlke commented 1 year ago

Thank you for the test image. I have made several changes to the vendored lj92.c file in imagecodecs and can confirm that it can now correctly decode your test image.

Also, from where you do have other lj92 files? We'd like to try them out, too!

I usually extract them from DNG and DCM files "found on the internet".

I am attaching the current test images and lj92.c code: ljpeg.zip