Add SkCodec methods for individual frames

Add a version of getFrameInfo that returns information about a single
frame, allowing a client to skip creating the entire vector.

Add getFrameCount, for determining the number of frames in the image.

Reimplement std::vector<FrameInfo> getFrameInfo with the new methods.

Updates to the test:
- getFrameInfo(size_t, FrameInfo*) fails before parsing
- Test both versions of getFrameInfo
- Recreate the codec between tests, to test parsing

Change-Id: I77c19087f2f8dcf2c536d80167b18ad1ca96ae94
Reviewed-on: https://skia-review.googlesource.com/13190
Reviewed-by: Matt Sarett <msarett@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Chris Blume <cblume@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index 6d4352214cf4f2ba5a6d655f9e8bc73bbed52409..aadcb059fe16450a83368a4635c4a622577ebca5 100644 (file)
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -593,6 +593,15 @@ public:
      */
     int outputScanline(int inputScanline) const;
 
+    /**
+     *  Return the number of frames in the image.
+     *
+     *  May require reading through the stream.
+     */
+    size_t getFrameCount() {
+        return this->onGetFrameCount();
+    }
+
     // The required frame for an independent frame is marked as
     // kNone.
     static constexpr size_t kNone = static_cast<size_t>(-1);
@@ -628,7 +637,18 @@ public:
     };
 
     /**
-     *  Return info about the frames in the image.
+     *  Return info about a single frame.
+     *
+     *  Only supported by multi-frame images. Does not read through the stream,
+     *  so it should be called after getFrameCount() to parse any frames that
+     *  have not already been parsed.
+     */
+    bool getFrameInfo(size_t index, FrameInfo* info) const {
+        return this->onGetFrameInfo(index, info);
+    }
+
+    /**
+     *  Return info about all the frames in the image.
      *
      *  May require reading through the stream to determine info about the
      *  frames (including the count).
@@ -637,9 +657,7 @@ public:
      *
      *  For single-frame images, this will return an empty vector.
      */
-    std::vector<FrameInfo> getFrameInfo() {
-        return this->onGetFrameInfo();
-    }
+    std::vector<FrameInfo> getFrameInfo();
 
     static constexpr int kRepetitionCountInfinite = -1;
 
@@ -800,9 +818,12 @@ protected:
                               SkTransferFunctionBehavior premulBehavior);
     SkColorSpaceXform* colorXform() const { return fColorXform.get(); }
 
