Creating a new wrapper for gif decoder

src/codec/SkCodec_libgif.cpp

+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkCodec_libgif.h"
+#include "SkCodecPriv.h"
+#include "SkColorPriv.h"
+#include "SkColorTable.h"
+#include "SkGifInterlaceIter.h"
+#include "SkStream.h"
+#include "SkSwizzler.h"
+#include "SkUtils.h"
+
+/*
+ * Checks the start of the stream to see if the image is a gif
+ */
+bool SkGifCodec::IsGif(SkStream* stream) {
+    char buf[GIF_STAMP_LEN];
+    if (stream->read(buf, GIF_STAMP_LEN) == GIF_STAMP_LEN) {
+        if (memcmp(GIF_STAMP,   buf, GIF_STAMP_LEN) == 0 ||
+                memcmp(GIF87_STAMP, buf, GIF_STAMP_LEN) == 0 ||
+                memcmp(GIF89_STAMP, buf, GIF_STAMP_LEN) == 0) {
+            return true;
+        }
+    }
+    return false;
+}
+
+/*
+ * Warning reporting function
+ */
+static void gif_warning(const char* msg) {
+    SkCodecPrintf("Gif Warning: %s\n", msg);
+}
+
+/*
+ * Error function
+ */
+static SkCodec::Result gif_error(const char* msg,
+        SkCodec::Result result = SkCodec::kInvalidInput) {
+    SkCodecPrintf("Gif Error: %s\n", msg);
+    return result;
+}
+
+
+/*
+ * Read function that will be passed to gif_lib
+ */
+static int32_t read_bytes_callback(GifFileType* fileType, GifByteType* out,
+        int32_t size) {
+    SkStream* stream = (SkStream*) fileType->UserData;
+    return (int32_t) stream->read(out, size);
+}
+
+/*
+ * Open the gif file
+ */
+static GifFileType* open_gif(SkStream* stream) {
+#if GIFLIB_MAJOR < 5
+    return DGifOpen(stream, read_bytes_callback);
+#else
+    return DGifOpen(stream, read_bytes_callback, NULL);
+#endif
+}
+
+ /*
+ * This function cleans up the gif object after the decode completes
+ * It is used in a SkAutoTCallIProc template
+ */
+int32_t SkGifCodec::CloseGif(GifFileType* gif) {
+#if GIFLIB_MAJOR < 5 || (GIFLIB_MAJOR == 5 && GIFLIB_MINOR == 0)
+    return DGifCloseFile(gif);
+#else
+    return DGifCloseFile(gif, NULL);
+#endif
+}
+
+/*
+ * This function free extension data that has been saved to assist the image
+ * decoder
+ */
+void SkGifCodec::FreeExtension(SavedImage* image) {
+    if (NULL != image->ExtensionBlocks) {
+#if GIFLIB_MAJOR < 5
+        FreeExtension(image);
+#else
+        GifFreeExtensions(&image->ExtensionBlockCount, &image->ExtensionBlocks);
+#endif
+    }
+}
+
+/*
+ * Check if a there is an index of the color table for a transparent pixel
+ */
+static int32_t find_trans_index(const SavedImage& image, uint32_t colorCount) {
+    // If there is a transparent index specified, it will be contained in an
+    // extension block.
+    for (int32_t i = 0; i < image.ExtensionBlockCount; i++) {
+        // Get an extension block
+        const ExtensionBlock& extBlock = image.ExtensionBlocks[i];
+
+        // Specifically, we need to check for a graphics control extension,
+        // which may contain transparency information.  Also, note that a valid
+        // graphics control extension is always four bytes.  The fourth byte
+        // is the transparent index (if it exists), so we need at least four
+        // bytes.
+        if (GRAPHICS_EXT_FUNC_CODE == extBlock.Function &&
+                extBlock.ByteCount >= 4) {
+
+            // Check the transparent color flag which indicates whether a
+            // transparent index exists.  It is the least significant bit of
+            // the first byte of the extension block.
+            if (1 == (extBlock.Bytes[0] & 1)) {
+
+                // Use uint32_t to prevent sign extending
+                uint32_t transIndex = extBlock.Bytes[3];
+                if (transIndex < colorCount) {
+                    return transIndex;
+                }
+            }

[scroggo, 2015/03/31 18:39:56]

nit: Could there be multiple graphics control extensions? Maybe we should break
after the first one if there was no valid transIndex?

I'm guessing there won't be large numbers of ExtensionBlocks, so it's probably
not a big deal.

[msarett, 2015/03/31 20:11:40]

There can be multiple GCEs because there can be multiple image frames in a gif.

The spec states that the scope of the GCE is the next image frame in the file,
and there can only be one GCE preceding an image frame.  Since the current gif
decoder only ever decodes one image frame, you are correct that we should only
have to look for one GCE.

So to finally answer the question, yes I think there should be a break.  I am
also going to reverse the loop so it starts by checking the most recent
extension it has encountered (This shouldn't impact valid images since there
should only be one GCE, but it's my guess that in weird cases it may be best to
go with the GCE closest to the image data).  I imagine that neither of these
changes will cause a change in behavior of any of the test images (but it's
worth monitoring).

Also, it's worth noting that both of these changes may cause the new decoder to
have different behavior than SkImageDecoder.  This function was adapted from
SkImageDecoder, and this is the first time that I have thought through it
properly.

[scroggo, 2015/03/31 20:39:11]

I wouldn't worry about reversing this. Since you're just guessing, we may be
better off as is. That said, since you've already changed it, this way seems
just as well to me.
Lots of our SkCodec behavior is different from SkImageDecoder, and hopefully
better!

+        }
+    }
+
+    // Flag indicating that a valid index was not found
+    return -1;
+}
+
+/*
+ * Assumes IsGif was called and returned true
+ * Creates a gif decoder
+ * Reads enough of the stream to determine the image format
+ */
+SkCodec* SkGifCodec::NewFromStream(SkStream* stream) {
+    // Read gif header, logical screen descriptor, and global color table
+    SkAutoTCallIProc<GifFileType, CloseGif> gif(open_gif(stream));
+
+    if (NULL == gif) {
+        gif_error("DGifOpen failed.\n");
+        return NULL;
+    }
+
+    // Get fields from header
+    const int32_t width = gif->SWidth;
+    const int32_t height = gif->SHeight;
+    if (width <= 0 || height <= 0) {
+        gif_error("Invalid dimensions.\n");
+        return NULL;
+    }
+
+    // Return the codec
+    // kIndex is the most natural color type for gifs, so we set this as
+    // the default.
+    // Many gifs specify a color table index for transparent pixels.  Every
+    // other pixel is guaranteed to be opaque.  Despite this, because of the
+    // possiblity of transparent pixels, we cannot assume that the image is
+    // opaque.  However, we can at least mark it as kPremul, since pixels will

[scroggo, 2015/03/31 18:39:56]

Maybe a note that it could be treated as unpremul, but premul is better since
that's what we support (and it would be more efficient even when unpremul is
supported)?

