Scanline decoding for jpeg

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 43 matching lines @@
     }
     return SkSwizzler::kUnknown;
 }
 
 /*
  * Convert a row of CMYK samples to RGBX in place.
  * Note that this method moves the row pointer.
  * @param width the number of pixels in the row that is being converted
  *              CMYK is stored as four bytes per pixel
  */
-static void convert_CMYK_to_RGB(uint8_t* row, uint32_t width) {
+static void convert_CMYK_to_RGB(uint8_t* row, size_t rowBytes) {

[scroggo, 2015/04/24 13:14:28]

It seems very weird to me to pass rowBytes here. Why not stick with width?

[msarett, 2015/04/24 15:08:44]

Yeah you are absolutely right.  I made the change because I felt it was more
convenient.  Looking back, it is just as convenient to pass width and certainly
more clear.

     // We will implement a crude conversion from CMYK -> RGB using formulas
     // from easyrgb.com.
     //
     // CMYK -> CMY
     // C = C * (1 - K) + K
     // M = M * (1 - K) + K
     // Y = Y * (1 - K) + K
     //
     // libjpeg actually gives us inverted CMYK, so we must subtract the
     // original terms from 1.
@@ skipping 21 matching lines @@
     // B = Y * K * 255
     //
     // As a final note, we have treated the CMYK values as if they were on
     // a scale from 0-1, when in fact they are 8-bit ints scaling from 0-255.
     // We must divide each CMYK component by 255 to obtain the true conversion
     // we should perform.
     // CMYK -> RGB
     // R = C * K / 255
     // G = M * K / 255
     // B = Y * K / 255
+    uint32_t width = rowBytes / 4;
     for (uint32_t x = 0; x < width; x++, row += 4) {
         row[0] = SkMulDiv255Round(row[0], row[3]);
         row[1] = SkMulDiv255Round(row[1], row[3]);
         row[2] = SkMulDiv255Round(row[2], row[3]);
         row[3] = 0xFF;
     }
 }
 
 bool SkJpegCodec::IsJpeg(SkStream* stream) {
     static const uint8_t jpegSig[] = { 0xFF, 0xD8, 0xFF };
@@ skipping 45 matching lines @@
         streamDeleter.detach();
         return codec;
     }
     return NULL;
 }
 
 SkJpegCodec::SkJpegCodec(const SkImageInfo& srcInfo, SkStream* stream,
         JpegDecoderMgr* decoderMgr)
     : INHERITED(srcInfo, stream)
     , fDecoderMgr(decoderMgr)
