Allow creating multiple scanline decoders.

Make getScanlineDecoder return a new object each time, which is
owned by the caller, and independent from any existing scanline
decoders and the SkCodec itself.

Since the SkCodec already contains the entire state machine, and it
is used by the scanline decoders, simply create a new SkCodec which
is now owned by the scanline decoder.

Move code that cleans up after using a scanline decoder into its
destructor

One side effect is that creating the first scanline decoder requires
a duplication of the stream and re-reading the header. (With some
more complexity/changes, we could pass the state machine to the
scanline decoder and make the SkCodec recreate its own state machine
instead.) The typical client of the scanline decoder (region decoder)
uses an SkMemoryStream, so the duplication is cheap, although we
should consider the extra time to reread the header/recreate the state
machine. (If/when we use the scanline decoder for other purposes,
where the stream may not be cheaply duplicated, we should consider
passing the state machine.)

One (intended) result of this change is that a client can create a
new scanline decoder in a new thread, and decode different pieces of
the image simultaneously.

In SkPngCodec::decodePalette, use fBitDepth rather than a parameter.

Review URL: https://codereview.chromium.org/1230033004

5 files changed, 69 insertions, 86 deletions

diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 760975ece0..a80b490274 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -16,7 +16,6 @@
 #ifndef SK_BUILD_FOR_ANDROID_FRAMEWORK
 #include "SkJpegCodec.h"
 #endif
-#include "SkScanlineDecoder.h"
 #include "SkStream.h"
 #include "SkWebpCodec.h"
 
@@ -80,12 +79,9 @@ SkCodec::SkCodec(const SkImageInfo& info, SkStream* stream)
     : fInfo(info)
     , fStream(stream)
     , fNeedsRewind(false)
-    , fScanlineDecoder(NULL)
 {}
 
