Make SkPngCodec decode progressively.

src/codec/SkPngCodec.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 "SkBitmap.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
@@ skipping 13 matching lines @@
 
 // This warning triggers false postives way too often in here.
 #if defined(__GNUC__) && !defined(__clang__)
     #pragma GCC diagnostic ignored "-Wclobbered"
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
 // Callback functions
 ///////////////////////////////////////////////////////////////////////////////
 
+// When setjmp is first called, it returns 0, meaning longjmp was not called.
+constexpr int kSetJmpOkay   = 0;
+// An error internal to libpng.
+constexpr int kPngError     = 1;
+// Passed to longjmp when we have decoded as many lines as we need.
+constexpr int kStopDecoding = 2;
+
 static void sk_error_fn(png_structp png_ptr, png_const_charp msg) {
     SkCodecPrintf("------ png error %s\n", msg);
-    longjmp(png_jmpbuf(png_ptr), 1);
+    longjmp(png_jmpbuf(png_ptr), kPngError);
 }
 
 void sk_warning_fn(png_structp, png_const_charp msg) {
     SkCodecPrintf("----- png warning %s\n", msg);
 }
 
-static void sk_read_fn(png_structp png_ptr, png_bytep data,
-                       png_size_t length) {
-    SkStream* stream = static_cast<SkStream*>(png_get_io_ptr(png_ptr));
-    const size_t bytes = stream->read(data, length);
-    if (bytes != length) {
-        // FIXME: We want to report the fact that the stream was truncated.
-        // One way to do that might be to pass a enum to longjmp so setjmp can
-        // specify the failure.
-        png_error(png_ptr, "Read Error!");
-    }
-}
-
 #ifdef PNG_READ_UNKNOWN_CHUNKS_SUPPORTED
 static int sk_read_user_chunk(png_structp png_ptr, png_unknown_chunkp chunk) {
     SkPngChunkReader* chunkReader = (SkPngChunkReader*)png_get_user_chunk_ptr(png_ptr);
     // readChunk() returning true means continue decoding
     return chunkReader->readChunk((const char*)chunk->name, chunk->data, chunk->size) ? 1 : -1;
 }
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
 // Helpers
 ///////////////////////////////////////////////////////////////////////////////
 
 class AutoCleanPng : public SkNoncopyable {
 public:
-    AutoCleanPng(png_structp png_ptr)
+    /*
+     *  This class does not take ownership of stream or reader, but if codecPtr
+     *  is non-NULL, and decodeBounds succeeds, it will have created a new
+     *  SkCodec (pointed to by *codecPtr) which will own/ref them, as well as
+     *  the png_ptr and info_ptr.
+     */
+    AutoCleanPng(png_structp png_ptr, SkStream* stream, SkPngChunkReader* reader,
+            SkCodec** codecPtr)
         : fPng_ptr(png_ptr)
-        , fInfo_ptr(nullptr) {}
+        , fInfo_ptr(nullptr)
+        , fDecodedBounds(false)
+        , fReadHeader(false)
+        , fStream(stream)
+        , fChunkReader(reader)
+        , fOutCodec(codecPtr)
+    {}
 
     ~AutoCleanPng() {
         // fInfo_ptr will never be non-nullptr unless fPng_ptr is.
         if (fPng_ptr) {
             png_infopp info_pp = fInfo_ptr ? &fInfo_ptr : nullptr;
             png_destroy_read_struct(&fPng_ptr, info_pp, nullptr);
         }
     }
 
     void setInfoPtr(png_infop info_ptr) {
         SkASSERT(nullptr == fInfo_ptr);
         fInfo_ptr = info_ptr;
     }
 
-    void release() {
+    /**
+     *  Reads enough of the input stream to decode the bounds.
+     *  @return false if the stream is not a valid PNG (or too short).
+     *          true if it read enough of the stream to determine the bounds.
+     *          In the latter case, the stream may have been read beyond the
+     *          point to determine the bounds, and the png_ptr will have saved
+     *          any extra data. Further, if the codecPtr supplied to the
+     *          constructor was not NULL, it will now point to a new SkCodec,
+     *          which owns (or refs, in the case of the SkPngChunkReader) the
+     *          inputs. If codecPtr was NULL, the png_ptr and info_ptr are
+     *          unowned, and it is up to the caller to destroy them.
+     */
+    bool decodeBounds();
+
+private:
+    png_structp         fPng_ptr;
+    png_infop           fInfo_ptr;
+    bool                fDecodedBounds;
+    bool                fReadHeader;
+    SkStream*           fStream;
+    SkPngChunkReader*   fChunkReader;
+    SkCodec**           fOutCodec;
+
+    /**
+     *  Supplied to libpng to call when it has read enough data to determine
+     *  bounds.
+     */
+    static void InfoCallback(png_structp png_ptr, png_infop info_ptr) {
+        // png_get_progressive_ptr returns the pointer we set on the png_ptr with
+        // png_set_progressive_read_fn
+        static_cast<AutoCleanPng*>(png_get_progressive_ptr(png_ptr))->infoCallback();
+    }
+
+    void infoCallback();
+
+    void releasePngPtrs() {
         fPng_ptr = nullptr;
         fInfo_ptr = nullptr;
     }
-
-private:
-    png_structp     fPng_ptr;
-    png_infop       fInfo_ptr;
 };
 #define AutoCleanPng(...) SK_REQUIRE_LOCAL_VAR(AutoCleanPng)
 
+bool AutoCleanPng::decodeBounds() {
+    if (setjmp(png_jmpbuf(fPng_ptr))) {
+        return false;
+    }
+
+    png_set_progressive_read_fn(fPng_ptr, this, InfoCallback, nullptr, nullptr);
+
+    // Arbitrary buffer size, though note that it matches (below)
+    // SkPngCodec::processData(). FIXME: Can we better suit this to the size of
+    // the PNG header?
+    constexpr size_t kBufferSize = 4096;
+    char buffer[kBufferSize];
+
+    while (true) {
+        const size_t bytesRead = fStream->read(buffer, kBufferSize);
+        if (!bytesRead) {
+            // We have read to the end of the input without decoding bounds.
+            break;
+        }
+
+        png_process_data(fPng_ptr, fInfo_ptr, (png_bytep) buffer, bytesRead);
+        if (fReadHeader) {
+            break;
+        }
+    }
+
+    // For safety, clear the pointer to this object.
+    png_set_progressive_read_fn(fPng_ptr, nullptr, nullptr, nullptr, nullptr);
+    return fDecodedBounds;
+}
+
+void SkPngCodec::processData() {
+    switch (setjmp(png_jmpbuf(fPng_ptr))) {
+        case kPngError:
+            // There was an error. Stop processing data.
+            // FIXME: Do we need to discard png_ptr?
+            return;
+        case kStopDecoding:
+            // We decoded all the lines we want.
+            return;
+        case kSetJmpOkay:
+            // Everything is okay.
+            break;
+        default:
+            // No other values should be passed to longjmp.
+            SkASSERT(false);
+    }
+
+    // Arbitrary buffer size
+    constexpr size_t kBufferSize = 4096;
+    char buffer[kBufferSize];
+
+    while (true) {
+        const size_t bytesRead = this->stream()->read(buffer, kBufferSize);
+        png_process_data(fPng_ptr, fInfo_ptr, (png_bytep) buffer, bytesRead);
+
+        if (!bytesRead) {
+            // We have read to the end of the input. Note that we quit *after*
+            // calling png_process_data, because decodeBounds may have told
+            // libpng to save the remainder of the buffer, in which case
+            // png_process_data will process the saved buffer, though the
+            // stream has no more to read.
+            break;
+        }
+    }
+}
+
 // Note: SkColorTable claims to store SkPMColors, which is not necessarily the case here.
 bool SkPngCodec::createColorTable(const SkImageInfo& dstInfo, int* ctableCount) {
 
     int numColors;
     png_color* palette;
     if (!png_get_PLTE(fPng_ptr, fInfo_ptr, &palette, &numColors)) {
         return false;
     }
 
     // Contents depend on tableColorType and our choice of if/when to premultiply:
@@ skipping 238 matching lines @@
         return SkColorSpace_Base::NewRGB(gammas, toXYZD50);
     }
 
 #endif // LIBPNG >= 1.6
 
     // Report that there is no color space information in the PNG.  SkPngCodec is currently
     // implemented to guess sRGB in this case.
     return nullptr;
 }
 
