Updated the get_images_from_skps tool to check for ICC profile support
Tool will now check for and output all unsuccessfully parsed ICC
profiles in input sksp images if --testColorCorrectionSupported is set
as a flag. All ICC-aware codecs had to be slightly modified in order to
expose this information, as the logic for accessing the ICC profiles is
all within the codecs. If --writeFailedImages is set, it will also
output all images whoses ICC profiles were not supported.
TBR=reed@google.com
BUG=skia:
Change-Id: Ic310d82bdebf92f8d3bc0ad3dcc688136b6de377
Reviewed-on: https://skia-review.googlesource.com/5355
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Robert Aftias <raftias@google.com>
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index 77f68fd..314bec4 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -785,6 +785,8 @@
return 0;
}
+ void setUnsupportedICC(bool SkDEBUGCODE(value)) { SkDEBUGCODE(fUnsupportedICC = value); }
+
private:
const SkEncodedInfo fEncodedInfo;
const SkImageInfo fSrcInfo;
@@ -800,6 +802,9 @@
int fCurrScanline;
bool fStartedIncrementalDecode;
+#ifdef SK_DEBUG
+ bool fUnsupportedICC = false;
+#endif
/**
* Return whether these dimensions are supported as a scale.
@@ -864,5 +869,7 @@
friend class DM::CodecSrc; // for fillIncompleteImage
friend class SkSampledCodec;
friend class SkIcoCodec;
+ friend struct Sniffer; // for fUnsupportedICC
+ friend class AutoCleanPng; // for setUnsupportedICC()
};
#endif // SkCodec_DEFINED
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 9a08755..7867fd0 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -229,6 +229,7 @@
Origin orientation = get_exif_orientation(decoderMgr->dinfo());
sk_sp<SkData> iccData = get_icc_profile(decoderMgr->dinfo());
sk_sp<SkColorSpace> colorSpace = nullptr;
+ bool unsupportedICC = false;
if (iccData) {
SkColorSpace_Base::InputColorFormat inputColorFormat =
SkColorSpace_Base::InputColorFormat::kRGB;
@@ -247,6 +248,7 @@
inputColorFormat);
if (!colorSpace) {
SkCodecPrintf("Could not create SkColorSpace from ICC data.\n");
+ unsupportedICC = true;
}
}
if (!colorSpace) {
@@ -256,8 +258,10 @@
const int width = decoderMgr->dinfo()->image_width;
const int height = decoderMgr->dinfo()->image_height;
- *codecOut = new SkJpegCodec(width, height, info, stream, decoderMgr.release(),
- std::move(colorSpace), orientation);
+ SkJpegCodec* codec = new SkJpegCodec(width, height, info, stream, decoderMgr.release(),
+ std::move(colorSpace), orientation);
+ codec->setUnsupportedICC(unsupportedICC);
+ *codecOut = codec;
} else {
SkASSERT(nullptr != decoderMgrOut);
*decoderMgrOut = decoderMgr.release();
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 758ffa5..b8576fb 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -326,8 +326,8 @@
#endif // LIBPNG >= 1.6
// Returns a colorSpace object that represents any color space information in
-// the encoded data. If the encoded data contains no color space, this will
-// return NULL.
+// the encoded data. If the encoded data contains an invalid/unsupported color space,
+// this will return NULL. If there is no color space information, it will guess sRGB
sk_sp<SkColorSpace> read_color_space(png_structp png_ptr, png_infop info_ptr) {
#if (PNG_LIBPNG_VER_MAJOR > 1) || (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 6)
@@ -410,9 +410,9 @@
#endif // LIBPNG >= 1.6
- // Report that there is no color space information in the PNG. SkPngCodec is currently
- // implemented to guess sRGB in this case.
- return nullptr;
+ // Report that there is no color space information in the PNG.
+ // Guess sRGB in this case.
+ return SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
}
void SkPngCodec::allocateStorage(const SkImageInfo& dstInfo) {
@@ -984,8 +984,9 @@
if (fOutCodec) {
SkASSERT(nullptr == *fOutCodec);
sk_sp<SkColorSpace> colorSpace = read_color_space(fPng_ptr, fInfo_ptr);
+ const bool unsupportedICC = !colorSpace;
if (!colorSpace) {
- // Treat unmarked pngs as sRGB.
+ // Treat unsupported/invalid color spaces as sRGB.
colorSpace = SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
}
@@ -1014,6 +1015,7 @@
*fOutCodec = new SkPngInterlacedDecoder(encodedInfo, imageInfo, fStream,
fChunkReader, fPng_ptr, fInfo_ptr, bitDepth, numberPasses);
}
+ (*fOutCodec)->setUnsupportedICC(unsupportedICC);
}
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 09913c7..dbc141e 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -62,10 +62,13 @@
WebPChunkIterator chunkIterator;
SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
sk_sp<SkColorSpace> colorSpace = nullptr;
+ bool unsupportedICC = false;
if (WebPDemuxGetChunk(demux, "ICCP", 1, &chunkIterator)) {
colorSpace = SkColorSpace::MakeICC(chunkIterator.chunk.bytes, chunkIterator.chunk.size);
+ if (!colorSpace) {
+ unsupportedICC = true;
+ }
}
-
if (!colorSpace) {
colorSpace = SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
}
@@ -140,8 +143,11 @@
}
SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8);
- return new SkWebpCodec(features.width, features.height, info, std::move(colorSpace),
- streamDeleter.release(), demux.release(), std::move(data));
+ SkWebpCodec* codecOut = new SkWebpCodec(features.width, features.height, info,
+ std::move(colorSpace), streamDeleter.release(),
+ demux.release(), std::move(data));
+ codecOut->setUnsupportedICC(unsupportedICC);
+ return codecOut;
}
SkISize SkWebpCodec::onGetScaledDimensions(float desiredScale) const {
diff --git a/tools/get_images_from_skps.cpp b/tools/get_images_from_skps.cpp
index d3a2343..c413c6d 100644
--- a/tools/get_images_from_skps.cpp
+++ b/tools/get_images_from_skps.cpp
@@ -7,6 +7,7 @@
#include "SkBitmap.h"
#include "SkCodec.h"
+#include "SkColorSpace.h"
#include "SkCommandLineFlags.h"
#include "SkData.h"
#include "SkJSONCPP.h"
@@ -19,12 +20,18 @@
#include "SkTHash.h"
+#include <iostream>
#include <map>
DEFINE_string2(skps, s, "skps", "A path to a directory of skps.");
DEFINE_string2(out, o, "img-out", "A path to an output directory.");
DEFINE_bool(testDecode, false, "Indicates if we want to test that the images decode successfully.");
-DEFINE_bool(writeImages, true, "Indicates if we want to write out images.");
+DEFINE_bool(writeImages, true,
+ "Indicates if we want to write out supported/decoded images.");
+DEFINE_bool(writeFailedImages, false,
+ "Indicates if we want to write out unsupported/failed to decode images.");
+DEFINE_bool(testICCSupport, false,
+ "Indicates if we want to test that the images with ICC profiles are supported");
DEFINE_string2(failuresJsonPath, j, "",
"Dump SKP and count of unknown images to the specified JSON file. Will not be "
"written anywhere if empty.");
@@ -32,6 +39,7 @@
static int gKnown;
static const char* gOutputDir;
static std::map<std::string, unsigned int> gSkpToUnknownCount = {};
+static std::map<std::string, unsigned int> gSkpToUnsupportedCount;
static SkTHashSet<SkMD5::Digest> gSeen;
@@ -79,21 +87,7 @@
SkASSERT(false);
}
- if (FLAGS_testDecode) {
- SkBitmap bitmap;
- SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
- bitmap.allocPixels(info);
- const SkCodec::Result result = codec->getPixels(
- info, bitmap.getPixels(), bitmap.rowBytes());
- if (SkCodec::kIncompleteInput != result && SkCodec::kSuccess != result)
- {
- SkDebugf("Decoding failed for %s\n", skpName.c_str());
- gSkpToUnknownCount[skpName]++;
- return;
- }
- }
-
- if (FLAGS_writeImages) {
+ auto writeImage = [&] {
SkString path;
path.appendf("%s/%d.%s", gOutputDir, gKnown, ext.c_str());
@@ -101,7 +95,43 @@
file.write(ptr, len);
SkDebugf("%s\n", path.c_str());
+ };
+
+
+ if (FLAGS_testDecode) {
+ SkBitmap bitmap;
+ SkImageInfo info = codec->getInfo().makeColorType(kN32_SkColorType);
+ bitmap.allocPixels(info);
+ const SkCodec::Result result = codec->getPixels(
+ info, bitmap.getPixels(), bitmap.rowBytes());
+ if (SkCodec::kIncompleteInput != result && SkCodec::kSuccess != result) {
+ SkDebugf("Decoding failed for %s\n", skpName.c_str());
+ gSkpToUnknownCount[skpName]++;
+ if (FLAGS_writeFailedImages) {
+ writeImage();
+ }
+ return;
+ }
}
+
+#ifdef SK_DEBUG
+ if (FLAGS_testICCSupport) {
+ if (codec->fUnsupportedICC) {
+ SkDebugf("Color correction failed for %s\n", skpName.c_str());
+ gSkpToUnsupportedCount[skpName]++;
+ if (FLAGS_writeFailedImages) {
+ writeImage();
+ }
+ return;
+ }
+ }
+#endif
+
+ if (FLAGS_writeImages) {
+ writeImage();
+ }
+
+
gKnown++;
}
@@ -116,7 +146,7 @@
int main(int argc, char** argv) {
SkCommandLineFlags::SetUsage(
"Usage: get_images_from_skps -s <dir of skps> -o <dir for output images> --testDecode "
- "-j <output JSON path>\n");
+ "-j <output JSON path> --testICCSupport --writeImages, --writeFailedImages\n");
SkCommandLineFlags::Parse(argc, argv);
const char* inputs = FLAGS_skps[0];
@@ -126,6 +156,12 @@
SkCommandLineFlags::PrintUsage();
return 1;
}
+#ifndef SK_DEBUG
+ if (FLAGS_testICCSupport) {
+ std::cerr << "--testICCSupport unavailable outside of SK_DEBUG builds" << std::endl;
+ return 1;
+ }
+#endif
SkOSFile::Iter iter(inputs, "skp");
for (SkString file; iter.next(&file); ) {
@@ -137,7 +173,6 @@
Sniffer sniff(file.c_str());
picture->serialize(&scratch, &sniff);
}
- int totalUnknowns = 0;
/**
JSON results are written out in the following format:
{
@@ -146,21 +181,37 @@
"skp4": 2,
...
},
+ "unsupported": {
+ "skp9": 13,
+ "skp17": 3,
+ ...
+ }
"totalFailures": 32,
+ "totalUnsupported": 9,
"totalSuccesses": 21,
}
*/
Json::Value fRoot;
+ int totalFailures = 0;
for(auto it = gSkpToUnknownCount.cbegin(); it != gSkpToUnknownCount.cend(); ++it)
{
SkDebugf("%s %d\n", it->first.c_str(), it->second);
- totalUnknowns += it->second;
+ totalFailures += it->second;
fRoot["failures"][it->first.c_str()] = it->second;
}
- SkDebugf("%d known, %d unknown\n", gKnown, totalUnknowns);
- fRoot["totalFailures"] = totalUnknowns;
+ fRoot["totalFailures"] = totalFailures;
+ int totalUnsupported = 0;
+#ifdef SK_DEBUG
+ for (const auto& unsupported : gSkpToUnsupportedCount) {
+ SkDebugf("%s %d\n", unsupported.first.c_str(), unsupported.second);
+ totalUnsupported += unsupported.second;
+ fRoot["unsupported"][unsupported.first] = unsupported.second;
+ }
+ fRoot["totalUnsupported"] = totalUnsupported;
+#endif
fRoot["totalSuccesses"] = gKnown;
- if (totalUnknowns > 0) {
+ SkDebugf("%d known, %d failures, %d unsupported\n", gKnown, totalFailures, totalUnsupported);
+ if (totalFailures > 0 || totalUnsupported > 0) {
if (!FLAGS_failuresJsonPath.isEmpty()) {
SkDebugf("Writing failures to %s\n", FLAGS_failuresJsonPath[0]);
SkFILEWStream stream(FLAGS_failuresJsonPath[0]);