tiff reader supports new sequential mode

good speedup for large tiffs

$ time ~/vips-7.26/bin/vips-7.26 vips im_copy wtc.tif x.v
real	0m12.728s
user	0m0.220s
sys	0m1.032s

$ time vips copy wtc.tif[sequential] x.v
real	0m4.328s
user	0m0.584s
sys	0m0.764s

new one was compiled with DEBUG, hence (partly) larger user time

ChangeLog

@@ -7,6 +7,7 @@
 - better im_shrink()
 - added vips_sequential()
 - new vips_sink_memory() keeps read ordering
+- tiff reader supports sequential read
 
 20/8/11 started 7.27.0
 - version bump for new dev cycle

TODO

@@ -1,8 +1,10 @@
 
-- add a sequential mode to all readers
+- add a sequential mode to jpeg reader
 
   tiff, jpg are the obvious ones
 
+- is the tif reader deadlocking sometimes?
+
 
 
 

libvips/foreign/tiff2vips.c

@@ -125,6 +125,9 @@
  * 5/12/11
  * 	- make into a simple function call ready to be wrapped as a new-style
  * 	  VipsForeign class
+ * 18/2/12
+ * 	- switch to sequential read
+ * 	- remove the lock ... tilecache does this for us
  */
 
 /*
@@ -209,10 +212,10 @@ typedef struct {
 	/* Geometry.
 	 */
 	uint32 twidth, theight;		/* Tile size */
-
+	uint32 rows_per_strip;
-	/* Only need one of these, since we mutex around TIFF*().
+	tsize_t scanline_size;
-	 */
+	tsize_t strip_size;
-	GMutex *tlock;			/* Lock for TIFF*() calls */
+	int number_of_strips;
 } ReadTiff;
 
 /* Handle TIFF errors here. Shared with vips2tiff.c. These can be called from
@@ -1131,14 +1134,11 @@ tiff_fill_region_aligned( VipsRegion *out, void *seq, void *a, void *b )
 
 	/* Read that tile directly into the vips tile.
 	 */
-	g_mutex_lock( rtiff->tlock );
 	if( TIFFReadTile( rtiff->tiff, 
 		VIPS_REGION_ADDR( out, r->left, r->top ), 
 		r->left, r->top, 0, 0 ) < 0 ) {
-		g_mutex_unlock( rtiff->tlock );
 		return( -1 );
 	}
-	g_mutex_unlock( rtiff->tlock );
 
 	return( 0 );
 }
@@ -1189,12 +1189,8 @@ tiff_fill_region( VipsRegion *out, void *seq, void *a, void *b, gboolean *stop )
 
 			/* Read that tile.
 			 */
-			g_mutex_lock( rtiff->tlock );
+			if( TIFFReadTile( rtiff->tiff, buf, x, y, 0, 0 ) < 0 ) 
-			if( TIFFReadTile( rtiff->tiff, buf, x, y, 0, 0 ) < 0 ) {
-				g_mutex_unlock( rtiff->tlock );
 				return( -1 );
-			}
-			g_mutex_unlock( rtiff->tlock );
 
 			/* The tile we read.
 			 */
@@ -1287,80 +1283,137 @@ read_tilewise( ReadTiff *rtiff, VipsImage *out )
 	return( 0 );
 }
 
-/* Stripwise reading - we assume strips are written top-to-bottom. Not sure if
- * this is always correct.
- */
 static int
