Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PIZ decompression error with tinyexr 1.0.2 #194

Closed
bitsawer opened this issue Jun 3, 2023 · 5 comments
Closed

PIZ decompression error with tinyexr 1.0.2 #194

bitsawer opened this issue Jun 3, 2023 · 5 comments
Labels

Comments

@bitsawer
Copy link

bitsawer commented Jun 3, 2023

Describe the issue

When updating Godot's tinyexr to 1.0.2, we encountered an error when loading .exr files that use PIZ compression. These files are also created and saved using tinyexr. Changing the compression type from TINYEXR_COMPRESSIONTYPE_PIZ to TINYEXR_COMPRESSIONTYPE_ZIP makes the save / load sequence work again. Some extra info:

  • tinyexr 1.0.2 can write these PIZ files but it can't read them (at least in the form Godot saves them). However, it can read PIZ files created by an earlier version of tinyexr.

  • Older tinyexr than 1.0.2 can read PIZ files created using new 1.0.2. However, the new 1.0.2 can't read the files it itself created, so it looks like a regression?

See the Godot bug report for more info: godotengine/godot#77746

I did some debugging and seems like at least some kind of error is detected at this line in the DecompressPiz function:

tinyexr/tinyexr.h

Line 3203 in 2a4dd61

return false;

After that, LoadEXRImageFromMemory() returns the error code -4 (TINYEXR_ERROR_INVALID_DATA) and this message:

"Invalid/Corrupted data found when decoding pixels."

Of course, it could also be that Godot is doing something wrong when loading the file. Here is the Godot's .exr loading code: https://github.com/godotengine/godot/blob/master/modules/tinyexr/image_loader_tinyexr.cpp

To Reproduce

Build / run Godot and follow the steps in the bug report: godotengine/godot#77746

Alternatively, try to open the attached .exr using tinyexr:

000.zip

Expected behavior

The .exr file should load succesfully.

Environment

  • Windows / Linux / Mac
  • GCC / Clang
@syoyo syoyo added the bug label Jun 3, 2023
@syoyo
Copy link
Owner

syoyo commented Jun 3, 2023

Good find!

The issue was it falsely returns error when input pixels are all zeros.

The reproducible 000.exr has a tile(Bitmap) with all zeros, whose are encoded as

minNonZero = BITMAP_SIZE - 1 // 8191
maxNonZero = 0
bitmap[0] = 0 // after evaluating bitmap[0] &= ~1,  since for (int i = 0; i < nData; ++i) bitmap[data[i] >> 3] |= (1 << (data[i] & 7)); will produce bitmap[0] = 1 when data[i] are all zeros.

tinyexr/tinyexr.h

Line 2971 in 2a4dd61

bitmap[0] &= ~1; // zero is not explicitly stored in

And for some reason I did a wrong fix to return false for minNonZero > maxNonZero condition

https://github.com/syoyo/tinyexr/pull/181/files#diff-9354598a2691180f0ea3ed30160185cc272f5e4371e0d48f133e455f520bdc59R3193

Possible fix

Check if

minNonZero == BITMAP_SIZE - 1 
maxNonZero == 0

when minNonZero > maxNonZero, and allow such a condition(produce all zero pixels)

@bitsawer
Copy link
Author

bitsawer commented Jun 3, 2023

That was fast. The fix seems to work, changing the code to:

minNonZero == BITMAP_SIZE - 1;
maxNonZero == 0;
bitmap[0] == 0xffffffff; // optional

fixes the Godot .exr loading, at least for this and other generated .exr images. I don't have a huge amount of images to test against right now, but I would assume this fix works for other images as well.

Any estimates for tinyexr 1.0.3 etc. release? Meanwhile, we'll probably patch our local tinyexr while waiting for the next release if this fix seems solid.

@syoyo syoyo closed this as completed in 9569185 Jun 3, 2023
@syoyo
Copy link
Owner

syoyo commented Jun 3, 2023

Thanks for testing! > at least for this and other generated .exr images

All other test passes so pushed the fix to release branch(v1.0.3).

@bitsawer Can we add 000.exr to tinyexr regression test files https://github.com/syoyo/tinyexr/tree/release/test/unit/regression ?

@bitsawer
Copy link
Author

bitsawer commented Jun 3, 2023

Thanks for the quick fix! I tested the 1.0.3 with Godot and it fixes the issue.

I generated the 000.exr file, so feel free to use it for whatever you need.

@syoyo
Copy link
Owner

syoyo commented Jun 4, 2023

Thanks! Add 000.exr to regression test case

2978130

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants