Enabling ico decoding with use of png and bmp decoders

src/codec/SkCodec_libico.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_libbmp.h"
+#include "SkCodec_libico.h"
+#include "SkCodec_libpng.h"
+#include "SkCodecPriv.h"
+#include "SkColorPriv.h"
+#include "SkData.h"
+#include "SkStream.h"
+#include "SkTDArray.h"
+#include "SkTSort.h"
+
+/*
+ * Checks the start of the stream to see if the image is an Ico or Cur
+ */
+bool SkIcoCodec::IsIco(SkStream* stream) {
+    const char icoSig[] = { '\x00', '\x00', '\x01', '\x00' };
+    const char curSig[] = { '\x00', '\x00', '\x02', '\x00' };
+    char buffer[sizeof(icoSig)];
+    return stream->read(buffer, sizeof(icoSig)) == sizeof(icoSig) &&
+            (!memcmp(buffer, icoSig, sizeof(icoSig)) ||
+            !memcmp(buffer, curSig, sizeof(curSig)));
+}
+
+/*
+ * Assumes IsIco was called and returned true
+ * Creates an Ico decoder
+ * Reads enough of the stream to determine the image format
+ */
+SkCodec* SkIcoCodec::NewFromStream(SkStream* stream) {
+    // Header size constants
+    static const uint32_t kIcoDirectoryBytes = 6;
+    static const uint32_t kIcoDirEntryBytes = 16;
+
+    // Read the directory header
+    SkAutoTDeleteArray<uint8_t> dirBuffer(
+            SkNEW_ARRAY(uint8_t, kIcoDirectoryBytes));
+    if (stream->read(dirBuffer.get(), kIcoDirectoryBytes) !=
+            kIcoDirectoryBytes) {
+        SkDebugf("Error: unable to read ico directory header.\n");
+        return NULL;
+    }
+
+    // Process the directory header
+    const uint16_t numImages = get_short(dirBuffer.get(), 4);
+    if (0 == numImages) {
+        SkDebugf("Error: No images embedded in ico.\n");
+        return NULL;
+    }
+
+    // Ensure that we can read all of indicated directory entries
+    SkAutoTDeleteArray<uint8_t> entryBuffer(
+            SkNEW_ARRAY(uint8_t, numImages*kIcoDirEntryBytes));
+    if (stream->read(entryBuffer.get(), numImages*kIcoDirEntryBytes) !=
+            numImages*kIcoDirEntryBytes) {
+        SkDebugf("Error: unable to read ico directory entries.\n");
+        return NULL;
+    }
+
+    // This structure is used to represent the vital information about entries
+    // in the directory header.  We will obtain this information for each
+    // directory entry.
+    struct Entry {
+        uint32_t offset;
+        uint32_t size;
+    };
+    SkAutoTDeleteArray<Entry> directoryEntries(SkNEW_ARRAY(Entry, numImages));
+
+    // Iterate over directory entries
+    for (uint32_t i = 0; i < numImages; i++) {
+        // The directory entry contains information such as width, height,
+        // bits per pixel, and number of colors in the color palette.  We will
+        // ignore these fields since they are repeated in the header of the
+        // embedded image.  In the event of an inconsistency, we would always
+        // defer to the value in the embedded header anyway.
+
+        // Specifies the size of the embedded image, including the header
+        uint32_t size = get_int(entryBuffer.get(), 8 + i*kIcoDirEntryBytes);
+
+        // Specifies the offset of the embedded image from the start of file.
+        // It does not indicate the start of the pixel data, but rather the
+        // start of the embedded image header.
+        uint32_t offset = get_int(entryBuffer.get(), 12 + i*kIcoDirEntryBytes);
+
+        // Save the vital fields
+        directoryEntries.get()[i].offset = offset;
+        directoryEntries.get()[i].size = size;
+    }
+
+    // It is "customary" that the embedded images will be stored in order of
+    // increasing offset.  However, the specification does not indicate that
+    // they must be stored in this order, so we will not trust that this is the
+    // case.  Here we sort the embedded images by increasing offset.
+    struct EntryLessThan {
+        bool operator() (Entry a, Entry b) const {
+            return a.offset < b.offset;
+        }
+    };
+    EntryLessThan lessThan;
+    SkTQSort(directoryEntries.get(), directoryEntries.get() + numImages - 1,
+            lessThan);
+
+    // Now will construct a candidate codec for each of the embedded images
+    uint32_t bytesRead = kIcoDirectoryBytes + numImages*kIcoDirEntryBytes;
+    SkTDArray<SkCodec*>* codecs = SkNEW(SkTDArray<SkCodec*>);

[scroggo, 2015/03/23 13:41:24]

This can leak.

SkTDArray has a copy constructor. If you use that, you don't need to use SkNEW
here. Note that it will be using malloc under the covers, so we'll still be
allocating memory somewhere.

And here's the ugly part: SkTDArray won't take care of calling SkDELETE on your
SkCodecs. You need to call SkTDArray::deleteAll() to take care of that. You
could put it in an SkAutoTCallVProc (in SkTemplates.h, along with
SkAutoTDelete), which lets you call a function when it goes out of scope. Your
function would call both deleteAll and delete the SkTDArray.

As an alternative, you could use an SkTArray<SkAutoTDelete<SkCodec>>. SkTArray
*does* call destructors, so this would delete each SkCodec when it goes out of
scope.

The tricky part for both is that when you pass ownership to the new codec you
need to make sure the original holder doesn't delete the entries. Maybe you use
an SkAutoTDelete<SkTArray<SkAutoTDelete<SkCodec>>>, although that's pretty
ugly...

[msarett, 2015/03/23 19:40:02]

Went with the SkAutoTDelete<SkTArray<SkAutoTDelete<SkCodec>>>

+    for (uint32_t i = 0; i < numImages; i++) {
+        uint32_t offset = directoryEntries.get()[i].offset;
+        uint32_t size = directoryEntries.get()[i].size;
+        
+        // Ensure that the offset is valid
+        if (offset < bytesRead) {
+            SkDebugf("Warning: invalid ico offset.\n");
+            continue;
+        }
+
+        // If we cannot skip, assume we have reached the end of the stream and
+        // stop trying to make codecs
+        if (stream->skip(offset - bytesRead) != offset - bytesRead) {
+            SkDebugf("Warning: could not skip to ico offset.\n");
+            break;
+        }
+        bytesRead = offset;
+
+        // Create a new stream for the embedded codec
+        SkAutoTUnref<SkData> data(SkData::NewFromStream(stream, size));
+        if (NULL == data.get()) {
+            SkDebugf("Warning: could not create embedded stream.\n");
+            break;
+        }
+        SkAutoTDelete<SkMemoryStream>
+                embeddedStream(SkNEW_ARGS(SkMemoryStream, (data.get())));
+        bytesRead += size;
+
+        // Check if the embedded codec is bmp or png and create the codec
+        SkAutoTDelete<SkCodec*> codec(SkNEW(SkCodec*));

[scroggo, 2015/03/23 13:41:25]

I think this is rarely something you want to do.

SkNewFromStream will return an SkCodec*. You'll immediately put that into your
list which takes care of deleting (and it's okay to copy the raw pointer, in
this case, since you know you're passing it to something that will take care of
deleting it).

[msarett, 2015/03/23 19:40:02]

Acknowledged.

+        const bool isPng = SkPngCodec::IsPng(embeddedStream);
+        if (!embeddedStream->rewind()) {

[scroggo, 2015/03/23 13:41:24]

SkMemoryStream::rewind will always return true. (Maybe use an assert:

SkAssertResult(embeddedStream->rewind());

Unlike SkASSERT, this is called in release mode.)

[msarett, 2015/03/23 19:40:02]

Done.

+            return NULL;
+        }
+        if (isPng) {
+            *codec = SkPngCodec::NewFromStream(embeddedStream.detach());
+        } else {
+            *codec = SkBmpCodec::NewFromIco(embeddedStream.detach());
+        }
+
+        // Save a valid codec
+        if (NULL != *codec) {
+            codecs->append(1, codec);
+        }
+    }
+
+    // Recognize if there are no valid codecs
+    uint32_t numValidImages = codecs->count();

[scroggo, 2015/03/23 13:41:24]

I think this can be const.

[msarett, 2015/03/23 19:40:02]

You are correct.  We are no longer using this variable, so there is no need to
fix.

+    if (0 == numValidImages) {
+        SkDebugf("Error: could not find any valid embedded ico codecs.\n");
+        return NULL;
+    }
+
+    // Use the largest codec as a "suggestion" for image info
+    uint32_t maxSize = 0;
+    uint32_t maxIndex = 0;
+    SkImageInfo info = SkImageInfo::MakeUnknown();
+    for (uint32_t i = 0; i < numValidImages; i++) {
+        codecs->operator[](i)->getInfo(&info);

[scroggo, 2015/03/23 13:41:25]

I think there's a version of getInfo which just returns an SkImageInfo, so you
don't need to call this, more complicated version.

[msarett, 2015/03/23 19:40:02]

Done.

+        uint32_t size = info.width() * info.height();
+        if (size > maxSize) {
+            maxSize = size;
+            maxIndex = i;
+        }
+    }
+    codecs->operator[](maxIndex)->getInfo(&info);
+
+    // Note that stream is owned by the embedded codec, the ico does not need
+    // direct access to the stream.
+    return SkNEW_ARGS(SkIcoCodec, (info, codecs->detach(), numValidImages));

[scroggo, 2015/03/23 13:41:25]

If you stored codecs as an SkTDArray or SkTArray in SkIcoCodec, you don't need
numValidImages.

[msarett, 2015/03/23 19:40:02]

Acknowledged.

+}
+
+/*
+ * Creates an instance of the decoder
+ * Called only by NewFromStream
+ */
+SkIcoCodec::SkIcoCodec(const SkImageInfo& info, SkCodec** embeddedCodecs,
+                       uint32_t numImages)
+    : INHERITED(info, NULL)
+    , fEmbeddedCodecs(embeddedCodecs)
+    , fNumImages(numImages)
+{}
+
+/*
+ * Chooses the best dimensions given the desired scale
+ */
+SkISize SkIcoCodec::onGetScaledDimensions(float desiredScale) const { 
+    // We set the dimensions to the largest candidate image by default.
+    // Regardless of the scale request, this is the largest image that we
+    // will decode.
+    if (desiredScale >= 1.0) {
+        return this->getOriginalInfo().dimensions();

[scroggo, 2015/03/23 13:41:25]

getInfo()

[msarett, 2015/03/23 19:40:02]

Done.

+    }
+
+    int origWidth = this->getOriginalInfo().width();
+    int origHeight = this->getOriginalInfo().height();
+    float desiredSize = desiredScale * origWidth * origHeight;
+    // At least one image will have smaller error than this initial value
+    float minError = origWidth * origHeight - desiredSize + 1.0;
+    int32_t minIndex = -1;
+    for (uint32_t i = 0; i < fNumImages; i++) {
+        int width = fEmbeddedCodecs.get()[i]->getOriginalInfo().width();;

[scroggo, 2015/03/23 13:41:25]

no need for double ;

[msarett, 2015/03/23 19:40:02]

Done.

+        int height = fEmbeddedCodecs.get()[i]->getOriginalInfo().width();;

[scroggo, 2015/03/23 13:41:25]

height()*

[msarett, 2015/03/23 19:40:02]

Done.

+        float error = SkTAbs(((float) (width * height)) - desiredSize);
+        if (error < minError) {
+            minError = error;
+            minIndex = (int32_t) i;
+        }
+    }
+    SkASSERT(minIndex >= 0);
+
+    return fEmbeddedCodecs.get()[minIndex]->getOriginalInfo().dimensions();
+}
+
+/*
+ * Initiates the Ico decode
+ */
+SkCodec::Result SkIcoCodec::onGetPixels(const SkImageInfo& dstInfo,
+                                        void* dst, size_t dstRowBytes,
+                                        const Options& opts, SkPMColor* ct,
+                                        int* ptr) {
+    for (uint32_t i = 0; i < fNumImages; i++) {
+        // If the dimensions match, try to decode
+        if (dstInfo.dimensions() ==
+                fEmbeddedCodecs.get()[i]->getOriginalInfo().dimensions()) {
+
+            // Perform the decode
+            Result result = fEmbeddedCodecs.get()[i]->getPixels(dstInfo, dst,
+                    dstRowBytes, &opts, ct, ptr);
+
+            // On a fatal error, keep trying to find an image to decode
+            if (kInvalidConversion == result || kInvalidInput == result) {
+                SkDebugf("Warning: Attempt to decode candidate ico failed.\n");
+                continue;
+            }
+
+            // On success or partial success, return the result
+            return result;
+        }
+    }
+
+    SkDebugf("Error: No ico candidate image with matching dimensions.\n");
+    return kInvalidScale;
+}