AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

In the case of subset decodes, we will often not decode to

Browse Source 
the bottom of the image.  In our code before this patch,
this would result in cleanup code in onFinish() never being
called.

We can allow subclasses to take ownership of the
SkScanlineDecoder in order to make sure that it is
finished/deleted before deleting the decode manager.

BUG=skia:

Review URL: https://codereview.chromium.org/1212593003
This commit is contained in:
msarett 2015-07-01 06:50:35 -07:00 committed by Commit bot
parent f06c389f0f
commit c0e80c139e
6 changed files with 85 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
include/codec/SkCodec.h
View File

@ -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;

36
include/codec/SkScanlineDecoder.h
View File

@ -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

22
src/codec/SkCodec_libpng.cpp
View File

@ -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;
};

36
src/codec/SkJpegCodec.cpp
View File

@ -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
@ -387,6 +393,25 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
return kSuccess;
}
/*
* 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
*/
@ -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;

6
src/codec/SkJpegCodec.h
View File

@ -94,6 +94,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
*/

4
tests/CodexTest.cpp
View File

@ -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);