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 "base/logging.h"
+#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) {}
+      m_codec(),
+      m_segmentStream(nullptr) {}
 
-GIFImageDecoder::~GIFImageDecoder() {}
+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);
+  // Add the segment to our stream

[scroggo_chromium, 2016/12/16 15:03:48]

I find this comment misleading. Our stream just wraps the SegmentReader; this
doesn't add individual segments to it.

[cblume, 2016/12/16 17:43:41]

You're right.
Aside from being unclear, it also isn't useful. I'll remove it.

+  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) {
+    m_codec.reset(SkCodec::NewFromStream(m_segmentStream));
+    if (!m_codec) {
+      // 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;
+    }
+  }
 }
 
 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|).
+  // 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.
+
+  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();
+  CHECK(m_codec);
+
+  std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo();
+  if (frameInfos.size() < index) {

[scroggo_chromium, 2016/12/16 15:03:48]

<=

[cblume, 2016/12/16 17:43:41]

Done.

+    return false;
+  }
+
+  return frameInfos[index].fFullyReceived;
 }
 
 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);
+
+  std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo();
+  if (frameInfos.size() < index) {
+    return 0;
+  }
+
+  return frameInfos[index].fDuration;
 }
 
-bool GIFImageDecoder::setFailed() {
-  m_reader.reset();
-  return ImageDecoder::setFailed();
+void GIFImageDecoder::decodeSize() {
+  CHECK(m_codec);
+
+  SkImageInfo imageInfo = m_codec->getInfo();
+  setSize(imageInfo.width(), imageInfo.height());
 }
 
-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;
+size_t GIFImageDecoder::decodeFrameCount() {
+  std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo();
+  return frameInfos.size();
+}
 
-  const GIFColorMap::Table& colorTable =
-      frameContext->localColorMap().isDefined()
-          ? frameContext->localColorMap().getTable()
-          : m_reader->globalColorMap().getTable();
+void GIFImageDecoder::initializeNewFrame(size_t index) {
+  CHECK(m_codec);
 
-  if (colorTable.isEmpty())
-    return true;
+  ImageFrame& frame = m_frameBufferCache[index];
+  std::vector<SkCodec::FrameInfo> frameInfos = m_codec->getFrameInfo();
 
-  GIFColorMap::Table::const_iterator colorTableIter = colorTable.begin();
+  frame.setOriginalFrameRect(IntRect(IntPoint(), size()));
+  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);
+  size_t requiredPreviousFrame = frameInfos[index].fRequiredFrame;
+  if (requiredPreviousFrame == SkCodec::kNone) {
+    requiredPreviousFrame = WTF::kNotFound;
+  }
+  frame.setRequiredPreviousFrameIndex(requiredPreviousFrame);
+}
 
-  // Initialize the frame if necessary.
-  ImageFrame& buffer = m_frameBufferCache[frameIndex];
-  if (!initFrameBuffer(frameIndex))
-    return false;
+void GIFImageDecoder::decode(size_t index) {
+  CHECK(m_codec);
 
-  const size_t transparentPixel = frameContext->transparentPixel();
-  GIFRow::const_iterator rowEnd = rowBegin + (xEnd - xBegin);
-  ImageFrame::PixelData* currentAddress = buffer.getAddr(xBegin, yBegin);
+  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();
+    }
 
