Animated WebP: Optimize decoding in case of seeking

Source/core/platform/image-decoders/ImageDecoder.h

 /*
  * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
  * Copyright (C) Research In Motion Limited 2009-2010. 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 46 matching lines @@
         enum FrameDisposalMethod {
             // If you change the numeric values of these, make sure you audit
             // all users, as some users may cast raw values to/from these
             // constants.
             DisposeNotSpecified,      // Leave frame in framebuffer
             DisposeKeep,              // Leave frame in framebuffer
             DisposeOverwriteBgcolor,  // Clear frame to transparent
             DisposeOverwritePrevious  // Clear frame to previous framebuffer
                                       // contents
         };
+        // Indicates how transparent pixels of the current frame rectangle are
+        // blended with those of the previous frame buffer.
+        // Notes:
+        // * GIF always uses 'BlendAtopPreviousFrame'.
+        // * WebP also gives 'BlendAtopBgcolor' option. This is useful for cases

[Peter Kasting, 2013/08/22 20:04:36]

Nit: So you never stated clearly in the private email thread whether WebP has an
actual bgcolor concept, or whether "clear to bgcolor" is always equivalent to
"clear to #00000000" or something.  If it's the latter, then you were probably
right that using "bgcolor" here can be misleading, and we should do something
like "Overwrite" or "DoNotBlend" instead.

[urvang (Google), 2013/08/22 23:25:14]

Short answer: WebP does have a background color concept:
https://gerrit.chromium.org/gerrit/gitweb?p=webm/libwebp.git;a=blob;f=doc/web...

Long answer: As noted there, renders like Chrome are allowed to ignore it and
use their own background color, which is 0x00000000.
GIF has a similar background color field (see the "background color index" here:
http://www.w3.org/Graphics/GIF/spec-gif89a.txt) and it is ignored by Chrome as
well.

Given this, which name sounds better?

[Peter Kasting, 2013/08/22 23:39:31]

Uggghhhh, I can't believe you guys put in something whose interpretation is
optional like this.  It should have either been mandated or dropped entirely. 
Its current state just leads to confusion; who is actually going to respect this
field?  What is the encoder going to assume the decode process will do?

I guess in this case you could say "BlendAtopBgcolor" and document it as "by
blending against transparent, i.e. simply overwriting the corresponding pixels"
or some such.  ("Blending against transparent" is chosen because you stuck with
AlphaBlendSource and for parallel structure with the comment above on
DisposeOverwriteBgcolor.)

[urvang (Google), 2013/08/23 00:13:35]

I completely understand your point. We would have liked to make it fixed either
way as well. But then the scenario is exactly same for GIF as well (even though
the spec doesn't mention it): the browsers always chose transparent background,
some image viewers choose transparent backgrounds, while a few viewers show the
background  color GIF specified. And we wanted to cover that particular use case
of GIF in WebP.
Updated comment.

[Peter Kasting, 2013/08/23 00:17:55]

"Behave inconsistently" is a bug, not a use case.  There's no use case for
people to make images intentionally behave differently on different viewers. 
And certainly anyone transcoding GIF to WebP can afford for WebP to lock this
sort of inconsistency into one behavior or the other.  After all, you didn't
preserve other parts of GIF people don't care about like the pixel aspect ratio
field or GIF Text Mode, did you?

Just nuke it from the WebP spec.  If the spec is locked, then document it as
deprecated, say that all viewers MUST display transparent, and note that its
value will never be set.  Seriously, I'm not kidding.  This is a really bad
thing to have put in the spec.

[urvang (Google), 2013/08/23 00:21:32]

Point taken.

The spec is, in fact, locked in terms of 'ANIM' chunk (which 'bgcolor' is a part
of). So, I'll discuss with team and possibly mark it as deprecated.

Thanks for the suggestion.

+        //   where one wants to transform a few opaque pixels of previous frame
+        //   into non-opaque pixels in the current frame.
+        enum AlphaBlendSourceForSubframe {

[Peter Kasting, 2013/08/22 20:04:36]

Nit: Since I was mistaken about thinking this was only necessary for cases where
you had a smaller subframe, and since the enums above both start "Frame", maybe
this should be "FrameCompositionMethod", or  "FrameCompositionSource", or
"FrameAlphaBlendSource".

Bonus points for optionally nuking the word "Frame" out of all three enums,
since they're already in "ImageFrame" and thus "Frame" is implied.

[urvang (Google), 2013/08/22 23:25:14]

Using the name 'AlphaBlendSource'.
Done.

+            BlendAtopPreviousFrame, // Compose the rectangle of current frame by blending it with corresponding pixels in the previous (disposed) framebuffer.

[Peter Kasting, 2013/08/22 20:04:36]

Nit: Put these comments above the enum values (and then wrap to 80 chars since
the rest of the comments here do so):

    enum X {
        // Blah blah
        X1,

        // Note blank line above for easier scanning
        X2,
    }

Also, maybe the comments should say "...with the corresponding pixels in the
initial buffer state (i.e. any previous frame buffer after having been properly
disposed)." or something (the current comment is a little confusing).

[urvang (Google), 2013/08/22 23:25:14]

Done.

+            BlendAtopBgcolor, // Compose the rectangle of current frame by over-writing the corresponding pixels in the previous (disposed) framebuffer.
+        };
         typedef uint32_t PixelData;
 
         ImageFrame();
 
         ImageFrame(const ImageFrame& other) { operator=(other); }
 
         // For backends which refcount their data, this operator doesn't need to
         // create a new copy of the image data, only increase the ref count.
         ImageFrame& operator=(const ImageFrame& other);
 
@@ skipping 30 matching lines @@
         // Returns a caller-owned pointer to the underlying native image data.
         // (Actual use: This pointer will be owned by BitmapImage and freed in
         // FrameData::clear()).
         PassRefPtr<NativeImageSkia> asNewNativeImage() const;
 
         bool hasAlpha() const;
         const IntRect& originalFrameRect() const { return m_originalFrameRect; }
         FrameStatus status() const { return m_status; }
         unsigned duration() const { return m_duration; }
         FrameDisposalMethod disposalMethod() const { return m_disposalMethod; }
+        AlphaBlendSourceForSubframe alphaBlendSource() const { return m_alphaBlendSource; }
         bool premultiplyAlpha() const { return m_premultiplyAlpha; }
         SkBitmap::Allocator* allocator() const { return m_allocator; }
         const SkBitmap& getSkBitmap() const { return m_bitmap->bitmap(); }
 
         size_t requiredPreviousFrameIndex() const
         {
             ASSERT(m_requiredPreviousFrameIndexValid);
             return m_requiredPreviousFrameIndex;
         }
 #if !ASSERT_DISABLED
         bool requiredPreviousFrameIndexValid() const { return m_requiredPreviousFrameIndexValid; }
 #endif
         void setHasAlpha(bool alpha);
         void setOriginalFrameRect(const IntRect& r) { m_originalFrameRect = r; }
         void setStatus(FrameStatus status);
         void setDuration(unsigned duration) { m_duration = duration; }
         void setDisposalMethod(FrameDisposalMethod method) { m_disposalMethod = method; }
+        void setAlphaBlendSource(AlphaBlendSourceForSubframe source) { m_alphaBlendSource = source; }
         void setPremultiplyAlpha(bool premultiplyAlpha) { m_premultiplyAlpha = premultiplyAlpha; }
         void setMemoryAllocator(SkBitmap::Allocator* allocator) { m_allocator = allocator; }
         void setSkBitmap(const SkBitmap& bitmap) { m_bitmap = NativeImageSkia::create(bitmap); }
 
         void setRequiredPreviousFrameIndex(size_t previousFrameIndex)
         {
             m_requiredPreviousFrameIndex = previousFrameIndex;
 #if !ASSERT_DISABLED
             m_requiredPreviousFrameIndexValid = true;
 #endif
@@ skipping 48 matching lines @@
 
         RefPtr<NativeImageSkia> m_bitmap;
         SkBitmap::Allocator* m_allocator;
         bool m_hasAlpha;
         // This will always just be the entire buffer except for GIF or WebP
         // frames whose original rect was smaller than the overall image size.
         IntRect m_originalFrameRect;
         FrameStatus m_status;
         unsigned m_duration;
         FrameDisposalMethod m_disposalMethod;
+        AlphaBlendSourceForSubframe m_alphaBlendSource;
         bool m_premultiplyAlpha;
 
         // The frame that must be decoded before this frame can be decoded.
         // WTF::notFound if this frame doesn't require any previous frame.
         // This is used by ImageDecoder::clearCacheExceptFrame(), and will never
         // be read for image formats that do not have multiple frames.
         size_t m_requiredPreviousFrameIndex;
 #if !ASSERT_DISABLED
         bool m_requiredPreviousFrameIndexValid;
 #endif
@@ skipping 164 matching lines @@
         // If the image has a cursor hot-spot, stores it in the argument
         // and returns true. Otherwise returns false.
         virtual bool hotSpot(IntPoint&) const { return false; }
 
         virtual void setMemoryAllocator(SkBitmap::Allocator* allocator)
         {
             // FIXME: this doesn't work for images with multiple frames.
             if (m_frameBufferCache.isEmpty()) {
                 m_frameBufferCache.resize(1);
                 m_frameBufferCache[0].setRequiredPreviousFrameIndex(
-                    findRequiredPreviousFrame(0));
+                    findRequiredPreviousFrame(0, false));
             }
             m_frameBufferCache[0].setMemoryAllocator(allocator);
         }
 
     protected:
         // Calculates the most recent frame whose image data may be needed in
-        // order to decode frame |frameIndex|, based on frame disposal methods.
+        // order to decode frame |frameIndex|, based on frame disposal methods
+        // and 'frameRectIsOpaque', where 'frameRectIsOpaque' signifies whether

[Peter Kasting, 2013/08/22 20:04:36]

Nit: Use || instead of '' to denote variable names

[urvang (Google), 2013/08/22 23:25:14]

Done.

+        // the rectangle of frame at 'frameIndex' is known to be opaque.
         // If no previous frame's data is required, returns WTF::notFound.
         //
         // This function requires that the previous frame's
         // |m_requiredPreviousFrameIndex| member has been set correctly. The
         // easiest way to ensure this is for subclasses to call this method and
         // store the result on the frame via setRequiredPreviousFrameIndex()
         // as soon as the frame has been created and parsed sufficiently to
         // determine the disposal method; assuming this happens for all frames
         // in order, the required invariant will hold.
         //
         // Image formats which do not use more than one frame do not need to
         // worry about this; see comments on
         // ImageFrame::m_requiredPreviousFrameIndex.
-        size_t findRequiredPreviousFrame(size_t frameIndex);
+        size_t findRequiredPreviousFrame(size_t frameIndex, bool frameRectIsOpaque);
 
         virtual void clearFrameBuffer(size_t frameIndex);
 
         RefPtr<SharedBuffer> m_data; // The encoded data.
         Vector<ImageFrame, 1> m_frameBufferCache;
         bool m_premultiplyAlpha;
         bool m_ignoreGammaAndColorProfile;
         ImageOrientation m_orientation;
 
     private:
         // Some code paths compute the size of the image as "width * height * 4"
         // and return it as a (signed) int.  Avoid overflow.
         static bool isOverSize(unsigned width, unsigned height)
         {
             unsigned long long total_size = static_cast<unsigned long long>(width)
                                           * static_cast<unsigned long long>(height);
             return total_size > ((1 << 29) - 1);
         }
 
         IntSize m_size;
         bool m_sizeAvailable;
         bool m_isAllDataReceived;
         bool m_failed;
     };
 
 } // namespace WebCore
 
 #endif