Skip to content

Commit 348e5e1

Browse files
jcupittjcupitt
committed
don't set JFIF res if we will set EXIF res
Some JPEG loaders give priority to JFIF resolution over EXIF resolution tags. This patch makes libvips not write the JFIF res tags if it will be writing the EXIF res tags. See libvips/ruby-vips#247
1 parent 801111a commit 348e5e1

File tree

3 files changed

+81
-12
lines changed

3 files changed

+81
-12
lines changed

‎ChangeLog

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
6/9/20 started 8.10.2
22
- update magicksave/load profile handling [kelilevi]
33
- better demand hint rules [kaas3000]
4+
- in jpegsave, don't set JFIF resolution if we set EXIF resolution
45

56
9/8/20 started 8.10.1
67
- fix markdown -> xml conversion in doc generation

‎libvips/foreign/jpeg2vips.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@
108108
* - revise for source IO
109109
* 5/5/20 angelmixu
110110
* - better handling of JFIF res unit 0
111+
* 13/9/20
112+
* - set resolution unit from JFIF
111113
*/
112114

113115
/*
@@ -506,6 +508,13 @@ read_jpeg_header( ReadJpeg *jpeg, VipsImage *out )
506508
break;
507509
}
508510

511+
#ifdef DEBUG
512+
if( cinfo->saw_JFIF_marker )
513+
printf( "read_jpeg_header: jfif _density %d, %d, unit %d\n",
514+
cinfo->X_density, cinfo->Y_density,
515+
cinfo->density_unit );
516+
#endif /*DEBUG*/
517+
509518
/* Get the jfif resolution. exif may overwrite this later. Default to
510519
* 72dpi (as EXIF does).
511520
*/
@@ -514,12 +523,6 @@ read_jpeg_header( ReadJpeg *jpeg, VipsImage *out )
514523
if( cinfo->saw_JFIF_marker &&
515524
cinfo->X_density != 1U &&
516525
cinfo->Y_density != 1U ) {
517-
#ifdef DEBUG
518-
printf( "read_jpeg_header: jfif _density %d, %d, unit %d\n",
519-
cinfo->X_density, cinfo->Y_density,
520-
cinfo->density_unit );
521-
#endif /*DEBUG*/
522-
523526
switch( cinfo->density_unit ) {
524527
case 0:
525528
/* X_density / Y_density gives the pixel aspect ratio.
@@ -535,13 +538,17 @@ read_jpeg_header( ReadJpeg *jpeg, VipsImage *out )
535538
*/
536539
xres = cinfo->X_density / 25.4;
537540
yres = cinfo->Y_density / 25.4;
541+
vips_image_set_string( out,
542+
VIPS_META_RESOLUTION_UNIT, "in" );
538543
break;
539544

540545
case 2:
541546
/* Pixels per cm.
542547
*/
543548
xres = cinfo->X_density / 10.0;
544549
yres = cinfo->Y_density / 10.0;
550+
vips_image_set_string( out,
551+
VIPS_META_RESOLUTION_UNIT, "cm" );
545552
break;
546553

547554
default:

‎libvips/foreign/vips2jpeg.c

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@
9494
* - revise for target IO
9595
* 18/2/20 Elad-Laufer
9696
* - add subsample_mode, deprecate no_subsample
97+
* 13/9/20
98+
* - only write JFIF resolution if we don't have EXIF
9799
*/
98100

99101
/*
@@ -412,6 +414,59 @@ write_profile_data (j_compress_ptr cinfo,
412414
}
413415
}
414416

417+
#ifndef HAVE_EXIF
418+
/* Set the JFIF resolution from the vips xres/yres tags.
419+
*/
420+
static void
421+
vips_jfif_resolution_from_image( struct jpeg_compress_struct *cinfo,
422+
VipsImage *image )
423+
{
424+
int xres, yres;
425+
const char *p;
426+
int unit;
427+
428+
/* Default to inches, more progs support it.
429+
*/
430+
unit = 1;
431+
if( vips_image_get_typeof( image, VIPS_META_RESOLUTION_UNIT ) &&
432+
!vips_image_get_string( image,
433+
VIPS_META_RESOLUTION_UNIT, &p ) ) {
434+
if( vips_isprefix( "cm", p ) )
435+
unit = 2;
436+
else if( vips_isprefix( "none", p ) )
437+
unit = 0;
438+
}
439+
440+
switch( unit ) {
441+
case 0:
442+
xres = VIPS_RINT( image->Xres );
443+
yres = VIPS_RINT( image->Yres );
444+
break;
445+
446+
case 1:
447+
xres = VIPS_RINT( image->Xres * 25.4 );
448+
yres = VIPS_RINT( image->Yres * 25.4 );
449+
break;
450+
451+
case 2:
452+
xres = VIPS_RINT( image->Xres * 10.0 );
453+
yres = VIPS_RINT( image->Yres * 10.0 );
454+
break;
455+
456+
default:
457+
g_assert_not_reached();
458+
break;
459+
}
460+
461+
VIPS_DEBUG_MSG( "vips_jfif_resolution_from_image: "
462+
"setting xres = %d, yres = %d, unit = %d\n", xres, yres, unit );
463+
464+
cinfo->density_unit = unit;
465+
cinfo->X_density = xres;
466+
cinfo->Y_density = yres;
467+
}
468+
#endif /*HAVE_EXIF*/
469+
415470
/* Write an ICC Profile from a file into the JPEG stream.
416471
*/
417472
static int
@@ -643,18 +698,24 @@ write_vips( Write *write, int qfac, const char *profile,
643698
}
644699
}
645700

646-
/* Don't write the APP0 JFIF headers if we are stripping.
701+
/* Only write the JFIF headers if we are not stripping and we have no
702+
* EXIF. Some readers get confused if you set both.
647703
*/
648-
if( strip )
649-
write->cinfo.write_JFIF_header = FALSE;
704+
write->cinfo.write_JFIF_header = FALSE;
705+
#ifndef HAVE_EXIF
706+
if( !strip ) {
707+
vips_jfif_resolution_from_image( &write->cinfo, write->in );
708+
write->cinfo.write_JFIF_header = TRUE;
709+
}
710+
#endif /*HAVE_EXIF*/
650711

651-
/* Build compress tables.
712+
/* Write app0 and build compress tables.
652713
*/
653714
jpeg_start_compress( &write->cinfo, TRUE );
654715

655-
/* Write any APP markers we need.
716+
/* All the other APP chunks come next.
656717
*/
657-
if( !strip ) {
718+
if( !strip ) {
658719
/* We need to rebuild the exif data block from any exif tags
659720
* on the image.
660721
*/

0 commit comments

Comments
 (0)