Bmp Image Decoding

src/codec/SkCodec_libbmp.cpp

Index: src/codec/SkCodec_libbmp.cpp
diff --git a/src/codec/SkCodec_libbmp.cpp b/src/codec/SkCodec_libbmp.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..56c9d1b7431f5d1eeeb365810a0c80b134a0ce03
--- /dev/null
+++ b/src/codec/SkCodec_libbmp.cpp
@@ -0,0 +1,524 @@
+/*
+ * 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 "SkColorTable.h"
+#include "SkEndian.h"
+#include "SkSwizzler.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

[scroggo, 2015/02/25 17:22:42]

This should not be indented. Same with other #ifdef etc.

+    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) {
+    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) {
+    // Read the first header and the size of the second header
+    SkAutoTDeleteArray<uint8_t> hBuffer(new uint8_t[kBmpHeaderBytes + 4]);

[scroggo, 2015/02/25 17:22:42]

You should use SkNEW_ARRAY (it can be found in SkPostConfig.h).

SkAutoTDeleteArray uses SkDELETE_ARRAY to correspond to this. (Typically our
clients convert directly to what you expect, in which case it doesn't make a
difference, but if someone were redefining SkNEW* and SkDELETE* we want to be
consistent.)

+    if (stream->read(hBuffer.get(), kBmpHeaderBytes + 4) !=

[msarett, 2015/02/24 21:56:06]

The size of the second header is technically the first field of the second
header.  By reading this field along with the first header, I can read the
second header in one call to stream->read instead of two.  I am hesitant to
change the constants in the header file to avoid this extra +4/-4 because the
values of the constant will then fail to match up with the bitmap spec.  Also I
think that changing the constant would fix this addition in some places but then
make it necessary to add in others.  Let me know what you think.

[scroggo, 2015/02/25 17:22:42]

Why not have something like:

const int kBmpHeaderBytesPlusFour = kBmpHeaderBytes + 4;

+            kBmpHeaderBytes + 4) {
+        SkDebugf("Error: unable to read first bitmap header.\n");
+        return NULL;
+    }
+    //uint16_t signature = get_short(hBuffer, 0);
+    //uint32_t totalBytes = get_int(hBuffer, 2);
+    //uint32_t reserved = get_int(hBuffer, 6);
+    uint32_t offset = get_int(hBuffer.get(), 10);

[scroggo, 2015/02/25 17:22:42]

Can these values be const?

More importantly, keep in mind that we're reading arbitrary data, so we need to
make sure that it's sensible. (Otherwise we end up with bugs like skbug.com/3401
and skbug.com/3426.)

On that note, maybe add a comment and/or assert that these calls to get_int are
safe. (I know I discouraged you from using asserts in the general case, but in
this case the numbers being compared are all compile time constants, so you
could even use SK_COMPILE_ASSERT. We'll be able to catch that in
tests/compilation without having to test every possible BMP file out there.)

It also might be nice to have comments explaining what offset and infoBytes
mean.

+    uint32_t infoBytes = get_int(hBuffer.get(), 14);
+    hBuffer.free();
+    
+    // Read the second header
+    BitmapHeaderType headerType = kUnknown_BitmapHeaderType;
+    SkAutoTDeleteArray<uint8_t> iBuffer(new uint8_t[infoBytes - 4]);

[scroggo, 2015/02/25 17:22:41]

Again, if you're using SkAutoTDeleteArray, which calls SkDELETE_ARRAY, you
should use SkNEW_ARRAY.

Also, I think it would be good to create a new variable and set it to infoBytes
- 4. You can give it a descriptive name, too, which will help me understand
what's going on here.

+    if (stream->read(iBuffer.get(), infoBytes - 4) != infoBytes - 4) {
+        SkDebugf("Error: unable to read second bitmap header.\n");
+        return NULL;
+    }
+    
+    // If applicable, set default values for the header fields
+    uint16_t bitsPerPixel;
+    uint32_t compression = kNone_BitmapCompressionMethod;
+    uint32_t numColors = 0;
+    uint32_t bytesPerColor = 3;
+    int width, height;
+    if (infoBytes > kBmpOS2V1Bytes) {
+        // Check for the many partial versions of the OS 2 header
+        if ((infoBytes <= kBmpOS2V2Bytes && !(infoBytes & 3))
+                || 42 == infoBytes || 46 == infoBytes) {
+            headerType = kOS2VX_BitmapHeaderType;
+        }
+        // Check for versions of the Windows headers
+        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;
+            default:
+                break;
+        }
+        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);
+        if (infoBytes - 4 >= 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);
+        if (infoBytes - 4 >= 32) {
+            numColors = get_int(iBuffer.get(), 28);
+        }
+        //uint32_t importantColors = get_int(iBuffer, infoBytes - 4, 32);
+        bytesPerColor = 4;
+    } else if (kBmpOS2V1Bytes == infoBytes) {
+        // This first OS 2 header has a unique format
+        headerType = kOS2V1_BitmapHeaderType;
+        width = (short) get_short(iBuffer.get(), 0);

[scroggo, 2015/02/25 17:22:42]

Why did you cast to a short? It looks like width and height are ints?

+        height = (short) get_short(iBuffer.get(), 2);
+        uint16_t planes = get_short(iBuffer.get(), 4);
+        bitsPerPixel = get_short(iBuffer.get(), 6);
+        compression = kNone_BitmapCompressionMethod;
+        numColors = 0;
+        bytesPerColor = 3;
+    } else {
+        SkDebugf("Error: second bitmap header size is invalid.\n");
+        return NULL;
+    }
+    
+    // Check the header type
+    if (kUnknown_BitmapHeaderType == headerType) {
+        SkDebugf("Warning: bitmap header type may not be supported.\n");

[scroggo, 2015/02/25 17:22:41]

What does this mean? (Also, it looks like it could be in the default case above
- is there any other way it could be unknown?)

+    }
+    
+    // Check for valid dimensions from header
+    bool inverted = true;
+    if (height < 0) {
+        height = -height;
+        inverted = false;
+    }
+    if (width <= 0 || width > kBmpMaxDim || !height || height > kBmpMaxDim) {
+        SkDebugf("Error: invalid bitmap dimensions.\n");
+        return NULL;
+    }
+
+    // Initialize bit masks
+    uint32_t redMask = 0, greenMask = 0, blueMask = 0, alphaMask = 0;
+    switch (bitsPerPixel) {
+        // Represent standard 16-bit format as bit masks (555)

[scroggo, 2015/02/25 17:22:42]

nit: This would be clearer to me if it said something like "RRRRRGGGGGBBBBB".
Maybe this is clear enough for other readers. (Does the comment belong inside
the case statement though?)

+        case 16:
+            redMask = 0x7C00;
+            greenMask = 0x03E0;
+            blueMask = 0x001F;
+            break;
+        default:

[scroggo, 2015/02/25 17:22:42]

Do you plan to support other bitsPerPixel? If so, maybe add a TODO? Otherwise,
could you just use an if statement?

[msarett, 2015/02/26 23:58:18]

Other bits per pixel are supported.  We just don't need default masks for the
others.  bPP < 16 uses indices.  24 and 32 bit are captured using RGB and RGBA
types (we don't need to use bit masks unless the compression type is explicitly
set as "bit masks", in which case the masks are set below).

However, this is a good place to check for a valid bPP input, rather than in the
decode function.

+            break;
+    }
+
+    // Determine the format of the input
+    uint32_t maskBytes = 0;
+    BitmapInputFormat inputFormat = kUnknown_BitmapInputFormat;
+    switch (compression) {
+        case kNone_BitmapCompressionMethod:
+            inputFormat = kStandard_BitmapInputFormat;
+            // Always respect alpha mask in V4+
+            if (headerType == kInfoV4_BitmapHeaderType ||
+                    headerType == kInfoV5_BitmapHeaderType) {
+                alphaMask = get_int(iBuffer.get(), 48);

[scroggo, 2015/02/25 17:22:41]

compile assert that 48 is safe.

+            }
+            break;
+        case k8BitRLE_BitmapCompressionMethod:
+            if (bitsPerPixel != 8) {
+                SkDebugf("Warning: correcting invalid bitmap format.\n");
+                bitsPerPixel = 8;
+            }
+            inputFormat = k8BitRLE_BitmapInputFormat;
+            break;
+        case k4BitRLE_BitmapCompressionMethod:
+            if (bitsPerPixel != 4) {
+                SkDebugf("Warning: correcting invalid bitmap format.\n");
+                bitsPerPixel = 4;
+            }
+            inputFormat = k4BitRLE_BitmapInputFormat;
+            break;
+        case kAlphaBitMasks_BitmapCompressionMethod:
+        case kBitMasks_BitmapCompressionMethod:
+            // Load the masks
+            if (headerType == kInfoV1_BitmapHeaderType) {
+                SkAutoTDeleteArray<uint8_t> mBuffer(new uint8_t[kBmpMaskBytes]);
+                if (stream->read(mBuffer.get(), kBmpMaskBytes) !=
+                        kBmpMaskBytes) {
+                    SkDebugf("Error: unable to read bit masks.\n");
+                    return NULL;
+                }
+                maskBytes = kBmpMaskBytes;
+                redMask = get_int(mBuffer.get(), 0);
+                greenMask = get_int(mBuffer.get(), 4);
+                blueMask = get_int(mBuffer.get(), 8);
+                mBuffer.free();

[scroggo, 2015/02/25 17:22:41]

This call is unnecessary. mBuffer will go out of scope on the next line, calling
free automatically (that's the benefit of using SkAutoTDeleteArray!)

+            } else if (headerType == kInfoV2_BitmapHeaderType ||

[scroggo, 2015/02/25 17:22:42]

Any reason not to make this a switch statement? - Oh, maybe you wanted to avoid
a switch within a switch? Yeah, that's ugly ...

+                        headerType == kInfoV3_BitmapHeaderType) {
+                redMask = get_int(iBuffer.get(), 36);
+                greenMask = get_int(iBuffer.get(), 40);
+                blueMask = get_int(iBuffer.get(), 44);
+            } else if (headerType == kInfoV4_BitmapHeaderType ||
+                        headerType == kInfoV5_BitmapHeaderType) {
+                redMask = get_int(iBuffer.get(), 36);
+                greenMask = get_int(iBuffer.get(), 40);
+                blueMask = get_int(iBuffer.get(), 44);
+                alphaMask = get_int(iBuffer.get(), 48);
+            } else if (headerType == kOS2VX_BitmapHeaderType) {

[scroggo, 2015/02/25 17:22:42]

Is this a TODO? It seems like we could have returned much earlier.

[msarett, 2015/02/26 23:58:18]

It is a TODO in the chromium code.  It is pretty low on my list of things to do.

+                SkDebugf("Error: huffman compression format unsupported.\n");
+                return NULL;
+            } else {
+                SkDebugf("Error: invalid compression format for header.\n");
+                return NULL;
+            }
+            inputFormat = kBitMask_BitmapInputFormat;
+            break;
+        case kJpeg_BitmapCompressionMethod:
+        case kPng_BitmapCompressionMethod:
+            SkDebugf("Error: compression format not supported.\n");
+            return NULL;
+        case kCMYK_BitmapCompressionMethod:
+        case kCMYK8BitRLE_BitmapCompressionMethod:
+        case kCMYK4BitRLE_BitmapCompressionMethod:
+            SkDebugf("Error: CMYK not supported for bitmap decoding.\n");
+            return NULL;
+        default:
+            SkDebugf("Error: invalid format for bitmap decoding.\n");
+            return NULL;
+    }
+    iBuffer.free();
+    
+    // Create mask array
+    uint32_t* masks = new uint32_t[4];

[scroggo, 2015/02/25 17:22:42]

Instead of an array, what do you think of using a struct?

struct ColorMasks {
  uint32_t redMask;
  uint32_t greenMask;
  uint32_t blueMask;
  uint32_t alphaMask;
};

That will make all the code dealing with these masks much clearer.

+    masks[0] = redMask;
+    masks[1] = greenMask;
+    masks[2] = blueMask;
+    masks[3] = alphaMask;
+
+    
+    // Check that the input format has been discovered
+    if (kUnknown_BitmapInputFormat == inputFormat) {
+        SkDebugf("Error: unknown bitmap input format.\n");
+        return NULL;

[scroggo, 2015/02/25 17:22:42]

This will leak masks. If you used SkAutoTDeleteArray, you can call detach()
later if masks needs to live on.

Alternatively, you could create masks much later, since it does not appear to be
used until the end.

+    }
+
+    // Verify the number of colors for the color table
+    if (bitsPerPixel < 16) {
+        int maxColors = 1 << bitsPerPixel;

[scroggo, 2015/02/25 17:22:42]

nit: could be const.

+        // Zero is a default for maxColors
+        // Also set numColors to maxColors when input is too large
+        if (numColors <= 0 || numColors > maxColors) {
+            numColors = maxColors;
+        }
+    }
+
+    // Construct the color table
+    // Note that if bPP >= 16, there still may be a color table.
+    // In this case, the decoder does not index into it, instead it stores a
+    // list of colors intended for optimization.

[scroggo, 2015/02/25 17:22:41]

What does this mean?

+    uint32_t colorBytes = numColors * bytesPerColor;
+    SkPMColor* colorTable = new SkPMColor[numColors];
+    if (numColors > 0) {
+        SkAutoTDeleteArray<uint8_t> cBuffer(new 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;
+        for (uint32_t i = 0; 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 (headerType == kInfoV4_BitmapHeaderType ||
+                    headerType == kInfoV5_BitmapHeaderType) {
+                alpha = (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;
+                }
+            }
+            colorTable[i] = SkPreMultiplyColor(SkColorSetARGBInline(alpha,
+                    red, green, blue));
+        }
+        cBuffer.free();

[scroggo, 2015/02/25 17:22:41]

Again, this is unnecessary.

+    }
+    
+    // Ensure that the stream now points to the start of the pixel array
+    uint32_t totalBytes = kBmpHeaderBytes + infoBytes + maskBytes +
+            numColors * bytesPerColor;
+    if (stream->skip(offset - totalBytes) != offset - totalBytes) {
+        SkDebugf("Error: unable to skip to image data.\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);
+    return SkNEW_ARGS(SkBmpCodec, (imageInfo, stream, bitsPerPixel,
+                                   inputFormat, masks, colorTable, inverted));
+}
+
+/*
+ *
+ * Creates an instance of the decoder
+ * Called only by NewFromStream
+ *
+ */
+SkBmpCodec::SkBmpCodec(const SkImageInfo& info, SkStream* stream,
+                       const uint16_t bitsPerPixel,
+                       const BitmapInputFormat inputFormat,
+                       uint32_t* masks,
+                       SkPMColor* colorTable,
+                       bool inverted)
+    : INHERITED(info, stream)
+    , fBitsPerPixel(bitsPerPixel)
+    , fInputFormat(inputFormat)
+    , fBitMasks(masks)
+    , fColorTable(colorTable)
+    , fInverted(inverted)
+{}
+
+/*
+ *
+ * Clean up memory used by the decoder
+ * Currently, we are using autodelete types and there is no work to be done
+ *
+ */
+SkBmpCodec::~SkBmpCodec() {}

