From 2c646a02e4f4f8536a99a0f56a0d8ef9d7f1ce2c Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Wed, 13 Apr 2016 08:39:29 +0100 Subject: [PATCH] better jpeg autorot jpeg write is more careful about removing exif tags that have been removed from the image metadata it failed previously if there were multiple Orientation tags --- ChangeLog | 1 + libvips/conversion/autorot.c | 23 +++++++-- libvips/foreign/vips2jpeg.c | 83 +++++++++++++++++++++++++++++-- libvips/include/vips/conversion.h | 1 + tools/vipsthumbnail.c | 7 +-- 5 files changed, 105 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index eb1986af..961f5d71 100644 --- a/ChangeLog +++ b/ChangeLog @@ -23,6 +23,7 @@ - @out_format in vips_system() can contain [options] - webpsave_buffer no longer ignores @lossless, thanks aaron42net - float tiff tagged as scRGB to match photoshop convention, thanks Murat +- better jpeg autorot, thanks otto 24/3/16 started 8.2.4 - fix nohalo and vsqbs interpolators, thanks Rafael diff --git a/libvips/conversion/autorot.c b/libvips/conversion/autorot.c index a9d4d1d0..17ccb120 100644 --- a/libvips/conversion/autorot.c +++ b/libvips/conversion/autorot.c @@ -33,6 +33,10 @@ */ +/* +#define DEBUG + */ + #ifdef HAVE_CONFIG_H #include #endif /*HAVE_CONFIG_H*/ @@ -64,7 +68,7 @@ vips_autorot_get_angle_sub( VipsImage *image, const char *orientation; if( vips_isprefix( "exif-", field ) && - vips_ispostfix( "-Orientation", field ) && + vips_ispostfix( field, "-Orientation" ) && !vips_image_get_string( image, field, &orientation ) ) { if( vips_isprefix( "6", orientation ) ) *angle = VIPS_ANGLE_D90; @@ -82,6 +86,10 @@ vips_autorot_get_angle_sub( VipsImage *image, * * http://www.80sidea.com/archives/2316 */ + + /* Stop searching. + */ + return( image ); } return( NULL ); @@ -106,6 +114,10 @@ vips_autorot_get_angle( VipsImage *image ) angle = VIPS_ANGLE_D0; (void) vips_image_map( image, vips_autorot_get_angle_sub, &angle ); +#ifdef DEBUG + printf( "vips_autorot_get_angle: %d\n", angle ); +#endif /*DEBUG*/ + return( angle ); } @@ -114,8 +126,13 @@ vips_autorot_remove_angle_sub( VipsImage *image, const char *field, GValue *value, void *my_data ) { if( vips_isprefix( "exif-", field ) && - vips_ispostfix( "-Orientation", field ) ) + vips_ispostfix( field, "-Orientation" ) ) { +#ifdef DEBUG + printf( "vips_autorot_remove_angle: %s\n", field ); +#endif /*DEBUG*/ + (void) vips_image_remove( image, field ); + } return( NULL ); } @@ -128,7 +145,7 @@ vips_autorot_remove_angle_sub( VipsImage *image, * * See also: vips_autorot_get_angle(). */ -static void +void vips_autorot_remove_angle( VipsImage *image ) { (void) vips_image_map( image, vips_autorot_remove_angle_sub, NULL ); diff --git a/libvips/foreign/vips2jpeg.c b/libvips/foreign/vips2jpeg.c index 71b71ae2..d714cfc9 100644 --- a/libvips/foreign/vips2jpeg.c +++ b/libvips/foreign/vips2jpeg.c @@ -49,7 +49,7 @@ * 24/11/11 * - turn into a set of write fns ready to be called from a class * 7/8/12 - * - use VIPS_META_RESOLUTION_UNIT to select resoltuion unit + * - use VIPS_META_RESOLUTION_UNIT to select resolution unit * 16/11/12 * - read ifds from exif fields * - optionally parse rationals as a/b @@ -72,6 +72,8 @@ * - set arbitrary exif tags from metadata * 25/11/15 * - don't write JFIF headers if we are stripping, thanks Benjamin + * 13/4/16 + * - remove deleted exif fields more carefully */ /* @@ -626,12 +628,87 @@ vips_exif_image_field( VipsImage *image, return( NULL ); } +typedef struct _VipsExif { + VipsImage *image; + ExifData *ed; + ExifContent *content; + GSList *to_remove; +} VipsExif; + +static void +vips_exif_exif_entry( ExifEntry *entry, VipsExif *ve ) +{ + const char *tag_name; + char vips_name_txt[256]; + VipsBuf vips_name = VIPS_BUF_STATIC( vips_name_txt ); + + if( !(tag_name = exif_tag_get_name( entry->tag )) ) + return; + + vips_buf_appendf( &vips_name, "exif-ifd%d-%s", + exif_entry_get_ifd( entry ), tag_name ); + + /* Does this field exist on the image? If not, schedule it for + * removal. + */ + if( !vips_image_get_typeof( ve->image, vips_buf_all( &vips_name ) ) ) + ve->to_remove = g_slist_prepend( ve->to_remove, entry ); +} + +static void * +vips_exif_exif_remove( ExifEntry *entry, VipsExif *ve ) +{ +#ifdef DEBUG +{ + const char *tag_name; + char vips_name_txt[256]; + VipsBuf vips_name = VIPS_BUF_STATIC( vips_name_txt ); + + tag_name = exif_tag_get_name( entry->tag ); + vips_buf_appendf( &vips_name, "exif-ifd%d-%s", + exif_entry_get_ifd( entry ), tag_name ); + + printf( "vips_exif_exif_remove: %s\n", vips_buf_all( &vips_name ) ); +} +#endif /*DEBUG*/ + + exif_content_remove_entry( ve->content, entry ); + + return( NULL ); +} + +static void +vips_exif_exif_content( ExifContent *content, VipsExif *ve ) +{ + ve->content = content; + ve->to_remove = NULL; + exif_content_foreach_entry( content, + (ExifContentForeachEntryFunc) vips_exif_exif_entry, ve ); + vips_slist_map2( ve->to_remove, + (VipsSListMap2Fn) vips_exif_exif_remove, ve, NULL ); + VIPS_FREEF( g_slist_free, ve->to_remove ); +} + static void vips_exif_update( ExifData *ed, VipsImage *image ) { + VipsExif ve; + VIPS_DEBUG_MSG( "vips_exif_update: \n" ); + /* Walk the image and update any stuff that's been changed in image + * metadata. + */ vips_image_map( image, vips_exif_image_field, ed ); + + /* Walk the exif and look for any fields which are NOT in image + * metadata. They must have been removed ... remove them from exif as + * well. + */ + ve.image = image; + ve.ed = ed; + exif_data_foreach_content( ed, + (ExifDataForeachContentFunc) vips_exif_exif_content, &ve ); } #endif /*HAVE_EXIF*/ @@ -667,9 +744,7 @@ write_blob( Write *write, const char *field, int app ) #endif /*DEBUG*/ jpeg_write_marker( &write->cinfo, app, - data, data_length ); - } - } + data, data_length ); } } return( 0 ); } diff --git a/libvips/include/vips/conversion.h b/libvips/include/vips/conversion.h index 2473531a..7c419f61 100644 --- a/libvips/include/vips/conversion.h +++ b/libvips/include/vips/conversion.h @@ -127,6 +127,7 @@ int vips_rot( VipsImage *in, VipsImage **out, VipsAngle angle, ... ) int vips_rot45( VipsImage *in, VipsImage **out, ... ) __attribute__((sentinel)); VipsAngle vips_autorot_get_angle( VipsImage *image ); +void vips_autorot_remove_angle( VipsImage *image ); int vips_autorot( VipsImage *in, VipsImage **out, ... ) __attribute__((sentinel)); int vips_zoom( VipsImage *in, VipsImage **out, int xfac, int yfac, ... ) diff --git a/tools/vipsthumbnail.c b/tools/vipsthumbnail.c index 82bf9031..34546e59 100644 --- a/tools/vipsthumbnail.c +++ b/tools/vipsthumbnail.c @@ -93,8 +93,6 @@ #include #include -#define ORIENTATION ("exif-ifd0-Orientation") - /* Default settings. We change the default to bicubic in main() if * this vips has been compiled with bicubic support. */ @@ -582,6 +580,9 @@ thumbnail_rotate( VipsObject *process, VipsImage *im ) if( rotate_image && angle != VIPS_ANGLE_D0 ) { + vips_info( "vipsthumbnail", "rotating by %s", + vips_enum_nick( VIPS_TYPE_ANGLE, angle ) ); + /* Need to copy to memory, we have to stay seq. */ t[0] = vips_image_new_memory(); @@ -590,7 +591,7 @@ thumbnail_rotate( VipsObject *process, VipsImage *im ) return( NULL ); im = t[1]; - (void) vips_image_remove( im, ORIENTATION ); + vips_autorot_remove_angle( im ); } return( im );