SkScaledCodec class

src/codec/SkJpegCodec.cpp

Index: src/codec/SkJpegCodec.cpp
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 84e90d444249142d69a68cd52cab59cebb37c30a..c4bfebc39b51e436d706a2fb3c689679ff01a5f4 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -11,6 +11,7 @@
 #include "SkJpegUtility_codec.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
+#include "SkScaledCodec.h"
 #include "SkScanlineDecoder.h"
 #include "SkStream.h"
 #include "SkTemplates.h"
@@ -146,6 +147,8 @@ SkJpegCodec::SkJpegCodec(const SkImageInfo& srcInfo, SkStream* stream,
         JpegDecoderMgr* decoderMgr)
     : INHERITED(srcInfo, stream)
     , fDecoderMgr(decoderMgr)
+    , fPartialNum(1)
+    , fPartialDenom(1)
 {}
 
 /*
@@ -189,6 +192,43 @@ SkISize SkJpegCodec::onGetScaledDimensions(float desiredScale) const {
     return SkISize::Make(dinfo.output_width, dinfo.output_height);
 }
 
+// The swizzler is only used for sampling in the x direction
+// when sampling a scanlineDecoder is used. The swizzler is not used in onGetPixels
+SkCodec::Result SkJpegCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
+                                               void* dst, size_t rowBytes,

[scroggo, 2015/08/05 15:36:00]

It looks like these parameters are not needed? I think you only need
requestedInfo and options?

[emmaleer, 2015/08/05 18:41:52]

Acknowledged.

+                                               const Options& options,
+                                               SkPMColor ctable[],
+                                               int* ctableCount) {
+
+    const SkColorType srcColorType = requestedInfo.colorType();

[scroggo, 2015/08/05 15:36:00]

It seems weird that the requestedInfo yields the srcColorType. Oh wait - the
swizzler for JPEG does not actually do any color conversion, right? So the src
and dst colorType are always going to be the same? In that case, maybe the names
should just be "info" and "colorType"? (FWIW, it looks like we do not even need
a variable for color type - we can just say

switch (info.colorType())

)

[emmaleer, 2015/08/05 18:41:52]

Yes, the destination and source colorTypes are the same. Okay, I've changed
requestedInfo to info.

+    SkSwizzler::SrcConfig srcConfig;
+    switch (srcColorType) {
+        case kGray_8_SkColorType:
+            srcConfig = SkSwizzler::kGray;
+            break;
+        case kRGBA_8888_SkColorType:
+            srcConfig = SkSwizzler::kRGBX;
+            break;
+        case kBGRA_8888_SkColorType:
+            srcConfig = SkSwizzler::kBGRX;
+            break;
+        case kRGB_565_SkColorType:
+            srcConfig = SkSwizzler::kRGB_565;
+            break;
+        default:
+            //would have exited before now if the colorType was supported by jpeg
+            SkASSERT(false);
+    }
+
+    fSwizzler.reset(SkSwizzler::CreateSwizzler(srcConfig, NULL, requestedInfo, 

[scroggo, 2015/08/05 15:36:00]

Sorry, I've gone back and forth on this. If the swizzler is only needed by the
SD, maybe this (and the swizzler) should go on the SD?

[emmaleer, 2015/08/05 18:41:52]

Yes, that makes sense. Now that the creation function, onStart, is in the SD
class, we can move initializeSwizzler, and the swizzler, into that class too. 
I need to first merge your code into my branch, and then I will move the
swizzler to the SD.

[scroggo, 2015/08/05 19:10:04]

sounds good

[emmaleer, 2015/08/06 13:45:46]

Swizzler is now in the ScanlineDecoder.

+                                               options.fZeroInitialized, this->getInfo().width()));
+    if (!fSwizzler) {
+        // FIXME: CreateSwizzler could fail for another reason.
+        return kUnimplemented;
+    }
+    return kSuccess;
+}
+
 /*
  * Handles rewinding the input stream if it is necessary
  */
