seems to work, needs more testing

2016-07-08 17:56:30 +01:00 · 2016-07-08 17:56:30 +01:00 · a5d9cf204c
commit a5d9cf204c
parent 40f8a8af85
3 changed files with 478 additions and 36 deletions
--- a/1
+++ b/1
@ -27,6 +27,7 @@
 - better quality for vips_resize() with linear/cubic kernels 
 - pyvips8 can create new metadata
 - better upsizing with vips_resize()
+- add imagemagick v7 support, thanks sachinwalia2k8 

 18/5/16 started 8.3.2
 - more robust vips image reading
--- a/libvips/foreign/foreign.c
+++ b/libvips/foreign/foreign.c
@ -1760,6 +1760,11 @@ vips_foreign_operation_init( void )
 	vips_foreign_load_magick_buffer_get_type(); 
 #endif /*HAVE_MAGICK*/

+#ifdef HAVE_MAGICK7
+	vips_foreign_load_magick7_file_get_type(); 
+	vips_foreign_load_magick7_buffer_get_type(); 
+#endif /*HAVE_MAGICK7*/
+
 #ifdef HAVE_CFITSIO
 	vips_foreign_load_fits_get_type(); 
 	vips_foreign_save_fits_get_type(); 
--- a/libvips/foreign/magick7load.c
+++ b/libvips/foreign/magick7load.c
@ -50,6 +50,8 @@

 #ifdef HAVE_MAGICK7

+#include <MagickWand/MagickWand.h>
+
 typedef struct _VipsForeignLoadMagick7 {
 	VipsForeignLoad parent_object;

@ -57,6 +59,19 @@ typedef struct _VipsForeignLoadMagick7 {
 	char *density;			/* Load at this resolution */
 	int page;			/* Load this page (frame) */

+	Image *image;
+	ImageInfo *image_info;
+	ExceptionInfo *exception;
+
+	int n_frames;			/* Number of frames in file */
+	Image **frames;			/* An Image* for each frame */
+	CacheView **cache_view; 	/* A CacheView for each frame */
+	int frame_height;	
+
+	/* Mutex to serialise calls to libMagick during threaded read.
+	 */
+	GMutex *lock;
+
 } VipsForeignLoadMagick7;

 typedef VipsForeignLoadClass VipsForeignLoadMagick7Class;
@ -76,6 +91,100 @@ vips_foreign_load_magick7_get_flags( VipsForeignLoad *load )
 	return( VIPS_FOREIGN_PARTIAL );
 }