-    virtual std::vector<FrameInfo> onGetFrameInfo() {
-        // empty vector - this is not animated.
-        return std::vector<FrameInfo>{};
+    virtual size_t onGetFrameCount() {
+        return 1;
+    }
+
+    virtual bool onGetFrameInfo(size_t, FrameInfo*) const {
+        return false;
     }
 
     virtual int onGetRepetitionCount() {

diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 5bdb3e2b59d9deb5ab3486b95adbb00ef18e2b4d..6a6fdc78c9ce75e4c90e3090cfeb29a43ba6460c 100644 (file)
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -489,3 +489,24 @@ bool SkCodec::initializeColorXform(const SkImageInfo& dstInfo,
 
     return true;
 }
+
+std::vector<SkCodec::FrameInfo> SkCodec::getFrameInfo() {
+    const size_t frameCount = this->getFrameCount();
+    switch (frameCount) {
+        case 0:
+            return std::vector<FrameInfo>{};
+        case 1:
+            if (!this->onGetFrameInfo(0, nullptr)) {
+                // Not animated.
+                return std::vector<FrameInfo>{};
+            }
+            // fall through
+        default: {
+            std::vector<FrameInfo> result(frameCount);
+            for (size_t i = 0; i < frameCount; ++i) {
+                SkAssertResult(this->onGetFrameInfo(i, &result[i]));
+            }
+            return result;
+        }
+    }
+}

diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 06c080313690d0b241c1aaf9424b46d3cd76af6b..2e0ec3057de06bc7524f81eb36caf4c7a2fef19e 100644 (file)
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -132,19 +132,29 @@ SkGifCodec::SkGifCodec(const SkEncodedInfo& encodedInfo, const SkImageInfo& imag
     reader->setClient(this);
 }
 
-std::vector<SkCodec::FrameInfo> SkGifCodec::onGetFrameInfo() {
+size_t SkGifCodec::onGetFrameCount() {
     fReader->parse(SkGifImageReader::SkGIFFrameCountQuery);
-    const size_t size = fReader->imagesCount();
-    std::vector<FrameInfo> result(size);
-    for (size_t i = 0; i < size; i++) {
-        const SkGIFFrameContext* frameContext = fReader->frameContext(i);
-        result[i].fDuration = frameContext->delayTime();
-        result[i].fRequiredFrame = frameContext->getRequiredFrame();
-        result[i].fFullyReceived = frameContext->isComplete();
-        result[i].fAlphaType = frameContext->hasAlpha() ? kUnpremul_SkAlphaType
-                                                        : kOpaque_SkAlphaType;
+    return fReader->imagesCount();
+}
+
+bool SkGifCodec::onGetFrameInfo(size_t i, SkCodec::FrameInfo* frameInfo) const {
+    if (i >= fReader->imagesCount()) {
+        return false;
     }
-    return result;
+
+    const SkGIFFrameContext* frameContext = fReader->frameContext(i);
+    if (!frameContext->reachedStartOfData()) {
+        return false;
+    }
+
+    if (frameInfo) {
+        frameInfo->fDuration = frameContext->delayTime();
+        frameInfo->fRequiredFrame = frameContext->getRequiredFrame();
+        frameInfo->fFullyReceived = frameContext->isComplete();
+        frameInfo->fAlphaType = frameContext->hasAlpha() ? kUnpremul_SkAlphaType
+                                                         : kOpaque_SkAlphaType;
+    }
+    return true;
 }
 
 int SkGifCodec::onGetRepetitionCount() {

diff --git a/src/codec/SkGifCodec.h b/src/codec/SkGifCodec.h
index 67654d3b550769f2a51ff01a67c17b5a51e4fc63..11714eb39edc0decc420b327d20f18d8d3f5ce23 100644 (file)
--- a/src/codec/SkGifCodec.h
+++ b/src/codec/SkGifCodec.h
@@ -50,7 +50,8 @@ protected:
 
     uint64_t onGetFillValue(const SkImageInfo&) const override;
 
-    std::vector<FrameInfo> onGetFrameInfo() override;
+    size_t onGetFrameCount() override;
+    bool onGetFrameInfo(size_t, FrameInfo*) const override;
     int onGetRepetitionCount() override;
 
     Result onStartIncrementalDecode(const SkImageInfo& /*dstInfo*/, void*, size_t,

diff --git a/tests/CodecAnimTest.cpp b/tests/CodecAnimTest.cpp
index dea4adbfd20e6d9b437c683800ba42620842e8e6..02253ef64f582751d25bd82ca670104b5c5ab029 100644 (file)
--- a/tests/CodecAnimTest.cpp
+++ b/tests/CodecAnimTest.cpp
@@ -94,20 +94,25 @@ DEF_TEST(Codec_frames, r) {
     #undef kUnpremul
 
     for (const auto& rec : gRecs) {
-        std::unique_ptr<SkStream> stream(GetResourceAsStream(rec.fName));
-        if (!stream) {
+        sk_sp<SkData> data(GetResourceAsData(rec.fName));
+        if (!data) {
             // Useful error statement, but sometimes people run tests without
             // resources, and they do not want to see these messages.
             //ERRORF(r, "Missing resources? Could not find '%s'", rec.fName);
             continue;
         }
 
-        std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
+        std::unique_ptr<SkCodec> codec(SkCodec::NewFromData(data));
         if (!codec) {
             ERRORF(r, "Failed to create an SkCodec from '%s'", rec.fName);
             continue;
         }
 
+        {
+            SkCodec::FrameInfo frameInfo;
+            REPORTER_ASSERT(r, !codec->getFrameInfo(0, &frameInfo));
+        }
+
         const int repetitionCount = codec->getRepetitionCount();
         if (repetitionCount != rec.fRepetitionCount) {
             ERRORF(r, "%s repetition count does not match! expected: %i\tactual: %i",
@@ -115,112 +120,153 @@ DEF_TEST(Codec_frames, r) {
         }
 
         const size_t expected = rec.fFrameCount;
-        const auto frameInfos = codec->getFrameInfo();
-        // getFrameInfo returns empty set for non-animated.
-        const size_t frameCount = frameInfos.size() == 0 ? 1 : frameInfos.size();
-        if (frameCount != expected) {
-            ERRORF(r, "'%s' expected frame count: %i\tactual: %i", rec.fName, expected, frameCount);
-            continue;
-        }
-
         if (rec.fRequiredFrames.size() + 1 != expected) {
             ERRORF(r, "'%s' has wrong number entries in fRequiredFrames; expected: %i\tactual: %i",
                    rec.fName, expected, rec.fRequiredFrames.size() + 1);
             continue;
         }
 
-        if (1 == frameCount) {
+        if (rec.fDurations.size() != expected) {
+            ERRORF(r, "'%s' has wrong number entries in fDurations; expected: %i\tactual: %i",
+                   rec.fName, expected, rec.fDurations.size());
             continue;
         }
 
-        auto to_string = [](SkAlphaType type) {
-            switch (type) {
-                case kUnpremul_SkAlphaType:
-                    return "unpremul";
-                case kOpaque_SkAlphaType:
-                    return "opaque";
-                default:
-                    return "other";
-            }
+        enum class TestMode {
+            kVector,
+            kIndividual,
         };
-        // From here on, we are only concerned with animated images.
-        REPORTER_ASSERT(r, frameInfos[0].fRequiredFrame == SkCodec::kNone);
-        REPORTER_ASSERT(r, frameInfos[0].fAlphaType == codec->getInfo().alphaType());
-        for (size_t i = 1; i < frameCount; i++) {
-            if (rec.fRequiredFrames[i-1] != frameInfos[i].fRequiredFrame) {
-                ERRORF(r, "%s's frame %i has wrong dependency! expected: %i\tactual: %i",
-                       rec.fName, i, rec.fRequiredFrames[i-1], frameInfos[i].fRequiredFrame);
+
+        for (auto mode : { TestMode::kVector, TestMode::kIndividual }) {
+            // Re-create the codec to reset state and test parsing.
+            codec.reset(SkCodec::NewFromData(data));
+
+            size_t frameCount;
+            std::vector<SkCodec::FrameInfo> frameInfos;
+            switch (mode) {
+                case TestMode::kVector:
+                    frameInfos = codec->getFrameInfo();
+                    // getFrameInfo returns empty set for non-animated.
+                    frameCount = frameInfos.empty() ? 1 : frameInfos.size();
+                    break;
+                case TestMode::kIndividual:
+                    frameCount = codec->getFrameCount();
+                    break;
             }
-            auto expectedAlpha = rec.fAlphaTypes[i-1];
-            auto alpha = frameInfos[i].fAlphaType;
-            if (expectedAlpha != alpha) {
-                ERRORF(r, "%s's frame %i has wrong alpha type! expected: %s\tactual: %s",
-                       rec.fName, i, to_string(expectedAlpha), to_string(alpha));
+
+            if (frameCount != expected) {
+                ERRORF(r, "'%s' expected frame count: %i\tactual: %i",
+                       rec.fName, expected, frameCount);
+                continue;
             }
-        }
 
-        // Compare decoding in two ways:
-        // 1. Provide the frame that a frame depends on, so the codec just has to blend.
-        //    (in the array cachedFrames)
-        // 2. Do not provide the frame that a frame depends on, so the codec has to decode all the
-        //    way back to a key-frame. (in a local variable uncachedFrame)
-        // The two should look the same.
-        std::vector<SkBitmap> cachedFrames(frameCount);
-        const auto& info = codec->getInfo().makeColorType(kN32_SkColorType);
-
-        auto decode = [&](SkBitmap* bm, bool cached, size_t index) {
-            bm->allocPixels(info);
-            if (cached) {
-                // First copy the pixels from the cached frame
-                const size_t requiredFrame = frameInfos[index].fRequiredFrame;
-                if (requiredFrame != SkCodec::kNone) {
-                    const bool success = cachedFrames[requiredFrame].copyTo(bm);
-                    REPORTER_ASSERT(r, success);
-                }
+            // From here on, we are only concerned with animated images.
+            if (1 == frameCount) {
+                continue;
             }
-            SkCodec::Options opts;
-            opts.fFrameIndex = index;
-            opts.fHasPriorFrame = cached;
-            const SkCodec::Result result = codec->getPixels(info, bm->getPixels(), bm->rowBytes(),
-                                                            &opts, nullptr, nullptr);
-            REPORTER_ASSERT(r, result == SkCodec::kSuccess);
-        };
 
-        for (size_t i = 0; i < frameCount; i++) {
-            SkBitmap& cachedFrame = cachedFrames[i];
-            decode(&cachedFrame, true, i);
-            SkBitmap uncachedFrame;
-            decode(&uncachedFrame, false, i);
-
-            // Now verify they're equal.
-            const size_t rowLen = info.bytesPerPixel() * info.width();
-            for (int y = 0; y < info.height(); y++) {
-                const void* cachedAddr = cachedFrame.getAddr(0, y);
-                SkASSERT(cachedAddr != nullptr);
-                const void* uncachedAddr = uncachedFrame.getAddr(0, y);
-                SkASSERT(uncachedAddr != nullptr);
-                const bool lineMatches = memcmp(cachedAddr, uncachedAddr, rowLen) == 0;
-                if (!lineMatches) {
-                    SkString name = SkStringPrintf("cached_%i", i);
-                    write_bm(name.c_str(), cachedFrame);
-                    name = SkStringPrintf("uncached_%i", i);
-                    write_bm(name.c_str(), uncachedFrame);
-                    ERRORF(r, "%s's frame %i is different depending on caching!", rec.fName, i);
-                    break;
+            for (size_t i = 0; i < frameCount; i++) {
+                SkCodec::FrameInfo frameInfo;
+                switch (mode) {
+                    case TestMode::kVector:
+                        frameInfo = frameInfos[i];
+                        break;
+                    case TestMode::kIndividual:
+                        REPORTER_ASSERT(r, codec->getFrameInfo(i, nullptr));
+                        REPORTER_ASSERT(r, codec->getFrameInfo(i, &frameInfo));
+                        break;
+                }
+
+                if (rec.fDurations[i] != frameInfo.fDuration) {
+                    ERRORF(r, "%s frame %i's durations do not match! expected: %i\tactual: %i",
+                           rec.fName, i, rec.fDurations[i], frameInfo.fDuration);
+                }
+
+                if (0 == i) {
+                    REPORTER_ASSERT(r, frameInfo.fRequiredFrame == SkCodec::kNone);
+                    REPORTER_ASSERT(r, frameInfo.fAlphaType == codec->getInfo().alphaType());
+                    continue;
+                }
+
+                if (rec.fRequiredFrames[i-1] != frameInfo.fRequiredFrame) {
+                    ERRORF(r, "%s's frame %i has wrong dependency! expected: %i\tactual: %i",
+                           rec.fName, i, rec.fRequiredFrames[i-1], frameInfo.fRequiredFrame);
+                }
+
+                auto to_string = [](SkAlphaType type) {
+                    switch (type) {
+                        case kUnpremul_SkAlphaType:
+                            return "unpremul";
+                        case kOpaque_SkAlphaType:
+                            return "opaque";
+                        default:
+                            return "other";
+                    }
+                };
+
+                auto expectedAlpha = rec.fAlphaTypes[i-1];
+                auto alpha = frameInfo.fAlphaType;
+                if (expectedAlpha != alpha) {
+                    ERRORF(r, "%s's frame %i has wrong alpha type! expected: %s\tactual: %s",
+                           rec.fName, i, to_string(expectedAlpha), to_string(alpha));
                 }
             }
-        }
 
-        if (rec.fDurations.size() != expected) {
-            ERRORF(r, "'%s' has wrong number entries in fDurations; expected: %i\tactual: %i",
-                   rec.fName, expected, rec.fDurations.size());
-            continue;
-        }
+            if (TestMode::kIndividual == mode) {
+                // No need to test decoding twice.
+                return;
+            }
+
+            // Compare decoding in two ways:
+            // 1. Provide the frame that a frame depends on, so the codec just has to blend.
+            //    (in the array cachedFrames)
+            // 2. Do not provide the frame that a frame depends on, so the codec has to decode
+            //    all the way back to a key-frame. (in a local variable uncachedFrame)
+            // The two should look the same.
+            std::vector<SkBitmap> cachedFrames(frameCount);
+            const auto& info = codec->getInfo().makeColorType(kN32_SkColorType);
 
-        for (size_t i = 0; i < frameCount; i++) {
-            if (rec.fDurations[i] != frameInfos[i].fDuration) {
-                ERRORF(r, "%s frame %i's durations do not match! expected: %i\tactual: %i",
-                       rec.fName, i, rec.fDurations[i], frameInfos[i].fDuration);
+            auto decode = [&](SkBitmap* bm, bool cached, size_t index) {
+                bm->allocPixels(info);
+                if (cached) {
+                    // First copy the pixels from the cached frame
+                    const size_t requiredFrame = frameInfos[index].fRequiredFrame;
+                    if (requiredFrame != SkCodec::kNone) {
+                        const bool success = cachedFrames[requiredFrame].copyTo(bm);
+                        REPORTER_ASSERT(r, success);
+                    }
+                }
+                SkCodec::Options opts;
+                opts.fFrameIndex = index;
+                opts.fHasPriorFrame = cached;
+                auto result = codec->getPixels(info, bm->getPixels(), bm->rowBytes(),
+                                               &opts, nullptr, nullptr);
+                REPORTER_ASSERT(r, result == SkCodec::kSuccess);
+            };
+
+            for (size_t i = 0; i < frameCount; i++) {
+                SkBitmap& cachedFrame = cachedFrames[i];
+                decode(&cachedFrame, true, i);
+                SkBitmap uncachedFrame;
+                decode(&uncachedFrame, false, i);
+
+                // Now verify they're equal.
+                const size_t rowLen = info.bytesPerPixel() * info.width();
+                for (int y = 0; y < info.height(); y++) {
+                    const void* cachedAddr = cachedFrame.getAddr(0, y);
+                    SkASSERT(cachedAddr != nullptr);
+                    const void* uncachedAddr = uncachedFrame.getAddr(0, y);
+                    SkASSERT(uncachedAddr != nullptr);
+                    const bool lineMatches = memcmp(cachedAddr, uncachedAddr, rowLen) == 0;
+                    if (!lineMatches) {
+                        SkString name = SkStringPrintf("cached_%i", i);
+                        write_bm(name.c_str(), cachedFrame);
+                        name = SkStringPrintf("uncached_%i", i);
+                        write_bm(name.c_str(), uncachedFrame);
+                        ERRORF(r, "%s's frame %i is different depending on caching!", rec.fName, i);
+                        break;
+                    }
+                }
             }
         }
     }