From 2e3eca7e292270c1055e3ba38a34e2877f9ad1dd Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Wed, 29 Jan 2020 15:44:55 +0000
Subject: [PATCH 01/16] another missing copy operation

The --page-height arg to savers needs a copy as well.
---
 libvips/foreign/foreign.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/libvips/foreign/foreign.c b/libvips/foreign/foreign.c
index aec3337a..d59016ce 100644
--- a/libvips/foreign/foreign.c
+++ b/libvips/foreign/foreign.c
@@ -1640,9 +1640,19 @@ vips_foreign_save_build( VipsObject *object )
 			save->background ) )
 			return( -1 );
 
-		if( save->page_height )
+		if( save->page_height ) {
+			VipsImage *x;
+
+			if( vips_copy( ready, &x, NULL ) ) {
+				VIPS_UNREF( ready );
+				return( -1 );
+			}
+			VIPS_UNREF( ready );
+			ready = x;
+
 			vips_image_set_int( ready, 
 				VIPS_META_PAGE_HEIGHT, save->page_height );
+		}
 
 		VIPS_UNREF( save->ready );
 		save->ready = ready;

From a158b15b97d90b6e3d5c6d0bebbd0b3af3d92257 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Wed, 29 Jan 2020 17:47:08 +0000
Subject: [PATCH 02/16] add LOGLUV TIFF support

libvips XYZ images load and save as libtiff LOGLUV

see https://github.com/libvips/libvips/issues/1506
---
 libvips/foreign/tiff2vips.c | 97 +++++++++++++++++++++++++++++++++++--
 libvips/foreign/vips2tiff.c | 78 +++++++++++++++++++++++++----
 2 files changed, 160 insertions(+), 15 deletions(-)

diff --git a/libvips/foreign/tiff2vips.c b/libvips/foreign/tiff2vips.c
index 56935afb..4c908bdd 100644
--- a/libvips/foreign/tiff2vips.c
+++ b/libvips/foreign/tiff2vips.c
@@ -193,6 +193,8 @@
  * 	- switch to source input
  * 18/11/19
  * 	- support ASSOCALPHA in any alpha band
+ * 27/1/20
+ * 	- read logluv images as XYZ
  */
 
 /*
@@ -294,6 +296,10 @@ typedef struct _RtiffHeader {
 	 */
 	uint32 read_height;
 	tsize_t read_size;
+
+	/* Scale factor to get absolute cd/m2 from XYZ.
+	 */
+	double stonits;
 } RtiffHeader;
 
 /* Scanline-type process function.
@@ -911,6 +917,52 @@ rtiff_parse_labs( Rtiff *rtiff, VipsImage *out )
 	return( 0 );
 }
 
+/* libtiff delivers logluv as illuminant-free 0-1 XYZ in 3 x float.
+ */
+static void
+rtiff_logluv_line( Rtiff *rtiff, VipsPel *q, VipsPel *p, int n, void *dummy )
+{
+	int samples_per_pixel = rtiff->header.samples_per_pixel;
+
+	float *p1;
+	float *q1;
+	int x;
+	int i; 
+
+	p1 = (float *) p;
+	q1 = (float *) q;
+	for( x = 0; x < n; x++ ) {
+		q1[0] = VIPS_D65_X0 * p1[0];
+		q1[1] = VIPS_D65_Y0 * p1[1];
+		q1[2] = VIPS_D65_Z0 * p1[2];
+
+		for( i = 3; i < samples_per_pixel; i++ ) 
+			q1[i] = p1[i];
+
+		q1 += samples_per_pixel;
+		p1 += samples_per_pixel;
+	}
+}
+
+/* LOGLUV images arrive from libtiff as float xyz.
+ */
+static int
+rtiff_parse_logluv( Rtiff *rtiff, VipsImage *out )
+{
+	if( rtiff_check_min_samples( rtiff, 3 ) ||
+		rtiff_check_interpretation( rtiff, PHOTOMETRIC_LOGLUV ) )
+		return( -1 );
+
+	out->Bands = rtiff->header.samples_per_pixel; 
+	out->BandFmt = VIPS_FORMAT_FLOAT; 
+	out->Coding = VIPS_CODING_NONE; 
+	out->Type = VIPS_INTERPRETATION_XYZ; 
+
+	rtiff->sfn = rtiff_logluv_line;
+
+	return( 0 );
+}
+
 /* Per-scanline process function for 1 bit images.
  */
 static void
@@ -1406,6 +1458,9 @@ rtiff_pick_reader( Rtiff *rtiff )
 			return( rtiff_parse_labs );
 	}
 
