Fill incomplete images in SkCodec parent class

src/codec/SkCodec_libpng.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCodec_libpng.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
@@ skipping 452 matching lines @@
         return false;
     }
 
     fPng_ptr = png_ptr;
     fInfo_ptr = info_ptr;
     return true;
 }
 
 SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
                                         size_t dstRowBytes, const Options& options,
-                                        SkPMColor ctable[], int* ctableCount) {
+                                        SkPMColor ctable[], int* ctableCount,
+                                        int* incompleteScanlines) {
     if (!conversion_possible(requestedInfo, this->getInfo())) {
         return kInvalidConversion;
     }
     if (options.fSubset) {
         // Subsets are not supported.
         return kUnimplemented;
     }
     if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
     if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }
 
     // 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;
     }
     // FIXME: Could we use the return value of setjmp to specify the type of
     // error?
+    int row = 0;
     if (setjmp(png_jmpbuf(fPng_ptr))) {
-        SkCodecPrintf("setjmp long jump!\n");
-        return kInvalidInput;
+        // 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 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.  Making our buffer size
+        // smaller would help, but what impact would it have on regular decode performance?  Should
+        // we investigate using a different API than png_read_row(s)?  Chromium uses
+        // png_process_data.
+        *incompleteScanlines = requestedInfo.height() - row;
+        return kIncompleteInput;
     }
 
     SkASSERT(fNumberPasses != INVALID_NUMBER_PASSES);
     SkAutoMalloc storage;
     void* dstRow = dst;
     if (fNumberPasses > 1) {
         const int width = requestedInfo.width();
         const int height = requestedInfo.height();
         const int bpp = SkSwizzler::BytesPerPixel(fSrcConfig);
         const size_t srcRowBytes = width * bpp;
@@ skipping 13 matching lines @@
         // Now swizzle it.
         uint8_t* srcRow = base;
         for (int y = 0; y < height; y++) {
             fReallyHasAlpha |= !SkSwizzler::IsOpaque(fSwizzler->swizzle(dstRow, srcRow));
             dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
             srcRow += srcRowBytes;
         }
     } else {
         storage.reset(requestedInfo.width() * SkSwizzler::BytesPerPixel(fSrcConfig));
         uint8_t* srcRow = static_cast<uint8_t*>(storage.get());
-        for (int y = 0; y < requestedInfo.height(); y++) {
+        for (; row < requestedInfo.height(); row++) {
             png_read_rows(fPng_ptr, &srcRow, png_bytepp_NULL, 1);
             fReallyHasAlpha |= !SkSwizzler::IsOpaque(fSwizzler->swizzle(dstRow, srcRow));
             dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
         }
     }
 
     // FIXME: do we need substituteTranspColor? Note that we cannot do it for
     // scanline decoding, but we could do it here. Alternatively, we could do
     // it as we go, instead of in post-processing like SkPNGImageDecoder.
 
     if (setjmp(png_jmpbuf(fPng_ptr))) {
         // We've already read all the scanlines. This is a success.
         return kSuccess;
     }
 
     // read rest of file, and get additional comment and time chunks in info_ptr
     png_read_end(fPng_ptr, fInfo_ptr);
     return kSuccess;
 }
 
+uint32_t SkPngCodec::onGetFillValue(const SkImageInfo& dstInfo) const {
+    const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
+    if (colorPtr) {
+        return get_color_table_fill_value(dstInfo.colorType(), colorPtr, 0);
+    }
+    return INHERITED::onGetFillValue(dstInfo);
+}
+
 class SkPngScanlineDecoder : public SkScanlineDecoder {
 public:
-    SkPngScanlineDecoder(const SkImageInfo& srcInfo, SkPngCodec* codec)
-        : INHERITED(srcInfo)
+    SkPngScanlineDecoder(SkPngCodec* codec)
+        : INHERITED(codec, codec->getInfo())
         , fCodec(codec)
         , fHasAlpha(false)
+        , fRow(0)
     {}
 
     SkCodec::Result onStart(const SkImageInfo& dstInfo,
                             const SkCodec::Options& options,
                             SkPMColor ctable[], int* ctableCount) override {
         if (!fCodec->rewindIfNeeded()) {
             return SkCodec::kCouldNotRewind;
         }
 
         if (!conversion_possible(dstInfo, this->getInfo())) {
             return SkCodec::kInvalidConversion;
         }
 
         // Check to see if scaling was requested.
         if (dstInfo.dimensions() != this->getInfo().dimensions()) {
             if (!SkScaledCodec::DimensionsSupportedForSampling(this->getInfo(), dstInfo)) {
                 return SkCodec::kInvalidScale;   
             }
         }
 
         const SkCodec::Result result = fCodec->initializeSwizzler(dstInfo, options, ctable,
-                                                                  ctableCount);
+                ctableCount);
         if (result != SkCodec::kSuccess) {
             return result;
         }
 
         fHasAlpha = false;
         fStorage.reset(this->getInfo().width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig));
         fSrcRow = static_cast<uint8_t*>(fStorage.get());
 
         return SkCodec::kSuccess;
     }
 