+    , fSwizzler(NULL)
+    , fSrcRowBytes(0)
 {}
 
 /*
  * Return a valid set of output dimensions for this decoder, given an input scale
  */
 SkISize SkJpegCodec::onGetScaledDimensions(float desiredScale) const {
     // libjpeg supports scaling by 1/1, 1/2, 1/4, and 1/8, so we will support these as well
     long scale;
     if (desiredScale > 0.75f) {
         scale = 1;
@@ skipping 39 matching lines @@
     // Always allow kN32 as the color type
     if (kN32_SkColorType == dst.colorType()) {
         return true;
     }
 
     // Otherwise require that the destination color type match our recommendation
     return dst.colorType() == src.colorType();
 }
 
 /*
+ * Handles rewinding the input stream if it is necessary
+ */
+bool SkJpegCodec::handleRewind() {
+    SkCodec::RewindState rewindState = this->rewindIfNeeded();
+    if (rewindState == kCouldNotRewind_RewindState) {
+        return fDecoderMgr->returnFalse("could not rewind");
+    } else if (rewindState == kRewound_RewindState) {
+        JpegDecoderMgr* decoderMgr = NULL;
+        if (!ReadHeader(this->stream(), NULL, &decoderMgr)) {
+            return fDecoderMgr->returnFalse("could not rewind");
+        }
+        SkASSERT(NULL != decoderMgr);
+        fDecoderMgr.reset(decoderMgr);
+    }
+    return true;

[scroggo, 2015/04/24 13:14:28]

nit: I think this should go inside the else statement, since no other paths lead
to it.

[msarett, 2015/04/24 15:08:44]

Hmmm I'm not quite sure I understand, but let me know if you like this better.

[scroggo, 2015/04/24 15:23:13]

Oh no, I was confused. I like it better the first way. I forgot there was a
third option for rewindState. Which reminds me that I think a switch statement
is more clear here.

[msarett, 2015/04/24 15:40:13]

Done.

+}
+
+/*
+ * Checks if we can scale to the requested dimensions and performs the scaling
+ */
+bool SkJpegCodec::handleScaling(const SkImageInfo& dstInfo) {

[scroggo, 2015/04/24 13:14:28]

You only need width and height here. Why not pass those two as parameters
instead?

Also, can we just name this "scale"? (or maybe something like
"scaleToDimensions"?)

[msarett, 2015/04/24 15:08:44]

Agreed.

+    // libjpeg can scale to 1/1, 1/2, 1/4, and 1/8
+    SkASSERT(1 == fDecoderMgr->dinfo()->scale_num);
+    SkASSERT(1 == fDecoderMgr->dinfo()->scale_denom);
+    jpeg_calc_output_dimensions(fDecoderMgr->dinfo());
+    const uint32_t dstWidth = dstInfo.width();
+    const uint32_t dstHeight = dstInfo.height();
+    while (fDecoderMgr->dinfo()->output_width != dstWidth ||
+            fDecoderMgr->dinfo()->output_height != dstHeight) {
+
+        // Return a failure if we have tried all of the possible scales
+        if (8 == fDecoderMgr->dinfo()->scale_denom ||
+                dstWidth > fDecoderMgr->dinfo()->output_width ||
+                dstHeight > fDecoderMgr->dinfo()->output_height) {
+            return fDecoderMgr->returnFalse("could not scale to requested dimensions");
+        }
+
+        // Try the next scale
+        fDecoderMgr->dinfo()->scale_denom *= 2;
+        jpeg_calc_output_dimensions(fDecoderMgr->dinfo());
+    }
+    return true;
+}
+
+/*
+ * Create the swizzler based on the encoded format
+ */
+void SkJpegCodec::initializeSwizzler(const SkImageInfo& dstInfo,
+                                     void* dst, size_t dstRowBytes,
+                                     const Options& options) {
+    SkSwizzler::SrcConfig srcConfig = get_src_config(*fDecoderMgr->dinfo());
+    fSwizzler.reset(SkSwizzler::CreateSwizzler(srcConfig, NULL, dstInfo, dst, dstRowBytes,
+            options.fZeroInitialized));
+    fSrcRowBytes = SkSwizzler::BytesPerPixel(srcConfig) * dstInfo.width();
+}
+
+/*
  * Performs the jpeg decode
  */
 SkCodec::Result SkJpegCodec::onGetPixels(const SkImageInfo& dstInfo,
                                          void* dst, size_t dstRowBytes,
                                          const Options& options, SkPMColor*, int*) {
+
     // Rewind the stream if needed
-    SkCodec::RewindState rewindState = this->rewindIfNeeded();
-    if (rewindState == kCouldNotRewind_RewindState) {
-        return kCouldNotRewind;
-    } else if (rewindState == kRewound_RewindState) {
-        JpegDecoderMgr* decoderMgr = NULL;
-        if (!ReadHeader(this->stream(), NULL, &decoderMgr)) {
-            return kCouldNotRewind;
-        }
-        SkASSERT(NULL != decoderMgr);
-        fDecoderMgr.reset(decoderMgr);
+    if (!this->handleRewind()) {
+        fDecoderMgr->returnFailure("could not rewind stream", kCouldNotRewind);
     }
 
     // Get a pointer to the decompress info since we will use it quite frequently
     jpeg_decompress_struct* dinfo = fDecoderMgr->dinfo();
 
     // Set the jump location for libjpeg errors
     if (setjmp(fDecoderMgr->getJmpBuf())) {
         return fDecoderMgr->returnFailure("setjmp", kInvalidInput);
     }
 
     // Check if we can decode to the requested destination
     if (!conversion_possible(dstInfo, this->getInfo())) {
         return fDecoderMgr->returnFailure("conversion_possible", kInvalidConversion);
     }
-    // Check if we can scale to the requested dimensions
-    // libjpeg can scale to 1/1, 1/2, 1/4, and 1/8
-    SkASSERT(1 == dinfo->scale_num);
-    SkASSERT(1 == dinfo->scale_denom);
-    jpeg_calc_output_dimensions(dinfo);
-    const uint32_t dstWidth = dstInfo.width();
-    const uint32_t dstHeight = dstInfo.height();
-    while (dinfo->output_width != dstWidth || dinfo->output_height != dstHeight) {
 
-        // Return a failure if we have tried all of the possible scales
-        if (8 == dinfo->scale_denom ||
-                dstWidth > dinfo->output_width ||
-                dstHeight > dinfo->output_height) {
-            return fDecoderMgr->returnFailure("cannot scale to requested dims", kInvalidScale);
-        }
-
-        // Try the next scale
-        dinfo->scale_denom *= 2;
-        jpeg_calc_output_dimensions(dinfo);
+    // Perform the necessary scaling
+    if (!this->handleScaling(dstInfo)) {
+        fDecoderMgr->returnFailure("cannot scale to requested dims", kInvalidScale);
     }
 
     // Now, given valid output dimensions, we can start the decompress
     if (!jpeg_start_decompress(dinfo)) {
         return fDecoderMgr->returnFailure("startDecompress", kInvalidInput);
     }
 
     // Create the swizzler
-    SkSwizzler::SrcConfig srcConfig = get_src_config(*dinfo);
-    SkAutoTDelete<SkSwizzler> swizzler(SkSwizzler::CreateSwizzler(srcConfig, NULL, dstInfo, dst,
-            dstRowBytes, options.fZeroInitialized));
-    if (NULL == swizzler) {
+    this->initializeSwizzler(dstInfo, dst, dstRowBytes, options);
+    if (NULL == fSwizzler) {
         return fDecoderMgr->returnFailure("getSwizzler", kInvalidInput);

[scroggo, 2015/04/24 13:14:28]

This is not new to this change, but I think kInvalidInput might not be correct
here. It means the stream was not valid. But initializing the swizzler does not
involve reading the stream, right? In SkPngCodec, I returned kUnimplemented,
thinking it would mean we had not implemented something in the swizzler, but
maybe it should be kInvalidParameters, meaning the row bytes or dst info was not
supported? I guess it's possible that the SrcConfig was invalid, which might
mean the stream is no good?

[msarett, 2015/04/24 15:08:44]

I think kUnimplemented sounds like the right choice until we can prove its
something else.  At least it is good to be consistent.

     }
-    const uint32_t srcBytesPerPixel = SkSwizzler::BytesPerPixel(srcConfig);
 
     // This is usually 1, but can also be 2 or 4.
     // If we wanted to always read one row at a time, we could, but we will save space and time
     // by using the recommendation from libjpeg.
     const uint32_t rowsPerDecode = dinfo->rec_outbuf_height;
     SkASSERT(rowsPerDecode <= 4);
 
     // Create a buffer to contain decoded rows (libjpeg requires a 2D array)
-    const uint32_t srcRowBytes = srcBytesPerPixel * dstWidth;
-    SkAutoTDeleteArray<uint8_t> srcBuffer(SkNEW_ARRAY(uint8_t, srcRowBytes * rowsPerDecode));
+    SkAutoTDeleteArray<uint8_t> srcBuffer(SkNEW_ARRAY(uint8_t, fSrcRowBytes * rowsPerDecode));

[scroggo, 2015/04/24 13:14:28]

Maybe SkASSERT(fSrcRowBytes != 0)

[msarett, 2015/04/24 15:08:44]

Done.

     JSAMPLE* srcRows[4];
     uint8_t* srcPtr = srcBuffer.get();
     for (uint8_t i = 0; i < rowsPerDecode; i++) {
         srcRows[i] = (JSAMPLE*) srcPtr;
-        srcPtr += srcRowBytes;
+        srcPtr += fSrcRowBytes;
     }
 
     // Ensure that we loop enough times to decode all of the rows
     // libjpeg will prevent us from reading past the bottom of the image
+    uint32_t dstHeight = dstInfo.height();
     for (uint32_t y = 0; y < dstHeight + rowsPerDecode - 1; y += rowsPerDecode) {
         // Read rows of the image
         uint32_t rowsDecoded = jpeg_read_scanlines(dinfo, srcRows, rowsPerDecode);
 
         // Convert to RGB if necessary
         if (JCS_CMYK == dinfo->out_color_space) {
-            convert_CMYK_to_RGB(srcRows[0], dstWidth * rowsDecoded);
+            convert_CMYK_to_RGB(srcRows[0], fSrcRowBytes * rowsDecoded);
         }
 
         // Swizzle to output destination
         for (uint32_t i = 0; i < rowsDecoded; i++) {
-            swizzler->next(srcRows[i]);
+            fSwizzler->next(srcRows[i]);
         }
 
         // If we cannot read enough rows, assume the input is incomplete
         if (rowsDecoded < rowsPerDecode && y + rowsDecoded < dstHeight) {
             // Fill the remainder of the image with black. This error handling
             // behavior is unspecified but SkCodec consistently uses black as
             // the fill color for opaque images.  If the destination is kGray,
             // the low 8 bits of SK_ColorBLACK will be used.  Conveniently,
             // these are zeros, which is the representation for black in kGray.
-            SkSwizzler::Fill(swizzler->getDstRow(), dstInfo, dstRowBytes,
+            SkSwizzler::Fill(fSwizzler->getDstRow(), dstInfo, dstRowBytes,
                     dstHeight - y - rowsDecoded, SK_ColorBLACK, NULL);
 
             // Prevent libjpeg from failing on incomplete decode
             dinfo->output_scanline = dstHeight;
 
             // Finish the decode and indicate that the input was incomplete.
             jpeg_finish_decompress(dinfo);
             return fDecoderMgr->returnFailure("Incomplete image data", kIncompleteInput);
         }
     }
     jpeg_finish_decompress(dinfo);
 
     return kSuccess;
 }
+
+/*
+ * Enable scanline decoding for jpegs
+ */
+class SkJpegScanlineDecoder : public SkScanlineDecoder {
+public:
+    SkJpegScanlineDecoder(const SkImageInfo& dstInfo, SkJpegCodec* codec)
+        : INHERITED(dstInfo)
+        , fCodec(codec)
+    {
+        fStorage.reset(fCodec->fSrcRowBytes);
+        fSrcRow = static_cast<uint8_t*>(fStorage.get());
+    }
+
+    SkImageGenerator::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", SkImageGenerator::kInvalidInput);
+        }
+
+        // Read rows one at a time
+        for (int y = 0; y < count; y++) {
+            // Read row of the image
+            uint32_t rowsDecoded = jpeg_read_scanlines(fCodec->fDecoderMgr->dinfo(), &fSrcRow, 1);

[msarett, 2015/04/22 19:52:21]

Here we intentionally decode rows one at a time.  In the regular decoder, we
decode rows in chunks of 1, 2, or 4 depending on a recommendation from the
decoder.

I chose one at a time because it's simpler to implement and **I think** it fits
better with the goal of the scanline decoder.  But we have the option to change
this to be more similar to the original decoder.

+            if (rowsDecoded != 1) {
+                SkSwizzler::Fill(dst, this->dstInfo(), rowBytes, count - y, SK_ColorBLACK, NULL);
+                return SkImageGenerator::kIncompleteInput;
+            }
+
+            // Convert to RGB if necessary
+            if (JCS_CMYK == fCodec->fDecoderMgr->dinfo()->out_color_space) {
+                convert_CMYK_to_RGB(fSrcRow, fCodec->fSrcRowBytes);
+            }
+
+            // Swizzle to output destination
+            fCodec->fSwizzler->setDstRow(dst);
+            fCodec->fSwizzler->next(fSrcRow);
+            dst = SkTAddOffset<void>(dst, rowBytes);
+        }
+
+        return SkImageGenerator::kSuccess;
+    }
+
+    SkImageGenerator::Result onSkipScanlines(int count) override {
+        // Set the jump location for libjpeg errors
+        if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
+            return fCodec->fDecoderMgr->returnFailure("setjmp", SkImageGenerator::kInvalidInput);
+        }
+
+        // Read rows but ignore the output
+        for (int y = 0; y < count; y++) {
+            jpeg_read_scanlines(fCodec->fDecoderMgr->dinfo(), &fSrcRow, 1);

[msarett, 2015/04/22 19:52:21]

AFAICT there is not a skip function in libjpeg.

We could read all the rows at once to skip them rather than skip them one at a
time.  The drawback is that we would have to allocate a 2D array of memory large
enough for count rows in order to read them all at once.

I chose this implementation because it seems simpler.  Let me know if you feel
that we should go the other way.

[scroggo, 2015/04/24 13:14:28]

This seems fine to me, until/unless we discover it is a problem. I am also not
aware of a way to skip in libjpeg (and SkImageDecoder_libjpeg also behaves like
this).

+        }
+
+        return SkImageGenerator::kSuccess;
+    }
+
+    void onFinish() override {
+        if (setjmp(fCodec->fDecoderMgr->getJmpBuf())) {
+            SkCodecPrintf("setjmp: Error in libjpeg finish_decompress\n");
+            return;
+        }
+
+        jpeg_finish_decompress(fCodec->fDecoderMgr->dinfo());
+    }
+
+private:
+    SkJpegCodec*        fCodec;     // unowned
+    SkAutoMalloc        fStorage;
+    uint8_t*            fSrcRow;

[scroggo, 2015/04/24 13:14:28]

// pointer into fStorage

[msarett, 2015/04/24 15:08:44]

Done.

+
+    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");
+        return NULL;
+    }
+
+    // Set the jump location for libjpeg errors
+    if (setjmp(fDecoderMgr->getJmpBuf())) {
+        SkCodecPrintf("setjmp: Error from libjpeg\n");
+        return NULL;
+    }
+
+    // Check if we can decode to the requested destination
+    if (!conversion_possible(dstInfo, this->getInfo())) {
+        SkCodecPrintf("Cannot convert to output type\n");
+        return NULL;
+    }
+
+    // Perform the necessary scaling
+    if (!this->handleScaling(dstInfo)) {
+        SkCodecPrintf("Cannot scale ot output dimensions\n");
+        return NULL;
+    }
+
+    // Now, given valid output dimensions, we can start the decompress
+    if (!jpeg_start_decompress(fDecoderMgr->dinfo())) {
+        SkCodecPrintf("start decompress failed\n");
+        return NULL;
+    }
+
+    // Create the swizzler
+    this->initializeSwizzler(dstInfo, NULL, dstInfo.minRowBytes(), options);
+    if (NULL == fSwizzler) {
+        SkCodecPrintf("Could not create swizzler\n");
+        return NULL;
+    }
+
+    // Return the new scanline decoder
+    return SkNEW_ARGS(SkJpegScanlineDecoder, (dstInfo, this));
+}