Addresses #214 by allowing up to 255 samples per pixel and allowing B…

[tromper]

…lackIsZero and WhiteIsZero to have up to 64 bits per sample. Also includes decode_geotiff_images unit test.

[fintelia]

#215 removed the same limit this does, but it would still be great to merge the test. Would you mind rebasing?

[weiji14]

#215 removed the same limit this does, but it would still be great to merge the test. Would you mind rebasing?

Been following #214 and am interested in this patch to enable reading multi-band GeoTIFFs at https://github.com/weiji14/cog3pio. Is it ok if I open another PR for this?

[weiji14]

It seems like this change is still needed to support multi-band GeoTIFFs after #215 is merged (the line would become if self.samples <= 64 => if rebasing on top of 4a2e768).

[fintelia]

It is a bit more complicated because other places in the code assume that ColorType::Gray means only a single band

[weiji14]

Typically for multi-band GeoTIFFs, each band would be ColorType::Gray (grayscale). Are you saying that we need to check the ColorType for each band?

[fintelia]

I refreshed my memory of this code. The main place that assumes that ColorType::Gray means the image has only a single band is actually in the image crate. Other users may be making a similar assumption, particularly because Decoder doesn't actually expose a way to query the number of bands.

We never actually marked ColorType non_exhaustive so it would require a major version bump, but I think it might make sense to add a ColorType::Multiband(bit_depth, bands) variant or something along those lines.

[weiji14]

Oh ok, you mean adding ColorType::Multiband to the enum at

image-tiff/src/lib.rs

Lines 22 to 43 in 895c70a

	pub enum ColorType {
	/// Pixel is grayscale
	Gray(u8),
	/// Pixel contains R, G and B channels
	RGB(u8),
	/// Pixel is an index into a color palette
	Palette(u8),
	/// Pixel is grayscale with an alpha channel
	GrayA(u8),
	/// Pixel is RGB with an alpha channel
	RGBA(u8),
	/// Pixel is CMYK
	CMYK(u8),
	/// Pixel is YCbCr
	YCbCr(u8),
	}

? I see that this seems similar to what was proposed at #147 (with a draft proof of concept implemented in Masterchef365/image-tiff@2bcc24c).

I'll see if I can squeeze out some time this week to implement this. It sounds like we just need to add the ColorType::Multiband variant, and modify the self.samples >= 1 branch somewhere here (under the TODO), while keeping the self.sample == 1 branch intact:

image-tiff/src/decoder/image.rs

Lines 350 to 361 in 895c70a

	PhotometricInterpretation::BlackIsZero | PhotometricInterpretation::WhiteIsZero
	if self.samples == 1 =>
	{
	Ok(ColorType::Gray(self.bits_per_sample))
	}
	// TODO: this is bad we should not fail at this point
	_ => Err(TiffError::UnsupportedError(
	TiffUnsupportedError::InterpretationWithBits(
	self.photometric_interpretation,
	vec![self.bits_per_sample; self.samples as usize],
	),

[weiji14]

Ok, the code was a lot more straightforward than I expected. Opened a PR at #224 to work on this.