Add animation support for WebP images

Source/core/platform/image-decoders/webp/WEBPImageDecoder.cpp

 /*
  * Copyright (C) 2010 Google 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
@@ skipping 15 matching lines @@
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "core/platform/image-decoders/webp/WEBPImageDecoder.h"
 
 #include "core/platform/PlatformInstrumentation.h"
 
 #ifdef QCMS_WEBP_COLOR_CORRECTION
 #include "qcms.h"
-#include "webp/demux.h"
-#else
-#undef ICCP_FLAG
+#endif
+
+#ifdef WEBP_ICC_ANIMATION_SUPPORT
+#include "RuntimeEnabledFeatures.h"
+#include "webp/format_constants.h"
+#endif
+
+#if (WEBP_DECODER_ABI_VERSION < 0x0163)
+// Backward emulation for versions earlier than 0.1.99.
+#define MODE_rgbA MODE_RGBA
+#define MODE_bgrA MODE_BGRA
+#define ALPHA_FLAG 0
+#define ICCP_FLAG 0

[Noel Gordon, 2013/05/01 19:11:41]

Remove. ICCP_FLAG use is guarded in decode() now.

[urvang (Google), 2013/05/01 22:25:16]

Done.

+#elif (WEBP_DECODER_ABI_VERSION <= 0x0200)
+// Backward emulation for versions earlier than 0.3.0.
+#define ALPHA_FLAG 0x000010
 #define ICCP_FLAG 0

[Noel Gordon, 2013/05/01 19:11:41]

Ditto.

[urvang (Google), 2013/05/01 22:25:16]

Done.

 #endif
 
-// Backward emulation for earlier versions than 0.1.99.
-#if (WEBP_DECODER_ABI_VERSION < 0x0163)
-#define MODE_rgbA MODE_RGBA
-#define MODE_bgrA MODE_BGRA
-#endif
-
 #if CPU(BIG_ENDIAN) || CPU(MIDDLE_ENDIAN)
 inline WEBP_CSP_MODE outputMode(bool hasAlpha) { return hasAlpha ? MODE_rgbA : MODE_RGBA; }
 #elif SK_B32_SHIFT
 inline WEBP_CSP_MODE outputMode(bool hasAlpha) { return hasAlpha ? MODE_rgbA : MODE_RGBA; }
 #else // LITTLE_ENDIAN, output BGRA pixels.
 inline WEBP_CSP_MODE outputMode(bool hasAlpha) { return hasAlpha ? MODE_bgrA : MODE_BGRA; }
 #endif
 
 namespace WebCore {
 
 WEBPImageDecoder::WEBPImageDecoder(ImageSource::AlphaOption alphaOption,
                                    ImageSource::GammaAndColorProfileOption gammaAndColorProfileOption)
     : ImageDecoder(alphaOption, gammaAndColorProfileOption)
     , m_decoder(0)
-    , m_hasAlpha(false)
     , m_formatFlags(0)
 #ifdef QCMS_WEBP_COLOR_CORRECTION
     , m_haveReadProfile(false)
     , m_transform(0)
+#endif
+#ifdef WEBP_ICC_ANIMATION_SUPPORT
     , m_decodedHeight(0)
+    , m_haveAlreadyParsedThisData(false)
+    , m_demux(0)
+    , m_demuxState(WEBP_DEMUX_PARSING_HEADER)
+    , m_haveReadAnimParams(false)
+    , m_repetitionCount(cAnimationLoopOnce)
 #endif
 {
-    WebPInitDecBuffer(&m_decoderBuffer);
 }
 
 WEBPImageDecoder::~WEBPImageDecoder()
 {
     clear();
 }
 
 void WEBPImageDecoder::clear()
 {
 #ifdef QCMS_WEBP_COLOR_CORRECTION
     if (m_transform)
         qcms_transform_release(m_transform);
     m_transform = 0;
 #endif
-    WebPFreeDecBuffer(&m_decoderBuffer);
-    if (m_decoder)
-        WebPIDelete(m_decoder);
+#ifdef WEBP_ICC_ANIMATION_SUPPORT
+    WebPDemuxDelete(m_demux);
+    m_demux = 0;
+#endif
+    clearDecoder();
+}
+
+void WEBPImageDecoder::clearDecoder()
+{
+    WebPIDelete(m_decoder);
     m_decoder = 0;
+#ifdef WEBP_ICC_ANIMATION_SUPPORT
+    m_decodedHeight = 0;
+#endif
 }
 
 bool WEBPImageDecoder::isSizeAvailable()
 {
-    if (!ImageDecoder::isSizeAvailable())
-         decode(true);
-
+    if (!ImageDecoder::isSizeAvailable()) {
+#ifdef WEBP_ICC_ANIMATION_SUPPORT
+        if (!updateDemuxer())
+            return 0;
+#else
+        decode(reinterpret_cast<const uint8_t*>(m_data->data()), m_data->size(), true, 0);
+#endif
+    }
     return ImageDecoder::isSizeAvailable();
 }
 
-ImageFrame* WEBPImageDecoder::frameBufferAtIndex(size_t index)
-{
-    if (index)
+size_t WEBPImageDecoder::frameCount()
+{
+#ifdef WEBP_ICC_ANIMATION_SUPPORT
+    if (!updateDemuxer())
         return 0;
-
+#else
     if (m_frameBufferCache.isEmpty()) {
         m_frameBufferCache.resize(1);
         m_frameBufferCache[0].setPremultiplyAlpha(m_premultiplyAlpha);
     }
-
-    ImageFrame& frame = m_frameBufferCache[0];
-    if (frame.status() != ImageFrame::FrameComplete) {
+#endif
+    return m_frameBufferCache.size();
+}
+
+ImageFrame* WEBPImageDecoder::frameBufferAtIndex(size_t index)
+{
+    if (index >= frameCount())
+        return 0;
+
+    ImageFrame& frame = m_frameBufferCache[index];
+    if (frame.status() == ImageFrame::FrameComplete)
+        return &frame;
+
+#ifdef WEBP_ICC_ANIMATION_SUPPORT
+    if (RuntimeEnabledFeatures::animatedWebPEnabled()) {
+        if (index && (m_frameBufferCache[index - 1].status() != ImageFrame::FrameComplete))
+            return 0; // We haven't fully decoded the previous frame yet.
+        ASSERT(m_demux);
+        WebPIterator fIter;

[Noel Gordon, 2013/05/01 19:11:41]

fIter again (abbrev). Perhaps call it buffer. 

[urvang (Google), 2013/05/01 22:25:16]

Calling it 'webpFrame' now.

+        if (!WebPDemuxGetFrame(m_demux, index + 1, &fIter))
+            return 0;
+        if ((m_formatFlags & ANIMATION_FLAG) && !initFrameBuffer(fIter, index))
+            return 0;
         PlatformInstrumentation::willDecodeImage("WEBP");
-        decode(false);
+        decode(fIter.fragment.bytes, fIter.fragment.size, false, index);
         PlatformInstrumentation::didDecodeImage();
-    }
+        WebPDemuxReleaseIterator(&fIter);
+        return &frame;
+    }
+#endif
+
+    ASSERT(!index);
+    PlatformInstrumentation::willDecodeImage("WEBP");
+    decode(reinterpret_cast<const uint8_t*>(m_data->data()), m_data->size(), false, index);
+    PlatformInstrumentation::didDecodeImage();
     return &frame;
 }
 
+#ifdef WEBP_ICC_ANIMATION_SUPPORT
+
+void WEBPImageDecoder::setData(SharedBuffer* data, bool allDataReceived)
+{
+    if (failed())
+        return;
+
+    ImageDecoder::setData(data, allDataReceived);
+
+    // Mark that we have new data.
+    if (m_demuxState != WEBP_DEMUX_DONE)
+        m_haveAlreadyParsedThisData = false;
+}
+
+bool WEBPImageDecoder::updateDemuxer()
+{
+    if (m_haveAlreadyParsedThisData)
+        return true;
+
+    static const size_t minSizeForDemux = RIFF_HEADER_SIZE + CHUNK_HEADER_SIZE;
+    if (m_data->size() < minSizeForDemux)
+        return false; // Wait for headers so that WebPDemuxPartial doesn't return null.
+
+    WebPDemuxDelete(m_demux);
+    WebPData inputData = { reinterpret_cast<const uint8_t*>(m_data->data()), m_data->size() };
+    m_demux = WebPDemuxPartial(&inputData, &m_demuxState);
+    if (!m_demux)
+        return setFailed(); // Must be a failure as we have at least 'minSizeForDemux' bytes.
+    m_haveAlreadyParsedThisData = true;

[Noel Gordon, 2013/05/01 19:11:41]

Should this line be moved somewhere before the return @195 ?  No matter how you
early return from this routine, I'm sure we have already parsed this data.

[urvang (Google), 2013/05/01 22:25:16]

Done.

+
+    if (m_demuxState <= WEBP_DEMUX_PARSING_HEADER)
+        return true; // Not enough data for parsing canvas width/height yet.

[Noel Gordon, 2013/05/01 19:11:41]

Return false; for consistency.

[urvang (Google), 2013/05/01 22:25:16]

Done.

+
+    if (!ImageDecoder::isSizeAvailable()) {
+        if (!setSize(WebPDemuxGetI(m_demux, WEBP_FF_CANVAS_WIDTH), WebPDemuxGetI(m_demux, WEBP_FF_CANVAS_HEIGHT)))
+            return setFailed();
+        m_formatFlags = WebPDemuxGetI(m_demux, WEBP_FF_FORMAT_FLAGS);
+    }
+    ASSERT(ImageDecoder::isSizeAvailable());
+    const bool hasAnimation = (m_formatFlags & ANIMATION_FLAG);

[Noel Gordon, 2013/05/01 19:11:41]

213-215. Could we move these lines up so we exit early, before we even try to
set the image size, and so on.

[urvang (Google), 2013/05/01 22:25:16]

Changed the code a bit to exit before setSize() call in such a case. See if this
is better.

+    if (!RuntimeEnabledFeatures::animatedWebPEnabled() && hasAnimation)
+        return setFailed();
+    const size_t newFrameCount = WebPDemuxGetI(m_demux, WEBP_FF_FRAME_COUNT);
+    if (hasAnimation && !m_haveReadAnimParams && (newFrameCount >= 1)) {

[Noel Gordon, 2013/05/01 19:11:41]

/curious newFrameCount >= 1.  Do single-frame images have loopCount and other
animation parameters?  

[urvang (Google), 2013/05/01 22:25:16]

Ideally, no. But theoretically, single-frame animated images are possible. 
The real reason: we are working on partial data. So, we check for 'newFrameCount
>= 1' to see if we have gone past the ANIM chunk (as noted in the comment).

+        // As we have parsed at least one frame (even if partially),
+        // we must already have parsed the animation properties.
+        // This is because ANIM chunk always precedes ANMF chunks.
+        const uint32_t loopCount = WebPDemuxGetI(m_demux, WEBP_FF_LOOP_COUNT);
+        // Note: The following casts an 'unsigned int' to 'int'. But that is fine, because loop count is always <= 16 bits.

[Noel Gordon, 2013/05/01 19:11:41]

How about turning the comment into code.

int loopCount = WebPDemuxGetI(m_demux, WEBP_FF_LOOP_COUNT) & 0xffff;

[urvang (Google), 2013/05/01 22:25:16]

That seems a bit ugly. Let me put an assert instead.

[Noel Gordon, 2013/05/02 18:00:27]

Maybe loopCount is just unnecessary noise.

m_repetitionCount = WebPDemuxGetI(m_demux, WEBP_FF_LOOP_COUNT) & 0xffff;
if (!m_repetitionCount)
    m_repetitionCount = cAnimationLoopInfinite;

+        m_repetitionCount = (!loopCount) ? cAnimationLoopInfinite : loopCount;

[Noel Gordon, 2013/05/01 19:11:41]

... = !loopCount ? cAnimationLoopInfinite ...

[urvang (Google), 2013/05/01 22:25:16]

Done.

+        m_haveReadAnimParams = true;
+    }
+    if (newFrameCount > m_frameBufferCache.size()) {
+        m_frameBufferCache.resize(newFrameCount);
+        for (size_t i = 0; i < newFrameCount; ++i)
+            m_frameBufferCache[i].setPremultiplyAlpha(m_premultiplyAlpha);
+    }
+    return true;
+}
+
+// FIXME: This method is very similar to the one in GIFImageDecoder.cpp and should be refactored.
+bool WEBPImageDecoder::initFrameBuffer(const WebPIterator& fIter, size_t frameIndex)

[Noel Gordon, 2013/05/01 19:11:41]

fIter.  Call it frame.

[urvang (Google), 2013/05/01 22:25:16]

Done.

+{
+    ImageFrame& buffer = m_frameBufferCache[frameIndex];
+    if (buffer.status() != ImageFrame::FrameEmpty) // Already initialized.
+        return true;
+
+    // Initialize the frame rect in our buffer.
+    IntRect frameRect(fIter.x_offset, fIter.y_offset, fIter.width, fIter.height);
+
+    // Make sure the frameRect doesn't extend outside the buffer.
+    if (frameRect.maxX() > size().width())
+        frameRect.setWidth(size().width() - fIter.x_offset);
+    if (frameRect.maxY() > size().height())
+        frameRect.setHeight(size().height() - fIter.y_offset);
+
+    const int left = upperBoundScaledX(frameRect.x());
+    const int right = lowerBoundScaledX(frameRect.maxX(), left);
+    const int top = upperBoundScaledY(frameRect.y());
+    const int bottom = lowerBoundScaledY(frameRect.maxY(), top);
+    buffer.setOriginalFrameRect(IntRect(left, top, right - left, bottom - top));
+
+    buffer.setDisposalMethod(fIter.dispose_method == WEBP_MUX_DISPOSE_BACKGROUND ? ImageFrame::DisposeOverwriteBgcolor : ImageFrame::DisposeKeep);

[Noel Gordon, 2013/05/01 19:11:41]

So note that the only disposals methods for a webp frame are:

ImageFrame::DisposeOverwriteBgcolor
ImageFrame::DisposeKeep

...

[urvang (Google), 2013/05/01 22:25:16]

see comment in 279

+    buffer.setDuration(fIter.duration);
+    buffer.setHasAlpha(m_formatFlags & ALPHA_FLAG);
+
+    if (!frameIndex) {
+        // This is the first frame, so we're not relying on any previous data.

[Noel Gordon, 2013/05/01 19:11:41]

... and that this if clause deals with !frameIndex case.

[urvang (Google), 2013/05/01 22:25:16]

see comment at 279

+        if (!buffer.setSize(scaledSize().width(), scaledSize().height()))
+            return setFailed();
+    } else {
+        // The starting state for this frame depends on the previous frame's
+        // disposal method.
+        const ImageFrame& prevBuffer = m_frameBufferCache[frameIndex - 1];
+        ASSERT(prevBuffer.status() == ImageFrame::FrameComplete);
+        const IntRect& prevRect = prevBuffer.originalFrameRect();
+        const ImageFrame::FrameDisposalMethod prevMethod = prevBuffer.disposalMethod();
+        if ((prevMethod == ImageFrame::DisposeKeep) || (prevMethod == ImageFrame::DisposeNotSpecified)) {

[Noel Gordon, 2013/05/01 19:11:41]

Is prevMethod == ImageFrame::DisposeNotSpecified a possible webp disposal
method?

[urvang (Google), 2013/05/01 22:25:16]

It's not. I kept this code snippet as similar to GIF one as possible for a
refactoring later.

+            // Preserve the last frame as the starting state for this frame.
+            if (!buffer.copyBitmapData(prevBuffer))
+                return setFailed();
+        } else { // prevMethod == ImageFrame::DisposeOverwriteBgcolor

[Noel Gordon, 2013/05/01 19:11:41]

This comment could break, an ASSERT would not.  Turn this comment into code.

[urvang (Google), 2013/05/01 22:25:16]

Done.

+            // We want to clear the previous frame to transparent, without
+            // affecting pixels in the image outside of the frame.
+            // So, we copy the whole previous buffer, then clear just its frame.
+            if (!frameIndex || prevRect.contains(IntRect(IntPoint(), scaledSize()))) {

[Noel Gordon, 2013/05/01 19:11:41]

Given the comment @261, what value does !frameIndex always have here?

[urvang (Google), 2013/05/01 22:25:16]

I see what you are getting  at.
But again, the idea is to keep this code snippet as similar to GIF one as
possible for a refactoring later. See the FIXME before this method.

[Noel Gordon, 2013/05/02 18:00:27]

Similar is fine: my view was a FIXME would also help the refactor.
 
// FIXME: "First frame" yet !frameIndex is always false here.

+                // Clearing the first frame, or a frame the size of the whole
+                // image, results in a completely empty image.
+                if (!buffer.setSize(scaledSize().width(), scaledSize().height()))
+                    return setFailed();
+            } else {
+                // Copy the whole previous buffer, then clear just its frame.
+                if (!buffer.copyBitmapData(prevBuffer))
+                    return setFailed();
+                for (int y = prevRect.y(); y < prevRect.maxY(); ++y) {
+                    for (int x = prevRect.x(); x < prevRect.maxX(); ++x)
+                        buffer.setRGBA(x, y, 0, 0, 0, 0);
+                }
+            }
+        }
+    }
+
+    // Update frame status to be partially complete.
+    buffer.setStatus(ImageFrame::FramePartial);
+    return true;
+}
+
+void WEBPImageDecoder::clearFrameBufferCache(size_t clearBeforeFrame)
+{
+    // We always preserve at least one frame.

[Noel Gordon, 2013/05/01 19:11:41]

The question is why?  The GIF decoder bails if m_frameBufferCache.isEmpty()
here.

[urvang (Google), 2013/05/01 22:25:16]

Note that GIF decoder never clears the 'end' (last frame) either.
In WebP case, due to 'DisposePrevious' not being possible, the above two
conditions can be merged to 'size() <= 1'.

And, the reason why we need at least one frame, is to construct the next frame
(as initFrame() needs previous frame). Added note.

[Noel Gordon, 2013/05/02 18:00:27]

Right, merged.  Thanks for the explanation.
 

+    if (m_frameBufferCache.size() <= 1)
+        return;
+
+    // Find the last frame we need to preserve in the cache to facilitate
+    // the construction of next frames (needed by initFrame() and
+    // applyPostProcessing()) . This frame is either:
+    //   * The last decoded frame in cache, OR
+    //   * The first frame (if cache doesn't contain any decoded frames).
+    const int lastFrame = std::min(clearBeforeFrame, m_frameBufferCache.size() - 1);
+    Vector<ImageFrame>::iterator i(m_frameBufferCache.begin() + lastFrame);
+    while ((i != m_frameBufferCache.begin()) && (i->status() != ImageFrame::FrameComplete))
+        --i;
+
+    // Now |i| holds the last frame we need to preserve; clear prior frames.
+    for (Vector<ImageFrame>::iterator j(m_frameBufferCache.begin()); j != i; ++j) {
+        ASSERT(j->status() != ImageFrame::FramePartial);
+        if (j->status() != ImageFrame::FrameEmpty)
+            j->clearPixelData();
+    }
+}
+
+#endif // WEBP_ICC_ANIMATION_SUPPORT
+
 #ifdef QCMS_WEBP_COLOR_CORRECTION
 
 void WEBPImageDecoder::createColorTransform(const char* data, size_t size)
 {
     if (m_transform)
         qcms_transform_release(m_transform);
     m_transform = 0;
 
     qcms_profile* deviceProfile = ImageDecoder::qcmsOutputDeviceProfile();
     if (!deviceProfile)
         return;
     qcms_profile* inputProfile = qcms_profile_from_memory(data, size);
     if (!inputProfile)
         return;
 
     // We currently only support color profiles for RGB profiled images.
     ASSERT(icSigRgbData == qcms_profile_get_color_space(inputProfile));
     // The input image pixels are RGBA format.
     qcms_data_type format = QCMS_DATA_RGBA_8;
     // FIXME: Don't force perceptual intent if the image profile contains an intent.
     m_transform = qcms_transform_create(inputProfile, format, deviceProfile, QCMS_DATA_RGBA_8, QCMS_INTENT_PERCEPTUAL);
 
     qcms_profile_release(inputProfile);
 }
 
-void WEBPImageDecoder::readColorProfile(const uint8_t* data, size_t size)
+void WEBPImageDecoder::readColorProfile()
 {
     WebPChunkIterator chunkIterator;
-    WebPData inputData = { data, size };
-    WebPDemuxState state;
-
-    WebPDemuxer* demuxer = WebPDemuxPartial(&inputData, &state);
-    if (!WebPDemuxGetChunk(demuxer, "ICCP", 1, &chunkIterator)) {
+    if (!WebPDemuxGetChunk(m_demux, "ICCP", 1, &chunkIterator)) {
         WebPDemuxReleaseChunkIterator(&chunkIterator);
-        WebPDemuxDelete(demuxer);
         return;
     }
 
     const char* profileData = reinterpret_cast<const char*>(chunkIterator.chunk.bytes);
     size_t profileSize = chunkIterator.chunk.size;
 
     // Only accept RGB color profiles from input class devices.
     bool ignoreProfile = false;
     if (profileSize < ImageDecoder::iccColorProfileHeaderLength)
         ignoreProfile = true;
     else if (!ImageDecoder::rgbColorProfile(profileData, profileSize))
         ignoreProfile = true;
     else if (!ImageDecoder::inputDeviceColorProfile(profileData, profileSize))
         ignoreProfile = true;
 
     if (!ignoreProfile)
         createColorTransform(profileData, profileSize);
 
     WebPDemuxReleaseChunkIterator(&chunkIterator);
-    WebPDemuxDelete(demuxer);
 }
 
-void WEBPImageDecoder::applyColorProfile(const uint8_t* data, size_t size, ImageFrame& buffer)
+#endif // QCMS_WEBP_COLOR_CORRECTION
+
+#ifdef WEBP_ICC_ANIMATION_SUPPORT
+void WEBPImageDecoder::applyPostProcessing(size_t frameIndex)
 {
+    ImageFrame& buffer = m_frameBufferCache[frameIndex];
     int width;
     int decodedHeight;
     if (!WebPIDecGetRGB(m_decoder, &decodedHeight, &width, 0, 0))
         return; // See also https://bugs.webkit.org/show_bug.cgi?id=74062
     if (decodedHeight <= 0)
         return;
-
-    if (!m_haveReadProfile) {
-        readColorProfile(data, size);
-        m_haveReadProfile = true;
-    }
-
     ASSERT(width == scaledSize().width());

[Noel Gordon, 2013/05/01 19:11:41]

These ASSERTs are gonna hurt.  The width here is that of the frame?  Can the
frame width be smaller than the image width returned by scaledSize().width()
(the canvas width)?

ASSERT(decodedHeight <= scaledSize().height()) is not tight enough.  The frame
height can be smaller than the canvas height?
 

[urvang (Google), 2013/05/01 22:25:16]

OK, to clear things up:
- The frame width/height *stored* in the WebP file can be smaller than canvas
width/height.
- *But*, a renderer needs to recreate the whole *canvas* corresponding to each
frame (due to disposal methods, etc). That's why the size of each frame is
initialized to be the same as that of the canvas. See line#262. The same is true
for GIF as well.
- The frame sizes are stored in ImageFrames using 'setOriginalFrameRect', and we
decode into that rectangle.

I had thought that this was evident, but it seems not.
Pls suggest where a comment might help.

[Noel Gordon, 2013/05/02 18:00:27]

So the frame width can be smaller than the canvas width.
 
Yes I know how buffer.setSize() works: it creates a canvas-width/height buffer.
So if that frame rectangle width is smaller than the canvas width say, my
question was: what width does WebPIDecGetRGB(m_decoder, &decodedHeight, &width,
0, 0) return?

The answer is either 1) the frame rectangle width, or 2) the canvas width.

If it's 1), the width ASSERT can fail.  If it's 2), I could construct a webp
image with a frame rectangle that would cause the code the write off the end of
the frame buffer into memory you don't own.

     ASSERT(decodedHeight <= scaledSize().height());
+    const int left = buffer.originalFrameRect().x();
+    const int top = buffer.originalFrameRect().y();
 
-    for (int y = m_decodedHeight; y < decodedHeight; ++y) {
-        uint8_t* row = reinterpret_cast<uint8_t*>(buffer.getAddr(0, y));
-        if (qcms_transform* transform = colorTransform())
-            qcms_transform_data_type(transform, row, row, width, QCMS_OUTPUT_RGBX);
-        uint8_t* pixel = row;
-        for (int x = 0; x < width; ++x, pixel += 4)
-            buffer.setRGBA(x, y, pixel[0], pixel[1], pixel[2], pixel[3]);
+#ifdef QCMS_WEBP_COLOR_CORRECTION
+    // Color Profile.

[Noel Gordon, 2013/05/01 19:11:41]

Redundant comment given the include guard right?

[urvang (Google), 2013/05/01 22:25:16]

Maybe, yes. But, the include guard can be removed later, right? I would still
keep the comment to be coherent -- one comment per each main task the method is
doing.

[Noel Gordon, 2013/05/02 18:00:27]

Blink style would encourage coherency as follows.

if ((m_formatFlags & ICCP_FLAG) && !ignoresGammaAndColorProfile())
    applyColorProfile(buffer,...);

The comment doesn't tell me anything that the code does not.
  

+    if ((m_formatFlags & ICCP_FLAG) && !ignoresGammaAndColorProfile()) {
+        if (!m_haveReadProfile) {
+            readColorProfile();
+            m_haveReadProfile = true;
+        }
+        for (int y = m_decodedHeight; y < decodedHeight; ++y) {
+            const int canvasY = top + y;
+            uint8_t* row = reinterpret_cast<uint8_t*>(buffer.getAddr(left, canvasY));
+            if (qcms_transform* transform = colorTransform())
+                qcms_transform_data_type(transform, row, row, width, QCMS_OUTPUT_RGBX);
+            uint8_t* pixel = row;
+            for (int x = 0; x < width; ++x, pixel += 4) {
+                const int canvasX = left + x;
+                buffer.setRGBA(canvasX, canvasY, pixel[0], pixel[1], pixel[2], pixel[3]);
+            }
+        }
+    }
+#endif // QCMS_WEBP_COLOR_CORRECTION
+
+    // Frame disposal:
+    // During the decoding of current frame, we may have set some pixels to be transparent (i.e. alpha < 255).
+    // However, the value of each of these pixels should have been determined by blending it against the value
+    // of that pixel in the previous frame. So, we correct these pixels based on disposal method of the previous
+    // frame and the previous frame buffer.
+    if ((m_formatFlags & ANIMATION_FLAG) && frameIndex) {
+        ImageFrame& prevBuffer = m_frameBufferCache[frameIndex - 1];
+        ImageFrame::FrameDisposalMethod prevMethod = prevBuffer.disposalMethod();
+        ASSERT(prevBuffer.status() == ImageFrame::FrameComplete);
+        if (prevMethod == ImageFrame::DisposeKeep) { // Restore transparent pixels to pixels in previous canvas.
+            for (int y = m_decodedHeight; y < decodedHeight; ++y) {
+                const int canvasY = top + y;
+                for (int x = 0; x < width; ++x) {
+                    const int canvasX = left + x;
+                    ImageFrame::PixelData& pixel = *buffer.getAddr(canvasX, canvasY);
+                    // FIXME: Use alpha-blending when alpha is between 0 and 255.
+                    // Alpha-blending is being implemented in: https://bugs.webkit.org/show_bug.cgi?id=17022
+                    if (!((pixel >> 24) & 0xff)) { // Need to restore.

[Noel Gordon, 2013/05/01 19:11:41]

Unless you want to break Android, write this as

  unsigned alpha = (pixel >> SK_A32_SHIFT) & 0xff;
  if (!alpha) { // Need to restore.
     ...

SK_A32_SHIFT on Android differs from the value used for desktop Chrome.

[urvang (Google), 2013/05/01 22:25:16]

Done.

+                        const ImageFrame::PixelData prevPixel = *prevBuffer.getAddr(canvasX, canvasY);

[Noel Gordon, 2013/05/01 19:11:41]

s/const// 

[urvang (Google), 2013/05/01 22:25:16]

I thought a const would be desirable?
Anyway, removing this assuming it has something to do with Webkit style.

+                        pixel = prevPixel;
+                    }
+                }
+            }
+        } else if (prevMethod == ImageFrame::DisposeOverwriteBgcolor) {
+            const IntRect& prevRect = prevBuffer.originalFrameRect();
+            // We need to restore transparent pixels to as they were just after initFrame() call. That is:
+            //   * Transparent if it belongs to prevRect <-- This is a no-op.
+            //   * Pixel in the previous canvas otherwise <-- Need to restore.
+            for (int y = m_decodedHeight; y < decodedHeight; ++y) {
+                const int canvasY = top + y;
+                for (int x = 0; x < width; ++x) {
+                    const int canvasX = left + x;
+                    ImageFrame::PixelData& pixel = *buffer.getAddr(canvasX, canvasY);
+                    const ImageFrame::PixelData prevPixel = *prevBuffer.getAddr(canvasX, canvasY);

[Noel Gordon, 2013/05/01 19:11:41]

Should prevPixel be defined inside the if clause where it is used similar to
line 434 ? 

s/const//

[urvang (Google), 2013/05/01 22:25:16]

Done.

+                    // FIXME: Use alpha-blending when alpha is between 0 and 255.
+                    if (!((pixel >> 24) & 0xff) && !prevRect.contains(IntPoint(canvasX, canvasY))) // Need to restore.

[Noel Gordon, 2013/05/01 19:11:41]

Again, don't break the droids.

  unsigned alpha = (pixel >> SK_A32_SHIFT) & 0xff;
  if (!alpha && !prevRect.contains( ...

[urvang (Google), 2013/05/01 22:25:16]

Done.

+                        pixel = prevPixel;
+                }
+            }
+        }
     }
 
     m_decodedHeight = decodedHeight;
 }
+#endif // WEBP_ICC_ANIMATION_SUPPORT
 
-#endif // QCMS_WEBP_COLOR_CORRECTION
-
-bool WEBPImageDecoder::decode(bool onlySize)
+bool WEBPImageDecoder::decode(const uint8_t* dataBytes, size_t dataSize, bool onlySize, size_t frameIndex)
 {
     if (failed())
         return false;
 
-    const uint8_t* dataBytes = reinterpret_cast<const uint8_t*>(m_data->data());
-    const size_t dataSize = m_data->size();
-
     if (!ImageDecoder::isSizeAvailable()) {
         static const size_t imageHeaderSize = 30;
         if (dataSize < imageHeaderSize)
             return false;
         int width, height;
-#ifdef QCMS_WEBP_COLOR_CORRECTION
-        WebPData inputData = { dataBytes, dataSize };
-        WebPDemuxState state;
-        WebPDemuxer* demuxer = WebPDemuxPartial(&inputData, &state);
-        if (!demuxer)
-            return setFailed();
-
-        width = WebPDemuxGetI(demuxer, WEBP_FF_CANVAS_WIDTH);
-        height = WebPDemuxGetI(demuxer, WEBP_FF_CANVAS_HEIGHT);
-        m_formatFlags = WebPDemuxGetI(demuxer, WEBP_FF_FORMAT_FLAGS);
-        m_hasAlpha = !!(m_formatFlags & ALPHA_FLAG);
-
-        WebPDemuxDelete(demuxer);
-        if (state <= WEBP_DEMUX_PARSING_HEADER)
-            return false;
-#elif (WEBP_DECODER_ABI_VERSION >= 0x0163)
+#if (WEBP_DECODER_ABI_VERSION >= 0x0163)
         WebPBitstreamFeatures features;
         if (WebPGetFeatures(dataBytes, dataSize, &features) != VP8_STATUS_OK)
             return setFailed();
         width = features.width;
         height = features.height;
-        m_hasAlpha = features.has_alpha;
+        m_formatFlags = features.has_alpha ? ALPHA_FLAG : 0;
 #else
         // Earlier version won't be able to display WebP files with alpha.
         if (!WebPGetInfo(dataBytes, dataSize, &width, &height))
             return setFailed();
-        m_hasAlpha = false;
 #endif
         if (!setSize(width, height))
             return setFailed();
     }
 
     ASSERT(ImageDecoder::isSizeAvailable());
     if (onlySize)
         return true;
 
-    ASSERT(!m_frameBufferCache.isEmpty());
-    ImageFrame& buffer = m_frameBufferCache[0];
+    ASSERT(m_frameBufferCache.size() > frameIndex);
+    ImageFrame& buffer = m_frameBufferCache[frameIndex];
     ASSERT(buffer.status() != ImageFrame::FrameComplete);
 
     if (buffer.status() == ImageFrame::FrameEmpty) {
-        if (!buffer.setSize(size().width(), size().height()))
+        if (!buffer.setSize(scaledSize().width(), scaledSize().height()))
             return setFailed();
         buffer.setStatus(ImageFrame::FramePartial);
-        buffer.setHasAlpha(m_hasAlpha);
+        buffer.setHasAlpha(m_formatFlags & ALPHA_FLAG);
         buffer.setOriginalFrameRect(IntRect(IntPoint(), size()));
     }
 
+    const IntRect& frameRect = buffer.originalFrameRect();
     if (!m_decoder) {
-        WEBP_CSP_MODE mode = outputMode(m_hasAlpha);
+        WEBP_CSP_MODE mode = outputMode(m_formatFlags & ALPHA_FLAG);
         if (!m_premultiplyAlpha)
             mode = outputMode(false);
+#ifdef QCMS_WEBP_COLOR_CORRECTION
         if ((m_formatFlags & ICCP_FLAG) && !ignoresGammaAndColorProfile())
             mode = MODE_RGBA; // Decode to RGBA for input to libqcms.
+#endif
+        WebPInitDecBuffer(&m_decoderBuffer);
         m_decoderBuffer.colorspace = mode;
         m_decoderBuffer.u.RGBA.stride = size().width() * sizeof(ImageFrame::PixelData);
-        m_decoderBuffer.u.RGBA.size = m_decoderBuffer.u.RGBA.stride * size().height();
+        m_decoderBuffer.u.RGBA.size = m_decoderBuffer.u.RGBA.stride * frameRect.height();
         m_decoderBuffer.is_external_memory = 1;
         m_decoder = WebPINewDecoder(&m_decoderBuffer);
         if (!m_decoder)
             return setFailed();
     }
 
-    m_decoderBuffer.u.RGBA.rgba = reinterpret_cast<uint8_t*>(buffer.getAddr(0, 0));
+    m_decoderBuffer.u.RGBA.rgba = reinterpret_cast<uint8_t*>(buffer.getAddr(frameRect.x(), frameRect.y()));
 
     switch (WebPIUpdate(m_decoder, dataBytes, dataSize)) {
     case VP8_STATUS_OK:
-        if ((m_formatFlags & ICCP_FLAG) && !ignoresGammaAndColorProfile()) 
-            applyColorProfile(dataBytes, dataSize, buffer);
+        applyPostProcessing(frameIndex);
         buffer.setStatus(ImageFrame::FrameComplete);
-        clear();
+        clearDecoder();
         return true;
     case VP8_STATUS_SUSPENDED:
-        if ((m_formatFlags & ICCP_FLAG) && !ignoresGammaAndColorProfile()) 
-            applyColorProfile(dataBytes, dataSize, buffer);
+        applyPostProcessing(frameIndex);
         return false;
     default:
-        clear();                         
+        clear();
         return setFailed();
     }
 }
 
 } // namespace WebCore