-static int bytes_per_pixel(int bitsPerPixel) {
-    // Note that we will have to change this implementation if we start
-    // supporting outputs from libpng that are less than 8-bits per component.
-    return bitsPerPixel / 8;
-}
-
 void SkPngCodec::allocateStorage(const SkImageInfo& dstInfo) {
     switch (fXformMode) {
         case kSwizzleOnly_XformMode:
-            fStorage.reset(SkAlign4(fSrcRowBytes));

[msarett, 2016/09/14 13:21:06]

Cool!  Now we decode into memory allocated by libpng instead of our own storage
buffer?

[scroggo_chromium, 2016/09/14 14:54:13]

Yes - png_process_data decodes into a buffer it passes to our callback. I don't
know whether they're any more efficient than we could be, but it's one less
thing for us to keep track of.

-            fSwizzlerSrcRow = fStorage.get();
             break;
         case kColorOnly_XformMode:
             // Intentional fall through.  A swizzler hasn't been created yet, but one will
             // be created later if we are sampling.  We'll go ahead and allocate
             // enough memory to swizzle if necessary.
         case kSwizzleColor_XformMode: {
-            size_t colorXformBytes = dstInfo.width() * sizeof(uint32_t);
-            fStorage.reset(SkAlign4(fSrcRowBytes) + colorXformBytes);
-            fSwizzlerSrcRow = fStorage.get();
-            fColorXformSrcRow = SkTAddOffset<uint32_t>(fSwizzlerSrcRow, SkAlign4(fSrcRowBytes));
+            const size_t colorXformBytes = dstInfo.width() * sizeof(uint32_t);
+            fStorage.reset(colorXformBytes);
+            fColorXformSrcRow = (uint32_t*) fStorage.get();
             break;
         }
     }
 }
 
