Add getYUV8Planes() API to SkCodec

include/codec/SkCodec.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 SkCodec_DEFINED
 #define SkCodec_DEFINED
 
@@ skipping 259 matching lines @@
      */
     Result getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, const Options*,
                      SkPMColor ctable[], int* ctableCount);
 
     /**
      *  Simplified version of getPixels() that asserts that info is NOT kIndex8_SkColorType and
      *  uses the default Options.
      */
     Result getPixels(const SkImageInfo& info, void* pixels, size_t rowBytes);
 
+    struct YUVSizeInfo {

[reed1, 2016/01/22 16:01:46]

Just so I understand the difference...

Size means logical number of bytes for width/height
Bytes means allocation number of bytes for width
?

[msarett, 2016/01/22 16:14:14]

Yes that's correct.

+        SkISize fYSize;
+        SkISize fUSize;
+        SkISize fVSize;
+
+        /**
+         * While the widths of the Y, U, and V planes are not restricted, the
+         * implementation requires that the width of the memory allocated for
+         * each plane be a multiple of DCTSIZE (which is always 8).
+         *
+         * This struct allows us to inform the client how many "widthBytes"
+         * that we need.  Note that we use the new idea of "widthBytes"
+         * because this idea is distinct from "rowBytes" (used elsewhere in
+         * Skia).  "rowBytes" allow the last row of the allocation to not
+         * include any extra padding, while, in this case, every single row of
+         * the allocation must be at least "widthBytes".
+         */
+        size_t fYWidthBytes;
+        size_t fUWidthBytes;
+        size_t fVWidthBytes;
+    };
+
+    /**
+     *  If decoding to YUV is supported, this returns true.  Otherwise, this
+     *  returns false and does not modify any of the parameters.
+     *
+     *  @param sizeInfo   Output parameter indicating the sizes and required
+     *                    allocation widths of the Y, U, and V planes.
+     *  @param colorSpace Output parameter.  If non-NULL this is set to kJPEG,
+     *                    otherwise this is ignored.
+     */
+    bool queryYUV8(YUVSizeInfo* sizeInfo, SkYUVColorSpace* colorSpace) const {
+        if (nullptr == sizeInfo) {

[reed1, 2016/01/22 16:01:46]

interesting: why is one parameter optional and the other required? Seems like if
the caller is just interested in knowing if YUV is supported than null for both
makes some sense. Or, since the colorspace is just an enum, seems like requiring
both is not onerous...

[msarett, 2016/01/22 16:14:14]

I'm fine with making both optional or both required.  What do you think is best?

For more background:

Making "colorSpace" optional is the same behavior from the old API.  IMO, this
makes sense because it seems fairly useless here.  We always return kJPEG. 
However, I've kept it because Brian says the GPU needs this enum to distinguish
JPEG from other types of YUV.

I'm not sure I see the point of making sizeInfo optional.  What kind of client
wants to know YUV is supported, but doesn't need the size of the planes?

[msarett, 2016/01/22 21:08:45]

Any thoughts?

+            return false;
+        }
+
+        return this->onQueryYUV8(sizeInfo, colorSpace);
+    }
+
+    /**
+     *  Returns kSuccess, or another value explaining the type of failure.
+     *  This always attempts to perform a full decode.  If the client only
+     *  wants size, it should call queryYUV8().
+     *
+     *  @param sizeInfo   Contains sizes of each of the Y, U, and V planes.
+     *                    The size of the Y plane will be the same size as the
+     *                    image.  The size of the U and V planes may be the
+     *                    same size as the image, or they may be a fraction of
+     *                    the size of the image (due to sampling).  It is
+     *                    quite common for the width to be sampled at a
+     *                    different rate than the height.  2 and 4 are common
+     *                    sampling rates.  The U and V planes are often the
+     *                    same size.
+     *                    Also contains, the width in bytes of each row of the
+     *                    Y, U, and V planes.  Each Y, U, or V "sample" is a
+     *                    single byte.  Also, the implementation requires that
+     *                    the width of the memory allocated be padded to a

[reed1, 2016/01/22 18:40:16]

This comment seems very JPEG specific. Do we need so much detail here, given the
comments on the YUVSizeInfo type itself?

Should the comments here just say something like:

the sizeinfo needs to match exactly the values returned by the query, except
that the widthbytes fields may be larger than the values returned by the query
(but not smaller).

?

[msarett, 2016/01/22 21:08:45]

I think the detail is a bit much.  Fixed.

So are we ok with keeping this parameter (and giving the client flexibility to
choose their own WidthBytes)?

+     *                    multiple of 8.  So the widthBytes should at least
+     *                    equal the Y, U, and V widths rounded up to the
+     *                    nearest multiples of 8.
+     *  @param planes     Memory for each of the Y, U, and V planes.
+     */
+    Result getYUV8Planes(const YUVSizeInfo& sizeInfo, void* planes[3]) {

[reed1, 2016/01/22 16:01:46]

Why do we pass the info back in? Can it (legally) be different from what was
returned by the query method?

[msarett, 2016/01/22 16:14:14]

The WidthBytes may be larger than the recommended WidthBytes (similar to
rowBytes I guess).

Having said that, I would be in favor of removing this parameter.  This means we
require that they use the WidthBytes returned from the query() function.  This
will always be aligned to 8 bytes.  Seems reasonable enough.

What do you think?

+        if (nullptr == planes || nullptr == planes[0] || nullptr == planes[1] ||
+                nullptr == planes[2]) {
+            return kInvalidInput;
+        }
+
+        if (!this->rewindIfNeeded()) {
+            return kCouldNotRewind;
+        }
+
+        return this->onGetYUV8Planes(sizeInfo, planes);
+    }
+
     /**
      * The remaining functions revolve around decoding scanlines.
      */
 
     /**
      *  Prepare for a scanline decode with the specified options.
      *
      *  After this call, this class will be ready to decode the first scanline.
      *
      *  This must be called in order to call getScanlines or skipScanlines.
@@ skipping 145 matching lines @@
      *  is located in the encoded data.
      *
      *  This will equal inputScanline, except in the case of strangely
      *  encoded image types (bottom-up bmps, interlaced gifs).
      */
     int outputScanline(int inputScanline) const;
 
 protected:
     SkCodec(const SkImageInfo&, SkStream*);
 
