gifload supports n and page-height

ChangeLog

@@ -13,6 +13,7 @@
 - add tiff multi-page read/write
 - add VIPS_META_PAGE_HEIGHT metadata
 - IM6/IM7 magickload supports page/n/page-height, all_frames deprecated
+- gifload supports n/page-height
 
 11/11/16 started 8.4.4
 - fix crash in vips.exe arg parsing on Windows, thanks Yury

TODO

@@ -1,30 +1,3 @@
-- all toilet roll loaders need to set "page-height"
-
-  magick, pdf, gif and tiff need to use the same page/n interface
-
-  magick6
-
-	deprecated all_frames, added n, added tests, added page-height
-
-  magick7
-
-	same
-
-  gifload has page, but no n
-
-  	add an n param
-
-	add page-height
-
-  pdfload has page, n, allows n == -1, sets page-height
-
-
-
-
-
-
-
-
 - not sure about utf8 error messages on win
 
 - strange: 

libvips/foreign/gifload.c

@@ -12,7 +12,7 @@
  * 19/8/16
  * 	- better transparency detection, thanks diegocsandrim
  * 25/11/16
- * 	- support @n
+ * 	- support @n, page-height
  */
 
 /*
@@ -428,6 +428,13 @@ vips_foreign_load_gif_render( VipsForeignLoadGif *gif, VipsImage *out )
 			}
 	}
 
+	/* We need a line buffer to decompress to.
+	 */
+	if( !gif->line ) 
+		if( !(gif->line = VIPS_ARRAY( gif, 
+			gif->file->SWidth, GifPixelType )) )
+			return( -1 ); 
+
 	if( file->Image.Interlace ) {
 		int i;
 
@@ -482,44 +489,17 @@ vips_foreign_load_gif_render( VipsForeignLoadGif *gif, VipsImage *out )
 	return( 0 );
 }
 
+/* Write the next page, if there is one, to @page. Set EOF if we hit the end of
+ * the file. @page must be a memory image of the right size. 
+ */
 static int
