Skip to content

Commit

Permalink
keep exif rationals as a/b
Browse files Browse the repository at this point in the history 
we went via double before, which caused awful problems trying to
generate nice rationals again on save

keep everything as a/b as long as we can
  • Loading branch information
@jcupitt
jcupitt committed Nov 16, 2012
1 parent fcc3302 commit 74f545f
Show file tree
Hide file tree
Showing 3 changed files with 190 additions and 57 deletions.
3 changes: 0 additions & 3 deletions TODO
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@

- test with interpolators




- the operation cache needs to detect invalidate

tricky!
Expand Down
83 changes: 58 additions & 25 deletions libvips/foreign/jpeg2vips.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@
* - switch to lazy reading
* 7/8/12
* - note EXIF resolution unit in VIPS_META_RESOLUTION_UNIT
* 16/11/12
* - tag exif fields with their ifd
* - attach rationals as a/b, don't convert to double
*/

/*
Expand Down Expand Up @@ -337,31 +340,56 @@ vips_exif_get_int( ExifData *ed,
}

static int
vips_exif_get_double( ExifData *ed,
ExifEntry *entry, unsigned long component, double *out )
vips_exif_get_rational( ExifData *ed,
ExifEntry *entry, unsigned long component, ExifRational *out )
{
ExifByteOrder bo = exif_data_get_byte_order( ed );
size_t sizeof_component = entry->size / entry->components;
size_t offset = component * sizeof_component;

if( entry->format == EXIF_FORMAT_RATIONAL ) {
ExifRational value;
ExifByteOrder bo = exif_data_get_byte_order( ed );
size_t sizeof_component = entry->size / entry->components;
size_t offset = component * sizeof_component;

value = exif_get_rational( entry->data + offset, bo );
*out = (double) value.numerator / value.denominator;
*out = exif_get_rational( entry->data + offset, bo );
}
else if( entry->format == EXIF_FORMAT_SRATIONAL ) {
ExifSRational value;
else
return( -1 );

value = exif_get_srational( entry->data + offset, bo );
*out = (double) value.numerator / value.denominator;
return( 0 );
}

static int
vips_exif_get_srational( ExifData *ed,
ExifEntry *entry, unsigned long component, ExifSRational *out )
{
if( entry->format == EXIF_FORMAT_SRATIONAL ) {
ExifByteOrder bo = exif_data_get_byte_order( ed );
size_t sizeof_component = entry->size / entry->components;
size_t offset = component * sizeof_component;

*out = exif_get_srational( entry->data + offset, bo );
}
else
return( -1 );

return( 0 );
}

static int
vips_exif_get_double( ExifData *ed,
ExifEntry *entry, unsigned long component, double *out )
{
ExifRational rv;
ExifSRational srv;

if( !vips_exif_get_rational( ed, entry, component, &rv ) )
*out = (double) rv.numerator / rv.denominator;
else if( !vips_exif_get_srational( ed, entry, component, &srv ) )
*out = (double) srv.numerator / srv.denominator;
else
return( -1 );

return( 0 );
}

/* Save an exif value to a string in a way that we can restore. We only bother
* for the simple formats (that a client might try to change) though.
*
Expand All @@ -372,12 +400,12 @@ vips_exif_to_s( ExifData *ed, ExifEntry *entry, VipsBuf *buf )
{
unsigned long i;
int iv;
double dv;
ExifRational rv;
ExifSRational srv;
char txt[256];

if( entry->format == EXIF_FORMAT_ASCII )
vips_buf_appendf( buf, "%s ", entry->data );

else if( entry->components < 10 &&
!vips_exif_get_int( ed, entry, 0, &iv ) ) {
for( i = 0; i < entry->components; i++ ) {
Expand All @@ -386,13 +414,19 @@ vips_exif_to_s( ExifData *ed, ExifEntry *entry, VipsBuf *buf )
}
}
else if( entry->components < 10 &&
!vips_exif_get_double( ed, entry, 0, &dv ) ) {
!vips_exif_get_rational( ed, entry, 0, &rv ) ) {
for( i = 0; i < entry->components; i++ ) {
vips_exif_get_double( ed, entry, i, &dv );
/* Need to be locale independent.
*/
g_ascii_dtostr( txt, 256, dv );
vips_buf_appendf( buf, "%s ", txt );
vips_exif_get_rational( ed, entry, i, &rv );
vips_buf_appendf( buf, "%u/%u ",
rv.numerator, rv.denominator );
}
}
else if( entry->components < 10 &&
!vips_exif_get_srational( ed, entry, 0, &srv ) ) {
for( i = 0; i < entry->components; i++ ) {
vips_exif_get_srational( ed, entry, i, &srv );
vips_buf_appendf( buf, "%d/%d ",
srv.numerator, srv.denominator );
}
}
else
Expand Down Expand Up @@ -438,12 +472,11 @@ attach_exif_content( ExifContent *content, VipsExif *ve )
}

