jokiazhang / metadata-extractor

Automatically exported from code.google.com/p/metadata-extractor
0 stars 0 forks source link

ExifReader fails for some unsigned rational values #45

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
http://code.google.com/p/metadata-extractor/source/browse/trunk/Source/com/drew/
metadata/exif/ExifReader.java#471

Unsigned rationals are currently being read using getInt32, which is inadequate 
for some values.  As Alex points out on the mailing list:

> I'm trying to get 2 TIFF resolution tags that are supposed to equate
to
>
> XResolution (1 Rational): 26023.6398751818
> YResolution (1 Rational): 26023.6398751818
>
> but what I'm getting is:
>
> X Resolution: -25771/165040 dots per cm
> Y Resolution: -25771/165040 dots per cm

The numerator is being read as -25771, whereas it should really be 4294941525.

The case block needs to be split in two.  I'd like to add a unit test for this 
that proves the issue will not come back.

I have asked Alex if it's possible to add a sample image that reproduces the 
problem to this issue.

Original issue reported on code.google.com by drewnoakes on 16 May 2012 at 8:53

GoogleCodeExporter commented 9 years ago
I have attached the offending TIFF.  TIFF tags are 282 (hex 0x011A) and 283 
(hex 0x011B).

How can I go about putting a band aid on this in my code, for now?

Original comment by alexfrom...@gmail.com on 16 May 2012 at 5:58

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Thanks for the sample image.

I think the following might work or should at least help.

In ExifReader.java, there's a switch statement that branches on type.  
Currently both FMT_SRATIONAL and FMT_URATIONAL have the same handler.  In fact 
they should be different:

case FMT_SRATIONAL:
    if (componentCount == 1) {
        directory.setRational(tagType, new Rational(reader.getInt32(tagValueOffset), reader.getInt32(tagValueOffset + 4)));
    } else if (componentCount > 1) {
        Rational[] rationals = new Rational[componentCount];
        for (int i = 0; i < componentCount; i++)
            rationals[i] = new Rational(reader.getInt32(tagValueOffset + (8 * i)), reader.getInt32(tagValueOffset + 4 + (8 * i)));
        directory.setRationalArray(tagType, rationals);
    }
    break;
case FMT_URATIONAL:
    if (componentCount == 1) {
        directory.setRational(tagType, new Rational(reader.getUInt32(tagValueOffset), reader.getUInt32(tagValueOffset + 4)));
    } else if (componentCount > 1) {
        Rational[] rationals = new Rational[componentCount];
        for (int i = 0; i < componentCount; i++)
            rationals[i] = new Rational(reader.getUInt32(tagValueOffset + (8 * i)), reader.getUInt32(tagValueOffset + 4 + (8 * i)));
        directory.setRationalArray(tagType, rationals);
    }
    break;

The method BufferReader.getUInt32 doesn't exist yet either.  It'll look 
something like:

public int getUInt32(int index) throws BufferBoundsException
{
    CheckBounds(index, 4);

    if (_isMotorolaByteOrder) {
        // Motorola - MSB first (big endian)
        return (((long)_buffer[index    ]) << 24 & 0xFF000000L) |
               (((long)_buffer[index + 1]) << 16 & 0xFF0000L) |
               (((long)_buffer[index + 2]) << 8  & 0xFF00L) |
               (((long)_buffer[index + 3])       & 0xFFL);
    } else {
        // Intel ordering - LSB first (little endian)
        return (((long)_buffer[index + 3]) << 24 & 0xFF000000L) |
               (((long)_buffer[index + 2]) << 16 & 0xFF0000L) |
               (((long)_buffer[index + 1]) << 8  & 0xFF00L) |
               (((long)_buffer[index    ])       & 0xFFL);
    }
}

I haven't tested those chunks of code, but they should get you started even if 
they don't work exactly.

Original comment by drewnoakes on 16 May 2012 at 7:03

GoogleCodeExporter commented 9 years ago
This issue has been fixed in the repo.  Please note that source control has 
moved from SVN to Git.  The old SVN repo is still available, however this and 
all future changes will be available only via Git.

Thanks very much for the bug report.  I'll get a release out soon.  In case you 
can't wait, you can build it yourself from the latest source.

Original comment by drewnoakes on 18 May 2012 at 10:10