[scroggo, 2015/02/25 17:22:42]

This may just be a matter of preference, but I feel like declaring and defining
an empty destructor just adds clutter.

+
+/*
+ *
+ * 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()) {
+        SkDebugf("Error: scaling not supported.\n");
+        return kInvalidScale;
+    }
+    
+    switch (fInputFormat) {
+        case k4BitRLE_BitmapInputFormat:
+        case k8BitRLE_BitmapInputFormat:
+            // TODO: Support RLE decoding
+            SkDebugf("RLE decoding not supported yet.\n");
+            return kUnimplemented;
+        case kBitMask_BitmapInputFormat:
+        case kStandard_BitmapInputFormat:
+            return decode(dstInfo, dst, dstRowBytes);
+        default:
+            SkDebugf("Error: unknown bitmap input format.\n");
+            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);
+    const uint32_t alphaMask = fBitMasks.get()[3];
+    
+    // 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::kIndex8;
+            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:
+            SkDebugf("Error: invalid number of bits per pixel.\n");

[scroggo, 2015/02/25 17:22:42]

We already knew this when creating the SkBmpCodec. Why not just return NULL from
NewFromStream?

+            return kInvalidInput;
+    }
+    
+    // If fixAlpha is false, it indicates that the image will be considered
+    // opaque.  If fixAlpha is true, 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.
+    bool fixAlpha = false;
+    if (alphaMask) {
+        fixAlpha = true;
+    }
+    
+    // Create swizzler
+    SkSwizzler* swizzler = SkSwizzler::CreateSwizzler(config, fColorTable.get(),
+            dstInfo, dst, dstRowBytes, false, fBitMasks.get(), fixAlpha,
+            fInverted);
+
+    // Allocate space for a row buffer and a source for the swizzler
+    uint8_t* srcBuffer = new uint8_t[paddedRowBytes];

[scroggo, 2015/02/25 17:22:42]

Use some form of auto deleter to avoid the memory leak below.

+
+    // Iterate over rows of the image
+    for (uint32_t row = 0; row < height; row++) {
+        // Read a row of the input
+        if (fStream->read(srcBuffer, paddedRowBytes) != paddedRowBytes) {
+            return kIncompleteInput;
+        }
+
+        // Decode the row in destination format
+        swizzler->next(srcBuffer);
+    }
+
+    // Clean up memory
+    delete [] srcBuffer;
+
+    // Finished decoding the entire image
+    return kSuccess;
+}