From e132e7be5f9108692254c37db592ea7611abbc15 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Wed, 12 Apr 2017 10:49:52 -0400 Subject: 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 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 Reviewed-by: Mike Reed Reviewed-by: Chris Blume Commit-Queue: Leon Scroggins --- include/codec/SkCodec.h | 35 ++++++-- src/codec/SkCodec.cpp | 21 +++++ src/codec/SkGifCodec.cpp | 32 ++++--- src/codec/SkGifCodec.h | 3 +- tests/CodecAnimTest.cpp | 226 ++++++++++++++++++++++++++++------------------- 5 files changed, 208 insertions(+), 109 deletions(-) diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h index 6d4352214c..aadcb059fe 100644 --- 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(-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 getFrameInfo() { - return this->onGetFrameInfo(); - } + std::vector getFrameInfo(); static constexpr int kRepetitionCountInfinite = -1; @@ -800,9 +818,12 @@ protected: SkTransferFunctionBehavior premulBehavior); SkColorSpaceXform* colorXform() const { return fColorXform.get(); } - virtual std::vector onGetFrameInfo() { - // empty vector - this is not animated. - return std::vector{}; + 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 5bdb3e2b59..6a6fdc78c9 100644 --- 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::getFrameInfo() { + const size_t frameCount = this->getFrameCount(); + switch (frameCount) { + case 0: + return std::vector{}; + case 1: + if (!this->onGetFrameInfo(0, nullptr)) { + // Not animated. + return std::vector{}; + } + // fall through + default: { + std::vector 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 06c0803136..2e0ec3057d 100644 --- 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 SkGifCodec::onGetFrameInfo() { +size_t SkGifCodec::onGetFrameCount() { fReader->parse(SkGifImageReader::SkGIFFrameCountQuery); - const size_t size = fReader->imagesCount(); - std::vector 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 67654d3b55..11714eb39e 100644 --- a/src/codec/SkGifCodec.h +++ b/src/codec/SkGifCodec.h @@ -50,7 +50,8 @@ protected: uint64_t onGetFillValue(const SkImageInfo&) const override; - std::vector 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 dea4adbfd2..02253ef64f 100644 --- 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 stream(GetResourceAsStream(rec.fName)); - if (!stream) { + sk_sp 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 codec(SkCodec::NewFromStream(stream.release())); + std::unique_ptr 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 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 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 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; + } + } } } } -- cgit v1.2.3