Move gif image decoder to SkCodec

third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp

 /*
  * Copyright (C) 2006 Apple Computer, 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.
  */
 
 #include "platform/image-decoders/gif/GIFImageDecoder.h"
 
-#include "platform/image-decoders/gif/GIFImageReader.h"
+#include <limits>
+#include "third_party/skia/include/core/SkImageInfo.h"
 #include "wtf/NotFound.h"
 #include "wtf/PtrUtil.h"
-#include <limits>
 
 namespace blink {
 
 GIFImageDecoder::GIFImageDecoder(AlphaOption alphaOption,
                                  const ColorBehavior& colorBehavior,
                                  size_t maxDecodedBytes)
     : ImageDecoder(alphaOption, colorBehavior, maxDecodedBytes),
-      m_repetitionCount(cAnimationLoopOnce) {}
-
-GIFImageDecoder::~GIFImageDecoder() {}
+      m_codec(),
+      m_segmentStream(nullptr) {}
+
+GIFImageDecoder::~GIFImageDecoder() {
+  if (!m_codec) {
+    // if we did not create m_codec and thus did not pass ownership to it
+    if (m_segmentStream) {
+      delete m_segmentStream;
+    }
+  }
+}
 
 void GIFImageDecoder::onSetData(SegmentReader* data) {
-  if (m_reader)
-    m_reader->setData(data);
+  if (!data) {
+    m_segmentStream = nullptr;

[scroggo_chromium, 2017/03/06 16:27:31]

This just means we've lost our pointer to it, so next time we'll create a new
one. But m_codec still holds on to the old one, so calling
m_segmentReader->setReader, as below, has no effect on m_codec.

Instead, I think you want to call

  m_segmentStream->setReader(null, false);

[cblume, 2017/03/07 01:18:11]

Done.
Interestingly, the crash in DeferredImageDecoderTestWoPlatform.mixImagesGif went
away when I set m_segmentStream = nullptr. But you are right, that is clearly
not the correct behavior.
I'll investigate the cause of this crash.

+    return;
+  }
+
+  if (!m_segmentStream) {
+    m_segmentStream = new SegmentStream();
+  }
+
+  m_segmentStream->setReader(data, isAllDataReceived());
+
+  // If we don't have a SkCodec yet, create one from the stream
+  if (!m_codec) {
+    SkCodec* codec = SkCodec::NewFromStream(m_segmentStream);
+    if (codec) {
+      m_codec.reset(codec);
+    } else {
+      // m_segmentStream's ownership is passed. It is deleted if SkCodec
+      // creation fails. In this case, release our reference so we can create a
+      // new SegmentStream later.
+      m_segmentStream = nullptr;
+      return;
+    }
+
+    // SkCodec::NewFromStream will read enough of the image to get the image
+    // size.
+    SkImageInfo imageInfo = m_codec->getInfo();
+    setSize(imageInfo.width(), imageInfo.height());
+  }
 }
 
 int GIFImageDecoder::repetitionCount() const {
+  if (!m_codec) {
+    return cAnimationLoopOnce;
+  }
+
   // 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|).
+  // packets sent back by the webserver) not always.
   //
-  // 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.
-  //
-  // 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.
+
+  std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo();
+  if (isAllDataReceived() && frameInfos.size() == 1)
+    return cAnimationNone;
+  if (failed())
+    return cAnimationLoopOnce;
+
+  int repetitionCount = m_codec->getRepetitionCount();
+  switch (repetitionCount) {
+    case 0:
+      return cAnimationLoopOnce;
+    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();
+  if (!m_codec) {
+    return false;
+  }
+
+  std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo();
+  if (frameInfos.size() <= index) {
+    return false;
+  }
+
+  return frameInfos[index].fFullyReceived;
 }
 
 float GIFImageDecoder::frameDurationAtIndex(size_t index) const {

[scroggo_chromium, 2017/03/06 16:27:31]

I wonder if we should instead just return m_frameBufferCache[index].duration()
(after ensuring index is within the range). That would make this the same as
WEBP (and the WIP APNG https://codereview.chromium.org/2618633004/), meaning
this no longer needs to be virtual (since it would still correctly return 0 for
a static image).

But then you would need to make sure you call ImageFrame::setDuration without
decoding. This was previously done in initializeNewFrame, which you appear to
have removed.

[cblume, 2017/03/07 01:18:11]

Done.

-  return (m_reader && (index < m_reader->imagesCount()) &&
-          m_reader->frameContext(index)->isHeaderDefined())
-             ? m_reader->frameContext(index)->delayTime()
-             : 0;
-}
-
-bool GIFImageDecoder::setFailed() {
-  m_reader.reset();
-  return ImageDecoder::setFailed();
-}
-
-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;
+  if (!m_codec) {
+    return 0;
+  }
+
+  std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo();
+  if (frameInfos.size() <= index) {
+    return 0;
+  }
+
+  return frameInfos[index].fDuration;
+}
+
+size_t GIFImageDecoder::decodeFrameCount() {
+  if (!m_codec) {
+    return 0;
+  }
+
+  std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo();
+  return frameInfos.size();
+}
+
+void GIFImageDecoder::decode(size_t index) {
+  if (failed())
+    return;
+
+  if (!m_codec)
+    return;
+
+  if (m_frameBufferCache.size() <= index) {
+    // It is a fatal error if all data is received and we have decoded all
+    // frames available but the file is truncated.
+    if (isAllDataReceived()) {
+      setFailed();
+    }
+
+    return;
+  }
+
+  updateAggressivePurging(index);
+
+  SkImageInfo imageInfo = m_codec->getInfo().makeColorType(kN32_SkColorType);
+
+  ImageFrame& frame = m_frameBufferCache[index];
+
+  std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo();
+  size_t requiredPreviousFrameIndex = frameInfos[index].fRequiredFrame;
+  if (requiredPreviousFrameIndex == SkCodec::kNone) {
+    requiredPreviousFrameIndex = WTF::kNotFound;
+  }
+  frame.setRequiredPreviousFrameIndex(requiredPreviousFrameIndex);
+
+  SkCodec::Options options;
+  options.fFrameIndex = index;
+  options.fHasPriorFrame = false;
+
+  if (frame.getStatus() == ImageFrame::FrameEmpty) {
+    IntSize frameSize = size();
+    frame.setOriginalFrameRect(IntRect(IntPoint(), frameSize));
+    frame.setDuration(frameInfos[index].fDuration);
+    // The disposal method is not required any more, but is left in place
+    // for the other image decoders that do not yet rely on SkCodec.
+    // For now, fill it with DisposeKeep.
+    frame.setDisposalMethod(ImageFrame::DisposeKeep);
+    if (requiredPreviousFrameIndex == WTF::kNotFound) {
+      frame.setSizeAndColorSpace(frameSize.width(), frameSize.height(),
+                                 colorSpaceForSkImages());
+    } else {
+      ImageFrame& requiredPreviousFrame =
+          m_frameBufferCache[requiredPreviousFrameIndex];
+
+      if (requiredPreviousFrame.getStatus() != ImageFrame::FrameComplete)
+        decode(requiredPreviousFrameIndex);
+
+      // We try to reuse |requiredPreviousFrame| as starting state to avoid
+      // copying. If canReusePreviousFrameBuffer returns false, we must copy
+      // the data since |requiredPreviousFrame| is necessary to decode this
+      // or later frames. In that case copy the data instead.
+      if ((!canReusePreviousFrameBuffer(index) ||
+           !frame.takeBitmapDataIfWritable(&requiredPreviousFrame)) &&
+          !frame.copyBitmapData(requiredPreviousFrame)) {
+        setFailed();
+        return;
       }
     }
-  } else {
-    for (; rowBegin != rowEnd; ++rowBegin, ++currentAddress) {
-      const size_t sourceValue = *rowBegin;
-      if ((sourceValue != transparentPixel) &&
-          (sourceValue < colorTable.size()))
-        *currentAddress = colorTableIter[sourceValue];
-      else
-        m_currentBufferSawAlpha = true;
-    }
-  }
-
-  // Tell the frame to copy the row data if need be.
-  if (repeatCount > 1)
-    buffer.copyRowNTimes(xBegin, xEnd, yBegin, yEnd);
-
-  buffer.setPixelsChanged(true);
-  return true;
-}
-
-bool GIFImageDecoder::parseCompleted() const {
-  return m_reader && m_reader->parseCompleted();
-}
-
-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.
-  if (!initFrameBuffer(frameIndex))
-    return false;  // initFrameBuffer() has already called setFailed().
-
-  m_frameBufferCache[frameIndex].setStatus(ImageFrame::FrameComplete);
-  if (!m_currentBufferSawAlpha)
-    correctAlphaWhenFrameBufferSawNoAlpha(frameIndex);
-
-  return true;
-}
-
-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);
-}
-
-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();
-}
-
-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));
-}
-
-void GIFImageDecoder::decode(size_t index) {
-  parse(GIFFrameCountQuery);
-
-  if (failed())
-    return;
-
-  updateAggressivePurging(index);
-
-  Vector<size_t> framesToDecode = findFramesToDecode(index);
-  for (auto i = framesToDecode.rbegin(); i != framesToDecode.rend(); ++i) {
-    if (!m_reader->decode(*i)) {
+
+    if (requiredPreviousFrameIndex != WTF::kNotFound) {
+      options.fHasPriorFrame = true;

[scroggo_chromium, 2017/03/06 16:27:31]

I was thinking you could move this into the block above. e.g.

if (frame.getStatus() == ImageFrame::FrameEmpty) {
  ...
  if (requiredPreviousFrameIndex == WTF::kNotFound) {
    ...
  } else {
    // Actually decode the prior frame and copy/steal it into
    // the frame buffer
    options.fHasPriorFrame = true;
  }
}

[cblume, 2017/03/07 01:18:10]

Done.

+    }
+
+    SkCodec::Result startIncrementalDecodeResult =
+        m_codec->startIncrementalDecode(imageInfo, frame.bitmap().getPixels(),
+                                        frame.bitmap().rowBytes(), &options,
+                                        nullptr, nullptr);
+    switch (startIncrementalDecodeResult) {
+      case SkCodec::kSuccess:
+        break;
+      case SkCodec::kIncompleteInput:
+        return;
+      default:
+        setFailed();
+        return;
+    }
+    frame.setStatus(ImageFrame::FramePartial);
+  }
+
+  int rowsDecoded = 0;
+  SkCodec::Result incrementalDecodeResult =
+      m_codec->incrementalDecode(&rowsDecoded);
+  switch (incrementalDecodeResult) {
+    case SkCodec::kSuccess:
+      frame.setPixelsChanged(true);
+      frame.setStatus(ImageFrame::FrameComplete);
+      break;
+    case SkCodec::kIncompleteInput:
+      // Only frame 0 may return kIncompleteInput without it being an error

[scroggo_chromium, 2017/03/06 16:27:31]

This comment does not match the code below. You do not actually check the index,
but you do check isAllDataReceived (which I think is also relevant).

[cblume, 2017/03/07 01:18:10]

The comment was not very clear.
I don't want to actually check if the frame is 0. I'll update the comments and
you can tell me if it is now clear. I think it is much better now.

+      if (isAllDataReceived()) {
+        setFailed();
+        return;
+      }
+
+      frame.setPixelsChanged(true);
+      frame.setStatus(ImageFrame::FramePartial);
+      break;
+    default:
       setFailed();
       return;
-    }
-
-    // If this returns false, we need more data to continue decoding.
-    if (!postDecodeProcessing(*i))
-      break;
-  }
-
-  // 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())
-    setFailed();
-}
-
-void GIFImageDecoder::parse(GIFParseQuery query) {
-  if (failed())
-    return;
-
-  if (!m_reader) {
-    m_reader = WTF::makeUnique<GIFImageReader>(this);
-    m_reader->setData(m_data);
-  }
-
-  if (!m_reader->parse(query))
-    setFailed();
-}
-
-void GIFImageDecoder::onInitFrameBuffer(size_t frameIndex) {
-  m_currentBufferSawAlpha = false;
-}
-
-bool GIFImageDecoder::canReusePreviousFrameBuffer(size_t frameIndex) const {
-  DCHECK(frameIndex < m_frameBufferCache.size());
-  return m_frameBufferCache[frameIndex].getDisposalMethod() !=
-         ImageFrame::DisposeOverwritePrevious;
+  }
+
+  if (frame.getStatus() == ImageFrame::FrameComplete) {
+    if (!postDecodeProcessing(index))
+      return;

[scroggo_chromium, 2017/03/06 16:27:31]

There are no statements after this return, so it does not make a difference.

And when I look at the implementation of postDecodeProcessing, it only returns
false based on the Status, so I think this should never return false.

Also, this should probably be called up above, when we set the status to
FrameComplete?

[cblume, 2017/03/07 01:18:11]

Done.

+  }
+}
+
+bool GIFImageDecoder::canReusePreviousFrameBuffer(size_t index) const {
+  DCHECK(index < m_frameBufferCache.size());
+
+  // If the current frame and the next frame depend on the same frame, we cannot
+  // reuse the old frame. We must preserve it for the next frame.
+  //
+  // However, if the current and next frame depend on different frames then we
+  // know the current frame is the last one to use the frame it depends on. That
+  // means the current frame can reuse the previous frame buffer.
+  //
+  // If we do not have information about the next frame yet, we cannot assume it
+  // is safe to reuse the previous frame buffer.
+
+  if (index + 1 >= m_frameBufferCache.size())
+    return false;
+
+  const ImageFrame& frame = m_frameBufferCache[index];
+  size_t requiredFrameIndex = frame.requiredPreviousFrameIndex();
+
+  const ImageFrame& nextFrame = m_frameBufferCache[index + 1];
+  size_t nextRequiredFrameIndex = nextFrame.requiredPreviousFrameIndex();
+
+  return requiredFrameIndex != nextRequiredFrameIndex;
 }
 
 }  // namespace blink