Revert "Add support for writing icc profiles to the jpeg encoder"

This reverts commit e3b95ce468c73af8247ba9582a7b84548b19f06a.

Reason for revert: This is breaking the Android auto roller. 

Original change's description:
> Add support for writing icc profiles to the jpeg encoder
> 
> Also, share the impl for skjpeg_error_mgr between the
> jpeg decoder and encoder.  They are already identical
> anyway.
> 
> BUG=skia:
> 
> Change-Id: I029312406dee8734744cc3058acd1f153aefcd83
> Reviewed-on: https://skia-review.googlesource.com/9971
> Reviewed-by: Leon Scroggins <scroggo@google.com>
> Commit-Queue: Matt Sarett <msarett@google.com>
> 

TBR=msarett@google.com,scroggo@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Change-Id: I05fb02610dbf4d9475f158d876d6495b0f7e9db4
Reviewed-on: https://skia-review.googlesource.com/10009
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>

9 files changed, 42 insertions, 83 deletions

diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 624bc25845..55ee3b5dbc 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -44,7 +44,9 @@ static uint32_t get_endian_int(const uint8_t* data, bool littleEndian) {
 }
 
 const uint32_t kExifHeaderSize = 14;
+const uint32_t kICCHeaderSize = 14;
 const uint32_t kExifMarker = JPEG_APP0 + 1;
+const uint32_t kICCMarker = JPEG_APP0 + 2;
 
 static bool is_orientation_marker(jpeg_marker_struct* marker, SkCodec::Origin* orientation) {
     if (kExifMarker != marker->marker || marker->data_length < kExifHeaderSize) {
@@ -110,10 +112,11 @@ static SkCodec::Origin get_exif_orientation(jpeg_decompress_struct* dinfo) {
 }
 
 static bool is_icc_marker(jpeg_marker_struct* marker) {
-    if (kICCMarker != marker->marker || marker->data_length < kICCMarkerHeaderSize) {
+    if (kICCMarker != marker->marker || marker->data_length < kICCHeaderSize) {
         return false;
     }
 
+    static const uint8_t kICCSig[] { 'I', 'C', 'C', '_', 'P', 'R', 'O', 'F', 'I', 'L', 'E', '\0' };
     return !memcmp(marker->data, kICCSig, sizeof(kICCSig));
 }
 
@@ -157,8 +160,8 @@ static sk_sp<SkData> get_icc_profile(jpeg_decompress_struct* dinfo) {
                 return nullptr;
             }
             markerSequence[markerIndex] = marker;
-            SkASSERT(marker->data_length >= kICCMarkerHeaderSize);
-            totalBytes += marker->data_length - kICCMarkerHeaderSize;
+            SkASSERT(marker->data_length >= kICCHeaderSize);
+            totalBytes += marker->data_length - kICCHeaderSize;
         }
     }
 
@@ -177,8 +180,8 @@ static sk_sp<SkData> get_icc_profile(jpeg_decompress_struct* dinfo) {
             return nullptr;
         }
 
-        void* src = SkTAddOffset<void>(marker->data, kICCMarkerHeaderSize);
-        size_t bytes = marker->data_length - kICCMarkerHeaderSize;
+        void* src = SkTAddOffset<void>(marker->data, kICCHeaderSize);
+        size_t bytes = marker->data_length - kICCHeaderSize;
         memcpy(dst, src, bytes);
         dst = SkTAddOffset<void>(dst, bytes);
     }
diff --git a/src/codec/SkJpegUtility.h b/src/codec/SkJpegUtility.h
index dea0753537..43391017b5 100644
--- a/src/codec/SkJpegUtility.h
+++ b/src/codec/SkJpegUtility.h
@@ -20,12 +20,6 @@ extern "C" {
     #include "jerror.h"
 }
 
-static constexpr uint32_t kICCMarker = JPEG_APP0 + 2;
-static constexpr uint32_t kICCMarkerHeaderSize = 14;
-static constexpr uint8_t kICCSig[] = {
-        'I', 'C', 'C', '_', 'P', 'R', 'O', 'F', 'I', 'L', 'E', '\0',
-};
-
 /*
  * Error handling struct
  */
