Move gif image decoder to SkCodec

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

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 25a00e19e67b15afea346b667c28500d2a13b27d..942d951cb6030df533af969863b3899e8d88a7b6 100644
--- a/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/gif/GIFImageDecoder.cpp
@@ -26,9 +26,9 @@
 #include "platform/image-decoders/gif/GIFImageDecoder.h"
 
 #include <limits>
-#include "platform/image-decoders/gif/GIFImageReader.h"
 #include "platform/wtf/NotFound.h"
 #include "platform/wtf/PtrUtil.h"
+#include "third_party/skia/include/core/SkImageInfo.h"
 
 namespace blink {
 
@@ -36,247 +36,259 @@ GIFImageDecoder::GIFImageDecoder(AlphaOption alpha_option,
                                  const ColorBehavior& color_behavior,
                                  size_t max_decoded_bytes)
     : ImageDecoder(alpha_option, color_behavior, max_decoded_bytes),
-      repetition_count_(kCAnimationLoopOnce) {}
-
-GIFImageDecoder::~GIFImageDecoder() {}
+      codec_(),
+      segment_stream_(nullptr),
+      frame_infos_() {}
+
+GIFImageDecoder::~GIFImageDecoder() {
+  if (!codec_) {
+    // if we did not create |codec_| and thus did not pass ownership to it
+    if (segment_stream_)
+      delete segment_stream_;
+  }
+}
 
 void GIFImageDecoder::OnSetData(SegmentReader* data) {
-  if (reader_)
-    reader_->setData(data);
+  if (!data) {
+    if (segment_stream_)
+      segment_stream_->SetReader(nullptr, false);
+    return;
+  }
+
+  if (!segment_stream_)
+    segment_stream_ = new SegmentStream();
+
+  segment_stream_->SetReader(data, IsAllDataReceived());
+
+  // If we don't have a SkCodec yet, create one from the stream
+  if (!codec_) {
+    codec_.reset(SkCodec::NewFromStream(segment_stream_, nullptr));
+    if (!codec_) {
+      // |segment_stream_|'s ownership is passed into NewFromStream.
+      // It is deleted if NewFromStream fails.
+      // If NewFromStream fails, we set |segment_stream_| to null so
+      // we aren't pointing to reclaimed memory.
+      segment_stream_ = nullptr;
+      return;
+    }
+
+    // SkCodec::NewFromStream will read enough of the image to get the image
+    // size.
+    SkImageInfo image_info = codec_->getInfo();
+    SetSize(image_info.width(), image_info.height());
+  }
 }
 
 int GIFImageDecoder::RepetitionCount() const {
+  if (!codec_)
+    return kCAnimationLoopOnce;
+
   // 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() && reader_->imagesCount() == 1)
-    repetition_count_ = kCAnimationNone;
-  else if (Failed() || (reader_ && (!reader_->imagesCount())))
-    repetition_count_ = kCAnimationLoopOnce;
-  else if (reader_ && reader_->loopCount() != cLoopCountNotSeen)
-    repetition_count_ = reader_->loopCount();
-  return repetition_count_;
+  // SkCodec will parse forward in the file if the repetition count has not been
+  // seen yet.
+
+  if (IsAllDataReceived() && frame_infos_.size() == 1)
+    return kCAnimationNone;
+  if (Failed())
+    return kCAnimationLoopOnce;
+
+  int repetition_count = codec_->getRepetitionCount();
+  switch (repetition_count) {
+    case 0:
+      return kCAnimationLoopOnce;
+    case SkCodec::kRepetitionCountInfinite:
+      return kCAnimationLoopInfinite;
+    default:
+      return repetition_count;
+  }
 }
 
 bool GIFImageDecoder::FrameIsCompleteAtIndex(size_t index) const {
-  return reader_ && (index < reader_->imagesCount()) &&
-         reader_->frameContext(index)->isComplete();
+  if (!codec_)
+    return false;
+
+  if (frame_infos_.size() <= index)
+    return false;
+
+  return frame_infos_[index].fFullyReceived;
 }
 
 float GIFImageDecoder::FrameDurationAtIndex(size_t index) const {
-  return (reader_ && (index < reader_->imagesCount()) &&
-          reader_->frameContext(index)->isHeaderDefined())
-             ? reader_->frameContext(index)->delayTime()
-             : 0;
+  if (index < frame_buffer_cache_.size())
+    return frame_buffer_cache_[index].Duration();
+  return 0;
 }
 
 bool GIFImageDecoder::SetFailed() {
-  reader_.reset();
+  segment_stream_ = nullptr;
+  codec_ = nullptr;

[scroggo_chromium, 2017/04/21 20:09:32]

Presumably codec_ is never null in this situation. If it were, we'd leak
segment_stream_

[cblume, 2017/04/21 23:47:38]

I see what you're saying.
Since this only happens when decoding fails, we could check if codec_ isn't null
and then delete segment_stream_. Even if that scenario never happens, neither
should calling SetFailed().
I am happy to favor on the side of defensive coding in case something changes
later or I understand correctly and codec_ could be non-null.

[scroggo_chromium, 2017/04/24 15:06:54]

  DCHECK(codec_);

should be a good defense.

[cblume, 2017/04/24 18:22:33]

Done.

   return ImageDecoder::SetFailed();
 }
 
-bool GIFImageDecoder::HaveDecodedRow(size_t frame_index,
-                                     GIFRow::const_iterator row_begin,
-                                     size_t width,
-                                     size_t row_number,
-                                     unsigned repeat_count,
-                                     bool write_transparent_pixels) {
-  const GIFFrameContext* frame_context = reader_->frameContext(frame_index);
-  // 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 x_begin = frame_context->xOffset();
-  const int y_begin = frame_context->yOffset() + row_number;
-  const int x_end = std::min(static_cast<int>(frame_context->xOffset() + width),
-                             Size().Width());
-  const int y_end = std::min(
-      static_cast<int>(frame_context->yOffset() + row_number + repeat_count),
-      Size().Height());
-  if (!width || (x_begin < 0) || (y_begin < 0) || (x_end <= x_begin) ||
-      (y_end <= y_begin))
-    return true;
-
-  const GIFColorMap::Table& color_table =
-      frame_context->localColorMap().isDefined()
-          ? frame_context->localColorMap().getTable()
-          : reader_->globalColorMap().getTable();
-
-  if (color_table.IsEmpty())
-    return true;
-
-  GIFColorMap::Table::const_iterator color_table_iter = color_table.begin();
-
-  // Initialize the frame if necessary.
-  ImageFrame& buffer = frame_buffer_cache_[frame_index];
-  if (!InitFrameBuffer(frame_index))
-    return false;
-
-  const size_t transparent_pixel = frame_context->transparentPixel();
-  GIFRow::const_iterator row_end = row_begin + (x_end - x_begin);
-  ImageFrame::PixelData* current_address = buffer.GetAddr(x_begin, y_begin);
-
-  // 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 (write_transparent_pixels) {
-    for (; row_begin != row_end; ++row_begin, ++current_address) {
-      const size_t source_value = *row_begin;
-      if ((source_value != transparent_pixel) &&
-          (source_value < color_table.size())) {
-        *current_address = color_table_iter[source_value];
-      } else {
-        *current_address = 0;
-        current_buffer_saw_alpha_ = true;
-      }
-    }
-  } else {
-    for (; row_begin != row_end; ++row_begin, ++current_address) {
-      const size_t source_value = *row_begin;
-      if ((source_value != transparent_pixel) &&
-          (source_value < color_table.size()))
-        *current_address = color_table_iter[source_value];
-      else
-        current_buffer_saw_alpha_ = true;
-    }
-  }
-
-  // Tell the frame to copy the row data if need be.
-  if (repeat_count > 1)
-    buffer.CopyRowNTimes(x_begin, x_end, y_begin, y_end);
-
-  buffer.SetPixelsChanged(true);
-  return true;
-}
-
-bool GIFImageDecoder::ParseCompleted() const {
-  return reader_ && reader_->parseCompleted();
-}
-
-bool GIFImageDecoder::FrameComplete(size_t frame_index) {
-  // Initialize the frame if necessary.  Some GIFs insert do-nothing frames,
-  // in which case we never reach haveDecodedRow() before getting here.
-  if (!InitFrameBuffer(frame_index))
-    return SetFailed();
-
-  if (!current_buffer_saw_alpha_)
-    CorrectAlphaWhenFrameBufferSawNoAlpha(frame_index);
-
-  frame_buffer_cache_[frame_index].SetStatus(ImageFrame::kFrameComplete);
+size_t GIFImageDecoder::DecodeFrameCount() {
+  if (!codec_)
+    return 0;
 
-  return true;
+  if (!Failed() && (segment_stream_ && !segment_stream_->IsCleared()))
+    frame_infos_ = codec_->getFrameInfo();
+  return frame_infos_.size();
 }
 
-void GIFImageDecoder::ClearFrameBuffer(size_t frame_index) {
-  if (reader_ && frame_buffer_cache_[frame_index].GetStatus() ==
-                     ImageFrame::kFramePartial) {
-    // Reset the state of the partial frame in the reader so that the frame
-    // can be decoded again when requested.
-    reader_->clearDecodeState(frame_index);
+void GIFImageDecoder::InitializeNewFrame(size_t index) {
+  DCHECK(codec_);
+
+  ImageFrame& frame = frame_buffer_cache_[index];
+  // SkCodec does not inform us if only a portion of the image was updated
+  // in the current frame. Because of this, rather than correctly filling in
+  // the frame rect, we set the frame rect to be the image's full size.
+  IntSize full_image_size = Size();
+  frame.SetOriginalFrameRect(IntRect(IntPoint(), full_image_size));
+  frame.SetDuration(frame_infos_[index].fDuration);
+  frame.SetHasAlpha(!SkAlphaTypeIsOpaque(frame_infos_[index].fAlphaType));
+  size_t required_previous_frame_index;
+  if (frame_infos_[index].fRequiredFrame == SkCodec::kNone) {
+    required_previous_frame_index = WTF::kNotFound;
+  } else {
+    required_previous_frame_index =
+        static_cast<size_t>(frame_infos_[index].fRequiredFrame);
   }
-  ImageDecoder::ClearFrameBuffer(frame_index);
+  frame.SetRequiredPreviousFrameIndex(required_previous_frame_index);
+  // 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::kDisposeKeep);
 }
 
-size_t GIFImageDecoder::DecodeFrameCount() {
-  Parse(kGIFFrameCountQuery);
-  // 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() ? frame_buffer_cache_.size() : reader_->imagesCount();
-}
+void GIFImageDecoder::Decode(size_t index) {
+  if (Failed())
+    return;
 
-void GIFImageDecoder::InitializeNewFrame(size_t index) {
-  ImageFrame* buffer = &frame_buffer_cache_[index];
-  const GIFFrameContext* frame_context = reader_->frameContext(index);
-  buffer->SetOriginalFrameRect(
-      Intersection(frame_context->frameRect(), IntRect(IntPoint(), Size())));
-  buffer->SetDuration(frame_context->delayTime());
-  buffer->SetDisposalMethod(frame_context->getDisposalMethod());
-  buffer->SetRequiredPreviousFrameIndex(
-      FindRequiredPreviousFrame(index, false));
-}
+  if (!codec_)
+    return;
 
-void GIFImageDecoder::Decode(size_t index) {
-  Parse(kGIFFrameCountQuery);
+  if (segment_stream_ && segment_stream_->IsCleared())
+    return;
+
+  if (frame_buffer_cache_.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();
 
-  if (Failed())
     return;
+  }
 
   UpdateAggressivePurging(index);
+  SkImageInfo image_info = codec_->getInfo().makeColorType(kN32_SkColorType);
+
+  SkCodec::Options options;
+  options.fFrameIndex = index;
+  options.fHasPriorFrame = false;
+  options.fZeroInitialized = SkCodec::kNo_ZeroInitialized;
+
+  ImageFrame& frame = frame_buffer_cache_[index];
+  if (frame.GetStatus() == ImageFrame::kFrameEmpty) {
+    size_t required_previous_frame_index = frame.RequiredPreviousFrameIndex();
+    if (required_previous_frame_index == WTF::kNotFound) {
+      frame.AllocatePixelData(Size().Width(), Size().Height(),
+                              ColorSpaceForSkImages());
+      frame.SetStatus(ImageFrame::kFrameAllocated);

[scroggo_chromium, 2017/04/21 20:09:32]

Should this be set inside AllocatePixelData?

[cblume, 2017/04/21 23:47:38]

Mmmm maybe. I also need it in the else branch here (which does not call
AllocatePixelData).

I was missing the call to SetStatus(Allocated) in the else branch on this PS,
which caused dependent frames to not call startIncrementalDecode. I've added it
in a new PS.

Anyway, if I leave it outside AllocatePixelData() then we have a nice balance
between these two branches where the reader can see both set the status to
allocated. If instead I move it inside AllocatePixelData() then the two branches
are lopsided.

Actually, since it happens no matter which branch is taken I should probably
move it out of the if/else. I could still have it inside AllocatePixelData() and
then just set the status twice for non-dependent frames.

Which do you prefer?

[scroggo_chromium, 2017/04/24 15:06:54]

Another option - change the status to Allocated inside the other methods that
make the Frame have an allocation (TakeBitmapDataIfWritable and CopyBitmapData).
Currently it looks like those two stick with FrameEmpty (although the caller
follows up by setting it to Partial).

[cblume, 2017/04/24 18:22:33]

Done.

+      frame.SetHasAlpha(!SkAlphaTypeIsOpaque(frame_infos_[index].fAlphaType));
+    } else {
+      ImageFrame& required_previous_frame =
+          frame_buffer_cache_[required_previous_frame_index];
+
+      if (required_previous_frame.GetStatus() != ImageFrame::kFrameComplete)
+        Decode(required_previous_frame_index);
+
+      // We try to reuse |required_previous_frame| as starting state to avoid
+      // copying. If CanReusePreviousFrameBuffer returns false, we must copy
+      // the data since |required_previous_frame| is necessary to decode this
+      // or later frames. In that case copy the data instead.
+      if ((!CanReusePreviousFrameBuffer(index) ||
+           !frame.TakeBitmapDataIfWritable(&required_previous_frame)) &&
+          !frame.CopyBitmapData(required_previous_frame)) {
+        SetFailed();
+        return;
+      }
+      options.fHasPriorFrame = true;

[scroggo_chromium, 2017/04/21 20:09:32]

After this, what should the FrameStatus be? Looking down below, it looks like we
want it to be kFrameAllocated, so we'll startIncrementalDecode, but it doesn't
look like it is. (I think it will be FrameEmpty?)

In the old code, InitFrameBuffer sets the status to FramePartial after it either
(A) allocates and zeroes or (B) steals/copies from the prior frame.

The interesting bit is that you've changed what Partial means - it used to mean
that we've at least gotten into the starting state for decode (A or B, above),
but now you're using it to mean that startIncrementalDecode succeeded.
Presumably Allocated means A or B has happened (though we don't yet change it to
Allocated in this case). Meanwhile, the other ImageDecoders still use the old
meaning of Partial, and never go into the Allocated state. I don't know that
this causes any direct problems, but it seems like an extra maintenance burden
(due to confusion).

[cblume, 2017/04/21 23:47:38]

Hahaha. Good eye. That is exactly the bug I fixed on the last patch set. :)
The status should have been set to allocated.


I see what you mean about redefining Partial.
Without updating all the other decoders, having two definitions is definitely a
bad thing.
Although, they may all already be using the new definition of Partial.


The other coders couldn't possibly fail between allocating/stealing/copying and
getting into the starting state for decode IIUC. So essentially, their status
would change to Allocated and then immediately change to Partial. So we don't
need to update them to add the superfluous Allocated status. It would be as if
the other decoders got to skip calling startIncrementalDecode and thus got to
skip the Allocated status.

So the definition of Partial for all image decoders should be "In a state where
we can decode the rest of the frame".

+    }
+  }
 
-  Vector<size_t> frames_to_decode = FindFramesToDecode(index);
-  for (auto i = frames_to_decode.rbegin(); i != frames_to_decode.rend(); ++i) {
-    if (!reader_->decode(*i)) {
-      SetFailed();
-      return;
+  if (frame.GetStatus() == ImageFrame::kFrameAllocated) {
+    SkCodec::Result start_incremental_decode_result =
+        codec_->startIncrementalDecode(image_info, frame.Bitmap().getPixels(),
+                                       frame.Bitmap().rowBytes(), &options,
+                                       nullptr, nullptr);
+    switch (start_incremental_decode_result) {
+      case SkCodec::kSuccess:
+        break;
+      case SkCodec::kIncompleteInput:
+        return;
+      default:
+        SetFailed();
+        return;
     }
+    frame.SetStatus(ImageFrame::kFramePartial);
+  }
+  int rows_decoded = 0;
+  SkCodec::Result incremental_decode_result =
+      codec_->incrementalDecode(&rows_decoded);
+  switch (incremental_decode_result) {
+    case SkCodec::kSuccess:
+      frame.SetHasAlpha(!SkAlphaTypeIsOpaque(frame_infos_[index].fAlphaType));
+      frame.SetPixelsChanged(true);
+      frame.SetStatus(ImageFrame::kFrameComplete);
+      PostDecodeProcessing(index);
+      break;
+    case SkCodec::kIncompleteInput:
+      if (FrameIsCompleteAtIndex(index) || IsAllDataReceived()) {
+        SetFailed();
+        return;
+      }
+      {
+        IntRect remaining_rect = frame.OriginalFrameRect();
+        remaining_rect.SetY(rows_decoded);
+        remaining_rect.SetHeight(remaining_rect.Height() - rows_decoded);
+        frame.ZeroFillFrameRect(remaining_rect);
+        frame.SetHasAlpha(true);
+      }
 
-    // If this returns false, we need more data to continue decoding.
-    if (!PostDecodeProcessing(*i))
+      frame.SetPixelsChanged(true);
       break;
+    default:
+      SetFailed();
+      return;
   }
-
-  // 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 >= frame_buffer_cache_.size() - 1 && IsAllDataReceived() &&
-      reader_ && !reader_->parseCompleted())
-    SetFailed();
 }
 
-void GIFImageDecoder::Parse(GIFParseQuery query) {
-  if (Failed())
-    return;
+bool GIFImageDecoder::CanReusePreviousFrameBuffer(size_t index) const {
+  DCHECK(index < frame_buffer_cache_.size());
 
-  if (!reader_) {
-    reader_ = WTF::MakeUnique<GIFImageReader>(this);
-    reader_->setData(data_);
-  }
+  // 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 (!reader_->parse(query))
-    SetFailed();
-}
+  if (index + 1 >= frame_buffer_cache_.size())
+    return false;
 
-void GIFImageDecoder::OnInitFrameBuffer(size_t frame_index) {
-  current_buffer_saw_alpha_ = false;
-}
+  const ImageFrame& frame = frame_buffer_cache_[index];
+  size_t required_frame_index = frame.RequiredPreviousFrameIndex();
+
+  const ImageFrame& next_frame = frame_buffer_cache_[index + 1];
+  size_t next_required_frame_index = next_frame.RequiredPreviousFrameIndex();
 
-bool GIFImageDecoder::CanReusePreviousFrameBuffer(size_t frame_index) const {
-  DCHECK(frame_index < frame_buffer_cache_.size());
-  return frame_buffer_cache_[frame_index].GetDisposalMethod() !=
-         ImageFrame::kDisposeOverwritePrevious;
+  return required_frame_index != next_required_frame_index;
 }
 
 }  // namespace blink