[Insight-developers] Two (big?) problems with itkTIFFImageIO.cxx

[Zachary Pincus]

Hello folks,

Lately I've been having some problems reading TIFF images in  
correctly. I had thought that this was a recent regression, but  
perhaps it isn't. (Though how I was getting decent results from my  
analyses with such a major error, I don't know.)

Anyhow, I've tracked the problem down, and it's actually two problems  
at the same time. The first and simplest is that ITK isn't dealing  
properly when certain TIFF tags are missing (though the spec allows  
them to be so). The second is that there's a method called  
TIFFReaderInternal::CanRead() which should be used to check whether  
the TIFF can be properly read; instead it seems to be used to  
determine whether to read the TIFF as RGBA.

These two problems combine so that when a TIFF with a missing tag  
(e.g. in my case 'SamplesPerPixel') is encountered, the default value  
for the tag isn't used, so TIFFReaderInternal::CanRead() returns  
false. At this point, ITK really should just give an error; instead  
it seems to *forcibly* try to read the file as RGBA, which is of  
course totally the wrong way to detect if an image is in fact RGBA.

I don't have the time or the knowledge of the TIFF IO code to deal  
with the latter problem, however below is an explanation of the  
former and a patch. If people agree (or I don't hear any disagreement  
for a while), I'll commit the fix.

Someone else really ought to look into the latter issue of why images  
for which TIFFReaderInternal::CanRead() returns false are still read  
in, incorrectly, as RGBA images. I suspect this has to do with  
calling TIFFReadRGBAImage() with the parameter "stopOnError" set to  
zero, which from the man page, appears to disable the ability of this  
function to signal that an image is not an RGBA TIFF by returning  
zero. Is there a *reason* TIFFReadRGBAImage was called in this way?

Zach Pincus

Program in Biomedical Informatics and Department of Biochemistry
Stanford University School of Medicine

Problem:
According to the TIFF specification (http://partners.adobe.com/public/ 
developer/en/tiff/TIFF6.pdf ), the following tags have default values  
(and thus can be optional in valid TIFF files):
'SamplesPerPixel', 'Compression', 'BitsPerSample',  
'PlanarConfig' (and others not of interest to ITK).

The ITK TIFF image IO doesn't attempt to fetch the default value if  
these tags aren't present, and thus fail to properly open otherwise- 
valid TIFF files.

Also, libtiff explicitly warns about trying to read values that have  
been passed in to TIFFGetField() if that function returns false.  
While it doesn't seem to over-write the values in the current libtiff  
version, there's no guarantee of that in the future, so I fixed an  
issue there too.

Following is a patch for itkTIFFImageIO.cxx to fix these problems and  
bring ITK TIFF reading up to spec:
diff -r1.28 itkTIFFImageIO.cxx
178,184c178,190
<     TIFFGetField(this->Image, TIFFTAG_SAMPLESPERPIXEL,
<                  &this->SamplesPerPixel);
<     TIFFGetField(this->Image, TIFFTAG_COMPRESSION, &this- 
 >Compression);
<     TIFFGetField(this->Image, TIFFTAG_BITSPERSAMPLE,
<                  &this->BitsPerSample);
<     TIFFGetField(this->Image, TIFFTAG_PHOTOMETRIC, &this- 
 >Photometrics);
<     TIFFGetField(this->Image, TIFFTAG_PLANARCONFIG, &this- 
 >PlanarConfig);
---
 >     TIFFGetFieldDefaulted(this->Image, TIFFTAG_SAMPLESPERPIXEL,
 >                           &this->SamplesPerPixel);
 >     TIFFGetFieldDefaulted(this->Image, TIFFTAG_COMPRESSION, &this- 
 >Compression);
 >     TIFFGetFieldDefaulted(this->Image, TIFFTAG_BITSPERSAMPLE,
 >                           &this->BitsPerSample);
 >     TIFFGetFieldDefaulted(this->Image, TIFFTAG_PLANARCONFIG, &this- 
 >PlanarConfig);
 >     // If TIFFGetField returns false, we can't trust it to have  
preserved the
 >     // prior state of the variable we passed in, so we reset it to  
the previous
 >     // state in that case.
 >     if (!TIFFGetField(this->Image, TIFFTAG_PHOTOMETRIC, &this- 
 >Photometrics))
 >       {
 >       this->Photometrics = 0;
 >       }