Decode GIFs with more codes than can fit in table.

Fixes #203

7 files changed, 180 insertions, 74 deletions

diff --git a/third_party/gif_decoder/build.gradle b/third_party/gif_decoder/build.gradle
index f7b4d4ab..8834279f 100644
--- a/third_party/gif_decoder/build.gradle
+++ b/third_party/gif_decoder/build.gradle
@@ -2,10 +2,12 @@ apply plugin: 'com.android.library'
 apply plugin: 'robolectric'
 
 dependencies {
+    androidTestCompile 'com.android.support:support-v4:19.1.0'
     androidTestCompile 'org.hamcrest:hamcrest-core:1.3'
     androidTestCompile 'org.hamcrest:hamcrest-library:1.3'
     androidTestCompile 'junit:junit:4.11'
     androidTestCompile 'org.mockito:mockito-all:1.9.5'
+    androidTestCompile 'org.robolectric:robolectric:2.4-SNAPSHOT'
 }
 
 android {
diff --git a/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/GifDecoderTest.java b/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/GifDecoderTest.java
new file mode 100644
index 00000000..aaf65558
--- /dev/null
+++ b/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/GifDecoderTest.java
@@ -0,0 +1,51 @@
+package com.bumptech.glide.gifdecoder;
+
+import android.graphics.Bitmap;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.Robolectric;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * Tests for {@link com.bumptech.glide.gifdecoder.GifDecoder}.
+ */
+@RunWith(RobolectricTestRunner.class)
+@Config(emulateSdk = 18)
+public class GifDecoderTest {
+
+    private MockProvider provider;
+
+    @Before
+    public void setUp() {
+        provider = new MockProvider();
+    }
+
+    @Test
+    public void testCanDecodeFramesFromTestGif() {
+        byte[] data = TestUtil.readResourceData("partial_gif_decode.gif");
+        GifHeaderParser headerParser = new GifHeaderParser();
+        headerParser.setData(data);
+        GifHeader header = headerParser.parseHeader();
+        GifDecoder decoder = new GifDecoder(provider);
+        decoder.setData(header, data);
+        decoder.advance();
+        Bitmap bitmap = decoder.getNextFrame();
+        assertNotNull(bitmap);
+        assertEquals(GifDecoder.STATUS_OK, decoder.getStatus());
+    }
+
+    private static class MockProvider implements GifDecoder.BitmapProvider {
+
+        @Override
+        public Bitmap obtain(int width, int height, Bitmap.Config config) {
+            Bitmap result = Bitmap.createBitmap(width, height, config);
+            Robolectric.shadowOf(result).setMutable(true);
+            return result;
+        }
+    }
+}
diff --git a/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/GifHeaderParserTest.java b/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/GifHeaderParserTest.java
index 3433153e..0cd0a7af 100644
--- a/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/GifHeaderParserTest.java
+++ b/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/GifHeaderParserTest.java
@@ -4,9 +4,7 @@ import com.bumptech.glide.gifdecoder.test.GifBytesTestUtil;
 import org.junit.Before;
 import org.junit.Test;
 
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 
@@ -53,7 +51,7 @@ public class GifHeaderParserTest {
 
     @Test
     public void testCanParseHeaderOfTestImageWithoutGraphicalExtension() throws IOException {
-        byte[] data = readResourceData("gif_without_graphical_control_extension.gif");
+        byte[] data = TestUtil.readResourceData("gif_without_graphical_control_extension.gif");
         parser.setData(data);
         GifHeader header = parser.parseHeader();
         assertEquals(1, header.frameCount);
@@ -151,31 +149,9 @@ public class GifHeaderParserTest {
         assertEquals(expectedFrames, header.frames.size());
     }
 
-    private InputStream openResource(String imageName) throws IOException {
-        return getClass().getResourceAsStream("/" + imageName);
-    }
-
-    private byte[] readResourceData(String imageName) {
-        ByteArrayOutputStream os = new ByteArrayOutputStream();
-        byte[] buffer = new byte[1024];
-        InputStream is = null;
-        try {
-            is = openResource(imageName);
-            int read;
-            while ((read = is.read(buffer)) != -1) {
-                os.write(buffer, 0, read);
-            }
-        } catch (IOException e) {
-            throw new RuntimeException(e);
-        } finally {
-            if (is != null) {
-                try {
-                    is.close();
-                } catch (IOException e) {
-                    // Ignore.
-                }
-            }
-        }
-        return os.toByteArray();
+    @Test(expected = IllegalStateException.class)
+    public void testThrowsIfParseHeaderCalledBeforeSetData() {
+        GifHeaderParser parser = new GifHeaderParser();
+        parser.parseHeader();
     }
 }
\ No newline at end of file
diff --git a/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/TestUtil.java b/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/TestUtil.java
new file mode 100644
index 00000000..ddacb68c
--- /dev/null
+++ b/third_party/gif_decoder/src/androidTest/java/com/bumptech/glide/gifdecoder/TestUtil.java
@@ -0,0 +1,40 @@
+package com.bumptech.glide.gifdecoder;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+final class TestUtil {
+
+    private TestUtil() {
+        // Utility class.
+    }
+
+    private static InputStream openResource(String imageName) throws IOException {
+        return TestUtil.class.getResourceAsStream("/" + imageName);
+    }
+
+    public static byte[] readResourceData(String imageName) {
+        ByteArrayOutputStream os = new ByteArrayOutputStream();
+        byte[] buffer = new byte[1024];
+        InputStream is = null;
+        try {
+            is = openResource(imageName);
+            int read;
+            while ((read = is.read(buffer)) != -1) {
+                os.write(buffer, 0, read);
+            }
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        } finally {
+            if (is != null) {
+                try {
+                    is.close();
+                } catch (IOException e) {
+                    // Ignore.
+                }
+            }
+        }
+        return os.toByteArray();
+    }
+}
diff --git a/third_party/gif_decoder/src/androidTest/resources/partial_gif_decode.gif b/third_party/gif_decoder/src/androidTest/resources/partial_gif_decode.gif
new file mode 100644
index 00000000..2e3b6f27
--- /dev/null
+++ b/third_party/gif_decoder/src/androidTest/resources/partial_gif_decode.gif
diff --git a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java
index e1f05455..4023eb14 100644
--- a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java
+++ b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifDecoder.java
@@ -69,6 +69,10 @@ public class GifDecoder {
      */
     public static final int STATUS_OPEN_ERROR = 2;
     /**
+     * Unable to fully decode the current frame.
+     */
+    public static final int STATUS_PARTIAL_DECODE = 3;
+    /**
      * max decoder pixel stack size.
      */
     private static final int MAX_STACK_SIZE = 4096;
@@ -90,6 +94,8 @@ public class GifDecoder {
      */
     private static final int DISPOSAL_PREVIOUS = 3;
 
+    private static final int NULL_CODE = -1;
+
     // Global File Header values and parsing flags.
     // Active color table.
     private int[] act;
@@ -115,6 +121,7 @@ public class GifDecoder {
     private Bitmap previousImage;
     private boolean savePrevious;
     private Bitmap.Config config;
+    private int status = STATUS_OK;
 
     /**
      * An interface that can be used to provide reused {@link android.graphics.Bitmap}s to avoid GCs from constantly
@@ -154,6 +161,18 @@ public class GifDecoder {
     }
 
     /**
+     * Returns the current status of the decoder.
+     *
+     * <p>
+     *     Status will update per frame to allow the caller to tell whether or not the current frame was decoded
+     *     successfully and/or completely. Format and open failures persist across frames.
+     * </p>
+     */
+    public int getStatus() {
+        return status;
+    }
+
+    /**
      * Move the animation frame counter forward.
      */
     public void advance() {
@@ -223,8 +242,12 @@ public class GifDecoder {
      */
     public Bitmap getNextFrame() {
         if (header.frameCount <= 0 || framePointer < 0) {
+            status = STATUS_FORMAT_ERROR;
+        }
+        if (status == STATUS_FORMAT_ERROR || status == STATUS_OPEN_ERROR) {
             return null;
         }
+        status = STATUS_OK;
 
         GifFrame frame = header.frames.get(framePointer);
 
@@ -247,7 +270,7 @@ public class GifDecoder {
         if (act == null) {
             Log.w(TAG, "No Valid Color Table");
             // No color table defined.
-            header.status = STATUS_FORMAT_ERROR;
+            status = STATUS_FORMAT_ERROR;
             return null;
         }
 
@@ -285,7 +308,7 @@ public class GifDecoder {
                 Log.w(TAG, "Error reading data from stream", e);
             }
         } else {
-            header.status = STATUS_OPEN_ERROR;
+            status = STATUS_OPEN_ERROR;
         }
 
         try {
@@ -296,7 +319,7 @@ public class GifDecoder {
             Log.w(TAG, "Error closing stream", e);
         }
 
-        return header.status;
+        return status;
     }
 
     public void clear() {
@@ -315,6 +338,7 @@ public class GifDecoder {
         this.id = id;
         this.header = header;
         this.data = data;
+        this.status = STATUS_OK;
         // Initialize the raw data buffer.
         rawData = ByteBuffer.wrap(data);
         rawData.rewind();
@@ -364,7 +388,7 @@ public class GifDecoder {
             }
         }
 
-        return header.status;
+        return status;
     }
 
     /**
@@ -479,7 +503,6 @@ public class GifDecoder {
             rawData.position(frame.bufferFrameStart);
         }
 
-        int nullCode = -1;
         int npix = (frame == null) ? header.width * header.height : frame.iw * frame.ih;
         int available, clear, codeMask, codeSize, endOfInformation, inCode, oldCode, bits, code, count, i, datum,
                 dataSize, first, top, bi, pi;
@@ -503,7 +526,7 @@ public class GifDecoder {
         clear = 1 << dataSize;
         endOfInformation = clear + 1;
         available = clear + 2;
-        oldCode = nullCode;
+        oldCode = NULL_CODE;
         codeSize = dataSize + 1;
         codeMask = (1 << codeSize) - 1;
         for (code = 0; code < clear; code++) {
@@ -515,73 +538,84 @@ public class GifDecoder {
         // Decode GIF pixel stream.
         datum = bits = count = first = top = pi = bi = 0;
         for (i = 0; i < npix; ) {
-            if (top == 0) {
-                if (bits < codeSize) {
-                    // Load bytes until there are enough bits for a code.
-                    if (count == 0) {
-                        // Read a new data block.
-                        count = readBlock();
-                        if (count <= 0) {
-                            break;
-                        }
-                        bi = 0;
-                    }
-                    datum += (((int) block[bi]) & 0xff) << bits;
-                    bits += 8;
-                    bi++;
-                    count--;
-                    continue;
+            // Load bytes until there are enough bits for a code.
+            if (count == 0) {
+                // Read a new data block.
+                count = readBlock();
+                if (count <= 0) {
+                    status = STATUS_PARTIAL_DECODE;
+                    break;
                 }
+                bi = 0;
+            }
+
+            datum += (((int) block[bi]) & 0xff) << bits;
+            bits += 8;
+            bi++;
+            count--;
+
+            while (bits >= codeSize) {
                 // Get the next code.
                 code = datum & codeMask;
                 datum >>= codeSize;
                 bits -= codeSize;
+
                 // Interpret the code.
-                if ((code > available) || (code == endOfInformation)) {
-                    break;
-                }
                 if (code == clear) {
                     // Reset decoder.
                     codeSize = dataSize + 1;
                     codeMask = (1 << codeSize) - 1;
                     available = clear + 2;
-                    oldCode = nullCode;
+                    oldCode = NULL_CODE;
                     continue;
                 }
-                if (oldCode == nullCode) {
+
+                if (code > available) {
+                    status = STATUS_PARTIAL_DECODE;
+                    break;
+                }
+
+                if (code == endOfInformation) {
+                    break;
+                }
+
+                if (oldCode == NULL_CODE) {
                     pixelStack[top++] = suffix[code];
                     oldCode = code;
                     first = code;
                     continue;
                 }
                 inCode = code;
-                if (code == available) {
+                if (code >= available) {
                     pixelStack[top++] = (byte) first;
                     code = oldCode;
                 }
-                while (code > clear) {
+                while (code >= clear) {
                     pixelStack[top++] = suffix[code];
                     code = prefix[code];
                 }
                 first = ((int) suffix[code]) & 0xff;
-                // Add a new string to the string table.
-                if (available >= MAX_STACK_SIZE) {
-                    break;
-                }
                 pixelStack[top++] = (byte) first;
-                prefix[available] = (short) oldCode;
-                suffix[available] = (byte) first;
-                available++;
-                if (((available & codeMask) == 0) && (available < MAX_STACK_SIZE)) {
-                    codeSize++;
-                    codeMask += available;
+
+                // Add a new string to the string table.
+                if (available < MAX_STACK_SIZE) {
+                    prefix[available] = (short) oldCode;
+                    suffix[available] = (byte) first;
+                    available++;
+                    if (((available & codeMask) == 0) && (available < MAX_STACK_SIZE)) {
+                        codeSize++;
+                        codeMask += available;
+                    }
                 }
                 oldCode = inCode;
+
+                while (top > 0) {
+                    // Pop a pixel off the pixel stack.
+                    top--;
+                    mainPixels[pi++] = pixelStack[top];
+                    i++;
+                }
             }
-            // Pop a pixel off the pixel stack.
-            top--;
-            mainPixels[pi++] = pixelStack[top];
-            i++;
         }
 
         // Clear missing pixels.
@@ -598,7 +632,7 @@ public class GifDecoder {
         try {
             curByte = rawData.get() & 0xFF;
         } catch (Exception e) {
-            header.status = STATUS_FORMAT_ERROR;
+            status = STATUS_FORMAT_ERROR;
         }
         return curByte;
     }
@@ -622,7 +656,7 @@ public class GifDecoder {
                 }
             } catch (Exception e) {
                 Log.w(TAG, "Error Reading Block", e);
-                header.status = STATUS_FORMAT_ERROR;
+                status = STATUS_FORMAT_ERROR;
             }
         }
         return n;
diff --git a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeaderParser.java b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeaderParser.java
index a74bb280..286a5602 100644
--- a/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeaderParser.java
+++ b/third_party/gif_decoder/src/main/java/com/bumptech/glide/gifdecoder/GifHeaderParser.java
@@ -45,6 +45,9 @@ public class GifHeaderParser {
     }
 
     public GifHeader parseHeader() {
+        if (rawData == null) {
+            throw new IllegalStateException("You must call setData() before parseHeader()");
+        }
         if (err()) {
             return header;
         }