Report repetition count in SkCodec
Add a new accessor to retrieve the repetition count.
Remove constants (and corresponding copyright) in SkCodecAnimation.
These may make sense for the calling code, but are not needed here.
kRepetitionCountInfinite corresponds to Blink's kAnimationLoopInfinite.
Move cLoopCountNotSeen to private. It is used to determine whether we
still need to parse. Add a new enum to the parse query - only parse
enough to determine the repetition count.
Unlike Chromium, SkGifCodec does not account for deleting the reader
(which SkGifCodec does not do) or failed decodes.
Add a test.
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2447863002
Review-Url: https://codereview.chromium.org/2447863002
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index c2519cb..f2a717c 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -607,8 +607,8 @@
/**
* Return info about the frames in the image.
*
- * May require reading through the stream to determine the number of
- * frames.
+ * May require reading through the stream to determine info about the
+ * frames (including the count).
*
* As such, future decoding calls may require a rewind.
*
@@ -618,6 +618,21 @@
return this->onGetFrameInfo();
}
+ static constexpr int kRepetitionCountInfinite = -1;
+
+ /**
+ * Return the number of times to repeat, if this image is animated.
+ *
+ * May require reading the stream to find the repetition count.
+ *
+ * As such, future decoding calls may require a rewind.
+ *
+ * For single-frame images, this will return 0.
+ */
+ int getRepetitionCount() {
+ return this->onGetRepetitionCount();
+ }
+
protected:
/**
* Takes ownership of SkStream*
@@ -766,6 +781,10 @@
return {};
}
+ virtual int onGetRepetitionCount() {
+ return 0;
+ }
+
/**
* Used for testing with qcms.
* FIXME: Remove this when we are done comparing with qcms.
diff --git a/resources/colorTables.gif b/resources/colorTables.gif
index f25d13c..cefd412 100644
--- a/resources/colorTables.gif
+++ b/resources/colorTables.gif
Binary files differ
diff --git a/src/codec/SkCodecAnimation.h b/src/codec/SkCodecAnimation.h
index d3fc553..b0495b8 100644
--- a/src/codec/SkCodecAnimation.h
+++ b/src/codec/SkCodecAnimation.h
@@ -5,31 +5,6 @@
* found in the LICENSE file.
*/
-/*
- * Copyright (C) 2015 Google Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE COMPUTER, INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
#ifndef SkCodecAnimation_DEFINED
#define SkCodecAnimation_DEFINED
@@ -38,25 +13,6 @@
class SkCodecAnimation {
public:
-
- // GIF and WebP support animation. The explanation below is in terms of GIF,
- // but the same constants are used for WebP, too.
- // GIFs have an optional 16-bit unsigned loop count that describes how an
- // animated GIF should be cycled. If the loop count is absent, the animation
- // cycles once; if it is 0, the animation cycles infinitely; otherwise the
- // animation plays n + 1 cycles (where n is the specified loop count). If the
- // GIF decoder defaults to kAnimationLoopOnce in the absence of any loop count
- // and translates an explicit "0" loop count to kAnimationLoopInfinite, then we
- // get a couple of nice side effects:
- // * By making kAnimationLoopOnce be 0, we allow the animation cycling code to
- // avoid special-casing it, and simply treat all non-negative loop counts
- // identically.
- // * By making the other two constants negative, we avoid conflicts with any
- // real loop count values.
- static const int kAnimationLoopOnce = 0;
- static const int kAnimationLoopInfinite = -1;
- static const int kAnimationNone = -2;
-
/**
* This specifies how the next frame is based on this frame.
*
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 1322396..277da03 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -139,6 +139,11 @@
return result;
}
+int SkGifCodec::onGetRepetitionCount() {
+ fReader->parse(SkGifImageReader::SkGIFLoopCountQuery);
+ return fReader->loopCount();
+}
+
void SkGifCodec::initializeColorTable(const SkImageInfo& dstInfo, size_t frameIndex) {
fCurrColorTable = fReader->getColorTable(dstInfo.colorType(), frameIndex);
fCurrColorTableIsReal = fCurrColorTable;
diff --git a/src/codec/SkGifCodec.h b/src/codec/SkGifCodec.h
index 9d8e0e4..5793766 100644
--- a/src/codec/SkGifCodec.h
+++ b/src/codec/SkGifCodec.h
@@ -49,6 +49,7 @@
uint64_t onGetFillValue(const SkImageInfo&) const override;
std::vector<FrameInfo> onGetFrameInfo() override;
+ int onGetRepetitionCount() override;
Result onStartIncrementalDecode(const SkImageInfo& /*dstInfo*/, void*, size_t,
const SkCodec::Options&, SkPMColor*, int*) override;
diff --git a/tests/CodecAnimTest.cpp b/tests/CodecAnimTest.cpp
index de36ce6..762cd47 100644
--- a/tests/CodecAnimTest.cpp
+++ b/tests/CodecAnimTest.cpp
@@ -25,21 +25,23 @@
// The size of this one should match fFrameCount for animated, empty
// otherwise.
std::vector<size_t> fDurations;
+ int fRepetitionCount;
} gRecs[] = {
- { "box.gif", 1, {}, {} },
- { "color_wheel.gif", 1, {}, {} },
- { "test640x479.gif", 4, { 0, 1, 2 }, { 200, 200, 200, 200 } },
- { "colorTables.gif", 2, { 0 }, { 1000, 1000 } },
+ { "box.gif", 1, {}, {}, 0 },
+ { "color_wheel.gif", 1, {}, {}, 0 },
+ { "test640x479.gif", 4, { 0, 1, 2 }, { 200, 200, 200, 200 },
+ SkCodec::kRepetitionCountInfinite },
+ { "colorTables.gif", 2, { 0 }, { 1000, 1000 }, 5 },
- { "arrow.png", 1, {}, {} },
- { "google_chrome.ico", 1, {}, {} },
- { "brickwork-texture.jpg", 1, {}, {} },
+ { "arrow.png", 1, {}, {}, 0 },
+ { "google_chrome.ico", 1, {}, {}, 0 },
+ { "brickwork-texture.jpg", 1, {}, {}, 0 },
#if defined(SK_CODEC_DECODES_RAW) && (!defined(_WIN32))
- { "dng_with_preview.dng", 1, {}, {} },
+ { "dng_with_preview.dng", 1, {}, {}, 0 },
#endif
- { "mandrill.wbmp", 1, {}, {} },
- { "randPixels.bmp", 1, {}, {} },
- { "yellow_rose.webp", 1, {}, {} },
+ { "mandrill.wbmp", 1, {}, {}, 0 },
+ { "randPixels.bmp", 1, {}, {}, 0 },
+ { "yellow_rose.webp", 1, {}, {}, 0 },
};
for (auto rec : gRecs) {
@@ -57,6 +59,12 @@
continue;
}
+ const int repetitionCount = codec->getRepetitionCount();
+ if (repetitionCount != rec.fRepetitionCount) {
+ ERRORF(r, "%s repetition count does not match! expected: %i\tactual: %i",
+ rec.fName, rec.fRepetitionCount, repetitionCount);
+ }
+
const size_t expected = rec.fFrameCount;
const auto frameInfos = codec->getFrameInfo();
// getFrameInfo returns empty set for non-animated.
diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp
index bbf5f9a..0ffc7ca 100644
--- a/third_party/gif/SkGifImageReader.cpp
+++ b/third_party/gif/SkGifImageReader.cpp
@@ -415,6 +415,11 @@
return true;
}
+ if (SkGIFLoopCountQuery == query && m_loopCount != cLoopCountNotSeen) {
+ // Loop count has already been parsed.
+ return true;
+ }
+
// SkGIFSizeQuery and SkGIFFrameCountQuery are negative, so this is only meaningful when >= 0.
const int lastFrameToParse = (int) query;
if (lastFrameToParse >= 0 && (int) m_frames.size() > lastFrameToParse
@@ -664,9 +669,14 @@
// Zero loop count is infinite animation loop request.
if (!m_loopCount)
- m_loopCount = SkCodecAnimation::kAnimationLoopInfinite;
+ m_loopCount = SkCodec::kRepetitionCountInfinite;
GETN(1, SkGIFNetscapeExtensionBlock);
+
+ if (SkGIFLoopCountQuery == query) {
+ m_streamBuffer.flush();
+ return true;
+ }
} else if (netscapeExtension == 2) {
// Wait for specified # of bytes to enter buffer.
diff --git a/third_party/gif/SkGifImageReader.h b/third_party/gif/SkGifImageReader.h
index 5936350..2b1b2d9 100644
--- a/third_party/gif/SkGifImageReader.h
+++ b/third_party/gif/SkGifImageReader.h
@@ -298,8 +298,6 @@
{
}
- static constexpr int cLoopCountNotSeen = -2;
-
~SkGifImageReader()
{
}
@@ -317,6 +315,8 @@
SkGIFSizeQuery = -1,
// Parse to the end, so we know about all frames.
SkGIFFrameCountQuery = -2,
+ // Parse until we see the loop count.
+ SkGIFLoopCountQuery = -3,
};
// Parse incoming GIF data stream into internal data structures.
@@ -340,7 +340,12 @@
// FIXME: This extra complexity is not necessary and we should just report m_frames.size().
return m_frames.back()->isHeaderDefined() ? m_frames.size() : m_frames.size() - 1;
}
- int loopCount() const { return m_loopCount; }
+ int loopCount() const {
+ if (cLoopCountNotSeen == m_loopCount) {
+ return 0;
+ }
+ return m_loopCount;
+ }
const SkGIFColorMap& globalColorMap() const
{
@@ -389,6 +394,8 @@
unsigned m_screenWidth; // Logical screen width & height.
unsigned m_screenHeight;
SkGIFColorMap m_globalColorMap;
+
+ static constexpr int cLoopCountNotSeen = -2;
int m_loopCount; // Netscape specific extension block to control the number of animation loops a GIF renders.
std::vector<std::unique_ptr<SkGIFFrameContext>> m_frames;
