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

Unified Diff: src/images/SkImageDecoder_libjpeg.cpp

Issue 22290002: Beginning work to refactor jpeg tile decoding. (Closed) Base URL: https://skia.googlecode.com/svn/trunk
Patch Set: Respond to comments. Created 7 years, 4 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 | « no previous file | no next file » | 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 914ceb7e8aa458a3ea4e011be2db27f73d0ead8a..788e3526d2d9880e62fcb9a8a612e6262765425e 100644
--- a/src/images/SkImageDecoder_libjpeg.cpp
+++ b/src/images/SkImageDecoder_libjpeg.cpp
@@ -30,7 +30,6 @@ extern "C" {
//#define TIME_DECODE
// this enables our rgb->yuv code, which is faster than libjpeg on ARM
-// disable for the moment, as we have some glitches when width != multiple of 4
#define WE_CONVERT_TO_YUV
// If ANDROID_RGB is defined by in the jpeg headers it indicates that jpeg offers
@@ -39,7 +38,7 @@ extern "C" {
//////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////
-static void overwrite_mem_buffer_size(j_decompress_ptr cinfo) {
+static void overwrite_mem_buffer_size(jpeg_decompress_struct* cinfo) {
#ifdef SK_BUILD_FOR_ANDROID
/* Check if the device indicates that it has a large amount of system memory
* if so, increase the memory allocation to 30MB instead of the default 5MB.
@@ -55,6 +54,14 @@ static void overwrite_mem_buffer_size(j_decompress_ptr cinfo) {
//////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////
+static void initialize_info(jpeg_decompress_struct* cinfo, skjpeg_source_mgr* src_mgr) {
+ SkASSERT(cinfo != NULL);
+ SkASSERT(src_mgr != NULL);
+ jpeg_create_decompress(cinfo);
+ overwrite_mem_buffer_size(cinfo);
+ cinfo->src = src_mgr;
+}
+
#ifdef SK_BUILD_FOR_ANDROID
class SkJPEGImageIndex {
public:
@@ -69,10 +76,17 @@ public:
~SkJPEGImageIndex() {
if (fHuffmanCreated) {
+ // Set to false before calling the libjpeg function, in case
+ // the libjpeg function calls longjmp. Our setjmp handler may
+ // attempt to delete this SkJPEGImageIndex, thus entering this
+ // destructor again. Setting fHuffmanCreated to false first
+ // prevents an infinite loop.
fHuffmanCreated = false;
jpeg_destroy_huffman_index(&fHuffmanIndex);
}
if (fDecompressStarted) {
+ // Like fHuffmanCreated, set to false before calling libjpeg
+ // function to prevent potential infinite loop.
fDecompressStarted = false;
jpeg_finish_decompress(&fCInfo);
}
@@ -91,6 +105,8 @@ public:
void destroyInfo() {
SkASSERT(fInfoInitialized);
SkASSERT(!fDecompressStarted);
+ // Like fHuffmanCreated, set to false before calling libjpeg
+ // function to prevent potential infinite loop.
fInfoInitialized = false;
jpeg_destroy_decompress(&fCInfo);
SkDEBUGCODE(fReadHeaderSucceeded = false;)
@@ -106,9 +122,7 @@ public:
*/
bool initializeInfoAndReadHeader() {
SkASSERT(!fInfoInitialized && !fDecompressStarted);
- jpeg_create_decompress(&fCInfo);
- overwrite_mem_buffer_size(&fCInfo);
- fCInfo.src = &fSrcMgr;
+ initialize_info(&fCInfo, &fSrcMgr);
fInfoInitialized = true;
const bool success = (JPEG_HEADER_OK == jpeg_read_header(&fCInfo, true));
SkDEBUGCODE(fReadHeaderSucceeded = success;)
@@ -193,6 +207,14 @@ private:
int fImageHeight;
#endif
+ /**
+ * Determine the appropriate bitmap config and out_color_space based on
+ * both the preference of the caller and the jpeg_color_space on the
+ * jpeg_decompress_struct passed in.
+ * Must be called after jpeg_read_header.
+ */
+ SkBitmap::Config getBitmapConfig(jpeg_decompress_struct*);
+
typedef SkImageDecoder INHERITED;
};
@@ -295,6 +317,131 @@ static void convert_CMYK_to_RGB(uint8_t* scanline, unsigned int width) {
}
}
+/**
+ * Common code for setting the error manager.
+ */
+static void set_error_mgr(jpeg_decompress_struct* cinfo, skjpeg_error_mgr* errorManager) {
+ SkASSERT(cinfo != NULL);
+ SkASSERT(errorManager != NULL);
+ cinfo->err = jpeg_std_error(errorManager);
+ errorManager->error_exit = skjpeg_error_exit;
+}
+
+/**
+ * Common code for turning off upsampling and smoothing. Turning these
+ * off helps performance without showing noticable differences in the
+ * resulting bitmap.
+ */
+static void turn_off_visual_optimizations(jpeg_decompress_struct* cinfo) {
+ SkASSERT(cinfo != NULL);
+ /* this gives about 30% performance improvement. In theory it may
+ reduce the visual quality, in practice I'm not seeing a difference
+ */
+ cinfo->do_fancy_upsampling = 0;
+
+ /* this gives another few percents */
+ cinfo->do_block_smoothing = 0;
+}
+
+/**
+ * Common code for setting the dct method.
+ */
+static void set_dct_method(const SkImageDecoder& decoder, jpeg_decompress_struct* cinfo) {
+ SkASSERT(cinfo != NULL);
+#ifdef DCT_IFAST_SUPPORTED
+ if (decoder.getPreferQualityOverSpeed()) {
+ cinfo->dct_method = JDCT_ISLOW;
+ } else {
+ cinfo->dct_method = JDCT_IFAST;
+ }
+#else
+ cinfo->dct_method = JDCT_ISLOW;
+#endif
+}
+
+SkBitmap::Config SkJPEGImageDecoder::getBitmapConfig(jpeg_decompress_struct* cinfo) {
+ SkASSERT(cinfo != NULL);
+
+ SrcDepth srcDepth = k32Bit_SrcDepth;
+ if (JCS_GRAYSCALE == cinfo->jpeg_color_space) {
+ srcDepth = k8BitGray_SrcDepth;
+ }
+
+ SkBitmap::Config config = this->getPrefConfig(srcDepth, /*hasAlpha*/ false);
+ switch (config) {
+ case SkBitmap::kA8_Config:
+ // Only respect A8 config if the original is grayscale,
+ // in which case we will treat the grayscale as alpha
+ // values.
+ if (cinfo->jpeg_color_space != JCS_GRAYSCALE) {
+ config = SkBitmap::kARGB_8888_Config;
+ }
+ break;
+ case SkBitmap::kARGB_8888_Config:
+ // Fall through.
+ case SkBitmap::kARGB_4444_Config:
+ // Fall through.
+ case SkBitmap::kRGB_565_Config:
+ // These are acceptable destination configs.
+ break;
+ default:
+ // Force all other configs to 8888.
+ config = SkBitmap::kARGB_8888_Config;
+ break;
+ }
+
+ switch (cinfo->jpeg_color_space) {
+ case JCS_CMYK:
+ // Fall through.
+ case JCS_YCCK:
+ // libjpeg cannot convert from CMYK or YCCK to RGB - here we set up
+ // so libjpeg will give us CMYK samples back and we will later
+ // manually convert them to RGB
+ cinfo->out_color_space = JCS_CMYK;
+ break;
+ case JCS_GRAYSCALE:
+ if (SkBitmap::kA8_Config == config) {
+ cinfo->out_color_space = JCS_GRAYSCALE;
+ break;
+ }
+ // The data is JCS_GRAYSCALE, but the caller wants some sort of RGB
+ // config. Fall through to set to the default.
+ default:
+ cinfo->out_color_space = JCS_RGB;
+ break;
+ }
+ return config;
+}
+
+#ifdef ANDROID_RGB
+/**
+ * Based on the config and dither mode, adjust out_color_space and
+ * dither_mode of cinfo.
+ */
+static void adjust_out_color_space_and_dither(jpeg_decompress_struct* cinfo,
+ SkBitmap::Config config,
+ const SkImageDecoder& decoder) {
+ SkASSERT(cinfo != NULL);
+ cinfo->dither_mode = JDITHER_NONE;
+ if (JCS_CMYK == cinfo->out_color_space) {
+ return;
+ }
+ switch(config) {
+ case SkBitmap::kARGB_8888_Config:
+ cinfo->out_color_space = JCS_RGBA_8888;
+ break;
+ case SkBitmap::kRGB_565_Config:
+ cinfo->out_color_space = JCS_RGB_565;
+ if (decoder.getDitherImage()) {
+ cinfo->dither_mode = JDITHER_ORDERED;
+ }
+ break;
+ default:
+ break;
+ }
+}
+#endif
+
bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
#ifdef TIME_DECODE
SkAutoTime atm("JPEG Decode");
@@ -303,11 +450,10 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
JPEGAutoClean autoClean;
jpeg_decompress_struct cinfo;
- skjpeg_error_mgr errorManager;
skjpeg_source_mgr srcManager(stream, this);
- cinfo.err = jpeg_std_error(&errorManager);
- errorManager.error_exit = skjpeg_error_exit;
+ skjpeg_error_mgr errorManager;
+ set_error_mgr(&cinfo, &errorManager);
// All objects need to be instantiated before this setjmp call so that
// they will be cleaned up properly if an error occurs.
@@ -315,14 +461,9 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
return return_false(cinfo, *bm, "setjmp");
}
- jpeg_create_decompress(&cinfo);
+ initialize_info(&cinfo, &srcManager);
autoClean.set(&cinfo);
- overwrite_mem_buffer_size(&cinfo);
-
- //jpeg_stdio_src(&cinfo, file);
- cinfo.src = &srcManager;
-
int status = jpeg_read_header(&cinfo, true);
if (status != JPEG_HEADER_OK) {
return return_false(cinfo, *bm, "read_header");
@@ -334,67 +475,17 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
*/
int sampleSize = this->getSampleSize();
-#ifdef DCT_IFAST_SUPPORTED
- if (this->getPreferQualityOverSpeed()) {
- cinfo.dct_method = JDCT_ISLOW;
- } else {
- cinfo.dct_method = JDCT_IFAST;
- }
-#else
- cinfo.dct_method = JDCT_ISLOW;
-#endif
+ set_dct_method(*this, &cinfo);
- cinfo.scale_num = 1;
+ SkASSERT(1 == cinfo.scale_num);
cinfo.scale_denom = sampleSize;
- /* this gives about 30% performance improvement. In theory it may
- reduce the visual quality, in practice I'm not seeing a difference
- */
- cinfo.do_fancy_upsampling = 0;
-
- /* this gives another few percents */
- cinfo.do_block_smoothing = 0;
-
- SrcDepth srcDepth = k32Bit_SrcDepth;
- /* default format is RGB */
- if (cinfo.jpeg_color_space == JCS_CMYK) {
- // libjpeg cannot convert from CMYK to RGB - here we set up
- // so libjpeg will give us CMYK samples back and we will
- // later manually convert them to RGB
- cinfo.out_color_space = JCS_CMYK;
- } else if (cinfo.jpeg_color_space == JCS_GRAYSCALE) {
- cinfo.out_color_space = JCS_GRAYSCALE;
- srcDepth = k8BitGray_SrcDepth;
- } else {
- cinfo.out_color_space = JCS_RGB;
- }
+ turn_off_visual_optimizations(&cinfo);
- SkBitmap::Config config = this->getPrefConfig(srcDepth, false);
- // only these make sense for jpegs
- if (SkBitmap::kA8_Config == config) {
- if (cinfo.jpeg_color_space != JCS_GRAYSCALE) {
- // Converting from a non grayscale image to A8 is
- // not currently supported.
- config = SkBitmap::kARGB_8888_Config;
- // Change the output from jpeg back to RGB.
- cinfo.out_color_space = JCS_RGB;
- }
- } else if (config != SkBitmap::kARGB_8888_Config &&
- config != SkBitmap::kARGB_4444_Config &&
- config != SkBitmap::kRGB_565_Config) {
- config = SkBitmap::kARGB_8888_Config;
- }
+ const SkBitmap::Config config = this->getBitmapConfig(&cinfo);
#ifdef ANDROID_RGB
- cinfo.dither_mode = JDITHER_NONE;
- if (SkBitmap::kARGB_8888_Config == config && JCS_CMYK != cinfo.out_color_space) {
- cinfo.out_color_space = JCS_RGBA_8888;
- } else if (SkBitmap::kRGB_565_Config == config && JCS_CMYK != cinfo.out_color_space) {
- cinfo.out_color_space = JCS_RGB_565;
- if (this->getDitherImage()) {
- cinfo.dither_mode = JDITHER_ORDERED;
- }
- }
+ adjust_out_color_space_and_dither(&cinfo, config, *this);
#endif
if (1 == sampleSize && SkImageDecoder::kDecodeBounds_Mode == mode) {
@@ -552,8 +643,7 @@ bool SkJPEGImageDecoder::onBuildTileIndex(SkStream* stream, int *width, int *hei
jpeg_decompress_struct* cinfo = imageIndex->cinfo();
skjpeg_error_mgr sk_err;
- cinfo->err = jpeg_std_error(&sk_err);
- sk_err.error_exit = skjpeg_error_exit;
+ set_error_mgr(cinfo, &sk_err);
// All objects need to be instantiated before this setjmp call so that
// they will be cleaned up properly if an error occurs.
@@ -578,9 +668,13 @@ bool SkJPEGImageDecoder::onBuildTileIndex(SkStream* stream, int *width, int *hei
return false;
}
- cinfo->out_color_space = JCS_RGBA_8888;
- cinfo->do_fancy_upsampling = 0;
- cinfo->do_block_smoothing = 0;
+ // FIXME: This sets cinfo->out_color_space, which we may change later
+ // based on the config in onDecodeSubset. This should be fine, since
+ // jpeg_init_read_tile_scanline will check out_color_space again after
+ // that change (when it calls jinit_color_deconverter).
+ (void) this->getBitmapConfig(cinfo);
+
+ turn_off_visual_optimizations(cinfo);
// instead of jpeg_start_decompress() we start a tiled decompress
if (!imageIndex->startTileDecompress()) {
@@ -588,10 +682,15 @@ bool SkJPEGImageDecoder::onBuildTileIndex(SkStream* stream, int *width, int *hei
}
SkASSERT(1 == cinfo->scale_num);
- *height = cinfo->output_height;
- *width = cinfo->output_width;
- fImageWidth = *width;
- fImageHeight = *height;
+ fImageWidth = cinfo->output_width;
+ fImageHeight = cinfo->output_height;
+
+ if (width) {
+ *width = fImageWidth;
+ }
+ if (height) {
+ *height = fImageHeight;
+ }
SkDELETE(fImageIndex);
fImageIndex = imageIndex.detach();
@@ -613,8 +712,8 @@ bool SkJPEGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) {
skjpeg_error_mgr errorManager;
- cinfo->err = jpeg_std_error(&errorManager);
- errorManager.error_exit = skjpeg_error_exit;
+ set_error_mgr(cinfo, &errorManager);
+
if (setjmp(errorManager.fJmpBuf)) {
return false;
}
@@ -622,32 +721,11 @@ bool SkJPEGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) {
int requestedSampleSize = this->getSampleSize();
cinfo->scale_denom = requestedSampleSize;
- if (this->getPreferQualityOverSpeed()) {
- cinfo->dct_method = JDCT_ISLOW;
- } else {
- cinfo->dct_method = JDCT_IFAST;
- }
-
- SkBitmap::Config config = this->getPrefConfig(k32Bit_SrcDepth, false);
- if (config != SkBitmap::kARGB_8888_Config &&
- config != SkBitmap::kARGB_4444_Config &&
- config != SkBitmap::kRGB_565_Config) {
- config = SkBitmap::kARGB_8888_Config;
- }
-
- /* default format is RGB */
- cinfo->out_color_space = JCS_RGB;
+ set_dct_method(*this, cinfo);
+ const SkBitmap::Config config = this->getBitmapConfig(cinfo);
#ifdef ANDROID_RGB
- cinfo->dither_mode = JDITHER_NONE;
- if (SkBitmap::kARGB_8888_Config == config) {
- cinfo->out_color_space = JCS_RGBA_8888;
- } else if (SkBitmap::kRGB_565_Config == config) {
- cinfo->out_color_space = JCS_RGB_565;
- if (this->getDitherImage()) {
- cinfo->dither_mode = JDITHER_ORDERED;
- }
- }
+ adjust_out_color_space_and_dither(cinfo, config, *this);
#endif
int startX = rect.fLeft;
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698