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"
 #include "SkColorSpace.h"
 #include "SkColorTable.h"
 #include "SkMath.h"
 #include "SkOpts.h"
 #include "SkPngCodec.h"
 #include "SkSize.h"
 #include "SkStream.h"
 #include "SkSwizzler.h"
 #include "SkTemplates.h"
 #include "SkUtils.h"
 
 ///////////////////////////////////////////////////////////////////////////////
 // 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)
+        , 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;
+    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 (fDecodedBounds) {
+            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.
-// TODO: If we add support for non-native swizzles, we'll need to handle that here.
 bool SkPngCodec::createColorTable(SkColorType dstColorType, bool premultiply, int* ctableCount) {
 
     int numColors;
     png_color* palette;
     if (!png_get_PLTE(fPng_ptr, fInfo_ptr, &palette, &numColors)) {
         return false;
     }
 
     // Note: These are not necessarily SkPMColors.
     SkPMColor colorPtr[256];
@@ skipping 158 matching lines @@
 
 #endif // LIBPNG >= 1.6
 
     // Finally, what should we do if there is no color space information in the PNG?
     // The specification says that this indicates "gamma is unknown" and that the
     // "color is device dependent".  I'm assuming we can represent this with NULL.
     // But should we guess sRGB?  Most images are sRGB, even if they don't specify.
     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;
-}
-
-// Subclass of SkPngCodec which supports scanline decoding
-class SkPngScanlineDecoder : public SkPngCodec {
+class SkPngNormalDecoder : public SkPngCodec {
 public:
-    SkPngScanlineDecoder(int width, int height, const SkEncodedInfo& info, SkStream* stream,
-            SkPngChunkReader* chunkReader, png_structp png_ptr, png_infop info_ptr, int bitDepth,
+    SkPngNormalDecoder(int width, int height, const SkEncodedInfo& info, SkStream* stream,
+            SkPngChunkReader* reader, png_structp png_ptr, png_infop info_ptr, int bitDepth,
             sk_sp<SkColorSpace> colorSpace)
-        : INHERITED(width, height, info, stream, chunkReader, png_ptr, info_ptr, bitDepth, 1,
-                    colorSpace)
-        , fSrcRow(nullptr)
+        : INHERITED(width, height, info, stream, reader, png_ptr, info_ptr, bitDepth,
+                std::move(colorSpace))
+        , 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())) {
-            return kInvalidConversion;
-        }
-
-        const Result result = this->initializeSwizzler(dstInfo, options, ctable,
-                                                       ctableCount);
-        if (result != kSuccess) {
-            return result;
-        }
-
-        fStorage.reset(this->getInfo().width() *
-                (bytes_per_pixel(this->getEncodedInfo().bitsPerPixel())));
-        fSrcRow = fStorage.get();
-
-        return kSuccess;
-    }
-
-    int onGetScanlines(void* dst, int count, size_t rowBytes) override {
-        // Assume that an error in libpng indicates an incomplete input.
-        int row = 0;
-        if (setjmp(png_jmpbuf(this->png_ptr()))) {
-            SkCodecPrintf("setjmp long jump!\n");
-            return row;
-        }
-
-        void* dstRow = dst;
-        for (; row < count; row++) {
-            png_read_row(this->png_ptr(), fSrcRow, nullptr);
-            this->swizzler()->swizzle(dstRow, fSrcRow);
-            dstRow = SkTAddOffset<void>(dstRow, rowBytes);
-        }
-
-        return row;
-    }
-
-    bool onSkipScanlines(int count) override {
-        // Assume that an error in libpng indicates an incomplete input.
-        if (setjmp(png_jmpbuf(this->png_ptr()))) {
-            SkCodecPrintf("setjmp long jump!\n");
-            return false;
-        }
-
-        for (int row = 0; row < count; row++) {
-            png_read_row(this->png_ptr(), fSrcRow, nullptr);
-        }
-        return true;
-    }
-
+    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:
-    SkAutoTMalloc<uint8_t>      fStorage;
-    uint8_t*                    fSrcRow;
+    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->swizzler()->swizzle(fDst, row);
+        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 (this->swizzler()->rowNeeded(fLinesDecoded)) {
+            this->swizzler()->swizzle(fDst, row);
+            fDst = SkTAddOffset<void>(fDst, fRowBytes);
+        }
+
+        fLinesDecoded++;
+
+        if (rowNum == fLastRow) {
+            // Fake error to stop decoding scanlines.
+            longjmp(png_jmpbuf(this->png_ptr()), kStopDecoding);
+        }
+    }
 };
 
