Create an SkAndroidCodec API separate from SkCodec

src/codec/SkAndroidCodec.cpp

+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "SkAndroidCodec.h"
+#include "SkScaledCodec.h"
+#include "SkCodec.h"
+#include "SkCodecPriv.h"
+#include "SkWebpAdapterCodec.h"
+
+static bool is_valid_sample_size(int sampleSize) {
+    // FIXME: As Leon has mentioned elsewhere, surely there is also a maximum sampleSize?

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

I've seen some really high ones. We just need to make sure that we do not crash
or behave badly if the size is too large.

+    return sampleSize > 0;
+}
+
+SkAndroidCodec::SkAndroidCodec(const SkImageInfo& info)
+    : fInfo(info)
+{}
+
+SkAndroidCodec* SkAndroidCodec::NewFromStream(SkStream* stream) {
+    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream));
+    if (nullptr == codec) {
+        return nullptr;
+    }
+
+    switch (codec->getEncodedFormat()) {
+        case kWEBP_SkEncodedFormat:
+            return new SkWebpAdapterCodec((SkWebpCodec*) codec.detach());
+        case kPNG_SkEncodedFormat:
+        case kJPEG_SkEncodedFormat:
+            return new SkSampledCodec(codec.detach());
+        default:
+            // FIXME: SkSampledCodec is temporarily disabled for other formats
+            // while focusing on the formats that are supported by
+            // BitmapRegionDecoder.
+            return nullptr;
+    }
+}
+
+SkAndroidCodec* SkAndroidCodec::NewFromData(SkData* data) {
+    if (!data) {
+        return nullptr;
+    }
+
+    return NewFromStream(new SkMemoryStream(data));
+}
+
+SkISize SkAndroidCodec::getSampledDimensions(int sampleSize) const {
+    if (!is_valid_sample_size(sampleSize)) {

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

Should we instead treat 0 as 1, as Android does?

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

I'm not sure.  It's annoying to catch...I've added code in three places.

But it's probably best to be robust and not depend on SkBitmapRegionDecoder to
catch this.

[scroggo, 2015/10/19 20:03:20]

Haha, agreed, it is annoying to catch it in three places. Since the Android API
only passes it once, maybe this would be simpler by handling it upstack...

[msarett, 2015/10/19 21:39:21]

Let's plan to handle it upstack for now.

+        return SkISize::Make(0, 0);
+    }
+
+    return this->onGetSampledDimensions(sampleSize);
+}
+
+bool SkAndroidCodec::getSubset(SkIRect* desiredSubset) const {
+    if (!desiredSubset || !is_valid_subset(*desiredSubset, fInfo.dimensions())) {
+        return false;
+    }
+
+    return this->onGetSubset(desiredSubset);
+}
+
+SkISize SkAndroidCodec::getSampledSubsetDimensions(int sampleSize, const SkIRect& subset) const {
+    if (!is_valid_sample_size(sampleSize)) {
+        return SkISize::Make(0, 0);
+    }
+
+    // We require that the input subset is a subset that is supported by SkAndroidCodec.
+    // We test this by calling getSubset() and verifying that no modifications are made
+    // to the subset.
+    SkIRect copySubset = subset;
+    if (!this->getSubset(&copySubset) || copySubset != subset) {
+        return SkISize::Make(0, 0);
+    }
+
+    return this->onGetSampledSubsetDimensions(sampleSize, subset);
+}
+
+SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& info, void* pixels,
+        size_t rowBytes, AndroidOptions* options) {
+    if (!pixels) {

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

There are some error cases that we don't check here because SkCodec will catch
them later anyway (missing color table etc.).  Not sure if this is ok.

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

Seems fine to me.

+        return SkCodec::kInvalidParameters;
+    }
+    if (rowBytes < info.minRowBytes()) {
+        return SkCodec::kInvalidParameters;
+    }
+
+    AndroidOptions defaultOptions;
+    if (!options) {
+        options = &defaultOptions;
+    }
+
+    SkISize supportedSize;
+    if (!options->fSubset) {
+        supportedSize = this->getSampledDimensions(options->fSampleSize);

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

I hope these calls are cheap - it seems like a client may have already called
them once to determine the size to use. (They probably are, but it's something
to think about.)

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

They are not trivial, but I *think* they are cheap compared to decoding an
image.

Regardless, I am moving them out of this function.

I think the tradeoff is similar to calling dimensionsSupported in
SkCodec::getPixels().  It is a bit redundant, but good to catch kInvalidScale
before calling onGetPixels().

In this class, I don't think we are moving toward a goal of catching these types
of errors before calling onGetAndroidPixels() (especially because
onGetAndroidPixels() calls SkCodec::getPixels()).  So we might as well move this
check out of here and be less redundant.

+    } else {
+        supportedSize = this->getSampledSubsetDimensions(options->fSampleSize, *options->fSubset);
+    }
+    if (supportedSize != info.dimensions()) {
+        return SkCodec::kInvalidParameters;
+    }
+
+    return this->onGetAndroidPixels(info, pixels, rowBytes, *options);
+}
+
+SkCodec::Result SkAndroidCodec::getAndroidPixels(const SkImageInfo& info, void* pixels,
+        size_t rowBytes) {
+    return this->getAndroidPixels(info, pixels, rowBytes, nullptr);
+}