From e4ba1059ddf4d9a1e41b01c378f1a5cc15669343 Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Mon, 30 Jan 2017 13:55:14 -0500 Subject: GIF: Only report a frame after knowing dependency Previously, getFrameInfo might report a frame that was truncated prior to setting its requiredFrame. As a result, fRequiredFrame may be different depending on how much data has already been received. If there is a local color table, do not report the frame until the color table has been received, since that is used to determine fRequiredFrame. If there is no local color table, set fRequiredFrame and report the frame after reading the header. Add a test. Replace make_from_resource with GetResourceAsData Change-Id: I1b697f766c1d0e1e12ab2ae1d27167af5193395d Reviewed-on: https://skia-review.googlesource.com/7756 Reviewed-by: Matt Sarett Commit-Queue: Leon Scroggins --- third_party/gif/SkGifImageReader.cpp | 6 ++++-- third_party/gif/SkGifImageReader.h | 31 +++++++++++++++++++++++-------- 2 files changed, 27 insertions(+), 10 deletions(-) (limited to 'third_party/gif') diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp index 2603807cb3..f617ccdc2a 100644 --- a/third_party/gif/SkGifImageReader.cpp +++ b/third_party/gif/SkGifImageReader.cpp @@ -457,7 +457,6 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query) case SkGIFLZWStart: { SkASSERT(!m_frames.empty()); auto* currentFrame = m_frames.back().get(); - setRequiredFrame(currentFrame); currentFrame->setDataSize(this->getOneByte()); GETN(1, SkGIFSubBlock); @@ -800,14 +799,17 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query) break; } + setRequiredFrame(currentFrame); GETN(1, SkGIFLZWStart); break; } case SkGIFImageColormap: { SkASSERT(!m_frames.empty()); - auto& cmap = m_frames.back()->localColorMap(); + auto* currentFrame = m_frames.back().get(); + auto& cmap = currentFrame->localColorMap(); cmap.setTablePosition(m_streamBuffer.markPosition()); + setRequiredFrame(currentFrame); GETN(1, SkGIFLZWStart); break; } diff --git a/third_party/gif/SkGifImageReader.h b/third_party/gif/SkGifImageReader.h index c51f4a8983..24c917af2a 100644 --- a/third_party/gif/SkGifImageReader.h +++ b/third_party/gif/SkGifImageReader.h @@ -202,7 +202,7 @@ public: , m_height(0) , m_transparentPixel(SkGIFColorMap::kNotFound) , m_disposalMethod(SkCodecAnimation::Keep_DisposalMethod) - , m_requiredFrame(SkCodec::kNone) + , m_requiredFrame(kUninitialized) , m_dataSize(0) , m_progressiveDisplay(false) , m_interlaced(false) @@ -242,8 +242,13 @@ public: void setTransparentPixel(size_t pixel) { m_transparentPixel = pixel; } SkCodecAnimation::DisposalMethod getDisposalMethod() const { return m_disposalMethod; } void setDisposalMethod(SkCodecAnimation::DisposalMethod disposalMethod) { m_disposalMethod = disposalMethod; } - size_t getRequiredFrame() const { return m_requiredFrame; } + + size_t getRequiredFrame() const { + SkASSERT(this->reachedStartOfData()); + return m_requiredFrame; + } void setRequiredFrame(size_t req) { m_requiredFrame = req; } + unsigned delayTime() const { return m_delayTime; } void setDelayTime(unsigned delay) { m_delayTime = delay; } bool isComplete() const { return m_isComplete; } @@ -266,7 +271,11 @@ public: const SkGIFColorMap& localColorMap() const { return m_localColorMap; } SkGIFColorMap& localColorMap() { return m_localColorMap; } + bool reachedStartOfData() const { return m_requiredFrame != kUninitialized; } + private: + static constexpr size_t kUninitialized = static_cast(-2); + int m_frameId; unsigned m_xOffset; unsigned m_yOffset; // With respect to "screen" origin. @@ -346,13 +355,19 @@ public: size_t imagesCount() const { - if (m_frames.empty()) - return 0; + // Report the first frame immediately, so the parser can stop when it + // sees the size on a SizeQuery. + const size_t frames = m_frames.size(); + if (frames <= 1) { + return frames; + } - // This avoids counting an empty frame when the file is truncated right after - // SkGIFControlExtension but before SkGIFImageHeader. - // 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; + // This avoids counting an empty frame when the file is truncated (or + // simply not yet complete) after receiving SkGIFControlExtension (and + // possibly SkGIFImageHeader) but before reading the color table. This + // ensures that we do not count a frame before we know its required + // frame. + return m_frames.back()->reachedStartOfData() ? frames : frames - 1; } int loopCount() const { if (cLoopCountNotSeen == m_loopCount) { -- cgit v1.2.3