diff --git a/src/images/SkJPEGImageEncoder.cpp b/src/images/SkJPEGImageEncoder.cpp
index e87e9aceb8..1f8f0d6394 100644
--- a/src/images/SkJPEGImageEncoder.cpp
+++ b/src/images/SkJPEGImageEncoder.cpp
@@ -10,11 +10,10 @@
 #ifdef SK_HAS_JPEG_LIBRARY
 
 #include "SkColorPriv.h"
-#include "SkImageEncoderFns.h"
-#include "SkJpegUtility.h"
 #include "SkJPEGWriteUtility.h"
 #include "SkStream.h"
 #include "SkTemplates.h"
+#include "transform_scanline.h"
 
 #include <stdio.h>
 
@@ -113,7 +112,7 @@ bool SkEncodeImageAsJPEG(SkWStream* stream, const SkPixmap& pixmap, int quality)
     SkAutoTMalloc<uint8_t>  storage;
 
     cinfo.err = jpeg_std_error(&sk_err);
-    sk_err.error_exit = skjpeg_write_error_exit;
+    sk_err.error_exit = skjpeg_error_exit;
     if (setjmp(sk_err.fJmpBuf)) {
         return false;
     }
@@ -142,23 +141,6 @@ bool SkEncodeImageAsJPEG(SkWStream* stream, const SkPixmap& pixmap, int quality)
 
     jpeg_start_compress(&cinfo, TRUE);
 
-    if (pixmap.colorSpace()) {
-        sk_sp<SkData> icc = icc_from_color_space(*pixmap.colorSpace());
-        if (icc) {
-            // Create a contiguous block of memory with the icc signature followed by the profile.
-            sk_sp<SkData> markerData =
-                    SkData::MakeUninitialized(kICCMarkerHeaderSize + icc->size());
-            uint8_t* ptr = (uint8_t*) markerData->writable_data();
-            memcpy(ptr, kICCSig, sizeof(kICCSig));
-            ptr += sizeof(kICCSig);
-            *ptr++ = 1; // This is the first marker.
-            *ptr++ = 1; // Out of one total markers.
-            memcpy(ptr, icc->data(), icc->size());
-
-            jpeg_write_marker(&cinfo, kICCMarker, markerData->bytes(), markerData->size());
-        }
-    }
-
     if (proc) {
         storage.reset(numComponents * pixmap.width());
     }
diff --git a/src/images/SkJPEGWriteUtility.cpp b/src/images/SkJPEGWriteUtility.cpp
index a5d2483898..aa0d3ec4d9 100644
--- a/src/images/SkJPEGWriteUtility.cpp
+++ b/src/images/SkJPEGWriteUtility.cpp
@@ -5,7 +5,7 @@
  * found in the LICENSE file.
  */
 
-#include "SkJpegUtility.h"
+
 #include "SkJPEGWriteUtility.h"
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -52,7 +52,7 @@ skjpeg_destination_mgr::skjpeg_destination_mgr(SkWStream* stream)
     this->term_destination = sk_term_destination;
 }
 
