SkScaledCodec class

src/codec/SkJpegCodec.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 "SkCodec.h"
 #include "SkJpegCodec.h"
 #include "SkJpegDecoderMgr.h"
@@ skipping 171 matching lines @@
     dinfo.global_state = DSTATE_READY;
     dinfo.num_components = 0;
     dinfo.scale_num = num;
     dinfo.scale_denom = denom;
     turbo_jpeg_calc_output_dimensions(&dinfo);
 
     // Return the calculated output dimensions for the given scale
     return SkISize::Make(dinfo.output_width, dinfo.output_height);
 }
 
+SkCodec::Result SkJpegCodec::initializeSwizzler(const SkImageInfo& requestedInfo,
+                                               void* dst, size_t rowBytes,
+                                               const Options& options,
+                                               SkPMColor ctable[],
+                                               int* ctableCount) {
+
+    const SkColorType srcColorType = requestedInfo.colorType();
+    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, dst, rowBytes,
+                                               options.fZeroInitialized, this->getInfo()));
+    if (!fSwizzler) {
+        // FIXME: CreateSwizzler could fail for another reason.
+        return kUnimplemented;
+    }
+    return kSuccess;
+}
+
 /*
  * Handles rewinding the input stream if it is necessary
  */
 bool SkJpegCodec::handleRewind() {
     switch(this->rewindIfNeeded()) {
         case kCouldNotRewind_RewindState:
             return fDecoderMgr->returnFalse("could not rewind");
         case kRewound_RewindState: {
             JpegDecoderMgr* decoderMgr = NULL;
             if (!ReadHeader(this->stream(), NULL, &decoderMgr)) {
@@ skipping 78 matching lines @@
     fDecoderMgr->dinfo()->scale_denom = 8;
     fDecoderMgr->dinfo()->scale_num = 8;
     turbo_jpeg_calc_output_dimensions(fDecoderMgr->dinfo());
     while (fDecoderMgr->dinfo()->output_width != dstWidth ||
             fDecoderMgr->dinfo()->output_height != dstHeight) {
 
         // Return a failure if we have tried all of the possible scales
         if (1 == fDecoderMgr->dinfo()->scale_num ||
                 dstWidth > fDecoderMgr->dinfo()->output_width ||
                 dstHeight > fDecoderMgr->dinfo()->output_height) {
+            this->fDecoderMgr->dinfo()->scale_num = 8;
+            turbo_jpeg_calc_output_dimensions(this->fDecoderMgr->dinfo());
             return fDecoderMgr->returnFalse("could not scale to requested dimensions");
         }
 
         // Try the next scale
         fDecoderMgr->dinfo()->scale_num -= 1;
         turbo_jpeg_calc_output_dimensions(fDecoderMgr->dinfo());
     }
     return true;
 }
 
@@ skipping 78 matching lines @@
 }
 
 /*
  * Enable scanline decoding for jpegs
  */
 class SkJpegScanlineDecoder : public SkScanlineDecoder {
 public:
     SkJpegScanlineDecoder(const SkImageInfo& dstInfo, SkJpegCodec* codec)
         : INHERITED(dstInfo)
         , fCodec(codec)
-    {}
+    {
+        fStorage.reset(fCodec->getInfo().width() * dstInfo.minRowBytes());

[scroggo, 2015/07/30 18:28:23]

We only need to do this if there is a swizzler, right?

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

Yes, I've added an if statement to only do this if there is a swizzler

+        fSrcRow = static_cast<uint8_t*>(fStorage.get());
+    }
 
     virtual ~SkJpegScanlineDecoder() {
         if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
             SkCodecPrintf("setjmp: Error in libjpeg finish_decompress\n");
             return;
         }
 
         // We may not have decoded the entire image.  Prevent libjpeg-turbo from failing on a
         // partial decode.
         fCodec->fDecoderMgr->dinfo()->output_scanline = fCodec->getInfo().height();
         turbo_jpeg_finish_decompress(fCodec->fDecoderMgr->dinfo());
     }
 
     SkCodec::Result onGetScanlines(void* dst, int count, size_t rowBytes) override {
         // Set the jump location for libjpeg errors
         if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
             return fCodec->fDecoderMgr->returnFailure("setjmp", SkCodec::kInvalidInput);
         }
+        // Read rows one at a time
+        JSAMPLE* dstRow;
+        if (fCodec->fSwizzler) {

[scroggo, 2015/07/30 18:28:23]

So it looks like these introduce a slight performance hit? Maybe we could try
something like the following:

constructor {
  if (fCodec->fSwizzler) {
    mScanlineProc = &decodeAndSwizzle;
  } else {
    mScanlineProc = &decodeDirect;
  }
}

onGetScanlineInternal(void* dstRow) {
  // Does everything inside the loop
}

onGetScanlines() {
  for (int i = 0; i < count; i++) {
    mScanlineProc;
  }
}

decodeAndSwizzle() {
  // setup
  onGetScanlineInternal
}

decodeDirect() {
  onGetScanlineInternal
}

Not sure if it would be faster, but something to consider. Maybe the difference
is neglible?

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

I think it's negligible. Also, I don't think this one specific if should drive a
function change this big, since all if statements decrease performance. 
We could, for example, do the same thing as above, but with different if
statements in onGetScanlines.
(such as "JCS_CMYK == fCodec->fDecoderMgr->dinfo()->out_color_space") 
Adding that change could potentially speed it up for any of the if statements.
Why should we choose to optimize this specific if statement?
I think if SkJpegCodec was performing too slow for users, then maybe we could go
back and optimize if statements. 
But since there is a minimal performance decrease, and the reason why we are
worried about this if statement and not other if statements isn't clear, I don't
think this change is worth it.
(Leon or Matt, if you think this change is worth it still, let me know)  

[msarett, 2015/07/31 12:17:59]

I think Leon's suggestion is a good one.  We did regress performance by 2.5%,
and while it's not awful, it's worth it to see if we can do better.  Taking a
closer look, it seems to me that the two possible causes are:
1) Memory allocation of fSrcRow (it looks like you've already fixed this to only
allocate when we need a swizzler)
2) If statements inside for loop in onGetScanlines()

Unless we gain the whole 2.5% back from the fix to (1), I think we should try
fixing (2) as well.  I think the idea of Leon's design is to factor the:
if (swizzler) {
 // code
} else {
  //other code
}
outside of the for loop, so we only check the branch once instead of height
times.  It seems logical to me.

[emmaleer, 2015/07/31 18:41:56]

So I re-ran the performance tests, after changing fSrcRow to only be allocated
if we are sampling, and the code is now faster than master by 0.61%. So that
change seemed to fix the slow down! 
I've added the new performance results to the same google-doc.

[scroggo, 2015/07/31 19:31:58]

That's great! I'm a little surprised that it is faster than master, though. That
is comparing running nanobench on JPEG, without used the scaled codec? (If it is
using the scaled codec, the difference may be us sampling versus letting JPEG
sample - not that I expect us to be faster there either, necessarily.)

Glad you tried the simple change and tested it!

[emmaleer, 2015/07/31 20:43:09]

Yes, it is comparing running nanobench on JPEG, without using the scaled codec.

+            // write data to storage row, then sample using swizzler         
+            dstRow = fSrcRow;
+        } else {
+            // write data directly to dst
+            dstRow = (JSAMPLE*) dst;
+        }
 
