From d737bee1470bbba8af5c9d74cbec2d731da33804 Mon Sep 17 00:00:00 2001 From: raftias Date: Thu, 8 Dec 2016 10:53:24 -0500 Subject: [PATCH] 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 Reviewed-by: Matt Sarett Commit-Queue: Robert Aftias --- include/codec/SkCodec.h | 7 +++ src/codec/SkJpegCodec.cpp | 8 +++- src/codec/SkPngCodec.cpp | 14 +++--- src/codec/SkWebpCodec.cpp | 12 +++-- tools/get_images_from_skps.cpp | 81 +++++++++++++++++++++++++++------- 5 files changed, 96 insertions(+), 26 deletions(-) diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h index 77f68fd761..314bec461c 100644 --- a/include/codec/SkCodec.h +++ b/include/codec/SkCodec.h @@ -785,6 +785,8 @@ protected: return 0; } + void setUnsupportedICC(bool SkDEBUGCODE(value)) { SkDEBUGCODE(fUnsupportedICC = value); } + private: const SkEncodedInfo fEncodedInfo; const SkImageInfo fSrcInfo; @@ -800,6 +802,9 @@ private: int fCurrScanline; bool fStartedIncrementalDecode; +#ifdef SK_DEBUG + bool fUnsupportedICC = false; +#endif /** * Return whether these dimensions are supported as a scale. @@ -864,5 +869,7 @@ private: 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 9a08755b64..7867fd0e8b 100644 --- a/src/codec/SkJpegCodec.cpp +++ b/src/codec/SkJpegCodec.cpp @@ -229,6 +229,7 @@ bool SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut, Origin orientation = get_exif_orientation(decoderMgr->dinfo()); sk_sp iccData = get_icc_profile(decoderMgr->dinfo()); sk_sp colorSpace = nullptr; + bool unsupportedICC = false; if (iccData) { SkColorSpace_Base::InputColorFormat inputColorFormat = SkColorSpace_Base::InputColorFormat::kRGB; @@ -247,6 +248,7 @@ bool SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut, inputColorFormat); if (!colorSpace) { SkCodecPrintf("Could not create SkColorSpace from ICC data.\n"); + unsupportedICC = true; } } if (!colorSpace) { @@ -256,8 +258,10 @@ bool SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut, 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 758ffa5130..b8576fbc6c 100644 --- a/src/codec/SkPngCodec.cpp +++ b/src/codec/SkPngCodec.cpp @@ -326,8 +326,8 @@ static constexpr float gSRGB_toXYZD50[] { #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 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 @@ sk_sp read_color_space(png_structp png_ptr, png_infop info_ptr) { #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 @@ void AutoCleanPng::infoCallback() { if (fOutCodec) { SkASSERT(nullptr == *fOutCodec); sk_sp 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 @@ void AutoCleanPng::infoCallback() { *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 09913c7919..dbc141ebd6 100644 --- a/src/codec/SkWebpCodec.cpp +++ b/src/codec/SkWebpCodec.cpp @@ -62,10 +62,13 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) { WebPChunkIterator chunkIterator; SkAutoTCallVProc autoCI(&chunkIterator); sk_sp 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 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) { } 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 d3a2343b88..c413c6d961 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 #include 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 @@ DEFINE_string2(failuresJsonPath, j, "", static int gKnown; static const char* gOutputDir; static std::map gSkpToUnknownCount = {}; +static std::map gSkpToUnsupportedCount; static SkTHashSet gSeen; @@ -79,29 +87,51 @@ struct Sniffer : public SkPixelSerializer { SkASSERT(false); } + auto writeImage = [&] { + SkString path; + path.appendf("%s/%d.%s", gOutputDir, gKnown, ext.c_str()); + + SkFILEWStream file(path.c_str()); + 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) - { + 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) { - SkString path; - path.appendf("%s/%d.%s", gOutputDir, gKnown, ext.c_str()); + writeImage(); + } - SkFILEWStream file(path.c_str()); - file.write(ptr, len); - SkDebugf("%s\n", path.c_str()); - } gKnown++; } @@ -116,7 +146,7 @@ struct Sniffer : public SkPixelSerializer { int main(int argc, char** argv) { SkCommandLineFlags::SetUsage( "Usage: get_images_from_skps -s -o --testDecode " - "-j \n"); + "-j --testICCSupport --writeImages, --writeFailedImages\n"); SkCommandLineFlags::Parse(argc, argv); const char* inputs = FLAGS_skps[0]; @@ -126,6 +156,12 @@ int main(int argc, char** argv) { 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 @@ int main(int argc, char** argv) { 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 @@ int main(int argc, char** argv) { "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]); -- 2.34.1