Skip to content

GitLab

T
tangled
Commit 607ea4c4 authored by Miguel Casas-Sanchez's avatar Miguel Casas-Sanchez Committed by Commit Bot
Browse files
Options

JPEGCodec{RobustSlow}::Decode: refactor to use unique_ptr. 

This CL removes the class DecompressDestroyer in favour of
a unique_ptr<> with specific Deleter. Less lines of code.

Functionality covered by gfx_unittest, also under asan bot.

Bug: 868400
Change-Id: I7ac2d254d6f5771a81620b14db7e610ff3b7e4b9
Reviewed-on: https://chromium-review.googlesource.com/1245467
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594638}
parent 1ef3d0c9
Hide whitespace changes
Inline Side-by-side
Showing with 69 additions and 113 deletions
ui/gfx/codec/chromeos/jpeg_codec_robust_slow.cc
View file @ 607ea4c4
...@@ -139,26 +139,9 @@ void RGBtoBGRA(const unsigned char* bgra, int pixel_width, unsigned char* rgb) { ...@@ -139,26 +139,9 @@ void RGBtoBGRA(const unsigned char* bgra, int pixel_width, unsigned char* rgb) {
} }
#endif // !defined(JCS_EXTENSIONS) #endif // !defined(JCS_EXTENSIONS)
// This class destroys the given jpeg_decompress object when it goes out of // jpeg_decompress_struct Deleter.
// scope. It simplifies the error handling in Decode (and even applies to the struct JpegDecompressStructDeleter {
// success case). void operator()(jpeg_decompress_struct* ptr) { jpeg_destroy_decompress(ptr); }
class IjgDecompressDestroyer {
public:
IjgDecompressDestroyer() : cinfo_(NULL) {}
~IjgDecompressDestroyer() { DestroyManagedObject(); }
void SetManagedObject(jpeg_decompress_struct* ci) {
DestroyManagedObject();
cinfo_ = ci;
}
void DestroyManagedObject() {
if (cinfo_) {
jpeg_destroy_decompress(cinfo_);
cinfo_ = NULL;
}
}
private:
jpeg_decompress_struct* cinfo_;
}; };
} // namespace } // namespace
...@@ -169,28 +152,26 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input, ...@@ -169,28 +152,26 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input,
std::vector<unsigned char>* output, std::vector<unsigned char>* output,
int* w, int* w,
int* h) { int* h) {
jpeg_decompress_struct cinfo; std::unique_ptr<jpeg_decompress_struct, JpegDecompressStructDeleter> cinfo(
IjgDecompressDestroyer destroyer; new jpeg_decompress_struct);
destroyer.SetManagedObject(&cinfo);
output->clear(); output->clear();
// We set up the normal JPEG error routines, then override error_exit. // We set up the normal JPEG error routines, then override error_exit.
// This must be done before the call to create_decompress. // This must be done before the call to create_decompress.
IjgCoderErrorMgr errmgr; IjgCoderErrorMgr errmgr;
cinfo.err = jpeg_std_error(&errmgr.pub); cinfo->err = jpeg_std_error(&errmgr.pub);
errmgr.pub.error_exit = IjgErrorExit; errmgr.pub.error_exit = IjgErrorExit;
// Establish the setjmp return context for IjgErrorExit to use. // Establish the setjmp return context for IjgErrorExit to use.
if (setjmp(errmgr.setjmp_buffer)) { if (setjmp(errmgr.setjmp_buffer)) {
// If we get here, the JPEG code has signaled an error. // If we get here, the JPEG code has signaled an error.
// See note in JPEGCodec::Encode() for why we need to destroy the cinfo // Release |cinfo| by hand to avoid use-after-free of |errmgr|.
// manually here. cinfo.reset();
destroyer.DestroyManagedObject();
return false; return false;
} }
// The destroyer will destroy() cinfo on exit. We don't want to set the // The destroyer will destroy() cinfo on exit. We don't want to set the
// destroyer's object until cinfo is initialized. // destroyer's object until cinfo is initialized.
jpeg_create_decompress(&cinfo); jpeg_create_decompress(cinfo.get());
// set up the source manager // set up the source manager
jpeg_source_mgr srcmgr; jpeg_source_mgr srcmgr;
...@@ -199,17 +180,17 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input, ...@@ -199,17 +180,17 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input,
srcmgr.skip_input_data = IjgSkipInputData; srcmgr.skip_input_data = IjgSkipInputData;
srcmgr.resync_to_restart = jpeg_resync_to_restart; // use default routine srcmgr.resync_to_restart = jpeg_resync_to_restart; // use default routine
srcmgr.term_source = IjgTermSource; srcmgr.term_source = IjgTermSource;
cinfo.src = &srcmgr; cinfo->src = &srcmgr;
IjgJpegDecoderState state(input, input_size); IjgJpegDecoderState state(input, input_size);
cinfo.client_data = &state; cinfo->client_data = &state;
// fill the file metadata into our buffer // fill the file metadata into our buffer
if (jpeg_read_header(&cinfo, true) != JPEG_HEADER_OK) if (jpeg_read_header(cinfo.get(), true) != JPEG_HEADER_OK)
return false; return false;
// we want to always get RGB data out // we want to always get RGB data out
switch (cinfo.jpeg_color_space) { switch (cinfo->jpeg_color_space) {
case JCS_GRAYSCALE: case JCS_GRAYSCALE:
case JCS_RGB: case JCS_RGB:
case JCS_YCbCr: case JCS_YCbCr:
...@@ -219,24 +200,22 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input, ...@@ -219,24 +200,22 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input,
// used by Chromium (i.e. RGB, RGBA, and BGRA) and we just map the input // used by Chromium (i.e. RGB, RGBA, and BGRA) and we just map the input
// parameters to a colorspace. // parameters to a colorspace.
if (format == FORMAT_RGB) { if (format == FORMAT_RGB) {
cinfo.out_color_space = JCS_RGB; cinfo->out_color_space = JCS_RGB;
cinfo.output_components = 3; cinfo->output_components = 3;
} else if (format == FORMAT_RGBA || } else if (format == FORMAT_RGBA ||
(format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) { (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
cinfo.out_color_space = JCS_EXT_RGBX; cinfo->out_color_space = JCS_EXT_RGBX;
cinfo.output_components = 4; cinfo->output_components = 4;
} else if (format == FORMAT_BGRA || } else if (format == FORMAT_BGRA ||
(format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) { (format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) {
cinfo.out_color_space = JCS_EXT_BGRX; cinfo->out_color_space = JCS_EXT_BGRX;
cinfo.output_components = 4; cinfo->output_components = 4;
} else { } else {
// We can exit this function without calling jpeg_destroy_decompress()
// because IjgDecompressDestroyer automaticaly calls it.
NOTREACHED() << "Invalid pixel format"; NOTREACHED() << "Invalid pixel format";
return false; return false;
} }
#else #else
cinfo.out_color_space = JCS_RGB; cinfo->out_color_space = JCS_RGB;
#endif #endif
break; break;
case JCS_CMYK: case JCS_CMYK:
...@@ -248,39 +227,39 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input, ...@@ -248,39 +227,39 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input,
return false; return false;
} }
#ifndef JCS_EXTENSIONS #ifndef JCS_EXTENSIONS
cinfo.output_components = 3; cinfo->output_components = 3;
#endif #endif
jpeg_calc_output_dimensions(&cinfo); jpeg_calc_output_dimensions(cinfo.get());
*w = cinfo.output_width; *w = cinfo->output_width;
*h = cinfo.output_height; *h = cinfo->output_height;
jpeg_start_decompress(&cinfo); jpeg_start_decompress(cinfo.get());
// FIXME(brettw) we may want to allow the capability for callers to request // FIXME(brettw) we may want to allow the capability for callers to request
// how to align row lengths as we do for the compressor. // how to align row lengths as we do for the compressor.
int row_read_stride = cinfo.output_width * cinfo.output_components; int row_read_stride = cinfo->output_width * cinfo->output_components;
#ifdef JCS_EXTENSIONS #ifdef JCS_EXTENSIONS
// Create memory for a decoded image and write decoded lines to the memory // Create memory for a decoded image and write decoded lines to the memory
// without conversions same as JPEGCodec::Encode(). // without conversions same as JPEGCodec::Encode().
int row_write_stride = row_read_stride; int row_write_stride = row_read_stride;
output->resize(row_write_stride * cinfo.output_height); output->resize(row_write_stride * cinfo->output_height);
for (int row = 0; row < static_cast<int>(cinfo.output_height); row++) { for (int row = 0; row < static_cast<int>(cinfo->output_height); row++) {
unsigned char* rowptr = &(*output)[row * row_write_stride]; unsigned char* rowptr = &(*output)[row * row_write_stride];
if (!jpeg_read_scanlines(&cinfo, &rowptr, 1)) if (!jpeg_read_scanlines(cinfo.get(), &rowptr, 1))
return false; return false;
} }
#else #else
if (format == FORMAT_RGB) { if (format == FORMAT_RGB) {
// easy case, row needs no conversion // easy case, row needs no conversion
int row_write_stride = row_read_stride; int row_write_stride = row_read_stride;
output->resize(row_write_stride * cinfo.output_height); output->resize(row_write_stride * cinfo->output_height);
for (int row = 0; row < static_cast<int>(cinfo.output_height); row++) { for (int row = 0; row < static_cast<int>(cinfo->output_height); row++) {
unsigned char* rowptr = &(*output)[row * row_write_stride]; unsigned char* rowptr = &(*output)[row * row_write_stride];
if (!jpeg_read_scanlines(&cinfo, &rowptr, 1)) if (!jpeg_read_scanlines(cinfo.get(), &rowptr, 1))
return false; return false;
} }
} else { } else {
...@@ -291,33 +270,32 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input, ...@@ -291,33 +270,32 @@ bool JPEGCodecRobustSlow::Decode(const unsigned char* input,
void (*converter)(const unsigned char* rgb, int w, unsigned char* out); void (*converter)(const unsigned char* rgb, int w, unsigned char* out);
if (format == FORMAT_RGBA || if (format == FORMAT_RGBA ||
(format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) { (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
row_write_stride = cinfo.output_width * 4; row_write_stride = cinfo->output_width * 4;
converter = AddAlpha; converter = AddAlpha;
} else if (format == FORMAT_BGRA || } else if (format == FORMAT_BGRA ||
(format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) { (format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) {
row_write_stride = cinfo.output_width * 4; row_write_stride = cinfo->output_width * 4;
converter = RGBtoBGRA; converter = RGBtoBGRA;
} else { } else {
NOTREACHED() << "Invalid pixel format"; NOTREACHED() << "Invalid pixel format";
jpeg_destroy_decompress(&cinfo); jpeg_destroy_decompress(cinfo.get());
return false; return false;
} }
output->resize(row_write_stride * cinfo.output_height); output->resize(row_write_stride * cinfo->output_height);
std::unique_ptr<unsigned char[]> row_data( std::unique_ptr<unsigned char[]> row_data(
new unsigned char[row_read_stride]); new unsigned char[row_read_stride]);
unsigned char* rowptr = row_data.get(); unsigned char* rowptr = row_data.get();
for (int row = 0; row < static_cast<int>(cinfo.output_height); row++) { for (int row = 0; row < static_cast<int>(cinfo->output_height); row++) {
if (!jpeg_read_scanlines(&cinfo, &rowptr, 1)) if (!jpeg_read_scanlines(cinfo.get(), &rowptr, 1))
return false; return false;
converter(rowptr, *w, &(*output)[row * row_write_stride]); converter(rowptr, *w, &(*output)[row * row_write_stride]);
} }
} }
#endif #endif
jpeg_finish_decompress(&cinfo); jpeg_finish_decompress(cinfo.get());
jpeg_destroy_decompress(&cinfo);
return true; return true;
} }
......
ui/gfx/codec/jpeg_codec.cc
View file @ 607ea4c4
...@@ -150,31 +150,14 @@ void SkipInputData(j_decompress_ptr cinfo, long num_bytes) { ...@@ -150,31 +150,14 @@ void SkipInputData(j_decompress_ptr cinfo, long num_bytes) {
// "Terminate source --- called by jpeg_finish_decompress() after all data has // "Terminate source --- called by jpeg_finish_decompress() after all data has
// been read to clean up JPEG source manager. NOT called by jpeg_abort() or // been read to clean up JPEG source manager. NOT called by jpeg_abort() or
// jpeg_destroy()." // jpeg_destroy()."
void TermSource(j_decompress_ptr cinfo) { void TermSource(j_decompress_ptr cinfo) {}
}
// This class destroys the given jpeg_decompress object when it goes out of // jpeg_decompress_struct Deleter.
// scope. It simplifies the error handling in Decode (and even applies to the struct JpegDecompressStructDeleter {
// success case). void operator()(jpeg_decompress_struct* ptr) {
class DecompressDestroyer { jpeg_destroy_decompress(ptr);
public: delete ptr;
DecompressDestroyer() : cinfo_(NULL) {
}
~DecompressDestroyer() {
DestroyManagedObject();
}
void SetManagedObject(jpeg_decompress_struct* ci) {
DestroyManagedObject();
cinfo_ = ci;
}
void DestroyManagedObject() {
if (cinfo_) {
jpeg_destroy_decompress(cinfo_);
cinfo_ = NULL;
}
} }
private:
jpeg_decompress_struct* cinfo_;
}; };
} // namespace } // namespace
...@@ -182,28 +165,26 @@ class DecompressDestroyer { ...@@ -182,28 +165,26 @@ class DecompressDestroyer {
bool JPEGCodec::Decode(const unsigned char* input, size_t input_size, bool JPEGCodec::Decode(const unsigned char* input, size_t input_size,
ColorFormat format, std::vector<unsigned char>* output, ColorFormat format, std::vector<unsigned char>* output,
int* w, int* h) { int* w, int* h) {
jpeg_decompress_struct cinfo; std::unique_ptr<jpeg_decompress_struct, JpegDecompressStructDeleter> cinfo(
DecompressDestroyer destroyer; new jpeg_decompress_struct);
destroyer.SetManagedObject(&cinfo);
output->clear(); output->clear();
// We set up the normal JPEG error routines, then override error_exit. // We set up the normal JPEG error routines, then override error_exit.
// This must be done before the call to create_decompress. // This must be done before the call to jpeg_create_decompress.
CoderErrorMgr errmgr; CoderErrorMgr errmgr;
cinfo.err = jpeg_std_error(&errmgr.pub); cinfo->err = jpeg_std_error(&errmgr.pub);
errmgr.pub.error_exit = ErrorExit; errmgr.pub.error_exit = ErrorExit;
// Establish the setjmp return context for ErrorExit to use. // Establish the setjmp return context for ErrorExit to use.
if (setjmp(errmgr.setjmp_buffer)) { if (setjmp(errmgr.setjmp_buffer)) {
// If we get here, the JPEG code has signaled an error. // If we get here, the JPEG code has signaled an error.
// See note in JPEGCodec::Encode() for why we need to destroy the cinfo // Release |cinfo| by hand to avoid use-after-free of |errmgr|.
// manually here. cinfo.reset();
destroyer.DestroyManagedObject();
return false; return false;
} }
// The destroyer will destroy() cinfo on exit. We don't want to set the // The destroyer will destroy() cinfo on exit. We don't want to set the
// destroyer's object until cinfo is initialized. // destroyer's object until cinfo is initialized.
jpeg_create_decompress(&cinfo); jpeg_create_decompress(cinfo.get());
// set up the source manager // set up the source manager
jpeg_source_mgr srcmgr; jpeg_source_mgr srcmgr;
...@@ -212,17 +193,17 @@ bool JPEGCodec::Decode(const unsigned char* input, size_t input_size, ...@@ -212,17 +193,17 @@ bool JPEGCodec::Decode(const unsigned char* input, size_t input_size,
srcmgr.skip_input_data = SkipInputData; srcmgr.skip_input_data = SkipInputData;
srcmgr.resync_to_restart = jpeg_resync_to_restart; // use default routine srcmgr.resync_to_restart = jpeg_resync_to_restart; // use default routine
srcmgr.term_source = TermSource; srcmgr.term_source = TermSource;
cinfo.src = &srcmgr; cinfo->src = &srcmgr;
JpegDecoderState state(input, input_size); JpegDecoderState state(input, input_size);
cinfo.client_data = &state; cinfo->client_data = &state;
// fill the file metadata into our buffer // fill the file metadata into our buffer
if (jpeg_read_header(&cinfo, true) != JPEG_HEADER_OK) if (jpeg_read_header(cinfo.get(), true) != JPEG_HEADER_OK)
return false; return false;
// we want to always get RGB data out // we want to always get RGB data out
switch (cinfo.jpeg_color_space) { switch (cinfo->jpeg_color_space) {
case JCS_GRAYSCALE: case JCS_GRAYSCALE:
case JCS_RGB: case JCS_RGB:
case JCS_YCbCr: case JCS_YCbCr:
...@@ -232,15 +213,13 @@ bool JPEGCodec::Decode(const unsigned char* input, size_t input_size, ...@@ -232,15 +213,13 @@ bool JPEGCodec::Decode(const unsigned char* input, size_t input_size,
// parameters to a colorspace. // parameters to a colorspace.
if (format == FORMAT_RGBA || if (format == FORMAT_RGBA ||
(format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) { (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
cinfo.out_color_space = JCS_EXT_RGBX; cinfo->out_color_space = JCS_EXT_RGBX;
cinfo.output_components = 4; cinfo->output_components = 4;
} else if (format == FORMAT_BGRA || } else if (format == FORMAT_BGRA ||
(format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) { (format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) {
cinfo.out_color_space = JCS_EXT_BGRX; cinfo->out_color_space = JCS_EXT_BGRX;
cinfo.output_components = 4; cinfo->output_components = 4;
} else { } else {
// We can exit this function without calling jpeg_destroy_decompress()
// because DecompressDestroyer automaticaly calls it.
NOTREACHED() << "Invalid pixel format"; NOTREACHED() << "Invalid pixel format";
return false; return false;
} }
...@@ -254,29 +233,28 @@ bool JPEGCodec::Decode(const unsigned char* input, size_t input_size, ...@@ -254,29 +233,28 @@ bool JPEGCodec::Decode(const unsigned char* input, size_t input_size,
return false; return false;
} }
jpeg_calc_output_dimensions(&cinfo); jpeg_calc_output_dimensions(cinfo.get());
*w = cinfo.output_width; *w = cinfo->output_width;
*h = cinfo.output_height; *h = cinfo->output_height;
jpeg_start_decompress(&cinfo); jpeg_start_decompress(cinfo.get());
// FIXME(brettw) we may want to allow the capability for callers to request // FIXME(brettw) we may want to allow the capability for callers to request
// how to align row lengths as we do for the compressor. // how to align row lengths as we do for the compressor.
int row_read_stride = cinfo.output_width * cinfo.output_components; int row_read_stride = cinfo->output_width * cinfo->output_components;
// Create memory for a decoded image and write decoded lines to the memory // Create memory for a decoded image and write decoded lines to the memory
// without conversions same as JPEGCodec::Encode(). // without conversions same as JPEGCodec::Encode().
int row_write_stride = row_read_stride; int row_write_stride = row_read_stride;
output->resize(row_write_stride * cinfo.output_height); output->resize(row_write_stride * cinfo->output_height);
for (int row = 0; row < static_cast<int>(cinfo.output_height); row++) { for (int row = 0; row < static_cast<int>(cinfo->output_height); row++) {
unsigned char* rowptr = &(*output)[row * row_write_stride]; unsigned char* rowptr = &(*output)[row * row_write_stride];
if (!jpeg_read_scanlines(&cinfo, &rowptr, 1)) if (!jpeg_read_scanlines(cinfo.get(), &rowptr, 1))
return false; return false;
} }
jpeg_finish_decompress(&cinfo); jpeg_finish_decompress(cinfo.get());
jpeg_destroy_decompress(&cinfo);
return true; return true;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment