diff options
-rw-r--r-- | include/codec/SkCodec.h | 23 | ||||
-rw-r--r-- | include/codec/SkScanlineDecoder.h | 36 | ||||
-rw-r--r-- | src/codec/SkCodec_libpng.cpp | 22 | ||||
-rw-r--r-- | src/codec/SkJpegCodec.cpp | 36 | ||||
-rw-r--r-- | src/codec/SkJpegCodec.h | 6 | ||||
-rw-r--r-- | tests/CodexTest.cpp | 4 |
6 files changed, 85 insertions, 42 deletions
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h index 2e47a261f3..e462be2e7d 100644 --- a/include/codec/SkCodec.h +++ b/include/codec/SkCodec.h @@ -168,15 +168,32 @@ protected: */ RewindState SK_WARN_UNUSED_RESULT rewindIfNeeded(); - /* - * + /** * Get method for the input stream - * */ SkStream* stream() { return fStream.get(); } + /** + * If the codec has a scanline decoder, return it (no ownership change occurs) + * else return NULL. + * The returned decoder is valid while the codec exists and the client has not + * created a new scanline decoder. + */ + SkScanlineDecoder* scanlineDecoder() { + return fScanlineDecoder.get(); + } + + /** + * Allow the codec subclass to detach and take ownership of the scanline decoder. + * This will likely be used when the scanline decoder needs to be destroyed + * in the destructor of the subclass. + */ + SkScanlineDecoder* detachScanlineDecoder() { + return fScanlineDecoder.detach(); + } + private: SkAutoTDelete<SkStream> fStream; bool fNeedsRewind; diff --git a/include/codec/SkScanlineDecoder.h b/include/codec/SkScanlineDecoder.h index d7f73dda56..eac31ef40f 100644 --- a/include/codec/SkScanlineDecoder.h +++ b/include/codec/SkScanlineDecoder.h @@ -15,8 +15,20 @@ class SkScanlineDecoder : public SkNoncopyable { public: - // Note for implementations: An SkScanlineDecoder will be deleted by (and - // therefore *before*) its associated SkCodec, in case the order matters. + /** + * Clean up after reading/skipping scanlines. + * + * It is possible that not all scanlines will have been read/skipped. In + * fact, in the case of subset decodes, it is likely that there will be + * scanlines at the bottom of the image that have been ignored. + * + * Note for implementations: An SkScanlineDecoder will be deleted by (and + * therefore *before*) its associated SkCodec, in case the order matters. + * However, while the SkCodec base class maintains ownership of the + * SkScanlineDecoder, the subclass will be deleted before the scanline + * decoder. If this is an issue, detachScanlineDecoder() provides + * a means for the subclass to take ownership of the SkScanlineDecoder. + */ virtual ~SkScanlineDecoder() {} /** @@ -34,7 +46,7 @@ public: return SkImageGenerator::kInvalidParameters; } const SkImageGenerator::Result result = this->onGetScanlines(dst, countLines, rowBytes); - this->checkForFinish(countLines); + fCurrScanline += countLines; return result; } @@ -54,7 +66,7 @@ public: return SkImageGenerator::kInvalidParameters; } const SkImageGenerator::Result result = this->onSkipScanlines(countLines); - this->checkForFinish(countLines); + fCurrScanline += countLines; return result; } @@ -97,21 +109,5 @@ private: virtual SkImageGenerator::Result onGetScanlines(void* dst, int countLines, size_t rowBytes) = 0; - /** - * Called after any set of scanlines read/skipped. Updates fCurrScanline, - * and, if we are at the end, calls onFinish(). - */ - void checkForFinish(int countLines) { - fCurrScanline += countLines; - if (fCurrScanline >= fDstInfo.height()) { - this->onFinish(); - } - } - - /** - * This function will be called after reading/skipping all scanlines to do - * any necessary cleanups. - */ - virtual void onFinish() {} // Default does nothing. }; #endif // SkScanlineDecoder_DEFINED diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp index 444e2ad87e..32004366f8 100644 --- a/src/codec/SkCodec_libpng.cpp +++ b/src/codec/SkCodec_libpng.cpp @@ -372,6 +372,12 @@ 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(); } @@ -514,6 +520,12 @@ bool SkPngCodec::handleRewind() { SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst, size_t rowBytes, const Options& options, SkPMColor ctable[], int* ctableCount) { + // 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 (!this->handleRewind()) { return kCouldNotRewind; } @@ -633,10 +645,6 @@ public: return SkImageGenerator::kSuccess; } - void onFinish() override { - fCodec->finish(); - } - bool onReallyHasAlpha() const override { return fHasAlpha; } private: @@ -716,10 +724,6 @@ public: return SkImageGenerator::kSuccess; } - void onFinish() override { - fCodec->finish(); - } - bool onReallyHasAlpha() const override { return fHasAlpha; } private: @@ -734,7 +738,7 @@ private: - + typedef SkScanlineDecoder INHERITED; }; diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp index 2eaff1a9a6..9c9bd77c13 100644 --- a/src/codec/SkJpegCodec.cpp +++ b/src/codec/SkJpegCodec.cpp @@ -297,9 +297,15 @@ 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()) { - fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind); + return fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind); } // Get a pointer to the decompress info since we will use it quite frequently @@ -388,6 +394,25 @@ 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(); + jpeg_finish_decompress(fDecoderMgr->dinfo()); + } +} + +/* * Enable scanline decoding for jpegs */ class SkJpegScanlineDecoder : public SkScanlineDecoder { @@ -443,15 +468,6 @@ public: return SkImageGenerator::kSuccess; } - void onFinish() override { - if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) { - SkCodecPrintf("setjmp: Error in libjpeg finish_decompress\n"); - return; - } - - jpeg_finish_decompress(fCodec->fDecoderMgr->dinfo()); - } - private: SkJpegCodec* fCodec; // unowned SkAutoMalloc fStorage; diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h index 87c925d9d1..f39bc49910 100644 --- a/src/codec/SkJpegCodec.h +++ b/src/codec/SkJpegCodec.h @@ -95,6 +95,12 @@ 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(); diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp index 5c969908b9..0edd3cf907 100644 --- a/tests/CodexTest.cpp +++ b/tests/CodexTest.cpp @@ -72,6 +72,10 @@ static void check(skiatest::Reporter* r, if (supportsScanlineDecoding) { bm.eraseColor(SK_ColorYELLOW); REPORTER_ASSERT(r, scanlineDecoder); + + // Regular decodes should be disabled after creating a scanline decoder + result = codec->getPixels(info, bm.getPixels(), bm.rowBytes(), NULL, NULL, NULL); + REPORTER_ASSERT(r, SkImageGenerator::kInvalidParameters == result); for (int y = 0; y < info.height(); y++) { result = scanlineDecoder->getScanlines(bm.getAddr(0, y), 1, 0); REPORTER_ASSERT(r, result == SkImageGenerator::kSuccess); |