-    SkCodec::Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
+    uint32_t onGetScanlines(void* dst, int count, size_t rowBytes) override {
+        // Assume that an error in libpng indicates an incomplete input.
         if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
             SkCodecPrintf("setjmp long jump!\n");
-            return SkCodec::kInvalidInput;
+            return fRow;

[scroggo, 2015/09/25 15:55:06]

Oh, wait, you said that "this works", and I thought you meant using a local
variable. That would be even better, if setjmp doesn't ruin it for us.

[msarett, 2015/10/01 12:44:52]

It does work...For some reason I only thought it would only work in
onGetPixels(), but clearly it will work here as well - we always start over at
zero.

I will fix.

         }
 
         void* dstRow = dst;
-        for (int i = 0; i < count; i++) {
+        for (fRow = 0; fRow < count; fRow++) {
             png_read_rows(fCodec->fPng_ptr, &fSrcRow, png_bytepp_NULL, 1);
             fHasAlpha |= !SkSwizzler::IsOpaque(fCodec->fSwizzler->swizzle(dstRow, fSrcRow));
             dstRow = SkTAddOffset<void>(dstRow, rowBytes);
         }
-        return SkCodec::kSuccess;
+        return fRow;
     }
 
-    SkCodec::Result onSkipScanlines(int count) override {
-        // FIXME: Could we use the return value of setjmp to specify the type of
-        // error?
+    uint32_t onSkipScanlines(int count) override {
+        // Assume that an error in libpng indicates an incomplete input.
         if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
             SkCodecPrintf("setjmp long jump!\n");
-            return SkCodec::kInvalidInput;
+            return fRow;
         }
         //there is a potential tradeoff of memory vs speed created by putting this in a loop. 
         //calling png_read_rows in a loop is insignificantly slower than calling it once with count 
         //as png_read_rows has it's own loop which calls png_read_row count times.
-        for (int i = 0; i < count; i++) {
+        for (fRow = 0; fRow < count; fRow++) {
             png_read_rows(fCodec->fPng_ptr, &fSrcRow, png_bytepp_NULL, 1);
         }
-        return SkCodec::kSuccess;
+        return fRow;
     }
 
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
     SkEncodedFormat onGetEncodedFormat() const override { 
         return kPNG_SkEncodedFormat;
     }
 
 
 private:
-    SkAutoTDelete<SkPngCodec>   fCodec;
+    SkPngCodec*                 fCodec; // Owned by parent class
     bool                        fHasAlpha;
+    uint32_t                    fRow;
     SkAutoMalloc                fStorage;
     uint8_t*                    fSrcRow;
 
     typedef SkScanlineDecoder INHERITED;
 };
 
 
 class SkPngInterlacedScanlineDecoder : public SkScanlineDecoder {
 public:
-    SkPngInterlacedScanlineDecoder(const SkImageInfo& srcInfo, SkPngCodec* codec)
-        : INHERITED(srcInfo)
+    SkPngInterlacedScanlineDecoder(SkPngCodec* codec)
+        : INHERITED(codec, codec->getInfo())
         , fCodec(codec)
         , fHasAlpha(false)
         , fCurrentRow(0)
-        , fHeight(srcInfo.height())
+        , fHeight(codec->getInfo().height())
         , fCanSkipRewind(false)
     {}
 
     SkCodec::Result onStart(const SkImageInfo& dstInfo,
                             const SkCodec::Options& options,
                             SkPMColor ctable[], int* ctableCount) override
     {
         if (!fCodec->rewindIfNeeded()) {
             return SkCodec::kCouldNotRewind;
         }
 
         if (!conversion_possible(dstInfo, this->getInfo())) {
             return SkCodec::kInvalidConversion;    
         }
 
         // Check to see if scaling was requested.
         if (dstInfo.dimensions() != this->getInfo().dimensions()) {
             if (!SkScaledCodec::DimensionsSupportedForSampling(this->getInfo(), dstInfo)) {
                 return SkCodec::kInvalidScale;
             }
         }
 
         const SkCodec::Result result = fCodec->initializeSwizzler(dstInfo, options, ctable,
-                                                                  ctableCount);
+                ctableCount);
         if (result != SkCodec::kSuccess) {
             return result;
         }
 
         fHasAlpha = false;
         fCurrentRow = 0;
         fHeight = dstInfo.height();
         fSrcRowBytes = this->getInfo().width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig);
         fGarbageRow.reset(fSrcRowBytes);
         fGarbageRowPtr = static_cast<uint8_t*>(fGarbageRow.get());
         fCanSkipRewind = true;
 
         return SkCodec::kSuccess;
     }
 
