From a7d2288bad94ce7e3ba3cb894f13edddf27fc000 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Mon, 21 Feb 2011 17:01:00 +0000
Subject: [PATCH] im_jpeg2vips() ignores weird APP1 chunks

Some JPEGs seem to have multiple APP1 chunks. There should only be one,
and it should only contain EXIF data (I think), but some seem to have
many.

This was causing problems: the loader was trying to read exif
from all chunks, and a second chunk with invalid data was zapping the
stored exif meta tag. As a result, things like

  vips im_copy a.jpg b.jpg

would appear to lose EXIF info if a.jpg had spurious APP1.
---
 ChangeLog                     |  1 +
 libvips/format/im_jpeg2vips.c | 29 ++++++++++++++++++++++-------
 libvips/format/im_vips2jpeg.c |  2 +-
 libvips/iofuncs/im_cp_desc.c  | 13 +++----------
 4 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e30eddfe..ddc278f8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -31,6 +31,7 @@
 - set MAP_NOCACHE on OS X, otherwise performance dives off a cliff with 
   files larger than memory
 - removed man pages, we are all gtk-doc now
+- im_jpeg2vips() ignores weird APP1 chunks
 
 30/11/10 started 7.24.0
 - bump for new stable
diff --git a/libvips/format/im_jpeg2vips.c b/libvips/format/im_jpeg2vips.c
index 6042cc2e..7fbdb512 100644
--- a/libvips/format/im_jpeg2vips.c
+++ b/libvips/format/im_jpeg2vips.c
@@ -30,6 +30,9 @@
  * 	- gtkdoc
  * 4/12/10
  * 	- attach the jpeg thumbnail and multiscan fields (thanks Mike)
+ * 21/2/10
+ * 	- only accept the first APP1 block which starts "Exif..." as exif
+ * 	  data, some jpegs seem to have several APP1s, argh
  */
 
 /*
@@ -59,8 +62,8 @@
  */
 
 /*
-#define DEBUG
 #define DEBUG_VERBOSE
+#define DEBUG
  */
 
 #ifdef HAVE_CONFIG_H
@@ -396,7 +399,18 @@ attach_thumbnail( IMAGE *im, ExifData *ed )
 static int
 read_exif( IMAGE *im, void *data, int data_length )
 {
-char *data_copy;
+	char *data_copy;
+
+	/* Horrifyingly, some JPEGs have several APP1 sections. We must only
+	 * use the first one that starts "Exif.."
+	 */
+	if( ((char *) data)[0] != 'E' ||
+		((char *) data)[1] != 'x' ||
+		((char *) data)[2] != 'i' ||
+		((char *) data)[3] != 'f' )
+		return( 0 );
+	if( im_header_get_typeof( im, IM_META_EXIF_NAME ) ) 
+		return( 0 );
 
 	/* Always attach a copy of the unparsed exif data.
 	 */
@@ -424,10 +438,11 @@ char *data_copy;
 
 		/* Attach informational fields for what we find.
 
-		FIXME ... better to have this in the UI layer?
+			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?
+			Or we could attach non-human-readable tags here (int, 
+			double etc) and then move the human stuff to the UI 
+			layer?
 
 		 */
 		exif_data_foreach_content( ed, 
@@ -521,7 +536,7 @@ read_jpeg_header( struct jpeg_decompress_struct *cinfo,
 			/* EXIF data.
 			 */
 #ifdef DEBUG
-			printf( "read_jpeg_header: seen %d bytes of EXIF\n",
+			printf( "read_jpeg_header: seen %d bytes of APP1\n",
 				p->data_length );
 #endif /*DEBUG*/
 			if( read_exif( out, p->data, p->data_length ) )
@@ -532,7 +547,7 @@ read_jpeg_header( struct jpeg_decompress_struct *cinfo,
 			/* ICC profile.
 			 */
 #ifdef DEBUG
-			printf( "read_jpeg_header: seen %d bytes of ICC\n",
+			printf( "read_jpeg_header: seen %d bytes of APP2\n",
 				p->data_length );
 #endif /*DEBUG*/
 
diff --git a/libvips/format/im_vips2jpeg.c b/libvips/format/im_vips2jpeg.c
index 6a55b97c..74b38608 100644
--- a/libvips/format/im_vips2jpeg.c
+++ b/libvips/format/im_vips2jpeg.c
@@ -376,7 +376,7 @@ write_exif( Write *write )
 	else 
 		ed = exif_data_new();
 
-	/* Set EXIF resolution from VIPS.
+	/* Update EXIF resolution from VIPS.
 	 */
 	if( set_exif_resolution( ed, write->in ) ) {
 		exif_data_free( ed );
diff --git a/libvips/iofuncs/im_cp_desc.c b/libvips/iofuncs/im_cp_desc.c
index a94dd4f5..030f2bf7 100644
--- a/libvips/iofuncs/im_cp_desc.c
+++ b/libvips/iofuncs/im_cp_desc.c
@@ -1,13 +1,4 @@
-/* @(#)  Function which copies IMAGE descriptor image2 to image1;
- * @(#) data, fd and filename are not copied
- * @(#) used to make programs simpler by copying most parameters
- * @(#) 
- * @(#) int 
- * @(#) im_cp_desc( image1, image2 )
- * @(#) IMAGE *image1, *image2;
- * @(#)
- * @(#) Returns 0 on success or -1 on fail.
- * @(#)
+/* copy descriptors between images 
  *
  * Copyright: Nicos Dessipris
  * Written on: 09/02/1990
@@ -30,6 +21,8 @@
  * 	- simplified ... no more skip the first line stuff
  * 4/1/07
  * 	- merge hists with history_list instead
+ * 20/1/10
+ * 	- gtk-doc
  */
 
 /*