+	if( photometric_interpretation == PHOTOMETRIC_LOGLUV ) 
+		return( rtiff_parse_logluv );
+
 	if( photometric_interpretation == PHOTOMETRIC_MINISWHITE ||
 		photometric_interpretation == PHOTOMETRIC_MINISBLACK ) {
 		if( bits_per_sample == 1 )
@@ -1436,6 +1491,15 @@ rtiff_set_header( Rtiff *rtiff, VipsImage *out )
 		TIFFSetField( rtiff->tiff, 
 			TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB );
 
+	/* Ask for LOGLUV as 3 x float XYZ. 
+	 */
+	if( rtiff->header.photometric_interpretation == PHOTOMETRIC_LOGLUV ) {
+		TIFFSetField( rtiff->tiff, 
+			TIFFTAG_SGILOGDATAFMT, SGILOGDATAFMT_FLOAT );
+
+		vips_image_set_double( out, "stonits", rtiff->header.stonits );
+	}
+
 	out->Xsize = rtiff->header.width;
 	out->Ysize = rtiff->header.height * rtiff->n;
 
@@ -2104,7 +2168,7 @@ rtiff_read_stripwise( Rtiff *rtiff, VipsImage *out )
 	/* Double check: in memcpy mode, the vips linesize should exactly
 	 * match the tiff line size.
 	 */
-	if( rtiff->memcpy ) {
+	if( rtiff->memcpy ) { 
 		size_t vips_line_size;
 
 		/* Lines are smaller in plane-separated mode.
@@ -2198,13 +2262,16 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header )
 
 	TIFFGetFieldDefaulted( rtiff->tiff, 
 		TIFFTAG_COMPRESSION, &header->compression );
+
+	/* Request YCbCr expansion. libtiff complains if you do this for
+	 * non-jpg images. We must set this here since it changes the result
+	 * of scanline_size.
+	 */
 	if( header->compression == COMPRESSION_JPEG )
-		/* We want to always expand subsampled YCBCR images to full 
-		 * RGB. 
-		 */
 		TIFFSetField( rtiff->tiff, 
 			TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB );
-        else if( header->photometric_interpretation == PHOTOMETRIC_YCBCR ) {
+
+        if( header->photometric_interpretation == PHOTOMETRIC_YCBCR ) {
 		/* We rely on the jpg decompressor to upsample chroma
 		 * subsampled images. If there is chroma subsampling but
 		 * no jpg compression, we have to give up.
@@ -2223,6 +2290,26 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header )
                 }
         }
 
+	if( header->photometric_interpretation == PHOTOMETRIC_LOGLUV ) {
+		if( header->compression != COMPRESSION_SGILOG &&
+			header->compression != COMPRESSION_SGILOG24 ) {
+			vips_error( "tiff2vips",
+				"%s", _( "not SGI-compressed LOGLUV" ) );
+			return( -1 );
+		}
+
+		/* Always get LOGLUV as 3 x float XYZ. We must set this here 
+		 * since it'll change the value of scanline_size.
+		 */
+		TIFFSetField( rtiff->tiff, 
+			TIFFTAG_SGILOGDATAFMT, SGILOGDATAFMT_FLOAT );
+	}
+
+	/* For logluv, the calibration factor to get to absolute luminance.
+	 */
+	if( !TIFFGetField( rtiff->tiff, TIFFTAG_STONITS, &header->stonits ) )
+		header->stonits = 1.0;
+
 	/* Arbitrary sanity-checking limits.
 	 */
 	if( header->width <= 0 ||
diff --git a/libvips/foreign/vips2tiff.c b/libvips/foreign/vips2tiff.c
index 63e338ac..0edcbe7e 100644
--- a/libvips/foreign/vips2tiff.c
+++ b/libvips/foreign/vips2tiff.c
@@ -189,6 +189,8 @@
  * 	- "squash" now squashes 3-band float LAB down to LABQ
  * 26/1/20
  * 	- add "depth" to set pyr depth
+ * 27/1/20
+ * 	- write XYZ images as logluv
  */
 
 /*
@@ -608,7 +610,6 @@ wtiff_write_header( Wtiff *wtiff, Layer *layer )
 {
 	TIFF *tif = layer->tif;
 
-	int format; 
 	int orientation; 
 
 	/* Output base header fields.
@@ -702,6 +703,21 @@ wtiff_write_header( Wtiff *wtiff, Layer *layer )
 			photometric = PHOTOMETRIC_CIELAB;
 			colour_bands = 3;
 		}
+		else if( wtiff->input->Type == VIPS_INTERPRETATION_XYZ ) { 
+			double stonits;
+
+			photometric = PHOTOMETRIC_LOGLUV;
+			/* Tell libtiff we will write as float XYZ.
+			 */
+			TIFFSetField( tif, 
+				TIFFTAG_SGILOGDATAFMT, SGILOGDATAFMT_FLOAT );
+			stonits = 1.0;
+			if( vips_image_get_typeof( wtiff->ready, "stonits" ) )
+				vips_image_get_double( wtiff->ready, 
+					"stonits", &stonits );
+			TIFFSetField( tif, TIFFTAG_STONITS, stonits );
+			colour_bands = 3;
+		}
 		else if( wtiff->ready->Type == VIPS_INTERPRETATION_CMYK &&
 			wtiff->ready->Bands >= 4 ) {
 			photometric = PHOTOMETRIC_SEPARATED;
@@ -764,17 +780,23 @@ wtiff_write_header( Wtiff *wtiff, Layer *layer )
 		TIFFSetField( tif, TIFFTAG_SUBFILETYPE, FILETYPE_REDUCEDIMAGE );
 
 	/* Sample format.
+	 *
+	 * Don't set for logluv: libtiff does this for us.
 	 */
-	format = SAMPLEFORMAT_UINT;
-	if( vips_band_format_isuint( wtiff->ready->BandFmt ) )
+	if( wtiff->input->Type != VIPS_INTERPRETATION_XYZ ) { 
+		int format; 
+
 		format = SAMPLEFORMAT_UINT;
-	else if( vips_band_format_isint( wtiff->ready->BandFmt ) )
-		format = SAMPLEFORMAT_INT;
-	else if( vips_band_format_isfloat( wtiff->ready->BandFmt ) )
-		format = SAMPLEFORMAT_IEEEFP;
-	else if( vips_band_format_iscomplex( wtiff->ready->BandFmt ) )
-		format = SAMPLEFORMAT_COMPLEXIEEEFP;
-	TIFFSetField( tif, TIFFTAG_SAMPLEFORMAT, format );
+		if( vips_band_format_isuint( wtiff->ready->BandFmt ) )
+			format = SAMPLEFORMAT_UINT;
+		else if( vips_band_format_isint( wtiff->ready->BandFmt ) )
+			format = SAMPLEFORMAT_INT;
+		else if( vips_band_format_isfloat( wtiff->ready->BandFmt ) )
+			format = SAMPLEFORMAT_IEEEFP;
+		else if( vips_band_format_iscomplex( wtiff->ready->BandFmt ) )
+			format = SAMPLEFORMAT_COMPLEXIEEEFP;
+		TIFFSetField( tif, TIFFTAG_SAMPLEFORMAT, format );
+	}
 
 	return( 0 );
 }
@@ -1039,6 +1061,11 @@ wtiff_new( VipsImage *input, const char *filename,
 		return( NULL );
 	}
 
+	/* XYZ images are written as libtiff LOGLUV.
+	 */
+	if( wtiff->ready->Type == VIPS_INTERPRETATION_XYZ ) 
+		wtiff->compression = COMPRESSION_SGILOG;
+
 	/* Multipage image?
 	 */
 	if( wtiff->page_height < wtiff->ready->Ysize ) {
@@ -1331,6 +1358,31 @@ LabS2Lab16( VipsPel *q, VipsPel *p, int n, int samples_per_pixel )
 	}
 }
 
+/* Convert VIPS D65 XYZ to TIFF scaled float illuminant-free xyz.
+ */
+static void
+XYZ2tiffxyz( VipsPel *q, VipsPel *p, int n, int samples_per_pixel )
+{
+	float *p1 = (float *) p;
+	float *q1 = (float *) q;
+
+	int x;
+
+        for( x = 0; x < n; x++ ) {
+		int i;
+
+                q1[0] = p1[0] / VIPS_D65_X0;
+                q1[1] = p1[1] / VIPS_D65_Y0;
+                q1[2] = p1[2] / VIPS_D65_Z0;
+
+		for( i = 3; i < samples_per_pixel; i++ )
+			q1[i] = p1[i];
+
+		q1 += samples_per_pixel;
+		p1 += samples_per_pixel;
+	}
+}
+
 /* Pack the pixels in @area from @in into a TIFF tile buffer.
  */
 static void
@@ -1358,6 +1410,8 @@ wtiff_pack2tiff( Wtiff *wtiff, Layer *layer,
 			LabQ2LabC( q, p, area->width );
 		else if( wtiff->squash ) 
 			eightbit2onebit( wtiff, q, p, area->width );
+		else if( wtiff->input->Type == VIPS_INTERPRETATION_XYZ )
+			XYZ2tiffxyz( q, p, area->width, in->im->Bands );
 		else if( (in->im->Bands == 1 || in->im->Bands == 2) && 
 			wtiff->miniswhite ) 
 			invert_band0( wtiff, q, p, area->width );
@@ -1449,6 +1503,10 @@ wtiff_layer_write_strip( Wtiff *wtiff, Layer *layer, VipsRegion *strip )
 			LabS2Lab16( wtiff->tbuf, p, im->Xsize, im->Bands );
 			p = wtiff->tbuf;
 		}
+		else if( wtiff->input->Type == VIPS_INTERPRETATION_XYZ ) {
+			XYZ2tiffxyz( wtiff->tbuf, p, im->Xsize, im->Bands );
+			p = wtiff->tbuf;
+		}
 		else if( wtiff->squash ) {
 			eightbit2onebit( wtiff, wtiff->tbuf, p, im->Xsize );
 			p = wtiff->tbuf;

From 81fa983121d8042864c129a0e6bee10a9b3b1baf Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Wed, 29 Jan 2020 18:12:37 +0000
Subject: [PATCH 03/16] oop, dropped an "else"

---
 ChangeLog                   | 1 +
 libvips/foreign/tiff2vips.c | 3 +--
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index dbbb6984..1341d6f2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,7 @@
 - add max and min to region shrink [rgluskin]
 - allow \ as an escape character in vips_break_token() [akemrir]
 - tiffsave has a "depth" param to set max pyr depth
+- libtiff LOGLUV images load and save as libvips XYZ 
 
 20/6/19 started 8.9.1
 - don't use the new source loaders for new_from_file or new_from_buffer, it
diff --git a/libvips/foreign/tiff2vips.c b/libvips/foreign/tiff2vips.c
index 4c908bdd..c8b7873e 100644
--- a/libvips/foreign/tiff2vips.c
+++ b/libvips/foreign/tiff2vips.c
@@ -2270,8 +2270,7 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header )
 	if( header->compression == COMPRESSION_JPEG )
 		TIFFSetField( rtiff->tiff, 
 			TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB );
-
-        if( header->photometric_interpretation == PHOTOMETRIC_YCBCR ) {
+	else if( header->photometric_interpretation == PHOTOMETRIC_YCBCR ) {
 		/* We rely on the jpg decompressor to upsample chroma
 		 * subsampled images. If there is chroma subsampling but
 		 * no jpg compression, we have to give up.

From 979422886b596c244074ab416c026ed23939e134 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Wed, 29 Jan 2020 18:26:16 +0000
Subject: [PATCH 04/16] revise tiff load/save doc comments

---
 libvips/foreign/tiffload.c | 4 ++--
 libvips/foreign/tiffsave.c | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/libvips/foreign/tiffload.c b/libvips/foreign/tiffload.c
index 70950410..96e0789a 100644
--- a/libvips/foreign/tiffload.c
+++ b/libvips/foreign/tiffload.c
@@ -456,8 +456,8 @@ vips_foreign_load_tiff_buffer_init( VipsForeignLoadTiffBuffer *buffer )
  *   during load
  *
  * Read a TIFF file into a VIPS image. It is a full baseline TIFF 6 reader, 
- * with extensions for tiled images, multipage images, LAB colour space, 
- * pyramidal images and JPEG compression. including CMYK and YCbCr.
+ * with extensions for tiled images, multipage images, XYZ and LAB colour 
+ * space, pyramidal images and JPEG compression, including CMYK and YCbCr.
  *
  * @page means load this page from the file. By default the first page (page
  * 0) is read. 
diff --git a/libvips/foreign/tiffsave.c b/libvips/foreign/tiffsave.c
index 281d486a..8644a820 100644
--- a/libvips/foreign/tiffsave.c
+++ b/libvips/foreign/tiffsave.c
@@ -551,6 +551,10 @@ vips_foreign_save_tiff_buffer_init( VipsForeignSaveTiffBuffer *buffer )
  * good for 1-bit images, and deflate is the best lossless compression TIFF 
  * can do. 
  *
+ * XYZ images are automatically saved as libtiff LOGLUV with SGILOG compression.
+ * Float LAB images are saved as float CIELAB. Set @squash to save as 8-bit 
+ * CIELAB.
+ *
  * Use @Q to set the JPEG compression factor. Default 75.
  *
  * User @level to set the ZSTD compression level. Use @lossless to

From 4aeedd971131ee86005e236fe1b6cfdcbd128cd6 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Thu, 30 Jan 2020 16:53:18 +0000
Subject: [PATCH 05/16] add "nearest" region shrink

---
 libvips/include/vips/region.h   |  2 ++
 libvips/iofuncs/enumtypes.c     |  1 +
 libvips/iofuncs/region.c        | 25 +++++++++++++++++++------
 test/test-suite/test_foreign.py |  5 ++++-
 4 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/libvips/include/vips/region.h b/libvips/include/vips/region.h
index bcce3c47..d52a3fa7 100644
--- a/libvips/include/vips/region.h
+++ b/libvips/include/vips/region.h
@@ -62,6 +62,7 @@ extern "C" {
  * @VIPS_REGION_SHRINK_MODE: use the mode
  * @VIPS_REGION_SHRINK_MAX: use the maximum
  * @VIPS_REGION_SHRINK_MIN: use the minimum
+ * @VIPS_REGION_SHRINK_NEAREST: use the top-left pixel
  *
  * How to calculate the output pixels when shrinking a 2x2 region.
  */
@@ -71,6 +72,7 @@ typedef enum {
 	VIPS_REGION_SHRINK_MODE,
 	VIPS_REGION_SHRINK_MAX,
 	VIPS_REGION_SHRINK_MIN,
+	VIPS_REGION_SHRINK_NEAREST,
 	VIPS_REGION_SHRINK_LAST
 } VipsRegionShrink;
 
diff --git a/libvips/iofuncs/enumtypes.c b/libvips/iofuncs/enumtypes.c
index 2f571331..767a43f9 100644
--- a/libvips/iofuncs/enumtypes.c
+++ b/libvips/iofuncs/enumtypes.c
@@ -905,6 +905,7 @@ vips_region_shrink_get_type( void )
 			{VIPS_REGION_SHRINK_MODE, "VIPS_REGION_SHRINK_MODE", "mode"},
 			{VIPS_REGION_SHRINK_MAX, "VIPS_REGION_SHRINK_MAX", "max"},
 			{VIPS_REGION_SHRINK_MIN, "VIPS_REGION_SHRINK_MIN", "min"},
+			{VIPS_REGION_SHRINK_NEAREST, "VIPS_REGION_SHRINK_NEAREST", "nearest"},
 			{VIPS_REGION_SHRINK_LAST, "VIPS_REGION_SHRINK_LAST", "last"},
 			{0, NULL, NULL}
 		};
diff --git a/libvips/iofuncs/region.c b/libvips/iofuncs/region.c
index ab201ff5..a59a5d9a 100644
--- a/libvips/iofuncs/region.c
+++ b/libvips/iofuncs/region.c
@@ -1283,8 +1283,6 @@ vips_region_shrink_uncoded_mean( VipsRegion *from,
         		tq[z] = v[index]; \
 		} \
 		\
-		/* Move on two pels in input. \
-		 */ \
 		p += ps << 1; \
 		q += ps; \
 	} \
@@ -1303,8 +1301,6 @@ vips_region_shrink_uncoded_mean( VipsRegion *from,
 				); \
 		} \
 		\
-		/* Move on two pels in input. \
-		 */ \
 		p += ps << 1; \
 		q += ps; \
 	} \
@@ -1323,8 +1319,20 @@ vips_region_shrink_uncoded_mean( VipsRegion *from,
 				); \
 		} \
 		\
-		/* Move on two pels in input. \
-		 */ \
+		p += ps << 1; \
+		q += ps; \
+	} \
+}
+
+#define SHRINK_TYPE_NEAREST( TYPE ) { \
+	for( x = 0; x < target->width; x++ ) { \
+		TYPE *tp = (TYPE *) p; \
+		TYPE *tp1 = (TYPE *) (p + ls); \
+		TYPE *tq = (TYPE *) q; \
+		\
+		for( z = 0; z < nb; z++ ) \
+        		tq[z] = tp[z]; \
+		\
 		p += ps << 1; \
 		q += ps; \
 	} \
@@ -1377,6 +1385,7 @@ VIPS_REGION_SHRINK( MAX );
 VIPS_REGION_SHRINK( MIN );
 VIPS_REGION_SHRINK( MODE );
 VIPS_REGION_SHRINK( MEDIAN );
+VIPS_REGION_SHRINK( NEAREST );
 
 /* Generate area @target in @to using pixels in @from. Non-complex.
  */
@@ -1405,6 +1414,10 @@ vips_region_shrink_uncoded( VipsRegion *from,
 			vips_region_shrink_uncoded_MIN( from, to, target );
 			break;
 
+		case VIPS_REGION_SHRINK_NEAREST:
+			vips_region_shrink_uncoded_NEAREST( from, to, target );
+			break;
+
 		default:
 			g_assert_not_reached();
 	}
