Create an SkAndroidCodec API separate from SkCodec

include/codec/SkAndroidCodec.h

+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkAndroidCodec_DEFINED
+#define SkAndroidCodec_DEFINED
+
+#include "SkCodec.h"
+#include "SkEncodedFormat.h"
+#include "SkStream.h"
+#include "SkTypes.h"
+
+/**
+ *  Abstract interface defining image codec functionality that is necessary for
+ *  Android.
+ */
+class SkAndroidCodec : SkNoncopyable {
+public:
+    /**
+     *  If this stream represents an encoded image that we know how to decode,
+     *  return an SkAndroidCodec that can decode it. Otherwise return NULL.
+     *
+     *  If NULL is returned, the stream is deleted immediately. Otherwise, the
+     *  SkCodec takes ownership of it, and will delete it when done with it.
+     */
+    static SkAndroidCodec* NewFromStream(SkStream*);
+
+    /**
+     *  If this data represents an encoded image that we know how to decode,
+     *  return an SkAndroidCodec that can decode it. Otherwise return NULL.
+     *
+     *  Will take a ref if it returns a codec, else will not affect the data.
+     */
+    static SkAndroidCodec* NewFromData(SkData*);
+
+    virtual ~SkAndroidCodec() {}
+
+
+    const SkImageInfo& getInfo() const { return fInfo; }
+
+    /**
+     *  Format of the encoded data.
+     */
+    SkEncodedFormat getEncodedFormat() const { return this->onGetEncodedFormat(); }
+
+    /**
+     *  Returns the dimensions of the scaled output image, for an input
+     *  sampleSize.
+     *
+     *  When the sample size divides evenly into the original dimensions, the
+     *  scaled output dimensions will simply be equal to the original
+     *  dimensions divided by the sample size.
+     *
+     *  When the sample size does not divide even into the original
+     *  dimensions, the codec may round up or down, depending on what is most
+     *  efficient to decode.
+     *
+     *  Finally, the codec will always recommend a non-zero output, so the output
+     *  dimension will always be one if the sampleSize is greater than the
+     *  original dimension.
+     */
+    SkISize getSampledDimensions(int sampleSize) const;
+
+    /**
+     *  Return (via desiredSubset) a subset which can decoded from this codec,
+     *  or false if the input subset is invalid.
+     *
+     *  @param desiredSubset in/out parameter
+     *                       As input, a desired subset of the original bounds
+     *                       (as specified by getInfo).
+     *                       As output, if true is returned, desiredSubset may
+     *                       have been modified to a subset which is
+     *                       supported. Although a particular change may have
+     *                       been made to desiredSubset to create something
+     *                       supported, it is possible other changes could
+     *                       result in a valid subset.  If false is returned,
+     *                       desiredSubset's value is undefined.
+     *  @return true         If the input desiredSubset is valid.
+     *                       desiredSubset may be modified to a subset
+     *                       supported by the codec.
+     *          false        If desiredSubset is invalid (NULL or not fully
+     *                       contained within the image).
+     */
+    bool getSubset(SkIRect* desiredSubset) const;

[msarett, 2015/10/16 18:42:16]

I could have (and maybe should have) called this getValidSubset().

In my mind, "invalid subsets" are the ones that aren't contained by the image. 
I didn't want to make it sound like this function would correct invalid subsets
for the client.

[scroggo, 2015/10/16 21:13:55]

I think it's odd that this one is different from the one on SkCodec. Are you
saying that it is named differently but it means the same thing?

Maybe we should come up with a better name for both of them. How about
"getSupportedSubset"?

[msarett, 2015/10/19 16:06:10]

Yes they are the same.

I like getSupportedSubset().  I'll add a TODO for SkCodec.h.

+
+    /**
+     *  Returns the dimensions of the scaled, partial output image, for an
+     *  input sampleSize and subset.
+     *
+     *  @param sampleSize Factor to scale down by.
+     *  @param subset     Must be a valid subset of the original image
+     *                    dimensions and a subset supported by SkAndroidCodec.
+     *                    getSubset() can be used to obtain a subset supported
+     *                    by SkAndroidCodec.
+     *  @return           Size of the scaled partial image.  Or zero size

[scroggo, 2015/10/16 21:13:55]

Alternatively, we could return a boolean, and modify a passed in SkISize.

[msarett, 2015/10/19 16:06:10]

Agreed.

I went for this approach for consistency with getScaledDimensions() and
getSampledDimensions().

I think returning a bool is probably a cleaner way to indicate an error.  I
would be in favor of a separate change to all of these APIs, but don't feel too
strongly.

+     *                    if either of the inputs is invalid.
+     */
+    SkISize getSampledSubsetDimensions(int sampleSize, const SkIRect& subset) const;
+
+    /**
+     *  Additional options to pass to getAndroidPixels().
+     */
+    struct AndroidOptions {
+        AndroidOptions()
+            : fZeroInitialized(SkCodec::kNo_ZeroInitialized)
+            , fSubset(nullptr)
+            , fColorPtr(nullptr)

[msarett, 2015/10/16 18:42:16]

At one point, you mentioned that we might want to move colorPtr and
colorCountPtr into Options in SkCodec.  I thought we could go ahead and see how
it looks in SkAndroidCodec.

+            , fColorCount(nullptr)
+            , fSampleSize(1)
+        {}
+
+        /**
+         *  Indicates is destination pixel memory is zero initialized.
+         */
+        SkCodec::ZeroInitialized fZeroInitialized;
+
+        /**
+         *  If not NULL, represents a subset of the original image to decode.
+         *
+         *  Must be within the bounds returned by getInfo().
+         *
+         *  If the EncodedFormat is kWEBP_SkEncodedFormat, the top and left
+         *  values must be even.
+         */
+        SkIRect* fSubset;
+
+        /**
+         *  If the client has requested a decode to kIndex8_SkColorType
+         *  (specified in the SkImageInfo), then the caller must provide
+         *  storage for up to 256 SkPMColor values in fColorPtr.  On success,
+         *  the codec must copy N colors into that storage, (where N is the
+         *  logical number of table entries) and set fColorCount to N.
+         *
+         *  If the client does not request kIndex8_SkColorType, then the last
+         *  two parameters may be NULL. If fColorCount is not null, it will be
+         *  set to 0.
+         */
+        SkPMColor* fColorPtr;
+        int*       fColorCount;
+
+        /**
+         *  The client may provide an integer downscale factor for the decode.
+         *  The codec may implement this downscaling by sampling or another
+         *  method if it is more efficient.
+         */
+        int fSampleSize;
+    };
+
+    /**
+     *  Decode into the given pixels, a block of memory of size at
+     *  least (info.fHeight - 1) * rowBytes + (info.fWidth *
+     *  bytesPerPixel)
+     *
+     *  Repeated calls to this function should give the same results,
+     *  allowing the PixelRef to be immutable.
+     *
+     *  @param info A description of the format (config, size)
+     *         expected by the caller.  This can simply be identical
+     *         to the info returned by getInfo().
+     *
+     *         This contract also allows the caller to specify
+     *         different output-configs, which the implementation can
+     *         decide to support or not.
+     *
+     *         A size that does not match getInfo() implies a request
+     *         to scale or subset. If the codec cannot perform this
+     *         scaling or subsetting, it will return an error code.
+     *
+     *  If info is kIndex8_SkColorType, then the caller must provide storage for up to 256
+     *  SkPMColor values in options->fColorPtr. On success the codec must copy N colors into
+     *  that storage, (where N is the logical number of table entries) and set
+     *  options->fColorCount to N.
+     *
+     *  If info is not kIndex8_SkColorType, options->fColorPtr and options->fColorCount may
+     *  be nullptr.
+     *
+     *  The AndroidOptions object is also used to specify any requested scaling or subsetting
+     *  using options->fSampleSize and options->fSubset.
+     *
+     *  @return Result kSuccess, or another value explaining the type of failure.
+     */
+    SkCodec::Result getAndroidPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,

[scroggo, 2015/10/16 21:13:55]

I don't think it's necessary to put "Android" in these names. (It seems a little
redundant to call SkAndroidCodec::getAndroidPixels (with an AndroidOptions, no
less!))

[msarett, 2015/10/19 16:06:10]

Yeah you're right.  I was struggling to come up with unique names from SkCodec. 
I'll add FIXMEs.

+            AndroidOptions* options);
+
+    /**
+     *  Simplified version of getAndroidPixels() where we supply the default AndroidOptions.
+     *
+     *  This will return an error if the info is kIndex_8_SkColorType and also will not perform
+     *  any scaling or subsetting.
+     */
+    SkCodec::Result getAndroidPixels(const SkImageInfo& info, void* pixels, size_t rowBytes);
+
+protected:
+
+    SkAndroidCodec(const SkImageInfo&);
+
+    virtual SkEncodedFormat onGetEncodedFormat() const = 0;
+
+    virtual SkISize onGetSampledDimensions(int sampleSize) const = 0;
+
+    virtual bool onGetSubset(SkIRect* desiredSubset) const = 0;
+
+    virtual SkISize onGetSampledSubsetDimensions(int sampleSize, const SkIRect& subset) const = 0;
+
+    virtual SkCodec::Result onGetAndroidPixels(const SkImageInfo& info, void* pixels,
+            size_t rowBytes, AndroidOptions& options) = 0;
+
+private:
+
+    const SkImageInfo& fInfo;

[scroggo, 2015/10/16 21:13:55]

Maybe a note that this will always be the info of the contained SkCodec? It
actually occurred to me independently (I glossed over it here, and thought about
it later) that this could be a const reference, but it's not something I would
typically see. I thought maybe it's relying on the implementation of SkCodec,
but given that SkCodec::getInfo returns a const reference, maybe this is fine
(just cannot change SkCodec's return value!_

[msarett, 2015/10/19 16:06:10]

Will add a note.  It may be a bit dangerous to depend on SkCodec in this way.

+};
+#endif // SkAndroidCodec_DEFINED