-
-class SkPngInterlacedScanlineDecoder : public SkPngCodec {
+class SkPngInterlacedDecoder : public SkPngCodec {
 public:
-    SkPngInterlacedScanlineDecoder(int width, int height, const SkEncodedInfo& info,
-            SkStream* stream, SkPngChunkReader* chunkReader, png_structp png_ptr,
-            png_infop info_ptr, int bitDepth, int numberPasses, sk_sp<SkColorSpace> colorSpace)
-        : INHERITED(width, height, info, stream, chunkReader, png_ptr, info_ptr, bitDepth,
-                    numberPasses, colorSpace)
-        , fHeight(-1)
-        , fCanSkipRewind(false)
-    {
-        SkASSERT(numberPasses != 1);
-    }
-
-    Result onStartScanlineDecode(const SkImageInfo& dstInfo, const Options& options,
-            SkPMColor ctable[], int* ctableCount) override {
-        if (!conversion_possible(dstInfo, this->getInfo())) {
-            return kInvalidConversion;
-        }
-
-        const Result result = this->initializeSwizzler(dstInfo, options, ctable,
-                                                       ctableCount);
-        if (result != kSuccess) {
-            return result;
-        }
-
-        fHeight = dstInfo.height();
-        // FIXME: This need not be called on a second call to onStartScanlineDecode.
-        fSrcRowBytes = this->getInfo().width() *
-                (bytes_per_pixel(this->getEncodedInfo().bitsPerPixel()));
-        fGarbageRow.reset(fSrcRowBytes);
-        fGarbageRowPtr = static_cast<uint8_t*>(fGarbageRow.get());
-        fCanSkipRewind = true;
-
-        return SkCodec::kSuccess;
-    }
-
-    int onGetScanlines(void* dst, int count, size_t dstRowBytes) 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;
+    SkPngInterlacedDecoder(int width, int height, const SkEncodedInfo& info, SkStream* stream,
+            SkPngChunkReader* reader, png_structp png_ptr, png_infop info_ptr, int bitDepth,
+            sk_sp<SkColorSpace> colorSpace, int numberPasses)
+        : INHERITED(width, height, info, stream, reader, png_ptr, info_ptr, bitDepth,
+                std::move(colorSpace))
+        , 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 {
-            // 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;
+            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);
             }
-            this->updateCurrScanline(currScanline);
-        }
-
-        if (setjmp(png_jmpbuf(this->png_ptr()))) {
-            SkCodecPrintf("setjmp long jump!\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* storagePtr = storage.get();
-        uint8_t* srcRow;
-        const int startRow = this->nextScanline();
-        for (int i = 0; i < this->numberPasses(); i++) {
-            // read rows we planned to skip into garbage row
-            for (int y = 0; y < startRow; y++){
-                png_read_row(this->png_ptr(), fGarbageRowPtr, nullptr);
-            }
-            // read rows we care about into buffer
-            srcRow = storagePtr;
-            for (int y = 0; y < count; y++) {
-                png_read_row(this->png_ptr(), srcRow, nullptr);
-                srcRow += fSrcRowBytes;
-            }
-            // read rows we don't want into garbage buffer
-            for (int y = 0; y < fHeight - startRow - count; y++) {
-                png_read_row(this->png_ptr(), fGarbageRowPtr, nullptr);
-            }
-        }
-        //swizzle the rows we care about
-        srcRow = storagePtr;
-        void* dstRow = dst;
-        for (int y = 0; y < count; y++) {
-            this->swizzler()->swizzle(dstRow, srcRow);
-            dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
-            srcRow += fSrcRowBytes;
-        }
-
-        return count;
-    }
-
-    bool onSkipScanlines(int count) override {
-        // The non-virtual version will update fCurrScanline.
-        return true;
-    }
-
-    SkScanlineOrder onGetScanlineOrder() const override {
-        return kNone_SkScanlineOrder;
-    }
-
-private:
-    int                         fHeight;
-    size_t                      fSrcRowBytes;
-    SkAutoMalloc                fGarbageRow;
-    uint8_t*                    fGarbageRowPtr;
-    // 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.
-    // SkPngInterlacedScanlineDecoder 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;
+        }
+    }
+
+    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->swizzler()->swizzle(dst, srcRow);
+            dst = SkTAddOffset<void>(dst, rowBytes);
+            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 call the callback 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()->sampleY();
+        void* dst = fDst;
+        for (int rowNum = fFirstRow; rowNum <= lastRow; rowNum += sampleY) {
+            this->swizzler()->swizzle(dst, srcRow);
+            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;
+    }
 };
 
 // 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);
