SkJpegCodec

src/codec/SkJpegCodec.cpp

Index: src/codec/SkJpegCodec.cpp
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..bda9fa144f65c4c09597ce0c15d12875145eb3d4
--- /dev/null
+++ b/src/codec/SkJpegCodec.cpp
@@ -0,0 +1,441 @@
+/*
+ * 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 "SkJpegCodec.h"
+
+#include "SkCodec.h"
+#include "SkJpegAutoClean.h"
+#include "SkJpegCodec.h"
+#include "SkJpegUtility.h"
+#include "SkCodecPriv.h"
+#include "SkColorPriv.h"
+#include "SkStream.h"
+#include "SkTemplates.h"
+#include "SkTypes.h"
+#define IDCT_SCALING_SUPPORTED

[scroggo, 2015/04/14 13:10:33]

Is this necessary?

Does it need to be defined before we include these other files? Otherwise,
please place this after.

[msarett, 2015/04/14 19:30:36]

This is a mistake.  I meant to delete this.

+// stdio is needed for jpeglib
+#include <stdio.h>

[scroggo, 2015/04/14 13:10:33]

Again, please keep these separate from the Skia includes.

[msarett, 2015/04/14 19:30:36]

Done.

+#include "jerror.h"
+#include "jpegint.h"
+#include "jpeglib.h"
+
+// ANDROID_RGB
+// If this is defined in the jpeg headers it indicates that jpeg offers
+// support for two additional formats: JCS_RGBA_8888 and JCS_RGB_565.
+
+/*
+ * Choose the size of the memory buffer on Android
+ */
+static void overwrite_mem_buffer_size(jpeg_decompress_struct* dinfo) {
+#ifdef SK_BUILD_FOR_ANDROID
+
+// Use 30 MB for devices with a large amount of system memory and 5MB otherwise
+// TODO: (msarett) This matches SkImageDecoder.  Why were these values chosen?
+#ifdef ANDROID_LARGE_MEMORY_DEVICE
+    dinfo->mem->max_memory_to_use = 30 * 1024 * 1024;
+#else
+    dinfo->mem->max_memory_to_use = 5 * 1024 * 1024;
+#endif
+
+#endif // SK_BUILD_FOR_ANDROID
+}
+
+/*
+ * Print informative error messages
+ */
+static void print_jpeg_decoder_errors(const jpeg_decompress_struct& dinfo, int width,
+        int height, const char caller[]) {
+    char buffer[JMSG_LENGTH_MAX];
+    dinfo.err->format_message((const j_common_ptr) &dinfo, buffer);
+    SkCodecPrintf("libjpeg error %d <%s> from %s [%d %d]\n",
+            dinfo.err->msg_code, buffer, caller, width, height);
+}
+
+static bool return_false(const jpeg_decompress_struct& dinfo, const char caller[]) {
+    print_jpeg_decoder_errors(dinfo, dinfo.output_width, dinfo.output_height, caller);
+    return false;
+}
+
+static SkCodec::Result return_failure(const jpeg_decompress_struct& dinfo, const char caller[],
+        SkCodec::Result result) {
+    print_jpeg_decoder_errors(dinfo, dinfo.output_width, dinfo.output_height, caller);
+    return result;
+}
+
+/*
+ * 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) {
+    // 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.
+    // CMYK -> CMY
+    // C = (1 - C) * (1 - (1 - K)) + (1 - K)
+    // M = (1 - M) * (1 - (1 - K)) + (1 - K)
+    // Y = (1 - Y) * (1 - (1 - K)) + (1 - K)
+    //
+    // Simplifying the above expression.
+    // CMYK -> CMY
+    // C = 1 - CK
+    // M = 1 - MK
+    // Y = 1 - YK
+    //
+    // CMY -> RGB
+    // R = (1 - C) * 255
+    // G = (1 - M) * 255
+    // B = (1 - Y) * 255
+    //
+    // Therefore the full conversion is below.  This can be verified at
+    // www.rapidtables.com (assuming inverted CMYK).
+    // CMYK -> RGB
+    // R = C * K * 255
+    // G = M * K * 255
+    // 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
+    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;
+    }
+}
+
+/*
+ * Get the preferred color type to decode to based on the properties of the jpeg.
+ * Also, inform libjpeg what format to decode to.
+ */
+SkColorType get_color_type(jpeg_decompress_struct* dinfo) {
+    SkASSERT(dinfo != NULL);
+
+    switch (dinfo->jpeg_color_space) {
+        case JCS_CMYK:
+        case JCS_YCCK:
+            // libjpeg cannot convert from CMYK or YCCK to RGB.
+            // Here, we ask libjpeg to give us CMYK samples back and
+            // we will later manually convert them to RGB.
+            dinfo->out_color_space = JCS_CMYK;
+            return kN32_SkColorType;
+        case JCS_GRAYSCALE:
+            dinfo->out_color_space = JCS_GRAYSCALE;
+            return kGray_8_SkColorType;
+        default:
+#ifdef ANDROID_RGB
+            dinfo->out_color_space = JCS_RGBA_8888;
+#else
+            dinfo->out_color_space = JCS_RGB;
+#endif
+            return kN32_SkColorType;
+    }
+}
+
+/*
+ * Get the config and bytes per pixel of the source data.
+ * Return whether the data is supported.
+ */
+static SkSwizzler::SrcConfig get_src_config(const jpeg_decompress_struct* dinfo) {
+    // We will manually convert CMYK to RGB
+    if (JCS_CMYK == dinfo->out_color_space) {
+        return SkSwizzler::kRGBX;
+    }
+
+    // RGB
+    if (3 == dinfo->out_color_components && JCS_RGB == dinfo->out_color_space) {
+        return SkSwizzler::kRGB;
+    }
+
+    // RGB for Android
+#ifdef ANDROID_RGB
+    if (JCS_RGBA_8888 == dinfo->out_color_space) {
+        return SkSwizzler::kRGBX;
+    }
+    if (JCS_RGB_565 == dinfo->out_color_space) {
+        return SkSwizzler::kRGB_565;
+    }
+#endif
+
+    // Grayscale
+    if (1 == dinfo->out_color_components && JCS_GRAYSCALE == dinfo->out_color_space) {
+        return SkSwizzler::kGray;
+    }
+
+    // Indicate that we cannot find a matching config
+    return SkSwizzler::kUnknown;

[scroggo, 2015/04/14 13:10:32]

I'm assuming CreateSwizzler will fail for kUnknown?

[msarett, 2015/04/14 19:30:36]

Yes it will return NULL.  And we do a NULL check on the return value from
CreateSwizzler.

+}
+
+bool SkJpegCodec::IsJpeg(SkStream* stream) {
+    static const uint8_t jpegSig[] = { 0xFF, 0xD8, 0xFF };
+    char buffer[sizeof(jpegSig)];
+    return stream->read(buffer, sizeof(jpegSig)) == sizeof(jpegSig) &&
+            !memcmp(buffer, jpegSig, sizeof(jpegSig));
+}
+
+bool SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut,
+        JpegAutoClean** decodeMgrOut) {
+    // The decompress info struct holds all of the information related to the decode
+    jpeg_decompress_struct* dinfo = SkNEW(jpeg_decompress_struct);

[scroggo, 2015/04/14 13:10:32]

Here is what I would do. Make dinfo a member of JpegAutoClean. Here, you'll
have:

SkAutoTDelete<JpegAutoClean> jpegAutoClean(SkNEW(JpegAutoClean);

It might be better named JpegDecoderMgr or something, since we're no longer
using it as an autodeleter.

You'll always use the difno from this object, rather than passing it to this
object.

[msarett, 2015/04/14 19:30:36]

Done.

+
+    // The source manager handles the input stream for libjpeg
+    skjpeg_source_mgr* srcMgr = SkNEW_ARGS(skjpeg_source_mgr, (stream));
+
+    // Set up the error manager.  This must be set before the call to jpeg_create_decompress
+    // in case there is a failure in initializing memory in libjpeg.
+    skjpeg_error_mgr* errorMgr = SkNEW(skjpeg_error_mgr);
+    dinfo->err = jpeg_std_error(errorMgr);
+    errorMgr->error_exit = skjpeg_err_exit;
+
+    // Use a containter to automatically free memory.
+    SkAutoTDelete<JpegAutoClean> decodeMgr(SkNEW_ARGS(JpegAutoClean, (dinfo, srcMgr, errorMgr)));
+
+    // All objects need to be instantiated before this setjmp call so that
+    // they will be cleaned up properly if an error occurs.
+    if (setjmp(errorMgr->fJmpBuf)) {
+        return return_false(*dinfo, "setjmp");
+    }
+
+    // Initialize the jpeg info struct to prepare to decode
+    jpeg_create_decompress(dinfo);
+    dinfo->src = srcMgr;
+    overwrite_mem_buffer_size(dinfo);
+
+    // Read the jpeg header
+    int status = jpeg_read_header(dinfo, true);
+    if (status != JPEG_HEADER_OK) {
+        return return_false(*dinfo, "read_header");
+    }
+
+    // Choose a method of performing the discrete cosine transform
+#ifdef DCT_IFAST_SUPPORTED

[msarett, 2015/04/13 20:54:06]

Do we want to use this?

DCT_IFAST is faster but less accurate.  AFAICT this is similar to the scaling
define, where it seems like it should be defined but it is always not defined.

[scroggo, 2015/04/14 13:10:33]

I've been following a bug where people are complaining that IFAST is uglier.
I'll send you the bug once I find it, but I think we do *not* want to use it.

[msarett, 2015/04/14 19:30:36]

Cool that will simplify things.  AFAICT we have the same issue with
DCT_IFAST_SUPPORTED where it is not defined when it seems that it should be.

[scroggo, 2015/04/15 00:31:05]

FYI: The bug I referenced is here:
https://code.google.com/p/chromium/issues/detail?id=385515

+    dinfo->dct_method = JDCT_IFAST;
+#else
+    dinfo->dct_method = JDCT_ISLOW;
+#endif
+
+    if (NULL != codecOut) {
+        // Recommend the color type to decode to
+        const SkColorType colorType = get_color_type(dinfo);
+
+        // Create image info object and the codec
+        const SkImageInfo& imageInfo = SkImageInfo::Make(dinfo->image_width,
+                dinfo->image_height, colorType, kOpaque_SkAlphaType);
+        *codecOut = SkNEW_ARGS(SkJpegCodec, (imageInfo, stream, decodeMgr.detach()));
+    } else {
+        SkASSERT(NULL != decodeMgrOut);
+        *decodeMgrOut = decodeMgr.detach();
+    }
+    return true;
+}
+
+SkCodec* SkJpegCodec::NewFromStream(SkStream* stream) {
+    SkAutoTDelete<SkStream> streamDeleter(stream);
+    SkCodec* codec = NULL;
+    if (ReadHeader(stream,  &codec, NULL)) {
+        // Codec has taken ownership of the stream, we do not need to delete it
+        SkASSERT(codec);
+        streamDeleter.detach();
+        return codec;
+    }
+    return NULL;
+}
+
+SkJpegCodec::SkJpegCodec(const SkImageInfo& srcInfo, SkStream* stream,
+        JpegAutoClean* decodeMgr)
+    : INHERITED(srcInfo, stream)
+    , fDecodeMgr(decodeMgr)
+{}
+
+/*
+ * Divide and round up
+ */
+long div_round_up (long a, long b) {
+    return (a + b - 1L) / b;
+}
+
+/*
+ * Return a valid set of output dimensions for this decoder, given an input scale
+ */
+SkISize SkJpegCodec::onGetScaledDimensions(float desiredScale) const {

[msarett, 2015/04/13 20:54:06]

This function is not in a finished state, but I didn't want to put any more time
into it until we discuss it further.

(1) I no longer for check for IDCT_SCALE_SUPPORTED.  I included an additional
file which made IDCT_SCALE_SUPPORTED always defined instead of always undefined.
 And then I realized that if it will always be defined, there is no point in
checking.

(2) I don't think I can use jcalc_output_dimensions().  The problem is that
jpeglib will error exit if jcalc_output_dimensions() is called before fDecodeMgr
is reset.  So if we try to call getScaledDimensions() after calling getPixels()
it will fail (the new test exposes this).  We also cannot call rewindIfNeeded()
here because of the const-ness of this function.  Regardless of if we were to
figure out a way to rewind in this function, I feel that calling this function
should not require rewinding.  I think the best solution is to use
jdiv_round_up().

(3) I have no idea why the linker thinks that jdiv_round_up is declared but not
defined.  The identical helper function that I have created is a temporary
workaround.

[scroggo, 2015/04/14 13:10:32]

It looks like you defined it yourself at the top of the file?
Agreed that we should not have to rewind here. I'm not clear on how we ever get
into a situation where we error exit.

const_cast allows you to modify const variables, if you needed to modify the
scale_denom.
Me neither. I'd like to better understand why we cannot use cal_output_dim
before investigating this further.

[msarett, 2015/04/14 19:30:36]

Yeah I forgot to remove the define at the top of the file.

I honestly prefer the jdiv approach to the calc_output_dimensions approach.  You
will notice that I was able to get calc_output_dimensions to work by setting a
few extra fields, but it's a bit hacky.  If num_components is not 0, it will seg
fault when trying to iterate over "components".

That said, I'm equally happy to not change it again :).

[scroggo, 2015/04/15 00:31:05]

It seems like both are a bit hacky:

Option 1:
Copy the code that calc_output_dimensions uses, and hope that it never changes.
Maybe in practice it doesn't. Also, we're in control of the version of libjpeg
used by Chrome and Android, so we ought to find out if something changed by
running our tests - unless, of course, it works the same for the numbers we
test, but not for other sizes.

My main concern about things changing is if we're suddenly reporting off by one,
when the caller asks for a scaled version, we'll just fail completely (returning
kInvalidScale).

Option 2:
Build a fake dinfo. This seems just as dangerous, in terms of relying on libjpeg
internals, but maybe if something about this changes we'll have a more obvious
failure (possibly a crash or failure to compile)?

[msarett, 2015/04/15 12:43:11]

I agree that it is unfortunate that both approaches rely on libjpeg internals.

Option 2 sounds good!  I would rather debug the more obvious failure.

+    // 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;
+    } else if (desiredScale > 0.375f) {
+        scale = 2;
+    } else if (desiredScale > 0.1875f) {
+        scale = 4;
+    } else {
+        scale = 8;
+    }
+
+    // Return the calculated output dimensions for the given scale
+    fDecodeMgr->dinfo()->scale_denom = 1;
+    return SkISize::Make(div_round_up((long) fDecodeMgr->dinfo()->image_width, scale),
+                         div_round_up((long) fDecodeMgr->dinfo()->image_height, scale));
+}
+
+/*
+ * Checks if the conversion between the input image and the requested output
+ * image has been implemented
+ */
+static bool conversion_possible(const SkImageInfo& dst,
+                                const SkImageInfo& src) {
+    // Ensure that the profile type is unchanged
+    if (dst.profileType() != src.profileType()) {
+        return false;
+    }
+
+    // Ensure that the alpha type is opaque
+    if (kOpaque_SkAlphaType != dst.alphaType()) {
+        return false;
+    }
+
+    // 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();
+}
+
+/*
+ * Initiates 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) {
+        JpegAutoClean* decodeMgr = NULL;
+        if (!ReadHeader(this->stream(), NULL, &decodeMgr)) {
+            return kCouldNotRewind;
+        }
+        SkASSERT(NULL != decodeMgr);
+        fDecodeMgr.reset(decodeMgr);
+    }
+
+    // Get the decompress info pointer
+    jpeg_decompress_struct* dinfo = fDecodeMgr->dinfo();
+
+    // Set a new jump location for libjpeg errors
+    skjpeg_error_mgr* errorMgr = (skjpeg_error_mgr*) dinfo->err;
+    if (setjmp(errorMgr->fJmpBuf)) {
+        return return_failure(*dinfo, "setjmp", kIncompleteInput);
+    }
+
+    // Check if we can decode to the requested destination
+    if (!conversion_possible(dstInfo, this->getInfo())) {
+        return return_failure(*dinfo, "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 return_failure(*dinfo, "cannot scale to requested dimensions", kInvalidScale);
+        }
+
+        // Try the next scale
+        dinfo->scale_denom *= 2;
+        jpeg_calc_output_dimensions(dinfo);
+    }
+
+    // Now, given valid output dimensions, we can start the decompress
+    if (!jpeg_start_decompress(dinfo)) {
+        return return_failure(*dinfo, "start_decompress", kInvalidInput);
+    }
+
+    // Create the swizzler
+    const SkSwizzler::SrcConfig srcConfig = get_src_config(dinfo);
+    SkAutoTDelete<SkSwizzler> swizzler(SkSwizzler::CreateSwizzler(srcConfig, NULL, dstInfo,
+            dst, dstRowBytes, options.fZeroInitialized));
+    if (NULL == swizzler) {
+        return return_failure(*dinfo, "CreateSwizzler", kInvalidInput);
+    }
+    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));
+    JSAMPLE* srcRows[4];
+    uint8_t* srcPtr = srcBuffer.get();
+    for (uint8_t i = 0; i < rowsPerDecode; i++) {
+        srcRows[i] = (JSAMPLE*) srcPtr;
+        srcPtr += srcRowBytes;
+    }
+
+    // Ensure that we loop enough times to decode all of the rows
+    // libjpeg will prevent us from reading past the bottom of the image
+    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);
+        }
+
+        // Swizzle to output destination
+        for (uint32_t i = 0; i < rowsDecoded; i++) {
+            swizzler->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.
+            SkSwizzler::Fill(swizzler->getDstRow(), dstInfo, dstRowBytes,
+                    dstHeight - y - rowsDecoded, SK_ColorBLACK, NULL);
+
+            // Suppress incomplete input message from libjpeg
+            // We will report our own warnings with SkCodecPrintf
+            dinfo->output_scanline = dstHeight;
+
+            // Finish the decode and indicate that the input was incomplete
+            jpeg_finish_decompress(dinfo);
+            return return_failure(*dinfo, "Incomplete image data", kIncompleteInput);
+        }
+    }
+    jpeg_finish_decompress(dinfo);
+
+    return kSuccess;
+}