diff --git a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java index 811c5803..29c3d347 100644 --- a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java +++ b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReader.java @@ -70,7 +70,7 @@ import java.util.List; */ public class JPEGImageReader extends ImageReaderBase { // TODO: Fix the (stream) metadata inconsistency issues. - // - Sun JPEGMetadata class does not (and can not be made to) support CMYK data.. We need to create all new metadata classes + // - Sun JPEGMetadata class does not (and can not be made to) support CMYK data.. We need to create all new metadata classes.. :-/ private final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.plugins.jpeg.debug")); @@ -543,7 +543,7 @@ public class JPEGImageReader extends ImageReaderBase { ImageInputStream stream = ImageIO.createImageInputStream(data); CompoundDirectory exifMetadata = (CompoundDirectory) new EXIFReader().read(stream); - /**/ + if (exifMetadata.directoryCount() == 2) { Directory ifd1 = exifMetadata.getDirectory(1); Entry compression = ifd1.getEntryById(TIFF.TAG_COMPRESSION); @@ -556,7 +556,7 @@ public class JPEGImageReader extends ImageReaderBase { if (width == null || height == null) { throw new IIOException("Missing dimensions for RAW EXIF thumbnail"); } - + Entry bitsPerSample = ifd1.getEntryById(TIFF.TAG_BITS_PER_SAMPLE); Entry samplesPerPixel = ifd1.getEntryById(TIFF.TAG_SAMPLES_PER_PIXELS); Entry photometricInterpretation = ifd1.getEntryById(TIFF.TAG_PHOTOMETRIC_INTERPRETATION); @@ -578,41 +578,49 @@ public class JPEGImageReader extends ImageReaderBase { int interpretation = photometricInterpretation != null ? ((Number) photometricInterpretation.getValue()).intValue() : 2; - // Read raw image data, either RGB or YCbCr - byte[] thumbData = readFully(stream, w * h * 3); - DataBuffer buffer = new DataBufferByte(thumbData, thumbData.length); - WritableRaster raster = Raster.createInterleavedRaster(buffer, w, h, w * 3, 3, new int[] {0, 1, 2}, null); + // IFD1 should contain strip offsets for uncompressed images + Entry offset = ifd1.getEntryById(TIFF.TAG_STRIP_OFFSETS); + if (offset != null) { + stream.seek(((Number) offset.getValue()).longValue()); - switch (interpretation) { - case 2: - // RGB - break; - case 6: - // YCbCr - YCbCrConverter.convertYCbCr2RGB(raster); - break; - default: - throw new IIOException("Unknown photometric interpretation for RAW EXIF thumbail: " + interpretation); + // Read raw image data, either RGB or YCbCr + int thumbSize = w * h * 3; + byte[] thumbData = readFully(stream, thumbSize); + + switch (interpretation) { + case 2: + // RGB + break; + case 6: + // YCbCr + for (int i = 0, thumbDataLength = thumbData.length; i < thumbDataLength; i++) { + YCbCrConverter.convertYCbCr2RGB(thumbData, thumbData, i); + } + break; + default: + throw new IIOException("Unknown photometric interpretation for RAW EXIF thumbail: " + interpretation); + } + + thumbnails.add(readRawThumbnail(thumbData, thumbData.length, 0, w, h)); } - - ColorModel cm = new ComponentColorModel(ColorSpace.getInstance(ColorSpace.CS_sRGB),false, false, Transparency.OPAQUE, DataBuffer.TYPE_BYTE); - - thumbnails.add(new BufferedImage(cm, raster, cm.isAlphaPremultiplied(), null)); } else if (compression == null || compression.getValue().equals(6)) { Entry jpegOffset = ifd1.getEntryById(TIFF.TAG_JPEG_INTERCHANGE_FORMAT); + + // IFD1 should contain jpeg offset for JPEG thumbnail if (jpegOffset != null) { - stream.seek((Long) jpegOffset.getValue()); + stream.seek(((Number) jpegOffset.getValue()).longValue()); InputStream adapter = IIOUtil.createStreamAdapter(stream); BufferedImage exifThumb = ImageIO.read(adapter); + if (exifThumb != null) { thumbnails.add(exifThumb); } + adapter.close(); } } } - //*/ return exifMetadata; } @@ -841,8 +849,7 @@ public class JPEGImageReader extends ImageReaderBase { JFIF jfif = getJFIF(); if (jfif != null && jfif.thumbnail != null) { - // TODO: Actually decode jfif - thumbnails.add(new BufferedImage(jfif.xThumbnail, jfif.yThumbnail, BufferedImage.TYPE_3BYTE_BGR)); + thumbnails.add(readRawThumbnail(jfif.thumbnail, jfif.thumbnail.length, 0, jfif.xThumbnail, jfif.yThumbnail)); } JFXX jfxx = getJFXX(); @@ -861,10 +868,10 @@ public class JPEGImageReader extends ImageReaderBase { int[] rgbs = new int[256]; for (int i = 0; i < rgbs.length; i++) { - int rgb = (jfxx.thumbnail[3 * i] & 0xff) << 16| - (jfxx.thumbnail[3 * i] & 0xff) << 8 | - (jfxx.thumbnail[3 * i] & 0xff); - + int rgb = (jfxx.thumbnail[3 * i] & 0xff) << 16 + | (jfxx.thumbnail[3 * i] & 0xff) << 8 + | (jfxx.thumbnail[3 * i] & 0xff); + rgbs[i] = rgb; } @@ -881,19 +888,13 @@ public class JPEGImageReader extends ImageReaderBase { w = jfxx.thumbnail[0] & 0xff; h = jfxx.thumbnail[1] & 0xff; - buffer = new DataBufferByte(jfxx.thumbnail, jfxx.thumbnail.length - 2, 2); - raster = Raster.createInterleavedRaster(buffer, w, h, w * 3, 3, new int[] {0, 1, 2}, null); - ColorModel cm = new ComponentColorModel(ColorSpace.getInstance(ColorSpace.CS_sRGB),false, false, Transparency.OPAQUE, DataBuffer.TYPE_BYTE); - - thumbnails.add(new BufferedImage(cm, raster, cm.isAlphaPremultiplied(), null)); + thumbnails.add(readRawThumbnail(jfxx.thumbnail, jfxx.thumbnail.length - 2, 2, w, h)); break; - default: processWarningOccurred("Unknown JFXX extension code: " + jfxx.extensionCode); } } - // TODO: Ideally we want to decode image data in getThumbnail, less ideally here, but at least not in getEXIFMetadata() CompoundDirectory exifMetadata = getEXIFMetadata(); // System.err.println("exifMetadata: " + exifMetadata); @@ -905,6 +906,16 @@ public class JPEGImageReader extends ImageReaderBase { } } + // TODO: Candidate for util method + private BufferedImage readRawThumbnail(final byte[] thumbnail, final int size, final int offset, int w, int h) { + DataBufferByte buffer;WritableRaster raster; + buffer = new DataBufferByte(thumbnail, size, offset); + raster = Raster.createInterleavedRaster(buffer, w, h, w * 3, 3, new int[] {0, 1, 2}, null); + ColorModel cm = new ComponentColorModel(ColorSpace.getInstance(ColorSpace.CS_sRGB),false, false, Transparency.OPAQUE, DataBuffer.TYPE_BYTE); + + return new BufferedImage(cm, raster, cm.isAlphaPremultiplied(), null); + } + @Override public int getNumThumbnails(final int imageIndex) throws IOException { readThumbnailMetadata(imageIndex); @@ -935,12 +946,10 @@ public class JPEGImageReader extends ImageReaderBase { public BufferedImage readThumbnail(int imageIndex, int thumbnailIndex) throws IOException { checkThumbnailBounds(imageIndex, thumbnailIndex); - // TODO: Thumbnail progress listeners... - - BufferedImage thumbnail = thumbnails.get(thumbnailIndex); processThumbnailStarted(imageIndex, thumbnailIndex); // For now: Clone. TODO: Do the actual decoding/reading here. - thumbnail = new BufferedImage(thumbnail.getColorModel(), thumbnail.copyData(null), thumbnail.getColorModel().isAlphaPremultiplied(), null); + BufferedImage cached = thumbnails.get(thumbnailIndex); + BufferedImage thumbnail = new BufferedImage(cached.getColorModel(), cached.copyData(null), cached.getColorModel().isAlphaPremultiplied(), null); processThumbnailProgress(100f); processThumbnailComplete(); diff --git a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStream.java b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStream.java index 9dd154b8..3c692dfa 100644 --- a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStream.java +++ b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStream.java @@ -208,6 +208,12 @@ final class JPEGSegmentImageInputStream extends ImageInputStreamImpl { return total; } + @SuppressWarnings({"FinalizeDoesntCallSuperFinalize"}) + @Override + protected void finalize() throws Throwable { + // Empty finalizer (for improved performance; no need to call super.finalize() in this case) + } + static class Segment { private final int marker; diff --git a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReaderTest.java b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReaderTest.java index ccc2c5e3..58c23ec0 100644 --- a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReaderTest.java +++ b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGImageReaderTest.java @@ -303,7 +303,7 @@ public class JPEGImageReaderTest extends ImageReaderAbstractTestCase 13) { @@ -243,7 +242,8 @@ public final class EXIFReader extends MetadataReader { } } - // TODO: For BigTiff allow size > 4 && <= 8 + Object value; + // TODO: For BigTiff allow size > 4 && <= 8 in addition if (valueLength > 0 && valueLength <= 4) { value = readValueInLine(pInput, type, count); pInput.skipBytes(4 - valueLength); @@ -278,8 +278,11 @@ public final class EXIFReader extends MetadataReader { switch (pType) { case 2: // ASCII - // TODO: This might be UTF-8 or ISO-8859-x, even though spec says ASCII + // TODO: This might be UTF-8 or ISO-8859-x, even though spec says NULL-terminated 7 bit ASCII // TODO: Fail if unknown chars, try parsing with ISO-8859-1 or file.encoding + if (pCount == 0) { + return ""; + } byte[] ascii = new byte[pCount]; pInput.readFully(ascii); int len = ascii[ascii.length - 1] == 0 ? ascii.length - 1 : ascii.length; @@ -364,23 +367,23 @@ public final class EXIFReader extends MetadataReader { case 5: // RATIONAL if (pCount == 1) { - return new Rational(pInput.readUnsignedInt(), pInput.readUnsignedInt()); + return createSafeRational(pInput.readUnsignedInt(), pInput.readUnsignedInt()); } Rational[] rationals = new Rational[pCount]; for (int i = 0; i < rationals.length; i++) { - rationals[i] = new Rational(pInput.readUnsignedInt(), pInput.readUnsignedInt()); + rationals[i] = createSafeRational(pInput.readUnsignedInt(), pInput.readUnsignedInt()); } return rationals; case 10: // SRATIONAL if (pCount == 1) { - return new Rational(pInput.readInt(), pInput.readInt()); + return createSafeRational(pInput.readInt(), pInput.readInt()); } Rational[] srationals = new Rational[pCount]; for (int i = 0; i < srationals.length; i++) { - srationals[i] = new Rational(pInput.readInt(), pInput.readInt()); + srationals[i] = createSafeRational(pInput.readInt(), pInput.readInt()); } return srationals; @@ -412,6 +415,15 @@ public final class EXIFReader extends MetadataReader { } } + private static Rational createSafeRational(final long numerator, final long denominator) throws IOException { + if (denominator == 0) { + // Bad data. + return Rational.NaN; + } + + return new Rational(numerator, denominator); + } + private int getValueLength(final int pType, final int pCount) { if (pType > 0 && pType <= TIFF.TYPE_LENGTHS.length) { return TIFF.TYPE_LENGTHS[pType - 1] * pCount; diff --git a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/Rational.java b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/Rational.java index 6b9aed06..b0e0a247 100644 --- a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/Rational.java +++ b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/exif/Rational.java @@ -53,10 +53,16 @@ public final class Rational extends Number implements Comparable { // TODO: Move to com.tm.lang? // Inspired by http://www.cs.princeton.edu/introcs/92symbolic/Rational.java.html and java.lang.Integer static final Rational ZERO = new Rational(0, 1); + static final Rational NaN = new Rational(); // TODO: This field needs thoughts/tests/spec/consistency check, see Float.NaN private final long numerator; private final long denominator; + private Rational() { + numerator = 0; + denominator = 0; + } + public Rational(final long pNumber) { this(pNumber, 1); } @@ -121,6 +127,10 @@ public final class Rational extends Number implements Comparable { @Override public double doubleValue() { + if (this == NaN) { + return Double.NaN; + } + return numerator / (double) denominator; } @@ -147,6 +157,10 @@ public final class Rational extends Number implements Comparable { @Override public String toString() { + if (this == NaN) { + return "NaN"; + } + return denominator == 1 ? Long.toString(numerator) : String.format("%s/%s", numerator, denominator); } diff --git a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReaderTest.java b/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReaderTest.java index f7d55011..d454b308 100644 --- a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReaderTest.java +++ b/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/exif/EXIFReaderTest.java @@ -30,10 +30,13 @@ package com.twelvemonkeys.imageio.metadata.exif; import com.twelvemonkeys.imageio.metadata.CompoundDirectory; import com.twelvemonkeys.imageio.metadata.Directory; +import com.twelvemonkeys.imageio.metadata.Entry; import com.twelvemonkeys.imageio.metadata.MetadataReaderAbstractTest; +import com.twelvemonkeys.imageio.stream.SubImageInputStream; import org.junit.Test; import javax.imageio.ImageIO; +import javax.imageio.stream.ImageInputStream; import java.io.IOException; import java.io.InputStream; @@ -131,4 +134,31 @@ public class EXIFReaderTest extends MetadataReaderAbstractTest { assertNull(ifd1.getEntryById(TIFF.TAG_IMAGE_WIDTH)); assertNull(ifd1.getEntryById(TIFF.TAG_IMAGE_HEIGHT)); } + + @Test + public void testReadBadDataZeroCount() throws IOException { + // This image seems to contain bad Exif. But as other tools are able to read, so should we.. + ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-rgb-thumbnail-bad-exif-kodak-dc210.jpg")); + stream.seek(12); + Directory directory = createReader().read(new SubImageInputStream(stream, 21674)); + + assertEquals(22, directory.size()); + + // Special case: Ascii string with count == 0, not ok according to spec (?), but we'll let it pass + assertEquals("", directory.getEntryById(TIFF.TAG_IMAGE_DESCRIPTION).getValue()); + } + + @Test + public void testReadBadDataRationalZeroDenominator() throws IOException { + // This image seems to contain bad Exif. But as other tools are able to read, so should we.. + ImageInputStream stream = ImageIO.createImageInputStream(getResource("/jpeg/exif-rgb-thumbnail-bad-exif-kodak-dc210.jpg")); + stream.seek(12); + Directory directory = createReader().read(new SubImageInputStream(stream, 21674)); + + // Special case: Rational with zero-denominator inside EXIF data + Directory exif = (Directory) directory.getEntryById(TIFF.TAG_EXIF_IFD).getValue(); + Entry entry = exif.getEntryById(EXIF.TAG_COMPRESSED_BITS_PER_PIXEL); + assertNotNull(entry); + assertEquals(Rational.NaN, entry.getValue()); + } } diff --git a/imageio/imageio-metadata/src/test/resources/jpeg/exif-rgb-thumbnail-bad-exif-kodak-dc210.jpg b/imageio/imageio-metadata/src/test/resources/jpeg/exif-rgb-thumbnail-bad-exif-kodak-dc210.jpg new file mode 100644 index 00000000..6ad31099 Binary files /dev/null and b/imageio/imageio-metadata/src/test/resources/jpeg/exif-rgb-thumbnail-bad-exif-kodak-dc210.jpg differ