From bd9c97feede8771e5f76d667bd07eb933eac5797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Szabo?= Date: Fri, 5 Jul 2019 17:23:29 +0200 Subject: [PATCH] Changes based on review --- libvips/foreign/gifload.c | 12 ++++++------ libvips/foreign/magicksave.c | 23 ++++++++++++++++------- libvips/foreign/vips2webp.c | 6 ++++-- libvips/foreign/webp2vips.c | 16 +++++++--------- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/libvips/foreign/gifload.c b/libvips/foreign/gifload.c index ba53075d..690f4a68 100644 --- a/libvips/foreign/gifload.c +++ b/libvips/foreign/gifload.c @@ -132,7 +132,7 @@ typedef struct _VipsForeignLoadGif { gboolean has_transparency; gboolean has_colour; - /* Delays between frames (in miliseconds). + /* Delays between frames (in milliseconds). */ int *delays; @@ -519,9 +519,9 @@ vips_foreign_load_gif_scan_extension( VipsForeignLoadGif *gif ) gif->has_transparency = TRUE; } - if( gif->n_pages % 64 == 0 ) { + if( gif->n_pages % 64 == 0 ) gif->delays = (int *) g_realloc( gif->delays, (gif->n_pages + 64) * sizeof(int) ); - } + gif->delays[gif->n_pages] = (extension[2] | (extension[3] << 8)) * 10; while( extension != NULL ) @@ -575,11 +575,11 @@ vips_foreign_load_gif_set_header( VipsForeignLoadGif *gif, VipsImage *image ) vips_image_set_int( image, "gif-loop", gif->loop ); if( gif->delays ) { - vips_image_set_int( image, "gif-delay", VIPS_RINT( gif->delays[0] / 10.0 ) ); + vips_image_set_int( image, + "gif-delay", VIPS_RINT( gif->delays[0] / 10.0 ) ); vips_image_set_array_int( image, "delay", gif->delays, gif->n_pages ); - } else { + } else vips_image_set_int( image, "gif-delay", 4 ); - } if( gif->comment ) vips_image_set_string( image, "gif-comment", gif->comment ); diff --git a/libvips/foreign/magicksave.c b/libvips/foreign/magicksave.c index 78687835..91445b86 100644 --- a/libvips/foreign/magicksave.c +++ b/libvips/foreign/magicksave.c @@ -70,6 +70,8 @@ typedef struct _VipsForeignSaveMagick { Image *current_image; int page_height; + int *delays; + int delays_length; /* The position of current_image in the output. */ @@ -111,9 +113,8 @@ vips_foreign_save_magick_next_image( VipsForeignSaveMagick *magick ) Image *image; int number; - int *numbers; - int numbers_length; const char *str; + int page_index; g_assert( !magick->current_image ); @@ -143,11 +144,13 @@ vips_foreign_save_magick_next_image( VipsForeignSaveMagick *magick ) im->Xsize, magick->page_height, magick->exception ) ) return( -1 ); - if( vips_image_get_typeof( im, "delay" ) && - !vips_image_get_array_int( im, "delay", &numbers, &numbers_length ) ) { - int page_index = magick->position.top / magick->page_height; - if( page_index < numbers_length ) - image->delay = (size_t) VIPS_RINT( numbers[page_index] / 10.0 ); + /* Delay must be converted from milliseconds into centiseconds + * as GIF image requires centiseconds. + */ + if ( magick->delays != NULL) { + page_index = magick->position.top / magick->page_height; + if( page_index < magick->delays_length ) + image->delay = (size_t) VIPS_RINT( magick->delays[page_index] / 10.0 ); } /* ImageMagick uses iterations like this (at least in gif save): @@ -343,6 +346,12 @@ vips_foreign_save_magick_build( VipsObject *object ) magick->page_height = vips_image_get_page_height( im ); + if( vips_image_get_typeof( im, "delay" ) && + vips_image_get_array_int( im, + "delay", &magick->delays, &magick->delays_length ) ) { + return( -1 ); + } + if( vips_sink_disc( im, vips_foreign_save_magick_write_block, magick ) ) return( -1 ); diff --git a/libvips/foreign/vips2webp.c b/libvips/foreign/vips2webp.c index f203c803..b3e3ecc7 100644 --- a/libvips/foreign/vips2webp.c +++ b/libvips/foreign/vips2webp.c @@ -308,7 +308,8 @@ get_array_int( VipsImage *image, const char *field, int* n ) static int extract_delay( int index, int *delays, int delays_length, int default_delay ) { - if( delays == NULL || index > delays_length ) return( default_delay ); + if( delays == NULL || index > delays_length ) + return( default_delay ); return( delays[index] ); } @@ -351,6 +352,7 @@ write_webp_anim( VipsWebPWrite *write, VipsImage *image, int page_height ) for( top = 0; top < image->Ysize; top += page_height ) { VipsImage *x; WebPPicture pic; + int page_index; if( vips_crop( image, &x, 0, top, image->Xsize, page_height, NULL ) ) @@ -373,7 +375,7 @@ write_webp_anim( VipsWebPWrite *write, VipsImage *image, int page_height ) WebPPictureFree( &pic ); - int page_index = top / page_height; + page_index = top / page_height; timestamp_ms += extract_delay( page_index, delays, delays_length, default_delay ); } diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index 2ee09def..6aa6882e 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -358,6 +358,7 @@ read_free( Read *read ) VIPS_FREEF( vips_tracked_close, read->fd ); VIPS_FREE( read->filename ); + VIPS_FREE( read->delays ); VIPS_FREE( read ); return( 0 ); @@ -475,29 +476,26 @@ read_header( Read *read, VipsImage *out ) VIPS_META_PAGE_HEIGHT, read->frame_height ); if ( read->frame_count > 1) { - read->delays = (int *) g_malloc( read->frame_count * sizeof(int) ); + int i; + read->delays = (int *) g_malloc0( read->frame_count * sizeof(int) ); - for( int i = 0; i < read->frame_count; i++ ) { - if( WebPDemuxGetFrame( read->demux, i + 1, &iter ) ) { + for( i = 0; i < read->frame_count; i++ ) { + if( WebPDemuxGetFrame( read->demux, i + 1, &iter ) ) read->delays[i] = iter.duration; - } else { - read->delays[i] = 0; - } } #ifdef DEBUG - for( int i = 0; i < read->frame_count; i++ ) { + for( i = 0; i < read->frame_count; i++ ) { printf( "webp2vips: frame = %d; duration = %d\n", i + 1, read->delays[i] ); } #endif /*DEBUG*/ vips_image_set_array_int( out, "delay", read->delays, read->frame_count ); - g_free( read->delays ); /* webp uses ms for delays, gif uses centiseconds. */ vips_image_set_int( out, "gif-delay", - VIPS_RINT( read->delays[0] / 10.0 ) ); + VIPS_RINT( read->delays[0] / 10.0 ) ); /* We need the alpha in an animation if: * - any frame has transparent pixels