Return GIFs with a single frame as Bitmaps.

This only affects the default decode path where
either a GIF or a Bitmap is an acceptable 
result. The GIF only path will still return 
GifDrawables for single frame GIFs.

Fixes #160.

library/src/androidTest/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java

@@ -246,7 +246,7 @@ public class GifDrawableTest {
     }
 
     @Test
-    public void testGetOpacityReturnsTransparentfDecoderHasTransparency() {
+    public void testGetOpacityReturnsTransparentIfDecoderHasTransparency() {
         when(gifDecoder.isTransparent()).thenReturn(true);
 
         assertEquals(PixelFormat.TRANSPARENT, drawable.getOpacity());
@@ -259,6 +259,14 @@ public class GifDrawableTest {
         assertEquals(PixelFormat.OPAQUE, drawable.getOpacity());
     }
 
+    @Test
+    public void testReturnsFrameCountFromDecoder() {
+        int expected = 4;
+        when(gifDecoder.getFrameCount()).thenReturn(expected);
+
+        assertEquals(expected, drawable.getFrameCount());
+    }
+
     @Test
     public void testRecycleCallsClearOnFrameManager() {
         drawable.recycle();

library/src/androidTest/java/com/bumptech/glide/load/resource/gifbitmap/GifBitmapResourceDecoderTest.java

-package com.bumptech.glide.load.resource.gifbitmap;
-
-import android.graphics.Bitmap;
-import android.os.ParcelFileDescriptor;
-
-import com.bumptech.glide.load.ResourceDecoder;
-import com.bumptech.glide.load.engine.Resource;
-import com.bumptech.glide.load.model.ImageVideoWrapper;
-import com.bumptech.glide.load.resource.gif.GifDrawable;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.robolectric.RobolectricTestRunner;
-
-import java.io.ByteArrayInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-
-import static org.junit.Assert.assertEquals;
-import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyInt;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-@RunWith(RobolectricTestRunner.class)
-public class GifBitmapResourceDecoderTest {
-    private ResourceDecoder<ImageVideoWrapper, Bitmap> bitmapDecoder;
-    private ResourceDecoder<InputStream, GifDrawable> gifDecoder;
-    private GifBitmapWrapperResourceDecoder decoder;
-
-    @SuppressWarnings("unchecked")
-    @Before
-    public void setUp() {
-        bitmapDecoder = mock(ResourceDecoder.class);
-        gifDecoder = mock(ResourceDecoder.class);
-        decoder = new GifBitmapWrapperResourceDecoder(bitmapDecoder, gifDecoder);
-    }
-
-    @Test
-    public void testDecoderUsesGifDecoderResultIfGif() throws IOException {
-        GifDrawable expected = mock(GifDrawable.class);
-        Resource<GifDrawable> gifDrawableResource = mock(Resource.class);
-        when(gifDrawableResource.get()).thenReturn(expected);
-        when(gifDecoder.decode(any(InputStream.class), anyInt(), anyInt())).thenReturn(gifDrawableResource);
-
-        byte[] data = new byte[] { 'G', 'I', 'F'};
-        ImageVideoWrapper wrapper = new ImageVideoWrapper(new ByteArrayInputStream(data), null);
-
-        Resource<GifBitmapWrapper> result = decoder.decode(wrapper, 100, 100);
-
-        assertEquals(expected, result.get().getGifResource().get());
-    }
-
-    @Test
-    public void testDecoderUsesBitmapDecoderIfStreamIsNotGif() throws IOException {
-        Bitmap expected = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888);
-        Resource<Bitmap> bitmapResource = mock(Resource.class);
-        when(bitmapResource.get()).thenReturn(expected);
-        when(bitmapDecoder.decode(any(ImageVideoWrapper.class), anyInt(), anyInt())).thenReturn(bitmapResource);
-
-        byte[] data = new byte[] { 'A', 'I', 'F'};
-        ImageVideoWrapper wrapper = new ImageVideoWrapper(new ByteArrayInputStream(data), null);
-
-        Resource<GifBitmapWrapper> result = decoder.decode(wrapper, 100, 100);
-
-        Bitmap bitmap = result.get().getBitmapResource().get();
-
-        assertEquals(expected, bitmap);
-    }
-
-    @Test
-    public void testDecoderUsesBitmapDecoderIfIsFileDescriptor() throws IOException {
-        Bitmap expected = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888);
-        Resource<Bitmap> bitmapResource = mock(Resource.class);
-        when(bitmapResource.get()).thenReturn(expected);
-        when(bitmapDecoder.decode(any(ImageVideoWrapper.class), anyInt(), anyInt())).thenReturn(bitmapResource);
-
-        ImageVideoWrapper wrapper = new ImageVideoWrapper(null, mock(ParcelFileDescriptor.class));
-
-        Resource<GifBitmapWrapper> result = decoder.decode(wrapper, 100, 100);
-
-        Bitmap bitmap = result.get().getBitmapResource().get();
-
-        assertEquals(expected, bitmap);
-    }
-}

library/src/androidTest/java/com/bumptech/glide/load/resource/gifbitmap/GifBitmapWrapperResourceDecoderTest.java

@@ -62,7 +62,7 @@ public class GifBitmapWrapperResourceDecoderTest {
         when(parser.parse(eq(bis))).thenReturn(ImageHeaderParser.ImageType.GIF);
         int width = 100;
         int height = 200;
-        Resource<GifDrawable> expected = mock(Resource.class);
+        Resource<GifDrawable> expected = mockGifResource();
 
         when(gifDecoder.decode(any(InputStream.class), eq(width), eq(height))).thenReturn(expected);
 
@@ -90,7 +90,7 @@ public class GifBitmapWrapperResourceDecoderTest {
         when(parser.parse(eq(bis))).thenReturn(ImageHeaderParser.ImageType.GIF);
         int width = 101;
         int height = 102;
-        Resource<GifDrawable> expected = mock(Resource.class);
+        Resource<GifDrawable> expected = mockGifResource();
         when(gifDecoder.decode(any(InputStream.class), eq(width), eq(height))).thenReturn(expected);
         when(bitmapDecoder.decode(any(ImageVideoWrapper.class), eq(width), eq(height)))
                 .thenReturn(mock(Resource.class));
@@ -132,6 +132,38 @@ public class GifBitmapWrapperResourceDecoderTest {
         assertEquals(expected, result.get().getBitmapResource());
     }
 
+    @Test
+    public void testReturnsBitmapWhenGifTypeButGifHasSingleFrame() throws IOException {
+        Resource<GifDrawable> gifResource = mockGifResource();
+        when(gifResource.get().getFrameCount()).thenReturn(1);
+
+        when(parser.parse(eq(bis))).thenReturn(ImageHeaderParser.ImageType.GIF);
+        when(gifDecoder.decode(any(InputStream.class), anyInt(), anyInt())).thenReturn(gifResource);
+
+        Resource<Bitmap> expected = mock(Resource.class);
+        when(bitmapDecoder.decode(any(ImageVideoWrapper.class), anyInt(), anyInt())).thenReturn(expected);
+
+        Resource<GifBitmapWrapper> result = decoder.decode(source, 100, 100);
+
+        assertEquals(expected, result.get().getBitmapResource());
+    }
+
+    @Test
+    public void testRecyclesGifResourceWhenGifTypeButGifHasSingleFrame() throws IOException {
+        Resource<GifDrawable> gifResource = mockGifResource();
+        when(gifResource.get().getFrameCount()).thenReturn(1);
+
+        when(parser.parse(eq(bis))).thenReturn(ImageHeaderParser.ImageType.GIF);
+        when(gifDecoder.decode(any(InputStream.class), anyInt(), anyInt())).thenReturn(gifResource);
+
+        Resource<Bitmap> expected = mock(Resource.class);
+        when(bitmapDecoder.decode(any(ImageVideoWrapper.class), anyInt(), anyInt())).thenReturn(expected);
+
+        decoder.decode(source, 100, 100);
+
+        verify(gifResource).recycle();
+    }
+
     @Test
     public void testDoesNotTryToParseTypeOrDecodeNullStream() throws IOException {
         when(source.getFileDescriptor()).thenReturn(mock(ParcelFileDescriptor.class));
@@ -171,4 +203,14 @@ public class GifBitmapWrapperResourceDecoderTest {
         assertThat(id, containsString(bitmapId));
         assertThat(id, containsString(gifId));
     }
+
+    @SuppressWarnings("unchecked")
+    private static Resource<GifDrawable> mockGifResource() {
+        GifDrawable drawable = mock(GifDrawable.class);
+        // Something > 1.
+        when(drawable.getFrameCount()).thenReturn(4);
+        Resource<GifDrawable> resource = mock(Resource.class);
+        when(resource.get()).thenReturn(drawable);
+        return resource;
+    }
 }

library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java

@@ -106,6 +106,10 @@ public class GifDrawable extends GlideDrawable implements GifFrameManager.FrameC
         return state.data;
     }
 
+    public int getFrameCount() {
+        return decoder.getFrameCount();
+    }
+
     private void resetLoopCount() {
         loopCount = 0;
     }

library/src/main/java/com/bumptech/glide/load/resource/gifbitmap/GifBitmapWrapperResourceDecoder.java

@@ -17,8 +17,8 @@ import java.io.InputStream;
  * from an {@link InputStream} or a {@link android.os.ParcelFileDescriptor ParcelFileDescriptor}.
  */
 public class GifBitmapWrapperResourceDecoder implements ResourceDecoder<ImageVideoWrapper, GifBitmapWrapper> {
-    private static final ImageTypeParser DEFAULT_PARSER = new DefaultImageTypeParser();
-    private static final BufferedStreamFactory DEFAULT_STREAM_FACTORY = new DefaultBufferedStreamFactory();
+    private static final ImageTypeParser DEFAULT_PARSER = new ImageTypeParser();
+    private static final BufferedStreamFactory DEFAULT_STREAM_FACTORY = new BufferedStreamFactory();
     // 2048 is rather arbitrary, for most well formatted image types we only need 32 bytes.
     // Visible for testing.
     static final int MARK_LIMIT_BYTES = 2048;
@@ -70,27 +70,53 @@ public class GifBitmapWrapperResourceDecoder implements ResourceDecoder<ImageVid
             bis.reset();
 
             if (type == ImageHeaderParser.ImageType.GIF) {
-                Resource<GifDrawable> gifResource = gifDecoder.decode(bis, width, height);
-                if (gifResource != null) {
-                    result = new GifBitmapWrapper(null, gifResource);
-                }
+                result = decodeGifWrapper(bis, width, height);
             }
         }
 
         if (result == null) {
-            // We can only reset the buffered InputStream, so to start from the beginning of the stream, we need to pass
-            // in a new source containing the buffered stream rather than the original stream.
+             // We can only reset the buffered InputStream, so to start from the beginning of the stream, we need to
+             // pass in a new source containing the buffered stream rather than the original stream.
             final ImageVideoWrapper wrapperToDecode;
             if (bis != null) {
                 wrapperToDecode = new ImageVideoWrapper(bis, source.getFileDescriptor());
             } else {
                 wrapperToDecode = source;
             }
-            Resource<Bitmap> bitmapResource = bitmapDecoder.decode(wrapperToDecode, width, height);
-            if (bitmapResource != null) {
-                result = new GifBitmapWrapper(bitmapResource, null);
+
+            result = decodeBitmapWrapper(wrapperToDecode, width, height);
+        }
+
+        return result;
+    }
+
+    private GifBitmapWrapper decodeGifWrapper(InputStream bis, int width, int height) throws IOException {
+        GifBitmapWrapper result = null;
+        Resource<GifDrawable> gifResource = gifDecoder.decode(bis, width, height);
+        if (gifResource != null) {
+            GifDrawable drawable = gifResource.get();
+            // We can more efficiently hold Bitmaps in memory, so for static GIFs, try to return Bitmaps
+            // instead. Returning a Bitmap incurs the cost of allocating the GifDrawable as well as the normal
+            // Bitmap allocation, but since we can encode the Bitmap out as a JPEG, future decodes will be
+            // efficient.
+            // TODO: fix the degenerate case where we cache only the source and so are constantly decoding both a GIF
+            // and a Bitmap.
+            if (drawable.getFrameCount() > 1) {
+                result = new GifBitmapWrapper(null, gifResource);
+            } else {
+                gifResource.recycle();
             }
         }
+        return result;
+    }
+
+    private GifBitmapWrapper decodeBitmapWrapper(ImageVideoWrapper toDecode, int width, int height) throws IOException {
+        GifBitmapWrapper result = null;
+
+        Resource<Bitmap> bitmapResource = bitmapDecoder.decode(toDecode, width, height);
+        if (bitmapResource != null) {
+            result = new GifBitmapWrapper(bitmapResource, null);
+        }
 
         return result;
     }
@@ -103,23 +129,15 @@ public class GifBitmapWrapperResourceDecoder implements ResourceDecoder<ImageVid
         return id;
     }
 
-    interface BufferedStreamFactory {
-        public InputStream build(InputStream is, byte[] buffer);
-    }
-
-    private static class DefaultBufferedStreamFactory implements BufferedStreamFactory {
-        @Override
+    // Visible for testing.
+    static class BufferedStreamFactory {
         public InputStream build(InputStream is, byte[] buffer) {
             return new RecyclableBufferedInputStream(is, buffer);
         }
     }
 
-    interface ImageTypeParser {
-        public ImageHeaderParser.ImageType parse(InputStream is) throws IOException;
-    }
-
-    private static class DefaultImageTypeParser implements ImageTypeParser {
-        @Override
+    // Visible for testing.
+    static class ImageTypeParser {
         public ImageHeaderParser.ImageType parse(InputStream is) throws IOException {
             return new ImageHeaderParser(is).getType();
         }