#254 Fix NPE reading TIFF Metadata when BitsPerSample not set

imageio/imageio-tiff/src/main/java/com/twelvemonkeys/imageio/plugins/tiff/TIFFImageMetadata.java

@@ -324,12 +324,7 @@ public final class TIFFImageMetadata extends AbstractMetadata {
         // Handle ColorSpaceType (RGB/CMYK/YCbCr etc)...
         Entry photometricTag = ifd.getEntryById(TIFF.TAG_PHOTOMETRIC_INTERPRETATION);
         int photometricValue = getValueAsInt(photometricTag); // No default for this tag!
-
+        int numChannelsValue = getSamplesPerPixelWithFallback();
-        Entry samplesPerPixelTag = ifd.getEntryById(TIFF.TAG_SAMPLES_PER_PIXEL);
-        Entry bitsPerSampleTag = ifd.getEntryById(TIFF.TAG_BITS_PER_SAMPLE);
-        int numChannelsValue = samplesPerPixelTag != null
-                               ? getValueAsInt(samplesPerPixelTag)
-                               : bitsPerSampleTag.valueCount();
 
         IIOMetadataNode colorSpaceType = new IIOMetadataNode("ColorSpaceType");
         chroma.appendChild(colorSpaceType);
@@ -419,6 +414,16 @@ public final class TIFFImageMetadata extends AbstractMetadata {
         return chroma;
     }
 
+    private int getSamplesPerPixelWithFallback() {
+        // SamplePerPixel defaults to 1, but we'll check BitsPerSample to be sure
+        Entry samplesPerPixelTag = ifd.getEntryById(TIFF.TAG_SAMPLES_PER_PIXEL);
+        Entry bitsPerSampleTag = ifd.getEntryById(TIFF.TAG_BITS_PER_SAMPLE);
+
+        return samplesPerPixelTag != null
+                               ? getValueAsInt(samplesPerPixelTag)
+                               : bitsPerSampleTag != null ? bitsPerSampleTag.valueCount() : 1;
+    }
+
     @Override
     protected IIOMetadataNode getStandardCompressionNode() {
         IIOMetadataNode compression = new IIOMetadataNode("Compression");
@@ -586,9 +591,7 @@ public final class TIFFImageMetadata extends AbstractMetadata {
         // TODO: See TIFFImageReader.getBitsPerSample + fix the metadata to have getAsXxxArray methods.
         // BitsPerSample (not required field for Class B/Bilevel, defaults to 1)
         Entry bitsPerSampleTag = ifd.getEntryById(TIFF.TAG_BITS_PER_SAMPLE);
-        String bitsPerSampleValue = bitsPerSampleTag == null &&
+        String bitsPerSampleValue = bitsPerSampleTag == null
-                                            (photometricInterpretationValue == TIFFBaseline.PHOTOMETRIC_WHITE_IS_ZERO ||
-                                                    photometricInterpretationValue == TIFFBaseline.PHOTOMETRIC_BLACK_IS_ZERO)
                                     ? "1"
                                     : bitsPerSampleTag.getValueAsString().replaceAll("\\[?\\]?,?", "");
 
@@ -596,10 +599,7 @@ public final class TIFFImageMetadata extends AbstractMetadata {
         node.appendChild(bitsPerSample);
         bitsPerSample.setAttribute("value", bitsPerSampleValue);
 
-        Entry samplesPerPixelTag = ifd.getEntryById(TIFF.TAG_SAMPLES_PER_PIXEL);
+        int numChannelsValue = getSamplesPerPixelWithFallback();
-        int numChannelsValue = samplesPerPixelTag != null
-                               ? getValueAsInt(samplesPerPixelTag)
-                               : bitsPerSampleTag.valueCount();
 
         // SampleMSB
         Entry fillOrderTag = ifd.getEntryById(TIFF.TAG_FILL_ORDER);

imageio/imageio-tiff/src/test/java/com/twelvemonkeys/imageio/plugins/tiff/TIFFImageMetadataTest.java

@@ -22,6 +22,8 @@ import javax.imageio.stream.ImageInputStream;
 import java.io.IOException;
 import java.net.URL;
 import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
 
 import static org.junit.Assert.*;
 
@@ -228,7 +230,6 @@ public class TIFFImageMetadataTest {
         ifdNode.appendChild(tiffField);
 
         assertNodeNotEquals("Modified tree does not differ", nativeTree, nativeTree2);
-
     }
 
     @Test
@@ -473,6 +474,67 @@ public class TIFFImageMetadataTest {
         metadata.setFromTree(nativeFormat, new IIOMetadataNode(nativeFormat)); // Requires at least one child node
     }
 
+    @Test
+    public void testStandardChromaSamplesPerPixel() {
+        Set<Entry> entries = new HashSet<>();
+        entries.add(new TIFFImageWriter.TIFFEntry(TIFF.TAG_PHOTOMETRIC_INTERPRETATION, TIFFBaseline.PHOTOMETRIC_RGB));
+        entries.add(new TIFFImageWriter.TIFFEntry(TIFF.TAG_SAMPLES_PER_PIXEL, 4));
+        entries.add(new TIFFImageWriter.TIFFEntry(TIFF.TAG_BITS_PER_SAMPLE, new int[] {8, 8, 8})); // This is incorrect, just making sure the correct value is selected
+
+        IIOMetadataNode chromaNode = new TIFFImageMetadata(entries).getStandardChromaNode();
+        assertNotNull(chromaNode);
+
+        IIOMetadataNode numChannels = (IIOMetadataNode) chromaNode.getElementsByTagName("NumChannels").item(0);
+        assertEquals("4", numChannels.getAttribute("value"));
+    }
+
+    @Test
+    public void testStandardChromaSamplesPerPixelFallbackBitsPerSample() {
+        Set<Entry> entries = new HashSet<>();
+        entries.add(new TIFFImageWriter.TIFFEntry(TIFF.TAG_PHOTOMETRIC_INTERPRETATION, TIFFBaseline.PHOTOMETRIC_RGB));
+        entries.add(new TIFFImageWriter.TIFFEntry(TIFF.TAG_BITS_PER_SAMPLE, new int[] {8, 8, 8}));
+
+        IIOMetadataNode chromaNode = new TIFFImageMetadata(entries).getStandardChromaNode();
+        assertNotNull(chromaNode);
+
+        IIOMetadataNode numChannels = (IIOMetadataNode) chromaNode.getElementsByTagName("NumChannels").item(0);
+        assertEquals("3", numChannels.getAttribute("value"));
+    }
+
+    @Test
+    public void testStandardChromaSamplesPerPixelFallbackDefault() {
+        Set<Entry> entries = new HashSet<>();
+        entries.add(new TIFFImageWriter.TIFFEntry(TIFF.TAG_PHOTOMETRIC_INTERPRETATION, TIFFBaseline.PHOTOMETRIC_BLACK_IS_ZERO));
+
+        IIOMetadataNode chromaNode = new TIFFImageMetadata(entries).getStandardChromaNode();
+        assertNotNull(chromaNode);
+        IIOMetadataNode numChannels = (IIOMetadataNode) chromaNode.getElementsByTagName("NumChannels").item(0);
+        assertEquals("1", numChannels.getAttribute("value"));
+    }
+
+    @Test
+    public void testStandardDataBitsPerSampleFallbackDefault() {
+        Set<Entry> entries = new HashSet<>();
+        entries.add(new TIFFImageWriter.TIFFEntry(TIFF.TAG_PHOTOMETRIC_INTERPRETATION, TIFFBaseline.PHOTOMETRIC_BLACK_IS_ZERO));
+
+        IIOMetadataNode dataNode = new TIFFImageMetadata(entries).getStandardDataNode();
+        assertNotNull(dataNode);
+        IIOMetadataNode numChannels = (IIOMetadataNode) dataNode.getElementsByTagName("BitsPerSample").item(0);
+        assertEquals("1", numChannels.getAttribute("value"));
+    }
+
+    @Test
+    public void testStandardNodeSamplesPerPixelFallbackDefault() {
+        Set<Entry> entries = new HashSet<>();
+        entries.add(new TIFFImageWriter.TIFFEntry(TIFF.TAG_PHOTOMETRIC_INTERPRETATION, TIFFBaseline.PHOTOMETRIC_RGB));
+
+        // Just to make sure we haven't accidentally missed something
+        IIOMetadataNode standardTree = (IIOMetadataNode) new TIFFImageMetadata(entries).getAsTree(IIOMetadataFormatImpl.standardMetadataFormatName);
+        assertNotNull(standardTree);
+    }
+
+    // TODO: Test that failed set leaves metadata unchanged
+
     private void assertSingleNodeWithValue(final NodeList fields, final int tag, int type, final String... expectedValue) {
         String tagNumber = String.valueOf(tag);
         String typeName = StringUtil.capitalize(TIFF.TYPE_NAMES[type].toLowerCase());
@@ -505,8 +567,6 @@ public class TIFFImageMetadataTest {
         assertTrue("No tag " + tagNumber + " found", foundTag);
     }
 
-    // TODO: Test that failed set leaves metadata unchanged
-
     static void createTIFFFieldNode(final IIOMetadataNode parentIFDNode, int tag, short type, Object value) {
         IIOMetadataNode fieldNode = new IIOMetadataNode("TIFFField");
         parentIFDNode.appendChild(fieldNode);