Merge branch '8.7'

ChangeLog

@@ -1,3 +1,4 @@
+<<<<<<< HEAD
 21/9/18 started 8.8.0
 - much faster smartcrop [lovell]
 - add low/high to smartcrop [jcupitt]
@@ -32,6 +33,9 @@
 - move vips_image_set_kill() and iskilled() to the public API
 - remove old c++ and python interfaces
 
+31/3/19 started 8.7.5
+- better buffer sizing in tiff reader [omira-sch]
+
 4/1/19 started 8.7.4
 - magickload with magick6 API did not chain exceptions correctly causing a
   memory leak under some conditions [kleisauke]

libvips/foreign/tiff2vips.c

@@ -183,6 +183,8 @@
  * 	- check for non-byte-multiple bits_per_sample [HongxuChen]
  * 16/8/18
  * 	- shut down the input file as soon as we can [kleisauke]
+ * 28/3/19 omira-sch
+ * 	- better buffer sizing 
  */
 
 /*
@@ -253,20 +255,29 @@ typedef struct _RtiffHeader {
 	 */
 	gboolean tiled;
 
-	/* Fields for tiled images.
+	/* Fields for tiled images, as returned by libtiff.
 	 */
 	uint32 tile_width;
 	uint32 tile_height;		
+	tsize_t tile_size;
 
-	/* Fields for strip images.
+	/* Fields for strip images, as returned by libtiff.
-	 *
-	 * If read_scanlinewise is TRUE, the strips are too large to read in a
-	 * single lump and we need to use the scanline API.
 	 */
 	uint32 rows_per_strip;
 	tsize_t strip_size;
 	int number_of_strips;
+
+	/* If read_scanlinewise is TRUE, the strips are too large to read in a
+	 * single lump and we will use the scanline API.
+	 */
 	gboolean read_scanlinewise;
+
+	/* Strip read geometry. These are the read params as they will be used
+	 * to read, with either TIFFReadScanline() or TIFFReadEncodedStrip(),
+	 * not as they are stored in the file.
+	 */
+	uint32 read_rows_per_strip;
+	tsize_t read_strip_size;
 } RtiffHeader;
 
 /* Scanline-type process function.
@@ -1262,7 +1273,12 @@ rtiff_parse_copy( Rtiff *rtiff, VipsImage *out )
 
 	rtiff->sfn = rtiff_memcpy_line;
 	rtiff->client = out;
-	rtiff->memcpy = TRUE;
+
+	/* We expand YCBCR images to RGB using JPEGCOLORMODE_RGB, and this
+	 * means we need a slightly larger read buffer for the edge pixels. In
+	 * turn, this means we can't just memcpy to libvips regions.
+	 */
+	rtiff->memcpy = photometric_interpretation != PHOTOMETRIC_YCBCR;
 
 	return( 0 );
 }
@@ -1297,14 +1313,6 @@ rtiff_pick_reader( Rtiff *rtiff )
 	if( photometric_interpretation == PHOTOMETRIC_PALETTE ) 
 		return( rtiff_parse_palette ); 
 
-	if( photometric_interpretation == PHOTOMETRIC_YCBCR ) { 
-		/* Sometimes JPEG in TIFF images are tagged as YCBCR. Ask
-		 * libtiff to convert to RGB for us.
-		 */
-		TIFFSetField( rtiff->tiff, 
-			TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB );
-	}
-
 	return( rtiff_parse_copy );
 }
 
@@ -1317,6 +1325,10 @@ rtiff_set_header( Rtiff *rtiff, VipsImage *out )
 	uint32 data_length;
 	void *data;
 
+	/* Request YCbCr expansion.
+	 */
+	TIFFSetField( rtiff->tiff, TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB );
+
 	out->Xsize = rtiff->header.width;
 	out->Ysize = rtiff->header.height * rtiff->n;
 
@@ -1408,19 +1420,6 @@ rtiff_set_header( Rtiff *rtiff, VipsImage *out )
 	return( 0 );
 }
 