+static void
+vips_foreign_load_magick7_dispose( GObject *gobject )
+{
+	VipsForeignLoadMagick7 *magick7 = (VipsForeignLoadMagick7 *) gobject;
+
+	int i;
+
+#ifdef DEBUG
+	printf( "vips_foreign_load_magick7_dispose: %p\n", gobject ); 
+#endif /*DEBUG*/
+
+	for( i = 0; i < magick7->n_frames; i++ ) {
+		VIPS_FREEF( DestroyCacheView, magick7->cache_view[i] ); 
+	}
+	VIPS_FREEF( DestroyImageList, magick7->image );
+	VIPS_FREEF( DestroyImageInfo, magick7->image_info ); 
+	VIPS_FREE( magick7->frames );
+	VIPS_FREEF( DestroyExceptionInfo, magick7->exception ); 
+	VIPS_FREEF( vips_g_mutex_free, magick7->lock );
+
+	G_OBJECT_CLASS( vips_foreign_load_magick7_parent_class )->
+		dispose( gobject );
+}
+
+static void *
+vips_foreign_load_magick7_genesis_cb( void *client )
+{
+#ifdef DEBUG
+	printf( "vips_foreign_load_magick7_genesis:\n" ); 
+#endif /*DEBUG*/
+
+	MagickCoreGenesis( vips_get_argv0(), MagickFalse );
+
+	return( NULL );
+}
+
+static void
+vips_foreign_load_magick7_genesis( void )
+{
+	static GOnce once = G_ONCE_INIT;
+
+	(void) g_once( &once, vips_foreign_load_magick7_genesis_cb, NULL );
+}
+
+static int
+vips_foreign_load_magick7_build( VipsObject *object )
+{
+	VipsForeignLoadMagick7 *magick7 = (VipsForeignLoadMagick7 *) object;
+
+#ifdef DEBUG
+	printf( "vips_foreign_load_magick7_build: %p\n", object ); 
+#endif /*DEBUG*/
+
+	vips_foreign_load_magick7_genesis();
+
+	magick7->image_info = CloneImageInfo( NULL );
+	magick7->exception = AcquireExceptionInfo();
+	magick7->lock = vips_g_mutex_new();
+
+	if( !magick7->image_info ) 
+		return( -1 );
+
+	/* Canvas resolution for rendering vector formats like SVG.
+	 */
+	VIPS_SETSTR( magick7->image_info->density, magick7->density );
+
+	/* When reading DICOM images, we want to ignore any
+	 * window_center/_width setting, since it may put pixels outside the
+	 * 0-65535 range and lose data. 
+	 *
+	 * These window settings are attached as vips metadata, so our caller
+	 * can interpret them if it wants.
+	 */
+  	SetImageOption( magick7->image_info, "dcm:display-range", "reset" );
+
+	if( !magick7->all_frames ) {
+		 /* I can't find docs for these fields, but this seems to work.
+		  */
+		char page[256];
+
+		magick7->image_info->scene = magick7->page;
+		magick7->image_info->number_scenes = 1;
+
+		vips_snprintf( page, 256, "%d", magick7->page );
+		magick7->image_info->scenes = strdup( page );
+	}
+
+	if( VIPS_OBJECT_CLASS( vips_foreign_load_magick7_parent_class )->
+		build( object ) )
+		return( -1 );
+
+	return( 0 );
+}
+
 static void
 vips_foreign_load_magick7_class_init( VipsForeignLoadMagick7Class *class )
 {
@ -84,11 +193,13 @@ vips_foreign_load_magick7_class_init( VipsForeignLoadMagick7Class *class )
 	VipsForeignClass *foreign_class = (VipsForeignClass *) class;
 	VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class;

+	gobject_class->dispose = vips_foreign_load_magick7_dispose;
 	gobject_class->set_property = vips_object_set_property;
 	gobject_class->get_property = vips_object_get_property;

 	object_class->nickname = "magickload_base";
 	object_class->description = _( "load with ImageMagick7" );
+	object_class->build = vips_foreign_load_magick7_build;

 	/* We need to be well to the back of the queue since vips's
 	 * dedicated loaders are usually preferable.
@ -126,6 +237,297 @@ vips_foreign_load_magick7_init( VipsForeignLoadMagick7 *magick7 )
 {
 }

+static void
+vips_foreign_load_magick7_error( VipsForeignLoadMagick7 *magick7 )
+{
+	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( magick7 );
+
+	vips_error( class->nickname, _( "Magick: %s %s" ),
+		magick7->exception->reason, 
+		magick7->exception->description );
+}
+
+static int
+vips_foreign_load_magick7_parse( VipsForeignLoadMagick7 *magick7, 
+	Image *image, VipsImage *out )
+{
+	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( magick7 );
+
+	const char *key;
+	Image *p;
+	int i;
+
+#ifdef DEBUG
+	printf( "image->depth = %zd\n", image->depth ); 
+	printf( "GetImageType() = %d\n", GetImageType( image ) );
+	printf( "GetPixelChannels() = %zd\n", GetPixelChannels( image ) ); 
+	printf( "image->columns = %zd\n", image->columns ); 
+	printf( "image->rows = %zd\n", image->rows ); 
+#endif /*DEBUG*/
+
+	out->Xsize = image->columns;
+	out->Ysize = image->rows;
+	out->Bands = GetPixelChannels( image ); 
+	magick7->frame_height = image->rows;
+
+	/* Depth can be 'fractional'. You'd think we should use
+	 * GetImageDepth() but that seems to compute something very complex. 
+	 */
+	out->BandFmt = -1;
+	if( image->depth >= 1 && image->depth <= 8 ) 
+		out->BandFmt = VIPS_FORMAT_UCHAR;
+	if( image->depth >= 9 && image->depth <= 16 ) 
+		out->BandFmt = VIPS_FORMAT_USHORT;
+	if( image->depth == 32 )
+		out->BandFmt = VIPS_FORMAT_FLOAT;
+	if( image->depth == 64 )
+		out->BandFmt = VIPS_FORMAT_DOUBLE;
+
+	if( out->BandFmt == -1 ) {
+		vips_error( class->nickname, 
+			_( "unsupported bit depth %zd" ), image->depth );
+		return( -1 );
+	}
+
+	switch( image->colorspace ) {
+	case GRAYColorspace:
+		if( out->BandFmt == VIPS_FORMAT_USHORT )
+			out->Type = VIPS_INTERPRETATION_GREY16;
+		else
+			out->Type = VIPS_INTERPRETATION_B_W;
+		break;
+
+	case RGBColorspace:
+		if( out->BandFmt == VIPS_FORMAT_USHORT )
+			out->Type = VIPS_INTERPRETATION_RGB16;
+		else
+			out->Type = VIPS_INTERPRETATION_RGB;
+		break;
+
+	case sRGBColorspace:
+		if( out->BandFmt == VIPS_FORMAT_USHORT )
+			out->Type = VIPS_INTERPRETATION_RGB16;
+		else
+			out->Type = VIPS_INTERPRETATION_sRGB;
+		break;
+
+	case CMYKColorspace:
+		out->Type = VIPS_INTERPRETATION_CMYK;
+		break;
+
+	default:
+		vips_error( class->nickname, 
+			_( "unsupported colorspace %d" ), 
+			(int) image->colorspace );
+		return( -1 );
+	}
+
+	switch( image->units ) {
+	case PixelsPerInchResolution:
+		out->Xres = image->resolution.x / 25.4;
+		out->Yres = image->resolution.y / 25.4;
+		vips_image_set_string( out, VIPS_META_RESOLUTION_UNIT, "in" );
+		break;
+
+	case PixelsPerCentimeterResolution:
+		out->Xres = image->resolution.x / 10.0;
+		out->Yres = image->resolution.y / 10.0;
+		vips_image_set_string( out, VIPS_META_RESOLUTION_UNIT, "cm" );
+		break;
+
+	default:
+		out->Xres = 1.0;
+		out->Yres = 1.0;
+		break;
+	}
+
+	/* Other fields.
+	 */
+	out->Coding = VIPS_CODING_NONE;
+
+	vips_image_pipelinev( out, VIPS_DEMAND_STYLE_SMALLTILE, NULL );
+
+	/* Get all the metadata.
+	 */
+	ResetImagePropertyIterator( image );
+	while( (key = GetNextImageProperty( image )) ) {
+		char name_text[256];
+		VipsBuf name = VIPS_BUF_STATIC( name_text );
+		const char *value;
+
+		value = GetImageProperty( image, key, magick7->exception );
+		if( !value ) {
+			vips_foreign_load_magick7_error( magick7 );
+			return( -1 ); 
+		}
+		vips_buf_appendf( &name, "magick-%s", key );
+		vips_image_set_string( out, vips_buf_all( &name ), value );
+	}
+
+	/* Do we have a set of equal-sized frames? Append them.
+
+	   	FIXME ... there must be an attribute somewhere from dicom read 
+		which says this is a volumetric image
+
+	 */
+	magick7->n_frames = 0;
+	for( p = image; p; (p = GetNextImageInList( p )) ) {
+		if( p->columns != (unsigned int) out->Xsize ||
+			p->rows != (unsigned int) out->Ysize ||
+			GetPixelChannels( p ) != out->Bands )
+			break;
+
+		magick7->n_frames += 1;
+	}
+	if( p ) 
+		/* Nope ... just do the first image in the list.
+		 */
+		magick7->n_frames = 1;
+
+#ifdef DEBUG
+	printf( "image has %d frames\n", magick7->n_frames );
+#endif /*DEBUG*/
+
+	/* If all_frames is off, just get the first one.
+	 */
+	if( !magick7->all_frames )
+		magick7->n_frames = 1;
+
+	/* Record frame pointers.
+	 */
+	out->Ysize *= magick7->n_frames;
+	if( !(magick7->frames = VIPS_ARRAY( NULL, magick7->n_frames, Image * )) )
+		return( -1 );
+	p = image;
+	for( i = 0; i < magick7->n_frames; i++ ) {
+		magick7->frames[i] = p;
+		p = GetNextImageInList( p );
+	}
+
+	return( 0 );
+}
+
+static void
+vips_foreign_load_magick7_unpack( VipsForeignLoadMagick7 *magick7,
+	VipsImage *im, VipsPel *q, Quantum *p, int n )
+{
+	const int ps = VIPS_IMAGE_SIZEOF_PEL( im ); 
+
+	int x, b;
+
+	for( x = 0; x < n; x++ ) { 
+		if( !GetPixelReadMask( magick7->image, p ) ) {
+			p += im->Bands; 
+			q += ps; 
+			continue;
+		}
+
+		for( b = 0; b < im->Bands; b++ ) {
+			PixelChannel channel = 
+				GetPixelChannelChannel( magick7->image, b );
+			PixelTrait traits = 
+				GetPixelChannelTraits( magick7->image, channel );
+
+			if( !(traits & UpdatePixelTrait) ) {
+				printf( "vips_foreign_load_magick7_unpack: "
+					"traits = %d\n", traits ); 
+				continue;
+			}
+
+			switch( im->BandFmt ) {
+			case VIPS_FORMAT_UCHAR:
+				((unsigned char *) q)[b] = p[b];
+				break;
+
+			case VIPS_FORMAT_USHORT:
+				((unsigned short *) q)[b] = p[b];
+				break;
+
+			case VIPS_FORMAT_FLOAT:
+				((float *) q)[b] = p[b];
+				break;
+
+			case VIPS_FORMAT_DOUBLE:
+				((double *) q)[b] = p[b];
+				break;
+
+			default:
+				g_assert_not_reached();
+			}
+		}
+
+		p += im->Bands;
+		q += ps; 
+	}
+}
+
+static int
+vips_foreign_load_magick7_fill_region( VipsRegion *or, 
+	void *seq, void *a, void *b, gboolean *stop )
+{
+	VipsForeignLoadMagick7 *magick7 = (VipsForeignLoadMagick7 *) a;
+	VipsRect *r = &or->valid;
+	VipsImage *im = or->im;
+
+	int y;
+
+	for( y = 0; y < r->height; y++ ) {
+		int top = r->top + y;
+		int frame = top / magick7->frame_height;
+		int line = top % magick7->frame_height;
+
+		Quantum *pixels;
+
+		g_mutex_lock( magick7->lock );
+		pixels = GetCacheViewAuthenticPixels( magick7->cache_view[frame],
+			r->left, line, r->width, 1, 
+			magick7->exception );
+		g_mutex_unlock( magick7->lock );
+
+		if( !pixels ) {
+			vips_foreign_load_magick7_error( magick7 ); 
+			return( -1 );
+		}
+
+		vips_foreign_load_magick7_unpack( magick7, im, 
+			VIPS_REGION_ADDR( or, r->left, top ), pixels, 
+			r->width ); 
+	}
+
+	return( 0 );
+}
+
+static int
+vips_foreign_load_magick7_header( VipsForeignLoadMagick7 *magick7 )
+{
+	VipsForeignLoad *load = (VipsForeignLoad *) magick7;
+
+	int i;
+
+#ifdef DEBUG
+	printf( "vips_foreign_load_magick7_header: %p\n", magick7 ); 
+#endif /*DEBUG*/
+
+	if( vips_foreign_load_magick7_parse( magick7, 
+		magick7->image, load->out ) )
+		return( -1 );
+
+	if( !(magick7->cache_view = VIPS_ARRAY( NULL, 
+		magick7->n_frames, CacheView * )) )
+		return( -1 );
+	for( i = 0; i < magick7->n_frames; i++ ) {
+		magick7->cache_view[i] = AcquireAuthenticCacheView( 
+			magick7->frames[i], magick7->exception );
+	}
+
+	if( vips_image_generate( load->out, 
+		NULL, vips_foreign_load_magick7_fill_region, NULL, 
+		magick7, NULL ) )
+		return( -1 );
+
+	return( 0 );
+}
+
 typedef struct _VipsForeignLoadMagick7File {
 	VipsForeignLoadMagick7 parent_object;

@ -141,37 +543,57 @@ G_DEFINE_TYPE( VipsForeignLoadMagick7File, vips_foreign_load_magick7_file,
 static gboolean
 ismagick7( const char *filename )
 {
-	VipsImage *t;
+	Image *image;
+	ImageInfo *image_info;
+	ExceptionInfo *exception;
 	int result;

-	t = vips_image_new();
-	vips_error_freeze();
-	result = vips__magick7_read_header( filename, t, FALSE, NULL, 0 );
-	g_object_unref( t );
-	vips_error_thaw();
+	vips_foreign_load_magick7_genesis();

-	return( result == 0 );
+	/* Horribly slow :-(
+	 */
+	image_info = CloneImageInfo( NULL );
+	exception = AcquireExceptionInfo();
+	vips_strncpy( image_info->filename, filename, MagickPathExtent );
+	image = PingImage( image_info, exception );
+	result = image != NULL;
+	VIPS_FREEF( DestroyImageList, image );
+	VIPS_FREEF( DestroyImageInfo, image_info ); 
+	VIPS_FREEF( DestroyExceptionInfo, exception ); 
+
+	return( result );
 }

-/* Unfortunately, libMagick7 does not support header-only reads very well. See
- *
- * http://www.imagemagick7.org/discourse-server/viewtopic.php?f=1&t=20017
- *
- * Test especially with BMP, GIF, TGA. So we are forced to read the entire 
- * image in the @header() method.
- */
 static int
 vips_foreign_load_magick7_file_header( VipsForeignLoad *load )
 {
 	VipsForeignLoadMagick7 *magick7 = (VipsForeignLoadMagick7 *) load;
-	VipsForeignLoadMagick7File *magick7_file = 
-		(VipsForeignLoadMagick7File *) load;
+	VipsForeignLoadMagick7File *file = (VipsForeignLoadMagick7File *) load;

-	if( vips__magick7_read( magick7_file->filename, 
-		load->out, magick7->all_frames, magick7->density, magick7->page ) )
+#ifdef DEBUG
+	printf( "vips_foreign_load_magick7_file_header: %p\n", load ); 
+#endif /*DEBUG*/
+
+	vips_strncpy( magick7->image_info->filename, file->filename, 
+		MagickPathExtent );
+
+	/* We'd love to use PingImage() here but sadly GetPixelChannels() is
+	 * always zero after Ping, so we are forced to use ReadImage().
+	 *
+	 * See:
+	 *
+	 * http://www.imagemagick.org/discourse-server/viewtopic.php?f=2&t=30043
+	 */
+	magick7->image = ReadImage( magick7->image_info, magick7->exception );
+	if( !magick7->image ) {
+		vips_foreign_load_magick7_error( magick7 ); 
 		return( -1 );
+	}

-	VIPS_SETSTR( load->out->filename, magick7_file->filename );
+	if( vips_foreign_load_magick7_header( magick7 ) )
+		return( -1 ); 
+
+	VIPS_SETSTR( load->out->filename, file->filename );

 	return( 0 );
 }
@ -223,25 +645,26 @@ G_DEFINE_TYPE( VipsForeignLoadMagick7Buffer, vips_foreign_load_magick7_buffer,
 static gboolean
 vips_foreign_load_magick7_buffer_is_a_buffer( const void *buf, size_t len )
 {
-	VipsImage *t;
+	Image *image;
+	ImageInfo *image_info;
+	ExceptionInfo *exception;
 	int result;

-	t = vips_image_new();
-	vips_error_freeze();
-	result = vips__magick7_read_buffer_header( buf, len, t, FALSE, NULL, 0 );
-	g_object_unref( t );
-	vips_error_thaw();
+	vips_foreign_load_magick7_genesis();

-	return( result == 0 );
+	/* Horribly slow :-(
+	 */
+	image_info = CloneImageInfo( NULL );
+	exception = AcquireExceptionInfo();
+	image = PingBlob( image_info, buf, len, exception );
+	result = image != NULL;
+	VIPS_FREEF( DestroyImageList, image );
+	VIPS_FREEF( DestroyImageInfo, image_info ); 
+	VIPS_FREEF( DestroyExceptionInfo, exception ); 
+
+	return( result );
 }

-/* Unfortunately, libMagick7 does not support header-only reads very well. See
- *
- * http://www.imagemagick7.org/discourse-server/viewtopic.php?f=1&t=20017
- *
- * Test especially with BMP, GIF, TGA. So we are forced to read the entire 
- * image in the @header() method.
- */
 static int
 vips_foreign_load_magick7_buffer_header( VipsForeignLoad *load )
 {
@ -249,10 +672,23 @@ vips_foreign_load_magick7_buffer_header( VipsForeignLoad *load )
 	VipsForeignLoadMagick7Buffer *magick7_buffer = 
 		(VipsForeignLoadMagick7Buffer *) load;

-	if( vips__magick7_read_buffer( 
-		magick7_buffer->buf->data, magick7_buffer->buf->length, 
-		load->out, magick7->all_frames, magick7->density, magick7->page ) )
+	/* We'd love to use PingBlob() here but sadly GetPixelChannels() is
+	 * always zero after Ping, so we are forced to use ReadImage().
+	 *
+	 * See:
+	 *
+	 * http://www.imagemagick.org/discourse-server/viewtopic.php?f=2&t=30043
+	 */
+	magick7->image = BlobToImage( magick7->image_info, 
+		magick7_buffer->buf->data, magick7_buffer->buf->length,
+		magick7->exception );
+	if( !magick7->image ) {
+		vips_foreign_load_magick7_error( magick7 ); 
 		return( -1 );
+	}
+
+	if( vips_foreign_load_magick7_header( magick7 ) )
+		return( -1 ); 

 	return( 0 );
 }