update magick6 loader

now supports page/n/page-height

ChangeLog

@@ -12,6 +12,7 @@
 - add vips_image_get_fields() to help bindings
 - add tiff multi-page read/write
 - add VIPS_META_PAGE_HEIGHT metadata
+- IM6 magickload supports page/n, all_frames deprecated
 
 11/11/16 started 8.4.4
 - fix crash in vips.exe arg parsing on Windows, thanks Yury

TODO

@@ -14,11 +14,7 @@
 
   magick6
 
-	john@kiwi:~/pics$ vips magickload nipguide.pdf x.v --all-frames
+	deprecated all_frames, added n, added tests
-	john@kiwi:~/pics$ vipsheader x.v
-	x.v: 595x842 ushort, 2 bands, grey16, magickload
-
-  	argh
 
   magick7
 

libvips/deprecated/im_magick2vips.c

@@ -50,7 +50,7 @@ im_magick2vips( const char *filename, IMAGE *out )
 #ifdef HAVE_MAGICK
 	/* Old behaviour was always to read all frames.
 	 */
-	return( vips__magick_read( filename, out, TRUE, NULL, 0 ) );
+	return( vips__magick_read( filename, out, NULL, 0, -1 ) );
 #else
 	vips_error( "im_magick2vips", 
 		"%s", _( "no libMagick support in your libvips" ) ); 

libvips/foreign/magick2vips.c

@@ -51,6 +51,8 @@
  * 	- add @page option, 0 by default
  * 18/4/16
  * 	- fix @page with graphicsmagick
+ * 25/11/16
+ * 	- remove @all_frames, add @n
  */
 
 /*
@@ -119,8 +121,8 @@
 typedef struct _Read {
 	char *filename;
 	VipsImage *im;
-	gboolean all_frames;
 	int page;
+	int n;
 
 	Image *image;
 	ImageInfo *image_info;
@@ -166,7 +168,7 @@ read_close( VipsImage *im, Read *read )
 
 static Read *
 read_new( const char *filename, VipsImage *im, 
-	gboolean all_frames, const char *density, int page )
+	const char *density, int page, int n )
 {
 	Read *read;
 	static int inited = 0;
@@ -180,11 +182,17 @@ read_new( const char *filename, VipsImage *im,
 		inited = 1;
 	}
 
+	/* IM doesn't use the -1 means end-of-file convention, change it to a
+	 * very large number.
+	 */
+	if( n == -1 )
+		n = 100000;
+
 	if( !(read = VIPS_NEW( im, Read )) )
 		return( NULL );
 	read->filename = filename ? g_strdup( filename ) : NULL;
-	read->all_frames = all_frames;
 	read->page = page;
+	read->n = n;
 	read->im = im;
 	read->image = NULL;
 	read->image_info = CloneImageInfo( NULL );
@@ -218,24 +226,24 @@ read_new( const char *filename, VipsImage *im,
   	SetImageOption( read->image_info, "dcm:display-range", "reset" );
 #endif /*HAVE_SETIMAGEOPTION*/
 
-	if( !all_frames ) {
+	if( read->page > 0 ) { 
 #ifdef HAVE_NUMBER_SCENES 
-		 /* I can't find docs for these fields, but this seems to work.
+		/* I can't find docs for these fields, but this seems to work.
-		  */
+		 */
 		char page[256];
 
 		read->image_info->scene = read->page;
-		read->image_info->number_scenes = 1;
+		read->image_info->number_scenes = read->n;
 
 		/* Some IMs must have the string version set as well.
 		 */
-		vips_snprintf( page, 256, "%d", read->page );
+		vips_snprintf( page, 256, "%d-%d", read->page, read->page + n );
 		read->image_info->scenes = strdup( page );
 #else /*!HAVE_NUMBER_SCENES*/
 		/* This works with GM 1.2.31 and probably others.
 		 */
 		read->image_info->subimage = read->page;
-		read->image_info->subrange = 1;
+		read->image_info->subrange = read->n;
 #endif
 	}
 
@@ -465,8 +473,17 @@ parse_header( Read *read )
 	for( p = image; p; (p = GetNextImageInList( p )) ) {
 		if( p->columns != (unsigned int) im->Xsize ||
 			p->rows != (unsigned int) im->Ysize ||
-			get_bands( p ) != im->Bands )
+			get_bands( p ) != im->Bands ) {
+#ifdef DEBUG
+			printf( "frame %d differs\n", read->n_frames );
+			printf( "%zdx%zd, %d bands\n", 
+				p->columns, p->rows, get_bands( p ) );
+			printf( "first frame is %dx%d, %d bands\n", 
+				im->Xsize, im->Ysize, im->Bands );
+#endif /*DEBUG*/
+
 			break;
+		}
 
 		read->n_frames += 1;
 	}
@@ -479,14 +496,11 @@ parse_header( Read *read )
 	printf( "image has %d frames\n", read->n_frames );
 #endif /*DEBUG*/
 
-	/* If all_frames is off, just get the first one.
+	if( read->n != -1 )
-	 */
+		read->n_frames = VIPS_MIN( read->n_frames, read->n );
-	if( !read->all_frames )
-		read->n_frames = 1;
 
 	/* Record frame pointers.
 	 */
-	im->Ysize *= read->n_frames;
 	if( !(read->frames = VIPS_ARRAY( NULL, read->n_frames, Image * )) )
 		return( -1 );
 	p = image;
@@ -495,6 +509,11 @@ parse_header( Read *read )
 		p = GetNextImageInList( p );
 	}
 
