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 <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 {
 
 GIFImageDecoder::GIFImageDecoder(AlphaOption alpha_option,
                                  const ColorBehavior& color_behavior,
                                  size_t max_decoded_bytes)
     : ImageDecoder(alpha_option, color_behavior, max_decoded_bytes),
-      repetition_count_(kAnimationLoopOnce) {}
-
-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);
+    return;
+  }
+
+  if (!segment_stream_)
+    segment_stream_ = new SegmentStream();
+
+  segment_stream_->SetReader(PassRefPtr<SegmentReader>(data));
+
+  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 kAnimationLoopOnce;
+
+  DCHECK(!Failed());
+
   // 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 kCLoopCountNotSeen, in which case we
-  // should default to looping once (the initial value for
-  // |repetition_count_|).
+  // 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 kCLoopCountNotSeen, 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
-  // |repetition_count_| 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() && reader_->ImagesCount() == 1)
-    repetition_count_ = kAnimationNone;
-  else if (Failed() || (reader_ && (!reader_->ImagesCount())))
-    repetition_count_ = kAnimationLoopOnce;
-  else if (reader_ && reader_->LoopCount() != kCLoopCountNotSeen)
-    repetition_count_ = reader_->LoopCount();
-  return repetition_count_;
+  // SkCodec will parse forward in the file if the repetition count has not been
+  // seen yet.
+  int repetition_count = codec_->getRepetitionCount();
+
+  switch (repetition_count) {
+    case 0: {
+      size_t frame_count = codec_->getFrameCount();
+      if (IsAllDataReceived() && frame_count == 1)
+        return kAnimationNone;
+
+      return kAnimationLoopOnce;
+    }
+    case SkCodec::kRepetitionCountInfinite:
+      return kAnimationLoopInfinite;
+    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();
+  DCHECK(codec_);
+
+  segment_stream_ = nullptr;
+  codec_ = nullptr;
   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.
-  // (frameC_context->xOffset, frame_context->yOffset). There is no guarantee
-  // that width == (size().width() - frame_context->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 |write_transparent_pixels|.
-  // 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;
+size_t GIFImageDecoder::DecodeFrameCount() {
+  if (!codec_)
+    return 0;
+
+  if (!Failed() && !segment_stream_->IsCleared())
+    frame_infos_ = codec_->getFrameInfo();

[scroggo_chromium, 2017/06/05 19:00:53]

We talked about calling codec_->getFrameCount() and then the version of
SkCodec::getFrameInfo() that takes an index to avoid copying the vector. Are you
planning to include that here?

[cblume, 2017/06/06 04:04:35]

Oh, I missed that patch to SkCodec::getFrameInfo() had landed.
I'll add it now.

Hrmmm a small problem with these get* functions doing work is I *may* need to
maintain local state about the value.

For example, if we enter a failed state and someone requests the frame count, I
don't want to do any more work parsing for that frame count. So any function of
mine which might fail also needs to first update a local copy of the frame
count. That way when if (Failed()) I can return that local frame count without
doing work.

I think in this case it is actually fine. I think after we call SetFailed() we
don't call DecodeFrameCount().

But it still leaves two issues: When segment_stream_ is cleared and
FrameIsCompleteAtIndex(), which is const. I can still call
codec_->getFrameCount() inside the const function because the pointer is const
but the pointed-to object isn't const. In a way, it is like the member is marked
mutable.

None of the tests fail when I try avoiding local state (which is my preference).
I'll go with that.

+  return frame_infos_.size();
+}
+
+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.
+  // The original frame rect is not used, anyway.
+  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);
+  }
+  frame.SetRequiredPreviousFrameIndex(required_previous_frame_index);
+  // The disposal method is not required any more, but is left in place

[scroggo_chromium, 2017/06/05 19:00:53]

Sorry, I still need to add this to SkCodec.

[cblume, 2017/06/06 04:04:35]

No problem.
Even without it, fRequiredFrame allows us to maintain existing behavior.

This comment is specifically about Blink::ImageFrame and why we still set a
disposal method there.

+  // for the other image decoders that do not yet rely on SkCodec.
+  // For now, fill it with DisposeKeep.
+  frame.SetDisposalMethod(ImageFrame::kDisposeKeep);
+}
+
+void GIFImageDecoder::Decode(size_t index) {
+  if (!codec_)
+    return;
+
+  DCHECK(!Failed());
+
+  DCHECK_LT(index, frame_buffer_cache_.size());
+
+  if (segment_stream_->IsCleared())
+    return;
+
+  UpdateAggressivePurging(index);
+  SkImageInfo image_info = codec_->getInfo().makeColorType(kN32_SkColorType);

[scroggo_chromium, 2017/06/05 19:00:53]

I think you also want to modify this to use ColorSpaceForSkImages() i.e.

  image_info = codec_->getInfo().makeColorType(kN32_SkColorType)
                                                     
.makeColorSpace(ColorSpaceForSkImages());

That way we tell the codec to decode into the color space that the ImageFrame's
SkBitmap claims to be.

msarett@, am I understanding that correctly?

[cblume, 2017/06/06 04:04:36]

Done.

+
+  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());
+    } 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;
       }
