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

TIFF: Add support for SAMPLEFORMAT_COMPLEXINT/SAMPLEFORMAT_COMPLEXIEEEFP #1026

Merged
merged 1 commit into from
Oct 6, 2024

Conversation

don-vip
Copy link
Contributor

@don-vip don-vip commented Sep 29, 2024

Fixes #884

I didn't add tests in the commit (although I wrote one) because the smallest sample file I could find is 25 Mb... Let me know if I should add it anyway:

--- a/imageio/imageio-tiff/src/test/java/com/twelvemonkeys/imageio/plugins/tiff/TIFFImageReaderTest.java
+++ b/imageio/imageio-tiff/src/test/java/com/twelvemonkeys/imageio/plugins/tiff/TIFFImageReaderTest.java
@@ -194,7 +194,9 @@ public class TIFFImageReaderTest extends ImageReaderAbstractTest<TIFFImageReader
                 new TestData(getClassLoaderResource("/tiff/planar-yuv420-jpeg-uncompressed.tif"), new Dimension(256, 64)), // YCbCr, JPEG coefficients, uncompressed, striped
                 new TestData(getClassLoaderResource("/tiff/planar-yuv420-jpeg-lzw.tif"), new Dimension(256, 64)), // YCbCr, JPEG coefficients,LZW compressed, striped
                 new TestData(getClassLoaderResource("/tiff/planar-yuv410-jpeg-uncompressed.tif"), new Dimension(256, 64)), // YCbCr, JPEG coefficients, uncompressed, striped
-                new TestData(getClassLoaderResource("/tiff/planar-yuv410-jpeg-lzw.tif"), new Dimension(256, 64)) // YCbCr, JPEG coefficients,LZW compressed, striped
+                new TestData(getClassLoaderResource("/tiff/planar-yuv410-jpeg-lzw.tif"), new Dimension(256, 64)), // YCbCr, JPEG coefficients,LZW compressed, striped
+                // Complex sample formats
+                new TestData(getClassLoaderResource("/tiff/CAPELLA_C13_SM_SLC_VV_20240924045643_20240924045645.tif"), new Dimension(1871, 5026)) // Complex signed integer
         );
     }

@haraldk
Copy link
Owner

haraldk commented Oct 6, 2024

Thanks for providing this! 👍🏻

So, no special handling of these complex types are needed? 😮

I will add a full review as soon as possible, I'm back in the office from Monday. But looks like this can be safely merged.

Can you send the sample file? Maybe we can trim it a bit in a hex editor, if it's uncompressed or tiled/striped. But I agree, 25 MB for testing this is a but much.

UPDATE: I see you have linked the file in your original description, never mind. 😀

Copy link
Owner

@haraldk haraldk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍🏻

But can you please also fix the exception messages as suggested, as we now support more sample formats?

We really should have tests, but I'm researching the options. It's not critical though, as there's no special logic for this compared to the non-complex sample formats.

@don-vip don-vip force-pushed the tiff-sample-formats branch from e32762e to 4897d55 Compare October 6, 2024 11:02
@don-vip
Copy link
Contributor Author

don-vip commented Oct 6, 2024

Thanks for the review! Honestly I don't know if more changes are needed, those simple changes were enough for me to read the sample file.

@haraldk haraldk merged commit 734b908 into haraldk:master Oct 6, 2024
18 checks passed
@haraldk
Copy link
Owner

haraldk commented Oct 6, 2024

I merged the PR. Thanks again!

@don-vip don-vip deleted the tiff-sample-formats branch October 6, 2024 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TIFF: Support SAMPLEFORMAT_COMPLEXINT (5)
2 participants