SkScaledCodec class

src/codec/SkScaledCodec.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 "SkCodecPriv.h"
+#include "SkScaledCodec.h"
+#include "SkStream.h"
+#include "SkWebpCodec.h"
+
+
+SkCodec* SkScaledCodec::NewFromStream(SkStream* stream) {
+    bool isWebp = SkWebpCodec::IsWebp(stream);

[scroggo, 2015/07/27 19:29:57]

I'd prefer to not include SkWebpCodec here, and require rewinding here.

Instead, you can do the following:

SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream));
if (codec->getEncodedFormat() == SkEncodedFormat::kWEBP_SkEncodedFormat) {
  // webp codec supports scaling and subsetting natively
  return codec.release();
}

// wrap in new SkScaledCodec.

[emmaleer, 2015/07/28 14:19:16]

Okay, I like that method better too.
Just checking, when you said codec.release() you meant codec.detach() right? 

[scroggo, 2015/07/28 15:58:10]

Yeah.

For a long time, C++ did not have a standard smart pointer, so everybody wrote
their own. Skia's (SkAutoTDelete) calls that method detach(), but others call
their analogous methods release().

[emmaleer, 2015/07/29 21:55:08]

Acknowledged.

+    if (!stream->rewind()) {
+        return NULL;
+    }
+    if (isWebp) {
+        // Webp supports scaling natively
+        return SkWebpCodec::NewFromStream(stream);  
+    }
+    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream));
+    if (NULL == codec) {
+        return NULL;
+    }
+    return SkNEW_ARGS(SkScaledCodec, (codec.detach()));
+}
+
+SkCodec* SkScaledCodec::NewFromData(SkData* data) {
+    if (!data) {
+        return NULL;
+    }
+    return NewFromStream(SkNEW_ARGS(SkMemoryStream, (data)));
+}
+
+SkScaledCodec::SkScaledCodec(SkCodec* codec)
+    : INHERITED(codec->getInfo(), NULL)
+    , fCodec(codec)
+{}
+
+SkScaledCodec::~SkScaledCodec() {}
+
+/*
+ * Return a valid set of output dimensions for this decoder, given an input scale
+ */
+SkISize SkScaledCodec::onGetScaledDimensions(float desiredScale) const {
+    // support scaling down by integer numbers. Ex: 1/2, 1/3, 1/4 ...
+
+    if (desiredScale > 0.5f) {
+        // sampleSize = 1
+        return fCodec->getInfo().dimensions();
+    }
+    // sampleSize determines the step size between samples
+    // Ex: sampleSize = 2, sample every second pixel in x and y directions
+    int sampleSize = 1 / desiredScale;
+
+    // if sampleSize > width or height, will only sample 1 pixel in that direction
+    int sampleX = SkMin32(sampleSize, this->getInfo().width());
+    int sampleY = SkMin32(sampleSize, this->getInfo().height());
+
+    int scaledWidth = this->getInfo().width() / sampleX;
+    int scaledHeight = this->getInfo().height() / sampleY;

[msarett, 2015/07/27 20:42:18]

You're probably already aware of this, but we need to be really thoughtful about
how to deal with cases where width/height and sampleX/Y don't divide evenly.

The decision to round down (as you do here) makes just as much sense as anything
else (as far as I know).

As an fyi, I think the built in scaling in libjpeg-turbo rounds up.

And then we control the rounding for webp (since we pass the desired dimensions
to libwebp).  It looks like we actually round for webp, rather taking the
floor() or ceil().

Not sure if any of this is relevant, but it feels like it may get tricky.

[emmaleer, 2015/07/28 14:19:16]

This is inconsistent. I think it would be better if they all did the same thing.
I think scaling down is best, but SkJpeg rounds up, and I don't think we should
choose to round up just because that's what Jpeg does... Unless we think
consistency is most important.  
Matt, did you find any information or comments saying why Jpeg chooses to round
up?
I've added a comment stating that we currently round down, but we can change
that in the future if we want.

[msarett, 2015/07/28 20:53:10]

AFAICT it's an arbitrary decision.

+
+    // Return the calculated output dimensions for the given scale
+    return SkISize::Make(scaledWidth, scaledHeight);
+}
+
+// check if scaling to dstInfo size from srcInfo size is possible
+static bool scaling_supported(const SkImageInfo&  dstInfo, const SkImageInfo& srcInfo){
+    const int dstWidth = dstInfo.width();
+    const int dstHeight = dstInfo.height();
+    const int srcWidth = srcInfo.width();
+    const int srcHeight = srcInfo.height();
+     // only support down sampling, not up sampling
+    if (dstWidth > srcWidth || dstHeight  > srcHeight) {
+        return false;
+    }
+    // check that srcWidth is scaled down by an integer value
+    int sampleSizeX = srcWidth / dstWidth;
+    if (srcWidth / sampleSizeX != dstWidth) {
+        return false;
+    }
+    // check that src height is scaled down by an integer value
+    int sampleSizeY = srcHeight / dstHeight;
+    if (srcHeight / sampleSizeY != dstHeight) {
+        return false;
+    }
+    // sampleX and sampleY should be equal unless the original sampleSize requested was larger
+    // than srcWidth or srcHeight. If so, the result of this is dstWidth or dstHeight = 1.
+    // This functionality allows for tall thin images to still be scaled down by scaling factors. 
+    if (sampleSizeX != sampleSizeY){
+        if (1 != dstWidth && 1 != dstHeight) {
+            return false;
+        }
+    }
+    return true;
+}
+
+SkCodec::Result SkScaledCodec::onGetPixels(const SkImageInfo& requestedInfo, void* dst,
+                                           size_t rowBytes, const Options& options,
+                                           SkPMColor ctable[], int* ctableCount) {
+    
+    Result result = fCodec->getPixels(requestedInfo, dst, rowBytes, &options, ctable, ctableCount);

[msarett, 2015/07/27 20:42:18]

I was going to comment that this seems strange, and maybe we should just check
if the dimensions of requestedInfo match the original dimensions.

But then I realized that this will allow codecs that support some scaling (ex:
jpeg) to try to use their built in scaling before using SkScaledCodec as a
fallback.  Was this cleverness intended?  Maybe some comments would help me feel
better about this :).

