You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2021/09/04 12:38:00 UTC
[jira] [Work logged] (IMAGING-285) Decoding of Rational Numbers
broken when large values present
[ https://issues.apache.org/jira/browse/IMAGING-285?focusedWorklogId=646593&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-646593 ]
ASF GitHub Bot logged work on IMAGING-285:
------------------------------------------
Author: ASF GitHub Bot
Created on: 04/Sep/21 12:37
Start Date: 04/Sep/21 12:37
Worklog Time Spent: 10m
Work Description: kinow commented on a change in pull request #164:
URL: https://github.com/apache/commons-imaging/pull/164#discussion_r702279148
##########
File path: src/main/java/org/apache/commons/imaging/common/RationalNumber.java
##########
@@ -73,8 +124,41 @@ private static long gcd(final long a, final long b) {
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
+ * sacrifice one bit of precision by dividing both the numerator and
+ * the denominator by 2. However, if that computation would result
+ * in a zero denominator, then the value cannot be negated and an
+ * exception is thrown.
+ * @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,
+ // it is necessary to adjust the
Review comment:
to adjust the?
##########
File path: src/main/java/org/apache/commons/imaging/common/RationalNumber.java
##########
@@ -73,8 +124,41 @@ private static long gcd(final long a, final long b) {
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
+ * sacrifice one bit of precision by dividing both the numerator and
+ * the denominator by 2. However, if that computation would result
Review comment:
by `2` or by the greatest common divisor?
##########
File path: src/main/java/org/apache/commons/imaging/formats/tiff/fieldtypes/FieldTypeRational.java
##########
@@ -31,11 +31,19 @@ public FieldTypeRational(final int type, final String name) {
@Override
public Object getValue(final TiffField entry) {
final byte[] bytes = entry.getByteArrayValue();
+ boolean unsignedType = true;
+ if(entry.getFieldType()==RATIONAL){
+ unsignedType = true;
+ }else if(entry.getFieldType()==SRATIONAL){
+ unsignedType = false;
+ }
Review comment:
What about:
```suggestion
boolean unsignedType = entry.getFieldType() != SRATIONAL;
```
##########
File path: src/main/java/org/apache/commons/imaging/common/RationalNumber.java
##########
@@ -20,6 +20,14 @@
/**
* 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.
Review comment:
:+1: thanks for the useful comment!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 646593)
Remaining Estimate: 0.5h (was: 40m)
Time Spent: 0.5h (was: 20m)
> Decoding of Rational Numbers broken when large values present
> -------------------------------------------------------------
>
> Key: IMAGING-285
> URL: https://issues.apache.org/jira/browse/IMAGING-285
> Project: Commons Imaging
> Issue Type: Bug
> Components: imaging.common.*
> Affects Versions: 1.0-alpha2
> Reporter: John Andrade
> Priority: Major
> Attachments: DJI_0267 cropped.JPG
>
> Original Estimate: 1h
> Time Spent: 0.5h
> Remaining Estimate: 0.5h
>
> Decoding Lat/Long EXIF data from JPEGs does not work for some values. I have attached a file where the conversion fails. I used the getLatitudeAsDegreesNorth and getLongitudeAsDegreesEast methods from the TiffImageMetaData.GPSInfo class. The values are close, but seemingly off by a few miles.
> I've traced the source and I believe the issue is with how the RationalNumber class uses two 32-bit signed integers for the numerator and denominator. The definition of a RATIONAL data type uses 32-bit unsigned numbers. It seems as if the RationalNumber class already expects this as it has a non-public static method called factoryMethod to create a RationalNumber from two 64-bit numbers.
> This error is introduced in the ByteConversions class, specifically the toRational method where it uses the regular RationalNumber constructor and thus ensures any rational numbers that have numerator or denominator greater than the max signed 32-bit value will produce invalid values.
> I am attaching a JPEG that has this problem. I had to crop it to reduce the size, but the EXIF data was preserved.
> The EXIF GPS data contained in the JPEG is:
> GpsLatitudeRef: "N"
> GpsLatitude: 38, 1, 36, 1, 4120083230, 70000000
> GpsLongitudeRef: "W"
> GpsLongitude: 90, 1, 12, 1, 2379156648, 70000000
> According to the properties of the image (right-clicking on Windows 10), the L/L of the image is:
> Latitude: 38: 36: 58.85833....
> Longitude: 90: 12: 33.98795... (Windows doesn't show E/W)
> These values convert to:
> 38.616349536627
> -90.2094410978095
> When I use the getLatitudeAsDegreesNorth and getLongitudeAsDegreesEast methods, I get the following values:
> 38.59930601561111
> -90.19239757679365
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)