+    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 info = SkEncodedInfo::Make(color, alpha, 8);
-
         if (1 == numberPasses) {
-            *outCodec = new SkPngScanlineDecoder(origWidth, origHeight, info, stream,
-                    chunkReader, png_ptr, info_ptr, bitDepth, colorSpace);
+            *fOutCodec = new SkPngNormalDecoder(origWidth, origHeight, info, fStream,
+                    fChunkReader, fPng_ptr, fInfo_ptr, bitDepth, std::move(colorSpace));
         } else {
-            *outCodec = new SkPngInterlacedScanlineDecoder(origWidth, origHeight, info, stream,
-                    chunkReader, png_ptr, info_ptr, bitDepth, numberPasses, colorSpace);
+            *fOutCodec = new SkPngInterlacedDecoder(origWidth, origHeight, info, fStream,
+                    fChunkReader, fPng_ptr, fInfo_ptr, bitDepth, std::move(colorSpace),
+                    numberPasses);
         }
     }
 
-    return true;
+    fDecodedBounds = 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);

[scroggo, 2016/05/26 21:53:32]

CMake build [1] is breaking with

/b/work/skia/src/codec/SkPngCodec.cpp: In member function 'void
AutoCleanPng::infoCallback()':
/b/work/skia/src/codec/SkPngCodec.cpp:792:39: error: 'png_process_data_pause'
was not declared in this scope
     png_process_data_pause(fPng_ptr, 1);

IIUC, our CMake build will just use the version of libpng that is installed on
the machine. This one happens to have an older version that does not have
png_process_data_pause.

This implementation requires png_process_data_pause in order to work, or the
ability to rewind the stream (further - we already require at least some limited
rewinding so that we can choose the format). Here are some options I see to
solve this:

- Don't support PNG with older versions of libpng (easiest option)
  - (Could be combined with upgrading the bot's version of libpng)
- Require being able to rewind further on older versions of libpng
  - Could be pretty straightforward, but the client will probably just always
fail to decode PNG
- Don't use an SkStream in SkCodec
  - I've been thinking about using something more like SegmentReader in Blink,
which would allow some limited rewinding
    that could support not being able to call png_process_data_pause. It's a big
change, though, and I think it makes more
    sense to make that switch after all the SkCodecs support suspending.

I toyed with starting with the old input method (png_read_info which calls
SkStream::read on exactly the bytes it needs) and then switching to
png_process_data for the rows. This doesn't work, though, because
png_process_data wants to start from the beginning.

Latest patch set goes with the first option.


[1]
https://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x8...

[msarett, 2016/05/27 12:52:31]

First option sounds good to me.  Looks like libpng has to be really, really old
to not have this function.

+
+    // Release the pointers, which are now owned by the codec or the caller is expected to
+    // take ownership.
+    this->releasePngPtrs();
 }
 
 SkPngCodec::SkPngCodec(int width, int height, const SkEncodedInfo& info, SkStream* stream,
                        SkPngChunkReader* chunkReader, png_structp png_ptr, png_infop info_ptr,
-                       int bitDepth, int numberPasses, sk_sp<SkColorSpace> colorSpace)
+                       int bitDepth, sk_sp<SkColorSpace> colorSpace)
     : INHERITED(width, height, info, stream, colorSpace)
     , fPngChunkReader(SkSafeRef(chunkReader))
     , fPng_ptr(png_ptr)
     , fInfo_ptr(info_ptr)
-    , 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
         SkASSERT(fInfo_ptr);
         png_destroy_read_struct(&fPng_ptr, &fInfo_ptr, nullptr);
         fPng_ptr = nullptr;
         fInfo_ptr = nullptr;
     }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // Getting the pixels
 ///////////////////////////////////////////////////////////////////////////////
 
-SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
-                                               const Options& options,
-                                               SkPMColor ctable[],
-                                               int* ctableCount) {
-    // FIXME: Could we use the return value of setjmp to specify the type of
-    // error?
+bool SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
+                                    const Options& options,
+                                    SkPMColor ctable[],
+                                    int* ctableCount) {
     if (setjmp(png_jmpbuf(fPng_ptr))) {
-        SkCodecPrintf("setjmp long jump!\n");
-        return kInvalidInput;
+        return false;
     }
     png_read_update_info(fPng_ptr, fInfo_ptr);
 
     if (SkEncodedInfo::kPalette_Color == this->getEncodedInfo().color()) {
         if (!this->createColorTable(requestedInfo.colorType(),
                 kPremul_SkAlphaType == requestedInfo.alphaType(), ctableCount)) {
-            return kInvalidInput;
+            return false;
         }
     }
 
     // Copy the color table to the client if they request kIndex8 mode
     copy_color_table(requestedInfo, fColorTable, ctable, ctableCount);
 
     // Create the swizzler.  SkPngCodec retains ownership of the color table.
     const SkPMColor* colors = get_color_ptr(fColorTable.get());
     fSwizzler.reset(SkSwizzler::CreateSwizzler(this->getEncodedInfo(), colors, requestedInfo,
             options));
     SkASSERT(fSwizzler);
 
-    return kSuccess;
+    return true;
 }
 
 
 bool SkPngCodec::onRewind() {
     // This sets fPng_ptr and fInfo_ptr to nullptr. If read_header
     // succeeds, they will be repopulated, and if it fails, they will
     // remain nullptr. Any future accesses to fPng_ptr and fInfo_ptr will
     // come through this function which will rewind and again attempt
     // to reinitialize them.
     this->destroyReadStruct();
@@ skipping 15 matching lines @@
                                         int* rowsDecoded) {
     if (!conversion_possible(requestedInfo, this->getInfo())) {
         return kInvalidConversion;
     }
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
     }
 
     // Note that ctable and ctableCount may be modified if there is a color table