-SkCodec::~SkCodec() {
-    SkDELETE(fScanlineDecoder);
-}
+SkCodec::~SkCodec() {}
 
 SkCodec::RewindState SkCodec::rewindIfNeeded() {
     // Store the value of fNeedsRewind so we can update it. Next read will
@@ -153,9 +149,7 @@ SkScanlineDecoder* SkCodec::getScanlineDecoder(const SkImageInfo& dstInfo, const
         options = &optsStorage;
     }
 
-    SkDELETE(fScanlineDecoder);
-    fScanlineDecoder = this->onGetScanlineDecoder(dstInfo, *options, ctable, ctableCount);
-    return fScanlineDecoder;
+    return this->onGetScanlineDecoder(dstInfo, *options, ctable, ctableCount);
 }
 
 SkScanlineDecoder* SkCodec::getScanlineDecoder(const SkImageInfo& dstInfo) {
diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index a5d3834313..699ab398d3 100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -120,7 +120,7 @@ typedef uint32_t (*PackColorProc)(U8CPU a, U8CPU r, U8CPU g, U8CPU b);
 
 // Note: SkColorTable claims to store SkPMColors, which is not necessarily
 // the case here.
-bool SkPngCodec::decodePalette(bool premultiply, int bitDepth, int* ctableCount) {
+bool SkPngCodec::decodePalette(bool premultiply, int* ctableCount) {
     int numPalette;
     png_colorp palette;
     png_bytep trans;
@@ -177,7 +177,7 @@ bool SkPngCodec::decodePalette(bool premultiply, int bitDepth, int* ctableCount)
         addressed by the bitdepth of the image and fill it with the last palette color or black if
         the palette is empty (really broken image).
     */
-    int colorCount = SkTMax(numPalette, 1 << SkTMin(bitDepth, 8));
+    int colorCount = SkTMax(numPalette, 1 << SkTMin(fBitDepth, 8));
     SkPMColor lastColor = index > 0 ? colorPtr[-1] : SkPackARGB32(0xFF, 0, 0, 0);
     for (; index < colorCount; index++) {
         *colorPtr++ = lastColor;
@@ -374,12 +374,6 @@ SkPngCodec::SkPngCodec(const SkImageInfo& info, SkStream* stream,
 {}
 
 SkPngCodec::~SkPngCodec() {
-    // First, ensure that the scanline decoder is left in a finished state.
-    SkAutoTDelete<SkScanlineDecoder> decoder(this->detachScanlineDecoder());
-    if (NULL != decoder) {
-        this->finish();
-    }
-
     this->destroyReadStruct();
 }
 
@@ -454,7 +448,7 @@ SkCodec::Result SkPngCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
         case kIndex_8_SkColorType:
             //decode palette to Skia format
             fSrcConfig = SkSwizzler::kIndex;
-            if (!this->decodePalette(kPremul_SkAlphaType == requestedInfo.alphaType(), fBitDepth,
+            if (!this->decodePalette(kPremul_SkAlphaType == requestedInfo.alphaType(),
                     ctableCount)) {
                 return kInvalidInput;
             }
@@ -523,11 +517,6 @@ SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void*
     if (!conversion_possible(requestedInfo, this->getInfo())) {
         return kInvalidConversion;
     }
-    // Do not allow a regular decode if the caller has asked for a scanline decoder
-    if (NULL != this->scanlineDecoder()) {
-        SkCodecPrintf("cannot getPixels() if a scanline decoder has been created\n");
-        return kInvalidParameters;
-    }
     if (requestedInfo.dimensions() != this->getInfo().dimensions()) {
         return kInvalidScale;
     }
@@ -596,6 +585,7 @@ void SkPngCodec::finish() {
         // We've already read all the scanlines. This is a success.
         return;
     }
+    // FIXME: Is this necessary?
     /* read rest of file, and get additional chunks in info_ptr - REQUIRED */
     png_read_end(fPng_ptr, fInfo_ptr);
 }
@@ -611,6 +601,10 @@ public:
         fSrcRow = static_cast<uint8_t*>(fStorage.get());
     }
 
+    ~SkPngScanlineDecoder() {
+        fCodec->finish();
+    }
+
     SkCodec::Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
         if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
             SkCodecPrintf("setjmp long jump!\n");
@@ -645,10 +639,10 @@ public:
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
 private:
-    SkPngCodec*         fCodec;     // Unowned.
-    bool                fHasAlpha;
-    SkAutoMalloc        fStorage;
-    uint8_t*            fSrcRow;
+    SkAutoTDelete<SkPngCodec>   fCodec;
+    bool                        fHasAlpha;
+    SkAutoMalloc                fStorage;
+    uint8_t*                    fSrcRow;
 
     typedef SkScanlineDecoder INHERITED;
 };
@@ -662,22 +656,21 @@ public:
         , fHasAlpha(false)
         , fCurrentRow(0)
         , fHeight(dstInfo.height())
-        , fRewindNeeded(false)
     {
         fSrcRowBytes = dstInfo.width() * SkSwizzler::BytesPerPixel(fCodec->fSrcConfig);
         fGarbageRow.reset(fSrcRowBytes);
         fGarbageRowPtr = static_cast<uint8_t*>(fGarbageRow.get());
     }
 
+    ~SkPngInterlacedScanlineDecoder() {
+        fCodec->finish();
+    }
+
     SkCodec::Result 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 (fRewindNeeded) {
-            if(false == fCodec->handleRewind()) {
-                return SkCodec::kCouldNotRewind;
-            }
-        } else {
-            fRewindNeeded = true;
+        if (!fCodec->handleRewind()) {
+            return SkCodec::kCouldNotRewind;
         }
         if (setjmp(png_jmpbuf(fCodec->fPng_ptr))) {
             SkCodecPrintf("setjmp long jump!\n");
@@ -724,14 +717,13 @@ public:
     bool onReallyHasAlpha() const override { return fHasAlpha; }
 
 private:
-    SkPngCodec*         fCodec;     // Unowned.
-    bool                fHasAlpha;
-    int                 fCurrentRow;
-    int                 fHeight;
-    size_t              fSrcRowBytes;
-    bool                fRewindNeeded;
-    SkAutoMalloc        fGarbageRow;
-    uint8_t*            fGarbageRowPtr;
+    SkAutoTDelete<SkPngCodec>   fCodec;
+    bool                        fHasAlpha;
+    int                         fCurrentRow;
+    int                         fHeight;
+    size_t                      fSrcRowBytes;
+    SkAutoMalloc                fGarbageRow;
+    uint8_t*                    fGarbageRowPtr;
 
     typedef SkScanlineDecoder INHERITED;
 };
@@ -747,25 +739,31 @@ SkScanlineDecoder* SkPngCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
     if (dstInfo.dimensions() != this->getInfo().dimensions()) {
         return NULL;
     }
-    if (!this->handleRewind()) {
+    // Create a new SkPngCodec, to be owned by the scanline decoder.
+    SkStream* stream = this->stream()->duplicate();
+    if (!stream) {
+        return NULL;
+    }
+    SkAutoTDelete<SkPngCodec> codec (static_cast<SkPngCodec*>(SkPngCodec::NewFromStream(stream)));
+    if (!codec) {
         return NULL;
     }
 
     // Note: We set dst to NULL since we do not know it yet. rowBytes is not needed,
     // since we'll be manually updating the dstRow, but the SkSwizzler requires it to
     // be at least dstInfo.minRowBytes.
-    if (this->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), options, ctable,
+    if (codec->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), options, ctable,
             ctableCount) != kSuccess) {
         SkCodecPrintf("failed to initialize the swizzler.\n");
         return NULL;
     }
 
-    SkASSERT(fNumberPasses != INVALID_NUMBER_PASSES);
-    if (fNumberPasses > 1) {
+    SkASSERT(codec->fNumberPasses != INVALID_NUMBER_PASSES);
+    if (codec->fNumberPasses > 1) {
         // interlaced image
-        return SkNEW_ARGS(SkPngInterlacedScanlineDecoder, (dstInfo, this));
+        return SkNEW_ARGS(SkPngInterlacedScanlineDecoder, (dstInfo, codec.detach()));
     }
 
-    return SkNEW_ARGS(SkPngScanlineDecoder, (dstInfo, this));
+    return SkNEW_ARGS(SkPngScanlineDecoder, (dstInfo, codec.detach()));
 }
 
diff --git a/src/codec/SkCodec_libpng.h b/src/codec/SkCodec_libpng.h
index c0fee74e96..cba5c2ad30 100644
--- a/src/codec/SkCodec_libpng.h
+++ b/src/codec/SkCodec_libpng.h
@@ -26,6 +26,9 @@ public:
     // Assumes IsPng was called and returned true.
     static SkCodec* NewFromStream(SkStream*);
     static bool IsPng(SkStream*);
+
+    virtual ~SkPngCodec();
+
 protected:
     Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, SkPMColor*, int*)
             override;
@@ -47,7 +50,6 @@ private:
     int                         fBitDepth;
 
     SkPngCodec(const SkImageInfo&, SkStream*, png_structp, png_infop, int);
-    ~SkPngCodec();
 
 
     // Helper to set up swizzler and color table. Also calls png_read_update_info.
@@ -56,7 +58,7 @@ private:
 
     // Calls rewindIfNeeded and returns true if the decoder can continue.
     bool handleRewind();
-    bool decodePalette(bool premultiply, int bitDepth, int* ctableCount);
+    bool decodePalette(bool premultiply, int* ctableCount);
     void finish();
     void destroyReadStruct();
 
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 858e305b06..28e1e12bfc 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -303,13 +303,6 @@ bool SkJpegCodec::scaleToDimensions(uint32_t dstWidth, uint32_t dstHeight) {
 SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
                                          void* dst, size_t dstRowBytes,
                                          const Options& options, SkPMColor*, int*) {
-
-    // Do not allow a regular decode if the caller has asked for a scanline decoder
-    if (NULL != this->scanlineDecoder()) {
-        return fDecoderMgr->returnFailure("cannot getPixels() if a scanline decoder has been"
-                "created", kInvalidParameters);
-    }
-
     // Rewind the stream if needed
     if (!this->handleRewind()) {
         return fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind);
@@ -380,25 +373,6 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
 }
 
 /*
- * We override the destructor to ensure that the scanline decoder is left in a
- * finished state before destroying the decode manager.
- */
-SkJpegCodec::~SkJpegCodec() {
-    SkAutoTDelete<SkScanlineDecoder> decoder(this->detachScanlineDecoder());
-    if (NULL != decoder) {
-        if (setjmp(fDecoderMgr->getJmpBuf())) {
-            SkCodecPrintf("setjmp: Error in libjpeg finish_decompress\n");
-            return;
-        }
-
-        // We may not have decoded the entire image.  Prevent libjpeg-turbo from failing on a
-        // partial decode.
-        fDecoderMgr->dinfo()->output_scanline = this->getInfo().height();
-        turbo_jpeg_finish_decompress(fDecoderMgr->dinfo());
-    }
-}
-
-/*
  * Enable scanline decoding for jpegs
  */
 class SkJpegScanlineDecoder : public SkScanlineDecoder {
@@ -408,6 +382,18 @@ public:
         , fCodec(codec)
     {}
 
+    virtual ~SkJpegScanlineDecoder() {
+        if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
+            SkCodecPrintf("setjmp: Error in libjpeg finish_decompress\n");
+            return;
+        }
+
+        // We may not have decoded the entire image.  Prevent libjpeg-turbo from failing on a
+        // partial decode.
+        fCodec->fDecoderMgr->dinfo()->output_scanline = fCodec->getInfo().height();
+        turbo_jpeg_finish_decompress(fCodec->fDecoderMgr->dinfo());
+    }
+
     SkCodec::Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
         // Set the jump location for libjpeg errors
         if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
@@ -460,7 +446,7 @@ public:
     }
 
 private:
-    SkJpegCodec* fCodec; // unowned
+    SkAutoTDelete<SkJpegCodec> fCodec;
 
     typedef SkScanlineDecoder INHERITED;
 };
@@ -480,24 +466,33 @@ SkScanlineDecoder* SkJpegCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
         return NULL;
     }
 