-  // 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;
-      }
+    return;
+  }
+
+  updateAggressivePurging(index);
+
+  SkImageInfo imageInfo = m_codec->getInfo().makeColorType(kN32_SkColorType);
+
+  SkCodec::Options options;
+  options.fFrameIndex = index;
+  options.fHasPriorFrame = false;
+
+  ImageFrame& frame = m_frameBufferCache[index];
+  if (frame.getStatus() == ImageFrame::FrameEmpty) {
+    size_t requiredPreviousFrameIndex = frame.requiredPreviousFrameIndex();
+    if (requiredPreviousFrameIndex != WTF::kNotFound) {
+      options.fHasPriorFrame = true;

[scroggo_chromium, 2016/12/16 15:03:48]

This isn't necessarily true. copyBitmapData returns a boolean. You need to check
whether it (or takeBitmapDataIfWritable, which should be checked first)
succeeds. If so, you have the prior frame. If not, this frame will still be
empty.

You also might want to decode prior frames manually. If you do not provide the
prior frame, SkCodec will decode it first, but will not store it for later. If
you want to cache a frame that index depends on, you need to decode it yourself.

[cblume, 2016/12/16 17:43:41]

I was thinking of not using takeBitmapDataIfWritable.
My intention is to leave that frame preserved. I assume takeBitmapDataIfWritable
will assume ownership of the bitmap itself (and thus not preserve the older
frame).

My reason for wanting to preserve the frame is I want to eventually have the
frames live further up in Blink, which can manage the cache more wisely. So I'm
trying to remove any what-stays-in-cache logic.

Although, maybe that should happen at a later time and for now we can
takeBitmapDataIfWritable.


Because I am leaving prior frames in tact (and relying on the purging behavior
for now), I believe we are keeping a cache of any frames we depend on manually.


As far as checking that copyBitmapData succeeds, you are completely correct. I
think in that case I may want to setFailed(); and return.

[scroggo_chromium, 2016/12/16 18:45:45]

That is correct.
But the net result for now is that you're leaving more in the cache. I think you
should stick with the current behavior for now.
Agreed.

[cblume, 2016/12/18 17:47:57]

I copied what we were previously doing.
However, it requires canReusePreviousFrameBuffer() to know the disposal method
of the frame, which I do not know at this level.
I *could* wait until we have looped through the image and then make sure no
latter frames require this one.
But that means the first pass cannot reuse buffers and the second pass requires
building up an inverted dependency list.

[scroggo_chromium, 2016/12/19 14:28:54]

Ah. Here's what I was thinking when I decided to leave the disposal method off
of SkCodec::FrameInfo: The next frame's required frame tells you what you need
to know. If it does not depend on this frame, no future frames will. No need to
wait for a full loop, but it does mean we have to have received part of the next
frame. Since Chromium will not attempt to decode frame i (where i > 0) until i
is fully received, this leaves a gap between the end of i until the end of the
header of (i + 1) where we do not know whether any frames depend on i. Not sure
how common that is, but if it is a problem, that is an argument for making
SkCodec provide that information.

[cblume, 2016/12/19 21:12:40]

Okay, I think I follow you now.

As an example, suppose:
frame:       0 1 2
depends on: -1 0 0

When we get to frame 1 (depends on 0), we cannot take the bitmap because a
future frame might need it (in this case, frame 2). So here we need to copy the
bitmap.

Going one step further:
frame:       0 1 2  3
depends on: -1 0 0 -1

If we are about to decode frame 2, and we see frame 3 does not depend on frame 0
any more, we know we can take it. I believe that is what you meant by needing
info about frame i + 1.

I'll update the code to reflect this.

[scroggo_chromium, 2016/12/19 21:16:15]

Yes. Taken a step further, it is impossible for frame 4 to depend on frame 0,
since 3 is independent.

[cblume, 2016/12/19 21:21:48]

Right. Sounds good.

+      ImageFrame& requiredPreviousFrame =
+          m_frameBufferCache[requiredPreviousFrameIndex];
+      frame.copyBitmapData(requiredPreviousFrame);
     }
-  } else {
-    for (; rowBegin != rowEnd; ++rowBegin, ++currentAddress) {
-      const size_t sourceValue = *rowBegin;
-      if ((sourceValue != transparentPixel) &&
-          (sourceValue < colorTable.size()))
-        *currentAddress = colorTableIter[sourceValue];
-      else
-        m_currentBufferSawAlpha = true;
+
+    SkCodec::Result startIncrementalDecodeResult =
+        m_codec->startIncrementalDecode(imageInfo, frame.bitmap().getPixels(),
+                                        frame.bitmap().rowBytes(), &options,
+                                        nullptr, nullptr);
+    if (startIncrementalDecodeResult == SkCodec::kSuccess) {

[scroggo_chromium, 2016/12/16 15:03:48]

nit: This empty block looks funny to me. Why not make this a switch statement,
or convert the second else to

  } else if (result != success) {

?

[cblume, 2016/12/16 17:43:41]

Done.

+    } else if (startIncrementalDecodeResult == SkCodec::kIncompleteInput) {
+      // TODO if startIncrementalDecode fails with partial input, do we need to
+      // call startIncrementalDecode again?

[scroggo_chromium, 2016/12/16 15:03:48]

Yes. I only recently (https://skia-review.googlesource.com/5758) made a change
where it would ever return incomplete, and I can see how it is a little
different from returning incomplete from incrementalDecode().

The intent is to say "we could not start, because there is not enough data", so
yes, another call to start is necessary.

So the line below should probably leave the frame FrameEmpty. In the case of the
first frame, we'll need to make sure it is initialized. For other frames, this
should never happen, because we wait until they're fully received to decode...
But this code doesn't know that, so we may need to take it into account. It
might be awkward here, since the frame might not be truly empty (if its required
frame was copied into it), but we use empty to signal to attempt to start again.

[cblume, 2016/12/16 17:43:41]

I was thinking that same thing (if we restart because it is left FrameEmpty then
we re-copy the dependent frame data).

I think what I might do instead is call this earlier, before copying the prior
frame contents. If the start succeeds, prepare the buffer and begin.

For that to work, I need to know that startIncrementalDecode won't actually do
any decoding -- it will only get things setup, right?

[scroggo_chromium, 2016/12/16 18:45:45]

It won't do any decoding, but it stores the options, including whether
fHasPriorFrame. But I guess since we'll setFailed if the copy from the prior
frame fails, we can just assume that fHasPriorFrame is true if the frame we have
is FrameComplete.

[cblume, 2016/12/18 17:47:57]

I didn't understand your last sentence.

I think I may have fixed this up. When startIncrementalDecode returns incomplete
it leaves the frame as FrameEmpty. Then we need Blink to attempt to decode the
same frame once it gets more data.

[scroggo_chromium, 2016/12/19 14:28:54]

I think the code should look something like:

  if (/* requires prior frame */) {
    options.fHasPriorFrame = true;
  }

  fCodec->startIncrementalDecode(...);
  // Assuming startIncrementalDecode returned kSuccess...

  if (!prevBuffer.copyBitmapData()) {
    return setFailed(); // <---
  }

If not for the line that calls setFailed(), we would be in a weird state where
we started the incremental decode, but lied about whether we put the prior
frame's data in the buffer before decoding. But copyBitmapData won't fail in the
general case (it could fail if we ran OOM, for example, but will not fail for
picking the wrong color type, since we just use N32 always), and if it does we
won't try to decode anyway.
That's what I was thinking.

[cblume, 2016/12/20 21:59:00]

Done.

+      frame.setStatus(ImageFrame::FramePartial);
+    } else {
+      setFailed();
+      return;
     }
   }
 
-  // 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)) {
-      setFailed();
-      return;
+  int rowsDecoded = 0;
+  SkCodec::Result incrementalDecodeResult =
+      m_codec->incrementalDecode(&rowsDecoded);
+  if (incrementalDecodeResult == SkCodec::kSuccess) {
+    frame.setStatus(ImageFrame::FrameComplete);
+  } else if (incrementalDecodeResult == SkCodec::kIncompleteInput) {
+    if (frame.getStatus() == ImageFrame::FrameEmpty && index == 0) {
+      // We want to display the bit of the frame we have decoded only if it is
+      // the first frame of an animation. That means we need to fill the rest
+      // of the image with transparent.
+      IntRect remainingRect = frame.originalFrameRect();
+      remainingRect.setHeight(remainingRect.height() - rowsDecoded);
+      frame.zeroFillFrameRect(remainingRect);
     }
 
-    // If this returns false, we need more data to continue decoding.
-    if (!postDecodeProcessing(*i))
-      break;
+    frame.setStatus(ImageFrame::FramePartial);
+  } else {
+    setFailed();
   }
 
-  // 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())
+  if (!postDecodeProcessing(index)) {
     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;
 }
 
 }  // namespace blink