static int
get_entry_rational( ExifData *ed, int ifd, ExifTag tag, double *out )
get_entry_double( ExifData *ed, int ifd, ExifTag tag, double *out )
{
ExifEntry *entry;

if( !(entry = exif_content_get_entry( ed->ifd[ifd], tag )) ||
entry->format != EXIF_FORMAT_RATIONAL ||
entry->components != 1 )
return( -1 );

Expand Down Expand Up @@ -472,8 +505,8 @@ set_vips_resolution( VipsImage *im, ExifData *ed )
/* The main image xres/yres are in ifd0. ifd1 has xres/yres of the
* image thumbnail, if any.
*/
if( get_entry_rational( ed, 0, EXIF_TAG_X_RESOLUTION, &xres ) ||
get_entry_rational( ed, 0, EXIF_TAG_Y_RESOLUTION, &yres ) ||
if( get_entry_double( ed, 0, EXIF_TAG_X_RESOLUTION, &xres ) ||
get_entry_double( ed, 0, EXIF_TAG_Y_RESOLUTION, &yres ) ||
get_entry_short( ed, 0, EXIF_TAG_RESOLUTION_UNIT, &unit ) ) {
vips_warn( "VipsJpeg",
"%s", _( "error reading resolution" ) );
Expand Down
161 changes: 132 additions & 29 deletions libvips/foreign/vips2jpeg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
* - 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
* 16/11/12
* - read ifds from exif fields
* - optionally parse rationals as a/b
*/

/*
Expand Down Expand Up @@ -247,6 +250,110 @@ vips_exif_set_int( ExifData *ed,
exif_set_slong( entry->data + offset, bo, value );
}

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.
*/
scale = (unsigned int) ((UINT_MAX - 1000) / value);
scale = scale == 0 ? 1 : scale;
rv->numerator = value * scale;
rv->denominator = scale;
}

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.
*/
scale = (int) ((INT_MAX - 1000) / value);
scale = scale == 0 ? 1 : scale;
srv->numerator = value * scale;
srv->denominator = scale;
}

/* Parse a char* into an ExifRational. We allow floats as well.
*/
static void
vips_exif_parse_rational( const char *str, ExifRational *rv )
{
if( sscanf( str, " %u / %u ", &rv->numerator, &rv->denominator ) == 2 )
return;
vips_exif_double_to_rational( g_ascii_strtod( str, NULL ), rv );
}

/* Parse a char* into an ExifSRational. We allow floats as well.
*/
static void
vips_exif_parse_srational( const char *str, ExifSRational *srv )
{
if( sscanf( str, " %d / %d ",
&srv->numerator, &srv->denominator ) == 2 )
return;
vips_exif_double_to_srational( g_ascii_strtod( str, NULL ), srv );
}

/* Does both signed and unsigned rationals from a char*.
*/
static void
vips_exif_set_rational( ExifData *ed,
ExifEntry *entry, unsigned long component, void *data )
{
char *value = (char *) data;

ExifByteOrder bo;
size_t sizeof_component;
size_t offset;

if( entry->components <= component ) {
VIPS_DEBUG_MSG( "vips_exif_set_rational: "
"too few components\n" );
return;
}

/* Wait until after the component check to make sure we cant get /0.
*/
bo = exif_data_get_byte_order( ed );
sizeof_component = entry->size / entry->components;
offset = component * sizeof_component;

VIPS_DEBUG_MSG( "vips_exif_set_rational: %s = \"%s\"\n",
exif_tag_get_title( entry->tag ), value );

if( entry->format == EXIF_FORMAT_RATIONAL ) {
ExifRational rv;

vips_exif_parse_rational( value, &rv );

VIPS_DEBUG_MSG( "vips_exif_set_rational: %u / %u\n",
rv.numerator,
rv.denominator );

exif_set_rational( entry->data + offset, bo, rv );
}
else if( entry->format == EXIF_FORMAT_SRATIONAL ) {
ExifSRational srv;

vips_exif_parse_srational( value, &srv );

VIPS_DEBUG_MSG( "vips_exif_set_rational: %d / %d\n",
srv.numerator, srv.denominator );

exif_set_srational( entry->data + offset, bo, srv );
}
}

/* Does both signed and unsigned rationals from a double*.
*
* Don't change the exit entry if the value currently there is a good
* approximation of the double we are trying to set.
*/
static void
vips_exif_set_double( ExifData *ed,
ExifEntry *entry, unsigned long component, void *data )
Expand All @@ -256,9 +363,11 @@ vips_exif_set_double( ExifData *ed,
ExifByteOrder bo;
size_t sizeof_component;
size_t offset;
double old_value;

if( entry->components <= component ) {
VIPS_DEBUG_MSG( "vips_exif_set_int: too few components\n" );
VIPS_DEBUG_MSG( "vips_exif_set_double: "
"too few components\n" );
return;
}

Expand All @@ -268,40 +377,37 @@ vips_exif_set_double( ExifData *ed,
sizeof_component = entry->size / entry->components;
offset = component * sizeof_component;

VIPS_DEBUG_MSG( "vips_exif_set_double: %s = %g\n",
VIPS_DEBUG_MSG( "vips_exif_set_double: %s = \"%s\"\n",
exif_tag_get_title( entry->tag ), value );

if( entry->format == EXIF_FORMAT_RATIONAL ) {
ExifRational rational;
unsigned int scale;
ExifRational rv;

/* We scale up to fill uint32, then set that as the
* denominator. Try to avoid generating 0.
*/
scale = (int) ((UINT_MAX - 1000) / value);
scale = scale == 0 ? 1 : scale;
rational.numerator = value * scale;
rational.denominator = scale;
rv = exif_get_rational( entry->data + offset, bo );
old_value = (double) rv.numerator / rv.denominator;
if( abs( old_value - value ) > 0.0001 ) {
vips_exif_double_to_rational( value, &rv );

VIPS_DEBUG_MSG( "vips_exif_set_double: %g / %g\n",
(double) rational.numerator,
(double) rational.denominator );
VIPS_DEBUG_MSG( "vips_exif_set_double: %u / %u\n",
rv.numerator,
rv.denominator );

exif_set_rational( entry->data + offset, bo, rational );
exif_set_rational( entry->data + offset, bo, rv );
}
}
else if( entry->format == EXIF_FORMAT_SRATIONAL ) {
ExifSRational rational;
int scale;
ExifSRational srv;

scale = (int) ((INT_MAX - 1000) / value);
scale = scale == 0 ? 1 : scale;
rational.numerator = value * scale;
rational.denominator = scale;
srv = exif_get_srational( entry->data + offset, bo );
old_value = (double) srv.numerator / srv.denominator;
if( abs( old_value - value ) > 0.0001 ) {
vips_exif_double_to_srational( value, &srv );

VIPS_DEBUG_MSG( "vips_exif_set_double: %d / %d\n",
rational.numerator, rational.denominator );
VIPS_DEBUG_MSG( "vips_exif_set_double: %d / %d\n",
srv.numerator, srv.denominator );

exif_set_srational( entry->data + offset, bo, rational );
exif_set_srational( entry->data + offset, bo, srv );
}
}
}

Expand Down Expand Up @@ -426,11 +532,8 @@ vips_exif_from_s( ExifData *ed, ExifEntry *entry, const char *value )
vips_exif_set_int( ed, entry, i, &value );
}
else if( entry->format == EXIF_FORMAT_RATIONAL ||
entry->format == EXIF_FORMAT_SRATIONAL ) {
double value = g_ascii_strtod( p, NULL );

vips_exif_set_double( ed, entry, i, &value );
}
entry->format == EXIF_FORMAT_SRATIONAL )
vips_exif_set_rational( ed, entry, i, (void *) p );

/* Skip to the next set of spaces, then to the beginning of
* the next item.
Expand Down

0 comments on commit 74f545f

Please sign in to comment.