JPEG YUV Decoding

src/images/SkImageDecoder_libjpeg.cpp

Index: src/images/SkImageDecoder_libjpeg.cpp
diff --git a/src/images/SkImageDecoder_libjpeg.cpp b/src/images/SkImageDecoder_libjpeg.cpp
index 99401e6b627cdc9fd023b304a6c40789c3fbec3d..101cecec9bc164afa06656d4b989ee400f26baab 100644
--- a/src/images/SkImageDecoder_libjpeg.cpp
+++ b/src/images/SkImageDecoder_libjpeg.cpp
@@ -54,6 +54,22 @@ SK_CONF_DECLARE(bool, c_suppressJPEGImageDecoderErrors,
                 "Suppress most JPG error messages when decode "
                 "function fails.");
 
+// Enum for YUV decoding

[scroggo, 2014/10/15 15:16:06]

Could these enums be moved closer to where they are used?

[sugoi1, 2014/10/15 17:47:43]

Done.

+enum YUVSubsampling {
+    kUNKNOWN_YUVSubsampling,
+    k410_YUVSubsampling,
+    k411_YUVSubsampling,
+    k420_YUVSubsampling,
+    k422_YUVSubsampling,
+    k440_YUVSubsampling,
+    k444_YUVSubsampling
+};
+
+enum SizeType {
+    kSizeForMemoryAllocation_SizeType,
+    kActualSize_SizeType
+};
+
 //////////////////////////////////////////////////////////////////////////
 //////////////////////////////////////////////////////////////////////////
 
@@ -102,6 +118,66 @@ static void initialize_info(jpeg_decompress_struct* cinfo, skjpeg_source_mgr* sr
     }
 }
 