-read_stripwise( ReadTiff *rtiff, VipsImage *out )
+tiff2vips_stripwise_generate( VipsRegion *or, 
+	void *seq, void *a, void *b, gboolean *stop )
 {
-	uint32 rows_per_strip;
+	ReadTiff *rtiff = (ReadTiff *) a;
-	tsize_t scanline_size;
+	tdata_t tbuf = (tdata_t) b; 
-	tsize_t strip_size;
+        VipsRect *r = &or->valid;
-	int number_of_strips;
 
-	VipsPel *vbuf;
+	tdata_t dst;
-	tdata_t tbuf;
 	tstrip_t strip;
 	tsize_t length;
-	int y;
-	int i;
-	VipsPel *p;
 
 #ifdef DEBUG
-	printf( "tiff2vips: read_stripwise\n" );
+	printf( "tiff2vips: read_stripwise_generate: top = %d, height = %d\n",
+		r->top, r->height );
 #endif /*DEBUG*/
 
-	if( parse_header( rtiff, out ) )
+	/* We're inside a tilecache where tiles are the full image width, so
-		return( -1 );
+	 * this should always be true.
-
-	if( !tfget32( rtiff->tiff, TIFFTAG_ROWSPERSTRIP, &rows_per_strip ) )
-		return( -1 );
-	scanline_size = TIFFScanlineSize( rtiff->tiff );
-	strip_size = TIFFStripSize( rtiff->tiff );
-	number_of_strips = TIFFNumberOfStrips( rtiff->tiff );
-
-#ifdef DEBUG
-	printf( "read_stripwise: rows_per_strip = %u\n", rows_per_strip );
-	printf( "read_stripwise: scanline_size = %d\n", scanline_size );
-	printf( "read_stripwise: strip_size = %d\n", strip_size );
-	printf( "read_stripwise: number_of_strips = %d\n", number_of_strips );
-#endif /*DEBUG*/
-
-	/* Make buffers.
 	 */
-	if( !(vbuf = VIPS_ARRAY( out, 
+	g_assert( r->left == 0 );
-			VIPS_IMAGE_SIZEOF_LINE( out ), VipsPel )) ||
+	g_assert( r->width == or->im->Xsize );
-		!(tbuf = vips_malloc( VIPS_OBJECT( out ), strip_size )) ) 
+	g_assert( VIPS_RECT_BOTTOM( r ) <= or->im->Ysize );
-		return( -1 );
 
-	for( strip = 0, y = 0; 
+	/* Tiles should always be on a strip boundary and exactly one strip
-		strip < number_of_strips; 
+	 * high.
-		strip += 1, y += rows_per_strip ) {
+	 */
-		length = TIFFReadEncodedStrip( rtiff->tiff, 
+	g_assert( r->top % rtiff->rows_per_strip == 0 );
-			strip, tbuf, (tsize_t) -1 );
+	g_assert( r->height == 
+		VIPS_MIN( rtiff->rows_per_strip, or->im->Ysize - r->top ) );
+
+	/* Read directly into the image if we can. Otherwise, we must read 
+	 * to a temp buffer then unpack into the image.
+	 */
+	if( rtiff->memcpy ) 
+		dst = VIPS_REGION_ADDR( or, 0, r->top );
+	else
+		dst = tbuf;
+
+	strip = r->top / rtiff->rows_per_strip;
+
+	length = TIFFReadEncodedStrip( rtiff->tiff, strip, dst, (tsize_t) -1 );
 	if( length == -1 ) {
 		vips_error( "tiff2vips", "%s", _( "read error" ) );
 		return( -1 );
 	}
 
-		for( p = tbuf, i = 0; 
+	/* If necessary, unpack to destination.
-			i < rows_per_strip && y + i < out->Ysize; 
-			i += 1, p += scanline_size ) {
-			/* If we need to unpack the data, go via a buffer.
-			 * Otherwise we can write directly from the strip.
 	 */
-			if( rtiff->memcpy ) {
+	if( !rtiff->memcpy ) {
-				if( vips_image_write_line( out, y + i, p ) ) 
+		int y;
+
+		for( y = 0; y < r->height; y++ ) { 
+			VipsPel *p = tbuf + y * rtiff->scanline_size;
+			VipsPel *q = VIPS_REGION_ADDR( or, 0, r->top );
+
+			rtiff->sfn( q, p, 
+				or->im->Xsize, rtiff->client );
+		}
+	}
+
+	return( 0 );
+}
+
+/* Stripwise reading.
+ *
+ * We could potentially read strips in any order, but this would give
+ * catastrophic performance for opertions like 90 degress rotate on a 
+ * large image. Only offer sequential read.
+ */
+static int
+read_stripwise( ReadTiff *rtiff, VipsImage *out )
+{
+	VipsImage **t = (VipsImage **) 
+		vips_object_local_array( VIPS_OBJECT( out ), 3 );
+
+	tdata_t tbuf;
+
+#ifdef DEBUG
+	printf( "tiff2vips: read_stripwise\n" );
+#endif /*DEBUG*/
+
+	t[0] = vips_image_new();
+
+	if( parse_header( rtiff, t[0] ) )
 		return( -1 );
-			}
+
-			else {
+        vips_demand_hint( t[0], 
-				rtiff->sfn( vbuf, p, 
+		VIPS_DEMAND_STYLE_FATSTRIP, NULL );
-					out->Xsize, rtiff->client );
+
-				if( vips_image_write_line( out, y + i, vbuf ) ) 
+	if( !tfget32( rtiff->tiff, 
+		TIFFTAG_ROWSPERSTRIP, &rtiff->rows_per_strip ) )
+		return( -1 );
+	rtiff->scanline_size = TIFFScanlineSize( rtiff->tiff );
+	rtiff->strip_size = TIFFStripSize( rtiff->tiff );
+	rtiff->number_of_strips = TIFFNumberOfStrips( rtiff->tiff );
+
+#ifdef DEBUG
+	printf( "read_stripwise: rows_per_strip = %u\n", 
+		rtiff->rows_per_strip );
+	printf( "read_stripwise: scanline_size = %d\n", 
+		rtiff->scanline_size );
+	printf( "read_stripwise: strip_size = %d\n", 
+		rtiff->strip_size );
+	printf( "read_stripwise: number_of_strips = %d\n", 
+		rtiff->number_of_strips );
+#endif /*DEBUG*/
+
+	/* Read each strip to this. We only need one for stripwise_generate)(_
+	 * since it's single-threaded by tilecache().
+	 */
+	if( !(tbuf = vips_malloc( VIPS_OBJECT( out ), rtiff->strip_size )) ) 
+		return( -1 );
+
+	/* Each tile in the cache is the size of a strip. Cache enough tiles
+	 * for two lines of SMALLTILE.
+	 */
+	if( 
+		vips_image_generate( t[0], 
+			NULL, tiff2vips_stripwise_generate, NULL, 
+			rtiff, tbuf ) ||
+		vips_sequential( t[0], &t[1], NULL ) ||
+		vips_tilecache( t[1], &t[2], 
+			"tile_width", t[0]->Xsize, 
+			"tile_height", rtiff->rows_per_strip,
+			"max_tiles", 
+				1 + (2 * VIPS__TILE_HEIGHT) / 
+					rtiff->rows_per_strip,
+			NULL ) ||
+		vips_image_write( t[2], out ) )
 		return( -1 );
-			}
-		}
-	}
 
 	return( 0 );
 }
