This is an automated email from the ASF dual-hosted git repository.
kinow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-imaging.git
commit 0fb5942c2144f53a0291b8da0184ea9f1520e863
Author: gwlucastrig <contact.tinf...@gmail.com>
AuthorDate: Mon Sep 13 20:08:04 2021 -0400
[IMAGING-285] Correction for rational number computations
---
.../commons/imaging/common/ByteConversions.java | 45 ++++++---
.../commons/imaging/common/RationalNumber.java | 107 +++++++++++++++++++--
.../formats/tiff/fieldtypes/FieldTypeRational.java | 9 +-
.../formats/tiff/taginfos/TagInfoRational.java | 2 +-
.../formats/tiff/taginfos/TagInfoRationals.java | 2 +-
.../formats/tiff/taginfos/TagInfoSRational.java | 2 +-
.../formats/tiff/taginfos/TagInfoSRationals.java | 2 +-
.../commons/imaging/common/RationalNumberTest.java | 15 +++
.../formats/tiff/TiffReadWriteTagsTest.java | 19 +++-
9 files changed, 174 insertions(+), 29 deletions(-)
diff --git
a/src/main/java/org/apache/commons/imaging/common/ByteConversions.java
b/src/main/java/org/apache/commons/imaging/common/ByteConversions.java
index 12d34e0..dd900d3 100644
--- a/src/main/java/org/apache/commons/imaging/common/ByteConversions.java
+++ b/src/main/java/org/apache/commons/imaging/common/ByteConversions.java
@@ -348,11 +348,27 @@ public final class ByteConversions {
return result;
}
- public static RationalNumber toRational(final byte[] bytes, final
ByteOrder byteOrder) {
- return toRational(bytes, 0, byteOrder);
- }
-
- private static RationalNumber toRational(final byte[] bytes, final int
offset, final ByteOrder byteOrder) {
+ /**
+ * Interprets the content of a specified bytes array to create
+ * an instance of the RationalNumber class.
+ * @param bytes a valid array dimensioned to at least 8.
+ * @param byteOrder the byte order for integer conversion
+ * @param unsignedType indicates whether the extracted value is
+ * an unsigned type.
+ * @return a valid instance
+ */
+ public static RationalNumber toRational(
+ final byte[] bytes,
+ final ByteOrder byteOrder,
+ final boolean unsignedType) {
+ return toRational(bytes, 0, byteOrder, unsignedType);
+ }
+
+ private static RationalNumber toRational(
+ final byte[] bytes,
+ final int offset,
+ final ByteOrder byteOrder,
+ final boolean unsignedType) {
final int byte0 = 0xff & bytes[offset + 0];
final int byte1 = 0xff & bytes[offset + 1];
final int byte2 = 0xff & bytes[offset + 2];
@@ -370,18 +386,25 @@ public final class ByteConversions {
numerator = (byte3 << 24) | (byte2 << 16) | (byte1 << 8) | byte0;
divisor = (byte7 << 24) | (byte6 << 16) | (byte5 << 8) | byte4;
}
- return new RationalNumber(numerator, divisor);
+ return new RationalNumber(numerator, divisor, unsignedType);
}
- public static RationalNumber[] toRationals(final byte[] bytes, final
ByteOrder byteOrder) {
- return toRationals(bytes, 0, bytes.length, byteOrder);
+ public static RationalNumber[] toRationals(
+ final byte[] bytes,
+ final ByteOrder byteOrder,
+ boolean unsignedType) {
+ return toRationals(bytes, 0, bytes.length, byteOrder, unsignedType);
}
- private static RationalNumber[] toRationals(final byte[] bytes,
- final int offset, final int length, final ByteOrder byteOrder) {
+ private static RationalNumber[] toRationals(
+ final byte[] bytes,
+ final int offset,
+ final int length,
+ final ByteOrder byteOrder,
+ boolean unsignedType) {
final RationalNumber[] result = new RationalNumber[length / 8];
for (int i = 0; i < result.length; i++) {
- result[i] = toRational(bytes, offset + 8 * i, byteOrder);
+ result[i] = toRational(bytes, offset + 8 * i, byteOrder,
unsignedType);
}
return result;
}
diff --git
a/src/main/java/org/apache/commons/imaging/common/RationalNumber.java
b/src/main/java/org/apache/commons/imaging/common/RationalNumber.java
index 89888c4..3ae186f 100644
--- a/src/main/java/org/apache/commons/imaging/common/RationalNumber.java
+++ b/src/main/java/org/apache/commons/imaging/common/RationalNumber.java
@@ -20,6 +20,14 @@ import java.text.NumberFormat;
/**
* Rational number, as used by the TIFF image format.
+ * <p>
+ * The TIFF format specifies two data types for rational numbers based on
+ * a pair of 32-bit integers. Rational is based on unsigned 32-bit integers
+ * and SRational is based on signed 32-bit integers. This treatment is
+ * problematic in Java because Java does not support unsigned types.
+ * To address this challenge, this class stores the numerator and divisor
+ * in long (64-bit) integers, applying masks as necessary for the unsigned
+ * type.
*/
public class RationalNumber extends Number {
@@ -28,12 +36,55 @@ public class RationalNumber extends Number {
// int-precision tolerance
private static final double TOLERANCE = 1E-8;
- public final int numerator;
- public final int divisor;
+ // The TIFF and EXIF specifications call for the use
+ // of 32 bit unsigned integers. Since Java does not have an
+ // unsigned type, this class widens the type to long in order
+ // to avoid unintended negative numbers.
+ public final long numerator;
+ public final long divisor;
+ public final boolean unsignedType;
+ /**
+ * Constructs an instance based on signed integers
+ * @param numerator a 32-bit signed integer
+ * @param divisor a non-zero 32-bit signed integer
+ */
public RationalNumber(final int numerator, final int divisor) {
this.numerator = numerator;
this.divisor = divisor;
+ this.unsignedType = false;
+ }
+
+ /**
+ * Constructs an instance supports either signed or unsigned integers.
+ * @param numerator a numerator in the indicated form (signed or unsigned)
+ * @param divisor a non-zero divisor in the indicated form (signed or
unsigned)
+ * @param unsignedType indicates whether the input values are to be treated
+ * as unsigned.
+ */
+ public RationalNumber(final int numerator, final int divisor, final
boolean unsignedType) {
+ this.unsignedType = unsignedType;
+ if (unsignedType) {
+ this.numerator = numerator & 0xffffffffL;
+ this.divisor = divisor & 0xffffffffL;
+ } else {
+ this.numerator = numerator;
+ this.divisor = divisor;
+ }
+ }
+
+ /**
+ * A private constructor for methods such as negate() that create instances
+ * of this class using the content of the current instance.
+ * @param numerator a valid numerator
+ * @param divisor a valid denominator
+ * @param unsignedType indicates how numerator and divisor values
+ * are to be interpreted.
+ */
+ private RationalNumber(final long numerator, final long divisor, boolean
unsignedType){
+ this.numerator = numerator;
+ this.divisor = divisor;
+ this.unsignedType = unsignedType;
}
static RationalNumber factoryMethod(long n, long d) {
@@ -73,8 +124,44 @@ public class RationalNumber extends Number {
return gcd(b, a % b);
}
+ /**
+ * Negates the value of the RationalNumber. If the numerator of this
+ * instance has its high-order bit set, then its value is too large
+ * to be treated as a Java 32-bit signed integer. In such a case, the
+ * only way that a RationalNumber instance can be negated is to
+ * divide both terms by a common divisor, if a non-zero common divisor
exists.
+ * However, if no such divisor exists, there is no numerically correct
+ * way to perform the negation. When a negation cannot be performed
correctly,
+ * this method throws an unchecked exception.
+ * @return a valid instance with a negated value.
+ */
public RationalNumber negate() {
- return new RationalNumber(-numerator, divisor);
+ long n = numerator;
+ long d = divisor;
+ if (unsignedType) {
+ // An instance of an unsigned type can be negated if and only if
+ // its high-order bit (the sign bit) is clear. If the bit is set,
+ // the value will be too large to convert to a signed type.
+ // In such a case it is necessary to adjust the numerator and
denominator
+ // by their greatest common divisor (gcd), if one exists.
+ // If no non-zero common divisor exists, an exception is thrown.
+ if ((n >> 31) == 1) {
+ // the unsigned value is so large that the high-order bit is
set
+ // it cannot be converted to a negative number. Check to see
+ // whether there is an option to reduce its magnitude.
+ long g = gcd(numerator, divisor);
+ if (g != 0) {
+ n /= g;
+ d /= g;
+ }
+ if ((n >> 31) == 1) {
+ throw new NumberFormatException(
+ "Unsigned numerator is too large to negate "
+ + numerator);
+ }
+ }
+ }
+ return new RationalNumber(-n, d, false);
}
@Override
@@ -84,17 +171,23 @@ public class RationalNumber extends Number {
@Override
public float floatValue() {
- return (float) numerator / (float) divisor;
+ // The computation uses double value in order to preserve
+ // as much of the precision of the source numerator and denominator
+ // as possible. Note that the expression
+ // return (float)numerator/(float) denominator
+ // could lose precision since a Java float only carries 24 bits
+ // of precision while an integer carries 32.
+ return (float) doubleValue();
}
@Override
public int intValue() {
- return numerator / divisor;
+ return (int)(numerator / divisor);
}
@Override
public long longValue() {
- return (long) numerator / (long) divisor;
+ return numerator / divisor;
}
@Override
@@ -112,7 +205,7 @@ public class RationalNumber extends Number {
public String toDisplayString() {
if ((numerator % divisor) == 0) {
- return Integer.toString(numerator / divisor);
+ return Long.toString(numerator / divisor);
}
final NumberFormat nf = NumberFormat.getInstance();
nf.setMaximumFractionDigits(3);
diff --git
a/src/main/java/org/apache/commons/imaging/formats/tiff/fieldtypes/FieldTypeRational.java
b/src/main/java/org/apache/commons/imaging/formats/tiff/fieldtypes/FieldTypeRational.java
index a18ca7e..644964c 100644
---
a/src/main/java/org/apache/commons/imaging/formats/tiff/fieldtypes/FieldTypeRational.java
+++
b/src/main/java/org/apache/commons/imaging/formats/tiff/fieldtypes/FieldTypeRational.java
@@ -31,11 +31,14 @@ public class FieldTypeRational extends FieldType {
@Override
public Object getValue(final TiffField entry) {
final byte[] bytes = entry.getByteArrayValue();
+ boolean unsignedType = entry.getFieldType() != SRATIONAL;
if (entry.getCount() == 1) {
- return ByteConversions.toRational(bytes,
- entry.getByteOrder());
+ return ByteConversions.toRational(
+ bytes,
+ entry.getByteOrder(),
+ unsignedType);
}
- return ByteConversions.toRationals(bytes, entry.getByteOrder());
+ return ByteConversions.toRationals(bytes, entry.getByteOrder(),
unsignedType);
}
@Override
diff --git
a/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoRational.java
b/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoRational.java
index c113ff6..4370efa 100644
---
a/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoRational.java
+++
b/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoRational.java
@@ -29,7 +29,7 @@ public class TagInfoRational extends TagInfo {
}
public RationalNumber getValue(final ByteOrder byteOrder, final byte[]
bytes) {
- return ByteConversions.toRational(bytes, byteOrder);
+ return ByteConversions.toRational(bytes, byteOrder, true);
}
public byte[] encodeValue(final ByteOrder byteOrder, final RationalNumber
value) {
diff --git
a/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoRationals.java
b/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoRationals.java
index e4fde59..a23bbec 100644
---
a/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoRationals.java
+++
b/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoRationals.java
@@ -29,7 +29,7 @@ public class TagInfoRationals extends TagInfo {
}
public RationalNumber[] getValue(final ByteOrder byteOrder, final byte[]
bytes) {
- return ByteConversions.toRationals(bytes, byteOrder);
+ return ByteConversions.toRationals(bytes, byteOrder, true);
}
public byte[] encodeValue(final ByteOrder byteOrder, final
RationalNumber... values) {
diff --git
a/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoSRational.java
b/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoSRational.java
index 8df641f..4f09114 100644
---
a/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoSRational.java
+++
b/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoSRational.java
@@ -29,7 +29,7 @@ public class TagInfoSRational extends TagInfo {
}
public RationalNumber getValue(final ByteOrder byteOrder, final byte[]
bytes) {
- return ByteConversions.toRational(bytes, byteOrder);
+ return ByteConversions.toRational(bytes, byteOrder, false);
}
public byte[] encodeValue(final ByteOrder byteOrder, final RationalNumber
value) {
diff --git
a/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoSRationals.java
b/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoSRationals.java
index 350ed13..4200f19 100644
---
a/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoSRationals.java
+++
b/src/main/java/org/apache/commons/imaging/formats/tiff/taginfos/TagInfoSRationals.java
@@ -29,7 +29,7 @@ public class TagInfoSRationals extends TagInfo {
}
public RationalNumber[] getValue(final ByteOrder byteOrder, final byte[]
bytes) {
- return ByteConversions.toRationals(bytes, byteOrder);
+ return ByteConversions.toRationals(bytes, byteOrder, false);
}
public byte[] encodeValue(final ByteOrder byteOrder, final
RationalNumber... values) {
diff --git
a/src/test/java/org/apache/commons/imaging/common/RationalNumberTest.java
b/src/test/java/org/apache/commons/imaging/common/RationalNumberTest.java
index c91fc4a..40cb1af 100644
--- a/src/test/java/org/apache/commons/imaging/common/RationalNumberTest.java
+++ b/src/test/java/org/apache/commons/imaging/common/RationalNumberTest.java
@@ -26,6 +26,10 @@ import org.apache.commons.imaging.internal.Debug;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import org.junit.jupiter.api.Test;
+
public class RationalNumberTest extends ImagingTest {
public static Stream<Double> data() {
@@ -120,4 +124,15 @@ public class RationalNumberTest extends ImagingTest {
Debug.debug();
}
+ @Test
+ public void testSpecialRationalNumber(){
+ RationalNumber test = new RationalNumber(0xF5937B1F, 70_000_000, true);
+ assertEquals(58.858331871428570, test.doubleValue(), 1.0e-14,
"Unsigned integer support failed for double conversion");
+ assertEquals(58.858334f, test.floatValue(), 1.0e-6f, "Float conversion
failed");
+ assertEquals(58L, test.longValue(), "Long value conversion failed");
+ assertEquals(58, test.intValue(), "Int value conversion failed");
+ assertThrows(NumberFormatException.class, () -> test.negate(), "Failed
to detect negation of large unsigned value");
+
+ }
+
}
diff --git
a/src/test/java/org/apache/commons/imaging/formats/tiff/TiffReadWriteTagsTest.java
b/src/test/java/org/apache/commons/imaging/formats/tiff/TiffReadWriteTagsTest.java
index c3f2ef8..0213afa 100644
---
a/src/test/java/org/apache/commons/imaging/formats/tiff/TiffReadWriteTagsTest.java
+++
b/src/test/java/org/apache/commons/imaging/formats/tiff/TiffReadWriteTagsTest.java
@@ -20,8 +20,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
-import java.util.Map;
-import java.util.TreeMap;
import org.apache.commons.imaging.FormatCompliance;
import org.apache.commons.imaging.ImageReadException;
@@ -31,6 +29,8 @@ import
org.apache.commons.imaging.common.bytesource.ByteSourceArray;
import org.apache.commons.imaging.formats.tiff.constants.GeoTiffTagConstants;
import org.apache.commons.imaging.formats.tiff.constants.GpsTagConstants;
import
org.apache.commons.imaging.formats.tiff.constants.MicrosoftHdPhotoTagConstants;
+import org.apache.commons.imaging.formats.tiff.constants.ExifTagConstants;
+import org.apache.commons.imaging.formats.tiff.constants.GpsTagConstants;
import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
import org.apache.commons.imaging.formats.tiff.write.TiffImageWriterLossy;
import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
@@ -50,6 +50,11 @@ public class TiffReadWriteTagsTest extends TiffBaseTest {
final String area = "A good area";
final float widthRes = 2.2f;
final double geoDoubleParams = -8.4;
+ RationalNumber exposureCompensation = new RationalNumber(-17, 10);
+ RationalNumber[] latitude = new RationalNumber[3];
+ latitude[0] = new RationalNumber(38, 1, true);
+ latitude[1] = new RationalNumber(36, 1, true);
+ latitude[2] = new RationalNumber(0xF5937B1E, 70_000_000, true);
final TiffOutputSet set = new TiffOutputSet();
final TiffOutputDirectory dir = set.getOrCreateRootDirectory();
@@ -62,15 +67,16 @@ public class TiffReadWriteTagsTest extends TiffBaseTest {
dir.add(GpsTagConstants.GPS_TAG_GPS_AREA_INFORMATION, area);
dir.add(MicrosoftHdPhotoTagConstants.EXIF_TAG_WIDTH_RESOLUTION,
widthRes);
dir.add(GeoTiffTagConstants.EXIF_TAG_GEO_DOUBLE_PARAMS_TAG,
geoDoubleParams);
+ dir.add(ExifTagConstants.EXIF_TAG_EXPOSURE_COMPENSATION,
exposureCompensation);
+ dir.add(GpsTagConstants.GPS_TAG_GPS_LATITUDE, latitude);
final TiffImageWriterLossy writer = new TiffImageWriterLossy();
final ByteArrayOutputStream tiff = new ByteArrayOutputStream();
writer.write(tiff, set);
final TiffReader reader = new TiffReader(true);
- final Map<String, Object> params = new TreeMap<>();
final FormatCompliance formatCompliance = new FormatCompliance("");
- final TiffContents contents = reader.readFirstDirectory(new
ByteSourceArray(tiff.toByteArray()), params, true, formatCompliance);
+ final TiffContents contents = reader.readDirectories(new
ByteSourceArray(tiff.toByteArray()), true, formatCompliance);
final TiffDirectory rootDir = contents.directories.get(0);
assertEquals(description,
rootDir.getSingleFieldValue(TiffTagConstants.TIFF_TAG_IMAGE_DESCRIPTION));
assertEquals(page,
rootDir.getFieldValue(TiffTagConstants.TIFF_TAG_PAGE_NUMBER, true)[0]);
@@ -83,5 +89,10 @@ public class TiffReadWriteTagsTest extends TiffBaseTest {
assertEquals(area,
rootDir.getFieldValue(GpsTagConstants.GPS_TAG_GPS_AREA_INFORMATION, true));
assertEquals(widthRes,
rootDir.getFieldValue(MicrosoftHdPhotoTagConstants.EXIF_TAG_WIDTH_RESOLUTION),
0.0);
assertEquals(geoDoubleParams,
rootDir.getFieldValue(GeoTiffTagConstants.EXIF_TAG_GEO_DOUBLE_PARAMS_TAG,
true)[0], 0.0);
+ assertEquals(exposureCompensation.doubleValue(),
rootDir.getFieldValue(ExifTagConstants.EXIF_TAG_EXPOSURE_COMPENSATION).doubleValue(),
0.0);
+ RationalNumber[] testLat =
rootDir.getFieldValue(GpsTagConstants.GPS_TAG_GPS_LATITUDE, true);
+ for (int i = 0; i < 3; i++) {
+ assertEquals(latitude[i].doubleValue(), testLat[i].doubleValue(),
0.0);
+ }
}
}