-    virtual SkISize onGetScaledDimensions(float /* desiredScale */) const {
+    virtual SkISize onGetScaledDimensions(float /*desiredScale*/) const {
         // By default, scaling is not supported.
         return this->getInfo().dimensions();
     }
 
     // FIXME: What to do about subsets??
     /**
      *  Subclasses should override if they support dimensions other than the
      *  srcInfo's.
      */
     virtual bool onDimensionsSupported(const SkISize&) {
         return false;
     }
 
     virtual SkEncodedFormat onGetEncodedFormat() const = 0;
 
     /**
      * @param rowsDecoded When the encoded image stream is incomplete, this function
      *                    will return kIncompleteInput and rowsDecoded will be set to
      *                    the number of scanlines that were successfully decoded.
      *                    This will allow getPixels() to fill the uninitialized memory.
      */
     virtual Result onGetPixels(const SkImageInfo& info,
                                void* pixels, size_t rowBytes, const Options&,
                                SkPMColor ctable[], int* ctableCount,
                                int* rowsDecoded) = 0;
 
-    virtual bool onGetValidSubset(SkIRect* /* desiredSubset */) const {
+    virtual bool onQueryYUV8(YUVSizeInfo*, SkYUVColorSpace*) const {
+        return false;
+    }
+
+    virtual Result onGetYUV8Planes(const YUVSizeInfo&, void*[3] /*planes*/) {
+        return kUnimplemented;
+    }
+
+    virtual bool onGetValidSubset(SkIRect* /*desiredSubset*/) const {
         // By default, subsets are not supported.
         return false;
     }
 
     /**
      *  If the stream was previously read, attempt to rewind.
      *
      *  If the stream needed to be rewound, call onRewind.
      *  @returns true if the codec is at the right position and can be used.
      *      false if there was a failure to rewind.
@@ skipping 144 matching lines @@
      *  not affect ownership.
      *
      *  Only valid during scanline decoding.
      */
     virtual SkSampler* getSampler(bool /*createIfNecessary*/) { return nullptr; }
 
     friend class SkSampledCodec;
     friend class SkIcoCodec;
 };
 #endif // SkCodec_DEFINED