always copy before exif_update

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 https://github.com/lovell/sharp/issues/1986
This commit is contained in:
John Cupitt 2019-11-28 14:45:02 +00:00
parent 20cee5d041
commit acd9101037
5 changed files with 67 additions and 36 deletions

View File

@ -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

View File

@ -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;
@ -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 );
@ -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;
@ -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 );
@ -176,7 +181,7 @@ 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;
@ -184,7 +189,7 @@ vips_foreign_save_heif_write_page( VipsForeignSaveHeif *heif, int page )
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 );
@ -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;
@ -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 );
@ -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 );
@ -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 );
@ -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 );
@ -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 );

View File

@ -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 );
} }
@ -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;
@ -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 );
} }

View File

@ -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
@ -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;
@ -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",
@ -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
@ -437,7 +446,7 @@ 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;
@ -445,11 +454,11 @@ vips_webp_add_chunks( VipsWebPWrite *write, VipsImage *image )
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 ) )
@ -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;
@ -480,10 +489,10 @@ 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 );
@ -491,8 +500,8 @@ vips_webp_add_metadata( VipsWebPWrite *write, VipsImage *image, gboolean strip )
/* 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 ) {
@ -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 );
} }

View File

@ -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;
} }
@ -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 );
} }