Add SkEncodedInfo to report properties of encoded image data

include/codec/SkEncodedInfo.h

Index: include/codec/SkEncodedInfo.h
diff --git a/include/codec/SkEncodedInfo.h b/include/codec/SkEncodedInfo.h
new file mode 100644
index 0000000000000000000000000000000000000000..a46c6fbec3e0133022511eb09ab0f4f42dc73b2a
--- /dev/null
+++ b/include/codec/SkEncodedInfo.h
@@ -0,0 +1,139 @@
+/*
+ * Copyright 2016 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkEncodedInfo_DEFINED
+#define SkEncodedInfo_DEFINED
+
+#include "SkImageInfo.h"
+
+struct SkEncodedInfo {
+public:
+
+    enum Alpha {

[scroggo, 2016/03/23 14:48:50]

Are these "Alpha" and "Color" (instead of "AlphaType", "ColorType") to
distinguish from SkAlphaType and SkColorType? "Type" seems to make sense here to
me.

[msarett, 2016/03/24 16:20:43]

I agree that Type makes sense.

The reason I left it off is because I made these enums part of SkEncodedInfo.  I
thought that "SkEncodedInfo::Color" was clear without adding more wordiness. 
And also being different than SkColorType is nice.

I don't feel strongly though.  Happy to change.

+        kOpaque_Alpha,
+        kUnpremul_Alpha,
+
+        // Each pixel is either fully opaque or fully transparent.
+        // There is no difference between requesting kPremul or kUnpremul.
+        kBinary_Alpha,
+    };
+
+    /*
+     * We strive to make the number of components per pixel obvious through
+     * our naming conventions.
+     * Ex: kRGB has 3 components.  kRGBA has 4 components.
+     *
+     * This sometimes results in redundant Alpha and Color information.
+     * Ex: kRGB images must also be kOpaque.
+     */
+    enum Color {
+        // PNG, WBMP

[scroggo, 2016/03/23 14:48:50]

Isn't WBMP only Gray?

[msarett, 2016/03/24 16:20:43]

Yes.  Making this more clear.

+        kGray_Color,
+        kGrayAlpha_Color,
+
+        // PNG, GIF, BMP
+        kPalette_Color,
+
+        // PNG, RAW
+        kRGB_Color,
+        kRGBA_Color,
+
+        // BMP
+        kBGR_Color,
+        kBGRX_Color,

[scroggo, 2016/03/23 14:48:49]

X means we ignore the alpha value, right?

[msarett, 2016/03/24 16:20:43]

Yes.  Alpha is ignored, image is always opaque.

+        kBGRA_Color,
+
+        // JPEG, WEBP
+        kYUV_Color,
+
+        // WEBP
+        kYUVA_Color,
+
+        // JPEG
+        kInvertedCMYK_Color,

[scroggo, 2016/03/23 14:48:50]

Maybe a note here about how the spec disagrees with what is used in practice?
(Isn't that what we determined?)

[msarett, 2016/03/24 16:20:43]

sgtm

Adding a comment.

+        kYCCK_Color,
+    };
+
+    static SkEncodedInfo Make(int width, int height, Color color, Alpha alpha,
+            int bitsPerComponent) {
+        SkASSERT(width > 0 && height > 0);
+
+        SkASSERT(1 == bitsPerComponent ||
+                 2 == bitsPerComponent ||
+                 4 == bitsPerComponent ||
+                 8 == bitsPerComponent ||
+                 16 == bitsPerComponent);
+
+        return SkEncodedInfo(width, height, color, alpha, bitsPerComponent);
+    }
+
+    /*
+     * Returns an SkImageInfo with Skia color and alpha types that are the
+     * closest possible match to the encoded info.
+     */
+    SkImageInfo makeImageInfo() const {

[reed1, 2016/03/22 19:12:44]

why do we have this?

[scroggo, 2016/03/23 14:48:49]

To implement SkCodec::getInfo(). Maybe we only need it for a transition, but I
can imagine that we want it to make it simple for the client to choose. This
gives them a good guess of a logical choice if they don't want to consider all
of the real possibilities.

+        switch (fColor) {
+            case kGray_Color:
+                SkASSERT(kOpaque_Alpha == fAlpha);

[scroggo, 2016/03/23 14:48:50]

Would it make sense to put all these asserts into the constructor? (I guess at
the moment, makeImageInfo() will always be called.)

[msarett, 2016/03/24 16:20:43]

Yes I think so.  Let's not depend on this being called.

+                return SkImageInfo::Make(fWidth, fHeight, kGray_8_SkColorType, kOpaque_SkAlphaType);
+            case kGrayAlpha_Color:
+                SkASSERT(kOpaque_Alpha != fAlpha);
+                return SkImageInfo::Make(fWidth, fHeight, kGray_8_SkColorType,
+                        kUnpremul_SkAlphaType);
+            case kPalette_Color: {
+                SkAlphaType alphaType = (kOpaque_Alpha == fAlpha) ? kOpaque_SkAlphaType :
+                        kUnpremul_SkAlphaType;
+                return SkImageInfo::Make(fWidth, fHeight, kIndex_8_SkColorType, alphaType);
+            }
+            case kRGB_Color:

[scroggo, 2016/03/23 14:48:49]

For RGB(A), should we instead return kRGBA_8888_SkColorType, and
kBGRA_8888_SkColorType for BGR(A)? Does that depend on us supporting the
non-native 32 bit swizzle?

[msarett, 2016/03/24 16:20:43]

I think we should, but maybe not yet.  I think this change would break
SkImageGenerator (and maybe other places) because they just use the color type
that we recommend.

Without supporting the non-native swizzle, the decode will just fail.  If we
start supporting the non-native swizzle, then they'll get swapped RB colors.

+            case kBGR_Color:
+            case kBGRX_Color:
+            case kYUV_Color:
+            case kInvertedCMYK_Color:
+            case kYCCK_Color:
+                SkASSERT(kOpaque_Alpha == fAlpha);
+                return SkImageInfo::Make(fWidth, fHeight, kN32_SkColorType, kOpaque_SkAlphaType);
+            case kRGBA_Color:
+            case kBGRA_Color:
+            case kYUVA_Color:
+                SkASSERT(kOpaque_Alpha != fAlpha);
+                return SkImageInfo::Make(fWidth, fHeight, kN32_SkColorType, kUnpremul_SkAlphaType);
+            default:
+                SkASSERT(false);
+                return SkImageInfo::MakeUnknown();
+        }
+    }
+
+    int width() const { return fWidth; }
+    int height() const { return fHeight; }
+
+    SkEncodedInfo()
+        : fWidth(0)
+        , fHeight(0)
+        , fColor(kRGBA_Color)

[scroggo, 2016/03/23 14:48:50]

The default constructor for SkImageInfo creates something that is invalid
(kUnknown_...) (although it *does* have a legitimate color profile type, which
does not have an Unknown option). Should this one be similar?

[msarett, 2016/03/24 16:20:43]

Yeah I think so.  Adding kUnknown_Color and kUnknown_Alpha.

+        , fAlpha(kUnpremul_Alpha)
+        , fBitsPerComponent(0)
+    {}
+
+private:
+
+    SkEncodedInfo(int width, int height, Color color, Alpha alpha, uint8_t bitsPerComponent)
+        : fWidth(width)
+        , fHeight(height)
+        , fColor(color)
+        , fAlpha(alpha)
+        , fBitsPerComponent(bitsPerComponent)
+    {}
+
+    int     fWidth;
+    int     fHeight;
+    Color   fColor;
+    Alpha   fAlpha;
+    uint8_t fBitsPerComponent;
+};
+
+#endif