Create a scanline decoder without creating a codec

include/codec/SkScanlineDecoder.h

Index: include/codec/SkScanlineDecoder.h
diff --git a/include/codec/SkScanlineDecoder.h b/include/codec/SkScanlineDecoder.h
index 8376e57c09d84bb48c1d7ae295ccd441a7feab48..436fc70907ad565914e0a11d2aef7d4c3ffa6622 100644
--- a/include/codec/SkScanlineDecoder.h
+++ b/include/codec/SkScanlineDecoder.h
@@ -16,6 +16,27 @@
 class SkScanlineDecoder : public SkNoncopyable {
 public:
     /**
+     *  If this stream represents an encoded image that we know how to decode
+     *  in scanlines, return an SkScanlineDecoder that can decode it. Otherwise
+     *  return NULL.
+     *
+     *  reset must be called in order to decode any scanlines.
+     *
+     *  If NULL is returned, the stream is deleted immediately. Otherwise, the
+     *  SkScanlineDecoder takes ownership of it, and will delete it when done
+     *  with it.
+     */
+    static SkScanlineDecoder* NewFromStream(SkStream*);
+
+    /**
+     *  Similar to NewFromStream, but reads from an SkData.
+     *
+     *  Will take a ref if it returns a scanline decoder, else will not affect
+     *  the data.
+     */
+    static SkScanlineDecoder* NewFromData(SkData*);
+
+    /**
      *  Clean up after reading/skipping scanlines.
      *
      *  It is possible that not all scanlines will have been read/skipped.  In
@@ -25,6 +46,40 @@ public:
     virtual ~SkScanlineDecoder() {}
 
     /**
+     *  Returns the default info, corresponding to the encoded data.
+     *
+     *  If reset has never been called, this is the info that will be used.
+     */
+    const SkImageInfo& getInfo() { return fSrcInfo; }
+
+    /**
+     *  Reset to the first scanline, with the specified options.
+     *
+     *  This may require rewinding the stream.
+     *
+     *  @param dstInfo Info of the destination. If the dimensions do not match
+     *      those of getInfo, this implies a scale.
+     *  @param options Contains decoding options, including if memory is zero
+     *      initialized.
+     *  @param ctable A pointer to a color table.  When dstInfo.colorType() is
+     *      kIndex8, this should be non-NULL and have enough storage for 256
+     *      colors.  The color table will be populated after decoding the palette.
+     *  @param ctableCount A pointer to the size of the color table.  When
+     *      dstInfo.colorType() is kIndex8, this should be non-NULL.  It will
+     *      be modified to the true size of the color table (<= 256) after
+     *      decoding the palette.
+     *  @return Enum representing success or reason for failure.
+     */
+    SkCodec::Result reset(const SkImageInfo& dstInfo, const SkCodec::Options* options,
+                          SkPMColor ctable[], int* ctableCount);
+
+    /**
+     *  Simplified version of reset() that asserts that info is NOT
+     *  kIndex8_SkColorType and uses the default Options.
+     */
+    SkCodec::Result reset(const SkImageInfo& dstInfo);

[msarett, 2015/07/30 12:58:46]

So can we get away with not calling reset() at all if the dstColorType is not
kIndex8 and we don't need to pass any options in?

One comments says:
"reset must be called in order to decode any scanlines"

Another says:
"If reset has never been called, this is the info that will be used."

I think I understand it after reading the comments over this function, but maybe
we can be a little more consistent throughout this file?

[scroggo, 2015/07/30 15:11:35]

Agreed. When I originally wrote the header, I made calling reset unnecessary if
you wanted to use the defaults. I later realized that that would require doing
work that is unnecessary if you did *not* want to use the defaults. I changed
some comments, but did not change them all :(

Thank you for diligently reading over the comments!
Haha, good point - if we stuck with the notion of allowing getting scanlines
without calling reset, kIndex_8 would need to be the exception.

+
+    /**
      *  Write the next countLines scanlines into dst.
      *
      *  @param dst Must be non-null, and large enough to hold countLines
@@ -34,6 +89,9 @@ public:
      *      a scanline based on the SkImageInfo used to create this object.
      */
     SkCodec::Result getScanlines(void* dst, int countLines, size_t rowBytes) {
+        if (fDstInfo.isEmpty()) {

[msarett, 2015/07/30 12:58:46]

Maybe there is a way to make to this check once rather than every time we call
getScanlines()?  I'm guessing the info reported by NewFromStream will never be
empty, since we are responsible for filling it in?  And then if we also check
that the info is not empty in reset(), can we avoid this check?

[scroggo, 2015/07/30 15:11:35]

Actually, what I was trying to check was that you *had* called reset.

Alternatively, we could just say: you better not call this without calling
reset, and assert this (instead of having this check). That would be faster in
release, and catch someone in debug trying to do something invalid.

[msarett, 2015/07/30 15:37:55]

+1 for using SkASSERT.

+            return SkCodec::kInvalidParameters;
+        }
         if ((rowBytes < fDstInfo.minRowBytes() && countLines > 1 ) || countLines <= 0
                 || fCurrScanline + countLines > fDstInfo.height()) {
             return SkCodec::kInvalidParameters;
@@ -52,6 +110,9 @@ public:
      *  false.
      */
     SkCodec::Result skipScanlines(int countLines) {
+        if (fDstInfo.isEmpty()) {

[msarett, 2015/07/30 12:58:46]

Same question as getScanlines().

[scroggo, 2015/07/30 15:11:35]

Acknowledged.

+            return SkCodec::kInvalidParameters;
+        }
         if (fCurrScanline + countLines > fDstInfo.height()) {
             // Arguably, we could just skip the scanlines which are remaining,
             // and return kSuccess. We choose to return invalid so the client
@@ -76,8 +137,9 @@ public:
     }
 
 protected:
-    SkScanlineDecoder(const SkImageInfo& requested)
-        : fDstInfo(requested)
+    SkScanlineDecoder(const SkImageInfo& srcInfo)
+        : fSrcInfo(srcInfo)
+        , fDstInfo()
         , fCurrScanline(0) {}
 
     virtual bool onReallyHasAlpha() const { return false; }
@@ -85,9 +147,14 @@ protected:
     const SkImageInfo& dstInfo() const { return fDstInfo; }
 
 private:
-    const SkImageInfo   fDstInfo;
+    const SkImageInfo   fSrcInfo;
+    SkImageInfo         fDstInfo;
     int                 fCurrScanline;
 
+    virtual SkCodec::Result onReset(const SkImageInfo& dstInfo,
+                                    const SkCodec::Options& options,
+                                    SkPMColor ctable[], int* ctableCount) = 0;
+
     // Naive default version just calls onGetScanlines on temp memory.
     virtual SkCodec::Result onSkipScanlines(int countLines) {
         SkAutoMalloc storage(fDstInfo.minRowBytes());