-void skjpeg_write_error_exit(j_common_ptr cinfo) {
+void skjpeg_error_exit(j_common_ptr cinfo) {
     skjpeg_error_mgr* error = (skjpeg_error_mgr*)cinfo->err;
 
     (*error->output_message) (cinfo);
diff --git a/src/images/SkJPEGWriteUtility.h b/src/images/SkJPEGWriteUtility.h
index 3f44863988..91d07a3616 100644
--- a/src/images/SkJPEGWriteUtility.h
+++ b/src/images/SkJPEGWriteUtility.h
@@ -18,7 +18,15 @@ extern "C" {
 
 #include <setjmp.h>
 
-void skjpeg_write_error_exit(j_common_ptr cinfo);
+/* Our error-handling struct.
+ *
+*/
+struct skjpeg_error_mgr : jpeg_error_mgr {
+    jmp_buf fJmpBuf;
+};
+
+
+void skjpeg_error_exit(j_common_ptr cinfo);
 
 /////////////////////////////////////////////////////////////////////////////
 /* Our destination struct for directing decompressed pixels to our stream
diff --git a/src/images/SkPNGImageEncoder.cpp b/src/images/SkPNGImageEncoder.cpp
index e28ae12dc0..2eac91d0ca 100644
--- a/src/images/SkPNGImageEncoder.cpp
+++ b/src/images/SkPNGImageEncoder.cpp
@@ -12,13 +12,14 @@
 #include "SkColor.h"
 #include "SkColorPriv.h"
 #include "SkDither.h"
-#include "SkImageEncoderFns.h"
+#include "SkICC.h"
 #include "SkMath.h"
 #include "SkStream.h"
 #include "SkString.h"
 #include "SkTemplates.h"
 #include "SkUnPreMultiply.h"
 #include "SkUtils.h"
+#include "transform_scanline.h"
 
 #include "png.h"
 
@@ -39,7 +40,9 @@ static void sk_write_fn(png_structp png_ptr, png_bytep data, png_size_t len) {
     }
 }
 
-static void set_icc(png_structp png_ptr, png_infop info_ptr, sk_sp<SkData> icc) {
+static void set_icc(png_structp png_ptr, png_infop info_ptr, const SkColorSpaceTransferFn& fn,
+                    const SkMatrix44& toXYZD50) {
+    sk_sp<SkData> icc = SkICC::WriteToICC(fn, toXYZD50);
 #if PNG_LIBPNG_VER_MAJOR > 1 || (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 5)
     const char* name = "Skia";
     png_const_bytep iccPtr = icc->bytes();
@@ -348,14 +351,17 @@ static bool do_encode(SkWStream* stream, const SkPixmap& pixmap,
     }
 
     if (pixmap.colorSpace()) {
+        SkColorSpaceTransferFn fn;
+        SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor);
         if (pixmap.colorSpace()->isSRGB()) {
             png_set_sRGB(png_ptr, info_ptr, PNG_sRGB_INTENT_PERCEPTUAL);
-        } else {
-            sk_sp<SkData> icc = icc_from_color_space(*pixmap.colorSpace());
-            if (icc) {
-                set_icc(png_ptr, info_ptr, std::move(icc));
-            }
+        } else if (pixmap.colorSpace()->isNumericalTransferFn(&fn) &&
+                   pixmap.colorSpace()->toXYZD50(&toXYZD50))
+        {
+            set_icc(png_ptr, info_ptr, fn, toXYZD50);
         }
+
+        // TODO: Should we support writing ICC profiles for additional color spaces?
     }
 
     png_set_sBIT(png_ptr, info_ptr, &sig_bit);
diff --git a/src/images/SkWEBPImageEncoder.cpp b/src/images/SkWEBPImageEncoder.cpp
index a9fcc31265..8797ff5925 100644
--- a/src/images/SkWEBPImageEncoder.cpp
+++ b/src/images/SkWEBPImageEncoder.cpp
@@ -20,11 +20,11 @@
 
 #include "SkBitmap.h"
 #include "SkColorPriv.h"
-#include "SkImageEncoderFns.h"
 #include "SkStream.h"
 #include "SkTemplates.h"
 #include "SkUnPreMultiply.h"
 #include "SkUtils.h"
+#include "transform_scanline.h"
 
 // A WebP decoder only, on top of (subset of) libwebp
 // For more information on WebP image format, and libwebp library, see:
diff --git a/src/images/SkImageEncoderFns.h b/src/images/transform_scanline.h
index 5120570c48..3c754275e0 100644
--- a/src/images/SkImageEncoderFns.h
+++ b/src/images/transform_scanline.h
@@ -4,9 +4,8 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
-
-#ifndef SkImageEncoderFns_DEFINED
-#define SkImageEncoderFns_DEFINED
+#ifndef transform_scanline_DEFINED
+#define transform_scanline_DEFINED
 
 /**
  * Functions to transform scanlines between packed-pixel formats.
@@ -15,7 +14,6 @@
 #include "SkBitmap.h"
 #include "SkColor.h"
 #include "SkColorPriv.h"
-#include "SkICC.h"
 #include "SkPreConfig.h"
 #include "SkRasterPipeline.h"
 #include "SkUnPreMultiply.h"
@@ -275,16 +273,4 @@ static inline void transform_scanline_F16_premul_to_8888(char* SK_RESTRICT dst,
     p.append(SkRasterPipeline::store_8888, (void**) &dst);
     p.run(0, width);
 }
-
-static inline sk_sp<SkData> icc_from_color_space(const SkColorSpace& cs) {
-    SkColorSpaceTransferFn fn;
-    SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor);
-    if (cs.isNumericalTransferFn(&fn) && cs.toXYZD50(&toXYZD50)) {
-        return SkICC::WriteToICC(fn, toXYZD50);
-    }
-
-    // TODO: Should we support writing ICC profiles for additional color spaces?
-    return nullptr;
-}
-
-#endif  // SkImageEncoderFns_DEFINED
+#endif  // transform_scanline_DEFINED
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index da75ffb414..65e5475e54 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1530,22 +1530,7 @@ DEF_TEST(Codec_InvalidAnimated, r) {
     }
 }
 
-static void encode_format(SkDynamicMemoryWStream* stream, const SkPixmap& pixmap,
-                          const SkEncodeOptions& opts, SkEncodedImageFormat format) {
-    switch (format) {
-        case SkEncodedImageFormat::kPNG:
-            SkEncodeImageAsPNG(stream, pixmap, opts);
-            break;
-        case SkEncodedImageFormat::kJPEG:
-            SkEncodeImageAsJPEG(stream, pixmap, opts);
-            break;
-        default:
-            SkASSERT(false);
-            break;
-    }
-}
-
-static void test_encode_icc(skiatest::Reporter* r, SkEncodedImageFormat format) {
+DEF_TEST(Codec_EncodeICC, r) {
     // Test with sRGB color space.
     SkBitmap srgbBitmap;
     SkImageInfo srgbInfo = SkImageInfo::MakeS32(1, 1, kOpaque_SkAlphaType);
@@ -1556,7 +1541,7 @@ static void test_encode_icc(skiatest::Reporter* r, SkEncodedImageFormat format)
     SkDynamicMemoryWStream srgbBuf;
     SkEncodeOptions opts;
     opts.fColorBehavior = SkEncodeOptions::ColorBehavior::kCorrect;
-    encode_format(&srgbBuf, pixmap, opts, format);
+    SkEncodeImageAsPNG(&srgbBuf, pixmap, opts);
     sk_sp<SkData> srgbData = srgbBuf.detachAsData();
     std::unique_ptr<SkCodec> srgbCodec(SkCodec::NewFromData(srgbData));
     REPORTER_ASSERT(r, srgbCodec->getInfo().colorSpace() == SkColorSpace::MakeSRGB().get());
@@ -1566,7 +1551,7 @@ static void test_encode_icc(skiatest::Reporter* r, SkEncodedImageFormat format)
     sk_sp<SkColorSpace> p3 = SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
                                                    SkColorSpace::kDCIP3_D65_Gamut);
     pixmap.setColorSpace(p3);
-    encode_format(&p3Buf, pixmap, opts, format);
+    SkEncodeImageAsPNG(&p3Buf, pixmap, opts);
     sk_sp<SkData> p3Data = p3Buf.detachAsData();
     std::unique_ptr<SkCodec> p3Codec(SkCodec::NewFromData(p3Data));
     REPORTER_ASSERT(r, p3Codec->getInfo().colorSpace()->gammaCloseToSRGB());
@@ -1579,12 +1564,7 @@ static void test_encode_icc(skiatest::Reporter* r, SkEncodedImageFormat format)
 
     for (int i = 0; i < 4; i++) {
         for (int j = 0; j < 4; j++) {
-            REPORTER_ASSERT(r, color_space_almost_equal(mat0.get(i, j), mat1.get(i, j)));
+            REPORTER_ASSERT(r, color_space_almost_equal(mat0.get(0, 0), mat1.get(0, 0)));
         }
     }
 }
-
-DEF_TEST(Codec_EncodeICC, r) {
-    test_encode_icc(r, SkEncodedImageFormat::kPNG);
-    test_encode_icc(r, SkEncodedImageFormat::kJPEG);
-}