Bmp Image Decoding

src/codec/SkCodec_libbmp.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 "SkColorPriv.h"
+#include "SkColorTable.h"
+#include "SkEndian.h"
+#include "SkStream.h"
+
+/*
+ *
+ * Get a byte from the buffer
+ *
+ */
+uint8_t get_byte(uint8_t* buffer, uint32_t i) {
+    return buffer[i];
+}
+
+/*
+ *
+ * Get a short from the buffer
+ *
+ */
+uint16_t get_short(uint8_t* buffer, uint32_t i) {
+    uint16_t result;
+    memcpy(&result, &(buffer[i]), 2);
+#ifdef SK_CPU_BENDIAN
+    return SkEndianSwap16(result);
+#else
+    return result;
+#endif
+}
+
+/*
+ *
+ * Get an int from the buffer
+ *
+ */
+uint32_t get_int(uint8_t* buffer, uint32_t i) {
+    uint32_t result;
+    memcpy(&result, &(buffer[i]), 4);
+#ifdef SK_CPU_BENDIAN
+    return SkEndianSwap32(result);
+#else
+    return result;
+#endif
+}
+
+/*
+ *
+ * Defines the version and type of the second bitmap header
+ *
+ */
+enum BitmapHeaderType {
+    kInfoV1_BitmapHeaderType,
+    kInfoV2_BitmapHeaderType,
+    kInfoV3_BitmapHeaderType,
+    kInfoV4_BitmapHeaderType,
+    kInfoV5_BitmapHeaderType,
+    kOS2V1_BitmapHeaderType,
+    kOS2VX_BitmapHeaderType,
+    kUnknown_BitmapHeaderType
+};
+
+/*
+ *
+ * Possible bitmap compression types
+ *
+ */
+enum BitmapCompressionMethod {
+    kNone_BitmapCompressionMethod =          0,
+    k8BitRLE_BitmapCompressionMethod =       1,
+    k4BitRLE_BitmapCompressionMethod =       2,
+    kBitMasks_BitmapCompressionMethod =      3,
+    kJpeg_BitmapCompressionMethod =          4,
+    kPng_BitmapCompressionMethod =           5,
+    kAlphaBitMasks_BitmapCompressionMethod = 6,
+    kCMYK_BitmapCompressionMethod =          11,
+    kCMYK8BitRLE_BitmapCompressionMethod =   12,
+    kCMYK4BitRLE_BitmapCompressionMethod =   13
+};
+
+/*
+ *
+ * Checks the start of the stream to see if the image is a bitmap
+ *
+ */
+bool SkBmpCodec::IsBmp(SkStream* stream) {
+    // TODO: Support "IC", "PT", "CI", "CP", "BA"
+    // TODO: ICO files may contain a BMP and need to use this decoder
+    const char bmpSig[] = { 'B', 'M' };
+    char buffer[sizeof(bmpSig)];
+    return stream->read(buffer, sizeof(bmpSig)) == sizeof(bmpSig) &&
+        !memcmp(buffer, bmpSig, sizeof(bmpSig));
+}
+
+/*
+ *
+ * Assumes IsBmp was called and returned true
+ * Creates a bitmap decoder
+ * Reads enough of the stream to determine the image format
+ *
+ */
+SkCodec* SkBmpCodec::NewFromStream(SkStream* stream) {
+    // Header size constants
+    static const uint32_t kBmpHeaderBytes = 14;
+    static const uint32_t kBmpHeaderBytesPlusFour = kBmpHeaderBytes + 4;
+    static const uint32_t kBmpOS2V1Bytes = 12;
+    static const uint32_t kBmpOS2V2Bytes = 64;
+    static const uint32_t kBmpInfoBaseBytes = 16;
+    static const uint32_t kBmpInfoV1Bytes = 40;
+    static const uint32_t kBmpInfoV2Bytes = 52;
+    static const uint32_t kBmpInfoV3Bytes = 56;
+    static const uint32_t kBmpInfoV4Bytes = 108;
+    static const uint32_t kBmpInfoV5Bytes = 124;
+    static const uint32_t kBmpMaskBytes = 12;
+
+    // Read the first header and the size of the second header
+    SkAutoTDeleteArray<uint8_t> hBuffer(
+            SkNEW_ARRAY(uint8_t, kBmpHeaderBytesPlusFour));
+    if (stream->read(hBuffer.get(), kBmpHeaderBytesPlusFour) !=
+            kBmpHeaderBytesPlusFour) {
+        SkDebugf("Error: unable to read first bitmap header.\n");
+        return NULL;
+    }
+    //uint16_t signature = get_short(hBuffer, 0);

[scroggo, 2015/03/04 17:10:30]

Can these be removed?

+
+    // The total bytes in the bmp file
+    // We only need to use this value for RLE decoding, so we will only check
+    // that it is valid in the RLE case.
+    const uint32_t totalBytes = get_int(hBuffer.get(), 2);
+
+    //uint32_t reserved = get_int(hBuffer, 6);
+
+    // The offset from the start of the file where the pixel data begins
+    const uint32_t offset = get_int(hBuffer.get(), 10);
+    if (offset < kBmpHeaderBytes + kBmpOS2V1Bytes) {
+        SkDebugf("Error: invalid starting location for pixel data\n");
+        return NULL;
+    }
+
+    // The size of the second (info) header in bytes
+    // The size is the first field of the second header, so we have already
+    // read the first four infoBytes.
+    const uint32_t infoBytes = get_int(hBuffer.get(), 14);
+    if (infoBytes < kBmpOS2V1Bytes) {
+        SkDebugf("Error: invalid second header size.\n");
+        return NULL;
+    }
+    const uint32_t infoBytesRemaining = infoBytes - 4;
+    hBuffer.free();
+
+    // Read the second header
+    SkAutoTDeleteArray<uint8_t> iBuffer(
+            SkNEW_ARRAY(uint8_t, infoBytesRemaining));
+    if (stream->read(iBuffer.get(), infoBytesRemaining) != infoBytesRemaining) {
+        SkDebugf("Error: unable to read second bitmap header.\n");
+        return NULL;
+    }
+
+    // The number of bits used per pixel in the pixel data
+    uint16_t bitsPerPixel;
+
+    // The compression method for the pixel data
+    uint32_t compression = kNone_BitmapCompressionMethod;
+
+    // Number of colors in the color table, defaults to 0 or max (see below)
+    uint32_t numColors = 0;
+
+    // Bytes per color in the color table, early versions use 3, most use 4
+    uint32_t bytesPerColor;
+
+    // The image width and height
+    int width, height;
+
+    // Determine image information depending on second header format
+    BitmapHeaderType headerType;
+    if (infoBytes >= kBmpInfoBaseBytes) {
+        // Check the version of the header
+        switch (infoBytes) {
+            case kBmpInfoV1Bytes:
+                headerType = kInfoV1_BitmapHeaderType;
+                break;
+            case kBmpInfoV2Bytes:
+                headerType = kInfoV2_BitmapHeaderType;
+                break;
+            case kBmpInfoV3Bytes:
+                headerType = kInfoV3_BitmapHeaderType;
+                break;
+            case kBmpInfoV4Bytes:
+                headerType = kInfoV4_BitmapHeaderType;
+                break;
+            case kBmpInfoV5Bytes:
+                headerType = kInfoV5_BitmapHeaderType;
+                break;
+            case 16:
+            case 20:
+            case 24:
+            case 28:
+            case 32:
+            case 36:
+            case 42:
+            case 46:
+            case 48:
+            case 60:
+            case kBmpOS2V2Bytes:
+                headerType = kOS2VX_BitmapHeaderType;
+                break;
+            default:
+                // We do not signal an error here because there is the
+                // possibility of new or undocumented bmp header types.  Most
+                // of the newer versions of bmp headers are similar to and
+                // build off of the older versions, so we may still be able to
+                // decode the bmp.
+                SkDebugf("Warning: unknown bmp header format.\n");
+                headerType = kUnknown_BitmapHeaderType;
+                break;
+        }
+        // We check the size of the header before entering the if statement.
+        // We should not reach this point unless the size is large enough for
+        // these required fields.
+        SkASSERT(infoBytesRemaining >= 12);
+        width = get_int(iBuffer.get(), 0);
+        height = get_int(iBuffer.get(), 4);
+        //uint16_t planes = get_short(iBuffer, 8);
+        bitsPerPixel = get_short(iBuffer.get(), 10);
+
+        // Some versions do not have this field, so we check before
+        // overwriting the default value.
+        if (infoBytesRemaining >= 16) {
+            compression = get_int(iBuffer.get(), 12);
+        }
+        //uint32_t imageBytes = get_int(iBuffer, 16);
+        //uint32_t horizontalResolution = get_int(iBuffer, 20);
+        //uint32_t verticalResolution = get_int(iBuffer, 24);
+
+        // Some versions do not have this field, so we check before
+        // overwriting the default value.
+        if (infoBytesRemaining >= 32) {
+            numColors = get_int(iBuffer.get(), 28);
+        }
+        //uint32_t importantColors = get_int(iBuffer, infoBytes - 4, 32);
+        bytesPerColor = 4;
+    } else if (infoBytes >= kBmpOS2V1Bytes) {
+        // The OS2V1 is treated separately because it has a unique format
+        headerType = kOS2V1_BitmapHeaderType;
+        width = (int) get_short(iBuffer.get(), 0);
+        height = (int) get_short(iBuffer.get(), 2);
+        //uint16_t planes = get_short(iBuffer.get(), 4);
+        bitsPerPixel = get_short(iBuffer.get(), 6);
+        bytesPerColor = 3;
+    } else {
+        // There are no valid bmp headers
+        SkDebugf("Error: second bitmap header size is invalid.\n");
+        return NULL;
+    }
+
+    // Check for valid dimensions from header
+    static const uint32_t kBmpMaxDim = 1 << 16;

[scroggo, 2015/03/04 17:10:30]

nit: This could be after the next if statement (it's not used until then).

+    SkSwizzler::RowOrder rowOrder = SkSwizzler::kBottomUp_RowOrder;
+    if (height < 0) {
+        height = -height;
+        rowOrder = SkSwizzler::kTopDown_RowOrder;
+    }
+    if (width < 0 || width >= kBmpMaxDim || height >= kBmpMaxDim) {
+        // TODO: Decide if we want to support really large bmps.
+        SkDebugf("Error: invalid bitmap dimensions.\n");
+        return NULL;
+    }
+
+    // Create mask struct
+    SkSwizzler::ColorMasks masks;
+    masks.redMask = 0;

[scroggo, 2015/03/04 17:10:30]

Maybe ColorMasks should have a constructor? (Or we can use memset here?)

+    masks.greenMask = 0;
+    masks.blueMask = 0;
+    masks.alphaMask = 0;
+
+    // Determine the input compression format and set bit masks if necessary
+    uint32_t maskBytes = 0;
+    BitmapInputFormat inputFormat = kUnknown_BitmapInputFormat;
+    switch (compression) {
+        case kNone_BitmapCompressionMethod:
+            inputFormat = kStandard_BitmapInputFormat;
+            // Always respect alpha mask in V4+
+            if (kInfoV4_BitmapHeaderType == headerType ||
+                    kInfoV5_BitmapHeaderType == headerType) {
+                // Header types are matched based on size.  If the header is
+                // V4+, we are guaranteed to be able at least this size.
+                SkASSERT(infoBytesRemaining > 52);
+                masks.alphaMask = get_int(iBuffer.get(), 48);
+            }
+            break;
+        case k8BitRLE_BitmapCompressionMethod:
+            if (bitsPerPixel != 8) {
+                SkDebugf("Warning: correcting invalid bitmap format.\n");
+                bitsPerPixel = 8;
+            }
+            inputFormat = kRLE_BitmapInputFormat;
+            break;
+        case k4BitRLE_BitmapCompressionMethod:
+            if (bitsPerPixel != 4) {
+                SkDebugf("Warning: correcting invalid bitmap format.\n");
+                bitsPerPixel = 4;
+            }
+            inputFormat = kRLE_BitmapInputFormat;
+            break;
+        case kAlphaBitMasks_BitmapCompressionMethod:
+        case kBitMasks_BitmapCompressionMethod:
+            // Load the masks
+            inputFormat = kBitMask_BitmapInputFormat;
+            switch (headerType) {
+                case kInfoV1_BitmapHeaderType: {
+                    // The V1 header stores the bit masks after the header
+                    SkAutoTDeleteArray<uint8_t> mBuffer(
+                            SkNEW_ARRAY(uint8_t, kBmpMaskBytes));
+                    if (stream->read(mBuffer.get(), kBmpMaskBytes) !=
+                            kBmpMaskBytes) {
+                        SkDebugf("Error: unable to read bit masks.\n");
+                        return NULL;
+                    }
+                    maskBytes = kBmpMaskBytes;
+                    masks.redMask = get_int(mBuffer.get(), 0);
+                    masks.greenMask = get_int(mBuffer.get(), 4);
+                    masks.blueMask = get_int(mBuffer.get(), 8);
+                    break;
+                }
+                case kInfoV4_BitmapHeaderType:
+                case kInfoV5_BitmapHeaderType:
+                    // Header types are matched based on size.  If the header
+                    // is V4+, we are guaranteed to be able at least this size.
+                    SkASSERT(infoBytesRemaining >= 52);
+                    masks.alphaMask = get_int(iBuffer.get(), 48);
+                    // Intentional fall through
+                case kInfoV2_BitmapHeaderType:
+                case kInfoV3_BitmapHeaderType:
+                    // Header types are matched based on size.  If the header
+                    // is V2+, we are guaranteed to be able at least this size.
+                    SkASSERT(infoBytesRemaining >= 48);
+                    masks.redMask = get_int(iBuffer.get(), 36);
+                    masks.greenMask = get_int(iBuffer.get(), 40);
+                    masks.blueMask = get_int(iBuffer.get(), 44);
+                    break;
+                case kOS2VX_BitmapHeaderType:
+                    // TODO: Decide if we intend to support this.
+                    //       It is unsupported in the previous version and
+                    //       in chromium.  I have not come across a test case
+                    //       that uses this format.
+                    SkDebugf("Error: huffman format unsupported.\n");
+                    return NULL;
+                default:
+                   SkDebugf("Error: invalid bmp bit masks header.\n");
+                   return NULL;
+            }
+            break;
+        case kJpeg_BitmapCompressionMethod:
+            if (24 == bitsPerPixel) {
+                inputFormat = kRLE_BitmapInputFormat;
+                break;
+            }
+        case kPng_BitmapCompressionMethod:
+            // TODO: Decide if we intend to support this.
+            //       It is unsupported in the previous version and
+            //       in chromium.  I think it is used mostly for printers.
+            SkDebugf("Error: compression format not supported.\n");
+            return NULL;
+        case kCMYK_BitmapCompressionMethod:
+        case kCMYK8BitRLE_BitmapCompressionMethod:
+        case kCMYK4BitRLE_BitmapCompressionMethod:
+            // TODO: Same as above.
+            SkDebugf("Error: CMYK not supported for bitmap decoding.\n");
+            return NULL;
+        default:
+            SkDebugf("Error: invalid format for bitmap decoding.\n");
+            return NULL;
+    }
+    iBuffer.free();
+
+    // Check for valid bits per pixel input
+    switch (bitsPerPixel) {
+        // In addition to more standard pixel compression formats, bmp supports
+        // the use of bit masks to determine pixel components.  The bmp standard
+        // format for representing 16-bit colors is 555 (XRRRRRGGGGGBBBBB),
+        // which does not map well to any Skia color formats.  For this reason,
+        // we will always enable mask mode with 16 bits per pixel.
+        case 16:
+            if (kBitMask_BitmapInputFormat != inputFormat) {
+                masks.redMask = 0x7C00;
+                masks.greenMask = 0x03E0;
+                masks.blueMask = 0x001F;
+            }
+            break;
+        case 1:
+        case 2:
+        case 4:
+        case 8:
+        case 24:
+        case 32:
+            break;
+        default:
+            SkDebugf("Error: invalid input value for bits per pixel.\n");
+            return NULL;
+    }
+
+    // Process the color table
+    uint32_t colorBytes = 0;
+    SkPMColor* colorTable = NULL;
+    if (bitsPerPixel < 16) {
+        // Verify the number of colors for the color table
+        const int maxColors = 1 << bitsPerPixel;
+        // Zero is a default for maxColors
+        // Also set numColors to maxColors when input is too large
+        if (numColors <= 0 || numColors > maxColors) {
+            numColors = maxColors;
+        }
+        colorTable = SkNEW_ARRAY(SkPMColor, maxColors);
+
+        // Construct the color table
+        colorBytes = numColors * bytesPerColor;
+        SkAutoTDeleteArray<uint8_t> cBuffer(SkNEW_ARRAY(uint8_t, colorBytes));
+        if (stream->read(cBuffer.get(), colorBytes) != colorBytes) {
+            SkDebugf("Error: unable to read color table.\n");
+            return NULL;
+        }
+        // We must respect the alpha channel for V4 and V5.  However, if it is
+        // all zeros, we will display the image as opaque rather than
+        // transparent.  This may require redoing some of the processing.
+        bool seenNonZeroAlpha = false;
+        uint32_t i = 0;
+        for (; i < numColors; i++) {
+            uint8_t blue = get_byte(cBuffer.get(), i*bytesPerColor);
+            uint8_t green = get_byte(cBuffer.get(), i*bytesPerColor + 1);
+            uint8_t red = get_byte(cBuffer.get(), i*bytesPerColor + 2);
+            uint8_t alpha = 0xFF;
+            if (kInfoV4_BitmapHeaderType == headerType ||
+                    kInfoV5_BitmapHeaderType == headerType) {
+                alpha = (masks.alphaMask >> 24) &
+                        get_byte(cBuffer.get(), i*bytesPerColor + 3);
+                if (!alpha && !seenNonZeroAlpha) {
+                    alpha = 0xFF;
+                } else {
+                    // If we see a non-zero alpha, we restart the loop
+                    seenNonZeroAlpha = true;
+                    i = -1;

[scroggo, 2015/03/04 17:10:30]

don't you want a 'continue' here? Otherwise, won't you be modifying
colorTable[-1] incorrectly?

+                }
+            }
+            colorTable[i] = SkPreMultiplyColor(SkColorSetARGBInline(alpha,

[scroggo, 2015/03/04 17:10:30]

As stated in an earlier patch set, you don't need this two step (create an
SkColor, convert to SkPMColor). You can just create an SkPMColor.

[msarett, 2015/03/05 23:13:17]

Sorry I missed this.  Had the wrong code in so many places.

+                    red, green, blue));
+        }
+        // To avoid segmentation faults on bad pixel data, fill the end of the
+        // color table with black.  This is the same the behavior as the
+        // chromium decoder.
+        for (; i < maxColors; i++) {
+            colorTable[i] = SkPackARGB32(0xFF, 0, 0, 0);
+        }
+    } else {
+        // We will not use the color table if bitsPerPixel >= 16, but if there
+        // is a color table, we may need to skip the color table bytes.
+        // We will assume that the maximum color table size is the same as when
+        // there are 8 bits per pixel (the largest color table actually used).
+        // Color tables for greater than 8 bits per pixel are somewhat
+        // undocumented.  It is indicated that they may exist to store a list
+        // of colors for optimization on devices with limited color display
+        // capacity.  While we do not know for sure, we will guess that any
+        // value of numColors greater than this maximum is invalid.
+        if (numColors <= (1 << 8)) {
+            colorBytes = numColors * bytesPerColor;
+            if (stream->skip(colorBytes) != colorBytes) {
+                SkDebugf("Error: Could not skip color table bytes.\n");
+                return NULL;
+            }
+        }
+    }
+
+    // Ensure that the stream now points to the start of the pixel array
+    uint32_t bytesRead = kBmpHeaderBytes + infoBytes + maskBytes + colorBytes;
+
+    // Check that we have not read past the pixel array offset
+    if(bytesRead > offset) {
+        // This may occur on OS 2.1 and other old versions where the color
+        // table defaults to max size, and the bmp tries to use a smaller color
+        // table.  This is invalid, and our decision is to indicate an error,
+        // rather than try to guess the intended size of the color table and
+        // rewind the stream to display the image.
+        SkDebugf("Error: pixel data offset less than header size.\n");
+        return NULL;
+    }
+
+    // Skip to the start of the pixel array
+    if (stream->skip(offset - bytesRead) != offset - bytesRead) {
+        SkDebugf("Error: unable to skip to image data.\n");
+        return NULL;
+    }
+
+    // Remaining bytes is only used for RLE
+    const int remainingBytes = totalBytes - offset;
+    if (remainingBytes <= 0 && kRLE_BitmapInputFormat == inputFormat) {
+        SkDebugf("Error: RLE requires valid input size.\n");
+        return NULL;
+    }
+
+    // Return the codec
+    // Use of image info for input format does not make sense given
+    // that the possible bitmap input formats do not match up with
+    // Skia color types.  Instead we use ImageInfo for width and height,
+    // and other fields for input format information.
+    const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
+            kN32_SkColorType, kPremul_SkAlphaType);

