GIF decoding to Index8, unit tests and misusing unit test as benchmark

third_party/WebKit/Source/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 150 matching lines @@
 
     // Calls decodeFrameCount() to get the frame count (if possible), without
     // decoding the individual frames.  Resizes m_frameBufferCache to the
     // correct size and returns its size.
     size_t frameCount();
 
     virtual int repetitionCount() const { return cAnimationNone; }
 
     // Decodes as much of the requested frame as possible, and returns an
     // ImageDecoder-owned pointer.
-    ImageFrame* frameBufferAtIndex(size_t);
+    ImageFrame* frameBufferAtIndex(size_t, ImageFrame::ColorType = ImageFrame::N32);

[scroggo_chromium, 2015/12/03 21:47:21]

It seems odd to me that the client asks for a particular color type. Does that
mean this might do a copy? (i.e. what happens if it already decoded that frame
to a different type?) Why doesn't the ImageDecoder just return the frame that it
has?

[aleksandar.stojiljkovic, 2015/12/04 00:07:34]

1st Reason is in discussion we had here:
https://codereview.chromium.org/1495693003/#msg8

Main reason though, once 1st reason is fixed, is for testing - wanted to keep
rgb decode having previous functionality and 1) keep it working and 2)compare
rgba-index8 in tests.

There is no copy, as you pointed out - the use case with frame decoded to one
color type and then asked in another is not existing, even in tests. I just
didn't add checks / asserts there. test use one decoder for all frames to rgba
or to "index9 + unsupported index8 frames in rgba)

Splitting to test method would help and some of the decode to this / can decode
to that would go away after changing skia API.

 
     // Whether the requested frame has alpha.
     virtual bool frameHasAlphaAtIndex(size_t) const;
 
     // Whether or not the frame is fully received.
     virtual bool frameIsCompleteAtIndex(size_t) const;
 
     // Duration for displaying a frame in seconds. This method is only used by
     // animated images.
     virtual float frameDurationAtIndex(size_t) const { return 0; }
@@ skipping 103 matching lines @@
             m_frameBufferCache.resize(1);
             m_frameBufferCache[0].setRequiredPreviousFrameIndex(
                 findRequiredPreviousFrame(0, false));
         }
         m_frameBufferCache[0].setMemoryAllocator(allocator);
     }
 
     virtual bool canDecodeToYUV() { return false; }
     virtual bool decodeToYUV() { return false; }
     virtual void setImagePlanes(PassOwnPtr<ImagePlanes>) { }
+    virtual bool canDecodeTo(size_t index, ImageFrame::ColorType outputType)
+    {
+        return outputType == ImageFrame::N32;
+    }
 
 protected:
     // Calculates the most recent frame whose image data may be needed in
     // order to decode frame |frameIndex|, based on frame disposal methods
     // and |frameRectIsOpaque|, where |frameRectIsOpaque| signifies whether
     // the rectangle of frame at |frameIndex| is known to be opaque.
     // If no previous frame's data is required, returns WTF::kNotFound.
     //
     // This function requires that the previous frame's
     // |m_requiredPreviousFrameIndex| member has been set correctly. The
@@ skipping 16 matching lines @@
     // Decodes the image sufficiently to determine the number of frames and
     // returns that number.
     virtual size_t decodeFrameCount() { return 1; }
 
     // Performs any additional setup of the requested frame after it has been
     // initially created, e.g. setting a duration or disposal method.
     virtual void initializeNewFrame(size_t) { }
 
     // Decodes the requested frame.
     virtual void decode(size_t) = 0;
+    // Decodes the requested frame to specified color type output.
+    virtual void decodeTo(size_t index, ImageFrame::ColorType outputColor)

[scroggo_chromium, 2015/12/03 21:47:21]

If we need to pass outputColor, why not merge this with decode?

[aleksandar.stojiljkovic, 2015/12/04 00:07:34]

Didn't want to change code in other decoders, that is why it is just wrapping
them for N32 (currently only supported output).

+    {
+        if (!canDecodeTo(index, outputColor))

[scroggo_chromium, 2015/12/03 21:47:21]

So the subclass can override decodeTo and not call canDecodeTo?

[aleksandar.stojiljkovic, 2015/12/04 00:07:34]

This would keep the existing decoders unchanged, just change it for gif decoder.

+            return;

[scroggo_chromium, 2015/12/03 21:47:21]

So this will silently fail?

[aleksandar.stojiljkovic, 2015/12/04 00:07:34]

If there is no frame in cache, ImageFrameGenerator::decode calling this would
return false since the frame is empty.
If there is frame in cache of wrong type it should release assert if frame is
already cached and of wrong type - and not here but in
ImageDecoder::frameBufferAtIndex since it is from there calling this only if
frame is not complete.

+        // this is the default implementation that is just wrapping decode().
+        // subclasses need to override this and canDecodeTo for supported colorTypes.
+        decode(index);
+    }
 
     RefPtr<SharedBuffer> m_data; // The encoded data.
     Vector<ImageFrame, 1> m_frameBufferCache;
     bool m_premultiplyAlpha;
     bool m_ignoreGammaAndColorProfile;
     ImageOrientation m_orientation;
 
     // The maximum amount of memory a decoded image should require. Ideally,
     // image decoders should downsample large images to fit under this limit
     // (and then return the downsampled size from decodedSize()). Ignoring
@@ skipping 13 matching lines @@
 
     IntSize m_size;
     bool m_sizeAvailable;
     bool m_isAllDataReceived;
     bool m_failed;
 };
 
 } // namespace blink
 
 #endif