diff --git a/test/test-suite/test_foreign.py b/test/test-suite/test_foreign.py
index 4c18af71..517f3379 100644
--- a/test/test-suite/test_foreign.py
+++ b/test/test-suite/test_foreign.py
@@ -419,11 +419,14 @@ class TestForeign:
         assert a.height == b.height
         assert a.avg() == b.avg()
 
-        # region-shrink added in 8.7
         x = pyvips.Image.new_from_file(TIF_FILE)
         buf = x.tiffsave_buffer(tile=True, pyramid=True, region_shrink="mean")
         buf = x.tiffsave_buffer(tile=True, pyramid=True, region_shrink="mode")
         buf = x.tiffsave_buffer(tile=True, pyramid=True, region_shrink="median")
+        buf = x.tiffsave_buffer(tile=True, pyramid=True, region_shrink="max")
+        buf = x.tiffsave_buffer(tile=True, pyramid=True, region_shrink="min")
+        buf = x.tiffsave_buffer(tile=True, pyramid=True,
+                                region_shrink="nearest")
 
     @skip_if_no("magickload")
     def test_magickload(self):

From e60030b6e80eb2e3c6d225594ef4fd598e53ddbc Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Thu, 30 Jan 2020 17:21:10 +0000
Subject: [PATCH 06/16] start reworking gifload for VipsSource

---
 libvips/foreign/gifload.c  | 311 +++++++++++--------------------------
 libvips/foreign/tiffload.c |   2 +-
 2 files changed, 94 insertions(+), 219 deletions(-)

diff --git a/libvips/foreign/gifload.c b/libvips/foreign/gifload.c
index 0ff2273b..59b5d7e7 100644
--- a/libvips/foreign/gifload.c
+++ b/libvips/foreign/gifload.c
@@ -134,6 +134,10 @@ typedef struct _VipsForeignLoadGif {
 	 */
 	int n;
 
+	/* Load from this source (set by subclasses).
+	 */
+	VipsSource *source;
+
 	GifFileType *file;
 
 	/* We decompress the whole thing to a huge RGBA memory image, and
@@ -203,23 +207,7 @@ typedef struct _VipsForeignLoadGif {
 
 } VipsForeignLoadGif;
 
-typedef struct _VipsForeignLoadGifClass {
-	VipsForeignLoadClass parent_class;
-
-	/* Open the reader (eg. the FILE we are reading from). giflib is
-	 * created in _header and freed in _dispose.
-	 */
-	int (*open)( VipsForeignLoadGif *gif );
-
-	/* Rewind the reader, eg. fseek() back to the start.
-	 */
-	void (*rewind)( VipsForeignLoadGif *gif );
-
-	/* Close the reader.
-	 */
-	void (*close)( VipsForeignLoadGif *gif );
-
-} VipsForeignLoadGifClass;
+typedef VipsForeignLoadClass VipsForeignLoadGifClass;
 
 G_DEFINE_ABSTRACT_TYPE( VipsForeignLoadGif, vips_foreign_load_gif,
 	VIPS_TYPE_FOREIGN_LOAD );
@@ -318,8 +306,6 @@ vips_foreign_load_gif_error( VipsForeignLoadGif *gif )
 static int
 vips_foreign_load_gif_close_giflib( VipsForeignLoadGif *gif )
 {
-	VipsForeignLoadGifClass *class = VIPS_FOREIGN_LOAD_GIF_GET_CLASS( gif );
-
 	VIPS_DEBUG_MSG( "vips_foreign_load_gif_close_giflib:\n" );
 
 #ifdef HAVE_GIFLIB_5
@@ -346,39 +332,54 @@ vips_foreign_load_gif_close_giflib( VipsForeignLoadGif *gif )
 	}
 #endif
 
-	class->close( gif );
+	vips_source_minimise( gif->source );
 
 	return( 0 );
 }
 
