Skip to content

Commit

Permalink
always copy before exif_update
Browse files Browse the repository at this point in the history
During write, we often call vips__exif_update(). This updates the exif
block from the other image metadata prior to save.

Always copy the image before calling this.

See lovell/sharp#1986
  • Loading branch information
jcupitt committed Nov 28, 2019
1 parent 20cee5d commit acd9101
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 36 deletions.
1 change: 1 addition & 0 deletions ChangeLog
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
- fix use of resolution-unit metadata on tiff save [kayarre] - fix use of resolution-unit metadata on tiff save [kayarre]
- support TIFF CIELAB images with alpha [angelmixu] - support TIFF CIELAB images with alpha [angelmixu]
- support TIFF with premultiplied alpha in any band - support TIFF with premultiplied alpha in any band
- block metadata changes on shared images [pvdz]


17/9/19 started 8.8.4 17/9/19 started 8.8.4
- improve compatibility with older imagemagick versions - improve compatibility with older imagemagick versions
Expand Down
43 changes: 27 additions & 16 deletions libvips/foreign/heifsave.c
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ typedef struct _VipsForeignSaveHeif {
*/ */
VipsForeignHeifCompression compression; VipsForeignHeifCompression compression;


/* The image we save. This is a copy of save->ready since we need to
* be able to update the metadata.
*/
VipsImage *image;

int page_width; int page_width;
int page_height; int page_height;
int n_pages; int n_pages;
Expand Down Expand Up @@ -107,6 +112,7 @@ vips_foreign_save_heif_dispose( GObject *gobject )
{ {
VipsForeignSaveHeif *heif = (VipsForeignSaveHeif *) gobject; VipsForeignSaveHeif *heif = (VipsForeignSaveHeif *) gobject;


VIPS_UNREF( heif->image );
VIPS_FREEF( heif_image_release, heif->img ); VIPS_FREEF( heif_image_release, heif->img );
VIPS_FREEF( heif_image_handle_release, heif->handle ); VIPS_FREEF( heif_image_handle_release, heif->handle );
VIPS_FREEF( heif_encoder_release, heif->encoder ); VIPS_FREEF( heif_encoder_release, heif->encoder );
Expand Down Expand Up @@ -134,13 +140,12 @@ static int
vips_foreign_save_heif_write_metadata( VipsForeignSaveHeif *heif ) vips_foreign_save_heif_write_metadata( VipsForeignSaveHeif *heif )
{ {
#ifdef HAVE_HEIF_CONTEXT_ADD_EXIF_METADATA #ifdef HAVE_HEIF_CONTEXT_ADD_EXIF_METADATA
VipsForeignSave *save = (VipsForeignSave *) heif;


int i; int i;
struct heif_error error; struct heif_error error;


for( i = 0; i < VIPS_NUMBER( libheif_metadata ); i++ ) for( i = 0; i < VIPS_NUMBER( libheif_metadata ); i++ )
if( vips_image_get_typeof( save->ready, if( vips_image_get_typeof( heif->image,
libheif_metadata[i].name ) ) { libheif_metadata[i].name ) ) {
const void *data; const void *data;
size_t length; size_t length;
Expand All @@ -150,7 +155,7 @@ vips_foreign_save_heif_write_metadata( VipsForeignSaveHeif *heif )
libheif_metadata[i].name ); libheif_metadata[i].name );
#endif /*DEBUG*/ #endif /*DEBUG*/


if( vips_image_get_blob( save->ready, if( vips_image_get_blob( heif->image,
VIPS_META_EXIF_NAME, &data, &length ) ) VIPS_META_EXIF_NAME, &data, &length ) )
return( -1 ); return( -1 );


Expand All @@ -176,15 +181,15 @@ vips_foreign_save_heif_write_page( VipsForeignSaveHeif *heif, int page )


#ifdef HAVE_HEIF_COLOR_PROFILE #ifdef HAVE_HEIF_COLOR_PROFILE
if( !save->strip && if( !save->strip &&
vips_image_get_typeof( save->ready, VIPS_META_ICC_NAME ) ) { vips_image_get_typeof( heif->image, VIPS_META_ICC_NAME ) ) {
const void *data; const void *data;
size_t length; size_t length;


#ifdef DEBUG #ifdef DEBUG
printf( "attaching profile ..\n" ); printf( "attaching profile ..\n" );
#endif /*DEBUG*/ #endif /*DEBUG*/


if( vips_image_get_blob( save->ready, if( vips_image_get_blob( heif->image,
VIPS_META_ICC_NAME, &data, &length ) ) VIPS_META_ICC_NAME, &data, &length ) )
return( -1 ); return( -1 );


Expand All @@ -201,7 +206,7 @@ vips_foreign_save_heif_write_page( VipsForeignSaveHeif *heif, int page )


#ifdef HAVE_HEIF_ENCODING_OPTIONS_ALLOC #ifdef HAVE_HEIF_ENCODING_OPTIONS_ALLOC
options = heif_encoding_options_alloc(); options = heif_encoding_options_alloc();
if( vips_image_hasalpha( save->ready ) ) if( vips_image_hasalpha( heif->image ) )
options->save_alpha_channel = 1; options->save_alpha_channel = 1;
#else /*!HAVE_HEIF_ENCODING_OPTIONS_ALLOC*/ #else /*!HAVE_HEIF_ENCODING_OPTIONS_ALLOC*/
options = NULL; options = NULL;
Expand All @@ -223,10 +228,10 @@ vips_foreign_save_heif_write_page( VipsForeignSaveHeif *heif, int page )
} }


#ifdef HAVE_HEIF_CONTEXT_SET_PRIMARY_IMAGE #ifdef HAVE_HEIF_CONTEXT_SET_PRIMARY_IMAGE
if( vips_image_get_typeof( save->ready, "heif-primary" ) ) { if( vips_image_get_typeof( heif->image, "heif-primary" ) ) {
int primary; int primary;


if( vips_image_get_int( save->ready, if( vips_image_get_int( heif->image,
"heif-primary", &primary ) ) "heif-primary", &primary ) )
return( -1 ); return( -1 );


Expand Down Expand Up @@ -299,6 +304,12 @@ vips_foreign_save_heif_build( VipsObject *object )
build( object ) ) build( object ) )
return( -1 ); return( -1 );


/* We must copy the input image since we will be updating the
* metadata when we write the exif.
*/
if( vips_copy( save->ready, &heif->image, NULL ) )
return( -1 );

error = heif_context_get_encoder_for_format( heif->ctx, error = heif_context_get_encoder_for_format( heif->ctx,
(enum heif_compression_format) heif->compression, (enum heif_compression_format) heif->compression,
&heif->encoder ); &heif->encoder );
Expand Down Expand Up @@ -328,16 +339,16 @@ vips_foreign_save_heif_build( VipsObject *object )
* heif_encoder_list_parameters(). * heif_encoder_list_parameters().
*/ */


heif->page_width = save->ready->Xsize; heif->page_width = heif->image->Xsize;
heif->page_height = vips_image_get_page_height( save->ready ); heif->page_height = vips_image_get_page_height( heif->image );
heif->n_pages = save->ready->Ysize / heif->page_height; heif->n_pages = heif->image->Ysize / heif->page_height;


/* Make a heif image the size of a page. We send sink_disc() output /* Make a heif image the size of a page. We send sink_disc() output
* here and write a frame each time it fills. * here and write a frame each time it fills.
*/ */
error = heif_image_create( heif->page_width, heif->page_height, error = heif_image_create( heif->page_width, heif->page_height,
heif_colorspace_RGB, heif_colorspace_RGB,
vips_image_hasalpha( save->ready ) ? vips_image_hasalpha( heif->image ) ?
heif_chroma_interleaved_RGBA : heif_chroma_interleaved_RGBA :
heif_chroma_interleaved_RGB, heif_chroma_interleaved_RGB,
&heif->img ); &heif->img );
Expand All @@ -348,7 +359,7 @@ vips_foreign_save_heif_build( VipsObject *object )


error = heif_image_add_plane( heif->img, heif_channel_interleaved, error = heif_image_add_plane( heif->img, heif_channel_interleaved,
heif->page_width, heif->page_height, heif->page_width, heif->page_height,
vips_image_hasalpha( save->ready ) ? 32 : 24 ); vips_image_hasalpha( heif->image ) ? 32 : 24 );
if( error.code ) { if( error.code ) {
vips__heif_error( &error ); vips__heif_error( &error );
return( -1 ); return( -1 );
Expand All @@ -359,13 +370,13 @@ vips_foreign_save_heif_build( VipsObject *object )


/* Just do this once, so we don't rebuild exif on every page. /* Just do this once, so we don't rebuild exif on every page.
*/ */
if( vips_image_get_typeof( save->ready, VIPS_META_EXIF_NAME ) ) if( vips_image_get_typeof( heif->image, VIPS_META_EXIF_NAME ) )
if( vips__exif_update( save->ready ) ) if( vips__exif_update( heif->image ) )
return( -1 ); return( -1 );


/* Write data. /* Write data.
*/ */
if( vips_sink_disc( save->ready, if( vips_sink_disc( heif->image,
vips_foreign_save_heif_write_block, heif ) ) vips_foreign_save_heif_write_block, heif ) )
return( -1 ); return( -1 );


Expand Down
11 changes: 10 additions & 1 deletion libvips/foreign/vips2jpeg.c
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ write_destroy( Write *write )
jpeg_destroy_compress( &write->cinfo ); jpeg_destroy_compress( &write->cinfo );
VIPS_FREE( write->row_pointer ); VIPS_FREE( write->row_pointer );
VIPS_UNREF( write->inverted ); VIPS_UNREF( write->inverted );
VIPS_UNREF( write->in );


g_free( write ); g_free( write );
} }
Expand All @@ -223,7 +224,7 @@ write_new( VipsImage *in )
if( !(write = g_new0( Write, 1 )) ) if( !(write = g_new0( Write, 1 )) )
return( NULL ); return( NULL );


write->in = in; write->in = NULL;
write->row_pointer = NULL; write->row_pointer = NULL;
write->cinfo.err = jpeg_std_error( &write->eman.pub ); write->cinfo.err = jpeg_std_error( &write->eman.pub );
write->cinfo.dest = NULL; write->cinfo.dest = NULL;
Expand All @@ -232,6 +233,14 @@ write_new( VipsImage *in )
write->eman.fp = NULL; write->eman.fp = NULL;
write->inverted = NULL; write->inverted = NULL;


/* We must copy the input image since we will be updating the
* metadata when we write the exif.
*/
if( vips_copy( in, &write->in, NULL ) ) {
write_destroy( write );
return( NULL );
}

return( write ); return( write );
} }


Expand Down
43 changes: 26 additions & 17 deletions libvips/foreign/vips2webp.c
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ vips_webp_write_unset( VipsWebPWrite *write )
WebPMemoryWriterClear( &write->memory_writer ); WebPMemoryWriterClear( &write->memory_writer );
VIPS_FREEF( WebPAnimEncoderDelete, write->enc ); VIPS_FREEF( WebPAnimEncoderDelete, write->enc );
VIPS_FREEF( WebPMuxDelete, write->mux ); VIPS_FREEF( WebPMuxDelete, write->mux );
VIPS_UNREF( write->image );
} }


static int static int
Expand All @@ -147,7 +148,7 @@ vips_webp_write_init( VipsWebPWrite *write, VipsImage *image,
gboolean min_size, int kmin, int kmax, gboolean min_size, int kmin, int kmax,
gboolean strip ) gboolean strip )
{ {
write->image = image; write->image = NULL;
write->Q = Q; write->Q = Q;
write->lossless = lossless; write->lossless = lossless;
write->preset = preset; write->preset = preset;
Expand All @@ -163,6 +164,14 @@ vips_webp_write_init( VipsWebPWrite *write, VipsImage *image,
write->enc = NULL; write->enc = NULL;
write->mux = NULL; write->mux = NULL;


/* We must copy the input image since we will be updating the
* metadata when we write the exif.
*/
if( vips_copy( image, &write->image, NULL ) ) {
vips_webp_write_unset( write );
return( -1 );
}

if( !WebPConfigInit( &write->config ) ) { if( !WebPConfigInit( &write->config ) ) {
vips_webp_write_unset( write ); vips_webp_write_unset( write );
vips_error( "vips2webp", vips_error( "vips2webp",
Expand Down Expand Up @@ -390,14 +399,14 @@ write_webp_anim( VipsWebPWrite *write, VipsImage *image, int page_height )
} }


static int static int
write_webp( VipsWebPWrite *write, VipsImage *image ) write_webp( VipsWebPWrite *write )
{ {
int page_height = vips_image_get_page_height( image ); int page_height = vips_image_get_page_height( write->image );


if( page_height < image->Ysize ) if( page_height < write->image->Ysize )
return( write_webp_anim( write, image, page_height ) ); return( write_webp_anim( write, write->image, page_height ) );
else else
return( write_webp_single( write, image ) ); return( write_webp_single( write, write->image ) );
} }


static void static void
Expand Down Expand Up @@ -437,19 +446,19 @@ vips_webp_set_chunk( VipsWebPWrite *write,
} }


static int static int
vips_webp_add_chunks( VipsWebPWrite *write, VipsImage *image ) vips_webp_add_chunks( VipsWebPWrite *write )
{ {
int i; int i;


for( i = 0; i < vips__n_webp_names; i++ ) { for( i = 0; i < vips__n_webp_names; i++ ) {
const char *vips_name = vips__webp_names[i].vips; const char *vips_name = vips__webp_names[i].vips;
const char *webp_name = vips__webp_names[i].webp; const char *webp_name = vips__webp_names[i].webp;


if( vips_image_get_typeof( image, vips_name ) ) { if( vips_image_get_typeof( write->image, vips_name ) ) {
const void *data; const void *data;
size_t length; size_t length;


if( vips_image_get_blob( image, if( vips_image_get_blob( write->image,
vips_name, &data, &length ) || vips_name, &data, &length ) ||
vips_webp_set_chunk( write, vips_webp_set_chunk( write,
webp_name, data, length ) ) webp_name, data, length ) )
Expand All @@ -461,13 +470,13 @@ vips_webp_add_chunks( VipsWebPWrite *write, VipsImage *image )
} }


static int static int
vips_webp_add_metadata( VipsWebPWrite *write, VipsImage *image, gboolean strip ) vips_webp_add_metadata( VipsWebPWrite *write )
{ {
WebPData data; WebPData data;


/* Rebuild the EXIF block, if any, ready for writing. /* Rebuild the EXIF block, if any, ready for writing.
*/ */
if( vips__exif_update( image ) ) if( vips__exif_update( write->image ) )
return( -1 ); return( -1 );


data.bytes = write->memory_writer.mem; data.bytes = write->memory_writer.mem;
Expand All @@ -480,19 +489,19 @@ vips_webp_add_metadata( VipsWebPWrite *write, VipsImage *image, gboolean strip )
return( -1 ); return( -1 );
} }


if( vips_image_get_typeof( image, "gif-loop" ) ) { if( vips_image_get_typeof( write->image, "gif-loop" ) ) {
int gif_loop; int gif_loop;


if( vips_image_get_int( image, "gif-loop", &gif_loop ) ) if( vips_image_get_int( write->image, "gif-loop", &gif_loop ) )
return( -1 ); return( -1 );


vips_webp_set_count( write, gif_loop ); vips_webp_set_count( write, gif_loop );
} }


/* Add extra metadata. /* Add extra metadata.
*/ */
if( !strip && if( !write->strip &&
vips_webp_add_chunks( write, image ) ) vips_webp_add_chunks( write ) )
return( -1 ); return( -1 );


if( WebPMuxAssemble( write->mux, &data ) != WEBP_MUX_OK ) { if( WebPMuxAssemble( write->mux, &data ) != WEBP_MUX_OK ) {
Expand Down Expand Up @@ -524,12 +533,12 @@ vips__webp_write_stream( VipsImage *image, VipsStreamo *streamo,
alpha_q, reduction_effort, min_size, kmin, kmax, strip ) ) alpha_q, reduction_effort, min_size, kmin, kmax, strip ) )
return( -1 ); return( -1 );


if( write_webp( &write, image ) ) { if( write_webp( &write ) ) {
vips_webp_write_unset( &write ); vips_webp_write_unset( &write );
return( -1 ); return( -1 );
} }


if( vips_webp_add_metadata( &write, image, strip ) ) { if( vips_webp_add_metadata( &write ) ) {
vips_webp_write_unset( &write ); vips_webp_write_unset( &write );
return( -1 ); return( -1 );
} }
Expand Down
5 changes: 3 additions & 2 deletions libvips/iofuncs/header.c
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ vips_image_set( VipsImage *image, const char *name, GValue *value )
/* If this image is shared, block metadata changes. /* If this image is shared, block metadata changes.
*/ */
if( G_OBJECT( image )->ref_count > 1 ) { if( G_OBJECT( image )->ref_count > 1 ) {
g_info( "can't set metadata \"%s\" on shared image", name ); g_warning( "can't set metadata \"%s\" on shared image", name );
return; return;
} }


Expand Down Expand Up @@ -1223,7 +1223,8 @@ vips_image_remove( VipsImage *image, const char *name )
/* If this image is shared, block metadata changes. /* If this image is shared, block metadata changes.
*/ */
if( G_OBJECT( image )->ref_count > 1 ) { if( G_OBJECT( image )->ref_count > 1 ) {
g_info( "can't remove metadata \"%s\" on shared image", name ); g_warning( "can't remove metadata \"%s\" on shared image",
name );
return( FALSE ); return( FALSE );
} }


Expand Down

0 comments on commit acd9101

Please sign in to comment.