-    SkCodec::Result onGetScanlines(void* dst, int count, size_t dstRowBytes) override {
+    uint32_t 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 onStart, so there is no reason to rewind.
             // Next time onGetScanlines is called, we will need to rewind.
             fCanSkipRewind = false;
         } else if (!fCodec->rewindIfNeeded()) {
-            return SkCodec::kCouldNotRewind;
+            return 0;
         }
 
         if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
             SkCodecPrintf("setjmp long jump!\n");
-            return SkCodec::kInvalidInput;
+            // 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;
         }
         const int number_passes = png_set_interlace_handling(fCodec->fPng_ptr);
         SkAutoMalloc storage(count * fSrcRowBytes);
         uint8_t* storagePtr = static_cast<uint8_t*>(storage.get());
         uint8_t* srcRow;
         for (int i = 0; i < number_passes; i++) {
             //read rows we planned to skip into garbage row
             for (int y = 0; y < fCurrentRow; y++){
                 png_read_rows(fCodec->fPng_ptr, &fGarbageRowPtr, png_bytepp_NULL, 1);
             }
@@ skipping 10 matching lines @@
         }
         //swizzle the rows we care about
         srcRow = storagePtr;
         void* dstRow = dst;
         for (int y = 0; y < count; y++) {
             fHasAlpha |= !SkSwizzler::IsOpaque(fCodec->fSwizzler->swizzle(dstRow, srcRow));
             dstRow = SkTAddOffset<void>(dstRow, dstRowBytes);
             srcRow += fSrcRowBytes;
         }
         fCurrentRow += count;
-        return SkCodec::kSuccess;
+        return count;
     }
 
-    SkCodec::Result onSkipScanlines(int count) override {
+    uint32_t onSkipScanlines(int count) override {
         //when ongetScanlines is called it will skip to fCurrentRow
         fCurrentRow += count;
-        return SkCodec::kSuccess;
+        return count;
     }
 
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
     SkScanlineOrder onGetScanlineOrder() const override {
         return kNone_SkScanlineOrder;
     }
 
     SkEncodedFormat onGetEncodedFormat() const override { 
         return kPNG_SkEncodedFormat;
     }
 
 private:
-    SkAutoTDelete<SkPngCodec>   fCodec;
+    SkPngCodec*                 fCodec; // Owned by parent class
     bool                        fHasAlpha;
     int                         fCurrentRow;
     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 onStart
     // followed by onGetScanlines does *not* require a rewind. Since
     // rewindIfNeeded does not have this flexibility, we need to add another
     // layer.
     bool                        fCanSkipRewind;
 
     typedef SkScanlineDecoder INHERITED;
 };
 
 SkScanlineDecoder* SkPngCodec::NewSDFromStream(SkStream* stream) {
     SkAutoTDelete<SkPngCodec> codec (static_cast<SkPngCodec*>(SkPngCodec::NewFromStream(stream)));
     if (!codec) {
         return nullptr;
     }
 
     codec->fNumberPasses = png_set_interlace_handling(codec->fPng_ptr);
     SkASSERT(codec->fNumberPasses != INVALID_NUMBER_PASSES);
 
-    const SkImageInfo& srcInfo = codec->getInfo();
     if (codec->fNumberPasses > 1) {
         // interlaced image
-        return new SkPngInterlacedScanlineDecoder(srcInfo, codec.detach());
+        return new SkPngInterlacedScanlineDecoder(codec.detach());
     }
 
-    return new SkPngScanlineDecoder(srcInfo, codec.detach());
+    return new SkPngScanlineDecoder(codec.detach());
 }