[emmaleer, 2015/07/28 14:19:16]

Yes, this was intended. Okay, I've added a comment describing what's happening.
Also, I call the decode an "fCodec native decode", does that make sense? For the
call to fCodec->getPixels()
If not I'll try to think of something better. 

[msarett, 2015/07/28 20:53:10]

Thanks!

+    if (kInvalidScale != result) {
+        // no scaling requested
+        return result;
+    }
+    // scaling requested
+
+    if (!scaling_supported(requestedInfo, fCodec->getInfo())) {
+        return kInvalidScale;
+    }
+
+    bool isInterlaced = fCodec->isInterlaced();

[msarett, 2015/07/27 20:42:18]

For now, isInterlaced() is a decent name for this.  I have a feeling that as we
go along, we will encounter some non-interlaced images that we can't scale for
one reason or another.  And also, there are some interlaced images (jpegs and
maybe gifs) that we can scale.  Maybe we should rename this isScalable() or
something?

[scroggo, 2015/07/27 20:47:24]

My first thought was the name isSampleable(), but even for images that return
false, we still sample (as a means to scale) them, we just do it after getting
all the scanlines.

[emmaleer, 2015/07/28 14:19:16]

What about PostYSample()? Since we have to sample after reading all the lines,
instead of during. 

[scroggo, 2015/07/28 15:58:10]

PostYSample() sounds to me like it actually does something (rather than
returning a property of the object). Along those lines, I might push for
requiresPostYSampling(), although I'm not sure that's clear either.

[msarett, 2015/07/28 20:53:10]

What about isHardToSample()?

[emmaleer, 2015/07/29 21:55:08]

I like isHardToSample, since it's more broad, and we will be using this function
for a variety of things. 

[scroggo, 2015/07/30 17:53:01]

It is broad, but it's not obvious to me what hard to sample means. What we want
it to mean is that we have to just decode every line and then sample, but that
doesn't seem obvious to me from the name. (To be fair, I haven't thought of a
better name...)

[emmaleer, 2015/07/30 22:27:55]

Should we use requiresPostSampling() ?
or what about canSampleMidWay() ?
I think canSampleMidWay makes more sense than requiresPostSampling

+    Options scaledOptions = options;
+    scaledOptions.fSampled = true;
+
+    int dstHeight = requestedInfo.height();
+    int srcHeight = fCodec->getInfo().height();
+    
+    SkImageInfo info = requestedInfo;
+    if (isInterlaced){
+        // must decode entire height for interlaced, then scale after
+        info = info.makeWH(requestedInfo.width(), srcHeight);
+    }
+
+    SkAutoTDelete<SkScanlineDecoder> scanlineDecoder(fCodec->getScanlineDecoder(info, 
+                                                     &scaledOptions, ctable, ctableCount));
+    if (NULL == scanlineDecoder) {
+        SkCodecPrintf("failed to create scanline decoder.\n");
+        return kInvalidInput;
+    }
+
+    // set sample values in x and y directions
+    int sampleY = srcHeight / dstHeight;
+    int Y0 = sampleY >> 1;
+    int sampleX = fCodec->getInfo().width() / requestedInfo.width();
+    if (!scanlineDecoder->setSampleX(sampleX)) {
+        return kInvalidScale;
+    }
+    
+    if (isInterlaced) {
+        SkAutoMalloc storage(srcHeight * rowBytes);
+        void* storagePtr = storage.get();
+        result = scanlineDecoder->getScanlines(storagePtr, srcHeight, rowBytes);
+
+        storagePtr += Y0 * rowBytes;
+        for (int y = 0; y < dstHeight; y++) {
+            memcpy(dst, storagePtr, rowBytes);
+            storagePtr += sampleY * rowBytes;
+            dst += rowBytes;
+        }
+    } else {
+        // not interlaced
+        scanlineDecoder->skipScanlines(Y0);
+        for (int y = 0; y < dstHeight; y++) {
+            scanlineDecoder->getScanlines(dst, 1, rowBytes);
+            if (y < dstHeight - 1) {
+                scanlineDecoder->skipScanlines(sampleY - 1);
+            }
+            dst += rowBytes;
+        }
+    }
+    return kSuccess;
+}