From d1094847a3f6fe73133487bb62175b7b0be449db Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 27 Jun 2019 16:37:11 +0100 Subject: [PATCH 1/7] remove no-alpha webp support We used to try to spot webp images with no alpha and load them as plain RGB, but it turns out this is difficult to do reliably, especially for animated images. This patch simply removes support, so all webp images now load as RGBA. See https://github.com/libvips/libvips/issues/1351 --- ChangeLog | 1 + libvips/foreign/webp2vips.c | 25 ++++++++++--------------- test/test-suite/test_foreign.py | 11 ++++++----- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index bf0e16f0..b759536d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 20/6/19 started 8.9.0 - add vips_image_get/set_array_int() +- remove support for no-alpha webp images, it didn't work well 24/5/19 started 8.8.1 - improve realpath() use on older libc diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index 695e87c1..95bc8056 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -20,6 +20,9 @@ * 30/4/19 * - deprecate shrink, use scale instead, and make it a double ... this * lets us do faster and more accurate thumbnailing + * 27/6/19 + * - remove non-alpha output: it's very hard to decide if a webp image + * really has zero transparency */ /* @@ -107,10 +110,6 @@ typedef struct { int frame_width; int frame_height; - /* TRUE for RGBA. - */ - int alpha; - /* Number of frames in file. */ int frame_count; @@ -446,11 +445,10 @@ read_header( Read *read, VipsImage *out ) flags = WebPDemuxGetI( read->demux, WEBP_FF_FORMAT_FLAGS ); - read->alpha = flags & ALPHA_FLAG; - if( read->alpha ) - read->config.output.colorspace = MODE_RGBA; - else - read->config.output.colorspace = MODE_RGB; + /* The alpha flag says if this square of pixels in this frame has an + * alpha, not if our output image should have an alpha. + */ + read->config.output.colorspace = MODE_RGBA; if( flags & ANIMATION_FLAG ) { int loop_count; @@ -534,8 +532,7 @@ read_header( Read *read, VipsImage *out ) read->frame = vips_image_new_memory(); vips_image_init_fields( read->frame, - read->frame_width, read->frame_height, - read->alpha ? 4 : 3, + read->frame_width, read->frame_height, 4, VIPS_FORMAT_UCHAR, VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, 1.0, 1.0 ); @@ -545,8 +542,7 @@ read_header( Read *read, VipsImage *out ) return( -1 ); vips_image_init_fields( out, - read->width, read->height, - read->alpha ? 4 : 3, + read->width, read->height, 4, VIPS_FORMAT_UCHAR, VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, 1.0, 1.0 ); @@ -598,8 +594,7 @@ read_frame( Read *read, frame = vips_image_new_memory(); vips_image_init_fields( frame, - width, height, - read->alpha ? 4 : 3, + width, height, 4, VIPS_FORMAT_UCHAR, VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, 1.0, 1.0 ); diff --git a/test/test-suite/test_foreign.py b/test/test-suite/test_foreign.py index b260705a..5383040c 100644 --- a/test/test-suite/test_foreign.py +++ b/test/test-suite/test_foreign.py @@ -23,6 +23,7 @@ class TestForeign: cls.tempdir = tempfile.mkdtemp() cls.colour = pyvips.Image.jpegload(JPEG_FILE) + cls.rgba = cls.colour.bandjoin(255) cls.mono = cls.colour.extract_band(1) # we remove the ICC profile: the RGB one will no longer be appropriate cls.mono.remove("icc-profile-data") @@ -463,22 +464,22 @@ class TestForeign: def test_webp(self): def webp_valid(im): a = im(10, 10) - assert_almost_equal_objects(a, [71, 166, 236]) + assert_almost_equal_objects(a, [70, 165, 235, 255]) assert im.width == 550 assert im.height == 368 - assert im.bands == 3 + assert im.bands == 4 self.file_loader("webpload", WEBP_FILE, webp_valid) self.buffer_loader("webpload_buffer", WEBP_FILE, webp_valid) self.save_load_buffer("webpsave_buffer", "webpload_buffer", - self.colour, 60) - self.save_load("%s.webp", self.colour) + self.rgba, 60) + self.save_load("%s.webp", self.rgba) # test lossless mode im = pyvips.Image.new_from_file(WEBP_FILE) buf = im.webpsave_buffer(lossless=True) im2 = pyvips.Image.new_from_buffer(buf, "") - assert im.avg() == im2.avg() + assert abs(im.avg() - im2.avg()) < 1 # higher Q should mean a bigger buffer b1 = im.webpsave_buffer(Q=10) From 72c103f95a3407ce35bdfdb78d189ceae8a26098 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 27 Jun 2019 17:27:53 +0100 Subject: [PATCH 2/7] Revert "remove no-alpha webp support" This reverts commit d1094847a3f6fe73133487bb62175b7b0be449db. --- ChangeLog | 1 - libvips/foreign/webp2vips.c | 25 +++++++++++++++---------- test/test-suite/test_foreign.py | 11 +++++------ 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index b759536d..bf0e16f0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,5 @@ 20/6/19 started 8.9.0 - add vips_image_get/set_array_int() -- remove support for no-alpha webp images, it didn't work well 24/5/19 started 8.8.1 - improve realpath() use on older libc diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index 95bc8056..695e87c1 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -20,9 +20,6 @@ * 30/4/19 * - deprecate shrink, use scale instead, and make it a double ... this * lets us do faster and more accurate thumbnailing - * 27/6/19 - * - remove non-alpha output: it's very hard to decide if a webp image - * really has zero transparency */ /* @@ -110,6 +107,10 @@ typedef struct { int frame_width; int frame_height; + /* TRUE for RGBA. + */ + int alpha; + /* Number of frames in file. */ int frame_count; @@ -445,10 +446,11 @@ read_header( Read *read, VipsImage *out ) flags = WebPDemuxGetI( read->demux, WEBP_FF_FORMAT_FLAGS ); - /* The alpha flag says if this square of pixels in this frame has an - * alpha, not if our output image should have an alpha. - */ - read->config.output.colorspace = MODE_RGBA; + read->alpha = flags & ALPHA_FLAG; + if( read->alpha ) + read->config.output.colorspace = MODE_RGBA; + else + read->config.output.colorspace = MODE_RGB; if( flags & ANIMATION_FLAG ) { int loop_count; @@ -532,7 +534,8 @@ read_header( Read *read, VipsImage *out ) read->frame = vips_image_new_memory(); vips_image_init_fields( read->frame, - read->frame_width, read->frame_height, 4, + read->frame_width, read->frame_height, + read->alpha ? 4 : 3, VIPS_FORMAT_UCHAR, VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, 1.0, 1.0 ); @@ -542,7 +545,8 @@ read_header( Read *read, VipsImage *out ) return( -1 ); vips_image_init_fields( out, - read->width, read->height, 4, + read->width, read->height, + read->alpha ? 4 : 3, VIPS_FORMAT_UCHAR, VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, 1.0, 1.0 ); @@ -594,7 +598,8 @@ read_frame( Read *read, frame = vips_image_new_memory(); vips_image_init_fields( frame, - width, height, 4, + width, height, + read->alpha ? 4 : 3, VIPS_FORMAT_UCHAR, VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, 1.0, 1.0 ); diff --git a/test/test-suite/test_foreign.py b/test/test-suite/test_foreign.py index 5383040c..b260705a 100644 --- a/test/test-suite/test_foreign.py +++ b/test/test-suite/test_foreign.py @@ -23,7 +23,6 @@ class TestForeign: cls.tempdir = tempfile.mkdtemp() cls.colour = pyvips.Image.jpegload(JPEG_FILE) - cls.rgba = cls.colour.bandjoin(255) cls.mono = cls.colour.extract_band(1) # we remove the ICC profile: the RGB one will no longer be appropriate cls.mono.remove("icc-profile-data") @@ -464,22 +463,22 @@ class TestForeign: def test_webp(self): def webp_valid(im): a = im(10, 10) - assert_almost_equal_objects(a, [70, 165, 235, 255]) + assert_almost_equal_objects(a, [71, 166, 236]) assert im.width == 550 assert im.height == 368 - assert im.bands == 4 + assert im.bands == 3 self.file_loader("webpload", WEBP_FILE, webp_valid) self.buffer_loader("webpload_buffer", WEBP_FILE, webp_valid) self.save_load_buffer("webpsave_buffer", "webpload_buffer", - self.rgba, 60) - self.save_load("%s.webp", self.rgba) + self.colour, 60) + self.save_load("%s.webp", self.colour) # test lossless mode im = pyvips.Image.new_from_file(WEBP_FILE) buf = im.webpsave_buffer(lossless=True) im2 = pyvips.Image.new_from_buffer(buf, "") - assert abs(im.avg() - im2.avg()) < 1 + assert im.avg() == im2.avg() # higher Q should mean a bigger buffer b1 = im.webpsave_buffer(Q=10) From 8a354c5aecff341f509af2ad24e24066161e144f Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 27 Jun 2019 18:44:38 +0100 Subject: [PATCH 3/7] improve webp rgba handling disable webp alpha output if all frame fill the canvas and are solid see https://github.com/libvips/libvips/issues/1351 --- ChangeLog | 1 + libvips/foreign/webp2vips.c | 47 ++++++++++++++++++++++++++----------- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/ChangeLog b/ChangeLog index bf0e16f0..ad086f4e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 20/6/19 started 8.9.0 - add vips_image_get/set_array_int() +- disable webp alpha output if all frame fill the canvas and are solid 24/5/19 started 8.8.1 - improve realpath() use on older libc diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index 695e87c1..28d0a0dd 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -97,17 +97,22 @@ typedef struct { */ double scale; + /* Size of each frame in input image coordinates. + */ + int canvas_width; + int canvas_height; + + /* Size of each frame, in scaled output image coordinates, + */ + int frame_width; + int frame_height; + /* Size of final output image. */ int width; int height; - /* Size of each frame. - */ - int frame_width; - int frame_height; - - /* TRUE for RGBA. + /* TRUE if we will save the final image as RGBA. */ int alpha; @@ -418,8 +423,6 @@ static int read_header( Read *read, VipsImage *out ) { WebPData data; - int canvas_width; - int canvas_height; int flags; int i; @@ -430,16 +433,19 @@ read_header( Read *read, VipsImage *out ) return( -1 ); } - canvas_width = WebPDemuxGetI( read->demux, WEBP_FF_CANVAS_WIDTH ); - canvas_height = WebPDemuxGetI( read->demux, WEBP_FF_CANVAS_HEIGHT ); + read->canvas_width = + WebPDemuxGetI( read->demux, WEBP_FF_CANVAS_WIDTH ); + read->canvas_height = + WebPDemuxGetI( read->demux, WEBP_FF_CANVAS_HEIGHT ); + /* We round-to-nearest cf. pdfload etc. */ - read->frame_width = VIPS_RINT( canvas_width * read->scale ); - read->frame_height = VIPS_RINT( canvas_height * read->scale ); + read->frame_width = VIPS_RINT( read->canvas_width * read->scale ); + read->frame_height = VIPS_RINT( read->canvas_height * read->scale ); #ifdef DEBUG - printf( "webp2vips: canvas_width = %d\n", canvas_width ); - printf( "webp2vips: canvas_height = %d\n", canvas_height ); + printf( "webp2vips: canvas_width = %d\n", read->canvas_width ); + printf( "webp2vips: canvas_height = %d\n", read->canvas_height ); printf( "webp2vips: frame_width = %d\n", read->frame_width ); printf( "webp2vips: frame_height = %d\n", read->frame_height ); #endif /*DEBUG*/ @@ -484,6 +490,19 @@ read_header( Read *read, VipsImage *out ) */ vips_image_set_int( out, "gif-delay", VIPS_RINT( read->delay / 10.0 ) ); + + /* If we find a frame which doesn't fill the canvas, + * we'll need to save as RGBA. + */ + do { + if( iter.x_offset != 0 || + iter.y_offset != 0 || + iter.width != read->canvas_width || + iter.height != read->canvas_height ) { + read->alpha = TRUE; + break; + } + } while( WebPDemuxNextFrame( &iter ) ); } WebPDemuxReleaseIterator( &iter ); From 7d8b6d9d9f0a7e4cc8b04d8a6897328981dd37b3 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 28 Jun 2019 04:09:31 +0100 Subject: [PATCH 4/7] note webp change --- libvips/foreign/webp2vips.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index 28d0a0dd..fce90906 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -20,6 +20,8 @@ * 30/4/19 * - deprecate shrink, use scale instead, and make it a double ... this * lets us do faster and more accurate thumbnailing + * 27/6/19 + * - disable alpha output if all frame fill the canvas and are solid */ /* From d3cd51a8c3e027ce7cc92dd853677f20a004be07 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 29 Jun 2019 11:50:26 +0100 Subject: [PATCH 5/7] check alpha on animation rects too see https://github.com/libvips/libvips/issues/1351#issuecomment-506942104 --- libvips/foreign/webp2vips.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index fce90906..06c94ad8 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -493,11 +493,13 @@ read_header( Read *read, VipsImage *out ) vips_image_set_int( out, "gif-delay", VIPS_RINT( read->delay / 10.0 ) ); - /* If we find a frame which doesn't fill the canvas, - * we'll need to save as RGBA. + /* We need the alpha in an animation if: + * - any frame has transparent pixels + * - any frame doesn't fill the whole canvas. */ do { - if( iter.x_offset != 0 || + if( iter.has_alpha || + iter.x_offset != 0 || iter.y_offset != 0 || iter.width != read->canvas_width || iter.height != read->canvas_height ) { From 20b9d770864dc6825e98faea30b0edd45ed053c5 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 29 Jun 2019 13:18:29 +0100 Subject: [PATCH 6/7] don't need to test xoff / yoff --- libvips/foreign/webp2vips.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index 06c94ad8..a620c25c 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -499,8 +499,6 @@ read_header( Read *read, VipsImage *out ) */ do { if( iter.has_alpha || - iter.x_offset != 0 || - iter.y_offset != 0 || iter.width != read->canvas_width || iter.height != read->canvas_height ) { read->alpha = TRUE; From aac01126af7a1115f2666abbf754cc3fd3676a75 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 29 Jun 2019 20:23:46 +0100 Subject: [PATCH 7/7] magicksave supports strip option --- libvips/foreign/gifload.c | 9 --------- libvips/foreign/magicksave.c | 5 ++++- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/libvips/foreign/gifload.c b/libvips/foreign/gifload.c index b43624a5..122e5f40 100644 --- a/libvips/foreign/gifload.c +++ b/libvips/foreign/gifload.c @@ -580,15 +580,6 @@ vips_foreign_load_gif_set_header( VipsForeignLoadGif *gif, VipsImage *image ) if( gif->comment ) vips_image_set_string( image, "gif-comment", gif->comment ); -{ - int array[10]; - int i; - - for( i = 0; i < 10; i++ ) - array[i] = i; - vips_image_set_array_int( image, "delay", array, 10 ); -} - return( 0 ); } diff --git a/libvips/foreign/magicksave.c b/libvips/foreign/magicksave.c index fc32f8dc..732e0e26 100644 --- a/libvips/foreign/magicksave.c +++ b/libvips/foreign/magicksave.c @@ -6,6 +6,8 @@ * 17/2/19 * - support ICC, XMP, EXIF, IPTC metadata * - write with a single call to vips_sink_disc() + * 29/6/19 + * - support "strip" option */ /* @@ -165,7 +167,8 @@ vips_foreign_save_magick_next_image( VipsForeignSaveMagick *magick ) */ image->dispose = BackgroundDispose; - if( magick_set_magick_profile( image, im, magick->exception ) ) { + if( !save->strip && + magick_set_magick_profile( image, im, magick->exception ) ) { magick_vips_error( class->nickname, magick->exception ); return( -1 ); }