diff --git a/libvips/foreign/jpeg.h b/libvips/foreign/jpeg.h index ac7e6edc..8c1e018a 100644 --- a/libvips/foreign/jpeg.h +++ b/libvips/foreign/jpeg.h @@ -77,6 +77,8 @@ typedef struct { void vips__new_output_message( j_common_ptr cinfo ); void vips__new_error_exit( j_common_ptr cinfo ); +int vips__set_exif_resolution( ExifData *ed, VipsImage *im ); + #ifdef __cplusplus } #endif /*__cplusplus*/ diff --git a/libvips/foreign/jpeg2vips.c b/libvips/foreign/jpeg2vips.c index b1d82165..2a4eba1b 100644 --- a/libvips/foreign/jpeg2vips.c +++ b/libvips/foreign/jpeg2vips.c @@ -350,17 +350,25 @@ show_values( ExifData *data ) #endif /*HAVE_EXIF*/ #ifdef HAVE_EXIF -static ExifData* -vips_exif_load_data_without_fix(const unsigned char* data, unsigned int size) +/* Like exif_data_new_from_data(), but don't default missing fields. + * + * If we do exif_data_new_from_data(), then missing fields are set to + * their default value and we won't know about it. + */ +static ExifData * +vips_exif_load_data_without_fix( void *data, int data_length ) { - ExifData *edata = exif_data_new(); + ExifData *ed; - if ( !edata ) - return NULL; + if( !(ed = exif_data_new()) ) { + vips_error( "VipsJpeg", "%s", _( "unable to init exif" ) ); + return( NULL ); + } - exif_data_unset_option(edata, EXIF_DATA_OPTION_FOLLOW_SPECIFICATION); - exif_data_load_data (edata, data, size); - return edata; + exif_data_unset_option( ed, EXIF_DATA_OPTION_FOLLOW_SPECIFICATION ); + exif_data_load_data( ed, data, data_length ); + + return( ed ); } static int @@ -542,34 +550,6 @@ get_entry_double( ExifData *ed, int ifd, ExifTag tag, double *out ) return( vips_exif_get_double( ed, entry, 0, out ) ); } -static void -set_entry_double( ExifData *ed, int ifd, ExifTag tag, double in) -{ - ExifEntry *entry; - - if( (entry = exif_content_get_entry( ed->ifd[ifd], tag )) ) { - /* Let's remove the current value */ - exif_content_remove_entry( ed->ifd[ifd], entry ); - entry = NULL; - } - - if ( !( entry = exif_entry_new() ) ) - return; - - entry->tag = tag; - exif_content_add_entry( ed->ifd[ifd], entry ); - - exif_entry_initialize( entry, tag ); - - /* This is a simple yet sufficient implementation */ - ExifRational value; - value.numerator = in * 1000; - value.denominator = 1000; - - if ( entry->data ) - exif_set_rational( entry->data, exif_data_get_byte_order( ed ), value ); -} - static int get_entry_int( ExifData *ed, int ifd, ExifTag tag, int *out ) { @@ -680,30 +660,24 @@ parse_exif( VipsImage *im, void *data, int data_length ) show_values( ed ); #endif /*DEBUG_VERBOSE*/ + /* Look for resolution fields and use them to set the VIPS + * xres/yres fields. + * + * If the fields are missing, write the ones from jfif. + */ + if( res_from_exif( im, ed ) && + vips__set_exif_resolution( ed, im ) ) + return( -1 ); + + /* Make sure all required fields are there before we attach to vips + * metadata. + */ + exif_data_fix( ed ); + /* Attach informational fields for what we find. - - FIXME ... better to have this in the UI layer? - - Or we could attach non-human-readable tags here (int, - double etc) and then move the human stuff to the UI - layer? - */ ve.image = im; ve.ed = ed; - - /* Look for resolution fields and use them to set the VIPS - * xres/yres fields. - * If the fields are missing, write the one from the jfif. - */ - if ( res_from_exif( im, ed ) ) { - set_entry_double( ed, 0, EXIF_TAG_X_RESOLUTION, im->Xres ); - set_entry_double( ed, 0, EXIF_TAG_Y_RESOLUTION, im->Yres ); - } - - /* Let's make sure the fields are valid. */ - exif_data_fix( ed ); - exif_data_foreach_content( ed, (ExifDataForeachContentFunc) attach_exif_content, &ve ); diff --git a/libvips/foreign/vips2jpeg.c b/libvips/foreign/vips2jpeg.c index 04ec6aa6..ceb36c89 100644 --- a/libvips/foreign/vips2jpeg.c +++ b/libvips/foreign/vips2jpeg.c @@ -273,29 +273,21 @@ vips_exif_set_int( ExifData *ed, static void vips_exif_double_to_rational( double value, ExifRational *rv ) { - unsigned int scale; - - /* We scale up to fill uint32, then set that as the - * denominator. Try to avoid generating 0. + /* We will usually set factors of 10, so use 1000 as the denominator + * and it'll probably be OK. */ - scale = (unsigned int) ((UINT_MAX - 1000) / value); - scale = scale == 0 ? 1 : scale; - rv->numerator = value * scale; - rv->denominator = scale; + rv->numerator = value * 1000; + rv->denominator = 1000; } static void vips_exif_double_to_srational( double value, ExifSRational *srv ) { - int scale; - - /* We scale up to fill int32, then set that as the - * denominator. Try to avoid generating 0. + /* We will usually set factors of 10, so use 1000 as the denominator + * and it'll probably be OK. */ - scale = (int) ((INT_MAX - 1000) / value); - scale = scale == 0 ? 1 : scale; - srv->numerator = value * scale; - srv->denominator = scale; + srv->numerator = value * 1000; + srv->denominator = 1000; } /* Parse a char* into an ExifRational. We allow floats as well. @@ -461,15 +453,17 @@ write_tag( ExifData *ed, int ifd, ExifTag tag, write_fn fn, void *data ) /* This is different, we set the xres/yres from the vips header rather than * from the exif tags on the image metadata. + * + * This is also called from the jpg reader to fix up bad exif resoltion. */ -static int -set_exif_resolution( ExifData *ed, VipsImage *im ) +int +vips__set_exif_resolution( ExifData *ed, VipsImage *im ) { double xres, yres; const char *p; int unit; - VIPS_DEBUG_MSG( "set_exif_resolution: vips res of %g, %g\n", + VIPS_DEBUG_MSG( "vips__set_exif_resolution: vips res of %g, %g\n", im->Xres, im->Yres ); /* Default to inches, more progs support it. @@ -823,7 +817,7 @@ write_exif( Write *write ) /* Update EXIF resolution from the vips image header. */ - if( set_exif_resolution( ed, write->in ) ) { + if( vips__set_exif_resolution( ed, write->in ) ) { exif_data_free( ed ); return( -1 ); }