[scroggo, 2015/03/04 17:10:30]

At this point, isn't it possible to know (at least for some formats) that the
image is opaque? If so, you should use kOpaque.

Also, FWIW, the plan is to make unpremul the default for Android. That doesn't
mean we have to make it the default on our end, although we should be
consistent. For SkPngCodec, I made unpremul the default, since that actually
matches the data better.

The downside to the default of unpremul is that Skia currently doesn't support
drawing unpremul (but we plan to).

+    return SkNEW_ARGS(SkBmpCodec, (imageInfo, stream, bitsPerPixel,
+                                   inputFormat, masks, colorTable, rowOrder,
+                                   remainingBytes));
+}
+
+/*
+ *
+ * Creates an instance of the decoder
+ * Called only by NewFromStream
+ *
+ */
+SkBmpCodec::SkBmpCodec(const SkImageInfo& info, SkStream* stream,
+                       uint16_t bitsPerPixel, BitmapInputFormat inputFormat,
+                       SkSwizzler::ColorMasks masks, SkPMColor* colorTable,
+                       SkSwizzler::RowOrder rowOrder,
+                       const uint32_t remainingBytes)
+    : INHERITED(info, stream)
+    , fBitsPerPixel(bitsPerPixel)
+    , fInputFormat(inputFormat)
+    , fBitMasks(masks)
+    , fColorTable(colorTable)
+    , fRowOrder(rowOrder)
+    , fRemainingBytes(remainingBytes)
+{}
+
+/*
+ *
+ * Initiates the bitmap decode
+ *
+ */
+SkCodec::Result SkBmpCodec::onGetPixels(const SkImageInfo& dstInfo,
+                                        void* dst, size_t dstRowBytes,
+                                        SkPMColor*, int*) {
+    // This version of the decoder does not support scaling
+    if (dstInfo.dimensions() != getOriginalInfo().dimensions()) {

[scroggo, 2015/03/04 17:10:30]

You'll need to do other checks, for example whether the alpha type is correct,
and check for a rewind. See SkPngCodec for examples (and maybe that should be
moved into a common place).

+        SkDebugf("Error: scaling not supported.\n");
+        return kInvalidScale;
+    }
+
+    switch (fInputFormat) {
+        case kRLE_BitmapInputFormat:
+            return decodeRLE(dstInfo, dst, dstRowBytes);
+        case kBitMask_BitmapInputFormat:
+        case kStandard_BitmapInputFormat:
+            return decode(dstInfo, dst, dstRowBytes);
+        default:
+            SkDebugf("Error: unknown bitmap input format.\n");

[scroggo, 2015/03/04 17:10:30]

When does this happen? Can we skip creating an SkBmpCodec in this case?

+            return kInvalidInput;
+    }
+}
+
+/*
+ *
+ * Performs the bitmap decoding for standard and bit masks input format
+ *
+ */
+SkCodec::Result SkBmpCodec::decode(const SkImageInfo& dstInfo,
+                                   void* dst, uint32_t dstRowBytes) {
+    // Set constant values
+    const int width = dstInfo.width();
+    const int height = dstInfo.height();
+    const uint32_t pixelsPerByte = 8 / fBitsPerPixel;
+    const uint32_t bytesPerPixel = fBitsPerPixel / 8;
+    const uint32_t unpaddedRowBytes = fBitsPerPixel < 16 ?
+            (width + pixelsPerByte - 1) / pixelsPerByte : width * bytesPerPixel;
+    const uint32_t paddedRowBytes = (unpaddedRowBytes + 3) & (~3);

[scroggo, 2015/03/04 17:10:30]

You're probably not familiar with it, but I believe this is the same as SkAlign4
(in SkTypes.h)?

+    const uint32_t alphaMask = fBitMasks.alphaMask;
+
+    // Get swizzler configuration
+    SkSwizzler::SrcConfig config;
+    switch (fBitsPerPixel) {
+        case 1:
+            config = SkSwizzler::kIndex1;
+            break;
+        case 2:
+            config = SkSwizzler::kIndex2;
+            break;
+        case 4:
+            config = SkSwizzler::kIndex4;
+            break;
+        case 8:
+            config = SkSwizzler::kIndex;
+            break;
+        case 16:
+            config = SkSwizzler::kMask16;
+            break;
+        case 24:
+            if (kBitMask_BitmapInputFormat == fInputFormat) {
+                config = SkSwizzler::kMask24;
+            } else {
+                config = SkSwizzler::kBGR;
+            }
+            break;
+        case 32:
+            if (kBitMask_BitmapInputFormat == fInputFormat) {
+                config = SkSwizzler::kMask32;
+            } else if (!alphaMask) {
+                config = SkSwizzler::kBGRX;
+            } else {
+                config = SkSwizzler::kBGRA;
+            }
+            break;
+        default:
+            SkASSERT(false);
+            return kInvalidInput;
+    }
+
+    // If alphaOption is kNormal, it indicates that the image will be
+    // considered as encoded.  If kTransparentAsOpaque, we will respect the
+    // value of the alpha channel if it is nonzero for any of the pixels.
+    // However, if it is always zero, we will consider the image opaque instead
+    // of transparent. This may require redoing some of the decoding.
+    SkSwizzler::AlphaOption alphaOption;
+    if (fBitsPerPixel < 16) {
+        alphaOption = SkSwizzler::kNormal_AlphaOption;
+    } else if (alphaMask) {
+        alphaOption = SkSwizzler::kTransparentAsOpaque_AlphaOption;
+    } else {
+        alphaOption = SkSwizzler::kOpaque_AlphaOption;
+    }
+
+    // Create swizzler
+    SkSwizzler* swizzler = SkSwizzler::CreateSwizzler(config, fColorTable.get(),
+            dstInfo, dst, dstRowBytes, false, fBitMasks, alphaOption,
+            fRowOrder);
+
+    // Allocate space for a row buffer and a source for the swizzler
+    SkAutoTDeleteArray<uint8_t> srcBuffer(SkNEW_ARRAY(uint8_t, paddedRowBytes));
+
+    // Iterate over rows of the image
+    for (uint32_t y = 0; y < height; y++) {
+        // Read a row of the input
+        if (stream()->read(srcBuffer.get(), paddedRowBytes) != paddedRowBytes) {
+            return kIncompleteInput;
+        }
+
+        // Decode the row in destination format
+        swizzler->next(srcBuffer.get());
+    }
+
+    // Finished decoding the entire image
+    return kSuccess;
+}
+
+/*
+ *
+ * Set an RLE pixel using the color table
+ *
+ */
+void SkBmpCodec::setRLEPixel(SkPMColor* dst, uint32_t dstRowBytes, int height,
+                             uint32_t x, uint32_t y, uint8_t index) {
+    if (SkSwizzler::kBottomUp_RowOrder == fRowOrder) {
+        y = height - y - 1;
+    }
+    SkPMColor* dstRow = SkTAddOffset<SkPMColor>(dst, y * dstRowBytes);
+    dstRow[x] = fColorTable.get()[index];
+}
+
+/*
+ *
+ * Performs the bitmap decoding for RLE input format
+ * RLE decoding is performed all at once, rather than a one row at a time
+ *
+ */
+SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo,
+                                      void* dst, uint32_t dstRowBytes) {
+    // Set RLE flags
+    static const uint8_t RLE_ESCAPE = 0;
+    static const uint8_t RLE_EOL = 0;
+    static const uint8_t RLE_EOF = 1;
+    static const uint8_t RLE_DELTA = 2;
+
+    // Set constant values
+    const int width = dstInfo.width();
+    const int height = dstInfo.height();
+    const uint32_t pixelsPerByte = 8 / fBitsPerPixel;
+    const uint32_t bytesPerPixel = fBitsPerPixel / 8;
+
+    // Input buffer parameters
+    uint32_t i = 0;

[scroggo, 2015/03/04 17:10:30]

It looks like in this case, "i" means "bytesRead"? I think the rest of this
method would be clearer with that name instead of i.

+    SkAutoTDeleteArray<uint8_t> buffer(SkNEW_ARRAY(uint8_t, fRemainingBytes));
+    uint32_t totalBytes = stream()->read(buffer.get(), fRemainingBytes);
+    if (totalBytes < fRemainingBytes) {
+        SkDebugf("Warning: incomplete RLE file.\n");
+    } else if (totalBytes <= 0) {
+        SkDebugf("Error: could not read RLE image data.\n");
+        return kInvalidInput;
+    }
+
+    // Destination parameters
+    uint32_t x = 0;
+    uint32_t y = 0;
+    // If the code skips pixels, remaining pixels are transparent or black
+    // TODO: Skip this is memory was already zeroed.

[scroggo, 2015/03/04 17:10:30]

if*

+    memset(dst, 0, dstRowBytes * height);
+    SkPMColor* dstPtr = (SkPMColor*) dst;
+
+    while (true) {
+        // Every entry takes at least two bytes
+        if (totalBytes - i < 2) {
+            SkDebugf("Warning: incomplete RLE input.\n");
+            return kIncompleteInput;
+        }
+
+        // Read the two bytes and verify we have not reached end of image
+        const uint8_t count = buffer.get()[i++];

[scroggo, 2015/03/04 17:10:30]

Can you elaborate what these variables mean? IIUC, "count" tells us something
like how many pixels or bytes to read, with RLE_ESCAPE being a sentinel. And
code means some kind of code?

Anywhere understanding what's going on requires having studied the bmp spec
would be a good place to add comments.

+        const uint8_t code = buffer.get()[i++];
+        if ((count || (code != RLE_EOF)) && y >= height) {

[scroggo, 2015/03/04 17:10:30]

I'm having trouble following this line.

Is it basically saying "if count/code says we're not done, but y says we are,
the input is malformed?"

Also, I think by treating count as a boolean you'll get a warning (which we turn
into an error) on Windows. I also think it's clearer to use a comparison. (You
use this pattern in a number of other places, e.g. when checking the alpha,
where I think it would be clearer and warning free to say what you mean.)

+            SkDebugf("Warning: invalid RLE input.\n");
+            return kIncompleteInput;
+        }
+
+        // Perform decoding
+        if (RLE_ESCAPE == count) {
+            switch (code) {
+                case RLE_EOL:
+                    x = 0;
+                    y++;
+                    break;
+                case RLE_EOF:
+                    return kSuccess;
+                case RLE_DELTA: {
+                    // Two bytes are needed to specify delta
+                    if (totalBytes - i < 2) {
+                        SkDebugf("Warning: incomplete RLE input\n");
+                        return kIncompleteInput;
+                    }
+                    // Modify x and y
+                    const uint8_t dx = buffer.get()[i++];
+                    const uint8_t dy = buffer.get()[i++];
+                    x += dx;
+                    y += dy;
+                    if (x > width || y > height) {
+                        SkDebugf("Warning: invalid RLE input.\n");
+                        return kIncompleteInput;
+                    }
+                    break;
+                }
+                default: { // Absolute mode
+                    // Check that we have enough bytes and that there are
+                    // enough pixels remaining in the row
+                    const uint32_t unpaddedBytes = fBitsPerPixel < 16 ?
+                            (code + pixelsPerByte - 1) / pixelsPerByte :
+                            code * bytesPerPixel;
+                    const uint32_t paddedBytes =
+                            (unpaddedBytes + 1) & (~1);

[scroggo, 2015/03/04 17:10:30]

I think you can use SkAlign2 here.

+                    if (x + code > width || totalBytes - i < paddedBytes) {

[scroggo, 2015/03/04 17:10:30]

So in this case, does code mean a number of pixels?

+                        SkDebugf("Warning: invalid RLE input.\n");
+                        return kIncompleteInput;
+                    }
+                    // Use the color table to set the coded number of pixels
+                    uint8_t num = code;
+                    while (num > 0) {
+                        switch(fBitsPerPixel) {
+                            case 4: {
+                                uint8_t val = buffer.get()[i++];
+                                setRLEPixel(dstPtr, dstRowBytes, height, x++, y,
+                                        val >> 4);
+                                num--;
+                                if (num) {
+                                    setRLEPixel(dstPtr, dstRowBytes, height,
+                                            x++, y, val & 0xF);
+                                    num--;
+                                }
+                                break;
+                            }
+                            case 8:
+                                setRLEPixel(dstPtr, dstRowBytes, height, x++, y,
+                                        buffer.get()[i++]);
+                                num--;
+                                break;
+                            case 24: {
+                                uint8_t blue = buffer.get()[i++];
+                                uint8_t green = buffer.get()[i++];
+                                uint8_t red = buffer.get()[i++];
+                                SkPMColor color = SkPreMultiplyARGB(
+                                        0xFF, red, green, blue);
+                                SkPMColor* dstRow = SkTAddOffset<SkPMColor>(
+                                        dstPtr, y * dstRowBytes);
+                                dstRow[x++] = color;
+                                num--;
+                            }
+                            default:
+                                SkASSERT(false);
+                                return kInvalidInput;
+                        }
+                    }
+                    // Skip a byte if necessary to maintain alignment
+                    if (unpaddedBytes & 1) {
+                        i++;
+                    }
+                    break;
+                }
+            }
+        } else { // Encoded mode
+            // Ensure we do not move past the end of the row
+            const uint32_t endX = SkTMin<uint32_t>(x + count, (uint32_t) width);
+
+            if (24 == fBitsPerPixel) {
+                // Check that there is enough data
+                if (totalBytes - i < 2) {
+                    SkDebugf("Warning: incomplete RLE input\n");
+                    return kIncompleteInput;
+                }
+
+                // Fill the pixels up to endX with the specified color
+                uint8_t blue = code;
+                uint8_t green = buffer.get()[i++];
+                uint8_t red = buffer.get()[i++];
+                SkPMColor color = SkPackARGB32(0xFF, red, green, blue);
+                SkPMColor* dstRow =
+                        SkTAddOffset<SkPMColor>(dstPtr, y * dstRowBytes);
+                while (x < endX) {
+                    dstRow[x++] = color;
+                }
+            } else {
+                // RLE8 has one color index that gets repeated
+                // RLE4 has two color indexes in the upper and lower 4 bits of
+                // the bytes, which are alternated
+                uint8_t indices[2] = { code, code };
+                if (4 == fBitsPerPixel) {
+                    indices[0] >>= 4;
+                    indices[1] &= 0xf;
+                }
+
+                // Set the indicated number of pixels
+                for (int which = 0; x < endX; x++) {
+                    setRLEPixel(dstPtr, dstRowBytes, height, x, y,
+                            indices[which]);
+                    which = !which;
+                }
+            }
+        }
+    }
+}