-    const Result result = this->initializeSwizzler(requestedInfo, options, ctable, ctableCount);
-    if (result != kSuccess) {
-        return result;
+    if (!this->initializeSwizzler(requestedInfo, options, ctable, ctableCount)) {
+        return kInvalidInput;   // or parameters?
     }
 
-    const int width = requestedInfo.width();
-    const int height = requestedInfo.height();
-    const int bpp = bytes_per_pixel(this->getEncodedInfo().bitsPerPixel());
-    const size_t srcRowBytes = width * bpp;
+    return this->decodeAllRows(dst, dstRowBytes, rowsDecoded);
+}
 
-    // FIXME: Could we use the return value of setjmp to specify the type of
-    // error?
-    int row = 0;
-    // This must be declared above the call to setjmp to avoid memory leaks on incomplete images.
-    SkAutoTMalloc<uint8_t> storage;
-    if (setjmp(png_jmpbuf(fPng_ptr))) {
-        // Assume that any error that occurs while reading rows is caused by an incomplete input.
-        if (fNumberPasses > 1) {
-            // FIXME (msarett): Handle incomplete interlaced pngs.
-            return (row == height) ? kSuccess : kInvalidInput;
-        }
-        // FIXME: We do a poor job on incomplete pngs compared to other decoders (ex: Chromium,
-        // Ubuntu Image Viewer).  This is because we use the default buffer size in libpng (8192
-        // bytes), and if we can't fill the buffer, we immediately fail.
-        // For example, if we try to read 8192 bytes, and the image (incorrectly) only contains
-        // half that, which may have been enough to contain a non-zero number of lines, we fail
-        // when we could have decoded a few more lines and then failed.
-        // The read function that we provide for libpng has no way of indicating that we have
-        // made a partial read.
-        // Making our buffer size smaller improves our incomplete decodes, but what impact does
-        // it have on regular decode performance?  Should we investigate using a different API
-        // instead of png_read_row?  Chromium uses png_process_data.
-        *rowsDecoded = row;
-        return (row == height) ? kSuccess : kIncompleteInput;
+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())) {
+        return kInvalidConversion;
     }
 
-    // FIXME: We could split these out based on subclass.
-    void* dstRow = dst;
-    if (fNumberPasses > 1) {
-        storage.reset(height * srcRowBytes);
-        uint8_t* const base = storage.get();
-
-        for (int i = 0; i < fNumberPasses; i++) {
-            uint8_t* srcRow = base;
-            for (int y = 0; y < height; y++) {
-                png_read_row(fPng_ptr, srcRow, nullptr);
-                srcRow += srcRowBytes;
-            }
-        }
-
-        // Now swizzle it.
-        uint8_t* srcRow = base;
-        for (; row < height; row++) {
-            fSwizzler->swizzle(dstRow, srcRow);
-            dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
-            srcRow += srcRowBytes;
-        }
-    } else {
-        storage.reset(srcRowBytes);
-        uint8_t* srcRow = storage.get();
-        for (; row < height; row++) {
-            png_read_row(fPng_ptr, srcRow, nullptr);
-            fSwizzler->swizzle(dstRow, srcRow);
-            dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
-        }
+    if (!this->initializeSwizzler(dstInfo, options, ctable, ctableCount)) {
+        return kInvalidInput;
     }
 
-    // read rest of file, and get additional comment and time chunks in info_ptr
-    png_read_end(fPng_ptr, fInfo_ptr);
+    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;
+}
 
-    return kSuccess;
+SkCodec::Result SkPngCodec::onIncrementalDecode(int* rowsDecoded) {
+    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;
     if (read_header(stream, chunkReader, &outCodec, nullptr, nullptr)) {
         // Codec has taken ownership of the stream.
         SkASSERT(outCodec);
         streamDeleter.release();
         return outCodec;
     }
 
     return nullptr;
 }