+static SkISize compute_yuv_size(const jpeg_decompress_struct& info, int component,

[scroggo, 2014/10/15 15:16:05]

Again, these functions appear to be defined well before they are used. Is that
necessary?

[sugoi1, 2014/10/15 17:47:43]

Moved them lower.

+                                SizeType sizeType)
+{

[scroggo, 2014/10/15 15:16:05]

style nit: open brace does not get its own line.

[sugoi1, 2014/10/15 17:47:44]

Done.

+    if (sizeType == kSizeForMemoryAllocation_SizeType) {
+        return SkISize::Make(info.cur_comp_info[component]->width_in_blocks * DCTSIZE,
+                             info.cur_comp_info[component]->height_in_blocks * DCTSIZE);
+    }
+    return SkISize::Make(info.cur_comp_info[component]->downsampled_width,
+                         info.cur_comp_info[component]->downsampled_height);
+}
+
+static YUVSubsampling yuv_subsampling(const jpeg_decompress_struct& info)
+{
+    if ((DCTSIZE == 8)
+        && (info.num_components == 3)
+        && (info.comps_in_scan >= info.num_components)
+        && (info.scale_denom <= 8)
+        && (info.cur_comp_info[0])
+        && (info.cur_comp_info[1])
+        && (info.cur_comp_info[2])
+        && (info.cur_comp_info[1]->h_samp_factor == 1)
+        && (info.cur_comp_info[1]->v_samp_factor == 1)
+        && (info.cur_comp_info[2]->h_samp_factor == 1)
+        && (info.cur_comp_info[2]->v_samp_factor == 1)) {

[scroggo, 2014/10/15 15:16:06]

In this case, I think the open brace should go on its own line (not yet in the
style guide, but I think it has unofficial approval), or the if condition's
indentation should not match the function's.

[sugoi1, 2014/10/15 17:47:43]

Done.

+        int h = info.cur_comp_info[0]->h_samp_factor;
+        int v = info.cur_comp_info[0]->v_samp_factor;
+        // 4:4:4 : (h == 1) && (v == 1)
+        // 4:4:0 : (h == 1) && (v == 2)
+        // 4:2:2 : (h == 2) && (v == 1)
+        // 4:2:0 : (h == 2) && (v == 2)
+        // 4:1:1 : (h == 4) && (v == 1)
+        // 4:1:0 : (h == 4) && (v == 2)
+        if (v == 1) {
+            switch (h) {
+            case 1:

[scroggo, 2014/10/15 15:16:06]

I think we don't match blink and/or chrome, but we indent 'case' once more from
'switch'.

[sugoi1, 2014/10/15 17:47:43]

Done.

+                return k444_YUVSubsampling;
+            case 2:
+                return k422_YUVSubsampling;
+            case 4:
+                return k411_YUVSubsampling;
+            default:
+                break;
+            }
+        } else if (v == 2) {
+            switch (h) {
+            case 1:
+                return k440_YUVSubsampling;
+            case 2:
+                return k420_YUVSubsampling;
+            case 4:
+                return k410_YUVSubsampling;
+            default:
+                break;
+            }
+        }
+    }
+
+    return kUNKNOWN_YUVSubsampling;
+}
+
 #ifdef SK_BUILD_FOR_ANDROID
 class SkJPEGImageIndex {
 public:
@@ -239,6 +315,9 @@ protected:
     virtual bool onDecodeSubset(SkBitmap* bitmap, const SkIRect& rect) SK_OVERRIDE;
 #endif
     virtual bool onDecode(SkStream* stream, SkBitmap* bm, Mode) SK_OVERRIDE;
+    virtual bool onDecodeYUV8Planes(SkStream* stream, SkISize componentSizes[3],
+                                    void* planes[3], size_t rowBytes[3],
+                                    SkYUVColorSpace* colorSpace) SK_OVERRIDE;
 
 private:
 #ifdef SK_BUILD_FOR_ANDROID
@@ -325,16 +404,21 @@ static bool skip_src_rows_tile(jpeg_decompress_struct* cinfo,
 // This guy exists just to aid in debugging, as it allows debuggers to just
 // set a break-point in one place to see all error exists.
 static bool return_false(const jpeg_decompress_struct& cinfo,
-                         const SkBitmap& bm, const char caller[]) {
+                         int width, int height, const char caller[]) {
     if (!(c_suppressJPEGImageDecoderErrors)) {
         char buffer[JMSG_LENGTH_MAX];
         cinfo.err->format_message((const j_common_ptr)&cinfo, buffer);
         SkDebugf("libjpeg error %d <%s> from %s [%d %d]\n",
-                 cinfo.err->msg_code, buffer, caller, bm.width(), bm.height());
+                 cinfo.err->msg_code, buffer, caller, width, height);
     }
     return false;   // must always return false
 }
 
+static bool return_false(const jpeg_decompress_struct& cinfo,
+                         const SkBitmap& bm, const char caller[]) {
+    return return_false(cinfo, bm.width(), bm.height(), caller);
+}
+
 // Convert a scanline of CMYK samples to RGBX in place. Note that this
 // method moves the "scanline" pointer in its processing
 static void convert_CMYK_to_RGB(uint8_t* scanline, unsigned int width) {
@@ -726,6 +810,171 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
     return true;
 }
 
+static void update_components_sizes(const jpeg_decompress_struct& cinfo, SkISize componentSizes[3],
+                                    SizeType sizeType) {
+    for (int i = 0; i < 3; ++i) {
+        componentSizes[i] = compute_yuv_size(cinfo, i, sizeType);
+    }
+}
+
+static bool output_raw_data(jpeg_decompress_struct& cinfo, void* planes[3], size_t rowBytes[3]) {
+	// U size and V size have to be the same if we're calling output_raw_data()
+	SkISize uvSize = compute_yuv_size(cinfo, 1, kSizeForMemoryAllocation_SizeType);

[scroggo, 2014/10/15 15:16:05]

nit: indentation (first two lines)

[sugoi1, 2014/10/15 17:47:43]

Oops, there were tabs in there. Removed.

+    SkASSERT(uvSize == compute_yuv_size(cinfo, 2, kSizeForMemoryAllocation_SizeType));
+
+    JSAMPARRAY bufferraw[3];
+    JSAMPROW bufferraw2[32];
+    bufferraw[0] = &bufferraw2[0]; // Y channel rows (8 or 16)
+    bufferraw[1] = &bufferraw2[16]; // U channel rows (8)
+    bufferraw[2] = &bufferraw2[24]; // V channel rows (8)
+    int yWidth = cinfo.output_width;
+    int yHeight = cinfo.output_height;
+    int yMaxH = yHeight - 1;
+    int v = cinfo.cur_comp_info[0]->v_samp_factor;
+    int uvMaxH = uvSize.height() - 1;
+    JSAMPROW outputY = static_cast<JSAMPROW>(planes[0]);
+    JSAMPROW outputU = static_cast<JSAMPROW>(planes[1]);
+    JSAMPROW outputV = static_cast<JSAMPROW>(planes[2]);
+    size_t rowBytesY = rowBytes[0];
+    size_t rowBytesU = rowBytes[1];
+    size_t rowBytesV = rowBytes[2];
+
+    int yScanlinesToRead = DCTSIZE * v;
+    SkAutoMalloc lastRowStorage(yWidth * 8);
+    JSAMPROW yLastRow = (JSAMPROW)lastRowStorage.get();
+    JSAMPROW uLastRow = yLastRow + 2 * yWidth;
+    JSAMPROW vLastRow = uLastRow + 2 * yWidth;
+    JSAMPROW dummyRow = vLastRow + 2 * yWidth;
+
+    while (cinfo.output_scanline < cinfo.output_height) {
+        // Request 8 or 16 scanlines: returns 0 or more scanlines.
+        bool hasYLastRow(false), hasUVLastRow(false);
+        // Assign 8 or 16 rows of memory to read the Y channel.
+        for (int i = 0; i < yScanlinesToRead; ++i) {
+            int scanline = (cinfo.output_scanline + i);
+            if (scanline < yMaxH) {
+                bufferraw2[i] = &outputY[scanline * rowBytesY];
+            } else if (scanline == yMaxH) {
+                bufferraw2[i] = yLastRow;
+                hasYLastRow = true;
+            } else {
+                bufferraw2[i] = dummyRow;
+            }
+        }
+        int scaledScanline = cinfo.output_scanline / v;
+        // Assign 8 rows of memory to read the U and V channels.
+        for (int i = 0; i < 8; ++i) {
+            int scanline = (scaledScanline + i);
+            if (scanline < uvMaxH) {
+                bufferraw2[16 + i] = &outputU[scanline * rowBytesU];
+                bufferraw2[24 + i] = &outputV[scanline * rowBytesV];
+            } else if (scanline == uvMaxH) {
+                bufferraw2[16 + i] = uLastRow;
+                bufferraw2[24 + i] = vLastRow;
+                hasUVLastRow = true;
+            } else {
+                bufferraw2[16 + i] = dummyRow;
+                bufferraw2[24 + i] = dummyRow;
+            }
+        }
+        JDIMENSION scanlinesRead = jpeg_read_raw_data(&cinfo, bufferraw, yScanlinesToRead);
+
+        if (scanlinesRead == 0)
+            return false;
+
+        if (hasYLastRow) {
+            memcpy(&outputY[yMaxH * rowBytesY], yLastRow, yWidth);
+        }
+        if (hasUVLastRow) {
+            memcpy(&outputU[uvMaxH * rowBytesU], uLastRow, uvSize.width());
+            memcpy(&outputV[uvMaxH * rowBytesV], vLastRow, uvSize.width());
+        }
+    }
+
+    cinfo.output_scanline = SkMin32(cinfo.output_scanline, cinfo.output_height);
+
+    return true;
+}
+
+bool SkJPEGImageDecoder::onDecodeYUV8Planes(SkStream* stream, SkISize componentSizes[3],
+                                            void* planes[3], size_t rowBytes[3],
+                                            SkYUVColorSpace* colorSpace) {
+#ifdef TIME_DECODE
+    SkAutoTime atm("JPEG Decode");

[scroggo, 2014/10/15 15:16:06]

Not sure if anyone uses TIME_DECODE, but shouldn't the string specify that it is
for decoding YUV8Planes?

[sugoi1, 2014/10/15 17:47:43]

Done.

+#endif
+
+    if (this->getSampleSize() != 1) {
+        return false; // Resizing not supported

[scroggo, 2014/10/15 15:16:05]

Is there a way we could let the client know this?

[sugoi1, 2014/10/15 17:47:43]

Well, it's not that it's impossible, it's just not supported yet. It could be
implemented. Worst case scenario here is that it will revert back to standard
RGBA decoding, which isn't catastrophic (It's the current case).

+    }
+
+    JPEGAutoClean autoClean;
+
+    jpeg_decompress_struct  cinfo;
+    skjpeg_source_mgr       srcManager(stream, this);
+
+    skjpeg_error_mgr errorManager;
+    set_error_mgr(&cinfo, &errorManager);
+
+    // All objects need to be instantiated before this setjmp call so that
+    // they will be cleaned up properly if an error occurs.
+    if (setjmp(errorManager.fJmpBuf)) {
+        return return_false(cinfo, 0, 0, "setjmp");
+    }
+
+    initialize_info(&cinfo, &srcManager);
+    autoClean.set(&cinfo);
+
+    int status = jpeg_read_header(&cinfo, true);
+    if (status != JPEG_HEADER_OK) {
+        return return_false(cinfo, 0, 0, "read_header");
+    }
+
+    if (cinfo.jpeg_color_space != JCS_YCbCr) {
+        // It's not an error to not be encoded in YUV, so no need to use return_false()
+        return false;
+    }
+
+    cinfo.out_color_space = JCS_YCbCr;
+    cinfo.raw_data_out = TRUE;
+
+    if (!planes || !planes[0] || !rowBytes || !rowBytes[0]) { // Compute size only
+        update_components_sizes(cinfo, componentSizes, kSizeForMemoryAllocation_SizeType);
+        return true;
+    }
+
+    set_dct_method(*this, &cinfo);
+
+    SkASSERT(1 == cinfo.scale_num);
+    cinfo.scale_denom = 1;
+
+    turn_off_visual_optimizations(&cinfo);
+
+#ifdef ANDROID_RGB
+    cinfo.dither_mode = JDITHER_NONE;
+#endif
+
+    /*  image_width and image_height are the original dimensions, available

[scroggo, 2014/10/15 15:16:06]

This comment appears to have been copied word for word, but does not apply here.

[sugoi1, 2014/10/15 17:47:43]

Actually, it does apply, output_width and output_height will be read from cinfo
inside output_raw_data (see yWidth, yHeight), so jpeg_start_decompress() has to
be executed before output_raw_data().

+        after jpeg_read_header(). To see the scaled dimensions, we have to call
+        jpeg_start_decompress(), and then read output_width and output_height.
+    */
+    if (!jpeg_start_decompress(&cinfo)) {
+        return return_false(cinfo, 0, 0, "start_decompress");
+    }
+
+    if (!output_raw_data(cinfo, planes, rowBytes)) {
+        return return_false(cinfo, 0, 0, "output_raw_data");
+    }
+
+    update_components_sizes(cinfo, componentSizes, kActualSize_SizeType);
+    jpeg_finish_decompress(&cinfo);
+
+    if (NULL != colorSpace) {

[reed1, 2014/10/15 15:08:08]

Does libjpeg actually support different spaces? If so, lets add a TODO.

// TODO: sniff the data (in cinfo?) to set the correct colorspace)

[sugoi1, 2014/10/15 17:47:44]

Not that I know of. AFAIK, JPEG always uses the same YUV coefficients.

+        *colorSpace = kJPEG_SkYUVColorSpace;
+    }
+
+    return true;
+}
+
 #ifdef SK_BUILD_FOR_ANDROID
 bool SkJPEGImageDecoder::onBuildTileIndex(SkStreamRewindable* stream, int *width, int *height) {