+/* Callback from the gif loader.
+ *
+ * Read up to len bytes into buffer, return number of bytes read, 0 for EOF.
+ */
+static int
+vips_giflib_read( GifFileType *file, GifByteType *buf, int n )
+{
+	VipsForeignLoadGif *gif = (VipsForeignLoadGif *) file->UserData;
+
+	gint64 read;
+
+	read = vips_source_read( gif->source, buf, n );
+	if( read == 0 )
+		gif->eof = TRUE;
+
+	return( (int) read ); 
+}
+
 /* Open any underlying file resource, then giflib.
  */
 static int
 vips_foreign_load_gif_open_giflib( VipsForeignLoadGif *gif )
 {
-	VipsForeignLoadGifClass *class = VIPS_FOREIGN_LOAD_GIF_GET_CLASS( gif );
-
 	VIPS_DEBUG_MSG( "vips_foreign_load_gif_open_giflib:\n" );
 
-	if( class->open( gif ) )
-		return( -1 );
+	g_assert( !gif->file );
 
 	/* Must always rewind before opening giflib again.
 	 */
-	class->rewind( gif );
+	vips_source_rewind( gif->source );
 
 #ifdef HAVE_GIFLIB_5
 {
 	int error;
 
-	if( !(gif->file = DGifOpen( gif, gif->read_func, &error )) ) {
+	if( !(gif->file = DGifOpen( gif, vips_giflib_read, &error )) ) {
 		vips_foreign_load_gif_error_vips( gif, error );
 		(void) vips_foreign_load_gif_close_giflib( gif );
 		return( -1 );
 	}
 }
 #else 
-	if( !(gif->file = DGifOpen( gif, gif->read_func )) ) { 
+	if( !(gif->file = DGifOpen( gif, vips_giflib_read )) ) { 
 		vips_foreign_load_gif_error_vips( gif, GifLastError() ); 
 		(void) vips_foreign_load_gif_close_giflib( gif );
 		return( -1 ); 
@@ -400,6 +401,7 @@ vips_foreign_load_gif_dispose( GObject *gobject )
 
 	vips_foreign_load_gif_close_giflib( gif );
 
+	VIPS_UNREF( gif->source );
 	VIPS_UNREF( gif->frame );
 	VIPS_UNREF( gif->previous );
 	VIPS_FREE( gif->comment );
@@ -423,30 +425,18 @@ vips_foreign_load_gif_get_flags( VipsForeignLoad *load )
 }
 
 static gboolean
-vips_foreign_load_gif_is_a_buffer( const void *buf, size_t len )
+vips_foreign_load_gif_is_a_source( VipsSource *source )
 {
-	const guchar *str = (const guchar *) buf;
+	const unsigned char *data;
 
-	if( len >= 4 &&
-		str[0] == 'G' &&
-		str[1] == 'I' &&
-		str[2] == 'F' &&
-		str[3] == '8' )
-		return( 1 );
+	if( (data = vips_source_sniff( source, 4 )) &&
+		data[0] == 'G' &&
+		data[1] == 'I' &&
+		data[2] == 'F' &&
+		data[3] == '8' )
+		return( TRUE );
 
-	return( 0 );
-}
-
-static gboolean
-vips_foreign_load_gif_is_a( const char *filename )
-{
-	unsigned char buf[4];
-
-	if( vips__get_bytes( filename, buf, 4 ) == 4 &&
-		vips_foreign_load_gif_is_a_buffer( buf, 4 ) )
-		return( 1 );
-
-	return( 0 );
+	return( FALSE );
 }
 
 /* Make sure delays is allocated and large enough.
@@ -686,7 +676,8 @@ vips_foreign_load_gif_set_header( VipsForeignLoadGif *gif, VipsImage *image )
 	 * Not the correct behavior as loop=1 became gif-loop=0
 	 * but we want to keep the old behavior untouched!
 	 */
-	vips_image_set_int( image, "gif-loop", gif->loop == 0 ? 0 : gif->loop - 1 );
+	vips_image_set_int( image, 
+		"gif-loop", gif->loop == 0 ? 0 : gif->loop - 1 );
 
 	if( gif->delays ) {
 		/* The deprecated gif-delay field is in centiseconds.
@@ -1079,7 +1070,6 @@ vips_foreign_load_gif_generate( VipsRegion *or,
 {
         VipsRect *r = &or->valid;
 	VipsForeignLoadGif *gif = (VipsForeignLoadGif *) a;
-	VipsForeignLoadGifClass *class = VIPS_FOREIGN_LOAD_GIF_GET_CLASS( gif );
 
 	int y;
 
@@ -1090,11 +1080,6 @@ vips_foreign_load_gif_generate( VipsRegion *or,
 		r->left, r->top, r->width, r->height );
 #endif /*DEBUG_VERBOSE*/
 
-	/* May have been minimised. Reopen the fp if necessary.
-	 */
-	if( class->open( gif ) )
-		return( -1 );
-
 	for( y = 0; y < r->height; y++ ) {
 		/* The page for this output line, and the line number in page.
 		 */
@@ -1169,9 +1154,7 @@ vips_foreign_load_gif_generate( VipsRegion *or,
 static void
 vips_foreign_load_gif_minimise( VipsObject *object, VipsForeignLoadGif *gif )
 {
-	VipsForeignLoadGifClass *class = VIPS_FOREIGN_LOAD_GIF_GET_CLASS( gif );
-
-	class->close( gif );
+	vips_source_minimise( gif->source );
 }
 
 static int
@@ -1231,22 +1214,6 @@ vips_foreign_load_gif_load( VipsForeignLoad *load )
 	return( 0 );
 }
 
-static int
-vips_foreign_load_gif_open( VipsForeignLoadGif *gif )
-{
-	return( 0 );
-}
-
-static void
-vips_foreign_load_gif_rewind( VipsForeignLoadGif *gif )
-{
-}
-
-static void
-vips_foreign_load_gif_close( VipsForeignLoadGif *gif )
-{
-}
-
 static void
 vips_foreign_load_gif_class_init( VipsForeignLoadGifClass *class )
 {
@@ -1267,10 +1234,6 @@ vips_foreign_load_gif_class_init( VipsForeignLoadGifClass *class )
 		vips_foreign_load_gif_get_flags_filename;
 	load_class->get_flags = vips_foreign_load_gif_get_flags;
 
-	class->open = vips_foreign_load_gif_open;
-	class->rewind = vips_foreign_load_gif_rewind;
-	class->close = vips_foreign_load_gif_close;
-
 	VIPS_ARG_INT( class, "page", 20,
 		_( "Page" ),
 		_( "Load this page from the file" ),
@@ -1308,14 +1271,6 @@ typedef struct _VipsForeignLoadGifFile {
 	 */
 	char *filename;
 
-	/* The FILE* we read from.
-	 */
-	FILE *fp;
-
-	/* If we close and reopen, save the ftell point here.
-	 */
-	long seek_position;
-
 } VipsForeignLoadGifFile;
 
 typedef VipsForeignLoadGifClass VipsForeignLoadGifFileClass;
@@ -1323,82 +1278,22 @@ typedef VipsForeignLoadGifClass VipsForeignLoadGifFileClass;
 G_DEFINE_TYPE( VipsForeignLoadGifFile, vips_foreign_load_gif_file,
 	vips_foreign_load_gif_get_type() );
 
-/* Our input function for file open. We can't use DGifOpenFileName(), since
- * that just calls open() and won't work with unicode on win32. We can't use
- * DGifOpenFileHandle() since that's an fd from open() and you can't pass those
- * across DLL boundaries on Windows.
- */
-static int 
-vips_giflib_file_read( GifFileType *gfile, GifByteType *buffer, int n )
-{
-	VipsForeignLoadGif *gif = (VipsForeignLoadGif *) gfile->UserData;
-	VipsForeignLoadGifFile *file = (VipsForeignLoadGifFile *) gif;
-
-	if( feof( file->fp ) )
-		gif->eof = TRUE;
-
-	return( (int) fread( (void *) buffer, 1, n, file->fp ) );
-}
-
-/* We have to have _open() as a vfunc since we want to be able to reopen in
- * _generate if we have been closed during _minimise. 
- */
 static int
-vips_foreign_load_gif_file_open( VipsForeignLoadGif *gif )
+vips_foreign_load_gif_file_build( VipsObject *object )
 {
-	VipsForeignLoad *load = (VipsForeignLoad *) gif;
-	VipsForeignLoadGifFile *file = (VipsForeignLoadGifFile *) gif;
+	VipsForeignLoadGifFile *file = (VipsForeignLoadGifFile *) object;
+	VipsForeignLoadGif *gif = (VipsForeignLoadGif *) object;
 
-	VIPS_DEBUG_MSG( "vips_foreign_load_gif_file_open:\n" );
-
-	if( !file->fp ) {
-		if( !(file->fp =
-			vips__file_open_read( file->filename, NULL, FALSE )) )
+	if( file->filename ) 
+		if( !(gif->source = 
+			vips_source_new_from_file( file->filename )) )
 			return( -1 );
 
-		/* Restore the read point if we are reopening.
-		 */
-		if( file->seek_position != -1 )
-			fseek( file->fp, file->seek_position, SEEK_SET );
+	if( VIPS_OBJECT_CLASS( vips_foreign_load_gif_file_parent_class )->
+		build( object ) )
+		return( -1 );
 
-		VIPS_SETSTR( load->out->filename, file->filename );
-		gif->read_func = vips_giflib_file_read;
-	}
-
-	return( VIPS_FOREIGN_LOAD_GIF_CLASS(
-		vips_foreign_load_gif_file_parent_class )->open( gif ) );
-}
-
-static void
-vips_foreign_load_gif_file_rewind( VipsForeignLoadGif *gif )
-{
-	VipsForeignLoadGifFile *file = (VipsForeignLoadGifFile *) gif;
-
-	VIPS_DEBUG_MSG( "vips_foreign_load_gif_file_rewind:\n" );
-
-	if( file->fp ) {
-		file->seek_position = 0;
-		fseek( file->fp, file->seek_position, SEEK_SET );
-	}
-
-	VIPS_FOREIGN_LOAD_GIF_CLASS(
-		vips_foreign_load_gif_file_parent_class )->rewind( gif );
-}
-
-static void
-vips_foreign_load_gif_file_close( VipsForeignLoadGif *gif )
-{
-	VipsForeignLoadGifFile *file = (VipsForeignLoadGifFile *) gif;
-
-	VIPS_DEBUG_MSG( "vips_foreign_load_gif_file_close:\n" );
-
-	if( file->fp ) {
-		file->seek_position = ftell( file->fp );
-		VIPS_FREEF( fclose, file->fp );
-	}
-
-	VIPS_FOREIGN_LOAD_GIF_CLASS(
-		vips_foreign_load_gif_file_parent_class )->close( gif );
+	return( 0 );
 }
 
 static const char *vips_foreign_gif_suffs[] = {
@@ -1406,6 +1301,20 @@ static const char *vips_foreign_gif_suffs[] = {
 	NULL
 };
 
+static gboolean
+vips_foreign_load_gif_file_is_a( const char *filename )
+{
+	VipsSource *source;
+	gboolean result;
+
+	if( !(source = vips_source_new_from_file( filename )) )
+		return( FALSE );
+	result = vips_foreign_load_gif_is_a_source( source );
+	VIPS_UNREF( source );
+
+	return( result );
+}
+
 static void
 vips_foreign_load_gif_file_class_init(
 	VipsForeignLoadGifFileClass *class )
@@ -1414,21 +1323,17 @@ vips_foreign_load_gif_file_class_init(
 	VipsObjectClass *object_class = (VipsObjectClass *) class;
 	VipsForeignClass *foreign_class = (VipsForeignClass *) class;
 	VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class;
-	VipsForeignLoadGifClass *gif_class = (VipsForeignLoadGifClass *) class;
 
 	gobject_class->set_property = vips_object_set_property;
 	gobject_class->get_property = vips_object_get_property;
 
 	object_class->nickname = "gifload";
 	object_class->description = _( "load GIF with giflib" );
+	object_class->build = vips_foreign_load_gif_file_build;
 
 	foreign_class->suffs = vips_foreign_gif_suffs;
 
-	load_class->is_a = vips_foreign_load_gif_is_a;
-
-	gif_class->open = vips_foreign_load_gif_file_open;
-	gif_class->rewind = vips_foreign_load_gif_file_rewind;
-	gif_class->close = vips_foreign_load_gif_file_close;
+	load_class->is_a = vips_foreign_load_gif_file_is_a;
 
 	VIPS_ARG_STRING( class, "filename", 1,
 		_( "Filename" ),
@@ -1442,7 +1347,6 @@ vips_foreign_load_gif_file_class_init(
 static void
 vips_foreign_load_gif_file_init( VipsForeignLoadGifFile *file )
 {
-	file->seek_position = -1;
 }
 
 typedef struct _VipsForeignLoadGifBuffer {
@@ -1450,12 +1354,7 @@ typedef struct _VipsForeignLoadGifBuffer {
 
 	/* Load from a buffer.
 	 */
-	VipsArea *buf;
-
-	/* Current read point, bytes left in buffer.
-	 */
-	VipsPel *p;
-	size_t bytes_to_go;
+	VipsArea *blob;
 
 } VipsForeignLoadGifBuffer;
 
@@ -1464,59 +1363,38 @@ typedef VipsForeignLoadGifClass VipsForeignLoadGifBufferClass;
 G_DEFINE_TYPE( VipsForeignLoadGifBuffer, vips_foreign_load_gif_buffer,
 	vips_foreign_load_gif_get_type() );
 
-/* Callback from the gif loader.
- *
- * Read up to len bytes into buffer, return number of bytes read, 0 for EOF.
- */
 static int
-vips_giflib_buffer_read( GifFileType *file, GifByteType *buf, int n )
+vips_foreign_load_gif_buffer_build( VipsObject *object )
 {
-	VipsForeignLoadGif *gif = (VipsForeignLoadGif *) file->UserData;
-	VipsForeignLoadGifBuffer *buffer = (VipsForeignLoadGifBuffer *) gif;
-	size_t will_read = VIPS_MIN( n, buffer->bytes_to_go );
+	VipsForeignLoadGif *gif = (VipsForeignLoadGif *) object;
+	VipsForeignLoadGifBuffer *buffer = 
+		(VipsForeignLoadGifBuffer *) object;
 
-	memcpy( buf, buffer->p, will_read );
-	buffer->p += will_read;
-	buffer->bytes_to_go -= will_read;
+	if( buffer->blob &&
+		!(gif->source = vips_source_new_from_memory( 
+			VIPS_AREA( buffer->blob )->data, 
+			VIPS_AREA( buffer->blob )->length )) )
+		return( -1 );
 
-	if( will_read == 0 )
-		gif->eof = TRUE;
+	if( VIPS_OBJECT_CLASS( vips_foreign_load_gif_buffer_parent_class )->
+		build( object ) )
+		return( -1 );
 
-	return( will_read ); 
+	return( 0 );
 }
 
-static int
-vips_foreign_load_gif_buffer_open( VipsForeignLoadGif *gif )
+static gboolean
+vips_foreign_load_gif_buffer_is_a_buffer( const void *buf, size_t len )
 {
-	VipsForeignLoadGifBuffer *buffer = (VipsForeignLoadGifBuffer *) gif;
+	VipsSource *source;
+	gboolean result;
 
-	VIPS_DEBUG_MSG( "vips_foreign_load_gif_buffer_open:\n" );
+	if( !(source = vips_source_new_from_memory( buf, len )) )
+		return( FALSE );
+	result = vips_foreign_load_gif_is_a_source( source );
+	VIPS_UNREF( source );
 
-	/* We can open several times -- make sure we don't move the read point
-	 * if we reopen.
-	 */
-	if( !buffer->p ) {
-		buffer->p = buffer->buf->data;
-		buffer->bytes_to_go = buffer->buf->length;
-		gif->read_func = vips_giflib_buffer_read;
-	}
-
-	return( VIPS_FOREIGN_LOAD_GIF_CLASS(
-		vips_foreign_load_gif_buffer_parent_class )->open( gif ) );
-}
-
-static void
-vips_foreign_load_gif_buffer_rewind( VipsForeignLoadGif *gif )
-{
-	VipsForeignLoadGifBuffer *buffer = (VipsForeignLoadGifBuffer *) gif;
-
-	VIPS_DEBUG_MSG( "vips_foreign_load_gif_buffer_rewind:\n" );
-
-	buffer->p = buffer->buf->data;
-	buffer->bytes_to_go = buffer->buf->length;
-
-	VIPS_FOREIGN_LOAD_GIF_CLASS(
-		vips_foreign_load_gif_buffer_parent_class )->rewind( gif );
+	return( result );
 }
 
 static void
@@ -1526,24 +1404,21 @@ vips_foreign_load_gif_buffer_class_init(
 	GObjectClass *gobject_class = G_OBJECT_CLASS( class );
 	VipsObjectClass *object_class = (VipsObjectClass *) class;
 	VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class;
-	VipsForeignLoadGifClass *gif_class = (VipsForeignLoadGifClass *) class;
 
 	gobject_class->set_property = vips_object_set_property;
 	gobject_class->get_property = vips_object_get_property;
 
 	object_class->nickname = "gifload_buffer";
 	object_class->description = _( "load GIF with giflib" );
+	object_class->build = vips_foreign_load_gif_buffer_build;
 
-	load_class->is_a_buffer = vips_foreign_load_gif_is_a_buffer;
-
-	gif_class->open = vips_foreign_load_gif_buffer_open;
-	gif_class->rewind = vips_foreign_load_gif_buffer_rewind;
+	load_class->is_a_buffer = vips_foreign_load_gif_buffer_is_a_buffer;
 
 	VIPS_ARG_BOXED( class, "buffer", 1,
 		_( "Buffer" ),
 		_( "Buffer to load from" ),
 		VIPS_ARGUMENT_REQUIRED_INPUT,
-		G_STRUCT_OFFSET( VipsForeignLoadGifBuffer, buf ),
+		G_STRUCT_OFFSET( VipsForeignLoadGifBuffer, blob ),
 		VIPS_TYPE_BLOB );
 
 }
diff --git a/libvips/foreign/tiffload.c b/libvips/foreign/tiffload.c
index 96e0789a..20b2b094 100644
--- a/libvips/foreign/tiffload.c
+++ b/libvips/foreign/tiffload.c
@@ -389,7 +389,7 @@ vips_foreign_load_tiff_buffer_build( VipsObject *object )
 			VIPS_AREA( buffer->blob )->length )) )
 		return( -1 );
 
-	if( VIPS_OBJECT_CLASS( vips_foreign_load_tiff_file_parent_class )->
+	if( VIPS_OBJECT_CLASS( vips_foreign_load_tiff_buffer_parent_class )->
 		build( object ) )
 		return( -1 );
 

From 63b755e73eb2952eba9c27bc7daa1e8830a8f5c4 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Thu, 30 Jan 2020 21:46:31 +0000
Subject: [PATCH 07/16] fix pytest

---
 libvips/foreign/gifload.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libvips/foreign/gifload.c b/libvips/foreign/gifload.c
index 59b5d7e7..4ef82b85 100644
--- a/libvips/foreign/gifload.c
+++ b/libvips/foreign/gifload.c
@@ -34,6 +34,8 @@
  * 	- check image and frame bounds, since giflib does not
  * 1/9/19
  * 	- improve early close again
+ * 30/1/19
+ * 	- rework on top of VipsSource
  */
 
 /*
@@ -332,7 +334,8 @@ vips_foreign_load_gif_close_giflib( VipsForeignLoadGif *gif )
 	}
 #endif
 
-	vips_source_minimise( gif->source );
+	if( gif->source )
+		vips_source_minimise( gif->source );
 
 	return( 0 );
 }

From acabd2dc08551efabafabba7042a1b57662e0578 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Thu, 30 Jan 2020 22:15:48 +0000
Subject: [PATCH 08/16] add gifload_source

---
 ChangeLog                      |  1 +
 libvips/foreign/foreign.c      |  2 +
 libvips/foreign/gifload.c      | 94 ++++++++++++++++++++++++++++++++++
 libvips/include/vips/foreign.h |  2 +
 4 files changed, 99 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 1341d6f2..01432673 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,7 @@
 - allow \ as an escape character in vips_break_token() [akemrir]
 - tiffsave has a "depth" param to set max pyr depth
 - libtiff LOGLUV images load and save as libvips XYZ 
+- add gifload_source
 
 20/6/19 started 8.9.1
 - don't use the new source loaders for new_from_file or new_from_buffer, it
diff --git a/libvips/foreign/foreign.c b/libvips/foreign/foreign.c
index d59016ce..86632973 100644
--- a/libvips/foreign/foreign.c
+++ b/libvips/foreign/foreign.c
@@ -2107,6 +2107,7 @@ vips_foreign_operation_init( void )
 
 	extern GType vips_foreign_load_gif_file_get_type( void ); 
 	extern GType vips_foreign_load_gif_buffer_get_type( void ); 
+	extern GType vips_foreign_load_gif_source_get_type( void ); 
 
 	vips_foreign_load_csv_get_type(); 
 	vips_foreign_save_csv_get_type(); 
@@ -2158,6 +2159,7 @@ vips_foreign_operation_init( void )
 #ifdef HAVE_GIFLIB
 	vips_foreign_load_gif_file_get_type(); 
 	vips_foreign_load_gif_buffer_get_type(); 
+	vips_foreign_load_gif_source_get_type(); 
 #endif /*HAVE_GIFLIB*/
 
 #ifdef HAVE_GSF
diff --git a/libvips/foreign/gifload.c b/libvips/foreign/gifload.c
index 4ef82b85..a4a6b3ac 100644
--- a/libvips/foreign/gifload.c
+++ b/libvips/foreign/gifload.c
@@ -36,6 +36,7 @@
  * 	- improve early close again
  * 30/1/19
  * 	- rework on top of VipsSource
+ * 	- add gifload_source
  */
 
 /*
@@ -1431,6 +1432,70 @@ vips_foreign_load_gif_buffer_init( VipsForeignLoadGifBuffer *buffer )
 {
 }
 
+typedef struct _VipsForeignLoadGifSource {
+	VipsForeignLoadGif parent_object;
+
+	/* Load from a source.
+	 */
+	VipsSource *source;
+
+} VipsForeignLoadGifSource;
+
+typedef VipsForeignLoadGifClass VipsForeignLoadGifSourceClass;
+
+G_DEFINE_TYPE( VipsForeignLoadGifSource, vips_foreign_load_gif_source,
+	vips_foreign_load_gif_get_type() );
+
+static int
+vips_foreign_load_gif_source_build( VipsObject *object )
+{
+	VipsForeignLoadGif *gif = (VipsForeignLoadGif *) object;
+	VipsForeignLoadGifSource *source = 
+		(VipsForeignLoadGifSource *) object;
+
+	if( source->source ) {
+		gif->source = source->source;
+		g_object_ref( gif->source );
+	}
+
+	if( VIPS_OBJECT_CLASS( vips_foreign_load_gif_source_parent_class )->
+		build( object ) )
+		return( -1 );
+
+	return( 0 );
+}
+
+static void
+vips_foreign_load_gif_source_class_init(
+	VipsForeignLoadGifSourceClass *class )
+{
+	GObjectClass *gobject_class = G_OBJECT_CLASS( class );
+	VipsObjectClass *object_class = (VipsObjectClass *) class;
+	VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class;
+
+	gobject_class->set_property = vips_object_set_property;
+	gobject_class->get_property = vips_object_get_property;
+
+	object_class->nickname = "gifload_source";
+	object_class->description = _( "load GIF with giflib" );
+	object_class->build = vips_foreign_load_gif_source_build;
+
+	load_class->is_a_source = vips_foreign_load_gif_is_a_source;
+
+	VIPS_ARG_OBJECT( class, "source", 1,
+		_( "Source" ),
+		_( "Source to load from" ),
+		VIPS_ARGUMENT_REQUIRED_INPUT, 
+		G_STRUCT_OFFSET( VipsForeignLoadGifSource, source ),
+		VIPS_TYPE_SOURCE );
+
+}
+
+static void
+vips_foreign_load_gif_source_init( VipsForeignLoadGifSource *source )
+{
+}
+
 #endif /*HAVE_GIFLIB*/
 
 /**
@@ -1515,3 +1580,32 @@ vips_gifload_buffer( void *buf, size_t len, VipsImage **out, ... )
 	return( result );
 }
 
+/**
+ * vips_gifload_source:
+ * @source: source to load
+ * @out: (out): image to write
+ * @...: %NULL-terminated list of optional named arguments
+ *
+ * Optional arguments:
+ *
+ * * @page: %gint, page (frame) to read
+ * * @n: %gint, load this many pages
+ *
+ * Exactly as vips_gifload(), but read from a source. 
+ *
+ * See also: vips_gifload().
+ *
+ * Returns: 0 on success, -1 on error.
+ */
+int
+vips_gifload_source( VipsSource *source, VipsImage **out, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, out );
+	result = vips_call_split( "gifload_source", ap, source, out );
+	va_end( ap );
+
+	return( result );
+}
diff --git a/libvips/include/vips/foreign.h b/libvips/include/vips/foreign.h
index 2447b3d1..1f84f4f9 100644
--- a/libvips/include/vips/foreign.h
+++ b/libvips/include/vips/foreign.h
@@ -597,6 +597,8 @@ int vips_gifload( const char *filename, VipsImage **out, ... )
 	__attribute__((sentinel));
 int vips_gifload_buffer( void *buf, size_t len, VipsImage **out, ... )
 	__attribute__((sentinel));
+int vips_gifload_source( VipsSource *source, VipsImage **out, ... )
+	__attribute__((sentinel));
 
 int vips_heifload( const char *filename, VipsImage **out, ... )
 	__attribute__((sentinel));

From e4db74746a195b4688b5c40d0cb2da5abccc224a Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Fri, 31 Jan 2020 15:25:05 +0000
Subject: [PATCH 09/16] fix a deadlock with --vips-leak

We were usingh a global lock for metadata changes, but some functions
triggered from callbacks in the metadata hash table could also attempt
to acquire the same mutex, leading to deadlock.

This patch gives metadata change it's own lock. Thanks DarthSim.

See https://github.com/libvips/libvips/issues/1542
---
 ChangeLog                      |  3 +++
 configure.ac                   |  6 ++---
 libvips/include/vips/private.h |  2 ++
 libvips/iofuncs/header.c       | 43 +++++++++++++++++-----------------
 libvips/iofuncs/init.c         |  1 +
 5 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9c168622..40a955be 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,6 @@
+31/1/19 started 8.9.2
+- fix a deadlock with --vips-leak [DarthSim]
+
 20/6/19 started 8.9.1
 - don't use the new source loaders for new_from_file or new_from_buffer, it
   will break the loader priority system
diff --git a/configure.ac b/configure.ac
index 43d0c85a..98173c27 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,7 +2,7 @@
 
 # also update the version number in the m4 macros below
 
-AC_INIT([vips], [8.9.1], [vipsip@jiscmail.ac.uk])
+AC_INIT([vips], [8.9.2], [vipsip@jiscmail.ac.uk])
 # required for gobject-introspection
 AC_PREREQ(2.62)
 
@@ -18,7 +18,7 @@ AC_CONFIG_MACRO_DIR([m4])
 # user-visible library versioning
 m4_define([vips_major_version], [8])
 m4_define([vips_minor_version], [9])
-m4_define([vips_micro_version], [1])
+m4_define([vips_micro_version], [2])
 m4_define([vips_version],
           [vips_major_version.vips_minor_version.vips_micro_version])
 
@@ -38,7 +38,7 @@ VIPS_VERSION_STRING=$VIPS_VERSION-`date -u -r ChangeLog`
 #   binary interface changes not backwards compatible?: reset age to 0
 
 LIBRARY_CURRENT=54
-LIBRARY_REVISION=1
+LIBRARY_REVISION=2
 LIBRARY_AGE=12
 
 # patched into include/vips/version.h
diff --git a/libvips/include/vips/private.h b/libvips/include/vips/private.h
index e28735fc..8716cf36 100644
--- a/libvips/include/vips/private.h
+++ b/libvips/include/vips/private.h
@@ -192,6 +192,8 @@ int vips__view_image( struct _VipsImage *image );
  */
 extern int _vips__argument_id;
 
+void vips__meta_init( void );
+
 #ifdef __cplusplus
 }
 #endif /*__cplusplus*/
diff --git a/libvips/iofuncs/header.c b/libvips/iofuncs/header.c
index 6a6347a3..dba1c767 100644
--- a/libvips/iofuncs/header.c
+++ b/libvips/iofuncs/header.c
@@ -36,6 +36,8 @@
  * 	- add vips_image_get_n_pages()
  * 20/6/19
  * 	- add vips_image_get/set_array_int()
+ * 31/1/19
+ * 	- lock for metadata changes
  */
 
 /*
@@ -130,6 +132,11 @@
  * these types, it can be copied between images efficiently.
  */
 
+/* Use in various small places where we need a mutex and it's not worth 
+ * making a private one.
+ */
+static GMutex *vips__meta_lock = NULL;
+
 /* We have to keep the gtype as a string, since we statically init this.
  */
 typedef struct _HeaderField {
@@ -934,10 +941,10 @@ meta_cp( VipsImage *dst, const VipsImage *src )
 		/* We lock with vips_image_set() to stop races in highly-
 		 * threaded applications.
 		 */
-		g_mutex_lock( vips__global_lock );
+		g_mutex_lock( vips__meta_lock );
 		vips_slist_map2( src->meta_traverse,
 			(VipsSListMap2Fn) meta_cp_field, dst, NULL );
-		g_mutex_unlock( vips__global_lock );
+		g_mutex_unlock( vips__meta_lock );
 	}
 
 	return( 0 );
@@ -1025,13 +1032,6 @@ vips_image_set( VipsImage *image, const char *name, GValue *value )
 	g_assert( name );
 	g_assert( value );
 
-        /* If this image is shared, block metadata changes.
-         */
-        if( G_OBJECT( image )->ref_count > 1 ) {
-                g_warning( "can't set metadata \"%s\" on shared image", name );
-                return;
-        }
-
 	meta_init( image );
 
 	/* We lock between modifying metadata and copying metadata between
@@ -1041,9 +1041,9 @@ vips_image_set( VipsImage *image, const char *name, GValue *value )
 	 * metadata copy on another -- this can lead to crashes in
 	 * highly-threaded applications.
 	 */
-	g_mutex_lock( vips__global_lock );
+	g_mutex_lock( vips__meta_lock );
 	(void) meta_new( image, name, value );
-	g_mutex_unlock( vips__global_lock );
+	g_mutex_unlock( vips__meta_lock );
 
 	/* If we're setting an EXIF data block, we need to automatically expand 
 	 * out all the tags. This will set things like xres/yres too.
@@ -1240,14 +1240,6 @@ vips_image_remove( VipsImage *image, const char *name )
 
 	result = FALSE;
 
-        /* If this image is shared, block metadata changes. 
-         */
-        if( G_OBJECT( image )->ref_count > 1 ) {
-                g_warning( "can't remove metadata \"%s\" on shared image", 
-			name );
-                return( result );
-        }
-
 	if( image->meta ) {
 		/* We lock between modifying metadata and copying metadata 
 		 * between images, see meta_cp().
@@ -1256,9 +1248,9 @@ vips_image_remove( VipsImage *image, const char *name )
 		 * racing with metadata copy on another -- this can lead to 
 		 * crashes in highly-threaded applications.
 		 */
-		g_mutex_lock( vips__global_lock );
+		g_mutex_lock( vips__meta_lock );
 		result = g_hash_table_remove( image->meta, name );
-		g_mutex_unlock( vips__global_lock );
+		g_mutex_unlock( vips__meta_lock );
 	}
 
 	return( result );
@@ -2027,3 +2019,12 @@ vips_image_get_history( VipsImage *image )
 
 	return( image->Hist ? image->Hist : "" );
 }
+
+/* Called during vips_init().
+ */
+void
+vips__meta_init( void )
+{
+	if( !vips__meta_lock ) 
+		vips__meta_lock = vips_g_mutex_new();
+}
diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c
index bd5a90cc..20ed972d 100644
--- a/libvips/iofuncs/init.c
+++ b/libvips/iofuncs/init.c
@@ -382,6 +382,7 @@ vips_init( const char *argv0 )
 
 	vips__threadpool_init();
 	vips__buffer_init();
+	vips__meta_init();
 
 	/* This does an unsynchronised static hash table init on first call --
 	 * we have to make sure we do this single-threaded. See: 

From 8a21f6ea52ed3d2aac68b05f4e502a66d266c153 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Fri, 31 Jan 2020 15:51:44 +0000
Subject: [PATCH 10/16] fix gif rendering for "waterfall.gif"

This GIF has dispose set to DISPOSAL_UNSPECIFIED and seems to mean
transparent.

This patch makes gifload use DISPOSAL_UNSPECIFIED as well as _DO_NOT to
mean reuse previous frame.

Thanks DarthSim.

See https://github.com/libvips/libvips/issues/1543
---
 ChangeLog                 |  1 +
 libvips/foreign/gifload.c | 30 +++++++++++++++++++++++++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 40a955be..1f8ddd3c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,6 @@
 31/1/19 started 8.9.2
 - fix a deadlock with --vips-leak [DarthSim]
+- better gifload behaviour for DISPOSAL_UNSPECIFIED [DarthSim]
 
 20/6/19 started 8.9.1
 - don't use the new source loaders for new_from_file or new_from_buffer, it
diff --git a/libvips/foreign/gifload.c b/libvips/foreign/gifload.c
index 0ff2273b..367c07d2 100644
--- a/libvips/foreign/gifload.c
+++ b/libvips/foreign/gifload.c
@@ -34,6 +34,9 @@
  * 	- check image and frame bounds, since giflib does not
  * 1/9/19
  * 	- improve early close again
+ * 31/1/20
+ * 	- treat DISPOSAL_UNSPECIFIED as _DO_NOT, since that's what many GIFs
+ * 	  in the wild appear to do
  */
 
 /*
@@ -852,8 +855,12 @@ vips_foreign_load_gif_render_line( VipsForeignLoadGif *gif,
 			/* In DISPOSE_DO_NOT mode, the previous frame shows
 			 * through (ie. we do nothing). In all other modes,
 			 * it's just transparent.
+			 *
+			 * Many GIFs use DISPOSAL_UNSPECIFIED to mean DO_NOT,
+			 * so use that for previous frame as well.
 			 */
-			if( gif->dispose != DISPOSE_DO_NOT )
+			if( gif->dispose != DISPOSE_DO_NOT &&
+				gif->dispose != DISPOSAL_UNSPECIFIED )
 				iq[x] = 0;
 		}
 		else
@@ -976,6 +983,20 @@ vips_foreign_load_gif_render( VipsForeignLoadGif *gif )
 	return( 0 );
 }
 
+#ifdef VIPS_DEBUG
+static const char *
+dispose2str( int dispose )
+{
+	switch( dispose ) {
+	case DISPOSAL_UNSPECIFIED: return( "DISPOSAL_UNSPECIFIED" );
+	case DISPOSE_DO_NOT: return( "DISPOSE_DO_NOT" );
+	case DISPOSE_BACKGROUND: return( "DISPOSE_BACKGROUND" );
+	case DISPOSE_PREVIOUS: return( "DISPOSE_PREVIOUS" );
+	default: return( "<unknown>" );
+	}
+}
+#endif /*VIPS_DEBUG*/
+
 static int
 vips_foreign_load_gif_extension( VipsForeignLoadGif *gif )
 {
@@ -998,15 +1019,18 @@ vips_foreign_load_gif_extension( VipsForeignLoadGif *gif )
 		 * is being set.
 		 */
 		gif->transparency = -1;
-		if( extension[1] & 0x1 )
+		if( extension[1] & 0x1 ) {
 			gif->transparency = extension[4];
+			VIPS_DEBUG_MSG( "vips_foreign_load_gif_extension: "
+				"transparency = %d\n", gif->transparency );
+		}
 
 		/* Set the current dispose mode. This is read during frame load
 		 * to set the meaning of background and transparent pixels.
 		 */
 		gif->dispose = (extension[1] >> 2) & 0x7;
 		VIPS_DEBUG_MSG( "vips_foreign_load_gif_extension: "
-			"dispose = %d\n", gif->dispose );
+			"dispose = %s\n", dispose2str( gif->dispose ) );
 	}
 
 	while( extension != NULL )

From bb20556b6d67c7c0316912b05959b2d1e91a80df Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Sun, 2 Feb 2020 11:13:41 +0000
Subject: [PATCH 11/16] ban ppm max_value < 0

Not allowed by spec, since pixels should be unsigned.
---
 ChangeLog                 | 1 +
 libvips/foreign/ppmload.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 1f8ddd3c..b0c34087 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,7 @@
 31/1/19 started 8.9.2
 - fix a deadlock with --vips-leak [DarthSim]
 - better gifload behaviour for DISPOSAL_UNSPECIFIED [DarthSim]
+- ban ppm max_value < 0 
 
 20/6/19 started 8.9.1
 - don't use the new source loaders for new_from_file or new_from_buffer, it
diff --git a/libvips/foreign/ppmload.c b/libvips/foreign/ppmload.c
index 78709f4b..6df91058 100644
--- a/libvips/foreign/ppmload.c
+++ b/libvips/foreign/ppmload.c
@@ -35,6 +35,8 @@
  * 	- redone with source/target
  * 	- sequential load, plus mmap for filename sources
  * 	- faster plus lower memory use
+ * 02/02/2020
+ * 	- ban max_vaue < 0 
  */
 
 /*
@@ -259,6 +261,12 @@ vips_foreign_load_ppm_parse_header( VipsForeignLoadPpm *ppm )
 			if( get_int( ppm->sbuf, &ppm->max_value ) )
 				return( -1 );
 
+			/* max_value must be > 0 and <= 65535, according to
+			 * the spec, but we allow up to 32 bits per pixel.
+			 */
+			if( ppm->max_value < 0 )
+				ppm->max_value = 0;
+
 			if( ppm->max_value > 255 )
 				ppm->bits = 16;
 			if( ppm->max_value > 65535 )

From 559ae542acbf0f1e544762890bb9129835b0d041 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Mon, 3 Feb 2020 14:49:21 +0000
Subject: [PATCH 12/16] revise flags for vipsthumbnail

The --rotate flag no longer did anything, so add a new --no-rotate flag
connected to the new no-rotate property. --rotate is still there, but
hidden and does nothing.

-o was much easier to remember than -f, so flip back to -o. -f still
works, but is a hidden synonym.

--iprofile, --eprofile were hard to remember. Add --import-profile and
--export-profile synonyms. iprofile / eprofile are still there and still
work, but are hidden.
---
 ChangeLog             |  1 +
 tools/vipsthumbnail.c | 46 +++++++++++++++++++++++++++++--------------
 2 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9dcea1a7..65715ee0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -6,6 +6,7 @@
 - tiffsave has a "depth" param to set max pyr depth
 - libtiff LOGLUV images load and save as libvips XYZ 
 - add gifload_source
+- revise vipsthumbnail flags
 
 31/1/19 started 8.9.2
 - fix a deadlock with --vips-leak [DarthSim]
diff --git a/tools/vipsthumbnail.c b/tools/vipsthumbnail.c
index 961d447d..3e9c37cc 100644
--- a/tools/vipsthumbnail.c
+++ b/tools/vipsthumbnail.c
@@ -96,6 +96,10 @@
  * 	- add --intent
  * 23/10/17
  * 	- --size Nx didn't work, argh ... thanks jrochkind 
+ * 3/2/20
+ * 	- add --no-rotate
+ * 	- add --import-profile / --export-profile names
+ * 	- back to -o for output
  */
 
 #ifdef HAVE_CONFIG_H
@@ -127,8 +131,8 @@ static char *import_profile = NULL;
 static gboolean delete_profile = FALSE;
 static gboolean linear_processing = FALSE;
 static gboolean crop_image = FALSE;
+static gboolean no_rotate_image = FALSE;
 static char *smartcrop_image = NULL;
-static gboolean rotate_image = FALSE;
 static char *thumbnail_intent = NULL;
 
 /* Deprecated and unused.
@@ -138,25 +142,22 @@ static gboolean nodelete_profile = FALSE;
 static gboolean verbose = FALSE;
 static char *convolution_mask = NULL;
 static char *interpolator = NULL;
+static gboolean rotate_image = FALSE;
 
 static GOptionEntry options[] = {
 	{ "size", 's', 0, 
 		G_OPTION_ARG_STRING, &thumbnail_size, 
 		N_( "shrink to SIZE or to WIDTHxHEIGHT" ), 
 		N_( "SIZE" ) },
-	{ "output", 'o', G_OPTION_FLAG_HIDDEN, 
+	{ "output", 'o', 0, 
 		G_OPTION_ARG_STRING, &output_format, 
-		N_( "set output to FORMAT" ), 
+		N_( "output to FORMAT" ), 
 		N_( "FORMAT" ) },
-	{ "format", 'f', 0, 
-		G_OPTION_ARG_STRING, &output_format, 
-		N_( "set output format string to FORMAT" ), 
-		N_( "FORMAT" ) },
-	{ "eprofile", 'e', 0, 
+	{ "export-profile", 'e', 0, 
 		G_OPTION_ARG_FILENAME, &export_profile, 
 		N_( "export with PROFILE" ), 
 		N_( "PROFILE" ) },
-	{ "iprofile", 'i', 0, 
+	{ "import-profile", 'i', 0, 
 		G_OPTION_ARG_FILENAME, &import_profile, 
 		N_( "import untagged images with PROFILE" ), 
 		N_( "PROFILE" ) },
@@ -171,13 +172,28 @@ static GOptionEntry options[] = {
 		G_OPTION_ARG_STRING, &thumbnail_intent, 
 		N_( "ICC transform with INTENT" ), 
 		N_( "INTENT" ) },
-	{ "rotate", 't', 0, 
-		G_OPTION_ARG_NONE, &rotate_image, 
-		N_( "auto-rotate" ), NULL },
 	{ "delete", 'd', 0, 
 		G_OPTION_ARG_NONE, &delete_profile, 
 		N_( "delete profile from exported image" ), NULL },
+	{ "no-rotate", 0, 0, 
+		G_OPTION_ARG_NONE, &no_rotate_image, 
+		N_( "don't auto-rotate" ), NULL },
 
+	{ "format", 'f', G_OPTION_FLAG_HIDDEN, 
+		G_OPTION_ARG_STRING, &output_format, 
+		N_( "set output format string to FORMAT" ), 
+		N_( "FORMAT" ) },
+	{ "eprofile", 0, G_OPTION_FLAG_HIDDEN, 
+		G_OPTION_ARG_FILENAME, &export_profile, 
+		N_( "export with PROFILE" ), 
+		N_( "PROFILE" ) },
+	{ "iprofile", 0, G_OPTION_FLAG_HIDDEN, 
+		G_OPTION_ARG_FILENAME, &import_profile, 
+		N_( "import untagged images with PROFILE" ), 
+		N_( "PROFILE" ) },
+	{ "rotate", 't', G_OPTION_FLAG_HIDDEN, 
+		G_OPTION_ARG_NONE, &rotate_image, 
+		N_( "(deprecated, does nothing)" ), NULL },
 	{ "crop", 'c', G_OPTION_FLAG_HIDDEN, 
 		G_OPTION_ARG_NONE, &crop_image, 
 		N_( "(deprecated, crop exactly to SIZE)" ), NULL },
@@ -280,11 +296,11 @@ thumbnail_process( VipsObject *process, const char *filename )
 	if( vips_thumbnail( filename, &image, thumbnail_width, 
 		"height", thumbnail_height, 
 		"size", size_restriction, 
-		"auto_rotate", rotate_image, 
+		"no-rotate", no_rotate_image, 
 		"crop", interesting, 
 		"linear", linear_processing, 
-		"import_profile", import_profile, 
-		"export_profile", export_profile, 
+		"import-profile", import_profile, 
+		"export-profile", export_profile, 
 		"intent", intent, 
 		NULL ) )
 		return( -1 );

From cf5cad2b3e4eeb34f39a86b66b42530216094a69 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Mon, 3 Feb 2020 16:57:10 +0000
Subject: [PATCH 13/16] make pipe read limit configurable

We had a 1gb limit on the amount of data we would read from a pipe
before giving up.

This patch adds vips_pipe_read_limit_set() and makes this limit
configurable.

See: https://github.com/libvips/libvips/issues/1540
---
 libvips/include/vips/connection.h |  2 +
 libvips/iofuncs/init.c            | 16 ++++---
 libvips/iofuncs/region.c          | 10 ++++-
 libvips/iofuncs/source.c          | 74 +++++++++++++++++++++++--------
 4 files changed, 76 insertions(+), 26 deletions(-)

diff --git a/libvips/include/vips/connection.h b/libvips/include/vips/connection.h
index 66cd5474..831cd471 100644
--- a/libvips/include/vips/connection.h
+++ b/libvips/include/vips/connection.h
@@ -89,6 +89,8 @@ GType vips_connection_get_type( void );
 const char *vips_connection_filename( VipsConnection *connection );
 const char *vips_connection_nick( VipsConnection *connection );
 
+void vips_pipe_read_limit_set( size_t limit );
+
 #define VIPS_TYPE_SOURCE (vips_source_get_type())
 #define VIPS_SOURCE( obj ) \
 	(G_TYPE_CHECK_INSTANCE_CAST( (obj), \
diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c
index 20ed972d..fc4ab5ae 100644
--- a/libvips/iofuncs/init.c
+++ b/libvips/iofuncs/init.c
@@ -127,6 +127,8 @@ int vips__leak = 0;
 GQuark vips__image_pixels_quark = 0; 
 #endif /*DEBUG_LEAK*/
 
+static gint64 vips_pipe_read_limit = 1024 * 1024 * 1024;
+
 /**
  * vips_get_argv0:
  *
@@ -428,19 +430,18 @@ vips_init( const char *argv0 )
 	g_free( locale );
 	bind_textdomain_codeset( GETTEXT_PACKAGE, "UTF-8" );
 
-	/* Deprecated, this is just for compat.
-	 */
 	if( g_getenv( "VIPS_INFO" ) || 
 		g_getenv( "IM_INFO" ) ) 
 		vips_info_set( TRUE );
-
 	if( g_getenv( "VIPS_PROFILE" ) )
 		vips_profile_set( TRUE );
-
-	/* Default various settings from env.
-	 */
 	if( g_getenv( "VIPS_TRACE" ) )
 		vips_cache_set_trace( TRUE );
+	if( g_getenv( "VIPS_PIPE_READ_LIMIT" ) ) 
+		vips_pipe_read_limit = 
+			g_ascii_strtoull( g_getenv( "VIPS_PIPE_READ_LIMIT" ),
+				NULL, 10 );
+	vips_pipe_read_limit_set( vips_pipe_read_limit );
 
 	/* Register base vips types.
 	 */
@@ -815,6 +816,9 @@ static GOptionEntry option_entries[] = {
 	{ "vips-version", 0, G_OPTION_FLAG_NO_ARG, 
 		G_OPTION_ARG_CALLBACK, (gpointer) &vips_lib_version_cb, 
 		N_( "print libvips version" ), NULL },
+	{ "vips-pipe-read-limit", 0, 0, 
+		G_OPTION_ARG_INT64, (gpointer) &vips_pipe_read_limit, 
+		N_( "read at most this many bytes from a pipe" ), NULL },
 	{ NULL }
 };
 
diff --git a/libvips/iofuncs/region.c b/libvips/iofuncs/region.c
index a59a5d9a..50b4dc83 100644
--- a/libvips/iofuncs/region.c
+++ b/libvips/iofuncs/region.c
@@ -1235,6 +1235,8 @@ vips_region_shrink_uncoded_mean( VipsRegion *from,
  * IS stable with respect to the initial arrangement of input values
  */
 #define SHRINK_TYPE_MEDIAN( TYPE ) { \
+	int ls = VIPS_REGION_LSKIP( from ); \
+	\
 	for( x = 0; x < target->width; x++ ) { \
 		TYPE *tp = (TYPE *) p; \
 		TYPE *tp1 = (TYPE *) (p + ls); \
@@ -1265,6 +1267,8 @@ vips_region_shrink_uncoded_mean( VipsRegion *from,
  * IS stable with respect to the initial arrangement of input values
  */
 #define SHRINK_TYPE_MODE( TYPE ) { \
+	int ls = VIPS_REGION_LSKIP( from ); \
+	\
 	for( x = 0; x < target->width; x++ ) { \
 		TYPE *tp = (TYPE *) p; \
 		TYPE *tp1 = (TYPE *) (p + ls); \
@@ -1289,6 +1293,8 @@ vips_region_shrink_uncoded_mean( VipsRegion *from,
 }
 
 #define SHRINK_TYPE_MAX( TYPE ) { \
+	int ls = VIPS_REGION_LSKIP( from ); \
+	\
 	for( x = 0; x < target->width; x++ ) { \
 		TYPE *tp = (TYPE *) p; \
 		TYPE *tp1 = (TYPE *) (p + ls); \
@@ -1307,6 +1313,8 @@ vips_region_shrink_uncoded_mean( VipsRegion *from,
 }
 
 #define SHRINK_TYPE_MIN( TYPE ) { \
+	int ls = VIPS_REGION_LSKIP( from ); \
+	\
 	for( x = 0; x < target->width; x++ ) { \
 		TYPE *tp = (TYPE *) p; \
 		TYPE *tp1 = (TYPE *) (p + ls); \
@@ -1327,7 +1335,6 @@ vips_region_shrink_uncoded_mean( VipsRegion *from,
 #define SHRINK_TYPE_NEAREST( TYPE ) { \
 	for( x = 0; x < target->width; x++ ) { \
 		TYPE *tp = (TYPE *) p; \
-		TYPE *tp1 = (TYPE *) (p + ls); \
 		TYPE *tq = (TYPE *) q; \
 		\
 		for( z = 0; z < nb; z++ ) \
@@ -1343,7 +1350,6 @@ static void \
 vips_region_shrink_uncoded_ ## OP( VipsRegion *from, \
 	VipsRegion *to, const VipsRect *target ) \
 { \
-	int ls = VIPS_REGION_LSKIP( from ); \
 	int ps = VIPS_IMAGE_SIZEOF_PEL( from->im ); \
 	int nb = from->im->Bands; \
  	\
diff --git a/libvips/iofuncs/source.c b/libvips/iofuncs/source.c
index 3c286007..3d0b1545 100644
--- a/libvips/iofuncs/source.c
+++ b/libvips/iofuncs/source.c
@@ -1,7 +1,10 @@
 /* A byte source/sink .. it can be a pipe, file descriptor, memory area, 
  * socket, node.js stream, etc.
  * 
- * J.Cupitt, 19/6/14
+ * 19/6/14
+ *
+ * 3/2/20
+ * 	- add vips_pipe_read_limit_set()
  */
 
 /*
@@ -84,10 +87,40 @@
 #define MODE_READWRITE BINARYIZE (O_RDWR)
 #define MODE_WRITE BINARYIZE (O_WRONLY | O_CREAT | O_TRUNC)
 
+/* -1 on a pipe isn't actually unbounded. Have a limit to prevent
+ * huge sources accidentally filling memory.
+ *
+ * This can be configured with vips_pipe_read_limit_set().
+ */
+static size_t vips__pipe_read_limit = 1024 * 1024 * 1024;
+
+/**
+ * vips_pipe_read_limit_set:
+ * @limit: maximum number of bytes to buffer from a pipe
+ *
+ * If a source does not support mmap or seek and the source is
+ * used with a loader that can only work from memory, then the data will be
+ * automatically read into memory to EOF before the loader starts. This can
+ * produce high memory use if the descriptor represents a large object.
+ *
+ * Use vips_pipe_read_limit_set() to limit the size of object that
+ * will be read in this way. The default is 1GB. 
+ *
+ * Set a value of -1 to mean no limit.
+ *
+ * See also: `--vips-pipe-read-limit` and the environment variable 
+ * `VIPS_PIPE_READ_LIMIT`.
+ */
+void
+vips_pipe_read_limit_set( size_t limit )
+{
+	vips__pipe_read_limit = limit;
+}
+
 G_DEFINE_TYPE( VipsSource, vips_source, VIPS_TYPE_CONNECTION );
 
 /* We can't test for seekability or length during _build, since the read and 
- * seek signal handlers may not have been connected yet. Instead, we test 
+ * seek signal handlers might not have been connected yet. Instead, we test 
  * when we first need to know.
  */
 static int
@@ -184,6 +217,9 @@ vips_source_sanity( VipsSource *source )
 		g_assert( source->length == -1 );
 	}
 	else {
+		/* Something like a seekable file.
+		 */
+
 		/* After we're done with the header, the sniff buffer should
 		 * be gone.
 		 */
@@ -377,6 +413,14 @@ vips_source_new_from_descriptor( int descriptor )
  *
  * Create an source attached to a file.
  *
+ * If this descriptor does not support mmap and the source is
+ * used with a loader that can only work from memory, then the data will be
+ * automatically read into memory to EOF before the loader starts. This can
+ * produce high memory use if the descriptor represents a large object.
+ *
+ * Use vips_pipe_read_limit_set() to limit the size of object that
+ * will be read in this way. The default is 1GB.
+ *
  * Returns: a new source.
  */
 VipsSource *
@@ -715,19 +759,14 @@ vips_source_read( VipsSource *source, void *buffer, size_t length )
 	return( total_read );
 }
 
-/* -1 on a pipe isn't actually unbounded. Have a limit to prevent
- * huge sources accidentally filling memory.
- *
- * 1gb. Why not.
- */
-static const int vips_pipe_read_limit = 1024 * 1024 * 1024;
-
 /* Read to a position. -1 means read to end of source. Does not change 
  * read_position.
  */
 static int
 vips_source_pipe_read_to_position( VipsSource *source, gint64 target )
 {
+	const char *nick = vips_connection_nick( VIPS_CONNECTION( source ) );
+
 	gint64 old_read_position;
 	unsigned char buffer[4096];
 
@@ -742,7 +781,7 @@ vips_source_pipe_read_to_position( VipsSource *source, gint64 target )
 		(target < 0 ||
 			(source->length != -1 && 
 			 target > source->length)) ) {
-		vips_error( vips_connection_nick( VIPS_CONNECTION( source ) ), 
+		vips_error( nick, 
 			_( "bad read to %" G_GINT64_FORMAT ), target );
 		return( -1 );
 	}
@@ -760,9 +799,9 @@ vips_source_pipe_read_to_position( VipsSource *source, gint64 target )
 			break;
 
 		if( target == -1 &&
-			source->read_position > vips_pipe_read_limit ) {
-			vips_error( vips_connection_nick( VIPS_CONNECTION( source ) ), 
-				"%s", _( "pipe too long" ) );
+			vips__pipe_read_limit != -1 &&
+			source->read_position > vips__pipe_read_limit ) {
+			vips_error( nick, "%s", _( "pipe too long" ) );
 			return( -1 );
 		}
 	}
@@ -1013,6 +1052,7 @@ vips_source_map_blob( VipsSource *source )
 gint64
 vips_source_seek( VipsSource *source, gint64 offset, int whence )
 {
+	const char *nick = vips_connection_nick( VIPS_CONNECTION( source ) );
 	VipsSourceClass *class = VIPS_SOURCE_GET_CLASS( source );
 
 	gint64 new_pos;
@@ -1039,8 +1079,7 @@ vips_source_seek( VipsSource *source, gint64 offset, int whence )
 			break;
 
 		default:
-			vips_error( vips_connection_nick( VIPS_CONNECTION( source ) ), 
-				"%s", _( "bad 'whence'" ) );
+			vips_error( nick, "%s", _( "bad 'whence'" ) );
 			return( -1 );
 		}
 	}
@@ -1066,8 +1105,7 @@ vips_source_seek( VipsSource *source, gint64 offset, int whence )
 			break;
 
 		default:
-			vips_error( vips_connection_nick( VIPS_CONNECTION( source ) ), 
-				"%s", _( "bad 'whence'" ) );
+			vips_error( nick, "%s", _( "bad 'whence'" ) );
 			return( -1 );
 		}
 	}
@@ -1081,7 +1119,7 @@ vips_source_seek( VipsSource *source, gint64 offset, int whence )
 	if( new_pos < 0 ||
 		(source->length != -1 && 
 		 new_pos > source->length) ) {
-		vips_error( vips_connection_nick( VIPS_CONNECTION( source ) ), 
+		vips_error( nick, 
 			_( "bad seek to %" G_GINT64_FORMAT ), new_pos );
                 return( -1 );
 	}

From 862e1ae214cad4aa1c38fd2a7d79e42cbfb421f2 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Mon, 3 Feb 2020 17:01:57 +0000
Subject: [PATCH 14/16] add VIPS_LEAK env var

---
 ChangeLog              | 3 +++
 libvips/iofuncs/init.c | 6 ++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 65715ee0..7c2e8aa9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -7,6 +7,9 @@
 - libtiff LOGLUV images load and save as libvips XYZ 
 - add gifload_source
 - revise vipsthumbnail flags
+- add VIPS_LEAK env var
+- add vips_pipe_read_limit_set(), --vips-pipe-read-limit,
+  VIPS_PIPE_READ_LIMIT
 
 31/1/19 started 8.9.2
 - fix a deadlock with --vips-leak [DarthSim]
diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c
index fc4ab5ae..966d95f8 100644
--- a/libvips/iofuncs/init.c
+++ b/libvips/iofuncs/init.c
@@ -435,6 +435,8 @@ vips_init( const char *argv0 )
 		vips_info_set( TRUE );
 	if( g_getenv( "VIPS_PROFILE" ) )
 		vips_profile_set( TRUE );
+	if( g_getenv( "VIPS_LEAK" ) )
+		vips_leak_set( TRUE );
 	if( g_getenv( "VIPS_TRACE" ) )
 		vips_cache_set_trace( TRUE );
 	if( g_getenv( "VIPS_PIPE_READ_LIMIT" ) ) 
@@ -1197,8 +1199,8 @@ vips_version( int flag )
  * vips_leak_set:
  * @leak: turn leak checking on or off
  *
- * Turn on or off vips leak checking. See also --vips-leak and
- * vips_add_option_entries(). 
+ * Turn on or off vips leak checking. See also --vips-leak,
+ * vips_add_option_entries() and the `VIPS_LEAK` environment variable.
  *
  * You should call this very early in your program. 
  */

From f8667994749f92fb7ca64132c0dc3a08aed23dab Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Wed, 5 Feb 2020 14:42:03 +0000
Subject: [PATCH 15/16] move pipe read limit to gint64

We had a mix of size_t and gint64. Just use gint64 everywhere.
---
 libvips/include/vips/connection.h | 2 +-
 libvips/iofuncs/init.c            | 2 +-
 libvips/iofuncs/source.c          | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libvips/include/vips/connection.h b/libvips/include/vips/connection.h
index 831cd471..d12a33a6 100644
--- a/libvips/include/vips/connection.h
+++ b/libvips/include/vips/connection.h
@@ -89,7 +89,7 @@ GType vips_connection_get_type( void );
 const char *vips_connection_filename( VipsConnection *connection );
 const char *vips_connection_nick( VipsConnection *connection );
 
-void vips_pipe_read_limit_set( size_t limit );
+void vips_pipe_read_limit_set( gint64 limit );
 
 #define VIPS_TYPE_SOURCE (vips_source_get_type())
 #define VIPS_SOURCE( obj ) \
diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c
index 966d95f8..d3a3310c 100644
--- a/libvips/iofuncs/init.c
+++ b/libvips/iofuncs/init.c
@@ -441,7 +441,7 @@ vips_init( const char *argv0 )
 		vips_cache_set_trace( TRUE );
 	if( g_getenv( "VIPS_PIPE_READ_LIMIT" ) ) 
 		vips_pipe_read_limit = 
-			g_ascii_strtoull( g_getenv( "VIPS_PIPE_READ_LIMIT" ),
+			g_ascii_strtoll( g_getenv( "VIPS_PIPE_READ_LIMIT" ),
 				NULL, 10 );
 	vips_pipe_read_limit_set( vips_pipe_read_limit );
 
diff --git a/libvips/iofuncs/source.c b/libvips/iofuncs/source.c
index 3d0b1545..af482d11 100644
--- a/libvips/iofuncs/source.c
+++ b/libvips/iofuncs/source.c
@@ -92,7 +92,7 @@
  *
  * This can be configured with vips_pipe_read_limit_set().
  */
-static size_t vips__pipe_read_limit = 1024 * 1024 * 1024;
+static gint64 vips__pipe_read_limit = 1024 * 1024 * 1024;
 
 /**
  * vips_pipe_read_limit_set:
@@ -112,7 +112,7 @@ static size_t vips__pipe_read_limit = 1024 * 1024 * 1024;
  * `VIPS_PIPE_READ_LIMIT`.
  */
 void
-vips_pipe_read_limit_set( size_t limit )
+vips_pipe_read_limit_set( gint64 limit )
 {
 	vips__pipe_read_limit = limit;
 }

From 9e6df7e0a6023a9a33ddd1af0b73b44b686c6bdb Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Fri, 7 Feb 2020 17:53:42 +0000
Subject: [PATCH 16/16] revise png comments

---
 libvips/foreign/vipspng.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/libvips/foreign/vipspng.c b/libvips/foreign/vipspng.c
index 2c12635e..ead58bf7 100644
--- a/libvips/foreign/vipspng.c
+++ b/libvips/foreign/vipspng.c
@@ -276,16 +276,18 @@ read_new( VipsSource *source, VipsImage *out, gboolean fail )
 		user_error_function, user_warning_function )) ) 
 		return( NULL );
 
-#ifdef PNG_SKIP_sRGB_CHECK_PROFILE
-	/* Prevent libpng (>=1.6.11) verifying sRGB profiles.
+	/* Prevent libpng (>=1.6.11) verifying sRGB profiles. Many PNGs have
+	 * broken profiles, but we still want to be able to open them.
 	 */
+#ifdef PNG_SKIP_sRGB_CHECK_PROFILE
 	png_set_option( read->pPng, 
 		PNG_SKIP_sRGB_CHECK_PROFILE, PNG_OPTION_ON );
 #endif /*PNG_SKIP_sRGB_CHECK_PROFILE*/
 
-#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-	/* Disable CRC checking in fuzzing mode.
+	/* Disable CRC checking in fuzzing mode. Most fuzzed images will have
+	 * bad CRCs so this check would break fuzzing.
 	 */
+#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
 	png_set_crc_action( read->pPng,
 		PNG_CRC_QUIET_USE, PNG_CRC_QUIET_USE );
 #endif /*FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION*/
@@ -813,9 +815,11 @@ write_new( VipsImage *in, VipsTarget *target )
 		return( NULL );
 	}
 
-#ifdef PNG_SKIP_sRGB_CHECK_PROFILE
-	/* Prevent libpng (>=1.6.11) verifying sRGB profiles.
+	/* Prevent libpng (>=1.6.11) verifying sRGB profiles. We are often
+	 * asked to copy images containing bad profiles, and this check would
+	 * prevent that.
 	 */
+#ifdef PNG_SKIP_sRGB_CHECK_PROFILE
 	png_set_option( write->pPng, 
 		PNG_SKIP_sRGB_CHECK_PROFILE, PNG_OPTION_ON );
 #endif /*PNG_SKIP_sRGB_CHECK_PROFILE*/