-        // Read rows one at a time
-        JSAMPLE* dstRow = (JSAMPLE*) dst;
         for (int y = 0; y < count; y++) {
             // Read row of the image
             uint32_t rowsDecoded =
                     turbo_jpeg_read_scanlines(fCodec->fDecoderMgr->dinfo(), &dstRow, 1);
             if (rowsDecoded != 1) {
                 SkSwizzler::Fill(
                         dstRow, this->dstInfo(), rowBytes, count - y, SK_ColorBLACK, NULL);
                 fCodec->fDecoderMgr->dinfo()->output_scanline = this->dstInfo().height();
                 return SkCodec::kIncompleteInput;
             }
 
             // Convert to RGBA if necessary
             if (JCS_CMYK == fCodec->fDecoderMgr->dinfo()->out_color_space) {
                 convert_CMYK_to_RGBA(dstRow, this->dstInfo().width());
             }
 
-            // Move to the next row
-            dstRow = SkTAddOffset<JSAMPLE>(dstRow, rowBytes);
+            if(fCodec->fSwizzler) {
+                // use swizzler to sample row
+                fCodec->fSwizzler->setDstRow(dst);
+                fCodec->fSwizzler->next(dstRow);
+                dst += rowBytes;

[scroggo, 2015/07/30 18:28:23]

Why not use SkTAddOffset?

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

Changed to to SkTAddOffset.

+            } 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);                    \
     }
 #endif
 
     SkCodec::Result onSkipScanlines(int count) override {
         // Set the jump location for libjpeg errors
         if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
             return fCodec->fDecoderMgr->returnFailure("setjmp", SkCodec::kInvalidInput);
         }
 
         turbo_jpeg_skip_scanlines(fCodec->fDecoderMgr->dinfo(), count);
 
         return SkCodec::kSuccess;
     }
 
 private:
     SkAutoTDelete<SkJpegCodec> fCodec;
+    SkAutoMalloc               fStorage;

[scroggo, 2015/07/30 18:28:23]

// Only used if sampling is needed.

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

Acknowledged.

+    uint8_t*                   fSrcRow;
 
     typedef SkScanlineDecoder INHERITED;
 };
 
 SkScanlineDecoder* SkJpegCodec::onGetScanlineDecoder(const SkImageInfo& dstInfo,
         const Options& options, SkPMColor ctable[], int* ctableCount) {
 
     // Rewind the stream if needed
     if (!this->handleRewind()) {
         SkCodecPrintf("Could not rewind\n");
@@ skipping 16 matching lines @@
     }
 
     // Check if we can decode to the requested destination and set the output color space
     if (!codec->setOutputColorSpace(dstInfo)) {
         SkCodecPrintf("Cannot convert to output type\n");
         return NULL;
     }
 
     // Perform the necessary scaling
     if (!codec->scaleToDimensions(dstInfo.width(), dstInfo.height())) {
-        SkCodecPrintf("Cannot scale to output dimensions\n");
-        return NULL;
+        // native scaling to dstInfo dimensions not supported
+
+        if (dstInfo.height() != this->getInfo().height()) {
+            // no non-native height sampling supported
+            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
     if (!turbo_jpeg_start_decompress(codec->fDecoderMgr->dinfo())) {
         SkCodecPrintf("start decompress failed\n");
         return NULL;
     }
 
     // Return the new scanline decoder
     return SkNEW_ARGS(SkJpegScanlineDecoder, (dstInfo, codec.detach()));
 }