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

Add tests for opening 2-5 layer uint16 greyscale TIFFs #1839

Closed
wants to merge 2 commits into from

Conversation

mairsbw
Copy link
Contributor

@mairsbw mairsbw commented Apr 19, 2016

These tests currently fail as support isn't available for these images. I'd like to add support for multiple-channel uint16 greyscale images to Pillow and writing tests is the first step. I'm not certain if Pillow is well-suited to these datatypes currently (given discussion from #1828, #1824, and #1819). I don't see a reason by Pillow shouldn't support an arbitrary number of channels for images, as it's quite useful for scientific imaging purposes, but my application only needs up to 5-channel uint16 support, so this just tests up to that.

@mairsbw
Copy link
Contributor Author

mairsbw commented Apr 19, 2016

A common use case for multiple greyscale channels is in working with multispectral/hyperspectral data, which is going to quickly become more commonplace. We expect 5 channels to really be a minimal value for the number of channels we'll be working with going forward. Should be up to 8/10 within the next couple of years. While images are generally recorded individually, they are commonly stitched into a single multi-layer GeoTIFF image. This is where Pillow support would come in.

@mairsbw
Copy link
Contributor Author

mairsbw commented Apr 19, 2016

I've also added the first commit that moves things closer to working by supporting these TIFFs in the open() function.

for j in range(i):
current_value = base_value + j
im.seek(j)
self.assertEqual(im.getpixel((0, 0)), current_value)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that this test is doing what you think it's doing.

The seek command is paging through the tiff, as if going from frame to frame in an animation, and looking for a single integer from the pixel location. That's not the kind of tiff you're providing. (and it's currently supported, if that's what you have for actual images)

The tiffs in the test are stored as 2,3,4,5 channel tiffs, where each pixel is an n tuple.

Furthermore, the last channel in each case is 0, so for the 2 channel image, the pixel would be (4660, 0), and the 5 would be (4660, 4661, 4662, 4663, 0).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I was confusing the GDAL API and Pillow's as GDAL uses seek() for TIFFs to move between bands. It looks like Pillow doesn't do the same.

So if I instead use the test:


    def test_open_tiff_uint16_multiband(self):
        """Test opening multiband TIFFs and reading all channels."""
        base_value = 4660
        for i in range(2, 6):
            infile = "Tests/images/uint16_{}_{}.tif".format(i, base_value)
            im = Image.open(infile)
            im.load()
            print(im.getpixel((0,0)))
            print(im.split())
        self.assertEqual(True, False) # So that the test will print stdout & stderr

then I see the following as output:

4660
(<PIL.Image.Image image mode=I;16 size=10x10 at 0xAD5BA73E48>,)
4660
(<PIL.Image.Image image mode=I;16 size=10x10 at 0xAD5BA73CC0>,)
4660
(<PIL.Image.Image image mode=I;16 size=10x10 at 0xAD5B921CF8>,)
4660
(<PIL.Image.Image image mode=I;16 size=10x10 at 0xAD5B921F60>,)

This this leads me to think that these multiband images are actually only loading as single-channel TIFFs. Is that correct?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that is what the 'I;16' mode means in the open support that you added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I figured, but I thought it was a start. Is there a way to signify greyscale bands in TiffImagePlugin.py:OPEN_INFO or is this going to require some more extensive changes?

Copy link
Member

Choose a reason for hiding this comment

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

No, there is literally no support for multiband images > 8bpp. Meaning, there's nothing in the unpacking, storage or access layers, let alone the various manipulations that would be required to have meaningful support of these images.

At the very least, it's going to require changes to Unpack.c, Pack.c, Access.c, Storage.c, Convert.c, and likely anywhere the IMAGE_TYPE_INT32 macro shows up. The reason that it hasn't been done before is more a matter that there's been one with the background, need, and time to get it done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I'm willing to work on this to try to get it operable as we work with scientific imaging at IntelinAir and support for uint16 and multiband imagery is important for us. It sounds like you're aware of many of the parts that will be needed to do this work. Can we open a tracking issue to discuss a plan for implementing these changes if you think they're worthwhile, as the conversation about the various uint16 and multiband support has gotten spread out over a multitude of issues so far.

This only gets us to opening the first band, it doesn't actually allow
the other bands to be retrieved.
@mairsbw
Copy link
Contributor Author

mairsbw commented Apr 20, 2016

I've also fixed the images and the tests to what I think should now be correct based on your latest comment. I've also confirmed that by removing e0bb623 the tests fail with not being able to even load the example TIFFs.

@igormq
Copy link

igormq commented Apr 26, 2017

Is this PR dead?

@radarhere radarhere changed the title Add tests for opening 2-5 layer uint16 greyscale TIFFs. Add tests for opening 2-5 layer uint16 greyscale TIFFs Jul 7, 2018
@radarhere radarhere added the TIFF label Sep 27, 2018
@radarhere
Copy link
Member

While the test and images from this PR might form part of a future PR addressing #1888, I don't think it's helpful including it by itself, so I'm going to close this.

@radarhere radarhere closed this Jul 19, 2019
yoursunny added a commit to yoursunny/Pillow that referenced this pull request Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants