Use SkJpegEncoder in gfx jpeg_codec

ui/gfx/codec/jpeg_codec.cc

Index: ui/gfx/codec/jpeg_codec.cc
diff --git a/ui/gfx/codec/jpeg_codec.cc b/ui/gfx/codec/jpeg_codec.cc
index 6d926378bec130b53d70bbb510a1287f42b78a08..8106e46185dcffc4492d99217e0ff9c3c4a50a45 100644
--- a/ui/gfx/codec/jpeg_codec.cc
+++ b/ui/gfx/codec/jpeg_codec.cc
@@ -11,6 +11,8 @@
 #include "base/logging.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkColorPriv.h"
+#include "third_party/skia/include/core/SkStream.h"
+#include "third_party/skia/include/encode/SkJpegEncoder.h"
 
 extern "C" {
 #if defined(USE_SYSTEM_LIBJPEG)
@@ -55,270 +57,47 @@ JPEGCodec::LibraryVariant JPEGCodec::JpegLibraryVariant() {
 }
 
 // Encoder ---------------------------------------------------------------------
-//
-// This code is based on nsJPEGEncoder from Mozilla.
-// Copyright 2005 Google Inc. (Brett Wilson, contributor)
 
 namespace {
 
-// Initial size for the output buffer in the JpegEncoderState below.
-static const int initial_output_buffer_size = 8192;
-
-struct JpegEncoderState {
-  explicit JpegEncoderState(std::vector<unsigned char>* o)
-      : out(o),
-        image_buffer_used(0) {
+class VectorWStream : public SkWStream {
+ public:
+  VectorWStream(std::vector<unsigned char>* dst) : dst_(dst) {
+    DCHECK(dst_);
+    DCHECK_EQ(0UL, dst->size());

[scroggo_chromium, 2017/05/30 14:27:52]

Not a huge deal, but in the first DCHECK you used dst_, and this time you used
dst. Might as well be consistent?

[msarett1, 2017/06/07 18:01:14]

Done.

   }
 
-  // Output buffer, of which 'image_buffer_used' bytes are actually used (this
-  // will often be less than the actual size of the vector because we size it
-  // so that libjpeg can write directly into it.
-  std::vector<unsigned char>* out;
-
-  // Number of bytes in the 'out' buffer that are actually used (see above).
-  size_t image_buffer_used;
-};
-
-// Initializes the JpegEncoderState for encoding, and tells libjpeg about where
-// the output buffer is.
-//
-// From the JPEG library:
-//  "Initialize destination. This is called by jpeg_start_compress() before
-//   any data is actually written. It must initialize next_output_byte and
-//   free_in_buffer. free_in_buffer must be initialized to a positive value."
-void InitDestination(jpeg_compress_struct* cinfo) {
-  JpegEncoderState* state = static_cast<JpegEncoderState*>(cinfo->client_data);
-  DCHECK(state->image_buffer_used == 0) << "initializing after use";
-
-  state->out->resize(initial_output_buffer_size);
-  state->image_buffer_used = 0;
-
-  cinfo->dest->next_output_byte = &(*state->out)[0];
-  cinfo->dest->free_in_buffer = initial_output_buffer_size;
-}
-
-// Resize the buffer that we give to libjpeg and update our and its state.
-//
-// From the JPEG library:
-//  "Callback used by libjpeg whenever the buffer has filled (free_in_buffer
-//   reaches zero). In typical applications, it should write out the *entire*
-//   buffer (use the saved start address and buffer length; ignore the current
-//   state of next_output_byte and free_in_buffer). Then reset the pointer &
-//   count to the start of the buffer, and return TRUE indicating that the
-//   buffer has been dumped. free_in_buffer must be set to a positive value
-//   when TRUE is returned. A FALSE return should only be used when I/O
-//   suspension is desired (this operating mode is discussed in the next
-//   section)."
-boolean EmptyOutputBuffer(jpeg_compress_struct* cinfo) {
-  JpegEncoderState* state = static_cast<JpegEncoderState*>(cinfo->client_data);
-
-  // note the new size, the buffer is full
-  state->image_buffer_used = state->out->size();
-
-  // expand buffer, just double size each time
-  state->out->resize(state->out->size() * 2);
-
-  // tell libjpeg where to write the next data
-  cinfo->dest->next_output_byte = &(*state->out)[state->image_buffer_used];
-  cinfo->dest->free_in_buffer = state->out->size() - state->image_buffer_used;
-  return 1;
-}
-
-// Cleans up the JpegEncoderState to prepare for returning in the final form.
-//
-// From the JPEG library:
-//  "Terminate destination --- called by jpeg_finish_compress() after all data
-//   has been written. In most applications, this must flush any data
-//   remaining in the buffer. Use either next_output_byte or free_in_buffer to
-//   determine how much data is in the buffer."
-void TermDestination(jpeg_compress_struct* cinfo) {
-  JpegEncoderState* state = static_cast<JpegEncoderState*>(cinfo->client_data);
-  DCHECK(state->out->size() >= state->image_buffer_used);
-
-  // update the used byte based on the next byte libjpeg would write to
-  state->image_buffer_used = cinfo->dest->next_output_byte - &(*state->out)[0];
-  DCHECK(state->image_buffer_used < state->out->size()) <<
-    "JPEG library busted, got a bad image buffer size";
-
-  // update our buffer so that it exactly encompases the desired data
-  state->out->resize(state->image_buffer_used);
-}
-
-#if !defined(JCS_EXTENSIONS)
-// Converts RGBA to RGB (removing the alpha values) to prepare to send data to
-// libjpeg. This converts one row of data in rgba with the given width in
-// pixels the the given rgb destination buffer (which should have enough space
-// reserved for the final data).
-void StripAlpha(const unsigned char* rgba, int pixel_width, unsigned char* rgb)
-{
-  for (int x = 0; x < pixel_width; x++)
-    memcpy(&rgb[x * 3], &rgba[x * 4], 3);
-}
-
-// Converts BGRA to RGB by reordering the color components and dropping the
-// alpha. This converts  one row of data in rgba with the given width in
-// pixels the the given rgb destination buffer (which should have enough space
-// reserved for the final data).
-void BGRAtoRGB(const unsigned char* bgra, int pixel_width, unsigned char* rgb)
-{
-  for (int x = 0; x < pixel_width; x++) {
-    const unsigned char* pixel_in = &bgra[x * 4];
-    unsigned char* pixel_out = &rgb[x * 3];
-    pixel_out[0] = pixel_in[2];
-    pixel_out[1] = pixel_in[1];
-    pixel_out[2] = pixel_in[0];
+  bool write(const void* buffer, size_t size) override {
+    dst_->insert(dst_->end(), reinterpret_cast<const unsigned char*>(buffer),

[scroggo_chromium, 2017/05/30 14:27:52]

nit: Use a temporary to make this more readable?

  auto* char_buffer = reinterpret_cast<const unsigned char*>(buffer);
  dst_->insert(dst_->end(), char_buffer, char_buffer + size);

[msarett1, 2017/06/07 18:01:14]

Done.

+                 reinterpret_cast<const unsigned char*>(buffer) + size);
+    return true;
   }
-}
-#endif  // !defined(JCS_EXTENSIONS)
 
-// This class destroys the given jpeg_compress object when it goes out of
-// scope. It simplifies the error handling in Encode (and even applies to the
-// success case).
-class CompressDestroyer {
- public:
-  CompressDestroyer() : cinfo_(NULL) {
-  }
-  ~CompressDestroyer() {
-    DestroyManagedObject();
-  }
-  void SetManagedObject(jpeg_compress_struct* ci) {
-    DestroyManagedObject();
-    cinfo_ = ci;
-  }
-  void DestroyManagedObject() {
-    if (cinfo_) {
-      jpeg_destroy_compress(cinfo_);
-      cinfo_ = NULL;
-    }
-  }
+  size_t bytesWritten() const override { return dst_->size(); }
+
  private:
-  jpeg_compress_struct* cinfo_;
+  // Does not have ownership.
+  std::vector<unsigned char>* dst_;
+};
 };
 
-}  // namespace
-
-bool JPEGCodec::Encode(const unsigned char* input, ColorFormat format,
-                       int w, int h, int row_byte_width,
-                       int quality, std::vector<unsigned char>* output) {
-  jpeg_compress_struct cinfo;
-  CompressDestroyer destroyer;
-  destroyer.SetManagedObject(&cinfo);
+bool JPEGCodec::Encode(const unsigned char* input,
+                       SkColorType colorType,
+                       int w,
+                       int h,
+                       int row_byte_width,
+                       int quality,
+                       std::vector<unsigned char>* output) {
   output->clear();
-#if !defined(JCS_EXTENSIONS)
-  unsigned char* row_buffer = NULL;
-#endif
-
-  // We set up the normal JPEG error routines, then override error_exit.
-  // This must be done before the call to create_compress.
-  CoderErrorMgr errmgr;
-  cinfo.err = jpeg_std_error(&errmgr.pub);
-  errmgr.pub.error_exit = ErrorExit;
+  VectorWStream dst(output);
 
-  // Establish the setjmp return context for ErrorExit to use.
-  if (setjmp(errmgr.setjmp_buffer)) {
-    // If we get here, the JPEG code has signaled an error.
-    // MSDN notes: "if you intend your code to be portable, do not rely on
-    // correct destruction of frame-based objects when executing a nonlocal
-    // goto using a call to longjmp."  So we delete the CompressDestroyer's
-    // object manually instead.
-    destroyer.DestroyManagedObject();
-#if !defined(JCS_EXTENSIONS)
-    delete[] row_buffer;
-#endif
-    return false;
-  }
+  SkImageInfo info = SkImageInfo::Make(w, h, colorType, kOpaque_SkAlphaType);
+  SkPixmap src(info, input, row_byte_width);
 
-  // The destroyer will destroy() cinfo on exit.
-  jpeg_create_compress(&cinfo);
+  SkJpegEncoder::Options options;
+  options.fQuality = quality;
 
-  cinfo.image_width = w;
-  cinfo.image_height = h;
-  cinfo.input_components = 3;
-#ifdef JCS_EXTENSIONS
-  // Choose an input colorspace and return if it is an unsupported one. Since
-  // libjpeg-turbo supports all input formats used by Chromium (i.e. RGB, RGBA,
-  // and BGRA), we just map the input parameters to a colorspace used by
-  // libjpeg-turbo.
-  if (format == FORMAT_RGB) {
-    cinfo.input_components = 3;
-    cinfo.in_color_space = JCS_RGB;
-  } else if (format == FORMAT_RGBA ||
-             (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
-    cinfo.input_components = 4;
-    cinfo.in_color_space = JCS_EXT_RGBX;
-  } else if (format == FORMAT_BGRA ||
-             (format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) {
-    cinfo.input_components = 4;
-    cinfo.in_color_space = JCS_EXT_BGRX;
-  } else {
-    // We can exit this function without calling jpeg_destroy_compress() because
-    // CompressDestroyer automaticaly calls it.
-    NOTREACHED() << "Invalid pixel format";
-    return false;
-  }
-#else
-  cinfo.in_color_space = JCS_RGB;
-#endif
-  cinfo.data_precision = 8;
-
-  jpeg_set_defaults(&cinfo);
-  jpeg_set_quality(&cinfo, quality, 1);  // quality here is 0-100
-
-  // set up the destination manager
-  jpeg_destination_mgr destmgr;
-  destmgr.init_destination = InitDestination;
-  destmgr.empty_output_buffer = EmptyOutputBuffer;
-  destmgr.term_destination = TermDestination;
-  cinfo.dest = &destmgr;
-
-  JpegEncoderState state(output);
-  cinfo.client_data = &state;
-
-  jpeg_start_compress(&cinfo, 1);
-
-  // feed it the rows, doing necessary conversions for the color format
-#ifdef JCS_EXTENSIONS
-  // This function already returns when the input format is not supported by
-  // libjpeg-turbo and needs conversion. Therefore, we just encode lines without
-  // conversions.
-  while (cinfo.next_scanline < cinfo.image_height) {
-    const unsigned char* row = &input[cinfo.next_scanline * row_byte_width];
-    jpeg_write_scanlines(&cinfo, const_cast<unsigned char**>(&row), 1);
-  }
-#else
-  if (format == FORMAT_RGB) {
-    // no conversion necessary
-    while (cinfo.next_scanline < cinfo.image_height) {
-      const unsigned char* row = &input[cinfo.next_scanline * row_byte_width];
-      jpeg_write_scanlines(&cinfo, const_cast<unsigned char**>(&row), 1);
-    }
-  } else {
-    // get the correct format converter
-    void (*converter)(const unsigned char* in, int w, unsigned char* rgb);
-    if (format == FORMAT_RGBA ||
-        (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
-      converter = StripAlpha;
-    } else if (format == FORMAT_BGRA ||
-               (format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) {
-      converter = BGRAtoRGB;
-    } else {
-      NOTREACHED() << "Invalid pixel format";
-      return false;
-    }
-
-    // output row after converting
-    row_buffer = new unsigned char[w * 3];
-
-    while (cinfo.next_scanline < cinfo.image_height) {
-      converter(&input[cinfo.next_scanline * row_byte_width], w, row_buffer);
-      jpeg_write_scanlines(&cinfo, &row_buffer, 1);
-    }
-    delete[] row_buffer;
-  }
-#endif
-
-  jpeg_finish_compress(&cinfo);
-  return true;
+  return SkJpegEncoder::Encode(&dst, src, options);
 }
 
 // Decoder --------------------------------------------------------------------
@@ -499,13 +278,10 @@ bool JPEGCodec::Decode(const unsigned char* input, size_t input_size,
 #ifdef JCS_EXTENSIONS
       // Choose an output colorspace and return if it is an unsupported one.
       // Same as JPEGCodec::Encode(), libjpeg-turbo supports all input formats
-      // used by Chromium (i.e. RGB, RGBA, and BGRA) and we just map the input
+      // used by Chromium (i.e. RGBA and BGRA) and we just map the input
       // parameters to a colorspace.
-      if (format == FORMAT_RGB) {
-        cinfo.out_color_space = JCS_RGB;
-        cinfo.output_components = 3;
-      } else if (format == FORMAT_RGBA ||
-                 (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
+      if (format == FORMAT_RGBA ||
+          (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
         cinfo.out_color_space = JCS_EXT_RGBX;
         cinfo.output_components = 4;
       } else if (format == FORMAT_BGRA ||
@@ -556,46 +332,33 @@ bool JPEGCodec::Decode(const unsigned char* input, size_t input_size,
       return false;
   }
 #else
-  if (format == FORMAT_RGB) {

[scroggo_chromium, 2017/05/30 14:27:52]

So this CL also removes the ability to decode to RGB? I'm guessing that is never
used? Please add this to the CL description. (Alternatively, should it be a
separate CL?)

[msarett1, 2017/06/07 18:01:14]

Acknowledged.  Removed in a previous CL.

-    // easy case, row needs no conversion
-    int row_write_stride = row_read_stride;
-    output->resize(row_write_stride * cinfo.output_height);
-
-    for (int row = 0; row < static_cast<int>(cinfo.output_height); row++) {
-      unsigned char* rowptr = &(*output)[row * row_write_stride];
-      if (!jpeg_read_scanlines(&cinfo, &rowptr, 1))
-        return false;
-    }
+  // Rows need conversion to output format: read into a temporary buffer and

[scroggo_chromium, 2017/05/30 14:27:52]

The diff doesn't clearly show this, but I think this code just got moved out of
the else case from FORMAT_RGB?

[msarett1, 2017/06/07 18:01:14]

Yes.  Should be clearer now.

+  // expand to the final one. Performance: we could avoid the extra
+  // allocation by doing the expansion in-place.
+  int row_write_stride;
+  void (*converter)(const unsigned char* rgb, int w, unsigned char* out);
+  if (format == FORMAT_RGBA ||
+      (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
+    row_write_stride = cinfo.output_width * 4;
+    converter = AddAlpha;
+  } else if (format == FORMAT_BGRA ||
+             (format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) {
+    row_write_stride = cinfo.output_width * 4;
+    converter = RGBtoBGRA;
   } else {
-    // Rows need conversion to output format: read into a temporary buffer and
-    // expand to the final one. Performance: we could avoid the extra
-    // allocation by doing the expansion in-place.
-    int row_write_stride;
-    void (*converter)(const unsigned char* rgb, int w, unsigned char* out);
-    if (format == FORMAT_RGBA ||
-        (format == FORMAT_SkBitmap && SK_R32_SHIFT == 0)) {
-      row_write_stride = cinfo.output_width * 4;
-      converter = AddAlpha;
-    } else if (format == FORMAT_BGRA ||
-               (format == FORMAT_SkBitmap && SK_B32_SHIFT == 0)) {
-      row_write_stride = cinfo.output_width * 4;
-      converter = RGBtoBGRA;
-    } else {
-      NOTREACHED() << "Invalid pixel format";
-      jpeg_destroy_decompress(&cinfo);
-      return false;
-    }
+    NOTREACHED() << "Invalid pixel format";
+    jpeg_destroy_decompress(&cinfo);
+    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(
-        new unsigned char[row_read_stride]);
-    unsigned char* rowptr = row_data.get();
-    for (int row = 0; row < static_cast<int>(cinfo.output_height); row++) {
-      if (!jpeg_read_scanlines(&cinfo, &rowptr, 1))
-        return false;
-      converter(rowptr, *w, &(*output)[row * row_write_stride]);
-    }
+  std::unique_ptr<unsigned char[]> row_data(new unsigned char[row_read_stride]);
+  unsigned char* rowptr = row_data.get();
+  for (int row = 0; row < static_cast<int>(cinfo.output_height); row++) {
+    if (!jpeg_read_scanlines(&cinfo, &rowptr, 1))
+      return false;
+    converter(rowptr, *w, &(*output)[row * row_write_stride]);
   }
 #endif