-void SkPngCodec::applyXformRow(void* dst, const void* src, SkColorType colorType,
-                               SkAlphaType alphaType, int width) {
+void SkPngCodec::applyXformRow(void* dst, const void* src, SkColorType colorType) {
     switch (fXformMode) {
         case kSwizzleOnly_XformMode:
             fSwizzler->swizzle(dst, (const uint8_t*) src);
             break;
         case kColorOnly_XformMode:
-            fColorXform->apply(dst, (const uint32_t*) src, width, colorType, alphaType);
+            fColorXform->apply(dst, (const uint32_t*) src, fXformWidth, colorType, fXformAlphaType);
             break;
         case kSwizzleColor_XformMode:
             fSwizzler->swizzle(fColorXformSrcRow, (const uint8_t*) src);
-            fColorXform->apply(dst, fColorXformSrcRow, width, colorType, alphaType);
+            fColorXform->apply(dst, fColorXformSrcRow, fXformWidth, colorType, fXformAlphaType);
             break;
     }
 }
 
-class SkPngNormalCodec : public SkPngCodec {
+class SkPngNormalDecoder : public SkPngCodec {
 public:
-    SkPngNormalCodec(const SkEncodedInfo& encodedInfo, const SkImageInfo& imageInfo,
-            SkStream* stream, SkPngChunkReader* chunkReader, png_structp png_ptr,
-            png_infop info_ptr, int bitDepth)
-        : INHERITED(encodedInfo, imageInfo, stream, chunkReader, png_ptr, info_ptr, bitDepth, 1)
+    SkPngNormalDecoder(const SkEncodedInfo& info, const SkImageInfo& imageInfo, SkStream* stream,
+            SkPngChunkReader* reader, png_structp png_ptr, png_infop info_ptr, int bitDepth)
+        : INHERITED(info, imageInfo, stream, reader, png_ptr, info_ptr, bitDepth)
+        , fLinesDecoded(0)
+        , fDst(nullptr)
+        , fRowBytes(0)
+        , fFirstRow(0)
+        , fLastRow(0)
     {}
 
-    Result onStartScanlineDecode(const SkImageInfo& dstInfo, const Options& options,
-            SkPMColor ctable[], int* ctableCount) override {
-        if (!conversion_possible(dstInfo, this->getInfo()) ||
-            !this->initializeXforms(dstInfo, options, ctable, ctableCount))
-        {
-            return kInvalidConversion;
-        }
-
-        this->allocateStorage(dstInfo);
-        return kSuccess;
-    }
-
-    int readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, int count, int startRow)
-    override {
-        SkASSERT(0 == startRow);
-
-        // Assume that an error in libpng indicates an incomplete input.
-        int y = 0;
-        if (setjmp(png_jmpbuf((png_struct*)fPng_ptr))) {
-            SkCodecPrintf("Failed to read row.\n");
-            return y;
-        }
-
-        SkAlphaType xformAlphaType = select_alpha_xform(dstInfo.alphaType(),
-                                                        this->getInfo().alphaType());
-        int width = fSwizzler ? fSwizzler->swizzleWidth() : dstInfo.width();
-
-        for (; y < count; y++) {
-            png_read_row(fPng_ptr, fSwizzlerSrcRow, nullptr);
-            this->applyXformRow(dst, fSwizzlerSrcRow, dstInfo.colorType(), xformAlphaType, width);
+    static void AllRowsCallback(png_structp png_ptr, png_bytep row, png_uint_32 rowNum, int /*pass*/) {
+        GetDecoder(png_ptr)->allRowsCallback(row, rowNum);
+    }
+
+    static void RowCallback(png_structp png_ptr, png_bytep row, png_uint_32 rowNum, int /*pass*/) {
+        GetDecoder(png_ptr)->rowCallback(row, rowNum);
+    }
+private:
+    int                         fLinesDecoded; // FIXME: Move to baseclass?
+    void*                       fDst;
+    size_t                      fRowBytes;
+
+    // Variables for partial decode
+    int                         fFirstRow;  // FIXME: Move to baseclass?
+    int                         fLastRow;
+
+    typedef SkPngCodec INHERITED;
+
+    static SkPngNormalDecoder* GetDecoder(png_structp png_ptr) {
+        return static_cast<SkPngNormalDecoder*>(png_get_progressive_ptr(png_ptr));
+    }
+
+    Result decodeAllRows(void* dst, size_t rowBytes, int* rowsDecoded) override {
+        const int height = this->getInfo().height();
+        png_set_progressive_read_fn(this->png_ptr(), this, nullptr, AllRowsCallback, nullptr);
+        fDst = dst;
+        fRowBytes = rowBytes;
+
+        fLinesDecoded = 0;
+
+        this->processData();
+
+        if (fLinesDecoded == height) {
+            return SkCodec::kSuccess;
+        }
+
+        if (rowsDecoded) {
+            *rowsDecoded = fLinesDecoded;
+        }
+
+        return SkCodec::kIncompleteInput;
+    }
+
+    void allRowsCallback(png_bytep row, int rowNum) {
+        SkASSERT(rowNum - fFirstRow == fLinesDecoded);
+        fLinesDecoded++;
+        this->applyXformRow(fDst, row, this->dstInfo().colorType());
+        fDst = SkTAddOffset<void>(fDst, fRowBytes);
+    }
+
+    void setRange(int firstRow, int lastRow, void* dst, size_t rowBytes) override {
+        png_set_progressive_read_fn(this->png_ptr(), this, nullptr, RowCallback, nullptr);
+        fFirstRow = firstRow;
+        fLastRow = lastRow;
+        fDst = dst;
+        fRowBytes = rowBytes;
+        fLinesDecoded = 0;
+    }
+
+    SkCodec::Result decode(int* rowsDecoded) override {
+        this->processData();
+
+        if (fLinesDecoded == fLastRow - fFirstRow + 1) {
+            return SkCodec::kSuccess;
+        }
+
+        if (rowsDecoded) {
+            *rowsDecoded = fLinesDecoded;
+        }
+
+        return SkCodec::kIncompleteInput;
+    }
+
+    void rowCallback(png_bytep row, int rowNum) {
+        if (rowNum < fFirstRow) {
+            // Ignore this row.
+            return;
+        }
+
+        SkASSERT(rowNum <= fLastRow);
+
+        // If there is no swizzler, all rows are needed.
+        if (!this->swizzler() || this->swizzler()->rowNeeded(fLinesDecoded)) {
+            this->applyXformRow(fDst, row, this->dstInfo().colorType());
+            fDst = SkTAddOffset<void>(fDst, fRowBytes);
+        }
+
+        fLinesDecoded++;
+
+        if (rowNum == fLastRow) {
+            // Fake error to stop decoding scanlines.
+            longjmp(png_jmpbuf(this->png_ptr()), kStopDecoding);
+        }
+    }
+};
+
+class SkPngInterlacedDecoder : public SkPngCodec {
+public:
+    SkPngInterlacedDecoder(const SkEncodedInfo& info, const SkImageInfo& imageInfo,
+            SkStream* stream, SkPngChunkReader* reader, png_structp png_ptr, png_infop info_ptr,
+            int bitDepth, int numberPasses)
+        : INHERITED(info, imageInfo, stream, reader, png_ptr, info_ptr, bitDepth)
+        , fNumberPasses(numberPasses)
+        , fFirstRow(0)
+        , fLastRow(0)
+        , fLinesDecoded(0)
+        , fInterlacedComplete(false)
+        , fPng_rowbytes(0)
+    {}
+
+    static void InterlacedRowCallback(png_structp png_ptr, png_bytep row, png_uint_32 rowNum, int pass) {
+        auto decoder = static_cast<SkPngInterlacedDecoder*>(png_get_progressive_ptr(png_ptr));
+        decoder->interlacedRowCallback(row, rowNum, pass);
+    }
+
+private:
+    const int               fNumberPasses;
+    int                     fFirstRow;
+    int                     fLastRow;
+    void*                   fDst;
+    size_t                  fRowBytes;
+    int                     fLinesDecoded;
+    bool                    fInterlacedComplete;
+    size_t                  fPng_rowbytes;
+    SkAutoTMalloc<png_byte> fInterlaceBuffer;
+
+    typedef SkPngCodec INHERITED;
+
+    // FIXME: Currently sharing interlaced callback for all rows and subset. It's not
+    // as expensive as the subset version of non-interlaced, but it still does extra
+    // work.
+    void interlacedRowCallback(png_bytep row, int rowNum, int pass) {
+        if (rowNum < fFirstRow || rowNum > fLastRow) {
+            // Ignore this row
+            return;
+        }
+
+        png_bytep oldRow = fInterlaceBuffer.get() + (rowNum - fFirstRow) * fPng_rowbytes;
+        png_progressive_combine_row(this->png_ptr(), oldRow, row);
+
+        if (0 == pass) {
+            // The first pass initializes all rows.
+            SkASSERT(row);
+            SkASSERT(fLinesDecoded == rowNum - fFirstRow);
+            fLinesDecoded++;
+        } else {
+            SkASSERT(fLinesDecoded == fLastRow - fFirstRow + 1);
+            if (fNumberPasses - 1 == pass && rowNum == fLastRow) {
+                // Last pass, and we have read all of the rows we care about. Note that
+                // we do not care about reading anything beyond the end of the image (or
+                // beyond the last scanline requested).
+                fInterlacedComplete = true;
+                // Fake error to stop decoding scanlines.
+                longjmp(png_jmpbuf(this->png_ptr()), kStopDecoding);
+            }
+        }
+    }
+
+    SkCodec::Result decodeAllRows(void* dst, size_t rowBytes, int* rowsDecoded) override {
+        const int height = this->getInfo().height();
+        this->setUpInterlaceBuffer(height);
+        png_set_progressive_read_fn(this->png_ptr(), this, nullptr, InterlacedRowCallback, nullptr);
+
+        fFirstRow = 0;
+        fLastRow = height - 1;
+        fLinesDecoded = 0;
+
+        this->processData();
+
+        png_bytep srcRow = fInterlaceBuffer.get();
+        // FIXME: When resuming, this may rewrite rows that did not change.
+        for (int rowNum = 0; rowNum < fLinesDecoded; rowNum++) {
+            this->applyXformRow(dst, srcRow, this->dstInfo().colorType());
             dst = SkTAddOffset<void>(dst, rowBytes);
-        }
-
-        return y;
-    }
-
-    int onGetScanlines(void* dst, int count, size_t rowBytes) override {
-        return this->readRows(this->dstInfo(), dst, rowBytes, count, 0);
-    }
-
-    bool onSkipScanlines(int count) override {
-        if (setjmp(png_jmpbuf((png_struct*)fPng_ptr))) {
-            SkCodecPrintf("Failed to skip row.\n");
-            return false;
-        }
-
-        for (int row = 0; row < count; row++) {
-            png_read_row(fPng_ptr, fSwizzlerSrcRow, nullptr);
-        }
-        return true;
-    }
-
-    typedef SkPngCodec INHERITED;
+            srcRow = SkTAddOffset<png_byte>(srcRow, fPng_rowbytes);
+        }
+        if (fInterlacedComplete) {
+            return SkCodec::kSuccess;
+        }
+
+        if (rowsDecoded) {
+            *rowsDecoded = fLinesDecoded;
+        }
+
+        return SkCodec::kIncompleteInput;
+    }
+
+    void setRange(int firstRow, int lastRow, void* dst, size_t rowBytes) override {
+        // FIXME: We could skip rows in the interlace buffer that we won't put in the output.
+        this->setUpInterlaceBuffer(lastRow - firstRow + 1);
+        png_set_progressive_read_fn(this->png_ptr(), this, nullptr, InterlacedRowCallback, nullptr);
+        fFirstRow = firstRow;
+        fLastRow = lastRow;
+        fDst = dst;
+        fRowBytes = rowBytes;
+        fLinesDecoded = 0;
+    }
+
+    SkCodec::Result decode(int* rowsDecoded) override {
+        this->processData();
+
+        // Now apply Xforms on all the rows that were decoded.
+        if (!fLinesDecoded) {
+            return SkCodec::kIncompleteInput;
+        }
+        const int lastRow = fLinesDecoded + fFirstRow - 1;
+        SkASSERT(lastRow <= fLastRow);
+
+        // FIXME: For resuming interlace, we may swizzle a row that hasn't changed. But it
+        // may be too tricky/expensive to handle that correctly.
+        png_bytep srcRow = fInterlaceBuffer.get();
+        const int sampleY = this->swizzler() ? this->swizzler()->sampleY() : 1;
+        void* dst = fDst;
+        for (int rowNum = fFirstRow; rowNum <= lastRow; rowNum += sampleY) {
+            this->applyXformRow(dst, srcRow, this->dstInfo().colorType());
+            dst = SkTAddOffset<void>(dst, fRowBytes);
+            srcRow = SkTAddOffset<png_byte>(srcRow, fPng_rowbytes * sampleY);
+        }
+
+        if (fInterlacedComplete) {
+            return SkCodec::kSuccess;
+        }
+
+        if (rowsDecoded) {
+            *rowsDecoded = fLinesDecoded;
+        }
+        return SkCodec::kIncompleteInput;
+    }
+
+    void setUpInterlaceBuffer(int height) {
+        fPng_rowbytes = png_get_rowbytes(this->png_ptr(), this->info_ptr());
+        fInterlaceBuffer.reset(fPng_rowbytes * height);
+        fInterlacedComplete = false;
+    }
 };
 
-class SkPngInterlacedCodec : public SkPngCodec {
-public:
-    SkPngInterlacedCodec(const SkEncodedInfo& encodedInfo, const SkImageInfo& imageInfo,
-            SkStream* stream, SkPngChunkReader* chunkReader, png_structp png_ptr,
-            png_infop info_ptr, int bitDepth, int numberPasses)
-        : INHERITED(encodedInfo, imageInfo, stream, chunkReader, png_ptr, info_ptr, bitDepth,
-                    numberPasses)
-        , fCanSkipRewind(false)
-    {
-        SkASSERT(numberPasses != 1);
-    }
-
-    Result onStartScanlineDecode(const SkImageInfo& dstInfo, const Options& options,
-            SkPMColor ctable[], int* ctableCount) override {
-        if (!conversion_possible(dstInfo, this->getInfo()) ||
-            !this->initializeXforms(dstInfo, options, ctable, ctableCount))
-        {
-            return kInvalidConversion;
-        }
-
-        this->allocateStorage(dstInfo);
-        fCanSkipRewind = true;
-        return SkCodec::kSuccess;
-    }
-
-    int readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, int count, int startRow)
-    override {
-        if (setjmp(png_jmpbuf((png_struct*)fPng_ptr))) {
-            SkCodecPrintf("Failed to get scanlines.\n");
-            // FIXME (msarett): Returning 0 is pessimistic.  If we can complete a single pass,
-            // we may be able to report that all of the memory has been initialized.  Even if we
-            // fail on the first pass, we can still report than some scanlines are initialized.
-            return 0;
-        }
-
-        SkAutoTMalloc<uint8_t> storage(count * fSrcRowBytes);
-        uint8_t* srcRow;
-        for (int i = 0; i < fNumberPasses; i++) {
-            // Discard rows that we planned to skip.
-            for (int y = 0; y < startRow; y++){
-                png_read_row(fPng_ptr, fSwizzlerSrcRow, nullptr);
-            }
-            // Read rows we care about into storage.
-            srcRow = storage.get();
-            for (int y = 0; y < count; y++) {
-                png_read_row(fPng_ptr, srcRow, nullptr);
-                srcRow += fSrcRowBytes;
-            }
-            // Discard rows that we don't need.
-            for (int y = 0; y < this->getInfo().height() - startRow - count; y++) {
-                png_read_row(fPng_ptr, fSwizzlerSrcRow, nullptr);
-            }
-        }
-
-        SkAlphaType xformAlphaType = select_alpha_xform(dstInfo.alphaType(),
-                                                        this->getInfo().alphaType());
-        int width = fSwizzler ? fSwizzler->swizzleWidth() : dstInfo.width();
-        srcRow = storage.get();
-        for (int y = 0; y < count; y++) {
-            this->applyXformRow(dst, srcRow, dstInfo.colorType(), xformAlphaType, width);
-            srcRow = SkTAddOffset<uint8_t>(srcRow, fSrcRowBytes);
-            dst = SkTAddOffset<void>(dst, rowBytes);
-        }
-
-        return count;
-    }
-
-    int onGetScanlines(void* dst, int count, size_t rowBytes) override {
-        // rewind stream if have previously called onGetScanlines,
-        // since we need entire progressive image to get scanlines
-        if (fCanSkipRewind) {
-            // We already rewound in onStartScanlineDecode, so there is no reason to rewind.
-            // Next time onGetScanlines is called, we will need to rewind.
-            fCanSkipRewind = false;
-        } else {
-            // rewindIfNeeded resets fCurrScanline, since it assumes that start
-            // needs to be called again before scanline decoding. PNG scanline
-            // decoding is the exception, since it needs to rewind between
-            // calls to getScanlines. Keep track of fCurrScanline, to undo the
-            // reset.
-            const int currScanline = this->nextScanline();
-            // This method would never be called if currScanline is -1
-            SkASSERT(currScanline != -1);
-
-            if (!this->rewindIfNeeded()) {
-                return kCouldNotRewind;
-            }
-            this->updateCurrScanline(currScanline);
-        }
-
-        return this->readRows(this->dstInfo(), dst, rowBytes, count, this->nextScanline());
-    }
-
-    bool onSkipScanlines(int count) override {
-        // The non-virtual version will update fCurrScanline.
-        return true;
-    }
-
-    SkScanlineOrder onGetScanlineOrder() const override {
-        return kNone_SkScanlineOrder;
-    }
-
-private:
-    // FIXME: This imitates behavior in SkCodec::rewindIfNeeded. That function
-    // is called whenever some action is taken that reads the stream and
-    // therefore the next call will require a rewind. So it modifies a boolean
-    // to note that the *next* time it is called a rewind is needed.
-    // SkPngInterlacedCodec has an extra wrinkle - calling
-    // onStartScanlineDecode followed by onGetScanlines does *not* require a
-    // rewind. Since rewindIfNeeded does not have this flexibility, we need to
-    // add another layer.
-    bool                        fCanSkipRewind;
-
-    typedef SkPngCodec INHERITED;
-};
-
 // Reads the header and initializes the output fields, if not NULL.
 //
 // @param stream Input data. Will be read to get enough information to properly
 //      setup the codec.
 // @param chunkReader SkPngChunkReader, for reading unknown chunks. May be NULL.
 //      If not NULL, png_ptr will hold an *unowned* pointer to it. The caller is
 //      expected to continue to own it for the lifetime of the png_ptr.
 // @param outCodec Optional output variable.  If non-NULL, will be set to a new
 //      SkPngCodec on success.
 // @param png_ptrp Optional output variable. If non-NULL, will be set to a new
 //      png_structp on success.
 // @param info_ptrp Optional output variable. If non-NULL, will be set to a new
 //      png_infop on success;
 // @return true on success, in which case the caller is responsible for calling
 //      png_destroy_read_struct(png_ptrp, info_ptrp).
 //      If it returns false, the passed in fields (except stream) are unchanged.
 static bool read_header(SkStream* stream, SkPngChunkReader* chunkReader, SkCodec** outCodec,
                         png_structp* png_ptrp, png_infop* info_ptrp) {
     // The image is known to be a PNG. Decode enough to know the SkImageInfo.
     png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr,
                                                  sk_error_fn, sk_warning_fn);
     if (!png_ptr) {
         return false;
     }
 
-    AutoCleanPng autoClean(png_ptr);
+    AutoCleanPng autoClean(png_ptr, stream, chunkReader, outCodec);
 
     png_infop info_ptr = png_create_info_struct(png_ptr);
     if (info_ptr == nullptr) {
         return false;
     }
 
     autoClean.setInfoPtr(info_ptr);
 
     // FIXME: Could we use the return value of setjmp to specify the type of
     // error?
     if (setjmp(png_jmpbuf(png_ptr))) {
         return false;
     }
 
-    png_set_read_fn(png_ptr, static_cast<void*>(stream), sk_read_fn);
-
 #ifdef PNG_READ_UNKNOWN_CHUNKS_SUPPORTED
     // Hookup our chunkReader so we can see any user-chunks the caller may be interested in.
     // This needs to be installed before we read the png header.  Android may store ninepatch
     // chunks in the header.
     if (chunkReader) {
         png_set_keep_unknown_chunks(png_ptr, PNG_HANDLE_CHUNK_ALWAYS, (png_byte*)"", 0);
         png_set_read_user_chunk_fn(png_ptr, (png_voidp) chunkReader, sk_read_user_chunk);
     }
 #endif
 
-    // The call to png_read_info() gives us all of the information from the
-    // PNG file before the first IDAT (image data chunk).
-    png_read_info(png_ptr, info_ptr);
+    const bool decodedBounds = autoClean.decodeBounds();
+
+    if (!decodedBounds) {
+        return false;
+    }
+
+    // On success, decodeBounds releases ownership of png_ptr and info_ptr.
+    if (png_ptrp) {
+        *png_ptrp = png_ptr;
+    }
+    if (info_ptrp) {
+        *info_ptrp = info_ptr;
+    }
+
+    // decodeBounds takes care of setting outCodec
+    if (outCodec) {
+        SkASSERT(*outCodec);
+    }
+    return true;
+}
+
+void AutoCleanPng::infoCallback() {
     png_uint_32 origWidth, origHeight;
     int bitDepth, encodedColorType;
-    png_get_IHDR(png_ptr, info_ptr, &origWidth, &origHeight, &bitDepth,
+    png_get_IHDR(fPng_ptr, fInfo_ptr, &origWidth, &origHeight, &bitDepth,
                  &encodedColorType, nullptr, nullptr, nullptr);
 
     // Tell libpng to strip 16 bit/color files down to 8 bits/color.
     // TODO: Should we handle this in SkSwizzler?  Could this also benefit
     //       RAW decodes?
     if (bitDepth == 16) {
         SkASSERT(PNG_COLOR_TYPE_PALETTE != encodedColorType);
-        png_set_strip_16(png_ptr);
+        png_set_strip_16(fPng_ptr);
     }
 
     // Now determine the default colorType and alphaType and set the required transforms.
     // Often, we depend on SkSwizzler to perform any transforms that we need.  However, we
     // still depend on libpng for many of the rare and PNG-specific cases.
     SkEncodedInfo::Color color;
     SkEncodedInfo::Alpha alpha;
     switch (encodedColorType) {
         case PNG_COLOR_TYPE_PALETTE:
             // Extract multiple pixels with bit depths of 1, 2, and 4 from a single
             // byte into separate bytes (useful for paletted and grayscale images).
             if (bitDepth < 8) {
                 // TODO: Should we use SkSwizzler here?
-                png_set_packing(png_ptr);
+                png_set_packing(fPng_ptr);
             }
 
             color = SkEncodedInfo::kPalette_Color;
             // Set the alpha depending on if a transparency chunk exists.
-            alpha = png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS) ?
+            alpha = png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS) ?
                     SkEncodedInfo::kUnpremul_Alpha : SkEncodedInfo::kOpaque_Alpha;
             break;
         case PNG_COLOR_TYPE_RGB:
-            if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
+            if (png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS)) {
                 // Convert to RGBA if transparency chunk exists.
-                png_set_tRNS_to_alpha(png_ptr);
+                png_set_tRNS_to_alpha(fPng_ptr);
                 color = SkEncodedInfo::kRGBA_Color;
                 alpha = SkEncodedInfo::kBinary_Alpha;
             } else {
                 color = SkEncodedInfo::kRGB_Color;
                 alpha = SkEncodedInfo::kOpaque_Alpha;
             }
             break;
         case PNG_COLOR_TYPE_GRAY:
             // Expand grayscale images to the full 8 bits from 1, 2, or 4 bits/pixel.
             if (bitDepth < 8) {
                 // TODO: Should we use SkSwizzler here?
-                png_set_expand_gray_1_2_4_to_8(png_ptr);
+                png_set_expand_gray_1_2_4_to_8(fPng_ptr);
             }
 
