sequential support for the jpg reader

ChangeLog

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

TODO

@@ -1,11 +1,25 @@
+- how do we maintain ordering?
 
-- add a sequential mode to jpeg reader
+  with very large numbers of thread seq is bound to break
 
-  tiff, jpg are the obvious ones
+  what guarantees sequentiality? tiles must be big enough that every thread
+  fits into one tile, and we must have exactly two tiles
+
+  so ... tile height must scale with nthreads and image width
+
+  is this nlines in get tile geometry?
+
+
+
+- interlaced jpg needs massive memory, we should have two jpg read modes, like
+  png
+
+
+
+- add ore sequential mode readers
 
 	$ grep -l write_line *.c
 	csv.c
-	jpeg2vips.c
 	matlab.c
 	openexr2vips.c
 	ppm.c

libvips/conversion/sequential.c

@@ -72,6 +72,8 @@ vips_sequential_generate( VipsRegion *or,
         VipsRect *r = &or->valid;
 	VipsRegion *ir = (VipsRegion *) seq;
 
+	VIPS_DEBUG_MSG( "vips_sequential_generate %d\n", r->top );
+
 	/* The y pos of the request must be the same as our current file
 	 * position.
 	 */

libvips/conversion/tilecache.c

@@ -320,6 +320,10 @@ vips_tile_cache_gen( VipsRegion *or,
 
 	g_mutex_lock( cache->lock );
 
+	VIPS_DEBUG_MSG( "vips_tile_cache_gen: "
+		"left = %d, top = %d, width = %d, height = %d\n",
+		r->left, r->top, r->width, r->height );
+
 	for( y = ys; y < VIPS_RECT_BOTTOM( r ); y += th )
 		for( x = xs; x < VIPS_RECT_RIGHT( r ); x += tw ) {
 			Tile *tile;

libvips/foreign/jpeg2vips.c

@@ -44,7 +44,7 @@
  * 9/1/12
  * 	- read jfif resolution as well as exif
  * 19/2/12
- * 	- keep a read struct about, ready to go lazy read
+ * 	- switch to lazy reading
  */
 
 /*
@@ -141,6 +141,10 @@ typedef struct _ReadJpeg {
 	struct jpeg_decompress_struct cinfo;
         ErrorManager eman;
 	gboolean invert_pels;
+
+	/* Set if we need to finish the decompress.
+	 */
+	gboolean decompressing;
 } ReadJpeg;
 
 static int
@@ -150,6 +154,9 @@ readjpeg_free( ReadJpeg *jpeg )
 
 	result = 0;
 
+	if( setjmp( jpeg->eman.jmp ) ) 
+		return( -1 );
+
 	if( jpeg->eman.pub.num_warnings != 0 ) {
 		if( jpeg->fail ) {
 			vips_error( "VipsJpeg", "%s", vips_error_buffer() );
@@ -167,6 +174,11 @@ readjpeg_free( ReadJpeg *jpeg )
 		jpeg->eman.pub.num_warnings = 0;
 	}
 
+	if( jpeg->decompressing ) {
+		jpeg_finish_decompress( &jpeg->cinfo );
+		jpeg->decompressing = FALSE;
+	}
+
 	VIPS_FREEF( fclose, jpeg->fp );
 	VIPS_FREE( jpeg->filename );
 	jpeg->eman.fp = NULL;
@@ -193,6 +205,7 @@ readjpeg_new( VipsImage *out, int shrink, gboolean fail )
 	jpeg->fail = fail;
 	jpeg->fp = NULL;
 	jpeg->filename = NULL;
+	jpeg->decompressing = FALSE;
 
         jpeg->cinfo.err = jpeg_std_error( &jpeg->eman.pub );
 	jpeg->eman.pub.error_exit = vips__new_error_exit;
@@ -611,7 +624,7 @@ read_xmp( VipsImage *im, void *data, size_t data_length )
 	}
 
 #ifdef DEBUG
-	printf( "read_xmp: attaching %d bytes of XMP\n", data_length );
+	printf( "read_xmp: attaching %zd bytes of XMP\n", data_length );
 #endif /*DEBUG*/
 
 	/* Always attach a copy of the unparsed exif data.
@@ -634,7 +647,7 @@ read_xmp( VipsImage *im, void *data, size_t data_length )
  * do 255-pel.
  */
 static int
-read_jpeg_header( ReadJpeg *jpeg )
+read_jpeg_header( ReadJpeg *jpeg, VipsImage *out )
 {
 	struct jpeg_decompress_struct *cinfo = &jpeg->cinfo;
 
@@ -714,21 +727,19 @@ read_jpeg_header( ReadJpeg *jpeg )
 
 	/* Set VIPS header.
 	 */
-	vips_image_init_fields( jpeg->out,
+	vips_image_init_fields( out,
 		cinfo->output_width, cinfo->output_height,
 		cinfo->output_components,
 		VIPS_FORMAT_UCHAR, VIPS_CODING_NONE,
 		interpretation,
 		xres, yres );
 
-	/* Best for us, probably.
+	vips_demand_hint( out, VIPS_DEMAND_STYLE_SMALLTILE, NULL );
-	 */
-	vips_demand_hint( jpeg->out, VIPS_DEMAND_STYLE_FATSTRIP, NULL );
 
 	/* Interlaced jpegs need lots of memory to read, so our caller needs
 	 * to know.
 	 */
-	(void) vips_image_set_int( jpeg->out, "jpeg-multiscan", 
+	(void) vips_image_set_int( out, "jpeg-multiscan", 
 		jpeg_has_multiple_scans( cinfo ) );
 
 	/* Look for EXIF and ICC profile.
@@ -754,14 +765,12 @@ read_jpeg_header( ReadJpeg *jpeg )
 			 */
 			if( p->data_length > 4 &&
 				vips_isprefix( "Exif", (char *) p->data ) &&
-				read_exif( jpeg->out, 
+				read_exif( out, p->data, p->data_length ) )
-					p->data, p->data_length ) )
 				return( -1 );
 
 			if( p->data_length > 4 &&
 				vips_isprefix( "http", (char *) p->data ) &&
-				read_xmp( jpeg->out, 
+				read_xmp( out, p->data, p->data_length ) )
-					p->data, p->data_length ) )
 				return( -1 );
 
 			break;
@@ -801,7 +810,7 @@ read_jpeg_header( ReadJpeg *jpeg )
 		int p;
 
 #ifdef DEBUG
-		printf( "read_jpeg_header: assembled %d byte ICC profile\n",
+		printf( "read_jpeg_header: assembled %zd byte ICC profile\n",
 			data_length );
 #endif /*DEBUG*/
 
@@ -814,56 +823,109 @@ read_jpeg_header( ReadJpeg *jpeg )
 			p += app2_data_length[i];
 		}
 
-		vips_image_set_blob( jpeg->out, VIPS_META_ICC_NAME, 
+		vips_image_set_blob( out, VIPS_META_ICC_NAME, 
 			(VipsCallbackFn) vips_free, data, data_length );
 	}
 
 	return( 0 );
 }
 
+static int
+read_jpeg_generate( VipsRegion *or, 
+	void *seq, void *a, void *b, gboolean *stop )
+{
+        VipsRect *r = &or->valid;
+	ReadJpeg *jpeg = (ReadJpeg *) a;
+	struct jpeg_decompress_struct *cinfo = &jpeg->cinfo;
+
+	JSAMPROW row_pointer[VIPS__TILE_HEIGHT];
+	int y;
+
+#ifdef DEBUG
+	printf( "read_jpeg_generate: line %d, %d rows\n", 
+		r->top, r->height );
+#endif /*DEBUG*/
+
+	/* We're inside a tilecache where tiles are the full image width, so
+	 * this should always be true.
+	 */
+	g_assert( r->left == 0 );
+	g_assert( r->width == or->im->Xsize );
+	g_assert( VIPS_RECT_BOTTOM( r ) <= or->im->Ysize );
+
+	/* Tiles should always be on a 8-pixel boundary and exactly one block
+	 * high.
+	 */
+	g_assert( r->top % VIPS__TILE_HEIGHT == 0 );
+	g_assert( r->height == 
+		VIPS_MIN( VIPS__TILE_HEIGHT, or->im->Ysize - r->top ) );
+
+	/* Here for longjmp() from vips__new_error_exit().
+	 */
+	if( setjmp( jpeg->eman.jmp ) ) 
+		return( -1 );
+
+	for( y = 0; y < r->height; y++ ) {
+		row_pointer[y] = (JSAMPLE *) 
+			VIPS_REGION_ADDR( or, 0, r->top + y );
+
+		/* No faster to read in groups and you have to loop
+		 * anyway.
+		 */
+		jpeg_read_scanlines( cinfo, &row_pointer[y], 1 );
+	}
+
+	if( jpeg->invert_pels ) {
+		int sz = cinfo->output_width * cinfo->output_components;
+		int x;
+
+		for( y = 0; y < r->height; y++ ) 
+			for( x = 0; x < sz; x++ )
+				row_pointer[y][x] = 255 - row_pointer[y][x];
+	}
+
+	return( 0 );
+}
+
 /* Read a cinfo to a VIPS image.
  */
 static int
-read_jpeg_image( ReadJpeg *jpeg )
+read_jpeg_image( ReadJpeg *jpeg, VipsImage *out )
 {
 	struct jpeg_decompress_struct *cinfo = &jpeg->cinfo;
-
+	VipsImage **t = (VipsImage **) 
-	int x, y, sz;
+		vips_object_local_array( VIPS_OBJECT( out ), 3 );
-	JSAMPROW row_pointer[1];
-
-	/* Get size of output line and make a buffer.
-	 */
-	sz = cinfo->output_width * cinfo->output_components;
-	row_pointer[0] = (JSAMPLE *) (*cinfo->mem->alloc_large) 
-		( (j_common_ptr) cinfo, JPOOL_IMAGE, sz );
-
-	jpeg_start_decompress( cinfo );
 
 	/* Here for longjmp() from vips__new_error_exit().
 	 */
-	if( setjmp( jpeg->eman.jmp ) ) {
+	if( setjmp( jpeg->eman.jmp ) ) 
-		jpeg_finish_decompress( cinfo );
-
 		return( -1 );
-	}
 
-	/* Process image.
+	t[0] = vips_image_new();
+	if( read_jpeg_header( jpeg, t[0] ) )
+		return( -1 );
+
+	/* Set decompressing to make readjpeg_free() call 
+	 * jpeg_stop_decompress().
 	 */
-	for( y = 0; y < jpeg->out->Ysize; y++ ) {
+	jpeg_start_decompress( cinfo );
-		/* We set an error handler that longjmps() out, so I don't
+	jpeg->decompressing = TRUE;
-		 * think this can fail.
-		 */
-		jpeg_read_scanlines( cinfo, &row_pointer[0], 1 );
 
-		if( jpeg->invert_pels ) 
+#ifdef DEBUG
-			for( x = 0; x < sz; x++ )
+	printf( "read_jpeg_image: starting deompress\n" );
-				row_pointer[0][x] = 255 - row_pointer[0][x];
+#endif /*DEBUG*/
 
-		if( vips_image_write_line( jpeg->out, y, row_pointer[0] ) )
+	if( vips_image_generate( t[0], 
-			return( -1 );
+			NULL, read_jpeg_generate, NULL, 
-	}
+			jpeg, NULL ) ||
-
+		vips_sequential( t[0], &t[1], NULL ) ||
-	jpeg_finish_decompress( cinfo );
+		vips_tilecache( t[1], &t[2], 
+			"tile_width", t[0]->Xsize, 
+			"tile_height", VIPS__TILE_HEIGHT,
+			"max_tiles", 4,
+			NULL ) ||
+		vips_image_write( t[2], out ) )
+		return( -1 );
 
 	return( 0 );
 }
@@ -903,13 +965,13 @@ vips__jpeg_read_file( const char *filename, VipsImage *out,
 
 	/* Convert!
 	 */
-	result = read_jpeg_header( jpeg ); 
+	if( header_only )
-	if( !header_only && !result )
+		result = read_jpeg_header( jpeg, out );
-		result = read_jpeg_image( jpeg ); 
+	else
+		result = read_jpeg_image( jpeg, out );
 
-	/* Close and tidy.
+	/* Don't call readjpeg_free(), we're probably still live.
 	 */
-	result |= readjpeg_free( jpeg );
 
 	return( result );
 }
@@ -1114,13 +1176,13 @@ vips__jpeg_read_buffer( void *buf, size_t len, VipsImage *out,
 
 	/* Convert!
 	 */
-	result = read_jpeg_header( jpeg );
+	if( header_only )
-	if( !header_only && !result )
+		result = read_jpeg_header( jpeg, out );
-		result = read_jpeg_image( jpeg );
+	else
+		result = read_jpeg_image( jpeg, out );
 
-	/* Close and tidy.
+	/* Don't call readjpeg_free(), we're probably still live.
 	 */
-	result |= readjpeg_free( jpeg );
 
 	return( result );
 }

libvips/foreign/jpegload.c

@@ -102,8 +102,7 @@ vips_foreign_load_jpeg_get_flags( VipsForeignLoad *load )
 {
 	/* The jpeg reader supports sequential read.
 	 */
-	//return( VIPS_FOREIGN_SEQUENTIAL );
+	return( VIPS_FOREIGN_SEQUENTIAL );
-	return( 0 );
 }
 
 static int
@@ -183,8 +182,7 @@ vips_foreign_load_jpeg_file_get_flags_filename( const char *filename )
 {
 	/* The jpeg reader supports sequential read.
 	 */
-	//return( VIPS_FOREIGN_SEQUENTIAL );
+	return( VIPS_FOREIGN_SEQUENTIAL );
-	return( 0 );
 }
 
 static gboolean

libvips/foreign/vips2jpeg.c

@@ -129,10 +129,10 @@ vips__new_output_message( j_common_ptr cinfo )
 	char buffer[JMSG_LENGTH_MAX];
 
 	(*cinfo->err->format_message)( cinfo, buffer );
-	vips_error( "VipsFile*Jpeg", _( "%s" ), buffer );
+	vips_error( "VipsJpeg", _( "%s" ), buffer );
 
 #ifdef DEBUG
-	printf( "jpeg.c: new_output_message: \"%s\"\n", buffer );
+	printf( "vips__new_output_message: \"%s\"\n", buffer );
 #endif /*DEBUG*/
 }
 
@@ -144,7 +144,7 @@ vips__new_error_exit( j_common_ptr cinfo )
 	ErrorManager *eman = (ErrorManager *) cinfo->err;
 
 #ifdef DEBUG
-	printf( "jpeg.c: new_error_exit\n" );
+	printf( "vips__new_error_exit:\n" );
 #endif /*DEBUG*/
 
 	/* Close the fp if necessary.
@@ -929,13 +929,13 @@ block_print( Block *block )
 {
 	int i;
 
-	printf( "total length = %d\n", block_length( block ) );
+	printf( "total length = %zd\n", block_length( block ) );
 	printf( "set of blocks:\n" );
 
 	i = 0;
 	for( block = block->first; block; block = block->next ) {
 		printf( "%d) %p, first = %p, next = %p"
-			"\t data = %p, size = %d, used = %d\n", 
+			"\t data = %p, size = %zd, used = %zd\n", 
 			i, block, block->first, block->next,
 			block->data, block->size, block->used );
 		i += 1;