-vips_foreign_load_gif_to_memory( VipsForeignLoadGif *gif, 
+vips_foreign_load_gif_page( VipsForeignLoadGif *gif, VipsImage *out )
-	VipsImage *previous, VipsImage *out )
 {
-	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( gif );
-
 	GifRecordType record;
+	int n_pages;
 
-	vips_image_init_fields( out, 
+	n_pages = 0;
-		gif->file->SWidth, gif->file->SHeight,
-		4, VIPS_FORMAT_UCHAR,
-		VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, 1.0, 1.0 );
 
-	/* We will have the whole GIF frame in memory, so we can render any 
-	 * area.
-	 */
-        vips_image_pipelinev( out, VIPS_DEMAND_STYLE_ANY, NULL );
-
-	/* We need a line buffer to decompress to.
-	 */
-	gif->line = VIPS_ARRAY( gif, gif->file->SWidth, GifPixelType );
-
-	/* Turn out into a memory image which we then render the GIF frames
-	 * into.
-	 */
-	if( vips_image_write_prepare( out ) )
-		return( -1 );
-
-	/* And init with the previous frame, if any.
-	 */
-	if( previous ) 
-		memcpy( VIPS_IMAGE_ADDR( out, 0, 0 ),
-			VIPS_IMAGE_ADDR( previous, 0, 0 ),
-			VIPS_IMAGE_SIZEOF_IMAGE( out ) );
-
-	/* Scan the GIF until we have enough to have completely rendered the
-	 * next frame.
-	 */
 	do { 
 		GifByteType *extension;
 		int ext_code;
@@ -541,10 +521,10 @@ vips_foreign_load_gif_to_memory( VipsForeignLoadGif *gif,
 			if( vips_foreign_load_gif_render( gif, out ) )
 				return( -1 ); 
 
-			gif->current_page += 1;
+			n_pages += 1;
 
-			VIPS_DEBUG_MSG( "gifload: frame %d:\n", 
+			VIPS_DEBUG_MSG( "gifload: page %d:\n", 
-				gif->current_page );
+				gif->current_page + n_pages );
 
 			break;
 
@@ -607,23 +587,41 @@ vips_foreign_load_gif_to_memory( VipsForeignLoadGif *gif,
 		default:
 			break;
 		}
-	} while( gif->current_page + frame_n <= gif->page && 
+	} while( n_pages < 1 &&
 		!gif->eof );
 
-	if( frame_n <= gif->page ) {
+	gif->current_page += n_pages;
-		vips_error( class->nickname, 
-			"%s", _( "too few frames in GIF file" ) );
-		return( -1 );
-	}
-
-	/* We've rendered to a memory image ... we can shut down the GIF
-	 * reader now.
-	 */
-	vips_foreign_load_gif_close( gif ); 
 
 	return( 0 );
 }
 
+static VipsImage *
+vips_foreign_load_gif_new_page( VipsForeignLoadGif *gif )
+{
+	VipsImage *out;
+
+	out = vips_image_new_memory();
+
+	vips_image_init_fields( out, 
+		gif->file->SWidth, gif->file->SHeight, 4, VIPS_FORMAT_UCHAR,
+		VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, 1.0, 1.0 );
+
+	/* We will have the whole GIF frame in memory, so we can render any 
+	 * area.
+	 */
+        vips_image_pipelinev( out, VIPS_DEMAND_STYLE_ANY, NULL );
+
+	/* Turn out into a memory image which we then render the GIF frames
+	 * into.
+	 */
+	if( vips_image_write_prepare( out ) ) {
+		g_object_unref( out ); 
+		return( NULL );
+	}
+
+	return( out );
+}
+
 static void
 unref_array( GSList *list )
 {
@@ -635,52 +633,112 @@ unref_array( GSList *list )
  * so we can't just allocate a large area.
  */
 static int
-vips_foreign_load_gif_frames( VipsForeignLoadGif *gif, VipsImage **out )
+vips_foreign_load_gif_pages( VipsForeignLoadGif *gif, VipsImage **out )
 {
 	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( gif );
 
 	GSList *frames;
+	VipsImage *frame;
 	VipsImage *previous;
 	VipsImage **t;
-	GifRecordType record;
 	int n_frames;
 	int i;
 
 	frames = NULL;
 	previous = NULL;
 
-	do {
+	/* Accumulate any start stuff up to the first frame we need.
-		VipsImage *frame;
+	 */
+	if( !(frame = vips_foreign_load_gif_new_page( gif )) ) 
+		return( -1 );
+	do { 
+		if( vips_foreign_load_gif_page( gif, frame ) ) {
+			g_object_unref( frame );
+			return( -1 );
+		}
+	} while( !gif->eof &&
+		gif->current_page <= gif->page );
 
-		if( vips_foreign_load_gif_to_memory( gif, previous, &frame ) ) {
+	if( gif->eof ) {
+		vips_error( class->nickname, 
+			"%s", _( "too few frames in GIF file" ) );
+		g_object_unref( frame );
+		return( -1 );
+	}
+
+	frames = g_slist_append( frames, frame );
+	previous = frame;
+
+	while( gif->n == -1 ||
+		gif->current_page < gif->page + gif->n ) {
+		/* We might need a frame for this read to render to.
+		 */
+		if( !(frame = vips_foreign_load_gif_new_page( gif )) ) {
 			unref_array( frames );
 			return( -1 );
 		}
-		frames = g_slist_append( frames, frame );
+
-		previous = frame;
+		/* And init with the previous frame, if any.
-	} while( (gif->n == -1 && !gif->eof) ||
+		 */
-		(gif->current_page < gif->page + gif->n) );
+		if( previous ) 
+			memcpy( VIPS_IMAGE_ADDR( frame, 0, 0 ),
+				VIPS_IMAGE_ADDR( previous, 0, 0 ),
+				VIPS_IMAGE_SIZEOF_IMAGE( frame ) );
+
+		if( vips_foreign_load_gif_page( gif, frame ) ) {
+			g_object_unref( frame ); 
+			unref_array( frames );
+			return( -1 );
+		}
+
+		if( gif->eof ) {
+			/* Nope, didn't need the new frame.
+			 */
+			g_object_unref( frame ); 
+			break;
+		}
+		else {
+			frames = g_slist_append( frames, frame );
+			previous = frame;
+		}
+	}
+
+	n_frames = g_slist_length( frames ); 
+
+	if( gif->eof &&
+		gif->n != -1 &&
+		n_frames < gif->n ) {
+		unref_array( frames );
+		vips_error( class->nickname, 
+			"%s", _( "too few frames in GIF file" ) );
+		return( -1 );
+	}
 
 	/* We've rendered to a set of memory images ... we can shut down the GIF
 	 * reader now.
 	 */
 	vips_foreign_load_gif_close( gif ); 
 
-	n_frames = gif->current_page - gif->page;
 	if( !(t = VIPS_ARRAY( gif, n_frames, VipsImage * )) ) { 
 		unref_array( frames );
 		return( -1 );
 	}
 
 	for( i = 0; i < n_frames; i++ )
-		t[i] = g_slist_nth( frames, i );
+		t[i] = (VipsImage *) g_slist_nth_data( frames, i );
 
-	if( vips_arrayjoin( t, out, n_frames, NULL ) ) { 
+	if( vips_arrayjoin( t, out, n_frames, 
+		"across", 1,
+		NULL ) ) { 
 		unref_array( frames );
 		return( -1 );
 	}
+
 	unref_array( frames );
 
+	if( n_frames > 1 )
+		vips_image_set_int( *out, VIPS_META_PAGE_HEIGHT, frame->Ysize );
+
 	return( 0 );
 }
 
@@ -693,11 +751,9 @@ vips_foreign_load_gif_load( VipsForeignLoad *load )
 
 	VipsImage *im;
 
-	/* Render to a memory image.
+	if( vips_foreign_load_gif_pages( gif, &t[0] ) )
-	 */
-	im = t[0] = vips_image_new_memory();
-	if( vips_foreign_load_gif_to_memory( gif, im ) )
 		return( -1 );
+	im = t[0];
 
 	/* Depending on what we found, transform and write to load->real.
 	 */

test/test_foreign.py

@@ -560,6 +560,18 @@ class TestForeign(unittest.TestCase):
         self.file_loader("gifload", self.gif_file, gif_valid)
         self.buffer_loader("gifload_buffer", self.gif_file, gif_valid)
 
+        x1 = Vips.Image.new_from_file(self.gif_anim_file )
+        x2 = Vips.Image.new_from_file(self.gif_anim_file, n = 2 )
+        self.assertEqual(x2.height, 2 * x1.height)
+        page_height = x2.get_value("page-height")
+        self.assertEqual(page_height, x1.height)
+
+        x2 = Vips.Image.new_from_file(self.gif_anim_file, n = -1 )
+        self.assertEqual(x2.height, 5 * x1.height)
+
+        x2 = Vips.Image.new_from_file(self.gif_anim_file, page = 1, n = -1 )
+        self.assertEqual(x2.height, 4 * x1.height)
+
     def test_svgload(self):
         x = Vips.type_find("VipsForeign", "svgload")
         if not x.is_instantiatable():