Index: third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp |
diff --git a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp |
index 2693402b40bbb8d0c6806fc0de57fdabb2cd0bfe..cee2d036217e661a236662f06e7efec3b3427bab 100644 |
--- a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp |
+++ b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp |
@@ -25,7 +25,7 @@ |
#include "platform/image-decoders/gif/GIFImageDecoder.h" |
-#include "platform/image-decoders/gif/GIFImageReader.h" |
+#include "base/logging.h" |
#include "wtf/NotFound.h" |
#include "wtf/PtrUtil.h" |
#include <limits> |
@@ -40,271 +40,168 @@ GIFImageDecoder::GIFImageDecoder(AlphaOption alphaOption, |
colorOptions, |
std::move(targetColorSpace), |
maxDecodedBytes), |
- m_repetitionCount(cAnimationLoopOnce) {} |
+ m_currentBufferSawAlpha(false), |
+ m_codec(), |
+ m_segmentStream(new SegmentStream{}) {} |
scroggo_chromium
2016/12/13 16:57:36
Why not make m_segmentStream a full member?
Oh wa
cblume
2016/12/14 09:16:56
Your idea about a raw pointer makes sense to me.
scroggo_chromium
2016/12/14 17:49:23
NewFromStream deletes the object - not the memory.
cblume
2016/12/16 02:57:10
Done.
|
GIFImageDecoder::~GIFImageDecoder() {} |
void GIFImageDecoder::onSetData(SegmentReader* data) { |
- if (m_reader) |
- m_reader->setData(data); |
+ // Add the segment to our stream |
+ m_segmentStream->setReader(data, isAllDataReceived()); |
+ |
+ // If we don't have a SkCodec yet, create one from the stream |
+ if (!m_codec) { |
+ m_codec.reset(SkCodec::NewFromStream(m_segmentStream.get())); |
+ } |
} |
int GIFImageDecoder::repetitionCount() const { |
+ CHECK(m_codec); |
+ |
// This value can arrive at any point in the image data stream. Most GIFs |
// in the wild declare it near the beginning of the file, so it usually is |
// set by the time we've decoded the size, but (depending on the GIF and the |
- // packets sent back by the webserver) not always. If the reader hasn't |
- // seen a loop count yet, it will return cLoopCountNotSeen, in which case we |
- // should default to looping once (the initial value for |
- // |m_repetitionCount|). |
- // |
- // There are some additional wrinkles here. First, ImageSource::clear() |
- // may destroy the reader, making the result from the reader _less_ |
- // authoritative on future calls if the recreated reader hasn't seen the |
- // loop count. We don't need to special-case this because in this case the |
- // new reader will once again return cLoopCountNotSeen, and we won't |
- // overwrite the cached correct value. |
- // |
- // Second, a GIF might never set a loop count at all, in which case we |
- // should continue to treat it as a "loop once" animation. We don't need |
- // special code here either, because in this case we'll never change |
- // |m_repetitionCount| from its default value. |
+ // packets sent back by the webserver) not always. |
// |
- // Third, we use the same GIFImageReader for counting frames and we might |
- // see the loop count and then encounter a decoding error which happens |
- // later in the stream. It is also possible that no frames are in the |
- // stream. In these cases we should just loop once. |
- if (isAllDataReceived() && parseCompleted() && m_reader->imagesCount() == 1) |
- m_repetitionCount = cAnimationNone; |
- else if (failed() || (m_reader && (!m_reader->imagesCount()))) |
- m_repetitionCount = cAnimationLoopOnce; |
- else if (m_reader && m_reader->loopCount() != cLoopCountNotSeen) |
- m_repetitionCount = m_reader->loopCount(); |
- return m_repetitionCount; |
+ // SkCodec will parse forward in the file if the repetition count has not been |
+ // seen yet. |
+ |
+ int repetitionCount = m_codec->getRepetitionCount(); |
+ switch (repetitionCount) { |
+ case 0: |
+ return cAnimationNone; |
+ case SkCodec::kRepetitionCountInfinite: |
+ return cAnimationLoopInfinite; |
+ default: |
+ return repetitionCount; |
+ } |
} |
bool GIFImageDecoder::frameIsCompleteAtIndex(size_t index) const { |
- return m_reader && (index < m_reader->imagesCount()) && |
- m_reader->frameContext(index)->isComplete(); |
-} |
- |
-float GIFImageDecoder::frameDurationAtIndex(size_t index) const { |
- return (m_reader && (index < m_reader->imagesCount()) && |
- m_reader->frameContext(index)->isHeaderDefined()) |
- ? m_reader->frameContext(index)->delayTime() |
- : 0; |
-} |
+ CHECK(m_codec); |
-bool GIFImageDecoder::setFailed() { |
- m_reader.reset(); |
- return ImageDecoder::setFailed(); |
-} |
+ std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo(); |
-bool GIFImageDecoder::haveDecodedRow(size_t frameIndex, |
- GIFRow::const_iterator rowBegin, |
- size_t width, |
- size_t rowNumber, |
- unsigned repeatCount, |
- bool writeTransparentPixels) { |
- const GIFFrameContext* frameContext = m_reader->frameContext(frameIndex); |
- // The pixel data and coordinates supplied to us are relative to the frame's |
- // origin within the entire image size, i.e. |
- // (frameContext->xOffset, frameContext->yOffset). There is no guarantee |
- // that width == (size().width() - frameContext->xOffset), so |
- // we must ensure we don't run off the end of either the source data or the |
- // row's X-coordinates. |
- const int xBegin = frameContext->xOffset(); |
- const int yBegin = frameContext->yOffset() + rowNumber; |
- const int xEnd = std::min(static_cast<int>(frameContext->xOffset() + width), |
- size().width()); |
- const int yEnd = std::min( |
- static_cast<int>(frameContext->yOffset() + rowNumber + repeatCount), |
- size().height()); |
- if (!width || (xBegin < 0) || (yBegin < 0) || (xEnd <= xBegin) || |
- (yEnd <= yBegin)) |
- return true; |
- |
- const GIFColorMap::Table& colorTable = |
- frameContext->localColorMap().isDefined() |
- ? frameContext->localColorMap().getTable() |
- : m_reader->globalColorMap().getTable(); |
- |
- if (colorTable.isEmpty()) |
- return true; |
- |
- GIFColorMap::Table::const_iterator colorTableIter = colorTable.begin(); |
- |
- // Initialize the frame if necessary. |
- ImageFrame& buffer = m_frameBufferCache[frameIndex]; |
- if (!initFrameBuffer(frameIndex)) |
- return false; |
- |
- const size_t transparentPixel = frameContext->transparentPixel(); |
- GIFRow::const_iterator rowEnd = rowBegin + (xEnd - xBegin); |
- ImageFrame::PixelData* currentAddress = buffer.getAddr(xBegin, yBegin); |
- |
- // We may or may not need to write transparent pixels to the buffer. |
- // If we're compositing against a previous image, it's wrong, and if |
- // we're writing atop a cleared, fully transparent buffer, it's |
- // unnecessary; but if we're decoding an interlaced gif and |
- // displaying it "Haeberli"-style, we must write these for passes |
- // beyond the first, or the initial passes will "show through" the |
- // later ones. |
- // |
- // The loops below are almost identical. One writes a transparent pixel |
- // and one doesn't based on the value of |writeTransparentPixels|. |
- // The condition check is taken out of the loop to enhance performance. |
- // This optimization reduces decoding time by about 15% for a 3MB image. |
- if (writeTransparentPixels) { |
- for (; rowBegin != rowEnd; ++rowBegin, ++currentAddress) { |
- const size_t sourceValue = *rowBegin; |
- if ((sourceValue != transparentPixel) && |
- (sourceValue < colorTable.size())) { |
- *currentAddress = colorTableIter[sourceValue]; |
- } else { |
- *currentAddress = 0; |
- m_currentBufferSawAlpha = true; |
- } |
- } |
- } else { |
- for (; rowBegin != rowEnd; ++rowBegin, ++currentAddress) { |
- const size_t sourceValue = *rowBegin; |
- if ((sourceValue != transparentPixel) && |
- (sourceValue < colorTable.size())) |
- *currentAddress = colorTableIter[sourceValue]; |
- else |
- m_currentBufferSawAlpha = true; |
- } |
+ // If https://skia-review.googlesource.com/c/5635/ lands |
+ if (index == 0) { |
+ return (frameInfoes.size() >= 1 || m_segmentStream->isAtEnd); |
scroggo_chromium
2016/12/13 16:57:36
frameInfos*
m_segmentStream->isAtEnd()* - but doe
cblume
2016/12/14 09:16:56
Done.
Yeah, we'll be wrong until the end of the s
scroggo_chromium
2016/12/14 17:49:23
My hope was that we didn't actually need to know w
cblume
2016/12/16 02:57:10
Done.
|
+ // Either We have found a second frame, which means frame 0 is complete, |
+ // or we have all the data, the image could be a single-frame and complete. |
} |
- // Tell the frame to copy the row data if need be. |
- if (repeatCount > 1) |
- buffer.copyRowNTimes(xBegin, xEnd, yBegin, yEnd); |
+ return frameInfos.size() > index; |
- buffer.setPixelsChanged(true); |
- return true; |
+ // If https://skia-review.googlesource.com/c/5703/ lands: |
+ // if (frameInfos.size() < index) { |
+ // // index out of bounds as far as we have parsed |
+ // return false; |
+ //} |
+ // |
+ // return frameInfos[index].fFullyReceived; |
} |
-bool GIFImageDecoder::parseCompleted() const { |
- return m_reader && m_reader->parseCompleted(); |
-} |
+float GIFImageDecoder::frameDurationAtIndex(size_t index) const { |
+ CHECK(m_codec); |
-bool GIFImageDecoder::frameComplete(size_t frameIndex) { |
- // Initialize the frame if necessary. Some GIFs insert do-nothing frames, |
- // in which case we never reach haveDecodedRow() before getting here. |
- ImageFrame& buffer = m_frameBufferCache[frameIndex]; |
- if (!initFrameBuffer(frameIndex)) |
- return false; // initFrameBuffer() has already called setFailed(). |
- |
- buffer.setStatus(ImageFrame::FrameComplete); |
- |
- if (!m_currentBufferSawAlpha) { |
- // The whole frame was non-transparent, so it's possible that the entire |
- // resulting buffer was non-transparent, and we can setHasAlpha(false). |
- if (buffer.originalFrameRect().contains(IntRect(IntPoint(), size()))) { |
- buffer.setHasAlpha(false); |
- buffer.setRequiredPreviousFrameIndex(kNotFound); |
- } else if (buffer.requiredPreviousFrameIndex() != kNotFound) { |
- // Tricky case. This frame does not have alpha only if everywhere |
- // outside its rect doesn't have alpha. To know whether this is |
- // true, we check the start state of the frame -- if it doesn't have |
- // alpha, we're safe. |
- const ImageFrame* prevBuffer = |
- &m_frameBufferCache[buffer.requiredPreviousFrameIndex()]; |
- ASSERT(prevBuffer->getDisposalMethod() != |
- ImageFrame::DisposeOverwritePrevious); |
- |
- // Now, if we're at a DisposeNotSpecified or DisposeKeep frame, then |
- // we can say we have no alpha if that frame had no alpha. But |
- // since in initFrameBuffer() we already copied that frame's alpha |
- // state into the current frame's, we need do nothing at all here. |
- // |
- // The only remaining case is a DisposeOverwriteBgcolor frame. If |
- // it had no alpha, and its rect is contained in the current frame's |
- // rect, we know the current frame has no alpha. |
- if ((prevBuffer->getDisposalMethod() == |
- ImageFrame::DisposeOverwriteBgcolor) && |
- !prevBuffer->hasAlpha() && |
- buffer.originalFrameRect().contains(prevBuffer->originalFrameRect())) |
- buffer.setHasAlpha(false); |
- } |
+ std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo(); |
+ |
+ if (frameInfos.size() < index) { |
+ // index out of bounds as far as we have parsed |
+ return 0; |
} |
- return true; |
+ return frameInfos[index].fDuration; |
} |
-void GIFImageDecoder::clearFrameBuffer(size_t frameIndex) { |
- if (m_reader && |
- m_frameBufferCache[frameIndex].getStatus() == ImageFrame::FramePartial) { |
- // Reset the state of the partial frame in the reader so that the frame |
- // can be decoded again when requested. |
- m_reader->clearDecodeState(frameIndex); |
- } |
- ImageDecoder::clearFrameBuffer(frameIndex); |
+void GIFImageDecoder::decodeSize() { |
+ CHECK(m_codec); |
+ |
+ SkImageInfo imageInfo = m_codec->getInfo(); |
+ setSize(imageInfo->width(), imageInfo->height); |
} |
size_t GIFImageDecoder::decodeFrameCount() { |
- parse(GIFFrameCountQuery); |
- // If decoding fails, |m_reader| will have been destroyed. Instead of |
- // returning 0 in this case, return the existing number of frames. This way |
- // if we get halfway through the image before decoding fails, we won't |
- // suddenly start reporting that the image has zero frames. |
- return failed() ? m_frameBufferCache.size() : m_reader->imagesCount(); |
+ std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo(); |
+ return frameInfos.size(); |
} |
void GIFImageDecoder::initializeNewFrame(size_t index) { |
- ImageFrame* buffer = &m_frameBufferCache[index]; |
- const GIFFrameContext* frameContext = m_reader->frameContext(index); |
- buffer->setOriginalFrameRect( |
- intersection(frameContext->frameRect(), IntRect(IntPoint(), size()))); |
- buffer->setDuration(frameContext->delayTime()); |
- buffer->setDisposalMethod(frameContext->getDisposalMethod()); |
- buffer->setRequiredPreviousFrameIndex( |
- findRequiredPreviousFrame(index, false)); |
+ CHECK(m_codec); |
+ |
+ if (m_frameBufferCache.size() < index) { |
scroggo_chromium
2016/12/13 16:57:36
I think you want <=, here and elsewhere. (And the
cblume
2016/12/14 09:16:56
Done.
I think you are right that it is not needed.
|
+ // index out of bounds as far as we have parsed |
+ return; |
+ } |
+ |
+ ImageFrame& frame = m_frameBufferCache[index]; |
+ std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo(); |
scroggo_chromium
2016/12/13 16:57:36
Since we call this so often, I think we should con
cblume
2016/12/14 09:16:56
I like the idea of SkCodec having a method that re
|
+ |
+ frame.setOriginalFrameRect(size()); |
+ frame.setDuration(frameInfos[index].fDuration); |
+ frame.setDisposalMethod(???); |
+ // TODO: SkCodec doesn't expose the disposal method. |
scroggo_chromium
2016/12/13 16:57:36
These might just be awkward during the transition.
cblume
2016/12/14 09:16:56
I agree. I'm hoping this stuff isn't handled by th
|
+ size_t requiredPreviousFrame = frameInfos[index].fRequiredFrame; |
+ if (requiredPreviousFrame == SkCodec::kNone) { |
+ requiredPreviousFrame = WTF::kNotFound; |
+ } |
+ frame.setRequiredPreviousFrameIndex(requiredPreviousFrame); |
} |
void GIFImageDecoder::decode(size_t index) { |
- parse(GIFFrameCountQuery); |
+ CHECK(m_codec); |
- if (failed()) |
+ if (m_frameBufferCache.size() < index) { |
+ // index out of bounds as far as we have parsed |
return; |
+ } |
updateAggressivePurging(index); |
- Vector<size_t> framesToDecode = findFramesToDecode(index); |
- for (auto i = framesToDecode.rbegin(); i != framesToDecode.rend(); ++i) { |
- if (!m_reader->decode(*i)) { |
- setFailed(); |
- return; |
- } |
+ SkImageInfo imageInfo = m_codec->getInfo(); |
+ if (imageInfo.colorType() == kIndex8_SkColorType) { |
+ // imageInfo's color type may be influenced by the untrusted image file. |
scroggo_chromium
2016/12/13 16:57:36
We report index8 if the GIF can support it (for th
cblume
2016/12/14 09:16:56
Done.
|
+ // Index8 has special behavior when getting the pixels, which we do not yet |
+ // handle |
+ return; |
+ } |
- // If this returns false, we need more data to continue decoding. |
- if (!postDecodeProcessing(*i)) |
- break; |
+ SkCodec::Options getPixelsOptions; |
+ getPixelsOptions.fFrameIndex = index; |
+ getPixelsOptions.fHasPriorFrame = false; |
+ |
+ // TODO: On the second loop, can we avoid re-copying and decoding? |
+ // Said another way, is there a way to know if we already populated this |
scroggo_chromium
2016/12/13 16:57:36
Check frame.getStatus()?
cblume
2016/12/14 09:16:56
Hrmmm I seem to be able to check for empty, incomp
scroggo_chromium
2016/12/14 17:49:23
If i was purged, m_frameBufferCache[i] should be F
cblume
2016/12/16 02:57:10
I guess we have two types of "complete": completel
|
+ // frame? |
+ ImageFrame& frame = m_frameBufferCache[index]; |
+ size_t requiredPreviousFrameIndex = frame.requiredPreviousFrameIndex(); |
+ if (requiredPreviousFrameIndex == WTF::kNotFound) { |
scroggo_chromium
2016/12/13 16:57:36
I think you mean != WTF::kNotFound?
cblume
2016/12/14 09:16:56
Done.
|
+ getPixelsOptions.fHasPriorFrame = true; |
+ ImageFrame& requiredPreviousFrame = |
+ m_frameBufferCache[requiredPreviousFrameIndex]; |
+ frame.copyBitmapData(requiredPreviousFrame); |
scroggo_chromium
2016/12/13 16:57:36
Doesn't this happen in initFrameBuffer?
https://c
cblume
2016/12/14 09:16:56
Done.
scroggo_chromium
2016/12/14 17:49:23
D'oh, and then I told you not to call that method.
cblume
2016/12/16 02:57:10
I took another look again at initFrameBuffer.
It c
scroggo_chromium
2016/12/16 15:03:47
I'd sync up with msarett@.
|
} |
- // It is also a fatal error if all data is received and we have decoded all |
- // frames available but the file is truncated. |
- if (index >= m_frameBufferCache.size() - 1 && isAllDataReceived() && |
- m_reader && !m_reader->parseCompleted()) |
+ SkCodec::Result getPixelsResult = |
scroggo_chromium
2016/12/13 16:57:36
You want to switch from getPixels() to startIncrem
cblume
2016/12/14 09:16:56
That makes sense.
I'll have to research the behav
scroggo_chromium
2016/12/14 17:49:23
As I understand it, Blink waits until a frame afte
cblume
2016/12/16 02:57:10
Done.
|
+ m_codec->getPixels(imageInfo, frame.getPixels(), frame.rowBytes(), |
+ &getPixelsOptions, nullptr, nullptr); |
+ if (getPixelResult != kSuccess) { |
scroggo_chromium
2016/12/13 16:57:36
kIncomplete is also okay, so long as it's the late
cblume
2016/12/14 09:16:56
Done.
|
setFailed(); |
-} |
- |
-void GIFImageDecoder::parse(GIFParseQuery query) { |
- if (failed()) |
return; |
+ } |
- if (!m_reader) { |
- m_reader = makeUnique<GIFImageReader>(this); |
- m_reader->setData(m_data); |
+ if (!postDecodeProcessing(index)) { |
+ return; |
} |
- if (!m_reader->parse(query)) |
- setFailed(); |
-} |
+ // If we do not end up landing https://skia-review.googlesource.com/c/5635/ |
+ // ... |
-void GIFImageDecoder::onInitFrameBuffer(size_t frameIndex) { |
- m_currentBufferSawAlpha = false; |
+ // It is also a fatal error if all data is received and we have decoded all |
+ // frames available but the file is truncated. |
+ if (index >= m_frameBufferCache.size() - 1 && isAllDataReceived()) { |
+ setFailed(); |
+ } |
} |
bool GIFImageDecoder::canReusePreviousFrameBuffer(size_t frameIndex) const { |