-            if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
-                png_set_tRNS_to_alpha(png_ptr);
+            if (png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS)) {
+                png_set_tRNS_to_alpha(fPng_ptr);
                 color = SkEncodedInfo::kGrayAlpha_Color;
                 alpha = SkEncodedInfo::kBinary_Alpha;
             } else {
                 color = SkEncodedInfo::kGray_Color;
                 alpha = SkEncodedInfo::kOpaque_Alpha;
             }
             break;
         case PNG_COLOR_TYPE_GRAY_ALPHA:
             color = SkEncodedInfo::kGrayAlpha_Color;
             alpha = SkEncodedInfo::kUnpremul_Alpha;
             break;
         case PNG_COLOR_TYPE_RGBA:
             color = SkEncodedInfo::kRGBA_Color;
             alpha = SkEncodedInfo::kUnpremul_Alpha;
             break;
         default:
             // All the color types have been covered above.
             SkASSERT(false);
             color = SkEncodedInfo::kRGBA_Color;
             alpha = SkEncodedInfo::kUnpremul_Alpha;
     }
 
-    int numberPasses = png_set_interlace_handling(png_ptr);
+    const int numberPasses = png_set_interlace_handling(fPng_ptr);
 
-    autoClean.release();
-    if (png_ptrp) {
-        *png_ptrp = png_ptr;
-    }
-    if (info_ptrp) {
-        *info_ptrp = info_ptr;
-    }
-
-    if (outCodec) {
-        sk_sp<SkColorSpace> colorSpace = read_color_space(png_ptr, info_ptr);
+    fReadHeader = true;
+    // 1 tells libpng to save any extra data. We may be able to be more efficient by saving
+    // it ourselves.
+    png_process_data_pause(fPng_ptr, 1);
+    fDecodedBounds = true;
+    if (fOutCodec) {
+        SkASSERT(nullptr == *fOutCodec);
+        sk_sp<SkColorSpace> colorSpace = read_color_space(fPng_ptr, fInfo_ptr);
         if (!colorSpace) {
             // Treat unmarked pngs as sRGB.
             colorSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
         }
 
         SkEncodedInfo encodedInfo = SkEncodedInfo::Make(color, alpha, 8);
         SkImageInfo imageInfo = encodedInfo.makeImageInfo(origWidth, origHeight, colorSpace);
 
         if (SkEncodedInfo::kOpaque_Alpha == alpha) {
             png_color_8p sigBits;
-            if (png_get_sBIT(png_ptr, info_ptr, &sigBits)) {
+            if (png_get_sBIT(fPng_ptr, fInfo_ptr, &sigBits)) {
                 if (5 == sigBits->red && 6 == sigBits->green && 5 == sigBits->blue) {
                     // Recommend a decode to 565 if the sBIT indicates 565.
                     imageInfo = imageInfo.makeColorType(kRGB_565_SkColorType);
                 }
             }
         }
 
         if (1 == numberPasses) {
-            *outCodec = new SkPngNormalCodec(encodedInfo, imageInfo, stream,
-                    chunkReader, png_ptr, info_ptr, bitDepth);
+            *fOutCodec = new SkPngNormalDecoder(encodedInfo, imageInfo, fStream,
+                    fChunkReader, fPng_ptr, fInfo_ptr, bitDepth);
         } else {
-            *outCodec = new SkPngInterlacedCodec(encodedInfo, imageInfo, stream,
-                    chunkReader, png_ptr, info_ptr, bitDepth, numberPasses);
+            *fOutCodec = new SkPngInterlacedDecoder(encodedInfo, imageInfo, fStream,
+                    fChunkReader, fPng_ptr, fInfo_ptr, bitDepth, numberPasses);
         }
     }
 
-    return true;
+
+    // Release the pointers, which are now owned by the codec or the caller is expected to
+    // take ownership.
+    this->releasePngPtrs();
 }
 
 SkPngCodec::SkPngCodec(const SkEncodedInfo& encodedInfo, const SkImageInfo& imageInfo,
                        SkStream* stream, SkPngChunkReader* chunkReader, void* png_ptr,
-                       void* info_ptr, int bitDepth, int numberPasses)
+                       void* info_ptr, int bitDepth)
     : INHERITED(encodedInfo, imageInfo, stream)
     , fPngChunkReader(SkSafeRef(chunkReader))
     , fPng_ptr(png_ptr)
     , fInfo_ptr(info_ptr)