+    SkStream* stream = this->stream()->duplicate();
+    if (!stream) {
+        return NULL;
+    }
+    SkAutoTDelete<SkJpegCodec> codec(static_cast<SkJpegCodec*>(SkJpegCodec::NewFromStream(stream)));
+    if (!codec) {
+        return NULL;
+    }
+
     // Check if we can decode to the requested destination and set the output color space
-    if (!this->setOutputColorSpace(dstInfo)) {
+    if (!codec->setOutputColorSpace(dstInfo)) {
         SkCodecPrintf("Cannot convert to output type\n");
         return NULL;
     }
 
     // Perform the necessary scaling
-    if (!this->scaleToDimensions(dstInfo.width(), dstInfo.height())) {
+    if (!codec->scaleToDimensions(dstInfo.width(), dstInfo.height())) {
         SkCodecPrintf("Cannot scale to output dimensions\n");
         return NULL;
     }
 
     // Now, given valid output dimensions, we can start the decompress
-    if (!turbo_jpeg_start_decompress(fDecoderMgr->dinfo())) {
+    if (!turbo_jpeg_start_decompress(codec->fDecoderMgr->dinfo())) {
         SkCodecPrintf("start decompress failed\n");
         return NULL;
     }
 
     // Return the new scanline decoder
-    return SkNEW_ARGS(SkJpegScanlineDecoder, (dstInfo, this));
+    return SkNEW_ARGS(SkJpegScanlineDecoder, (dstInfo, codec.detach()));
 }
diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h
index 122c6e4453..1844269e6b 100644
--- a/src/codec/SkJpegCodec.h
+++ b/src/codec/SkJpegCodec.h
@@ -95,12 +95,6 @@ private:
     SkJpegCodec(const SkImageInfo& srcInfo, SkStream* stream, JpegDecoderMgr* decoderMgr);
 
     /*
-     * Explicit destructor is used to ensure that the scanline decoder is deleted
-     * before the decode manager.
-     */
-    ~SkJpegCodec() override;
-
-    /*
      * Handles rewinding the input stream if it is necessary
      */
     bool handleRewind();