[msarett, 2015/03/31 20:11:41]

Agreed

+    // either have a 0xFF alpha component or be completely zeroed.
+    const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
+            kIndex_8_SkColorType, kPremul_SkAlphaType);
+    return SkNEW_ARGS(SkGifCodec, (imageInfo, stream, gif.detach()));
+}
+
+SkGifCodec::SkGifCodec(const SkImageInfo& srcInfo, SkStream* stream,
+                       GifFileType* gif)
+    : INHERITED(srcInfo, stream)
+    , fGif(gif)
+{}
+
+/*
+ * Checks if the conversion between the input image and the requested output
+ * image has been implemented
+ */
+static bool conversion_possible(const SkImageInfo& dst,
+                                const SkImageInfo& src) {
+    // Ensure that the profile type is unchanged
+    if (dst.profileType() != src.profileType()) {
+        return false;
+    }
+
+    // Check for supported color and alpha types
+    switch (dst.colorType()) {
+        case kN32_SkColorType:
+            return src.alphaType() == dst.alphaType() ||
+                    (kPremul_SkAlphaType == dst.alphaType() &&
+                    kUnpremul_SkAlphaType == src.alphaType());

[scroggo, 2015/03/31 18:39:57]

The source will always be premul, right? And it could easily be treated as
unpremul. I think we really want to say:

return dst.alphaType() == kPremul_SkAlphaType
  || dst.alphaType() == kUnpremul_SkAlphaType;

It seems like it would be nice to support kOpaque, if there is no transparent
color, but it seems like we cannot know that yet?

[msarett, 2015/03/31 20:11:40]

I agree and fixed the return statement.

We discussed supporting kOpaque in person.  It's doable, but low priority.

+        default:
+            return false;
+    }
+}
+
+/*
+ * Initiates the gif decode
+ */
+SkCodec::Result SkGifCodec::onGetPixels(const SkImageInfo& dstInfo,
+                                        void* dst, size_t dstRowBytes,
+                                        const Options& opts, SkPMColor*, int*) {
+    // Check for valid input parameters
+    if (!this->rewindIfNeeded()) {
+        return kCouldNotRewind;
+    }
+    if (dstInfo.dimensions() != this->getInfo().dimensions()) {
+        return gif_error("Scaling not supported.\n", kInvalidScale);
+    }
+    if (!conversion_possible(dstInfo, this->getInfo())) {
+        return gif_error("Cannot convert input type to output type.\n",
+                kInvalidConversion);
+    }
+
+    // Use this as a container to hold information about any gif extension
+    // blocks.  This generally stores transparency and animation instructions.
+    SavedImage saveExt;
+    SkAutoTCallVProc<SavedImage, FreeExtension> autoFreeExt(&saveExt);
+    saveExt.ExtensionBlocks = NULL;
+    saveExt.ExtensionBlockCount = 0;
+    GifByteType* extData;
+#if GIFLIB_MAJOR >= 5
+    int32_t extFunction;
+#endif
+
+    // We will loop over components of gif images until we find an image.  Once
+    // we find an image, we will decode and return it.  While many gif files
+    // contain more than one image, we will simply decode the first image.
+    const int32_t width = dstInfo.width();
+    const int32_t height = dstInfo.height();
+    GifRecordType recordType = UNDEFINED_RECORD_TYPE;
+    while (TERMINATE_RECORD_TYPE != recordType) {

[scroggo, 2015/03/31 18:39:56]

Is it possible that a file never contains TERMINATE_RECORD_TYPE? I guess in that
case we'll fail on some call to DGifGetRecordType down below? 

Unless, of course, it's a valid image, in which case we return as soon as we
have the image.

[msarett, 2015/03/31 20:11:40]

Yes this will always find a way to fail inside or outside the loop.  If we hit
a:
terminate record type - we exit the loop and fail
extension record type - we process it and keep looping
image record type - we decode it and return
invalid record type - dgiflib returns an error (which we catch)

The default case should never be reached (which I have just made clear).

+        // Get the current record type
+        if (GIF_ERROR == DGifGetRecordType(fGif, &recordType)) {
+            return gif_error("DGifGetRecordType failed.\n", kInvalidInput);
+        }
+
+        switch (recordType) {
+            case IMAGE_DESC_RECORD_TYPE: {
+                // Read the image descriptor
+                if (GIF_ERROR == DGifGetImageDesc(fGif)) {
+                    return gif_error("DGifGetImageDesc failed.\n",
+                            kInvalidInput);
+                }
+
+                // If reading the image descriptor is successful, the image
+                // count will be incremented
+                SkASSERT(fGif->ImageCount >= 1);
+                SavedImage* image = &fGif->SavedImages[fGif->ImageCount - 1];

[scroggo, 2015/03/31 18:39:56]

It looks like we're using the last image? Why not the first? (Or are they stored
backwards, so this *is* the first?)

[msarett, 2015/03/31 20:11:40]

We are always using and returning the first image.  So every time this code is
executed ImageCount will be 1 and we will be accessing SavedImages[0].  We need
to access SavedImages[0] because giflib uses it to store information from the
image descriptor such as width, height, etc.

I think the confusion is from unnecessary future proofing.  If we were ever to
want to decode the next image in the file, ImageCount would be 2 and we would
use SavedImages[1].

+
+                // Process the descriptor
+                const GifImageDesc& desc = image->ImageDesc;
+                int32_t imageLeft = desc.Left;
+                int32_t imageTop = desc.Top;
+                int32_t innerWidth = desc.Width;
+                int32_t innerHeight = desc.Height;
+                // Fail on non-positive dimensions
+                if (innerWidth <= 0 || innerHeight <= 0) {
+                    return gif_error("Invalid dimensions for inner image.\n",
+                            kInvalidInput);
+                }
+                // Treat the following cases as warnings and try to fix
+                if (innerWidth > width) {
+                    gif_warning("Inner image too wide, shrinking.\n");
+                    innerWidth = width;
+                    imageLeft = 0;
+                } else if (imageLeft + innerWidth > width) {
+                    gif_warning("Shifting inner image to left to fit.\n");
+                    imageLeft = width - innerWidth;
+                } else if (imageLeft < 0) {
+                    gif_warning("Shifting image to right to fit\n");
+                    imageLeft = 0;
+                }
+                if (innerHeight > height) {
+                    gif_warning("Inner image too tall, shrinking.\n");
+                    innerHeight = height;
+                    imageTop = 0;
+                } else if (imageTop + innerHeight > height) {
+                    gif_warning("Shifting inner image up to fit.\n");
+                    imageTop = height - innerHeight;
+                } else if (imageTop < 0) {
+                    gif_warning("Shifting image down to fit\n");
+                    imageTop = 0;
+                }
+
+                // Set up the color table
+                uint32_t colorCount = 0;
+                // Allocate maximum storage to deal with invalid indices safely
+                const uint32_t maxColors = 256;
+                SkPMColor colorTable[maxColors];
+                ColorMapObject* colorMap = fGif->Image.ColorMap;
+                // If there is no local color table, use the global color table
+                if (NULL == colorMap) {
+                    colorMap = fGif->SColorMap;
+                }
+                if (NULL != colorMap) {
+                    colorCount = colorMap->ColorCount;

[scroggo, 2015/03/31 18:39:56]

Is ColorCount created by giflib (therefore reliable) or is it directly taken
from the encoded data (and therefore could be wrong)? 

[msarett, 2015/03/31 20:11:40]

colorCount is reliable.  giflib calculates it and returns a NULL colorMap if it
is invalid.

+                    SkASSERT(colorCount ==
+                            (unsigned) (1 << (colorMap->BitsPerPixel)));
+                    SkASSERT(colorCount <= 256);
+                    for (uint32_t i = 0; i < colorCount; i++) {
+                        colorTable[i] = SkPackARGB32(0xFF,
+                                                   colorMap->Colors[i].Red,

[scroggo, 2015/03/31 18:39:56]

nit: This should line up with 0xFF or be indented 8 spaces.

[msarett, 2015/03/31 20:11:40]

Done.

+                                                   colorMap->Colors[i].Green,
+                                                   colorMap->Colors[i].Blue);
+                    }
+                }
+
+                // This is used to fill unspecified pixels in the image data.
+                uint32_t fillIndex = fGif->SBackGroundColor;
+                bool fillBackground = true;
+                ZeroInitialized zeroInit = opts.fZeroInitialized;
+
+                // Gifs have the option to specify the color at a single
+                // index of the color table as transparent.
+                {
+                    int32_t transIndex = find_trans_index(saveExt, colorCount);

[scroggo, 2015/03/31 18:39:56]

nit: What if we compared against colorCount here, instead of passing it into
find_trans_index? I'm a little torn on which is better; maybe it's fine as is.

[msarett, 2015/03/31 20:11:41]

I think this is much better, and it is now possible, since we are only checking
one GCE for a valid index.

+
+                    // If the background is already zeroed and we have a valid
+                    // transparent index, we do not need to fill the background.
+                    if (transIndex >= 0) {
+                        colorTable[transIndex] = SK_ColorTRANSPARENT;
+                        // If there is a transparent index, we also use this as
+                        // the fill index.
+                        fillIndex = transIndex;
+                        fillBackground = (kYes_ZeroInitialized != zeroInit);
+                    } else if (fillIndex >= colorCount) {
+                        // If the fill index is invalid, we default to 0.  This
+                        // behavior is unspecified but matches SkImageDecoder.
+                        fillIndex = 0;
+                    }
+                }
+
+                // Fill in the color table for indices greater than color count.
+                // This allows for predictable, safe behavior.
+                for (uint32_t i = colorCount; i < maxColors; i++) {
+                    colorTable[i] = colorTable[fillIndex];
+                } 
+
+                // Check if image is only a subset of the image frame
+                SkAutoTDelete<SkSwizzler> swizzler(NULL);
+                if (innerWidth < width || innerHeight < height) {
+
+                    // Modify the destination info
+                    const SkImageInfo subsetDstInfo =
+                            dstInfo.makeWH(innerWidth, innerHeight);
+
+                    // Fill the destination with the fill color
+                    // FIXME: This may not be the behavior that we want for
+                    //        animated gifs where we draw on top of the
+                    //        previous frame.
+                    SkColorType dstColorType = dstInfo.colorType();
+                    if (fillBackground) {
+                        switch (dstColorType) {
+                            case kN32_SkColorType:
+                                sk_memset32((SkPMColor*) dst,
+                                    colorTable[fillIndex],
+                                    ((int) dstRowBytes) * height
+                                    / sizeof(SkPMColor));
+                                break;
+                            default:
+                                SkASSERT(false);
+                                break;
+                        }
+                    }
+
+                    // Modify the dst pointer
+                    const int32_t dstBytesPerPixel =
+                            SkColorTypeBytesPerPixel(dstColorType);
+                    void* subsetDst = SkTAddOffset<void*>(dst,
+                            dstRowBytes * imageTop +
+                            dstBytesPerPixel * imageLeft);
+
+                    // Create the subset swizzler
+                    swizzler.reset(SkSwizzler::CreateSwizzler(
+                            SkSwizzler::kIndex, colorTable, subsetDstInfo,
+                            subsetDst, dstRowBytes, zeroInit));
+                } else {
+                    // Create the fully dimensional swizzler
+                    swizzler.reset(SkSwizzler::CreateSwizzler(
+                            SkSwizzler::kIndex, colorTable, dstInfo, dst,
+                            dstRowBytes, zeroInit));
+                }
+
+                // Stores output from dgiflib and input to the swizzler
+                SkAutoTDeleteArray<uint8_t>
+                        buffer(SkNEW_ARRAY(uint8_t, innerWidth));
+
+                // Check the interlace flag and iterate over rows of the input
+                if (fGif->Image.Interlace) {
+                    // In interlace mode, the rows of input are rearranged in
+                    // the output image.  We use an iterator to take care of
+                    // the rearranging.
+                    SkGifInterlaceIter iter(innerHeight);
+                    for (int32_t y = 0; y < innerHeight; y++) {
+                        if (GIF_ERROR == DGifGetLine(fGif, buffer.get(),
+                                innerWidth)) {
+                            // Recover from error by filling remainder of image
+                            if (fillBackground) {
+                                memset(buffer.get(), fillIndex, innerWidth);
+                                for (; y < innerHeight; y++) {
+                                    swizzler->next(buffer.get(), iter.nextY());
+                                }
+                            }
+                            return gif_error(SkStringPrintf(
+                                    "Could not decode line %d of %d.\n",
+                                    y, height - 1).c_str(), kIncompleteInput);
+                        }
+                        swizzler->next(buffer.get(), iter.nextY());
+                    }
+                } else {
+                    // Standard mode
+                    for (int32_t y = 0; y < innerHeight; y++) {
+                        if (GIF_ERROR == DGifGetLine(fGif, buffer.get(),
+                                innerWidth)) {
+                            if (fillBackground) {
+                                SkPMColor* dstPtr = (SkPMColor*) SkTAddOffset
+                                        <void*>(dst, y * dstRowBytes);
+                                sk_memset32(dstPtr, colorTable[fillIndex],
+                                        (height - y) * ((int) dstRowBytes)
+                                        / sizeof(SkPMColor));
+                            }
+                            return gif_error(SkStringPrintf(
+                                    "Could not decode line %d of %d.\n",
+                                    y, height - 1).c_str(), kIncompleteInput);
+                        }
+                        swizzler->next(buffer.get());
+                    }
+                }
+
+                // FIXME: Gif files may have multiple images stored in a single
+                //        file.  This is most commonly used to enable
+                //        animations.  Since we are leaving animated gifs as a
+                //        TODO, we will return kSuccess after decoding the
+                //        first image in the file.  This is the same behavior
+                //        as SkImageDecoder_libgif.
+                //
+                //        Most times this works pretty well, but sometimes it
+                //        doesn't.  For example, I have an animated test image
+                //        where the first image in the file is 1x1, but the
+                //        subsequent images are meaningful.  This currently
+                //        displays the 1x1 image, which is not ideal.  Right
+                //        now I am leaving this as an issue that will be
+                //        addressed when we implement animated gifs.
+                //
+                //        It is also possible (not explicitly disallowed in the
+                //        specification) that gif files provide multiple
+                //        images in a single file that are all meant to be
+                //        displayed in the same frame together.  I will
+                //        currently leave this unimplemented until I find a
+                //        test case that expects this behavior.
+                return kSuccess;
+            }
+
+            // Extensions are used to specify special properties of the image
+            // such as transparency or animation.
+            case EXTENSION_RECORD_TYPE:

[scroggo, 2015/03/31 18:39:57]

Does this part add the extension that provides the transparent index?

[msarett, 2015/03/31 20:11:40]

Yes.

+                // Read extension data
+#if GIFLIB_MAJOR < 5
+                if (GIF_ERROR ==
+                        DGifGetExtension(fGif, &saveExt.Function, &extData)) {
+#else
+                if (GIF_ERROR ==
+                        DGifGetExtension(fGif, &extFunction, &extData)) {
+#endif
+                    return gif_error("Could not get extension.\n",
+                            kIncompleteInput);
+                }
+
+                // Create an extension block with our data
+                while (NULL != extData) {
+                    // Add a single block
+#if GIFLIB_MAJOR < 5
+                    if (GIF_ERROR == AddExtensionBlock(&saveExt, extData[0],
+                            &extData[1])) {
+#else
+                    if (GIF_ERROR ==
+                            GifAddExtensionBlock(&saveExt.ExtensionBlockCount,
+                            &saveExt.ExtensionBlocks, extFunction, extData[0],
+                            &extData[1])) {
+#endif
+                        return gif_error("Could not add extension block.\n",
+                                kIncompleteInput);
+                    }
+                    // Move to the next block
+                    if (GIF_ERROR == DGifGetExtensionNext(fGif, &extData)) {
+                        return gif_error("Could not get next extension.\n",
+                                kIncompleteInput);
+                    }
+#if GIFLIB_MAJOR < 5
+                    saveExt.Function = 0;
+#endif
+                }
+                break;
+
+            // Signals the end of the gif file
+            case TERMINATE_RECORD_TYPE:

[scroggo, 2015/03/31 18:39:56]

Any reason to single this out from default? The behavior is the same.

[msarett, 2015/03/31 20:11:40]

The default case should never be reached.  I will make this clear.

+                break;
+
+            default:
+                break;
+        }
+    }
+
+    return gif_error("Could not find any images to decode in gif file.\n",
+            kInvalidInput);
+}