Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(74)

Unified Diff: src/images/SkImageDecoder_libjpeg.cpp

Issue 647023006: Qualify the return value of SkImageDecoder::decode (Closed) Base URL: https://skia.googlesource.com/skia.git/+/master
Patch Set: fix a sample Created 6 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/images/SkImageDecoder_libico.cpp ('k') | src/images/SkImageDecoder_libpng.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/images/SkImageDecoder_libjpeg.cpp
diff --git a/src/images/SkImageDecoder_libjpeg.cpp b/src/images/SkImageDecoder_libjpeg.cpp
index 21e96be2d3049e091a48ef1728bf7554522ad684..10466640046ce60f9876b6714312cc7a213e95f7 100644
--- a/src/images/SkImageDecoder_libjpeg.cpp
+++ b/src/images/SkImageDecoder_libjpeg.cpp
@@ -238,7 +238,7 @@ protected:
virtual bool onBuildTileIndex(SkStreamRewindable *stream, int *width, int *height) SK_OVERRIDE;
virtual bool onDecodeSubset(SkBitmap* bitmap, const SkIRect& rect) SK_OVERRIDE;
#endif
- virtual bool onDecode(SkStream* stream, SkBitmap* bm, Mode) SK_OVERRIDE;
+ virtual Result onDecode(SkStream* stream, SkBitmap* bm, Mode) SK_OVERRIDE;
virtual bool onDecodeYUV8Planes(SkStream* stream, SkISize componentSizes[3],
void* planes[3], size_t rowBytes[3],
SkYUVColorSpace* colorSpace) SK_OVERRIDE;
@@ -325,9 +325,11 @@ static bool skip_src_rows_tile(jpeg_decompress_struct* cinfo,
}
#endif
+///////////////////////////////////////////////////////////////////////////////
+
// This guy exists just to aid in debugging, as it allows debuggers to just
// set a break-point in one place to see all error exists.
-static bool return_false(const jpeg_decompress_struct& cinfo,
+static void print_jpeg_decoder_errors(const jpeg_decompress_struct& cinfo,
int width, int height, const char caller[]) {
if (!(c_suppressJPEGImageDecoderErrors)) {
char buffer[JMSG_LENGTH_MAX];
@@ -335,14 +337,28 @@ static bool return_false(const jpeg_decompress_struct& cinfo,
SkDebugf("libjpeg error %d <%s> from %s [%d %d]\n",
cinfo.err->msg_code, buffer, caller, width, height);
}
- return false; // must always return false
+}
+
+static bool return_false(const jpeg_decompress_struct& cinfo,
+ const char caller[]) {
+ print_jpeg_decoder_errors(cinfo, 0, 0, caller);
+ return false;
}
static bool return_false(const jpeg_decompress_struct& cinfo,
const SkBitmap& bm, const char caller[]) {
- return return_false(cinfo, bm.width(), bm.height(), caller);
+ print_jpeg_decoder_errors(cinfo, bm.width(), bm.height(), caller);
+ return false;
+}
+
+static SkImageDecoder::Result return_failure(const jpeg_decompress_struct& cinfo,
+ const SkBitmap& bm, const char caller[]) {
+ print_jpeg_decoder_errors(cinfo, bm.width(), bm.height(), caller);
+ return SkImageDecoder::kFailure;
}
+///////////////////////////////////////////////////////////////////////////////
+
// Convert a scanline of CMYK samples to RGBX in place. Note that this
// method moves the "scanline" pointer in its processing
static void convert_CMYK_to_RGB(uint8_t* scanline, unsigned int width) {
@@ -491,7 +507,6 @@ static void adjust_out_color_space_and_dither(jpeg_decompress_struct* cinfo,
#endif
}
-
/**
Sets all pixels in given bitmap to SK_ColorWHITE for all rows >= y.
Used when decoding fails partway through reading scanlines to fill
@@ -537,7 +552,7 @@ static bool get_src_config(const jpeg_decompress_struct& cinfo,
return true;
}
-bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
+SkImageDecoder::Result SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
#ifdef TIME_DECODE
SkAutoTime atm("JPEG Decode");
#endif
@@ -553,7 +568,7 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
// All objects need to be instantiated before this setjmp call so that
// they will be cleaned up properly if an error occurs.
if (setjmp(errorManager.fJmpBuf)) {
- return return_false(cinfo, *bm, "setjmp");
+ return return_failure(cinfo, *bm, "setjmp");
}
initialize_info(&cinfo, &srcManager);
@@ -561,7 +576,7 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
int status = jpeg_read_header(&cinfo, true);
if (status != JPEG_HEADER_OK) {
- return return_false(cinfo, *bm, "read_header");
+ return return_failure(cinfo, *bm, "read_header");
}
/* Try to fulfill the requested sampleSize. Since jpeg can do it (when it
@@ -588,8 +603,9 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
// individual pixel. It is very unlikely to be opaque, since
// an opaque A8 bitmap would not be very interesting.
// Otherwise, a jpeg image is opaque.
- return bm->setInfo(SkImageInfo::Make(cinfo.image_width, cinfo.image_height,
- colorType, alphaType));
+ bool success = bm->setInfo(SkImageInfo::Make(cinfo.image_width, cinfo.image_height,
+ colorType, alphaType));
+ return success ? kSuccess : kFailure;
}
/* image_width and image_height are the original dimensions, available
@@ -613,10 +629,11 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
// individual pixel. It is very unlikely to be opaque, since
// an opaque A8 bitmap would not be very interesting.
// Otherwise, a jpeg image is opaque.
- return bm->setInfo(SkImageInfo::Make(smpl.scaledWidth(), smpl.scaledHeight(),
- colorType, alphaType));
+ bool success = bm->setInfo(SkImageInfo::Make(smpl.scaledWidth(), smpl.scaledHeight(),
+ colorType, alphaType));
+ return success ? kSuccess : kFailure;
} else {
- return return_false(cinfo, *bm, "start_decompress");
+ return return_failure(cinfo, *bm, "start_decompress");
}
}
sampleSize = recompute_sampleSize(sampleSize, cinfo);
@@ -624,7 +641,7 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
#ifdef SK_SUPPORT_LEGACY_IMAGEDECODER_CHOOSER
// should we allow the Chooser (if present) to pick a colortype for us???
if (!this->chooseFromOneChoice(colorType, cinfo.output_width, cinfo.output_height)) {
- return return_false(cinfo, *bm, "chooseFromOneChoice");
+ return return_failure(cinfo, *bm, "chooseFromOneChoice");
}
#endif
@@ -636,10 +653,10 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
bm->setInfo(SkImageInfo::Make(sampler.scaledWidth(), sampler.scaledHeight(),
colorType, alphaType));
if (SkImageDecoder::kDecodeBounds_Mode == mode) {
- return true;
+ return kSuccess;
}
if (!this->allocPixelRef(bm, NULL)) {
- return return_false(cinfo, *bm, "allocPixelRef");
+ return return_failure(cinfo, *bm, "allocPixelRef");
}
SkAutoLockPixels alp(*bm);
@@ -662,15 +679,16 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
// so return early. We will return a partial image.
fill_below_level(cinfo.output_scanline, bm);
cinfo.output_scanline = cinfo.output_height;
- break; // Skip to jpeg_finish_decompress()
+ jpeg_finish_decompress(&cinfo);
+ return kPartialSuccess;
}
if (this->shouldCancelDecode()) {
- return return_false(cinfo, *bm, "shouldCancelDecode");
+ return return_failure(cinfo, *bm, "shouldCancelDecode");
}
rowptr += bpr;
}
jpeg_finish_decompress(&cinfo);
- return true;
+ return kSuccess;
}
#endif
@@ -679,11 +697,11 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
int srcBytesPerPixel;
if (!get_src_config(cinfo, &sc, &srcBytesPerPixel)) {
- return return_false(cinfo, *bm, "jpeg colorspace");
+ return return_failure(cinfo, *bm, "jpeg colorspace");
}
if (!sampler.begin(bm, sc, *this)) {
- return return_false(cinfo, *bm, "sampler.begin");
+ return return_failure(cinfo, *bm, "sampler.begin");
}
SkAutoMalloc srcStorage(cinfo.output_width * srcBytesPerPixel);
@@ -691,7 +709,7 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
// Possibly skip initial rows [sampler.srcY0]
if (!skip_src_rows(&cinfo, srcRow, sampler.srcY0())) {
- return return_false(cinfo, *bm, "skip rows");
+ return return_failure(cinfo, *bm, "skip rows");
}
// now loop through scanlines until y == bm->height() - 1
@@ -703,10 +721,11 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
// so return early. We will return a partial image.
fill_below_level(y, bm);
cinfo.output_scanline = cinfo.output_height;
- break; // Skip to jpeg_finish_decompress()
+ jpeg_finish_decompress(&cinfo);
+ return kSuccess;
}
if (this->shouldCancelDecode()) {
- return return_false(cinfo, *bm, "shouldCancelDecode");
+ return return_failure(cinfo, *bm, "shouldCancelDecode");
}
if (JCS_CMYK == cinfo.out_color_space) {
@@ -720,20 +739,22 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
}
if (!skip_src_rows(&cinfo, srcRow, sampler.srcDY() - 1)) {
- return return_false(cinfo, *bm, "skip rows");
+ return return_failure(cinfo, *bm, "skip rows");
}
}
// we formally skip the rest, so we don't get a complaint from libjpeg
if (!skip_src_rows(&cinfo, srcRow,
cinfo.output_height - cinfo.output_scanline)) {
- return return_false(cinfo, *bm, "skip rows");
+ return return_failure(cinfo, *bm, "skip rows");
}
jpeg_finish_decompress(&cinfo);
- return true;
+ return kSuccess;
}
+///////////////////////////////////////////////////////////////////////////////
+
enum SizeType {
kSizeForMemoryAllocation_SizeType,
kActualSize_SizeType
@@ -878,8 +899,9 @@ static bool output_raw_data(jpeg_decompress_struct& cinfo, void* planes[3], size
}
JDIMENSION scanlinesRead = jpeg_read_raw_data(&cinfo, bufferraw, yScanlinesToRead);
- if (scanlinesRead == 0)
+ if (scanlinesRead == 0) {
return false;
+ }
if (hasYLastRow) {
memcpy(&outputY[yMaxH * rowBytesY], yLastRow, yWidth);
@@ -917,7 +939,7 @@ bool SkJPEGImageDecoder::onDecodeYUV8Planes(SkStream* stream, SkISize componentS
// All objects need to be instantiated before this setjmp call so that
// they will be cleaned up properly if an error occurs.
if (setjmp(errorManager.fJmpBuf)) {
- return return_false(cinfo, 0, 0, "setjmp YUV8");
+ return return_false(cinfo, "setjmp YUV8");
}
initialize_info(&cinfo, &srcManager);
@@ -925,7 +947,7 @@ bool SkJPEGImageDecoder::onDecodeYUV8Planes(SkStream* stream, SkISize componentS
int status = jpeg_read_header(&cinfo, true);
if (status != JPEG_HEADER_OK) {
- return return_false(cinfo, 0, 0, "read_header YUV8");
+ return return_false(cinfo, "read_header YUV8");
}
if (cinfo.jpeg_color_space != JCS_YCbCr) {
@@ -957,11 +979,11 @@ bool SkJPEGImageDecoder::onDecodeYUV8Planes(SkStream* stream, SkISize componentS
jpeg_start_decompress(), and then read output_width and output_height.
*/
if (!jpeg_start_decompress(&cinfo)) {
- return return_false(cinfo, 0, 0, "start_decompress YUV8");
+ return return_false(cinfo, "start_decompress YUV8");
}
if (!output_raw_data(cinfo, planes, rowBytes)) {
- return return_false(cinfo, 0, 0, "output_raw_data");
+ return return_false(cinfo, "output_raw_data");
}
update_components_sizes(cinfo, componentSizes, kActualSize_SizeType);
@@ -974,6 +996,8 @@ bool SkJPEGImageDecoder::onDecodeYUV8Planes(SkStream* stream, SkISize componentS
return true;
}
+///////////////////////////////////////////////////////////////////////////////
+
#ifdef SK_BUILD_FOR_ANDROID
bool SkJPEGImageDecoder::onBuildTileIndex(SkStreamRewindable* stream, int *width, int *height) {
« no previous file with comments | « src/images/SkImageDecoder_libico.cpp ('k') | src/images/SkImageDecoder_libpng.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698