+	if( read->n_frames > 1 ) {
+		vips_image_set_int( im, VIPS_META_PAGE_HEIGHT, im->Ysize );
+		im->Ysize *= read->n_frames;
+	}
+
 	return( 0 );
 }
 
@@ -711,8 +730,8 @@ magick_fill_region( VipsRegion *out,
 }
 
 int
-vips__magick_read( const char *filename, VipsImage *out, 
+vips__magick_read( const char *filename, 
-	gboolean all_frames, const char *density, int page )
+	VipsImage *out, const char *density, int page, int n )
 {
 	Read *read;
 
@@ -720,7 +739,7 @@ vips__magick_read( const char *filename, VipsImage *out,
 	printf( "magick2vips: vips__magick_read: %s\n", filename );
 #endif /*DEBUG*/
 
-	if( !(read = read_new( filename, out, all_frames, density, page )) )
+	if( !(read = read_new( filename, out, density, page, n )) )
 		return( -1 );
 
 #ifdef DEBUG
@@ -750,8 +769,8 @@ vips__magick_read( const char *filename, VipsImage *out,
  * http://www.imagemagick.org/discourse-server/viewtopic.php?f=1&t=20017
  */
 int
-vips__magick_read_header( const char *filename, VipsImage *im, 
+vips__magick_read_header( const char *filename, 
-	gboolean all_frames, const char *density, int page )
+	VipsImage *out, const char *density, int page, int n )
 {
 	Read *read;
 
@@ -759,7 +778,7 @@ vips__magick_read_header( const char *filename, VipsImage *im,
 	printf( "vips__magick_read_header: %s\n", filename );
 #endif /*DEBUG*/
 
-	if( !(read = read_new( filename, im, all_frames, density, page )) )
+	if( !(read = read_new( filename, out, density, page, n )) )
 		return( -1 );
 
 #ifdef DEBUG
@@ -778,7 +797,8 @@ vips__magick_read_header( const char *filename, VipsImage *im,
 	if( parse_header( read ) ) 
 		return( -1 );
 
-	if( im->Xsize <= 0 || im->Ysize <= 0 ) {
+	if( out->Xsize <= 0 || 
+		out->Ysize <= 0 ) {
 		vips_error( "magick2vips", "%s", _( "bad image size" ) );
 		return( -1 );
 	}
@@ -791,8 +811,8 @@ vips__magick_read_header( const char *filename, VipsImage *im,
 }
 
 int
-vips__magick_read_buffer( const void *buf, const size_t len, VipsImage *out,
+vips__magick_read_buffer( const void *buf, const size_t len, 
-	gboolean all_frames, const char *density, int page )
+	VipsImage *out, const char *density, int page, int n )
 {
 	Read *read;
 
@@ -800,7 +820,7 @@ vips__magick_read_buffer( const void *buf, const size_t len, VipsImage *out,
 	printf( "magick2vips: vips__magick_read_buffer: %p %zu\n", buf, len );
 #endif /*DEBUG*/
 
-	if( !(read = read_new( NULL, out, all_frames, density, page )) )
+	if( !(read = read_new( NULL, out, density, page, n )) )
 		return( -1 );
 
 #ifdef DEBUG
@@ -827,8 +847,7 @@ vips__magick_read_buffer( const void *buf, const size_t len, VipsImage *out,
 
 int
 vips__magick_read_buffer_header( const void *buf, const size_t len, 
-	VipsImage *im, 
+	VipsImage *out, const char *density, int page, int n )
-	gboolean all_frames, const char *density, int page )
 {
 	Read *read;
 
@@ -836,7 +855,7 @@ vips__magick_read_buffer_header( const void *buf, const size_t len,
 	printf( "vips__magick_read_buffer_header: %p %zu\n", buf, len );
 #endif /*DEBUG*/
 
-	if( !(read = read_new( NULL, im, all_frames, density, page )) )
+	if( !(read = read_new( NULL, out, density, page, n )) )
 		return( -1 );
 
 #ifdef DEBUG
@@ -854,8 +873,8 @@ vips__magick_read_buffer_header( const void *buf, const size_t len,
 	if( parse_header( read ) ) 
 		return( -1 );
 
-	if( im->Xsize <= 0 || 
+	if( out->Xsize <= 0 || 
-		im->Ysize <= 0 ) {
+		out->Ysize <= 0 ) {
 		vips_error( "magick2vips", "%s", _( "bad image size" ) );
 		return( -1 );
 	}

libvips/foreign/magickload.c

@@ -8,6 +8,8 @@
  * 	- add @all_frames option, off by default
  * 14/2/16
  * 	- add @page option, 0 by default
+ * 25/11/16
+ * 	- add @n, deprecate @all_frames (just sets n = -1)
  */
 
 /*
@@ -61,9 +63,13 @@
 typedef struct _VipsForeignLoadMagick {
 	VipsForeignLoad parent_object;
 
-	gboolean all_frames;		/* Load all frames */
+	/* Deprecated. Just sets n = -1.
+	 */
+	gboolean all_frames;
+
 	char *density;			/* Load at this resolution */
 	int page;			/* Load this page (frame) */
+	int n;				/* Load this many pages */
 
 } VipsForeignLoadMagick;
 
@@ -110,7 +116,7 @@ vips_foreign_load_magick_class_init( VipsForeignLoadMagickClass *class )
 	VIPS_ARG_BOOL( class, "all_frames", 3, 
 		_( "all_frames" ), 
 		_( "Read all frames from an image" ),
-		VIPS_ARGUMENT_OPTIONAL_INPUT,
+		VIPS_ARGUMENT_OPTIONAL_INPUT | VIPS_ARGUMENT_DEPRECATED,
 		G_STRUCT_OFFSET( VipsForeignLoadMagick, all_frames ),
 		FALSE );
 
@@ -127,11 +133,19 @@ vips_foreign_load_magick_class_init( VipsForeignLoadMagickClass *class )
 		VIPS_ARGUMENT_OPTIONAL_INPUT,
 		G_STRUCT_OFFSET( VipsForeignLoadMagick, page ),
 		0, 100000, 0 );
+
+	VIPS_ARG_INT( class, "n", 6,
+		_( "n" ),
+		_( "Load this many pages" ),
+		VIPS_ARGUMENT_OPTIONAL_INPUT,
+		G_STRUCT_OFFSET( VipsForeignLoadMagick, n ),
+		-1, 100000, 1 );
 }
 
 static void
 vips_foreign_load_magick_init( VipsForeignLoadMagick *magick )
 {
+	magick->n = 1;
 }
 
 typedef struct _VipsForeignLoadMagickFile {
@@ -154,7 +168,7 @@ ismagick( const char *filename )
 
 	t = vips_image_new();
 	vips_error_freeze();
-	result = vips__magick_read_header( filename, t, FALSE, NULL, 0 );
+	result = vips__magick_read_header( filename, t, NULL, 0, 1 );
 	g_object_unref( t );
 	vips_error_thaw();
 
@@ -175,8 +189,11 @@ vips_foreign_load_magick_file_header( VipsForeignLoad *load )
 	VipsForeignLoadMagickFile *magick_file = 
 		(VipsForeignLoadMagickFile *) load;
 
+	if( magick->all_frames )
+		magick->n = -1;
+
 	if( vips__magick_read( magick_file->filename, 
-		load->out, magick->all_frames, magick->density, magick->page ) )
+		load->out, magick->density, magick->page, magick->n ) )
 		return( -1 );
 
 	VIPS_SETSTR( load->out->filename, magick_file->filename );
@@ -236,7 +253,7 @@ vips_foreign_load_magick_buffer_is_a_buffer( const void *buf, size_t len )
 
 	t = vips_image_new();
 	vips_error_freeze();
-	result = vips__magick_read_buffer_header( buf, len, t, FALSE, NULL, 0 );
+	result = vips__magick_read_buffer_header( buf, len, t, NULL, 0, 1 );
 	g_object_unref( t );
 	vips_error_thaw();
 
@@ -257,9 +274,12 @@ vips_foreign_load_magick_buffer_header( VipsForeignLoad *load )
 	VipsForeignLoadMagickBuffer *magick_buffer = 
 		(VipsForeignLoadMagickBuffer *) load;
 
+	if( magick->all_frames )
+		magick->n = -1;
+
 	if( vips__magick_read_buffer( 
 		magick_buffer->buf->data, magick_buffer->buf->length, 
-		load->out, magick->all_frames, magick->density, magick->page ) )
+		load->out, magick->density, magick->page, magick->n ) )
 		return( -1 );
 
 	return( 0 );
@@ -307,8 +327,8 @@ vips_foreign_load_magick_buffer_init( VipsForeignLoadMagickBuffer *buffer )
  *
  * Optional arguments:
  *
- * * @all_frames: %gboolean, load all frames in sequence
  * * @page: %gint, load from this page
+ * * @n: %gint, load this many pages
  * * @density: string, canvas resolution for rendering vector formats like SVG
  *
  * Read in an image using libMagick, the ImageMagick library. This library can
@@ -322,7 +342,8 @@ vips_foreign_load_magick_buffer_init( VipsForeignLoadMagickBuffer *buffer )
  * "--with-magickpackage" configure option.
  *
  * Normally it will only load the first image in a many-image sequence (such
- * as a GIF). Set @all_frames to true to read the whole image sequence. 
+ * as a GIF or a PDF). Use @page and @n to set the start page and number of
+ * pages to load. Set @n to -1 to load all pages from @page onwards.
  *
  * @density is "WxH" in DPI, e.g. "600x300" or "600" (default is "72x72"). See
  * the [density 
@@ -355,8 +376,8 @@ vips_magickload( const char *filename, VipsImage **out, ... )
  *
  * Optional arguments:
  *
- * * @all_frames: %gboolean, load all frames in sequence
  * * @page: %gint, load from this page
+ * * @n: %gint, load this many pages
  * * @density: string, canvas resolution for rendering vector formats like SVG
  *
  * Read an image memory block using libMagick into a VIPS image. Exactly as

libvips/foreign/pforeign.h

@@ -120,14 +120,14 @@ int vips__fits_read( const char *filename, VipsImage *out );
 int vips__fits_write( VipsImage *in, const char *filename );
 
 int vips__magick_read( const char *filename, 
-	VipsImage *out, gboolean all_frames, const char *density, int page );
+	VipsImage *out, const char *density, int page, int n );
 int vips__magick_read_header( const char *filename, 
-	VipsImage *out, gboolean all_frames, const char *density, int page );
+	VipsImage *out, const char *density, int page, int n );
 
 int vips__magick_read_buffer( const void *buf, const size_t len,
-	VipsImage *out, gboolean all_frames, const char *density, int page );
+	VipsImage *out, const char *density, int page, int n );
 int vips__magick_read_buffer_header( const void *buf, const size_t len,
-	VipsImage *out, gboolean all_frames, const char *density, int page );
+	VipsImage *out, const char *density, int page, int n );
 
 extern const char *vips__mat_suffs[];
 

test/test_foreign.py

@@ -383,6 +383,7 @@ class TestForeign(unittest.TestCase):
         #self.assertEqual(im.height, height * 2)
 
         # all-frames should load every frame of the animation
+        # (though all-frames is deprecated)
         im = Vips.Image.magickload(self.gif_anim_file)
         width = im.width
         height = im.height
@@ -390,6 +391,16 @@ class TestForeign(unittest.TestCase):
         self.assertEqual(im.width, width)
         self.assertEqual(im.height, height * 5)
 
+        # page/n let you pick a range of pages
+        im = Vips.Image.magickload(self.gif_anim_file)
+        width = im.width
+        height = im.height
+        im = Vips.Image.magickload(self.gif_anim_file, page = 1, n = 2)
+        self.assertEqual(im.width, width)
+        self.assertEqual(im.height, height * 2)
+        page_height = im.get_value("page-height")
+        self.assertEqual(page_height, height)
+
         # should work for dicom
         im = Vips.Image.magickload(self.dicom_file)
         self.assertEqual(im.width, 128)