Merge pull request #1341 from WeatherGod/fix-ifd

Fix and extend Geotiff IFD handling to allow all integer types

cdm/misc/src/main/java/ucar/nc2/geotiff/GeoTiff.java

@@ -326,7 +326,7 @@ private void writeIFDEntry(FileChannel channel, IFDEntry ifd, int start) throws
     }
   }
 
-  private int writeValues(ByteBuffer buffer, IFDEntry ifd) {
+  static int writeValues(ByteBuffer buffer, IFDEntry ifd) {
     int done = 0;
 
     if (ifd.type == FieldType.ASCII) {
@@ -354,23 +354,33 @@ private int writeValues(ByteBuffer buffer, IFDEntry ifd) {
     return done;
   }
 
-  private int writeIntValue(ByteBuffer buffer, IFDEntry ifd, int v) {
+  static int writeIntValue(ByteBuffer buffer, IFDEntry ifd, int v) {
     switch (ifd.type.code) {
       case 1:
+      case 2:
+      case 6:
+      case 7:
+        // unsigned byte and ascii
+        // signed byte and undefined (usually treated as raw binary)
         buffer.put((byte) v);
         return 1;
       case 3:
+      case 8:
+        // unsigned and signed short
         buffer.putShort((short) v);
         return 2;
       case 4:
       case 5:
+      case 9:
+      case 10:
+        // unsigned and signed rational and 32-bit integer
         buffer.putInt(v);
         return 4;
     }
     return 0;
   }
 
-  private int writeSValue(ByteBuffer buffer, IFDEntry ifd) {
+  static int writeSValue(ByteBuffer buffer, IFDEntry ifd) {
     buffer.put(ifd.valueS.getBytes(StandardCharsets.UTF_8));
     int size = ifd.valueS.length();
     if ((size & 1) != 0)
@@ -517,7 +527,7 @@ private IFDEntry readIFDEntry(FileChannel channel, int start) throws IOException
     return ifd;
   }
 
-  private void readValues(ByteBuffer buffer, IFDEntry ifd) {
+  static void readValues(ByteBuffer buffer, IFDEntry ifd) {
 
     if (ifd.type == FieldType.ASCII) {
       ifd.valueS = readSValue(buffer, ifd);
@@ -545,25 +555,41 @@ private void readValues(ByteBuffer buffer, IFDEntry ifd) {
 
   }
 
-  private int readIntValue(ByteBuffer buffer, IFDEntry ifd) {
+  static int readIntValue(ByteBuffer buffer, IFDEntry ifd) {
     switch (ifd.type.code) {
       case 1:
       case 2:
-        return (int) buffer.get();
+        // unsigned byte and unsigned ascii
+        return (int) buffer.get() & 0xff;
       case 3:
+        // unsigned short
         return readUShortValue(buffer);
       case 4:
       case 5:
+        // unsigned rational and unsigned 32-bit integer
+        // Yes, this can lead to truncation. This is a bug
+        // in the design of the IFDEntry API
+        return (int) (buffer.getInt() & 0xffffffffL);
+      case 6:
+      case 7:
+        // signed byte and "undefined" (usually treated as binary data)
+        return (int) buffer.get();
+      case 8:
+        // signed short
+        return (int) buffer.getShort();
+      case 9:
+      case 10:
+        // signed rational and signed 32-bit integer
         return buffer.getInt();
     }
     return 0;
   }
 
-  private int readUShortValue(ByteBuffer buffer) {
+  static int readUShortValue(ByteBuffer buffer) {
     return buffer.getShort() & 0xffff;
   }
 
-  private String readSValue(ByteBuffer buffer, IFDEntry ifd) {
+  static String readSValue(ByteBuffer buffer, IFDEntry ifd) {
     byte[] dst = new byte[ifd.count];
     buffer.get(dst);
     return new String(dst, StandardCharsets.UTF_8);

cdm/misc/src/test/java/ucar/nc2/geotiff/TestIFDEntry.java

@@ -0,0 +1,94 @@
+/*
+ * Copyright (c) 1998-2024 University Corporation for Atmospheric Research/Unidata
+ * See LICENSE for license information.
+ */
+
+package ucar.nc2.geotiff;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import java.lang.invoke.MethodHandles;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * IFDEntry read/write
+ *
+ * @author Ben Root
+ * @since 5/6/2024
+ */
+@RunWith(Parameterized.class)
+public class TestIFDEntry {
+  private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+  private final IFDEntry ifd;
+  private final int testValue;
+
+  @Parameterized.Parameters(name = "{0}_{1}")
+  public static List<Object[]> getTestParameters() {
+    List<Object[]> result = new ArrayList<>();
+    // Unsigned
+    result.add(new Object[] {new IFDEntry(null, FieldType.BYTE, 1), 0});
+    result.add(new Object[] {new IFDEntry(null, FieldType.BYTE, 1), Byte.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.BYTE, 1), 255});
+    result.add(new Object[] {new IFDEntry(null, FieldType.ASCII, 1), 0});
+    result.add(new Object[] {new IFDEntry(null, FieldType.ASCII, 1), Byte.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.ASCII, 1), 255});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SHORT, 1), 0});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SHORT, 1), Byte.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SHORT, 1), Short.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SHORT, 1), 65535});
+    result.add(new Object[] {new IFDEntry(null, FieldType.LONG, 1), 0});
+    result.add(new Object[] {new IFDEntry(null, FieldType.LONG, 1), Byte.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.LONG, 1), Short.MAX_VALUE});
+    // NOTE: because of the API design, unsigned longs can't be properly read or written
+    // for all possible values because Java's integer is signed.
+    result.add(new Object[] {new IFDEntry(null, FieldType.LONG, 1), Integer.MAX_VALUE});
+
+    // Signed
+    result.add(new Object[] {new IFDEntry(null, FieldType.SBYTE, 1), 0});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SBYTE, 1), Byte.MIN_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SBYTE, 1), Byte.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SSHORT, 1), 0});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SSHORT, 1), Byte.MIN_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SSHORT, 1), -Byte.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SSHORT, 1), Byte.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SSHORT, 1), Short.MIN_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SSHORT, 1), Short.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SLONG, 1), 0});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SLONG, 1), -Byte.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SLONG, 1), Byte.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SLONG, 1), -Short.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SLONG, 1), Short.MAX_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SLONG, 1), Integer.MIN_VALUE});
+    result.add(new Object[] {new IFDEntry(null, FieldType.SLONG, 1), Integer.MAX_VALUE});
+
+    return result;
+  }
+
+  public TestIFDEntry(IFDEntry ifd, int testValue) {
+    this.ifd = ifd;
+    this.testValue = testValue;
+  }
+
+  @Test
+  public void testRoundtrip() {
+    // 16 bytes should be more than enough
+    ByteBuffer buffer = ByteBuffer.allocate(16);
+    ByteOrder byteOrder = ByteOrder.BIG_ENDIAN;
+    buffer.order(byteOrder);
+
+    int writeSize = GeoTiff.writeIntValue(buffer, ifd, testValue);
+    Assert.assertEquals(ifd.type.size, writeSize);
+    buffer.position(0);
+
+    int readValue = GeoTiff.readIntValue(buffer, ifd);
+
+    Assert.assertEquals(testValue, readValue);
+  }
+}