-/* The size of the buffer written by TIFFReadTile(). We can't use 
- * TIFFTileSize() since that ignores the setting of TIFFTAG_JPEGCOLORMODE. If
- * this pseudo tag has been set and the tile is encoded with YCbCr, the tile
- * is returned with chrominance upsampled. 
- *
- * This seems not to happen for old-style jpeg-compressed tiles. 
- */
-static size_t
-rtiff_tile_size( Rtiff *rtiff )
-{
-	return( TIFFTileRowSize( rtiff->tiff ) * rtiff->header.tile_height );
-}
-
 /* Allocate a tile buffer. Have one of these for each thread so we can unpack
  * to vips in parallel.
  */
@@ -1428,11 +1427,9 @@ static void *
 rtiff_seq_start( VipsImage *out, void *a, void *b )
 {
 	Rtiff *rtiff = (Rtiff *) a;
-	tsize_t size;
 	tdata_t *buf;
 
-	size = rtiff_tile_size( rtiff );
+	if( !(buf = vips_malloc( NULL, rtiff->header.tile_size )) )
-	if( !(buf = vips_malloc( NULL, size )) )
 		return( NULL );
 
 	return( (void *) buf );
@@ -1501,7 +1498,7 @@ rtiff_fill_region( VipsRegion *out,
 
 	/* Sizeof a line of bytes in the TIFF tile.
 	 */
-	int tls = rtiff_tile_size( rtiff ) / tile_height;
+	int tls = rtiff->header.tile_size / tile_height;
 
 	/* Sizeof a pel in the TIFF file. This won't work for formats which
 	 * are <1 byte per pel, like onebit :-( Fortunately, it's only used
@@ -1714,12 +1711,10 @@ rtiff_read_tilewise( Rtiff *rtiff, VipsImage *out )
 	 * match the tifftile size.
 	 */
 	if( rtiff->memcpy ) {
-		size_t vips_tile_size;
+		size_t vips_tile_size = VIPS_IMAGE_SIZEOF_PEL( t[0] ) * 
-
-		vips_tile_size = VIPS_IMAGE_SIZEOF_PEL( t[0] ) * 
 			tile_width * tile_height; 
 
-		if( rtiff_tile_size( rtiff ) != vips_tile_size ) { 
+		if( rtiff->header.tile_size != vips_tile_size ) { 
 			vips_error( "tiff2vips", 
 				"%s", _( "unsupported tiff image type" ) );
 			return( -1 );
@@ -1761,15 +1756,15 @@ static int
 rtiff_strip_read_interleaved( Rtiff *rtiff, tstrip_t strip, tdata_t buf )
 {
 	int samples_per_pixel = rtiff->header.samples_per_pixel;
-	int rows_per_strip = rtiff->header.rows_per_strip;
+	int read_rows_per_strip = rtiff->header.read_rows_per_strip;
 	int bits_per_sample = rtiff->header.bits_per_sample;
-	int strip_y = strip * rows_per_strip;
+	int strip_y = strip * read_rows_per_strip;
 
 	if( rtiff->header.separate ) {
 		int page_width = rtiff->header.width;
 		int page_height = rtiff->header.height;
-		int strips_per_plane = 1 + (page_height - 1) / rows_per_strip;
+		int strips_per_plane = 1 + (page_height - 1) / read_rows_per_strip;
-		int strip_height = VIPS_MIN( rows_per_strip, 
+		int strip_height = VIPS_MIN( read_rows_per_strip, 
 			page_height - strip_y ); 
 		int pels_per_strip = page_width * strip_height;
 		int bytes_per_sample = bits_per_sample >> 3; 
@@ -1809,7 +1804,7 @@ rtiff_stripwise_generate( VipsRegion *or,
 	void *seq, void *a, void *b, gboolean *stop )
 {
 	Rtiff *rtiff = (Rtiff *) a;
-	int rows_per_strip = rtiff->header.rows_per_strip;
+	int read_rows_per_strip = rtiff->header.read_rows_per_strip;
 	int page_height = rtiff->header.height;
 	tsize_t scanline_size = TIFFScanlineSize( rtiff->tiff );
         VipsRect *r = &or->valid;
@@ -1837,7 +1832,7 @@ rtiff_stripwise_generate( VipsRegion *or,
 	 * strip in the image.
 	 */
 	g_assert( r->height == 
-		VIPS_MIN( rows_per_strip, or->im->Ysize - r->top ) ); 
+		VIPS_MIN( read_rows_per_strip, or->im->Ysize - r->top ) ); 
 
 	/* And check that y_pos is correct. It should be, since we are inside
 	 * a vips_sequential().
@@ -1861,7 +1856,7 @@ rtiff_stripwise_generate( VipsRegion *or,
 
 		/* Strip number.
 		 */
-		tstrip_t strip_no = y_page / rows_per_strip;
+		tstrip_t strip_no = y_page / read_rows_per_strip;
 
 		VipsRect image, page, strip, hit;
 
@@ -1879,9 +1874,9 @@ rtiff_stripwise_generate( VipsRegion *or,
 		page.height = page_height;
 
 		strip.left = 0;
-		strip.top = page.top + strip_no * rows_per_strip;
+		strip.top = page.top + strip_no * read_rows_per_strip;
 		strip.width = rtiff->out->Xsize;
-		strip.height = rows_per_strip;
+		strip.height = read_rows_per_strip;
 
 		/* Clip strip against page and image ... the final strip will 
 		 * be smaller.
@@ -1991,6 +1986,10 @@ rtiff_read_stripwise( Rtiff *rtiff, VipsImage *out )
 		rtiff->header.strip_size );
 	printf( "rtiff_read_stripwise: header.number_of_strips = %d\n", 
 		rtiff->header.number_of_strips );
+	printf( "rtiff_read_stripwise: header.read_rows_per_strip = %u\n", 
+		rtiff->header.read_rows_per_strip );
+	printf( "rtiff_read_stripwise: header.read_strip_size = %zd\n", 
+		rtiff->header.read_strip_size );
 #endif /*DEBUG*/
 
 	/* Double check: in memcpy mode, the vips linesize should exactly
@@ -2022,7 +2021,7 @@ rtiff_read_stripwise( Rtiff *rtiff, VipsImage *out )
 	 */
 	if( rtiff->header.separate ) {
 		if( !(rtiff->plane_buf = vips_malloc( VIPS_OBJECT( out ), 
-			rtiff->header.strip_size )) ) 
+			rtiff->header.read_strip_size )) ) 
 			return( -1 );
 	}
 
@@ -2039,14 +2038,13 @@ rtiff_read_stripwise( Rtiff *rtiff, VipsImage *out )
 		rtiff->n > 1 ) { 
 		tsize_t size;
 
-		size = rtiff->header.strip_size;
+		size = rtiff->header.read_strip_size;
 		if( rtiff->header.separate )
 			size *= rtiff->header.samples_per_pixel;
 
 		if( !(rtiff->contig_buf = 
 			vips_malloc( VIPS_OBJECT( out ), size )) ) 
 			return( -1 );
-
 	}
 
 	/* rows_per_strip can be very large if this is a separate plane image,
@@ -2057,7 +2055,7 @@ rtiff_read_stripwise( Rtiff *rtiff, VipsImage *out )
 			NULL, rtiff_stripwise_generate, NULL, 
 			rtiff, NULL ) ||
 		vips_sequential( t[0], &t[1], 
-			"tile_height", rtiff->header.rows_per_strip,
+			"tile_height", rtiff->header.read_rows_per_strip,
 			NULL ) ||
 		rtiff_autorotate( rtiff, t[1], &t[2] ) ||
 		rtiff_unpremultiply( rtiff, t[2], &t[3] ) ||
@@ -2075,6 +2073,12 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header )
 	uint16 extra_samples_count;
 	uint16 *extra_samples_types;
 
+	/* We want to always expand subsampled YCBCR images to full RGB. We
+	 * need to set this pseudo tag early, since it affects the value you
+	 * get from TIFFStripSize() etc. 
+	 */
+	TIFFSetField( rtiff->tiff, TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB );
+
 	if( !tfget32( rtiff->tiff, TIFFTAG_IMAGEWIDTH, &header->width ) ||
 		!tfget32( rtiff->tiff, TIFFTAG_IMAGELENGTH, &header->height ) ||
 		!tfget16( rtiff->tiff, 
@@ -2088,11 +2092,11 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header )
 
 	/* Arbitrary sanity-checking limits.
 	 */
-	if( header->width <= 0 || 
+	if( header->width <= 0 ||
-		header->width > VIPS_MAX_COORD || 
+		header->width > VIPS_MAX_COORD ||
-		header->height <= 0 || 
+		header->height <= 0 ||
 		header->height > VIPS_MAX_COORD ) {
-		vips_error( "tiff2vips", 
+		vips_error( "tiff2vips",
 			"%s", _( "width/height out of range" ) );
 		return( -1 );
 	}
@@ -2130,11 +2134,26 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header )
 				TIFFTAG_TILELENGTH, &header->tile_height ) )
 			return( -1 );
 
+		/* Arbitrary sanity-checking limits.
+		 */
+		if( header->tile_width <= 0 ||
+			header->tile_width > 10000 ||
+			header->tile_height <= 0 ||
+			header->tile_height > 10000 ) {
+			vips_error( "tiff2vips",
+				"%s", _( "tile size out of range" ) );
+			return( -1 );
+		}
+
+		header->tile_size = TIFFTileSize( rtiff->tiff );
+
 		/* Stop some compiler warnings.
 		 */
 		header->rows_per_strip = 0; 
 		header->strip_size = 0; 
 		header->number_of_strips = 0; 
+		header->read_rows_per_strip = 0;
+		header->read_strip_size = 0;
 	}
 	else {
 		if( !tfget32( rtiff->tiff, 
@@ -2142,15 +2161,6 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header )
 			return( -1 );
 		header->strip_size = TIFFStripSize( rtiff->tiff );
 		header->number_of_strips = TIFFNumberOfStrips( rtiff->tiff );
-		header->read_scanlinewise = FALSE;
-
-		/* rows_per_strip can be 2 ** 32 - 1, meaning the whole image. 
-		 * Clip this down to height to avoid confusing vips. 
-		 *
-		 * And it musn't be zero.
-		 */
-		header->rows_per_strip = 
-			VIPS_CLIP( 1, header->rows_per_strip, header->height );
 
 		/* libtiff has two strip-wise readers. TIFFReadEncodedStrip()
 		 * decompresses an entire strip to memory. It's fast, but it
@@ -2166,16 +2176,32 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header )
 		 */
 		if( header->rows_per_strip > 128 &&
 			!header->separate ) {
-			header->rows_per_strip = 1;
-			header->strip_size = TIFFScanlineSize( rtiff->tiff );
-			header->number_of_strips = header->height;
 			header->read_scanlinewise = TRUE;
+			header->read_rows_per_strip = 1;
+			header->read_strip_size = 
+				TIFFScanlineSize( rtiff->tiff );
+		}
+		else {
+			header->read_scanlinewise = FALSE;
+
+			/* rows_per_strip can be 2 ** 32 - 1, meaning the 
+			 * whole image. Clip this down to height to avoid 
+			 * confusing vips. 
+			 *
+			 * And it musn't be zero.
+			 */
+			header->read_rows_per_strip = 
+				VIPS_CLIP( 1, 
+					header->rows_per_strip, 
+					header->height );
+			header->read_strip_size = header->strip_size;
 		}
 
 		/* Stop some compiler warnings.
 		 */
 		header->tile_width = 0;
 		header->tile_height = 0;
+		header->tile_size = 0;
 	}
 
 	TIFFGetFieldDefaulted( rtiff->tiff, TIFFTAG_EXTRASAMPLES,
@@ -2207,8 +2233,8 @@ rtiff_header_equal( RtiffHeader *h1, RtiffHeader *h2 )
 			return( 0 );
 	}
 	else {
-		if( h1->rows_per_strip != h2->rows_per_strip ||
+		if( h1->read_rows_per_strip != h2->read_rows_per_strip ||
-			h1->strip_size != h2->strip_size ||
+			h1->read_strip_size != h2->read_strip_size ||
 			h1->number_of_strips != h2->number_of_strips )
 			return( 0 );
 	}