diff --git a/ChangeLog b/ChangeLog index 37295272..d5ff4c21 100644 --- a/ChangeLog +++ b/ChangeLog @@ -20,7 +20,7 @@ - fix race in temp filename creation [lhecker] - add @reduction_effort param to webpsave [lovell] - add @option_string param to thumbnail_buffer [kleisauke] -- add XMP, IPCT, ICC etc. to magickload +- add XMP, IPCT, ICC, EXIF etc. support to magickload/magicksave 4/1/19 started 8.7.4 - magickload with magick6 API did not chain exceptions correctly causing a diff --git a/libvips/foreign/magick.c b/libvips/foreign/magick.c index 04c607a3..6b69bd87 100644 --- a/libvips/foreign/magick.c +++ b/libvips/foreign/magick.c @@ -43,6 +43,7 @@ #include #include +#include #include "pforeign.h" #include "magick.h" @@ -107,6 +108,28 @@ magick_set_profile( Image *image, return( result ); } +void * +magick_profile_map( Image *image, MagickMapProfileFn fn, void *a ) +{ + char *name; + + ResetImageProfileIterator( image ); + while( (name = GetNextImageProfile( image )) ) { + const StringInfo *profile; + void *data; + size_t length; + void *result; + + profile = GetImageProfile( image, name ); + data = GetStringInfoDatum( profile ); + length = GetStringInfoLength( profile ); + if( (result = fn( image, name, data, length, a )) ) + return( result ); + } + + return( NULL ); +} + ExceptionInfo * magick_acquire_exception( void ) { @@ -262,6 +285,44 @@ magick_set_profile( Image *image, return( result ); } +void * +magick_profile_map( Image *image, MagickMapProfileFn fn, void *a ) +{ + const char *name; + const void *data; + size_t length; + void *result; + +#ifdef HAVE_RESETIMAGEPROFILEITERATOR + ResetImageProfileIterator( image ); + while( (name = GetNextImageProfile( image )) ) { + const StringInfo *profile; + + profile = GetImageProfile( image, name ); + data = GetStringInfoDatum( profile ); + length = GetStringInfoLength( profile ); + if( (result = fn( image, name, data, length, a )) ) + return( result ); + } +#else /*!HAVE_RESETIMAGEPROFILEITERATOR*/ +{ + ImageProfileIterator *iter; + + iter = AllocateImageProfileIterator( image ); + while( NextImageProfile( iter, + &name, (const unsigned char **) &data, &length ) ) { + if( (result = fn( image, name, data, length, a )) ) { + DeallocateImageProfileIterator( iter ); + return( result ); + } + } + DeallocateImageProfileIterator( iter ); +} +#endif /*HAVE_RESETIMAGEPROFILEITERATOR*/ + + return( NULL ); +} + ExceptionInfo * magick_acquire_exception( void ) { @@ -422,4 +483,101 @@ magick_genesis( void ) VIPS_ONCE( &once, magick_genesis_cb, NULL ); } +/* Set vips metadata from a magick profile. + */ +static void * +magick_set_vips_profile_cb( Image *image, + const char *name, const void *data, size_t length, void *a ) +{ + VipsImage *im = (VipsImage *) a; + + char name_text[256]; + VipsBuf vips_name = VIPS_BUF_STATIC( name_text ); + + if( strcmp( name, "XMP" ) == 0 ) + vips_buf_appendf( &vips_name, VIPS_META_XMP_NAME ); + else if( strcmp( name, "IPTC" ) == 0 ) + vips_buf_appendf( &vips_name, VIPS_META_IPTC_NAME ); + else if( strcmp( name, "ICM" ) == 0 ) + vips_buf_appendf( &vips_name, VIPS_META_ICC_NAME ); + else if( strcmp( name, "EXIF" ) == 0 ) + vips_buf_appendf( &vips_name, VIPS_META_EXIF_NAME ); + else + vips_buf_appendf( &vips_name, "magickprofile-%s", name ); + + vips_image_set_blob_copy( im, + vips_buf_all( &vips_name ), data, length ); + + if( strcmp( name, "exif" ) == 0 ) + (void) vips__exif_parse( im ); + + return( NULL ); +} + +/* Set vips metadata from ImageMagick profiles. + */ +int +magick_set_vips_profile( VipsImage *im, Image *image ) +{ + if( magick_profile_map( image, magick_set_vips_profile_cb, im ) ) + return( -1 ); + + return( 0 ); +} + +typedef struct { + Image *image; + ExceptionInfo *exception; +} CopyProfileInfo; + +static void * +magick_set_magick_profile_cb( VipsImage *im, + const char *name, GValue *value, CopyProfileInfo *info ) +{ + char txt[256]; + VipsBuf buf = VIPS_BUF_STATIC( txt ); + const void *data; + size_t length; + + if( strcmp( name, VIPS_META_XMP_NAME ) == 0 ) + vips_buf_appendf( &buf, "XMP" ); + else if( strcmp( name, VIPS_META_IPTC_NAME ) == 0 ) + vips_buf_appendf( &buf, "IPTC" ); + else if( strcmp( name, VIPS_META_ICC_NAME ) == 0 ) + vips_buf_appendf( &buf, "ICM" ); + else if( strcmp( name, VIPS_META_EXIF_NAME ) == 0 ) + vips_buf_appendf( &buf, "EXIF" ); + else if( vips_isprefix( "magickprofile-", name ) ) + vips_buf_appendf( &buf, + "%s", name + strlen( "magickprofile-" ) ); + + if( vips_buf_is_empty( &buf ) ) + return( NULL ); + if( !vips_image_get_typeof( im, name ) ) + return( NULL ); + if( vips_image_get_blob( im, name, &data, &length ) ) + return( im ); + + if( !magick_set_profile( info->image, + vips_buf_all( &buf ), data, length, info->exception ) ) + return( im ); + + return( NULL ); +} + +int +magick_set_magick_profile( Image *image, + VipsImage *im, ExceptionInfo *exception ) +{ + CopyProfileInfo info; + + info.image = image; + info.exception = exception; + if( vips_image_map( im, + (VipsImageMapFn) magick_set_magick_profile_cb, &info ) ) + return( -1 ); + + return( 0 ); +} + #endif /*HAVE_MAGICK*/ diff --git a/libvips/foreign/magick.h b/libvips/foreign/magick.h index 83d8a8df..0c78e3c6 100644 --- a/libvips/foreign/magick.h +++ b/libvips/foreign/magick.h @@ -55,6 +55,9 @@ void *magick_images_to_blob( const ImageInfo *image_info, Image *images, size_t *length, ExceptionInfo *exception ); void magick_set_property( Image *image, const char *property, const char *value, ExceptionInfo *exception ); +typedef void *(*MagickMapProfileFn)( Image *image, + const char *name, const void *data, size_t length, void *a ); +void *magick_profile_map( Image *image, MagickMapProfileFn fn, void *a ); int magick_set_profile( Image *image, const char *name, const void *data, size_t length, ExceptionInfo *exception ); @@ -74,4 +77,8 @@ void magick_vips_error( const char *domain, ExceptionInfo *exception ); void magick_genesis( void ); +int magick_set_vips_profile( VipsImage *im, Image *image ); +int magick_set_magick_profile( Image *image, + VipsImage *im, ExceptionInfo *exception ); + #endif /*HAVE_MAGICK6*/ diff --git a/libvips/foreign/magick2vips.c b/libvips/foreign/magick2vips.c index 0b7c7f97..3338dc26 100644 --- a/libvips/foreign/magick2vips.c +++ b/libvips/foreign/magick2vips.c @@ -112,6 +112,7 @@ #include #include +#include #include @@ -423,43 +424,10 @@ parse_header( Read *read ) vips_image_pipelinev( im, VIPS_DEMAND_STYLE_SMALLTILE, NULL ); -#ifdef HAVE_RESETIMAGEPROFILEITERATOR -{ - /* "Profiles" are things like icc profiles, xmp, iptc, etc. and are - * stored as blobs, since they may contain embedded \0. + /* Set vips metadata from ImageMagick profiles. */ - char *key; - - ResetImageProfileIterator( image ); - while( (key = GetNextImageProfile( image )) ) { - char name_text[256]; - VipsBuf name = VIPS_BUF_STATIC( name_text ); - const StringInfo *profile; - void *data; - size_t length; - - if( strcmp( key, "xmp" ) == 0 ) - vips_buf_appendf( &name, VIPS_META_XMP_NAME ); - else if( strcmp( key, "iptc" ) == 0 ) - vips_buf_appendf( &name, VIPS_META_IPTC_NAME ); - else if( strcmp( key, "icc" ) == 0 ) - vips_buf_appendf( &name, VIPS_META_ICC_NAME ); - else if( strcmp( key, "exif" ) == 0 ) - vips_buf_appendf( &name, VIPS_META_EXIF_NAME ); - else - vips_buf_appendf( &name, "magickprofile-%s", key ); - - profile = GetImageProfile( image, key ); - data = GetStringInfoDatum( profile ); - length = GetStringInfoLength( profile ); - vips_image_set_blob_copy( im, vips_buf_all( &name ), - data, length ); - - if( strcmp( key, "exif" ) == 0 ) - (void) vips__exif_parse( im ); - } -} -#endif /*HAVE_RESETIMAGEPROFILEITERATOR*/ + if( magick_set_vips_profile( im, image ) ) + return( -1 ); #ifdef HAVE_RESETIMAGEPROPERTYITERATOR { diff --git a/libvips/foreign/magick7load.c b/libvips/foreign/magick7load.c index d9e16fd2..66d394e7 100644 --- a/libvips/foreign/magick7load.c +++ b/libvips/foreign/magick7load.c @@ -546,36 +546,10 @@ vips_foreign_load_magick7_parse( VipsForeignLoadMagick7 *magick7, vips_image_set_string( out, vips_buf_all( &name ), value ); } - /* All the binary metadata. + /* Set vips metadata from ImageMagick profiles. */ - ResetImageProfileIterator( image ); - while( (key = GetNextImageProfile( image )) ) { - char name_text[256]; - VipsBuf name = VIPS_BUF_STATIC( name_text ); - const StringInfo *profile; - void *data; - size_t length; - - if( strcmp( key, "xmp" ) == 0 ) - vips_buf_appendf( &name, VIPS_META_XMP_NAME ); - else if( strcmp( key, "iptc" ) == 0 ) - vips_buf_appendf( &name, VIPS_META_IPTC_NAME ); - else if( strcmp( key, "icc" ) == 0 ) - vips_buf_appendf( &name, VIPS_META_ICC_NAME ); - else if( strcmp( key, "exif" ) == 0 ) - vips_buf_appendf( &name, VIPS_META_EXIF_NAME ); - else - vips_buf_appendf( &name, "magickprofile-%s", key ); - - profile = GetImageProfile( image, key ); - data = GetStringInfoDatum( profile ); - length = GetStringInfoLength( profile ); - vips_image_set_blob_copy( out, vips_buf_all( &name ), - data, length ); - - if( strcmp( key, "exif" ) == 0 ) - (void) vips__exif_parse( im ); - } + if( magick_set_vips_profile( out, image ) ) + return( -1 ); magick7->n_pages = GetImageListLength( GetFirstImageInList( image ) ); #ifdef DEBUG diff --git a/libvips/foreign/magicksave.c b/libvips/foreign/magicksave.c index bedcae4c..38e3799b 100644 --- a/libvips/foreign/magicksave.c +++ b/libvips/foreign/magicksave.c @@ -89,59 +89,14 @@ vips_foreign_save_magick_dispose( GObject *gobject ) dispose( gobject ); } -typedef struct { - VipsForeignSaveMagick *magick; - Image *image; -} CopyProfileInfo; - -static void * -vips_foreign_save_magick_copy_profile( VipsImage *im, - const char *name, GValue *value, CopyProfileInfo *info ) -{ - char txt[256]; - VipsBuf buf = VIPS_BUF_STATIC( txt ); - const void *data; - size_t length; - int result; - - if( strcmp( name, VIPS_META_XMP_NAME ) == 0 ) - vips_buf_appendf( &buf, "xmp" ); - else if( strcmp( name, VIPS_META_IPTC_NAME ) == 0 ) - vips_buf_appendf( &buf, "iptc" ); - else if( strcmp( name, VIPS_META_ICC_NAME ) == 0 ) - vips_buf_appendf( &buf, "icc" ); - else if( strcmp( name, VIPS_META_EXIF_NAME ) == 0 ) - vips_buf_appendf( &buf, "exif" ); - else if( vips_isprefix( "magickprofile-", name ) ) - vips_buf_appendf( &buf, - "%s", name + strlen( "magickprofile-" ) ); - - if( vips_buf_is_empty( &buf ) ) - return( NULL ); - if( !vips_image_get_typeof( im, name ) ) - return( NULL ); - if( vips_image_get_blob( im, name, &data, &length ) ) - return( im ); - - result = magick_set_profile( info->image, - vips_buf_all( &buf ), data, length, info->magick->exception ); - if( !result ) { - VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( info->magick ); - - magick_vips_error( class->nickname, info->magick->exception ); - return( im ); - } - - return( NULL ); -} - static int vips_foreign_save_magick_set_properties( VipsForeignSaveMagick *magick, Image *image, VipsImage *im ) { + VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( magick ); + int number; const char *str; - CopyProfileInfo info; if( vips_image_get_typeof( im, "gif-delay" ) && !vips_image_get_int( im, "gif-delay", &number ) ) @@ -155,12 +110,10 @@ vips_foreign_save_magick_set_properties( VipsForeignSaveMagick *magick, !vips_image_get_string( im, "gif-comment", &str ) ) magick_set_property( image, "comment", str, magick->exception ); - info.magick = magick; - info.image = image; - if( vips_image_map( im, - (VipsImageMapFn) vips_foreign_save_magick_copy_profile, - &info ) ) - return( -1 ); + if( magick_set_magick_profile( image, im, magick->exception ) ) { + magick_vips_error( class->nickname, magick->exception ); + return( -1 ); + } return( 0 ); } diff --git a/test/test-suite/test_foreign.py b/test/test-suite/test_foreign.py index b92940c9..6591d9a8 100644 --- a/test/test-suite/test_foreign.py +++ b/test/test-suite/test_foreign.py @@ -1,5 +1,6 @@ # vim: set fileencoding=utf-8 : -import gc + +import sys import os import shutil import tempfile @@ -428,12 +429,6 @@ class TestForeign: # some IMs are 3 bands, some are 1, can't really test # assert im.bands == 1 - # added in 8.7 - self.save_load_file(".bmp", "", self.colour, 0) - self.save_load_buffer("magicksave_buffer", "magickload_buffer", - self.colour, 0, format="BMP") - self.save_load("%s.bmp", self.colour) - # libvips has its own sniffer for ICO, test that with open(ICO_FILE, 'rb') as f: buf = f.read() @@ -442,6 +437,27 @@ class TestForeign: assert im.width == 16 assert im.height == 16 + # added in 8.7 + @skip_if_no("magicksave") + def test_magicksave(self): + # save to a file and load again ... we can't use save_load_file since + # we want to make sure we use magickload/save + # don't use BMP - GraphicsMagick always adds an alpha + # don't use TIF - IM7 will save as 16-bit + filename = temp_filename(self.tempdir, ".jpg") + + self.colour.magicksave(filename) + x = pyvips.Image.magickload(filename) + + assert self.colour.width == x.width + assert self.colour.height == x.height + assert self.colour.bands == x.bands + max_diff = (self.colour - x).abs().max() + assert max_diff < 20 + + self.save_load_buffer("magicksave_buffer", "magickload_buffer", + self.colour, 20, format="JPG") + @skip_if_no("webpload") def test_webp(self): def webp_valid(im):