Remove TYPE_SUB_IFD for next() in IfdParser

We should not read the offset because there are
too many exception. Users should check the id of the tag
and decide how to read the block by themselves.

Change-Id: Ie3f162c5c6dc25f15107e193adc080c7b51112b5

diff --git a/src/com/android/gallery3d/exif/ExifTag.java b/src/com/android/gallery3d/exif/ExifTag.java
index c9e08cc..4f1db55 100644 (file)
--- a/src/com/android/gallery3d/exif/ExifTag.java
+++ b/src/com/android/gallery3d/exif/ExifTag.java
@@ -160,24 +160,11 @@ public class ExifTag {
     private final short mTagId;
     private final short mDataType;
     private final int mDataCount;
-    private final int mOffset;
 
     ExifTag(short tagId, short type, int dataCount) {
         mTagId = tagId;
         mDataType = type;
         mDataCount = dataCount;
-        mOffset = -1;
-    }
-
-    ExifTag(short tagId, short type, int dataCount, int offset) {
-        mTagId = tagId;
-        mDataType = type;
-        mDataCount = dataCount;
-        mOffset = offset;
-    }
-
-    public int getOffset() {
-        return mOffset;
     }
 
     public short getTagId() {
@@ -188,6 +175,10 @@ public class ExifTag {
         return mDataType;
     }
 
+    public int getDataSize() {
+        return getComponentCount() * getElementSize(getDataType());
+    }
+
     public int getComponentCount() {
         return mDataCount;
     }

diff --git a/src/com/android/gallery3d/exif/IfdParser.java b/src/com/android/gallery3d/exif/IfdParser.java
index c15a5b6..b0b0bce 100644 (file)
--- a/src/com/android/gallery3d/exif/IfdParser.java
+++ b/src/com/android/gallery3d/exif/IfdParser.java
@@ -18,16 +18,11 @@ package com.android.gallery3d.exif;
 
 import java.io.IOException;
 import java.nio.charset.Charset;
-import java.util.Comparator;
-import java.util.TreeSet;
+import java.util.Map.Entry;
+import java.util.TreeMap;
 
 public class IfdParser {
 
-    // special sub IDF tags
-    private static final short EXIF_IDF = (short) 0x8769;
-    private static final short GPS_IDF = (short) 0x8825;
-    private static final short INTEROPERABILITY_IDF = (short) 0xA005;
-
     private static final int TAG_SIZE = 12;
 
     private final TiffInputStream mTiffStream;
@@ -36,20 +31,15 @@ public class IfdParser {
     private int mNextOffset;
     private int mOffsetToNextIfd = 0;
 
-    private TreeSet<ExifTag> mCorrespondingTag = new TreeSet<ExifTag>(
-            new Comparator<ExifTag>() {
-        @Override
-        public int compare(ExifTag lhs, ExifTag rhs) {
-            return lhs.getOffset() - rhs.getOffset();
-        }
-    });
+    private TreeMap<Integer, ExifTag> mCorrespondingTag = new TreeMap<Integer, ExifTag>();
+
     private ExifTag mCurrTag;
+    private int mCurrTagOffset;
 
     public static final int TYPE_NEW_TAG = 0;
     public static final int TYPE_VALUE_OF_PREV_TAG = 1;
     public static final int TYPE_NEXT_IFD = 2;
     public static final int TYPE_END = 3;
-    public static final int TYPE_SUB_IFD = 4;
 
     IfdParser(TiffInputStream tiffStream, int offset) throws IOException {
         mTiffStream = tiffStream;
@@ -76,13 +66,11 @@ public class IfdParser {
         }
 
         if (!mCorrespondingTag.isEmpty()) {
-            mCurrTag = mCorrespondingTag.pollFirst();
-            skipTo(mCurrTag.getOffset());
-            if (isSubIfdTag(mCurrTag.getTagId())) {
-                return TYPE_SUB_IFD;
-            } else {
-                return TYPE_VALUE_OF_PREV_TAG;
-            }
+            Entry<Integer, ExifTag> entry = mCorrespondingTag.pollFirstEntry();
+            mCurrTag = entry.getValue();
+            mCurrTagOffset = entry.getKey();
+            skipTo(entry.getKey());
+            return TYPE_VALUE_OF_PREV_TAG;
         } else {
             if (offset <= mOffsetToNextIfd) {
                 skipTo(mOffsetToNextIfd);
@@ -103,28 +91,24 @@ public class IfdParser {
             throw new ExifInvalidFormatException(
                     "Number of component is larger then Integer.MAX_VALUE");
         }
-
-        if (ExifTag.getElementSize(dataFormat) * numOfComp > 4
-                || isSubIfdTag(tagId)) {
-            int offset = mTiffStream.readInt();
-            return new ExifTag(tagId, dataFormat, (int) numOfComp, offset);
-        } else {
-            return new ExifTag(tagId, dataFormat, (int) numOfComp);
-        }
+        return new ExifTag(tagId, dataFormat, (int) numOfComp);
     }
 
     public ExifTag getCorrespodingExifTag() {
-        return mCurrTag.getOffset() != mTiffStream.getReadByteCount() ? null : mCurrTag;
+        return mCurrTagOffset != mTiffStream.getReadByteCount() ? null : mCurrTag;
     }
 
-    public void waitValueOfTag(ExifTag tag) {
-        mCorrespondingTag.add(tag);
+    public void waitValueOfTag(ExifTag tag, long offset) {
+        if (offset > Integer.MAX_VALUE || offset < 0) {
+            throw new IllegalArgumentException(offset + " must be in 0 ~ " + Integer.MAX_VALUE);
+        }
+        mCorrespondingTag.put((int) offset, tag);
     }
 
     public void skipTo(int offset) throws IOException {
         mTiffStream.skipTo(offset);
-        while (!mCorrespondingTag.isEmpty() && mCorrespondingTag.first().getOffset() < offset) {
-            mCorrespondingTag.pollFirst();
+        while (!mCorrespondingTag.isEmpty() && mCorrespondingTag.firstKey() < offset) {
+            mCorrespondingTag.pollFirstEntry();
         }
     }
 
@@ -179,8 +163,4 @@ public class IfdParser {
         int denomi = readInt();
         return new Rational(nomi, denomi);
     }
-
-    private static boolean isSubIfdTag(short tagId) {
-        return tagId == EXIF_IDF || tagId == GPS_IDF || tagId == INTEROPERABILITY_IDF;
-    }
 }
\ No newline at end of file

diff --git a/tests/src/com/android/gallery3d/exif/ExifParserTest.java b/tests/src/com/android/gallery3d/exif/ExifParserTest.java
index 3c9d276..a2c5383 100644 (file)
--- a/tests/src/com/android/gallery3d/exif/ExifParserTest.java
+++ b/tests/src/com/android/gallery3d/exif/ExifParserTest.java
@@ -120,8 +120,10 @@ public class ExifParserTest extends ActivityTestCase {
             switch (type) {
                 case IfdParser.TYPE_NEW_TAG:
                     ExifTag tag = ifdParser.readTag();
-                    if (tag.getOffset() > 0) {
-                        ifdParser.waitValueOfTag(tag);
+                    if (tag.getDataSize() > 4 || tag.getTagId() == ExifTag.TIFF_TAG.TAG_EXIF_IFD) {
+                        long offset = ifdParser.readUnsignedInt();
+                        assertTrue(offset <= Integer.MAX_VALUE);
+                        ifdParser.waitValueOfTag(tag, offset);
                     } else {
                         checkTag(tag, ifdParser, IFD0_VALUE);
                         tagNumber++;
@@ -130,14 +132,14 @@ public class ExifParserTest extends ActivityTestCase {
                 case IfdParser.TYPE_NEXT_IFD:
                     parseIfd1(ifdParser.parseIfdBlock());
                     break;
-                case IfdParser.TYPE_SUB_IFD:
-                    assertEquals(ExifTag.TIFF_TAG.TAG_EXIF_IFD,
-                            ifdParser.getCorrespodingExifTag().getTagId());
-                    parseExifIfd(ifdParser.parseIfdBlock());
-                    break;
                 case IfdParser.TYPE_VALUE_OF_PREV_TAG:
-                    checkTag(ifdParser.getCorrespodingExifTag(), ifdParser, IFD0_VALUE);
-                    tagNumber++;
+                    tag = ifdParser.getCorrespodingExifTag();
+                    if(tag.getTagId() == ExifTag.TIFF_TAG.TAG_EXIF_IFD) {
+                        parseExifIfd(ifdParser.parseIfdBlock());
+                    } else {
+                        checkTag(ifdParser.getCorrespodingExifTag(), ifdParser, IFD0_VALUE);
+                        tagNumber++;
+                    }
                     break;
             }
             type = ifdParser.next();
@@ -153,8 +155,10 @@ public class ExifParserTest extends ActivityTestCase {
             switch (type) {
                 case IfdParser.TYPE_NEW_TAG:
                     ExifTag tag = ifdParser.readTag();
-                    if (tag.getOffset() > 0) {
-                        ifdParser.waitValueOfTag(tag);
+                    if (tag.getDataSize() > 4) {
+                        long offset = ifdParser.readUnsignedInt();
+                        assertTrue(offset <= Integer.MAX_VALUE);
+                        ifdParser.waitValueOfTag(tag, offset);
                     } else {
                         checkTag(tag, ifdParser, IFD1_VALUE);
                         tagNumber++;
@@ -163,9 +167,6 @@ public class ExifParserTest extends ActivityTestCase {
                 case IfdParser.TYPE_NEXT_IFD:
                     fail("Find a ifd after ifd1");
                     break;
-                case IfdParser.TYPE_SUB_IFD:
-                    fail("Find a sub ifd in ifd1");
-                    break;
                 case IfdParser.TYPE_VALUE_OF_PREV_TAG:
                     checkTag(ifdParser.getCorrespodingExifTag(), ifdParser, IFD1_VALUE);
                     tagNumber++;
@@ -184,8 +185,10 @@ public class ExifParserTest extends ActivityTestCase {
             switch (type) {
                 case IfdParser.TYPE_NEW_TAG:
                     ExifTag tag = ifdParser.readTag();
-                    if (tag.getOffset() > 0) {
-                        ifdParser.waitValueOfTag(tag);
+                    if (tag.getDataSize() > 4) {
+                        long offset = ifdParser.readUnsignedInt();
+                        assertTrue(offset <= Integer.MAX_VALUE);
+                        ifdParser.waitValueOfTag(tag, offset);
                     } else {
                         checkTag(tag, ifdParser, EXIF_IFD_VALUE);
                         tagNumber++;
@@ -194,9 +197,6 @@ public class ExifParserTest extends ActivityTestCase {
                 case IfdParser.TYPE_NEXT_IFD:
                     fail("Find a ifd after exif ifd");
                     break;
-                case IfdParser.TYPE_SUB_IFD:
-                    fail("Find a sub ifd in exif ifd");
-                    break;
                 case IfdParser.TYPE_VALUE_OF_PREV_TAG:
                     checkTag(ifdParser.getCorrespodingExifTag(), ifdParser, EXIF_IFD_VALUE);
                     tagNumber++;
@@ -282,10 +282,6 @@ public class ExifParserTest extends ActivityTestCase {
                 case IfdParser.TYPE_NEXT_IFD:
                     parseIfd1(ifdParser.parseIfdBlock());
                     break;
-                case IfdParser.TYPE_SUB_IFD:
-                    // We won't get this since to skip everything
-                    fail("Get sub ifd but we've skip everything");
-                    break;
                 case IfdParser.TYPE_VALUE_OF_PREV_TAG:
                     // We won't get this since to skip everything
                     fail("Get value of previous tag but we've skip everything");
@@ -304,23 +300,25 @@ public class ExifParserTest extends ActivityTestCase {
                 case IfdParser.TYPE_NEW_TAG:
                     ExifTag tag = ifdParser.readTag();
                     // only interested in these two tags
-                    if (tag.getOffset() > 0) {
+                    if (tag.getDataSize() > 4 || tag.getTagId() == ExifTag.TIFF_TAG.TAG_EXIF_IFD) {
                         if(tag.getTagId() == ExifTag.TIFF_TAG.TAG_MODEL
                                 || tag.getTagId() == ExifTag.TIFF_TAG.TAG_EXIF_IFD) {
-                            ifdParser.waitValueOfTag(tag);
+                            long offset = ifdParser.readUnsignedInt();
+                            assertTrue(offset <= Integer.MAX_VALUE);
+                            ifdParser.waitValueOfTag(tag, offset);
                         }
                     }
                     break;
                 case IfdParser.TYPE_NEXT_IFD:
                     parseIfd1(ifdParser.parseIfdBlock());
                     break;
-                case IfdParser.TYPE_SUB_IFD:
-                    assertEquals(ExifTag.TIFF_TAG.TAG_EXIF_IFD,
-                            ifdParser.getCorrespodingExifTag().getTagId());
-                    parseExifIfd(ifdParser.parseIfdBlock());
-                    break;
                 case IfdParser.TYPE_VALUE_OF_PREV_TAG:
-                    checkTag(ifdParser.getCorrespodingExifTag(), ifdParser, IFD0_VALUE);
+                    tag = ifdParser.getCorrespodingExifTag();
+                    if(tag.getTagId() == ExifTag.TIFF_TAG.TAG_EXIF_IFD) {
+                        parseExifIfd(ifdParser.parseIfdBlock());
+                    } else {
+                        checkTag(ifdParser.getCorrespodingExifTag(), ifdParser, IFD0_VALUE);
+                    }
                     break;
             }
             type = ifdParser.next();