diff --git a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/EXIFThumbnailReader.java b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/EXIFThumbnailReader.java index a54645ae..828056f9 100644 --- a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/EXIFThumbnailReader.java +++ b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/EXIFThumbnailReader.java @@ -32,9 +32,12 @@ import com.twelvemonkeys.imageio.metadata.Directory; import com.twelvemonkeys.imageio.metadata.Entry; import com.twelvemonkeys.imageio.metadata.exif.TIFF; import com.twelvemonkeys.imageio.util.IIOUtil; +import com.twelvemonkeys.lang.Validate; import javax.imageio.IIOException; +import javax.imageio.ImageReader; import javax.imageio.stream.ImageInputStream; +import javax.imageio.stream.MemoryCacheImageInputStream; import java.awt.image.BufferedImage; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -51,14 +54,16 @@ import java.util.Arrays; * @version $Id: EXIFThumbnail.java,v 1.0 18.04.12 12:19 haraldk Exp$ */ final class EXIFThumbnailReader extends ThumbnailReader { + private final ImageReader reader; private final Directory ifd; private final ImageInputStream stream; private final int compression; private transient SoftReference cachedThumbnail; - public EXIFThumbnailReader(ThumbnailReadProgressListener progressListener, int imageIndex, int thumbnailIndex, Directory ifd, ImageInputStream stream) { + public EXIFThumbnailReader(ThumbnailReadProgressListener progressListener, ImageReader jpegReader, int imageIndex, int thumbnailIndex, Directory ifd, ImageInputStream stream) { super(progressListener, imageIndex, thumbnailIndex); + this.reader = Validate.notNull(jpegReader); this.ifd = ifd; this.stream = stream; @@ -126,7 +131,13 @@ final class EXIFThumbnailReader extends ThumbnailReader { input = new SequenceInputStream(new ByteArrayInputStream(fakeEmptyExif), input); try { - return readJPEGThumbnail(input); + MemoryCacheImageInputStream stream = new MemoryCacheImageInputStream(input); + try { + return readJPEGThumbnail(reader, stream); + } + finally { + stream.close(); + } } finally { input.close(); diff --git a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JFXXThumbnailReader.java b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JFXXThumbnailReader.java index cce8b3bb..8de14dcb 100644 --- a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JFXXThumbnailReader.java +++ b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/JFXXThumbnailReader.java @@ -29,10 +29,14 @@ package com.twelvemonkeys.imageio.plugins.jpeg; import com.twelvemonkeys.image.InverseColorMapIndexColorModel; +import com.twelvemonkeys.imageio.stream.ByteArrayImageInputStream; +import com.twelvemonkeys.lang.Validate; import javax.imageio.IIOException; +import javax.imageio.ImageReader; +import javax.imageio.metadata.IIOMetadata; +import javax.imageio.stream.ImageInputStream; import java.awt.image.*; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.lang.ref.SoftReference; @@ -45,12 +49,14 @@ import java.lang.ref.SoftReference; */ final class JFXXThumbnailReader extends ThumbnailReader { + private final ImageReader reader; private final JFXXSegment segment; private transient SoftReference cachedThumbnail; - protected JFXXThumbnailReader(final ThumbnailReadProgressListener progressListener, final int imageIndex, final int thumbnailIndex, final JFXXSegment segment) { + protected JFXXThumbnailReader(final ThumbnailReadProgressListener progressListener, ImageReader jpegReader, final int imageIndex, final int thumbnailIndex, final JFXXSegment segment) { super(progressListener, imageIndex, thumbnailIndex); + this.reader = Validate.notNull(jpegReader); this.segment = segment; } @@ -79,11 +85,30 @@ final class JFXXThumbnailReader extends ThumbnailReader { return thumbnail; } + public IIOMetadata readMetadata() throws IOException { + ImageInputStream input = new ByteArrayImageInputStream(segment.thumbnail); + + try { + reader.setInput(input); + + return reader.getImageMetadata(0); + } + finally { + input.close(); + } + } + private BufferedImage readJPEGCached(boolean pixelsExposed) throws IOException { BufferedImage thumbnail = cachedThumbnail != null ? cachedThumbnail.get() : null; if (thumbnail == null) { - thumbnail = readJPEGThumbnail(new ByteArrayInputStream(segment.thumbnail)); + ImageInputStream stream = new ByteArrayImageInputStream(segment.thumbnail); + try { + thumbnail = readJPEGThumbnail(reader, stream); + } + finally { + stream.close(); + } } cachedThumbnail = pixelsExposed ? null : new SoftReference(thumbnail); 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 2f33ac12..210975cb 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 @@ -41,11 +41,15 @@ import com.twelvemonkeys.imageio.metadata.jpeg.JPEGSegment; import com.twelvemonkeys.imageio.metadata.jpeg.JPEGSegmentUtil; import com.twelvemonkeys.imageio.util.ProgressListenerBase; import com.twelvemonkeys.lang.Validate; +import com.twelvemonkeys.xml.XMLSerializer; +import org.w3c.dom.Element; import org.w3c.dom.Node; +import org.w3c.dom.NodeList; import javax.imageio.*; import javax.imageio.event.IIOReadUpdateListener; import javax.imageio.event.IIOReadWarningListener; +import javax.imageio.metadata.IIOInvalidTreeException; import javax.imageio.metadata.IIOMetadata; import javax.imageio.metadata.IIOMetadataNode; import javax.imageio.spi.ImageReaderSpi; @@ -57,6 +61,7 @@ import java.awt.color.ICC_ColorSpace; import java.awt.color.ICC_Profile; import java.awt.image.*; import java.io.*; +import java.nio.charset.Charset; import java.util.*; import java.util.List; @@ -97,12 +102,16 @@ public class JPEGImageReader extends ImageReaderBase { private final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.plugins.jpeg.debug")); + /** Internal constant for referring all APP segments */ + private static final int ALL_APP_MARKERS = -1; + /** Segment identifiers for the JPEG segments we care about reading. */ private static final Map> SEGMENT_IDENTIFIERS = createSegmentIds(); private static Map> createSegmentIds() { Map> map = new LinkedHashMap>(); + /* // JFIF/JFXX APP0 markers map.put(JPEG.APP0, JPEGSegmentUtil.ALL_IDS); @@ -114,6 +123,12 @@ public class JPEGImageReader extends ImageReaderBase { // Adobe APP14 marker map.put(JPEG.APP14, Collections.singletonList("Adobe")); + //*/ + + // Need all APP markers to be able to re-generate proper metadata later + for (int appMarker = JPEG.APP0; appMarker <= JPEG.APP15; appMarker++) { + map.put(appMarker, JPEGSegmentUtil.ALL_IDS); + } // SOFn markers map.put(JPEG.SOF0, null); @@ -135,6 +150,7 @@ public class JPEGImageReader extends ImageReaderBase { /** Our JPEG reading delegate */ private final ImageReader delegate; + private ImageReader thumbnailReader; /** Listens to progress updates in the delegate, and delegates back to this instance */ private final ProgressDelegator progressDelegator; @@ -163,6 +179,10 @@ public class JPEGImageReader extends ImageReaderBase { segments = null; thumbnails = null; + if (thumbnailReader != null) { + thumbnailReader.reset(); + } + installListeners(); } @@ -170,6 +190,11 @@ public class JPEGImageReader extends ImageReaderBase { public void dispose() { super.dispose(); + if (thumbnailReader != null) { + thumbnailReader.dispose(); + thumbnailReader = null; + } + delegate.dispose(); } @@ -317,6 +342,7 @@ public class JPEGImageReader extends ImageReaderBase { System.out.println("ICC color profile: " + profile); } + // TODO: Possible to optimize slightly, to avoid readAsRaster for non-CMyK and other good types? return readImageAsRasterAndReplaceColorProfile(imageIndex, param, sof, sourceCSType, adobeDCT, ensureDisplayProfile(profile)); } @@ -673,7 +699,8 @@ public class JPEGImageReader extends ImageReaderBase { List appSegments = Collections.emptyList(); for (JPEGSegment segment : segments) { - if (segment.marker() == marker && (identifier == null || identifier.equals(segment.identifier()))) { + if ((marker == ALL_APP_MARKERS && segment.marker() >= JPEG.APP0 && segment.marker() <= JPEG.APP15 || segment.marker() == marker) + && (identifier == null || identifier.equals(segment.identifier()))) { if (appSegments == Collections.EMPTY_LIST) { appSegments = new ArrayList(segments.size()); } @@ -928,7 +955,7 @@ public class JPEGImageReader extends ImageReaderBase { case JFXXSegment.JPEG: case JFXXSegment.INDEXED: case JFXXSegment.RGB: - thumbnails.add(new JFXXThumbnailReader(thumbnailProgressDelegator, imageIndex, thumbnails.size(), jfxx)); + thumbnails.add(new JFXXThumbnailReader(thumbnailProgressDelegator, getThumbnailReader(), imageIndex, thumbnails.size(), jfxx)); break; default: processWarningOccurred("Unknown JFXX extension code: " + jfxx.extensionCode); @@ -956,7 +983,7 @@ public class JPEGImageReader extends ImageReaderBase { // 1 = no compression, 6 = JPEG compression (default) if (compression == null || compression.getValue().equals(1) || compression.getValue().equals(6)) { - thumbnails.add(new EXIFThumbnailReader(thumbnailProgressDelegator, 0, thumbnails.size(), ifd1, stream)); + thumbnails.add(new EXIFThumbnailReader(thumbnailProgressDelegator, getThumbnailReader(), 0, thumbnails.size(), ifd1, stream)); } else { processWarningOccurred("EXIF IFD with unknown compression (expected 1 or 6): " + compression.getValue()); @@ -967,6 +994,14 @@ public class JPEGImageReader extends ImageReaderBase { } } + private ImageReader getThumbnailReader() throws IOException { + if (thumbnailReader == null) { + thumbnailReader = delegate.getOriginatingProvider().createReaderInstance(); + } + + return thumbnailReader; + } + @Override public int getNumThumbnails(final int imageIndex) throws IOException { readThumbnailMetadata(imageIndex); @@ -1004,43 +1039,240 @@ public class JPEGImageReader extends ImageReaderBase { @Override public IIOMetadata getImageMetadata(int imageIndex) throws IOException { + // TODO: Extract metadata handling in separate class, for less mess and easier testing + // We filter out pretty much everything from the stream.. + // Meaning we have to read *all APP segments* and re-insert into metadata. + + // TODO: There's a bug in the merging code in JPEGMetadata mergeUnknownNode that makes sure all "unknown" nodes are added twice in certain conditions.... ARGHBL... + // TODO: 1: Work around + // TODO: 2: REPORT BUG! + + List appSegments = getAppSegments(ALL_APP_MARKERS, null); +// System.out.println("appSegments: " + appSegments); + IIOMetadata metadata = delegate.getImageMetadata(imageIndex); if (metadata != null) { String format = metadata.getNativeMetadataFormatName(); IIOMetadataNode tree = (IIOMetadataNode) metadata.getAsTree(format); - Node jpegVariety = tree.getElementsByTagName("JPEGvariety").item(0); + IIOMetadataNode jpegVariety = (IIOMetadataNode) tree.getElementsByTagName("JPEGvariety").item(0); + IIOMetadataNode markerSequence = (IIOMetadataNode) tree.getElementsByTagName("markerSequence").item(0); - // TODO: Allow EXIF (as app1EXIF) in the JPEGvariety (sic) node. - // As EXIF is (a subset of) TIFF, (and the EXIF data is a valid TIFF stream) probably use something like: - // http://download.java.net/media/jai-imageio/javadoc/1.1/com/sun/media/imageio/plugins/tiff/package-summary.html#ImageMetadata - /* - from: http://docs.oracle.com/javase/6/docs/api/javax/imageio/metadata/doc-files/jpeg_metadata.html + JFIFSegment jfifSegment = getJFIF(); + JFXXSegment jfxxSegment = getJFXX(); + AdobeDCTSegment adobeDCT = getAdobeDCT(); + ICC_Profile embeddedICCProfile = getEmbeddedICCProfile(true); + SOFSegment sof = getSOF(); - In future versions of the JPEG metadata format, other varieties of JPEG metadata may be supported (e.g. Exif) - by defining other types of nodes which may appear as a child of the JPEGvariety node. + boolean hasRealJFIF = false; + boolean hasRealJFXX = false; + boolean hasRealICC = false; - (Note that an application wishing to interpret Exif metadata given a metadata tree structure in the - javax_imageio_jpeg_image_1.0 format must check for an unknown marker segment with a tag indicating an - APP1 marker and containing data identifying it as an Exif marker segment. Then it may use application-specific - code to interpret the data in the marker segment. If such an application were to encounter a metadata tree - formatted according to a future version of the JPEG metadata format, the Exif marker segment might not be - unknown in that format - it might be structured as a child node of the JPEGvariety node. + if (jfifSegment != null) { + // Normal case, conformant JFIF with 1 or 3 components + // TODO: Test if we have CMY or other bad color space? + // TODO: Remove JFIF if app14Adobe transform is YCCK (and isn't incorrect...) + if (sof.componentsInFrame() == 1 || sof.componentsInFrame() == 3) { + IIOMetadataNode jfif = new IIOMetadataNode("app0JFIF"); + jfif.setAttribute("majorVersion", String.valueOf(jfifSegment.majorVersion)); + jfif.setAttribute("minorVersion", String.valueOf(jfifSegment.minorVersion)); + jfif.setAttribute("resUnits", String.valueOf(jfifSegment.units)); + jfif.setAttribute("Xdensity", String.valueOf(jfifSegment.xDensity)); + jfif.setAttribute("Ydensity", String.valueOf(jfifSegment.yDensity)); + jfif.setAttribute("thumbWidth", String.valueOf(jfifSegment.xThumbnail)); + jfif.setAttribute("thumbHeight", String.valueOf(jfifSegment.yThumbnail)); - Thus, it is important for an application to specify which version to use by passing the string identifying - the version to the method/constructor used to obtain an IIOMetadata object.) - */ + jpegVariety.appendChild(jfif); + hasRealJFIF = true; - IIOMetadataNode app2ICC = new IIOMetadataNode("app2ICC"); - app2ICC.setUserObject(getEmbeddedICCProfile(true)); - Node jpegVarietyFirstChild = jpegVariety.getFirstChild(); - if (jpegVarietyFirstChild != null) { - jpegVarietyFirstChild.appendChild(app2ICC); + // Add app2ICC and JFXX as proper nodes + if (embeddedICCProfile != null) { + IIOMetadataNode app2ICC = new IIOMetadataNode("app2ICC"); + app2ICC.setUserObject(embeddedICCProfile); + jfif.appendChild(app2ICC); + hasRealICC = true; + } + + if (jfxxSegment != null) { + IIOMetadataNode JFXX = new IIOMetadataNode("JFXX"); + jfif.appendChild(JFXX); + IIOMetadataNode app0JFXX = new IIOMetadataNode("app0JFXX"); + app0JFXX.setAttribute("extensionCode", String.valueOf(jfxxSegment.extensionCode)); + + JFXXThumbnailReader reader = new JFXXThumbnailReader(null, getThumbnailReader(), imageIndex, -1, jfxxSegment); + + IIOMetadataNode jfifThumb; + switch (jfxxSegment.extensionCode) { + case JFXXSegment.JPEG: + jfifThumb = new IIOMetadataNode("JFIFthumbJPEG"); + // Contains it's own "markerSequence" with full DHT, DQT, SOF etc... + IIOMetadata thumbMeta = reader.readMetadata(); + Node thumbTree = thumbMeta.getAsTree(format); + jfifThumb.appendChild(thumbTree.getLastChild()); + app0JFXX.appendChild(jfifThumb); + break; + + case JFXXSegment.INDEXED: + jfifThumb = new IIOMetadataNode("JFIFthumbPalette"); + jfifThumb.setAttribute("thumbWidth", String.valueOf(reader.getWidth())); + jfifThumb.setAttribute("thumbHeight", String.valueOf(reader.getHeight())); + app0JFXX.appendChild(jfifThumb); + break; + + case JFXXSegment.RGB: + jfifThumb = new IIOMetadataNode("JFIFthumbRGB"); + jfifThumb.setAttribute("thumbWidth", String.valueOf(reader.getWidth())); + jfifThumb.setAttribute("thumbHeight", String.valueOf(reader.getHeight())); + app0JFXX.appendChild(jfifThumb); + break; + + default: + processWarningOccurred(String.format("Unknown JFXX extension code: %d", jfxxSegment.extensionCode)); + } + + JFXX.appendChild(app0JFXX); + hasRealJFXX = true; + } + } + else { + // Typically CMYK with JFIF segment (Adobe or similar). + processWarningOccurred(String.format( + "Incompatible JFIF marker segment in stream. " + + "SOF%d has %d color components, JFIF allows only 1 or 3 components. Ignoring JFIF marker.", + sof.marker & 0xf, sof.componentsInFrame() + )); + } } - // new XMLSerializer(System.err, System.getProperty("file.encoding")).serialize(tree, false); + // Special case: Broken AdobeDCT segment, inconsistent with SOF, use values from SOF + if (adobeDCT != null && adobeDCT.getTransform() == AdobeDCTSegment.YCCK && sof.componentsInFrame() < 4) { + processWarningOccurred(String.format( + "Invalid Adobe App14 marker. Indicates YCCK/CMYK data, but SOF%d has %d color components. " + + "Ignoring Adobe App14 marker.", + sof.marker & 0xf, sof.componentsInFrame() + )); - metadata.mergeTree(format, tree); + // Remove bad AdobeDCT + NodeList app14Adobe = tree.getElementsByTagName("app14Adobe"); + for (int i = app14Adobe.getLength() - 1; i >= 0; i--) { + Node item = app14Adobe.item(i); + item.getParentNode().removeChild(item); + } + + + // TODO: Add app14 as "unknown" marker? +// IIOMetadataNode app14Adobe = new IIOMetadataNode("app14Adobe"); +// app14Adobe.setAttribute("version", String.valueOf(adobeDCT.getVersion())); +// app14Adobe.setAttribute("flags0", String.valueOf(adobeDCT.getFlags0())); +// app14Adobe.setAttribute("flags1", String.valueOf(adobeDCT.getFlags1())); +// app14Adobe.setAttribute("transform", String.valueOf(sof.componentsInFrame() == 3 ? )); +// markerSequence.appendChild(app14Adobe); + } + + Node next = null; + for (JPEGSegment segment : appSegments) { + // TODO: Except real app0JFIF, app0JFXX, app2ICC and app14Adobe, add all the app segments that we filtered away as "unknown" markers + if (segment.marker() == JPEG.APP0 && "JFIF".equals(segment.identifier()) && hasRealJFIF) { + continue; + } + else if (segment.marker() == JPEG.APP0 && "JFXX".equals(segment.identifier()) && hasRealJFXX) { + continue; + } + else if (segment.marker() == JPEG.APP1 && "Exif".equals(segment.identifier()) /* always inserted */) { + continue; + } + else if (segment.marker() == JPEG.APP2 && "ICC_PROFILE".equals(segment.identifier()) && hasRealICC) { + continue; + } + else if (segment.marker() == JPEG.APP14 && "Adobe".equals(segment.identifier()) /* always inserted */) { + continue; + } + + IIOMetadataNode unknown = new IIOMetadataNode("unknown"); + unknown.setAttribute("MarkerTag", Integer.toString(segment.marker() & 0xff)); + + DataInputStream stream = new DataInputStream(segment.data()); + + try { + String identifier = segment.identifier(); + int off = identifier != null ? identifier.length() + 1 : 0; + + byte[] data = new byte[off + segment.length()]; + + if (identifier != null) { + System.arraycopy(identifier.getBytes(Charset.forName("ASCII")), 0, data, 0, identifier.length()); + } + + stream.readFully(data, off, segment.length()); + + unknown.setUserObject(data); + } + finally { + stream.close(); + } + + if (next == null) { + next = markerSequence.getFirstChild(); + } + + markerSequence.insertBefore(unknown, next); + } + + // Inconsistency issue in the com.sun classes, it can read metadata with dht containing + // more than 4 children, but will not allow setting such a tree... + // We'll split AC/DC tables into separate dht nodes. + NodeList dhts = markerSequence.getElementsByTagName("dht"); + for (int j = 0; j < dhts.getLength(); j++) { + Node dht = dhts.item(j); + NodeList dhtables = dht.getChildNodes(); + + if (dhtables.getLength() > 4) { + IIOMetadataNode acTables = new IIOMetadataNode("dht"); + dht.getParentNode().insertBefore(acTables, dht.getNextSibling()); + + + // Split into 2 dht nodes, one for AC and one for DC + for (int i = 0; i < dhtables.getLength(); i++) { + Element dhtable = (Element) dhtables.item(i); + String tableClass = dhtable.getAttribute("class"); + if ("1".equals(tableClass)) { + dht.removeChild(dhtable); + acTables.appendChild(dhtable); + } + } + } + } + + // TODO: Allow EXIF (as app1EXIF) in the JPEGvariety (sic) node. + // As EXIF is (a subset of) TIFF, (and the EXIF data is a valid TIFF stream) probably use something like: + // http://download.java.net/media/jai-imageio/javadoc/1.1/com/sun/media/imageio/plugins/tiff/package-summary.html#ImageMetadata + /* + from: http://docs.oracle.com/javase/6/docs/api/javax/imageio/metadata/doc-files/jpeg_metadata.html + + In future versions of the JPEG metadata format, other varieties of JPEG metadata may be supported (e.g. Exif) + by defining other types of nodes which may appear as a child of the JPEGvariety node. + + (Note that an application wishing to interpret Exif metadata given a metadata tree structure in the + javax_imageio_jpeg_image_1.0 format must check for an unknown marker segment with a tag indicating an + APP1 marker and containing data identifying it as an Exif marker segment. Then it may use application-specific + code to interpret the data in the marker segment. If such an application were to encounter a metadata tree + formatted according to a future version of the JPEG metadata format, the Exif marker segment might not be + unknown in that format - it might be structured as a child node of the JPEGvariety node. + + Thus, it is important for an application to specify which version to use by passing the string identifying + the version to the method/constructor used to obtain an IIOMetadata object.) + */ + +// new XMLSerializer(System.out, System.getProperty("file.encoding")).serialize(tree, false); + + try { + metadata.setFromTree(format, tree); + } + catch (IIOInvalidTreeException e) { + new XMLSerializer(System.out, System.getProperty("file.encoding")).serialize(tree, false); + throw e; + } +// metadata.mergeTree(format, tree); // TODO: Merging does not work, as the "unknown" tags duplicate insert bug ruins everything. Try set instead... } return metadata; @@ -1379,9 +1611,11 @@ public class JPEGImageReader extends ImageReaderBase { // System.err.println("thumbnail: " + thumbnail); showIt(thumbnail, String.format("Thumbnail: %s [%d x %d]", file.getName(), thumbnail.getWidth(), thumbnail.getHeight())); } + + reader.getImageMetadata(0); } catch (IIOException e) { - System.err.println("Could not read thumbnails: " + e.getMessage()); + System.err.println("Could not read thumbnails: " + arg + ": " + e.getMessage()); e.printStackTrace(); } } 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 23ab8f8b..77b7c10b 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 @@ -97,8 +97,9 @@ final class JPEGSegmentImageInputStream extends ImageInputStreamImpl { marker = 0xff00 | stream.readUnsignedByte(); } + // TODO: Optionally skip JFIF only for non-JFIF conformant streams // TODO: Refactor to make various segments optional, we probably only want the "Adobe" APP14 segment, 'Exif' APP1 and very few others - if (isAppSegmentMarker(marker) && marker != JPEG.APP0 && !(marker == JPEG.APP1 && isAppSegmentWithId("Exif", stream)) && marker != JPEG.APP14) { + if (isAppSegmentMarker(marker) && !(marker == JPEG.APP1 && isAppSegmentWithId("Exif", stream)) && marker != JPEG.APP14) { int length = stream.readUnsignedShort(); // Length including length field itself stream.seek(realPosition + 2 + length); // Skip marker (2) + length } diff --git a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/ThumbnailReader.java b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/ThumbnailReader.java index 6354ef93..370022c6 100644 --- a/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/ThumbnailReader.java +++ b/imageio/imageio-jpeg/src/main/java/com/twelvemonkeys/imageio/plugins/jpeg/ThumbnailReader.java @@ -28,12 +28,12 @@ package com.twelvemonkeys.imageio.plugins.jpeg; -import javax.imageio.ImageIO; +import javax.imageio.ImageReader; +import javax.imageio.stream.ImageInputStream; import java.awt.*; import java.awt.color.ColorSpace; import java.awt.image.*; import java.io.IOException; -import java.io.InputStream; /** * ThumbnailReader @@ -42,6 +42,7 @@ import java.io.InputStream; * @author last modified by $Author: haraldk$ * @version $Id: ThumbnailReader.java,v 1.0 18.04.12 12:22 haraldk Exp$ */ +// TODO: Get rid of the com.sun import!! abstract class ThumbnailReader { private final ThumbnailReadProgressListener progressListener; @@ -49,10 +50,11 @@ abstract class ThumbnailReader { protected final int thumbnailIndex; protected ThumbnailReader(final ThumbnailReadProgressListener progressListener, final int imageIndex, final int thumbnailIndex) { - this.progressListener = progressListener; + this.progressListener = progressListener != null ? progressListener : new NullProgressListener(); this.imageIndex = imageIndex; this.thumbnailIndex = thumbnailIndex; } + protected final void processThumbnailStarted() { progressListener.processThumbnailStarted(imageIndex, thumbnailIndex); } @@ -65,8 +67,20 @@ abstract class ThumbnailReader { progressListener.processThumbnailComplete(); } - static protected BufferedImage readJPEGThumbnail(InputStream stream) throws IOException { - return ImageIO.read(stream); + static protected BufferedImage readJPEGThumbnail(final ImageReader reader, final ImageInputStream stream) throws IOException { +// try { +// try { + reader.setInput(stream); + + return reader.read(0); +// } +// finally { +// input.close(); +// } +// } +// finally { +// reader.dispose(); +// } } static protected BufferedImage readRawThumbnail(final byte[] thumbnail, final int size, final int offset, int w, int h) { @@ -82,4 +96,15 @@ abstract class ThumbnailReader { public abstract int getWidth() throws IOException; public abstract int getHeight() throws IOException; + + private static class NullProgressListener implements ThumbnailReadProgressListener { + public void processThumbnailStarted(int imageIndex, int thumbnailIndex) { + } + + public void processThumbnailProgress(float percentageDone) { + } + + public void processThumbnailComplete() { + } + } } diff --git a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/EXIFThumbnailReaderTest.java b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/EXIFThumbnailReaderTest.java index f1b92e27..9a13a17f 100644 --- a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/EXIFThumbnailReaderTest.java +++ b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/EXIFThumbnailReaderTest.java @@ -74,7 +74,7 @@ public class EXIFThumbnailReaderTest extends AbstractThumbnailReaderTest { assertEquals(2, ifds.directoryCount()); - return new EXIFThumbnailReader(progressListener, imageIndex, thumbnailIndex, ifds.getDirectory(1), exifStream); + return new EXIFThumbnailReader(progressListener, ImageIO.getImageReadersByFormatName("JPEG").next(), imageIndex, thumbnailIndex, ifds.getDirectory(1), exifStream); } @Test diff --git a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JFXXThumbnailReaderTest.java b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JFXXThumbnailReaderTest.java index 4f8db3ff..0b8a6402 100644 --- a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JFXXThumbnailReaderTest.java +++ b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JFXXThumbnailReaderTest.java @@ -34,6 +34,7 @@ import com.twelvemonkeys.imageio.metadata.jpeg.JPEGSegmentUtil; import org.junit.Test; import org.mockito.InOrder; +import javax.imageio.ImageIO; import javax.imageio.stream.ImageInputStream; import java.awt.image.BufferedImage; import java.io.IOException; @@ -63,7 +64,7 @@ public class JFXXThumbnailReaderTest extends AbstractThumbnailReaderTest { assertFalse(segments.isEmpty()); JPEGSegment jfxx = segments.get(0); - return new JFXXThumbnailReader(progressListener, imageIndex, thumbnailIndex, JFXXSegment.read(jfxx.data(), jfxx.length())); + return new JFXXThumbnailReader(progressListener, ImageIO.getImageReadersByFormatName("jpeg").next(), imageIndex, thumbnailIndex, JFXXSegment.read(jfxx.data(), jfxx.length())); } @Test 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 0a811122..6666d9ac 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 @@ -29,26 +29,33 @@ package com.twelvemonkeys.imageio.plugins.jpeg; import com.twelvemonkeys.imageio.util.ImageReaderAbstractTestCase; +import org.hamcrest.core.IsInstanceOf; import org.junit.Test; +import org.mockito.internal.matchers.GreaterThan; +import org.w3c.dom.NamedNodeMap; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; -import javax.imageio.IIOException; -import javax.imageio.ImageIO; -import javax.imageio.ImageReadParam; -import javax.imageio.ImageTypeSpecifier; +import javax.imageio.*; import javax.imageio.event.IIOReadWarningListener; import javax.imageio.metadata.IIOMetadata; +import javax.imageio.metadata.IIOMetadataNode; +import javax.imageio.plugins.jpeg.JPEGHuffmanTable; +import javax.imageio.plugins.jpeg.JPEGQTable; import javax.imageio.spi.IIORegistry; import javax.imageio.spi.ImageReaderSpi; +import javax.imageio.stream.ImageInputStream; import java.awt.*; import java.awt.color.ColorSpace; +import java.awt.color.ICC_Profile; import java.awt.image.BufferedImage; import java.awt.image.DataBufferByte; import java.io.IOException; -import java.util.Arrays; -import java.util.Iterator; +import java.util.*; import java.util.List; import static org.junit.Assert.*; +import static org.junit.Assert.assertArrayEquals; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -627,15 +634,15 @@ public class JPEGImageReaderTest extends ImageReaderAbstractTestCase> 16) & 0xff, (expectedRGB[i] >> 16) & 0xff, 5); - assertEquals((actualRGB >> 8) & 0xff, (expectedRGB[i] >> 8) & 0xff, 5); - assertEquals((actualRGB) & 0xff, (expectedRGB[i]) & 0xff, 5); + assertEquals((actualRGB >> 8) & 0xff, (expectedRGB[i] >> 8) & 0xff, 5); + assertEquals((actualRGB ) & 0xff, (expectedRGB[i] ) & 0xff, 5); } } // TODO: Test RGBA/YCbCrA handling @Test - public void testReadMetadataMaybeNull() throws IOException { + public void testReadMetadata() throws IOException { // Just test that we can read the metadata without exceptions JPEGImageReader reader = createReader(); @@ -646,11 +653,276 @@ public class JPEGImageReaderTest extends ImageReaderAbstractTestCase(0)); + + NodeList unknowns = markerSequence.getElementsByTagName("unknown"); + for (int j = 0; j < unknowns.getLength(); j++) { + IIOMetadataNode unknown = (IIOMetadataNode) unknowns.item(j); + assertNotNull(unknown.getUserObject()); // All unknowns must have user object (data array) + } } catch (IIOException e) { - System.err.println(String.format("WARNING: Reading metadata failed for %s image %s: %s", testData, i, e.getMessage())); + fail(String.format("Reading metadata failed for %s image %s: %s", testData, i, e.getMessage())); } } } } + + @Test + public void testReadInconsistentMetadata() throws IOException { + // A collection of JPEG files that makes the JPEGImageReader throw exception "Inconsistent metadata read from stream"... + List resources = Arrays.asList( + "/jpeg/jfif-jfxx-thumbnail-olympus-d320l.jpg", // Ok + "/jpeg/gray-sample.jpg", // Ok + "/jpeg/cmyk-sample.jpg", + "/jpeg/cmyk-sample-multiple-chunk-icc.jpg", + "/jpeg/invalid-icc-duplicate-sequence-numbers-rgb-xerox-dc250-heavyweight-1-progressive-jfif.jpg", + "/jpeg/no-image-types-rgb-us-web-coated-v2-ms-photogallery-exif.jpg" + ); + + for (String resource : resources) { + // Just test that we can read the metadata without exceptions + JPEGImageReader reader = createReader(); + ImageInputStream stream = ImageIO.createImageInputStream(getClassLoaderResource(resource)); + + try { + reader.setInput(stream); + IIOMetadata metadata = reader.getImageMetadata(0); + assertNotNull(String.format("%s: null metadata", resource), metadata); + + Node tree = metadata.getAsTree(metadata.getNativeMetadataFormatName()); + assertNotNull(tree); +// new XMLSerializer(System.err, System.getProperty("file.encoding")).serialize(tree, false); + + } + catch (IIOException e) { + AssertionError fail = new AssertionError(String.format("Reading metadata failed for %ss: %s", resource, e.getMessage())); + fail.initCause(e); + throw fail; + } + finally { + stream.close(); + } + } + } + + @Test + public void testReadMetadataEqualReference() throws IOException { + // Compares the metadata for JFIF-conformant files with metadata from com.sun...JPEGImageReader + JPEGImageReader reader = createReader(); + ImageReader referenceReader; + + try { + @SuppressWarnings("unchecked") + Class spiClass = (Class) Class.forName("com.sun.imageio.plugins.jpeg.JPEGImageReaderSpi"); + ImageReaderSpi provider = spiClass.newInstance(); + referenceReader = provider.createReaderInstance(); + } + catch (Throwable t) { + System.err.println("WARNING: Could not create ImageReader for reference (missing dependency): " + t.getMessage()); + return; + } + + for (TestData testData : getTestData()) { + reader.setInput(testData.getInputStream()); + referenceReader.setInput(testData.getInputStream()); + + for (int i = 0; i < reader.getNumImages(true); i++) { + try { + IIOMetadata reference = referenceReader.getImageMetadata(i); + + try { + IIOMetadata metadata = reader.getImageMetadata(i); + + String[] formatNames = reference.getMetadataFormatNames(); + for (String formatName : formatNames) { + Node referenceTree = reference.getAsTree(formatName); + Node actualTree = metadata.getAsTree(formatName); + +// new XMLSerializer(System.out, System.getProperty("file.encoding")).serialize(actualTree, false); + assertTreesEquals(String.format("Metadata differs for %s image %s ", testData, i), referenceTree, actualTree); + } + } + catch (IIOException e) { + AssertionError fail = new AssertionError(String.format("Reading metadata failed for %s image %s: %s", testData, i, e.getMessage())); + fail.initCause(e); + throw fail; + } + } + catch (IIOException ignore) { + // The reference reader will fail on certain images, we'll just ignore that + System.err.println(String.format("WARNING: Reading reference metadata failed for %s image %s: %s", testData, i, ignore.getMessage())); + } + } + } + } + + private void assertTreesEquals(String message, Node expectedTree, Node actualTree) { + if (expectedTree == actualTree) { + return; + } + + if (expectedTree == null) { + assertNull(actualTree); + } + + assertEquals(String.format("%s: Node names differ", message), expectedTree.getNodeName(), actualTree.getNodeName()); + + NamedNodeMap expectedAttributes = expectedTree.getAttributes(); + NamedNodeMap actualAttributes = actualTree.getAttributes(); + assertEquals(String.format("%s: Number of attributes for <%s> differ", message, expectedTree.getNodeName()), expectedAttributes.getLength(), actualAttributes.getLength()); + for (int i = 0; i < expectedAttributes.getLength(); i++) { + Node item = expectedAttributes.item(i); + assertEquals(String.format("%s: \"%s\" attribute for <%s> differ", message, item.getNodeName(), expectedTree.getNodeName()), item.getNodeValue(), actualAttributes.getNamedItem(item.getNodeName()).getNodeValue()); + } + + // Test for equal user objects. + // - array equals or reflective equality... Most user objects does not have a decent equals method.. :-P + if (expectedTree instanceof IIOMetadataNode) { + assertTrue(String.format("%s: %s not an IIOMetadataNode", message, expectedTree.getNodeName()), actualTree instanceof IIOMetadataNode); + + Object expectedUserObject = ((IIOMetadataNode) expectedTree).getUserObject(); + + if (expectedUserObject != null) { + Object actualUserObject = ((IIOMetadataNode) actualTree).getUserObject(); + assertNotNull(String.format("%s: User object missing for <%s>", message, expectedTree.getNodeName()), actualUserObject); + assertEqualUserObjects(String.format("%s: User objects for <%s MarkerTag\"%s\"> differ", message, expectedTree.getNodeName(), ((IIOMetadataNode) expectedTree).getAttribute("MarkerTag")), expectedUserObject, actualUserObject); + } + } + + // Sort nodes to make sure that sequence of equally named tags does not matter + List expectedChildren = sortNodes(expectedTree.getChildNodes()); + List actualChildren = sortNodes(actualTree.getChildNodes()); + + assertEquals(String.format("%s: Number of child nodes for %s differ", message, expectedTree.getNodeName()), expectedChildren.size(), actualChildren.size()); + + for (int i = 0; i < expectedChildren.size(); i++) { + assertTreesEquals(message + "<" + expectedTree.getNodeName() + ">", expectedChildren.get(i), actualChildren.get(i)); + } + } + + private void assertEqualUserObjects(String message, Object expectedUserObject, Object actualUserObject) { + if (expectedUserObject.equals(actualUserObject)) { + return; + } + + if (expectedUserObject instanceof ICC_Profile) { + if (actualUserObject instanceof ICC_Profile) { + assertArrayEquals(message, ((ICC_Profile) expectedUserObject).getData(), ((ICC_Profile) actualUserObject).getData()); + return; + } + } + else if (expectedUserObject instanceof byte[]) { + if (actualUserObject instanceof byte[]) { + assertArrayEquals(message, (byte[]) expectedUserObject, (byte[]) actualUserObject); + return; + } + } + else if (expectedUserObject instanceof JPEGHuffmanTable) { + if (actualUserObject instanceof JPEGHuffmanTable) { + assertArrayEquals(message, ((JPEGHuffmanTable) expectedUserObject).getLengths(), ((JPEGHuffmanTable) actualUserObject).getLengths()); + assertArrayEquals(message, ((JPEGHuffmanTable) expectedUserObject).getValues(), ((JPEGHuffmanTable) actualUserObject).getValues()); + return; + } + } + else if (expectedUserObject instanceof JPEGQTable) { + if (actualUserObject instanceof JPEGQTable) { + assertArrayEquals(message, ((JPEGQTable) expectedUserObject).getTable(), ((JPEGQTable) actualUserObject).getTable()); + return; + } + } + + fail(expectedUserObject.getClass().getName()); + } + + private List sortNodes(final NodeList nodes) { + ArrayList sortedNodes = new ArrayList(new AbstractList() { + @Override + public IIOMetadataNode get(int index) { + return (IIOMetadataNode) nodes.item(index); + } + + @Override + public int size() { + return nodes.getLength(); + } + }); + + Collections.sort( + sortedNodes, + new Comparator() { + public int compare(IIOMetadataNode left, IIOMetadataNode right) { + int res = left.getNodeName().compareTo(right.getNodeName()); + if (res != 0) { + return res; + } + + // Compare attribute values + NamedNodeMap leftAttributes = left.getAttributes(); // TODO: We should sort left's attributes as well, for stable sorting + handle diffs in attributes + NamedNodeMap rightAttributes = right.getAttributes(); + + for (int i = 0; i < leftAttributes.getLength(); i++) { + Node leftAttribute = leftAttributes.item(i); + Node rightAttribute = rightAttributes.getNamedItem(leftAttribute.getNodeName()); + + if (rightAttribute == null) { + return 1; + } + + res = leftAttribute.getNodeValue().compareTo(rightAttribute.getNodeValue()); + if (res != 0) { + return res; + } + } + + if (left.getUserObject() instanceof byte[] && right.getUserObject() instanceof byte[]) { + byte[] leftBytes = (byte[]) left.getUserObject(); + byte[] rightBytes = (byte[]) right.getUserObject(); + + if (leftBytes.length < rightBytes.length) { + return 1; + } + + if (leftBytes.length > rightBytes.length) { + return -1; + } + + if (leftBytes.length > 0) { + for (int i = 0; i < leftBytes.length; i++) { + if (leftBytes[i] < rightBytes[i]) { + return -1; + } + if (leftBytes[i] > rightBytes[i]) { + return 1; + } + } + } + } + + return 0; + } + } + ); + + return sortedNodes; + } } diff --git a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStreamTest.java b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStreamTest.java index 4027a56d..b42fa9d2 100644 --- a/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStreamTest.java +++ b/imageio/imageio-jpeg/src/test/java/com/twelvemonkeys/imageio/plugins/jpeg/JPEGSegmentImageInputStreamTest.java @@ -77,7 +77,7 @@ public class JPEGSegmentImageInputStreamTest { public void testStreamRealData() throws IOException { ImageInputStream stream = new JPEGSegmentImageInputStream(ImageIO.createImageInputStream(getClassLoaderResource("/jpeg/invalid-icc-duplicate-sequence-numbers-rgb-internal-kodak-srgb-jfif.jpg"))); assertEquals(JPEG.SOI, stream.readUnsignedShort()); - assertEquals(JPEG.APP0, stream.readUnsignedShort()); + assertEquals(JPEG.DQT, stream.readUnsignedShort()); } @Test @@ -88,7 +88,7 @@ public class JPEGSegmentImageInputStreamTest { // NOTE: read(byte[], int, int) must always read len bytes (or until EOF), due to known bug in Sun code assertEquals(20, stream.read(bytes, 0, 20)); - assertArrayEquals(new byte[] {(byte) 0xFF, (byte) 0xD8, (byte) 0xFF, (byte) 0xE0, 0x0, 0x10, 'J', 'F', 'I', 'F', 0x0, 0x1, 0x1, 0x1, 0x1, (byte) 0xCC, 0x1, (byte) 0xCC, 0, 0}, bytes); + assertArrayEquals(new byte[] {(byte) 0xFF, (byte) 0xD8, (byte) 0xFF, (byte) 0xDB, 0x0, 0x43, 0x0, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1, 0x1}, bytes); } @Test @@ -102,7 +102,7 @@ public class JPEGSegmentImageInputStreamTest { assertThat(length, new LessOrEqual(10203l)); // In no case should length increase - assertEquals(9625l, length); // May change, if more chunks are passed to reader... + assertEquals(9607L, length); // May change, if more chunks are passed to reader... } @Test @@ -110,18 +110,15 @@ public class JPEGSegmentImageInputStreamTest { ImageInputStream stream = new JPEGSegmentImageInputStream(ImageIO.createImageInputStream(getClassLoaderResource("/jpeg/no-image-types-rgb-us-web-coated-v2-ms-photogallery-exif.jpg"))); List appSegments = JPEGSegmentUtil.readSegments(stream, JPEGSegmentUtil.APP_SEGMENTS); - assertEquals(3, appSegments.size()); + assertEquals(2, appSegments.size()); - assertEquals(JPEG.APP0, appSegments.get(0).marker()); - assertEquals("JFIF", appSegments.get(0).identifier()); + assertEquals(JPEG.APP1, appSegments.get(0).marker()); + assertEquals("Exif", appSegments.get(0).identifier()); - assertEquals(JPEG.APP1, appSegments.get(1).marker()); - assertEquals("Exif", appSegments.get(1).identifier()); + assertEquals(JPEG.APP14, appSegments.get(1).marker()); + assertEquals("Adobe", appSegments.get(1).identifier()); - assertEquals(JPEG.APP14, appSegments.get(2).marker()); - assertEquals("Adobe", appSegments.get(2).identifier()); - - // And thus, no XMP, no ICC_PROFILE or other segments + // And thus, no JFIF, no XMP, no ICC_PROFILE or other segments } @Test @@ -133,7 +130,7 @@ public class JPEGSegmentImageInputStreamTest { length++; } - assertEquals(9299l, length); // Sanity check: same as file size + assertEquals(9281L, length); // Sanity check: same as file size } @Test @@ -141,13 +138,10 @@ public class JPEGSegmentImageInputStreamTest { ImageInputStream stream = new JPEGSegmentImageInputStream(ImageIO.createImageInputStream(getClassLoaderResource("/jpeg/jfif-padded-segments.jpg"))); List appSegments = JPEGSegmentUtil.readSegments(stream, JPEGSegmentUtil.APP_SEGMENTS); - assertEquals(2, appSegments.size()); + assertEquals(1, appSegments.size()); - assertEquals(JPEG.APP0, appSegments.get(0).marker()); - assertEquals("JFIF", appSegments.get(0).identifier()); - - assertEquals(JPEG.APP1, appSegments.get(1).marker()); - assertEquals("Exif", appSegments.get(1).identifier()); + assertEquals(JPEG.APP1, appSegments.get(0).marker()); + assertEquals("Exif", appSegments.get(0).identifier()); stream.seek(0l); @@ -156,6 +150,6 @@ public class JPEGSegmentImageInputStreamTest { length++; } - assertEquals(1079L, length); // Sanity check: same as file size, except padding and the filtered ICC_PROFILE segment + assertEquals(1061L, length); // Sanity check: same as file size, except padding and the filtered ICC_PROFILE segment } }