From 4227606f5a6dbba8d562332f4f2f495135d71fa2 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Mon, 14 Sep 2020 17:38:20 +0100
Subject: [PATCH] fix tiff thumbnail from buffer and source

We were missing the new tiff thumbnail logic on the source and buffer
paths.

see https://github.com/libvips/libvips/issues/1815
---
 ChangeLog                   |  1 +
 libvips/foreign/jpeg2vips.c | 19 +++++++---
 libvips/foreign/vips2jpeg.c | 73 ++++++++++++++++++++++++++++++++++---
 3 files changed, 81 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 49a1e20e..ff6ab5b6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,7 @@
 - update magicksave/load profile handling [kelilevi]
 - better demand hint rules [kaas3000]
 - fix tiff thumbnail from buffer and source [vansante]
+- in jpegsave, don't set JFIF resolution if we set EXIF resolution
 
 9/8/20 started 8.10.1
 - fix markdown -> xml conversion in doc generation
diff --git a/libvips/foreign/jpeg2vips.c b/libvips/foreign/jpeg2vips.c
index 80964ecc..08d8e7c3 100644
--- a/libvips/foreign/jpeg2vips.c
+++ b/libvips/foreign/jpeg2vips.c
@@ -108,6 +108,8 @@
  * 	- revise for source IO
  * 5/5/20 angelmixu
  * 	- better handling of JFIF res unit 0
+ * 13/9/20
+ * 	- set resolution unit from JFIF 
  */
 
 /*
@@ -506,6 +508,13 @@ read_jpeg_header( ReadJpeg *jpeg, VipsImage *out )
 		break;
 	}
 
+#ifdef DEBUG
+	if( cinfo->saw_JFIF_marker )
+		printf( "read_jpeg_header: jfif _density %d, %d, unit %d\n",
+			cinfo->X_density, cinfo->Y_density,
+			cinfo->density_unit );
+#endif /*DEBUG*/
+
 	/* Get the jfif resolution. exif may overwrite this later. Default to
 	 * 72dpi (as EXIF does).
 	 */
@@ -514,12 +523,6 @@ read_jpeg_header( ReadJpeg *jpeg, VipsImage *out )
 	if( cinfo->saw_JFIF_marker &&
 		cinfo->X_density != 1U && 
 		cinfo->Y_density != 1U ) {
-#ifdef DEBUG
-		printf( "read_jpeg_header: jfif _density %d, %d, unit %d\n",
-			cinfo->X_density, cinfo->Y_density,
-			cinfo->density_unit );
-#endif /*DEBUG*/
-
 		switch( cinfo->density_unit ) {
 		case 0:
 			/* X_density / Y_density gives the pixel aspect ratio.
@@ -535,6 +538,8 @@ read_jpeg_header( ReadJpeg *jpeg, VipsImage *out )
 			 */
 			xres = cinfo->X_density / 25.4;
 			yres = cinfo->Y_density / 25.4;
+			vips_image_set_string( out, 
+				VIPS_META_RESOLUTION_UNIT, "in" );
 			break;
 
 		case 2:
@@ -542,6 +547,8 @@ read_jpeg_header( ReadJpeg *jpeg, VipsImage *out )
 			 */
 			xres = cinfo->X_density / 10.0;
 			yres = cinfo->Y_density / 10.0;
+			vips_image_set_string( out, 
+				VIPS_META_RESOLUTION_UNIT, "cm" );
 			break;
 
 		default:
diff --git a/libvips/foreign/vips2jpeg.c b/libvips/foreign/vips2jpeg.c
index c748fad8..83495072 100644
--- a/libvips/foreign/vips2jpeg.c
+++ b/libvips/foreign/vips2jpeg.c
@@ -94,6 +94,8 @@
  * 	- revise for target IO
  * 18/2/20 Elad-Laufer
  * 	- add subsample_mode, deprecate no_subsample
+ * 13/9/20
+ * 	- only write JFIF resolution if we don't have EXIF
  */
 
 /*
@@ -412,6 +414,59 @@ write_profile_data (j_compress_ptr cinfo,
   }
 }
 
+#ifndef HAVE_EXIF
+/* Set the JFIF resolution from the vips xres/yres tags.
+ */
+static void
+vips_jfif_resolution_from_image( struct jpeg_compress_struct *cinfo, 
+	VipsImage *image )
+{
+	int xres, yres;
+	const char *p;
+	int unit;
+
+	/* Default to inches, more progs support it.
+	 */
+	unit = 1;
+	if( vips_image_get_typeof( image, VIPS_META_RESOLUTION_UNIT ) &&
+		!vips_image_get_string( image, 
+			VIPS_META_RESOLUTION_UNIT, &p ) ) {
+		if( vips_isprefix( "cm", p ) ) 
+			unit = 2;
+		else if( vips_isprefix( "none", p ) ) 
+			unit = 0;
+	}
+
+	switch( unit ) {
+	case 0:
+		xres = VIPS_RINT( image->Xres );
+		yres = VIPS_RINT( image->Yres );
+		break;
+
+	case 1:
+		xres = VIPS_RINT( image->Xres * 25.4 );
+		yres = VIPS_RINT( image->Yres * 25.4 );
+		break;
+
+	case 2:
+		xres = VIPS_RINT( image->Xres * 10.0 );
+		yres = VIPS_RINT( image->Yres * 10.0 );
+		break;
+
+	default:
+		g_assert_not_reached();
+		break;
+	}
+
+	VIPS_DEBUG_MSG( "vips_jfif_resolution_from_image: "
+		"setting xres = %d, yres = %d, unit = %d\n", xres, yres, unit );
+
+	cinfo->density_unit = unit;
+	cinfo->X_density = xres;
+	cinfo->Y_density = yres;
+}
+#endif /*HAVE_EXIF*/
+
 /* Write an ICC Profile from a file into the JPEG stream.
  */
 static int
@@ -643,18 +698,24 @@ write_vips( Write *write, int qfac, const char *profile,
 		}
 	}
 
-	/* Don't write the APP0 JFIF headers if we are stripping.
+	/* Only write the JFIF headers if we are not stripping and we have no
+	 * EXIF. Some readers get confused if you set both.
 	 */
-	if( strip ) 
-		write->cinfo.write_JFIF_header = FALSE;
+	write->cinfo.write_JFIF_header = FALSE;
+#ifndef HAVE_EXIF
+	if( !strip ) {
+		vips_jfif_resolution_from_image( &write->cinfo,  write->in );
+		write->cinfo.write_JFIF_header = TRUE;
+	}
+#endif /*HAVE_EXIF*/
 
-	/* Build compress tables.
+	/* Write app0 and build compress tables.
 	 */
 	jpeg_start_compress( &write->cinfo, TRUE );
 
-	/* Write any APP markers we need.
+	/* All the other APP chunks come next.
 	 */
-	if( !strip ) { 
+	if( !strip ) {
 		/* We need to rebuild the exif data block from any exif tags
 		 * on the image.
 		 */