@@ -272,10 +312,10 @@ bool SkJpegCodec::setOutputColorSpace(const SkImageInfo& dst) {
 }
 
 /*
- * Checks if we can scale to the requested dimensions and scales the dimensions
- * if possible
+ * Checks if we can natively scale to the requested dimensions and natively scales the 
+ * dimensions if possible
  */
-bool SkJpegCodec::scaleToDimensions(uint32_t dstWidth, uint32_t dstHeight) {
+bool SkJpegCodec::nativelyScaleToDimensions(uint32_t dstWidth, uint32_t dstHeight) {
     // libjpeg-turbo can scale to 1/8, 1/4, 3/8, 1/2, 5/8, 3/4, 7/8, and 1/1
     fDecoderMgr->dinfo()->scale_denom = 8;
     fDecoderMgr->dinfo()->scale_num = 8;
@@ -287,7 +327,10 @@ bool SkJpegCodec::scaleToDimensions(uint32_t dstWidth, uint32_t dstHeight) {
         if (1 == fDecoderMgr->dinfo()->scale_num ||
                 dstWidth > fDecoderMgr->dinfo()->output_width ||
                 dstHeight > fDecoderMgr->dinfo()->output_height) {
-            return fDecoderMgr->returnFalse("could not scale to requested dimensions");
+            // reset native scale settings on failure because this may be supported by the swizzler 
+            this->fDecoderMgr->dinfo()->scale_num = 8;
+            turbo_jpeg_calc_output_dimensions(this->fDecoderMgr->dinfo());
+            return false;
         }
 
         // Try the next scale
@@ -327,7 +370,7 @@ SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
     }
 
     // Perform the necessary scaling
-    if (!this->scaleToDimensions(dstInfo.width(), dstInfo.height())) {
+    if (!this->nativelyScaleToDimensions(dstInfo.width(), dstInfo.height())) {
         return fDecoderMgr->returnFailure("cannot scale to requested dims", kInvalidScale);
     }
 
@@ -394,7 +437,16 @@ public:
         : INHERITED(dstInfo)
         , fCodec(codec)
         , fOpts(opts)
-    {}
+    {
+        if(fCodec->fSwizzler) {
+            int colorBytes = (codec->fDecoderMgr->dinfo()->out_color_space == JCS_RGB565) ? 2 :

[scroggo, 2015/08/05 15:36:00]

Maybe make this a static function that takes a dinfo and returns colorBytes?

That said, it looks like both times you actually want the row bytes - maybe just
a function that returns rowbytes?

(it could also be a function on JpegDecoderMgr, although if it's only needed in
this file, and only operates on a dinfo, a static function might be better. I
also wonder if jpeg has a function that computes this?)

[msarett, 2015/08/05 15:54:56]

Sorry for making a suggestion that led to code duplication...ugh.

I think a static helper function that computes rowBytes makes sense.

I'm not aware of jpeg have anything built in to compute this.  When I was
allocating a row buffer inside libjpeg-turbo, DRC suggested something similar to
this code - which I think is a good indication that jpeg doesn't have anything
better.

[emmaleer, 2015/08/05 18:41:52]

Acknowledged.

+                              codec->fDecoderMgr->dinfo()->out_color_components;
+            fStorage.reset(codec->fDecoderMgr->dinfo()->output_width * colorBytes);
+            fSrcRow = static_cast<uint8_t*>(fStorage.get());
+        } else {
+            fSrcRow = NULL;
+        }
+    }
 
     virtual ~SkJpegScanlineDecoder() {
         if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
@@ -413,9 +465,16 @@ public:
         if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
             return fCodec->fDecoderMgr->returnFailure("setjmp", SkCodec::kInvalidInput);
         }
-
         // Read rows one at a time
-        JSAMPLE* dstRow = (JSAMPLE*) dst;
+        JSAMPLE* dstRow;
+        if (fCodec->fSwizzler) {
+            // write data to storage row, then sample using swizzler         
+            dstRow = fSrcRow;
+        } else {
+            // write data directly to dst
+            dstRow = (JSAMPLE*) dst;
+        }
+
         for (int y = 0; y < count; y++) {
             // Read row of the image
             uint32_t rowsDecoded =
@@ -432,22 +491,28 @@ public:
 
             // Convert to RGBA if necessary
             if (JCS_CMYK == fCodec->fDecoderMgr->dinfo()->out_color_space) {
-                convert_CMYK_to_RGBA(dstRow, this->dstInfo().width());
+                convert_CMYK_to_RGBA(dstRow, fCodec->fDecoderMgr->dinfo()->output_width);
             }
 
-            // Move to the next row
-            dstRow = SkTAddOffset<JSAMPLE>(dstRow, rowBytes);
+            if(fCodec->fSwizzler) {
+                // use swizzler to sample row
+                fCodec->fSwizzler->swizzle(dst, dstRow);
+                dst = SkTAddOffset<JSAMPLE>(dst, rowBytes);
+            } else {
+                dstRow = SkTAddOffset<JSAMPLE>(dstRow, rowBytes);
+            }
         }
-
         return SkCodec::kSuccess;
     }
 
 #ifndef TURBO_HAS_SKIP
-#define turbo_jpeg_skip_scanlines(dinfo, count)                              \
-    SkAutoMalloc storage(dinfo->output_width * dinfo->out_color_components); \
-    uint8_t* storagePtr = static_cast<uint8_t*>(storage.get());              \
-    for (int y = 0; y < count; y++) {                                        \
-        turbo_jpeg_read_scanlines(dinfo, &storagePtr, 1);                    \
+#define turbo_jpeg_skip_scanlines(dinfo, count)                                                \
+    int colorBytes = (codec->fDecoderMgr->dinfo()->out_color_space == JCS_RGB565) ? 2 :        \

[scroggo, 2015/08/05 15:36:00]

colorBytes is unused - I assume you want to change storage below to output_width
* colorBytes? (That said, as stated above, I think you can just write a function
that returns the row bytes.)

[emmaleer, 2015/08/05 18:41:52]

Acknowledged.

+                      codec->fDecoderMgr->dinfo()->out_color_components;                       \
+    SkAutoMalloc storage(dinfo->output_width * dinfo->out_color_components);                   \
+    uint8_t* storagePtr = static_cast<uint8_t*>(storage.get());                                \
+    for (int y = 0; y < count; y++) {                                                          \
+        turbo_jpeg_read_scanlines(dinfo, &storagePtr, 1);                                      \
     }
 #endif
 
@@ -464,6 +529,8 @@ public:
 
 private:
     SkAutoTDelete<SkJpegCodec> fCodec;
+    SkAutoMalloc               fStorage;    // Only used if sampling is needed
+    uint8_t*                   fSrcRow;     // Only used if sampling is needed
     const SkCodec::Options&    fOpts;
 
     typedef SkScanlineDecoder INHERITED;
@@ -499,10 +566,22 @@ SkScanlineDecoder* SkJpegCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
         return NULL;
     }
 
+    SkImageInfo scaledInfo = dstInfo;
+
     // Perform the necessary scaling
-    if (!codec->scaleToDimensions(dstInfo.width(), dstInfo.height())) {
-        SkCodecPrintf("Cannot scale to output dimensions\n");
-        return NULL;
+    if (!codec->nativelyScaleToDimensions(dstInfo.width(), dstInfo.height())) {
+        // full native scaling to dstInfo dimensions not supported
+
+        if (!SkScaledCodec::DimensionsSupportedForSampling(this->getInfo(), dstInfo)) {
+            return NULL;
+        }
+        // create swizzler for sampling
+        if (codec->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), options, ctable,
+                                      ctableCount)
+                                      != kSuccess) {
+            SkCodecPrintf("failed to initialize the swizzler.\n");
+            return NULL;
+        }
     }
 
     // Now, given valid output dimensions, we can start the decompress
@@ -512,5 +591,5 @@ SkScanlineDecoder* SkJpegCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
     }
 
     // Return the new scanline decoder
-    return SkNEW_ARGS(SkJpegScanlineDecoder, (dstInfo, codec.detach(), options));
+    return SkNEW_ARGS(SkJpegScanlineDecoder, (scaledInfo, codec.detach(), options));
 }