-    , fSwizzlerSrcRow(nullptr)
     , fColorXformSrcRow(nullptr)
-    , fSrcRowBytes(imageInfo.width() * (bytes_per_pixel(this->getEncodedInfo().bitsPerPixel())))
-    , fNumberPasses(numberPasses)
     , fBitDepth(bitDepth)
 {}
 
 SkPngCodec::~SkPngCodec() {
     this->destroyReadStruct();
 }
 
 void SkPngCodec::destroyReadStruct() {
     if (fPng_ptr) {
         // We will never have a nullptr fInfo_ptr with a non-nullptr fPng_ptr
@@ skipping 43 matching lines @@
         }
     }
 
     // Copy the color table to the client if they request kIndex8 mode.
     copy_color_table(dstInfo, fColorTable, ctable, ctableCount);
 
     this->initializeSwizzler(dstInfo, options);
     return true;
 }
 
+void SkPngCodec::initializeXformAlphaAndWidth() {
+    fXformAlphaType = select_alpha_xform(this->dstInfo().alphaType(), this->getInfo().alphaType());
+    fXformWidth = this->swizzler() ? this->swizzler()->swizzleWidth() : this->dstInfo().width();
+}
+
 static inline bool apply_xform_on_decode(SkColorType dstColorType, SkEncodedInfo::Color srcColor) {
     // We will apply the color xform when reading the color table, unless F16 is requested.
     return SkEncodedInfo::kPalette_Color != srcColor || kRGBA_F16_SkColorType == dstColorType;
 }
 
 void SkPngCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& options) {
     SkImageInfo swizzlerInfo = dstInfo;
     Options swizzlerOptions = options;
     fXformMode = kSwizzleOnly_XformMode;
     if (fColorXform && apply_xform_on_decode(dstInfo.colorType(), this->getEncodedInfo().color())) {
@@ skipping 52 matching lines @@
         !this->initializeXforms(dstInfo, options, ctable, ctableCount))
     {
         return kInvalidConversion;
     }
 
     if (options.fSubset) {
         return kUnimplemented;
     }
 
     this->allocateStorage(dstInfo);
-    int count = this->readRows(dstInfo, dst, rowBytes, dstInfo.height(), 0);
-    if (count > dstInfo.height()) {
-        *rowsDecoded = count;
-        return kIncompleteInput;
+    this->initializeXformAlphaAndWidth();
+    return this->decodeAllRows(dst, rowBytes, rowsDecoded);
+}
+
+SkCodec::Result SkPngCodec::onStartIncrementalDecode(const SkImageInfo& dstInfo,
+        void* dst, size_t rowBytes, const SkCodec::Options& options,
+        SkPMColor* ctable, int* ctableCount) {
+    if (!conversion_possible(dstInfo, this->getInfo()) ||
+        !this->initializeXforms(dstInfo, options, ctable, ctableCount))
+    {
+        return kInvalidConversion;
     }
 
+    this->allocateStorage(dstInfo);
+
+    int firstRow, lastRow;
+    if (options.fSubset) {
+        firstRow = options.fSubset->top();
+        lastRow = options.fSubset->bottom() - 1;
+    } else {
+        firstRow = 0;
+        lastRow = dstInfo.height() - 1;
+    }
+    this->setRange(firstRow, lastRow, dst, rowBytes);
     return kSuccess;
 }
 
+SkCodec::Result SkPngCodec::onIncrementalDecode(int* rowsDecoded) {
+    // FIXME: Only necessary on the first call.
+    this->initializeXformAlphaAndWidth();

[msarett, 2016/09/14 13:21:06]

Why can't you call this in startIncrementalDecode()?

[scroggo_chromium, 2016/09/14 14:54:13]

I almost called it inside initializeXforms (which is called by
startIncrementalDecode) but the reason incremental decoding needs to be split up
is because SkSampledCodec needs to set the sampling rate after calling
startIncrementalDecode. That changes the swizzle width (and possibly create's a
swizzler/sampler), so we need to do this after that. (We could also split up the
alpha type and width, but I kind of like keeping them together...)

As an alternative, we could make setting the sampling rate call back into the
codec to tell it to call this method again, although that adds some more
complexity.

+
+    return this->decode(rowsDecoded);
+}
+
 uint32_t SkPngCodec::onGetFillValue(SkColorType colorType) const {
     const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
     if (colorPtr) {
         return get_color_table_fill_value(colorType, colorPtr, 0);
     }
     return INHERITED::onGetFillValue(colorType);
 }
 
 SkCodec* SkPngCodec::NewFromStream(SkStream* stream, SkPngChunkReader* chunkReader) {
     SkAutoTDelete<SkStream> streamDeleter(stream);
 
-    SkCodec* outCodec;
+    SkCodec* outCodec = nullptr;

[msarett, 2016/09/14 13:21:06]

Why this change?  Did you intend to use a single return statement?

[scroggo_chromium, 2016/09/14 14:54:13]

I wanted to make sure that if I got a codec out of here it wasn't garbage.

     if (read_header(stream, chunkReader, &outCodec, nullptr, nullptr)) {
         // Codec has taken ownership of the stream.
         SkASSERT(outCodec);
         streamDeleter.release();
         return outCodec;
     }
 
     return nullptr;
 }