-    }
-  } 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);
-
-  return true;
-}
-
-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);
-  }
-  ImageDecoder::ClearFrameBuffer(frame_index);
-}
-
-size_t GIFImageDecoder::DecodeFrameCount() {
-  Parse(kGIFFrameCountQuery);
-  // If decoding fails, |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::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));
-}
-
-void GIFImageDecoder::Decode(size_t index) {
-  Parse(kGIFFrameCountQuery);
-
-  if (Failed())
-    return;
-
-  UpdateAggressivePurging(index);
-
-  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)) {
+      options.fHasPriorFrame = true;
+    }
+  }
+
+  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.SetPixelsChanged(true);
+      frame.SetStatus(ImageFrame::kFrameComplete);
+      PostDecodeProcessing(index);
+      break;
+    case SkCodec::kIncompleteInput:
+      if (FrameIsCompleteAtIndex(index) || IsAllDataReceived()) {
+        SetFailed();

[scroggo_chromium, 2017/06/05 19:00:53]

I think we may want to do this *after* zeroing the remaining lines, because I
think we may continue to show a frame after calling SetFailed().

[cblume, 2017/06/06 04:04:35]

Done.

+        return;
+      }
+      {
+        IntRect remaining_rect = frame.OriginalFrameRect();

[scroggo_chromium, 2017/06/05 19:00:53]

It seems weird to start with OriginalFrameRect. If it was returning what it
claims to, this would be wrong. I would start with the dimensions of Size()

[cblume, 2017/06/06 04:04:35]

Done.

+        remaining_rect.SetY(rows_decoded);
+        remaining_rect.SetHeight(remaining_rect.Height() - rows_decoded);
+        frame.ZeroFillFrameRect(remaining_rect);

[scroggo_chromium, 2017/06/05 19:00:53]

We only need to do this the first time we get an incomplete decode. The second
time, it will already be zeroed.

[cblume, 2017/06/06 04:04:36]

Done.

+        // ZeroFillFrameRect() resets the alpha to true.

[scroggo_chromium, 2017/06/05 19:00:53]

I think you had a patch that changed this (but maybe it was rolled into another
CL)? Would it make sense to change it so we don't need this line + comments?

[cblume, 2017/06/06 04:04:36]

Right. The CL is here:
https://codereview.chromium.org/2762643004/

I haven't gotten back to it yet since it isn't a high priority.

I think it might be best to land this patch as-is and update that CL separately
after this lands.

Maybe I'm misunderstanding what you're saying.

+        // We want to preserve the alpha. So we need to set it back.
+        frame.SetHasAlpha(!SkAlphaTypeIsOpaque(frame_infos_[index].fAlphaType));

[scroggo_chromium, 2017/06/05 19:00:53]

Maybe it would be clearer that we're setting it back if we did somehting like:

  bool oldAlpha = frame.HasAlpha();
  frame.ZeroFillFrameRect(remaining_rect);
  frame.setHasAlpha(oldAlpha);

Or, since we'll only ever change it back to false:

  bool actuallyOpaque = frame.HasAlpha();
  frame.ZeroFillFrameRect(remaining_rect);
  if (actuallyOpaque) {
    frame.setHasAlpha(false);
  }

[cblume, 2017/06/06 04:04:35]

Done.
I went with the first version which I think is easier to immediately understand.
But I'm happy to do the other if you want.

+      }
+
+      frame.SetPixelsChanged(true);
+      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 >= frame_buffer_cache_.size() - 1 && IsAllDataReceived() &&
-      reader_ && !reader_->ParseCompleted())
-    SetFailed();
-}
-
-void GIFImageDecoder::Parse(GIFParseQuery query) {
-  if (Failed())
-    return;
-
-  if (!reader_) {
-    reader_ = WTF::MakeUnique<GIFImageReader>(this);
-    reader_->SetData(data_);
-  }
-
-  if (!reader_->Parse(query))
-    SetFailed();
-}
-
-void GIFImageDecoder::OnInitFrameBuffer(size_t frame_index) {
-  current_buffer_saw_alpha_ = false;
-}
-
-bool GIFImageDecoder::CanReusePreviousFrameBuffer(size_t frame_index) const {
-  DCHECK(frame_index < frame_buffer_cache_.size());
-  return frame_buffer_cache_[frame_index].GetDisposalMethod() !=
-         ImageFrame::kDisposeOverwritePrevious;
+  }
+}
+
+bool GIFImageDecoder::CanReusePreviousFrameBuffer(size_t index) const {
+  DCHECK(index < frame_buffer_cache_.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 >= frame_buffer_cache_.size())
+    return 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();
+
+  return required_frame_index != next_required_frame_index;
 }
 
 }  // namespace blink