From f0a9d82bf7a6137c69f47a959708351e93e7b8c2 Mon Sep 17 00:00:00 2001 From: Lovell Fuller Date: Sat, 7 Jun 2014 22:49:15 +0100 Subject: [PATCH] Always convert to sRGB colourspace to prevent libwebp segfault #58 --- src/sharp.cc | 35 ++++++++++++++++++++--------------- tests/perf.js | 35 ----------------------------------- tests/unit.js | 6 ++++++ 3 files changed, 26 insertions(+), 50 deletions(-) diff --git a/src/sharp.cc b/src/sharp.cc index 459d786f..3ef87108 100755 --- a/src/sharp.cc +++ b/src/sharp.cc @@ -359,46 +359,51 @@ class ResizeWorker : public NanAsyncWorker { } g_object_unref(canvased); + // Always convert to sRGB colour space + VipsImage *colourspaced = vips_image_new(); + vips_colourspace(sharpened, &colourspaced, VIPS_INTERPRETATION_sRGB, NULL); + g_object_unref(sharpened); + // Output if (baton->file_out == "__jpeg" || (baton->file_out == "__input" && inputImageType == JPEG)) { // Write JPEG to buffer - if (vips_jpegsave_buffer(sharpened, &baton->buffer_out, &baton->buffer_out_len, "strip", TRUE, "Q", baton->quality, "optimize_coding", TRUE, "interlace", baton->progressive, NULL)) { - return resize_error(baton, sharpened); + if (vips_jpegsave_buffer(colourspaced, &baton->buffer_out, &baton->buffer_out_len, "strip", TRUE, "Q", baton->quality, "optimize_coding", TRUE, "interlace", baton->progressive, NULL)) { + return resize_error(baton, colourspaced); } } else if (baton->file_out == "__png" || (baton->file_out == "__input" && inputImageType == PNG)) { // Write PNG to buffer - if (vips_pngsave_buffer(sharpened, &baton->buffer_out, &baton->buffer_out_len, "strip", TRUE, "compression", baton->compressionLevel, "interlace", baton->progressive, NULL)) { - return resize_error(baton, sharpened); + if (vips_pngsave_buffer(colourspaced, &baton->buffer_out, &baton->buffer_out_len, "strip", TRUE, "compression", baton->compressionLevel, "interlace", baton->progressive, NULL)) { + return resize_error(baton, colourspaced); } } else if (baton->file_out == "__webp" || (baton->file_out == "__input" && inputImageType == WEBP)) { // Write WEBP to buffer - if (vips_webpsave_buffer(sharpened, &baton->buffer_out, &baton->buffer_out_len, "strip", TRUE, "Q", baton->quality, NULL)) { - return resize_error(baton, sharpened); + if (vips_webpsave_buffer(colourspaced, &baton->buffer_out, &baton->buffer_out_len, "strip", TRUE, "Q", baton->quality, NULL)) { + return resize_error(baton, colourspaced); } } else if (is_jpeg(baton->file_out)) { // Write JPEG to file - if (vips_jpegsave(sharpened, baton->file_out.c_str(), "strip", TRUE, "Q", baton->quality, "optimize_coding", TRUE, "interlace", baton->progressive, NULL)) { - return resize_error(baton, sharpened); + if (vips_jpegsave(colourspaced, baton->file_out.c_str(), "strip", TRUE, "Q", baton->quality, "optimize_coding", TRUE, "interlace", baton->progressive, NULL)) { + return resize_error(baton, colourspaced); } } else if (is_png(baton->file_out)) { // Write PNG to file - if (vips_pngsave(sharpened, baton->file_out.c_str(), "strip", TRUE, "compression", baton->compressionLevel, "interlace", baton->progressive, NULL)) { - return resize_error(baton, sharpened); + if (vips_pngsave(colourspaced, baton->file_out.c_str(), "strip", TRUE, "compression", baton->compressionLevel, "interlace", baton->progressive, NULL)) { + return resize_error(baton, colourspaced); } } else if (is_webp(baton->file_out)) { // Write WEBP to file - if (vips_webpsave(sharpened, baton->file_out.c_str(), "strip", TRUE, "Q", baton->quality, NULL)) { - return resize_error(baton, sharpened); + if (vips_webpsave(colourspaced, baton->file_out.c_str(), "strip", TRUE, "Q", baton->quality, NULL)) { + return resize_error(baton, colourspaced); } } else if (is_tiff(baton->file_out)) { // Write TIFF to file - if (vips_tiffsave(sharpened, baton->file_out.c_str(), "strip", TRUE, "compression", VIPS_FOREIGN_TIFF_COMPRESSION_JPEG, "Q", baton->quality, NULL)) { - return resize_error(baton, sharpened); + if (vips_tiffsave(colourspaced, baton->file_out.c_str(), "strip", TRUE, "compression", VIPS_FOREIGN_TIFF_COMPRESSION_JPEG, "Q", baton->quality, NULL)) { + return resize_error(baton, colourspaced); } } else { (baton->err).append("Unsupported output " + baton->file_out); } - g_object_unref(sharpened); + g_object_unref(colourspaced); vips_thread_shutdown(); } diff --git a/tests/perf.js b/tests/perf.js index 212730b6..0f3cd72a 100755 --- a/tests/perf.js +++ b/tests/perf.js @@ -330,18 +330,6 @@ async.series({ } }); } - }).add("sharp-file-buffer-sequentialRead", { - defer: true, - fn: function(deferred) { - sharp(inputPng).sequentialRead().resize(width, height).toBuffer(function(err, buffer) { - if (err) { - throw err; - } else { - assert.notStrictEqual(null, buffer); - deferred.resolve(); - } - }); - } }).on("cycle", function(event) { console.log(" png " + String(event.target)); }).on("complete", function() { @@ -408,18 +396,6 @@ async.series({ } }); } - }).add("sharp-file-buffer-sequentialRead", { - defer: true, - fn: function(deferred) { - sharp(inputWebp).sequentialRead().resize(width, height).toBuffer(function(err, buffer) { - if (err) { - throw err; - } else { - assert.notStrictEqual(null, buffer); - deferred.resolve(); - } - }); - } }).on("cycle", function(event) { console.log("webp " + String(event.target)); }).on("complete", function() { @@ -449,17 +425,6 @@ async.series({ } }); } - }).add("sharp-file-file-sequentialRead", { - defer: true, - fn: function(deferred) { - sharp(inputTiff).sequentialRead().resize(width, height).toFile(outputTiff, function(err) { - if (err) { - throw err; - } else { - deferred.resolve(); - } - }); - } }).on("cycle", function(event) { console.log("tiff " + String(event.target)); }).on("complete", function() { diff --git a/tests/unit.js b/tests/unit.js index 82857cd0..1f7cdfa5 100755 --- a/tests/unit.js +++ b/tests/unit.js @@ -262,5 +262,11 @@ async.series([ } catch (e) {} assert(!fail); done(); + }, + // Check colour space conversion occurs from TIFF to WebP (this used to segfault) + function(done) { + sharp(inputTiff).webp().then(function() { + done(); + }); } ]);