From c294c5869c38e7dda75067c981f1465a18353386 Mon Sep 17 00:00:00 2001 From: Harald Kuhr Date: Wed, 17 Jan 2018 19:24:31 +0100 Subject: [PATCH] #368 related clean-up --- .../imageio/metadata/AbstractDirectory.java | 2 +- .../imageio/metadata/tiff/TIFFReader.java | 98 +++++++++---------- .../imageio/metadata/exif/EXIFReaderTest.java | 4 +- .../imageio/metadata/tiff/TIFFReaderTest.java | 4 +- 4 files changed, 49 insertions(+), 59 deletions(-) diff --git a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/AbstractDirectory.java b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/AbstractDirectory.java index 93cf0c0f..2802ee63 100644 --- a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/AbstractDirectory.java +++ b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/AbstractDirectory.java @@ -40,7 +40,7 @@ import static com.twelvemonkeys.lang.Validate.noNullElements; * @version $Id: AbstractDirectory.java,v 1.0 Nov 11, 2009 5:31:04 PM haraldk Exp$ */ public abstract class AbstractDirectory implements Directory { - private final List entries = new ArrayList(); + private final List entries = new ArrayList<>(); private final List unmodifiable = Collections.unmodifiableList(entries); protected AbstractDirectory(final Collection entries) { diff --git a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReader.java b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReader.java index 0b3af5da..12f5692e 100644 --- a/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReader.java +++ b/imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReader.java @@ -28,7 +28,6 @@ package com.twelvemonkeys.imageio.metadata.tiff; -import com.twelvemonkeys.imageio.metadata.CompoundDirectory; import com.twelvemonkeys.imageio.metadata.Directory; import com.twelvemonkeys.imageio.metadata.Entry; import com.twelvemonkeys.imageio.metadata.MetadataReader; @@ -105,24 +104,41 @@ public final class TIFFReader extends MetadataReader { throw new IIOException(String.format("Wrong TIFF magic in input data: %04x, expected: %04x", magic, TIFF.TIFF_MAGIC)); } - long directoryOffset = readOffset(input); + return readLinkedIFDs(input); + } - return readDirectory(input, directoryOffset, true); + private TIFFDirectory readLinkedIFDs(final ImageInputStream input) throws IOException { + long nextOffset = readOffset(input); + + List ifds = new ArrayList<>(); + + // Read linked IFDs + while (nextOffset != 0) { + try { + ifds.add(readIFD(input, nextOffset)); + + nextOffset = readOffset(input); + } + catch (EOFException eof) { + // catch EOF here as missing EOF marker + nextOffset = 0; + } + } + + return new TIFFDirectory(ifds); } private long readOffset(final ImageInputStream input) throws IOException { return longOffsets ? input.readLong() : input.readUnsignedInt(); } - // TODO: Consider re-writing so that the linked IFD parsing is done externally to the method - protected Directory readDirectory(final ImageInputStream pInput, final long pOffset, final boolean readLinked) throws IOException { - List ifds = new ArrayList<>(); - List entries = new ArrayList<>(); - + private IFD readIFD(final ImageInputStream pInput, final long pOffset) throws IOException { pInput.seek(pOffset); long entryCount = readEntryCount(pInput); + List entries = new ArrayList<>(); + for (int i = 0; i < entryCount; i++) { try { TIFFEntry entry = readEntry(pInput); @@ -136,60 +152,27 @@ public final class TIFFReader extends MetadataReader { } } - if (readLinked) { - long nextOffset; - - try { - nextOffset = readOffset(pInput); - } - catch (EOFException e) { - // catch EOF here as missing EOF marker - nextOffset = 0; - } - - // Read linked IFDs - if (nextOffset != 0) { - CompoundDirectory next = (CompoundDirectory) readDirectory(pInput, nextOffset, true); - - for (int i = 0; i < next.directoryCount(); i++) { - Directory directory = next.getDirectory(i); - - // Linked directories might be empty if nextOffset is after EOF (skip these) - if (directory.size() > 0) { - ifds.add((IFD) directory); - } - } - } - } - // TODO: Consider leaving to client code what sub-IFDs to parse (but always parse TAG_SUB_IFD). - readSubdirectories(pInput, entries, + readSubIFDs(pInput, entries, Arrays.asList(TIFF.TAG_EXIF_IFD, TIFF.TAG_GPS_IFD, TIFF.TAG_INTEROP_IFD, TIFF.TAG_SUB_IFD) ); - ifds.add(0, new IFD(entries)); - - return new TIFFDirectory(ifds); + return new IFD(entries); } private long readEntryCount(final ImageInputStream pInput) throws IOException { - try { - return longOffsets ? pInput.readLong() : pInput.readUnsignedShort(); - } - catch (EOFException e) { - // Treat EOF here as empty Sub-IFD - return 0; - } + return longOffsets ? pInput.readLong() : pInput.readUnsignedShort(); } - // TODO: Might be better to leave this for client code, as it's tempting go really overboard and support any possible embedded format.. - private void readSubdirectories(ImageInputStream input, List entries, List subIFDIds) throws IOException { + private void readSubIFDs(ImageInputStream input, List entries, List subIFDIds) throws IOException { if (subIFDIds == null || subIFDIds.isEmpty()) { return; } + long initialPosition = input.getStreamPosition(); + for (int i = 0, entriesSize = entries.size(); i < entriesSize; i++) { - TIFFEntry entry = (TIFFEntry) entries.get(i); + TIFFEntry entry = entries.get(i); int tagId = (Integer) entry.getIdentifier(); if (subIFDIds.contains(tagId)) { @@ -199,10 +182,14 @@ public final class TIFFReader extends MetadataReader { List subIFDs = new ArrayList<>(pointerOffsets.length); for (long pointerOffset : pointerOffsets) { - CompoundDirectory subDirectory = (CompoundDirectory) readDirectory(input, pointerOffset, false); - - for (int j = 0; j < subDirectory.directoryCount(); j++) { - subIFDs.add((IFD) subDirectory.getDirectory(j)); + try { + subIFDs.add(readIFD(input, pointerOffset)); + } + catch (EOFException ignore) { + // TODO: Issue warning + if (DEBUG) { + ignore.printStackTrace(); + } } } @@ -210,7 +197,7 @@ public final class TIFFReader extends MetadataReader { // Replace the entry with parsed data entries.set(i, new TIFFEntry(tagId, entry.getType(), subIFDs.get(0))); } - else { + else if (!subIFDs.isEmpty()) { // Replace the entry with parsed data entries.set(i, new TIFFEntry(tagId, entry.getType(), subIFDs.toArray(new IFD[subIFDs.size()]))); } @@ -225,6 +212,9 @@ public final class TIFFReader extends MetadataReader { } } } + + // Restore initial position + input.seek(initialPosition); } private long[] getPointerOffsets(final Entry entry) throws IIOException { @@ -537,7 +527,7 @@ public final class TIFFReader extends MetadataReader { Directory directory; if (args.length > 1) { - directory = reader.readDirectory(stream, pos, false); + directory = reader.readIFD(stream, pos); } else { directory = reader.read(stream); 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 0a0cfe3a..b768abd8 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 @@ -240,9 +240,9 @@ public class EXIFReaderTest extends MetadataReaderAbstractTest { // The interop IFD isn't there (offset points to outside the TIFF structure)... // Have double-checked using ExifTool, which says "Warning : Bad InteropOffset SubDirectory start" - Directory interop = (Directory) exif.getEntryById(TIFF.TAG_INTEROP_IFD).getValue(); + Object interop = exif.getEntryById(TIFF.TAG_INTEROP_IFD).getValue(); assertNotNull(interop); - assertEquals(0, interop.size()); + assertEquals(240L, interop); } @Test diff --git a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReaderTest.java b/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReaderTest.java index 9ed1ef5e..66c8e1b8 100644 --- a/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReaderTest.java +++ b/imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/tiff/TIFFReaderTest.java @@ -254,9 +254,9 @@ public class TIFFReaderTest extends MetadataReaderAbstractTest { // The interop IFD isn't there (offset points to outside the TIFF structure)... // Have double-checked using ExifTool, which says "Warning : Bad InteropOffset SubDirectory start" - Directory interop = (Directory) exif.getEntryById(TIFF.TAG_INTEROP_IFD).getValue(); + Object interop = exif.getEntryById(TIFF.TAG_INTEROP_IFD).getValue(); assertNotNull(interop); - assertEquals(0, interop.size()); + assertEquals(240L, interop); } @Test