aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--include/codec/SkCodec.h23
-rw-r--r--include/codec/SkScanlineDecoder.h36
-rw-r--r--src/codec/SkCodec_libpng.cpp22
-rw-r--r--src/codec/SkJpegCodec.cpp36
-rw-r--r--src/codec/SkJpegCodec.h6
-rw-r--r--tests/CodexTest.cpp4
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);