Revert "Revert "Add support for writing icc profiles to the jpeg encoder""
This reverts commit dda14b9b7ac13dba9214f484fc6270b3ccf4b68b.
Reason for revert: <INSERT REASONING HERE>
Original change's description:
> Revert "Add support for writing icc profiles to the jpeg encoder"
>
> This reverts commit 4ef01482025e2e629e35458aa214436d3b4138e8.
>
> Reason for revert: This breaks the android autoroller.
>
> 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: I4d67f28126388fef3057d62b6e0b203e21ed4afb
> > Reviewed-on: https://skia-review.googlesource.com/10011
> > 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: Idbb9918370e8384e39d6b7d1c3bcd9545ce4cfd1
> Reviewed-on: https://skia-review.googlesource.com/10017
> Reviewed-by: Derek Sollenberger <djsollen@google.com>
> Commit-Queue: Derek Sollenberger <djsollen@google.com>
>
TBR=djsollen@google.com,msarett@google.com,scroggo@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Change-Id: I9c1df3f497a9187ac017e464976fd8f0333bad0e
Reviewed-on: https://skia-review.googlesource.com/10030
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Matt Sarett <msarett@google.com>
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 55ee3b5..624bc25 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -44,9 +44,7 @@
}
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) {
@@ -112,11 +110,10 @@
}
static bool is_icc_marker(jpeg_marker_struct* marker) {
- if (kICCMarker != marker->marker || marker->data_length < kICCHeaderSize) {
+ if (kICCMarker != marker->marker || marker->data_length < kICCMarkerHeaderSize) {
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));
}
@@ -160,8 +157,8 @@
return nullptr;
}
markerSequence[markerIndex] = marker;
- SkASSERT(marker->data_length >= kICCHeaderSize);
- totalBytes += marker->data_length - kICCHeaderSize;
+ SkASSERT(marker->data_length >= kICCMarkerHeaderSize);
+ totalBytes += marker->data_length - kICCMarkerHeaderSize;
}
}
@@ -180,8 +177,8 @@
return nullptr;
}
- void* src = SkTAddOffset<void>(marker->data, kICCHeaderSize);
- size_t bytes = marker->data_length - kICCHeaderSize;
+ void* src = SkTAddOffset<void>(marker->data, kICCMarkerHeaderSize);
+ size_t bytes = marker->data_length - kICCMarkerHeaderSize;
memcpy(dst, src, bytes);
dst = SkTAddOffset<void>(dst, bytes);
}
diff --git a/src/codec/SkJpegPriv.h b/src/codec/SkJpegPriv.h
new file mode 100644
index 0000000..e4e5b12
--- /dev/null
+++ b/src/codec/SkJpegPriv.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+
+#ifndef SkJpegPriv_DEFINED
+#define SkJpegPriv_DEFINED
+
+#include "SkStream.h"
+
+#include <setjmp.h>
+// stdio is needed for jpeglib
+#include <stdio.h>
+
+extern "C" {
+ #include "jpeglib.h"
+ #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
+ */
+struct skjpeg_error_mgr : jpeg_error_mgr {
+ jmp_buf fJmpBuf;
+};
+
+#endif
diff --git a/src/codec/SkJpegUtility.h b/src/codec/SkJpegUtility.h
index 4339101..33f4fbd 100644
--- a/src/codec/SkJpegUtility.h
+++ b/src/codec/SkJpegUtility.h
@@ -9,6 +9,7 @@
#ifndef SkJpegUtility_codec_DEFINED
#define SkJpegUtility_codec_DEFINED
+#include "SkJpegPriv.h"
#include "SkStream.h"
#include <setjmp.h>
@@ -21,13 +22,6 @@
}
/*
- * Error handling struct
- */
-struct skjpeg_error_mgr : jpeg_error_mgr {
- jmp_buf fJmpBuf;
-};
-
-/*
* Error handling function
*/
void skjpeg_err_exit(j_common_ptr cinfo);
diff --git a/src/images/transform_scanline.h b/src/images/SkImageEncoderFns.h
similarity index 94%
rename from src/images/transform_scanline.h
rename to src/images/SkImageEncoderFns.h
index 3c75427..5120570 100644
--- a/src/images/transform_scanline.h
+++ b/src/images/SkImageEncoderFns.h
@@ -4,8 +4,9 @@
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
-#ifndef transform_scanline_DEFINED
-#define transform_scanline_DEFINED
+
+#ifndef SkImageEncoderFns_DEFINED
+#define SkImageEncoderFns_DEFINED
/**
* Functions to transform scanlines between packed-pixel formats.
@@ -14,6 +15,7 @@
#include "SkBitmap.h"
#include "SkColor.h"
#include "SkColorPriv.h"
+#include "SkICC.h"
#include "SkPreConfig.h"
#include "SkRasterPipeline.h"
#include "SkUnPreMultiply.h"
@@ -273,4 +275,16 @@
p.append(SkRasterPipeline::store_8888, (void**) &dst);
p.run(0, width);
}
-#endif // transform_scanline_DEFINED
+
+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
diff --git a/src/images/SkJPEGImageEncoder.cpp b/src/images/SkJPEGImageEncoder.cpp
index 1f8f0d63..014a0ae 100644
--- a/src/images/SkJPEGImageEncoder.cpp
+++ b/src/images/SkJPEGImageEncoder.cpp
@@ -10,10 +10,10 @@
#ifdef SK_HAS_JPEG_LIBRARY
#include "SkColorPriv.h"
+#include "SkImageEncoderFns.h"
#include "SkJPEGWriteUtility.h"
#include "SkStream.h"
#include "SkTemplates.h"
-#include "transform_scanline.h"
#include <stdio.h>
@@ -141,6 +141,23 @@
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.h b/src/images/SkJPEGWriteUtility.h
index 91d07a3..3765e7e 100644
--- a/src/images/SkJPEGWriteUtility.h
+++ b/src/images/SkJPEGWriteUtility.h
@@ -9,6 +9,7 @@
#ifndef SkJpegUtility_DEFINED
#define SkJpegUtility_DEFINED
+#include "SkJpegPriv.h"
#include "SkStream.h"
extern "C" {
@@ -18,14 +19,6 @@
#include <setjmp.h>
-/* Our error-handling struct.
- *
-*/
-struct skjpeg_error_mgr : jpeg_error_mgr {
- jmp_buf fJmpBuf;
-};
-
-
void skjpeg_error_exit(j_common_ptr cinfo);
/////////////////////////////////////////////////////////////////////////////
diff --git a/src/images/SkPNGImageEncoder.cpp b/src/images/SkPNGImageEncoder.cpp
index 2eac91d..e28ae12 100644
--- a/src/images/SkPNGImageEncoder.cpp
+++ b/src/images/SkPNGImageEncoder.cpp
@@ -12,14 +12,13 @@
#include "SkColor.h"
#include "SkColorPriv.h"
#include "SkDither.h"
-#include "SkICC.h"
+#include "SkImageEncoderFns.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"
@@ -40,9 +39,7 @@
}
}
-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);
+static void set_icc(png_structp png_ptr, png_infop info_ptr, sk_sp<SkData> icc) {
#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();
@@ -351,17 +348,14 @@
}
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 if (pixmap.colorSpace()->isNumericalTransferFn(&fn) &&
- pixmap.colorSpace()->toXYZD50(&toXYZD50))
- {
- set_icc(png_ptr, info_ptr, fn, toXYZD50);
+ } else {
+ sk_sp<SkData> icc = icc_from_color_space(*pixmap.colorSpace());
+ if (icc) {
+ set_icc(png_ptr, info_ptr, std::move(icc));
+ }
}
-
- // 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 8797ff5..a9fcc31 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/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 65e5475..da75ffb 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1530,7 +1530,22 @@
}
}
-DEF_TEST(Codec_EncodeICC, 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) {
// Test with sRGB color space.
SkBitmap srgbBitmap;
SkImageInfo srgbInfo = SkImageInfo::MakeS32(1, 1, kOpaque_SkAlphaType);
@@ -1541,7 +1556,7 @@
SkDynamicMemoryWStream srgbBuf;
SkEncodeOptions opts;
opts.fColorBehavior = SkEncodeOptions::ColorBehavior::kCorrect;
- SkEncodeImageAsPNG(&srgbBuf, pixmap, opts);
+ encode_format(&srgbBuf, pixmap, opts, format);
sk_sp<SkData> srgbData = srgbBuf.detachAsData();
std::unique_ptr<SkCodec> srgbCodec(SkCodec::NewFromData(srgbData));
REPORTER_ASSERT(r, srgbCodec->getInfo().colorSpace() == SkColorSpace::MakeSRGB().get());
@@ -1551,7 +1566,7 @@
sk_sp<SkColorSpace> p3 = SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
SkColorSpace::kDCIP3_D65_Gamut);
pixmap.setColorSpace(p3);
- SkEncodeImageAsPNG(&p3Buf, pixmap, opts);
+ encode_format(&p3Buf, pixmap, opts, format);
sk_sp<SkData> p3Data = p3Buf.detachAsData();
std::unique_ptr<SkCodec> p3Codec(SkCodec::NewFromData(p3Data));
REPORTER_ASSERT(r, p3Codec->getInfo().colorSpace()->gammaCloseToSRGB());
@@ -1564,7 +1579,12 @@
for (int i = 0; i < 4; i++) {
for (int j = 0; j < 4; j++) {
- REPORTER_ASSERT(r, color_space_almost_equal(mat0.get(0, 0), mat1.get(0, 0)));
+ REPORTER_ASSERT(r, color_space_almost_equal(mat0.get(i, j), mat1.get(i, j)));
}
}
}
+
+DEF_TEST(Codec_EncodeICC, r) {
+ test_encode_icc(r, SkEncodedImageFormat::kPNG);
+ test_encode_icc(r, SkEncodedImageFormat::kJPEG);
+}