@@ -1369,7 +1422,6 @@ static void
 readtiff_destroy( VipsObject *object, ReadTiff *rtiff )
 {
 	VIPS_FREEF( TIFFClose, rtiff->tiff );
-	VIPS_FREEF( g_mutex_free, rtiff->tlock );
 }
 
 static ReadTiff *
@@ -1388,7 +1440,6 @@ readtiff_new( const char *filename, VipsImage *out, int page )
 	rtiff->memcpy = FALSE;
 	rtiff->twidth = 0;
 	rtiff->theight = 0;
-	rtiff->tlock = g_mutex_new();
 	g_signal_connect( out, "close", 
 		G_CALLBACK( readtiff_destroy ), rtiff ); 
 

libvips/foreign/tiffload.c

@@ -77,6 +77,8 @@ vips_foreign_load_tiff_get_flags_filename( const char *filename )
 	flags = 0;
 	if( vips__istifftiled( filename ) ) 
 		flags |= VIPS_FOREIGN_PARTIAL;
+	else
+		flags |= VIPS_FOREIGN_SEQUENTIAL;
 
 	return( flags );
 }

libvips/foreign/vipspng.c

@@ -287,7 +287,7 @@ png2vips_header( Read *read, VipsImage *out )
 	/* We're always supposed to set dhint.
 	 */
         vips_demand_hint( out, 
-		VIPS_DEMAND_STYLE_THINSTRIP, NULL );
+		VIPS_DEMAND_STYLE_FATSTRIP, NULL );
 
 	return( 0 );
 }

libvips/iofuncs/sinkmemory.c

@@ -302,7 +302,7 @@ sink_memory_init( SinkMemory *memory, VipsImage *image )
 }
 
 /**
- * vips_sink_memory2:
+ * vips_sink_memory:
  * @im: generate this image to memory
  *
  * Loops over an image, generating it to a memory buffer attached to the
@@ -313,7 +313,7 @@ sink_memory_init( SinkMemory *memory, VipsImage *image )
  * Returns: 0 on success, or -1 on error.
  */
 int
-vips_sink_memory2( VipsImage *image )
+vips_sink_memory( VipsImage *image )
 {
 	SinkMemory memory;
 	int result;