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 77f68fd761695c64528c56e5001b2fbbb8cf3643..314bec461c6ebe81edf116c9805f7fa3f9a06945 100644 (file)
--- 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 9a08755b644e25d44948955e7e40fa6a5ee6b9ad..7867fd0e8be59e7cc9440658d3cdfcc996bff889 100644 (file)
--- 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<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 @@ 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 758ffa51305509d8c8750ff536bf511b9fcb9ef4..b8576fbc6ceb903e5aaeac28dff12c69edc2f5c1 100644 (file)
--- 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<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 @@ sk_sp<SkColorSpace> 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<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 @@ 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 09913c791942a26ad9ef71f356e0f4de295bf51c..dbc141ebd67d89b752a1d6503d5548a48cbd77bd 100644 (file)
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -62,10 +62,13 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
     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 @@ 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 d3a2343b88cf79791d3d73f67c726c1b29affa1b..c413c6d96120fd8182042b111b978d796e7f6617 100644 (file)
--- 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 @@ DEFINE_string2(failuresJsonPath, j, "",
 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,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 <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 @@ 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]);