new-style jpeg save works

ChangeLog

@@ -50,6 +50,8 @@
 - unlink temps on rewind on *nix, less likely to leave temps on a crash
 - added complex conj as a basic operation
 - rect/polar/conj work o any format, not just complex
+- new VipsFile system for load/save in image formats
+- options now introspectable, try "vips jpegsave"
 
 12/10/11 started 7.26.6
 - NOCACHE was not being set correctly on OS X causing performance 

TODO

@@ -1,6 +1,11 @@
-- try jpeg save
+- get image.c using the new system for vips_image_new_from_file()
 
-  get image.c using the new system
+  CLI stuff image->new_from_string() needs to use new system too, chcek this
+
+  savers need it too ... delayed save system and also
+  image->output_to_arg()
+
+  make compat wrappers for old im_jpeg2vips() and im_vips2jpeg()
 
   move format/* to deprecated
 

libvips/colour/colour.c

@@ -761,3 +761,126 @@ im_col_dE00( float L1, float a1, float b1,
 	return( dE00 );
 }
 
+/* Quick hack wrappers for common colour functions in the new style.
+ */
+
+int
+vips_LabQ2disp( VipsImage *in, VipsImage **out, 
+	struct im_col_display *disp, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, disp );
+	result = vips_call_split( "im_LabQ2disp", ap, in, out, disp );
+	va_end( ap );
+
+	return( result );
+}
+
+int
+vips_rad2float( VipsImage *in, VipsImage **out, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, out );
+	result = vips_call_split( "im_rad2float", ap, in, out );
+	va_end( ap );
+
+	return( result );
+}
+
+int
+vips_LabS2LabQ( VipsImage *in, VipsImage **out, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, out );
+	result = vips_call_split( "im_LabS2LabQ", ap, in, out );
+	va_end( ap );
+
+	return( result );
+}
+
+int
+vips_LabQ2Lab( VipsImage *in, VipsImage **out, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, out );
+	result = vips_call_split( "im_LabQ2Lab", ap, in, out );
+	va_end( ap );
+
+	return( result );
+}
+
+int
+vips_LCh2Lab( VipsImage *in, VipsImage **out, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, out );
+	result = vips_call_split( "im_LCh2Lab", ap, in, out );
+	va_end( ap );
+
+	return( result );
+}
+
+int
+vips_Yxy2Lab( VipsImage *in, VipsImage **out, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, out );
+	result = vips_call_split( "im_Yxy2Lab", ap, in, out );
+	va_end( ap );
+
+	return( result );
+}
+
+int
+vips_UCS2XYZ( VipsImage *in, VipsImage **out, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, out );
+	result = vips_call_split( "im_UCS2XYZ", ap, in, out );
+	va_end( ap );
+
+	return( result );
+}
+
+int
+vips_Lab2XYZ( VipsImage *in, VipsImage **out, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, out );
+	result = vips_call_split( "im_Lab2XYZ", ap, in, out );
+	va_end( ap );
+
+	return( result );
+}
+
+int
+vips_XYZ2disp( VipsImage *in, VipsImage **out, 
+	struct im_col_display *disp, ... )
+{
+	va_list ap;
+	int result;
+
+	va_start( ap, disp );
+	result = vips_call_split( "im_XYZ2disp", ap, in, out, disp );
+	va_end( ap );
+
+	return( result );
+}
+
+

libvips/file/Makefile.am

@@ -4,6 +4,7 @@ libfile_la_SOURCES = \
 	jpeg.c \
 	jpeg.h \
 	jpegload.c \
+	jpegsave.c \
 	file.c 
 
 INCLUDES = -I${top_srcdir}/libvips/include @VIPS_CFLAGS@ @VIPS_INCLUDES@

libvips/file/file.c

@@ -597,18 +597,26 @@ vips_file_find_load( const char *filename )
 
 G_DEFINE_ABSTRACT_TYPE( VipsFileSave, vips_file_save, VIPS_TYPE_FILE );
 
+static void
+vips_file_save_dispose( GObject *gobject )
+{
+	VipsFileSave *save = VIPS_FILE_SAVE( gobject );
+
+	VIPS_UNREF( save->ready );
+
+	G_OBJECT_CLASS( vips_file_save_parent_class )->dispose( gobject );
+}
+
 static void
 vips_file_save_print_class( VipsObjectClass *object_class, VipsBuf *buf )
 {
+	VipsFileSaveClass *class = VIPS_FILE_SAVE_CLASS( object_class );
+
 	VIPS_OBJECT_CLASS( vips_file_save_parent_class )->
 		print_class( object_class, buf );
-	vips_buf_appends( buf, ", " );
 
-	/*
+	vips_buf_appendf( buf, ", %s", 
-	VipsFileSaveClass *class = VIPS_FILE_SAVE_CLASS( object_class );
+		VIPS_ENUM_NICK( VIPS_TYPE_SAVEABLE, class->saveable ) );
-	if( class->save )
-		vips_buf_appends( buf, "save " );
-	 */
 }
 
 /* Generate the saveable image.
@@ -619,196 +627,192 @@ vips_file_convert_saveable( VipsFileSave *save )
 	VipsFileSaveClass *class = VIPS_FILE_SAVE_GET_CLASS( save );
 	VipsImage *in = save->in;
 
+	/* in holds a reference to the output of our chain as we build it.
+	 */
+	g_object_ref( in );
+
 	/* If this is an VIPS_CODING_LABQ, we can go straight to RGB.
 	 */
 	if( in->Coding == VIPS_CODING_LABQ ) {
-		IMAGE *t = im_open_local( out, "conv:1", "p" );
+		VipsImage *out;
-		static void *table = NULL;
 
-		/* Make sure fast LabQ2disp tables are built. 7 is sRGB.
+		if( vips_LabQ2disp( in, &out, im_col_displays( 7 ), NULL ) ) {
-		 */
+			g_object_unref( in );
-		if( !table ) 
+			return( -1 );
-			table = im_LabQ2disp_build_table( NULL, 
-				im_col_displays( 7 ) );
-
-		if( !t || im_LabQ2disp_table( in, t, table ) ) {
-			im_close( out );
-			return( NULL );
 		}
+		g_object_unref( in );
 
-		in = t;
+		in = out;
 	}
 
-	/* If this is an IM_CODING_RAD, we go to float RGB or XYZ. We should
+	/* If this is an VIPS_CODING_RAD, we go to float RGB or XYZ. We should
 	 * probably un-gamma-correct the RGB :(
 	 */
-	if( in->Coding == IM_CODING_RAD ) {
+	if( in->Coding == VIPS_CODING_RAD ) {
-		IMAGE *t;
+		VipsImage *out;
 
-		if( !(t = im_open_local( out, "conv:1", "p" )) || 
+		if( vips_rad2float( in, &out, NULL ) ) {
-			im_rad2float( in, t ) ) {
+			g_object_unref( in );
-			im_close( out );
+			return( -1 );
-			return( NULL );
 		}
+		g_object_unref( in );
 
-		in = t;
+		in = out;
 	}
 
 	/* Get the bands right. 
 	 */
-	if( in->Coding == IM_CODING_NONE ) {
+	if( in->Coding == VIPS_CODING_NONE ) {
-		if( in->Bands == 2 && saveable != IM__RGBA ) {
+		if( in->Bands == 2 && 
-			IMAGE *t = im_open_local( out, "conv:1", "p" );
+			class->saveable != VIPS_SAVEABLE_RGBA ) {
+			VipsImage *out;
 
-			if( !t || im_extract_band( in, t, 0 ) ) {
+			if( vips_extract_band( in, &out, 0, NULL ) ) {
-				im_close( out );
+				g_object_unref( in );
-				return( NULL );
+				return( -1 );
 			}
+			g_object_unref( in );
 
-			in = t;
+			in = out;
 		}
-		else if( in->Bands > 3 && saveable == IM__RGB ) {
+		else if( in->Bands > 3 && 
-			IMAGE *t = im_open_local( out, "conv:1", "p" );
+			class->saveable == VIPS_SAVEABLE_RGB ) {
+			VipsImage *out;
 
-			if( !t ||
+			if( vips_extract_band( in, &out, 0, 
-				im_extract_bands( in, t, 0, 3 ) ) {
+				"n", 3,
-				im_close( out );
+				NULL ) ) {
-				return( NULL );
+				g_object_unref( in );
+				return( -1 );
 			}
+			g_object_unref( in );
 
-			in = t;
+			in = out;
 		}
 		else if( in->Bands > 4 && 
-			(saveable == IM__RGB_CMYK || saveable == IM__RGBA) ) {
+			(class->saveable == VIPS_SAVEABLE_RGB_CMYK || 
-			IMAGE *t = im_open_local( out, "conv:1", "p" );
+			 class->saveable == VIPS_SAVEABLE_RGBA) ) {
+			VipsImage *out;
 
-			if( !t ||
+			if( vips_extract_band( in, &out, 0, 
-				im_extract_bands( in, t, 0, 4 ) ) {
+				"n", 4,
-				im_close( out );
+				NULL ) ) {
-				return( NULL );
+				g_object_unref( in );
+				return( -1 );
 			}
+			g_object_unref( in );
 
-			in = t;
+			in = out;
 		}
 
-		/* Else we have saveable IM__ANY and we don't chop bands down.
+		/* Else we have VIPS_SAVEABLE_ANY and we don't chop bands down.
 		 */
 	}
 
 	/* Interpret the Type field for colorimetric images.
 	 */
-	if( in->Bands == 3 && in->BandFmt == IM_BANDFMT_SHORT && 
+	if( in->Bands == 3 && 
-		in->Type == IM_TYPE_LABS ) {
+		in->BandFmt == VIPS_FORMAT_SHORT && 
-		IMAGE *t = im_open_local( out, "conv:1", "p" );
+		in->Type == VIPS_INTERPRETATION_LABS ) {
+		VipsImage *out;
 
-		if( !t || im_LabS2LabQ( in, t ) ) {
+		if( vips_LabS2LabQ( in, &out, NULL ) ) {
-			im_close( out );
+			g_object_unref( in );
-			return( NULL );
+			return( -1 );
 		}
+		g_object_unref( in );
 
-		in = t;
+		in = out;
 	}
 
-	if( in->Coding == IM_CODING_LABQ ) {
+	if( in->Coding == VIPS_CODING_LABQ ) {
-		IMAGE *t = im_open_local( out, "conv:1", "p" );
+		VipsImage *out;
 
-		if( !t || im_LabQ2Lab( in, t ) ) {
+		if( vips_LabQ2Lab( in, &out, NULL ) ) {
-			im_close( out );
+			g_object_unref( in );
-			return( NULL );
+			return( -1 );
 		}
+		g_object_unref( in );
 
-		in = t;
+		in = out;
 	}
 
-	if( in->Coding != IM_CODING_NONE ) {
+	if( in->Coding != VIPS_CODING_NONE ) {
-		im_close( out );
+		g_object_unref( in );
-		return( NULL );
+		return( -1 );
 	}
 
-	if( in->Bands == 3 && in->Type == IM_TYPE_LCH ) {
+	if( in->Bands == 3 && 
-		IMAGE *t[2];
+		in->Type == VIPS_INTERPRETATION_LCH ) {
+		VipsImage *out;
 
-                if( im_open_local_array( out, t, 2, "conv-1", "p" ) ||
+                if( vips_LCh2Lab( in, &out, NULL ) ) {
-			im_clip2fmt( in, t[0], IM_BANDFMT_FLOAT ) ||
+			g_object_unref( in );
-			im_LCh2Lab( t[0], t[1] ) ) {
+			return( -1 );
-			im_close( out );
-			return( NULL );
 		}
+		g_object_unref( in );
 
-		in = t[1];
+		in = out;
 	}
 
-	if( in->Bands == 3 && in->Type == IM_TYPE_YXY ) {
+	if( in->Bands == 3 && 
-		IMAGE *t[2];
+		in->Type == VIPS_INTERPRETATION_YXY ) {
+		VipsImage *out;
 
-                if( im_open_local_array( out, t, 2, "conv-1", "p" ) ||
+                if( vips_Yxy2Lab( in, &out, NULL ) ) {
-			im_clip2fmt( in, t[0], IM_BANDFMT_FLOAT ) ||
+			g_object_unref( in );
-			im_Yxy2XYZ( t[0], t[1] ) ) {
+			return( -1 );
-			im_close( out );
-			return( NULL );
 		}
+		g_object_unref( in );
 
-		in = t[1];
+		in = out;
 	}
 
-	if( in->Bands == 3 && in->Type == IM_TYPE_UCS ) {
+	if( in->Bands == 3 && 
-		IMAGE *t[2];
+		in->Type == VIPS_INTERPRETATION_UCS ) {
+		VipsImage *out;
 
-                if( im_open_local_array( out, t, 2, "conv-1", "p" ) ||
+                if( vips_UCS2XYZ( in, &out, NULL ) ) {
-			im_clip2fmt( in, t[0], IM_BANDFMT_FLOAT ) ||
+			g_object_unref( in );
-			im_UCS2XYZ( t[0], t[1] ) ) {
+			return( -1 );
-			im_close( out );
-			return( NULL );
 		}
+		g_object_unref( in );
 
-		in = t[1];
+		in = out;
 	}
 
-	if( in->Bands == 3 && in->Type == IM_TYPE_LAB ) {
+	if( in->Bands == 3 && 
-		IMAGE *t[2];
+		in->Type == VIPS_INTERPRETATION_LAB ) {
+		VipsImage *out;
 
-                if( im_open_local_array( out, t, 2, "conv-1", "p" ) ||
+		if( vips_XYZ2disp( in, &out, im_col_displays( 7 ), NULL ) ) {
-			im_clip2fmt( in, t[0], IM_BANDFMT_FLOAT ) ||
+			g_object_unref( in );
-			im_Lab2XYZ( t[0], t[1] ) ) {
+			return( -1 );
-			im_close( out );
-			return( NULL );
 		}
+		g_object_unref( in );
 
-		in = t[1];
+		in = out;
-	}
-
-	if( in->Bands == 3 && in->Type == IM_TYPE_XYZ ) {
-		IMAGE *t[2];
-
-                if( im_open_local_array( out, t, 2, "conv-1", "p" ) ||
-			im_clip2fmt( in, t[0], IM_BANDFMT_FLOAT ) ||
-			im_XYZ2disp( t[0], t[1], im_col_displays( 7 ) ) ) {
-			im_close( out );
-			return( NULL );
-		}
-
-		in = t[1];
 	}
 
 	/* Cast to the output format.
 	 */
 	{
-		IMAGE *t = im_open_local( out, "conv:1", "p" );
+		VipsImage *out;
 
-		if( !t || im_clip2fmt( in, t, format_table[in->BandFmt] ) ) {
+		if( vips_cast( in, &out, 
-			im_close( out );
+			class->format_table[in->BandFmt], NULL ) ) {
-			return( NULL );
+			g_object_unref( in );
+			return( -1 );
 		}
+		g_object_unref( in );
 
-		in = t;
+		in = out;
 	}
 
-	if( im_copy( in, out ) ) {
+	VIPS_UNREF( save->ready );
-		im_close( out );
+	save->ready = in;
-		return( NULL );
-	}
 
-	return( out );
+	return( 0 );
 }
 
 static int
@@ -835,6 +839,7 @@ vips_file_save_class_init( VipsFileSaveClass *class )
 	GObjectClass *gobject_class = G_OBJECT_CLASS( class );
 	VipsObjectClass *object_class = (VipsObjectClass *) class;
 
+	gobject_class->dispose = vips_file_save_dispose;
 	gobject_class->set_property = vips_object_set_property;
 	gobject_class->get_property = vips_object_get_property;
 
@@ -964,8 +969,10 @@ void
 vips_file_operation_init( void )
 {
 	extern GType vips_file_load_jpeg_get_type( void ); 
+	extern GType vips_file_save_jpeg_get_type( void ); 
 
 #ifdef HAVE_JPEG
 	vips_file_load_jpeg_get_type(); 
+	vips_file_save_jpeg_get_type(); 
 #endif /*HAVE_JPEG*/
 }

libvips/file/jpegsave.c

@@ -126,6 +126,25 @@
 typedef struct _VipsFileSaveJpeg {
 	VipsFileSave parent_object;
 
+	/* Quality factor.
+	 */
+	int Q;
+
+	/* Profile to embed .. "none" means don't attach a profile.
+	 */
+	char *profile;
+
+	/* Stuff we track during write.
+	 */
+	struct jpeg_compress_struct cinfo;
+        ErrorManager eman;
+	JSAMPROW *row_pointer;
+	char *profile_bytes;
+	unsigned int profile_length;
+
+	/* We sometimes need to invert the image before saving.
+	 */
+	VipsImage *inverted;
 
 } VipsFileSaveJpeg;
 
@@ -133,36 +152,710 @@ typedef VipsFileSaveClass VipsFileSaveJpegClass;
 
 G_DEFINE_TYPE( VipsFileSaveJpeg, vips_file_save_jpeg, VIPS_TYPE_FILE_SAVE );
 
-static int
+static void
-vips_file_load_save_build( VipsObject *object )
+vips_file_save_jpeg_dispose( GObject *gobject )
 {
+	VipsFileSaveJpeg *save = (VipsFileSaveJpeg *) gobject;
+
+	VIPS_UNREF( save->inverted );
+
+	G_OBJECT_CLASS( vips_file_save_jpeg_parent_class )->dispose( gobject );
+}
+
+static void
+vips_file_save_jpeg_finalize( GObject *gobject )
+{
+	VipsFileSaveJpeg *save = (VipsFileSaveJpeg *) gobject;
+
+	jpeg_destroy_compress( &save->cinfo );
+	VIPS_FREEF( fclose, save->eman.fp );
+	VIPS_FREE( save->row_pointer );
+	VIPS_FREE( save->profile_bytes );
+
+	G_OBJECT_CLASS( vips_file_save_jpeg_parent_class )->finalize( gobject );
+}
+
+#ifdef HAVE_EXIF
+static void
+vips_exif_set_int( ExifData *ed, 
+	ExifEntry *entry, unsigned long component, void *data )
+{
+	int value = *((int *) data);
+
+	ExifByteOrder bo;
+	size_t sizeof_component;
+	size_t offset = component;
+
+	if( entry->components <= component ) {
+		VIPS_DEBUG_MSG( "vips_exif_set_int: too few components\n" );
+		return;
+	}
+
+	/* Wait until after the component check to make sure we cant get /0.
+	 */
+	bo = exif_data_get_byte_order( ed );
+	sizeof_component = entry->size / entry->components;
+	offset = component * sizeof_component;
+
+	VIPS_DEBUG_MSG( "vips_exif_set_int: %s = %d\n",
+		exif_tag_get_title( entry->tag ), value );
+
+	if( entry->format == EXIF_FORMAT_SHORT ) 
+		exif_set_short( entry->data + offset, bo, value );
+	else if( entry->format == EXIF_FORMAT_SSHORT ) 
+		exif_set_sshort( entry->data + offset, bo, value );
+	else if( entry->format == EXIF_FORMAT_LONG ) 
+		exif_set_long( entry->data + offset, bo, value );
+	else if( entry->format == EXIF_FORMAT_SLONG ) 
+		exif_set_slong( entry->data + offset, bo, value );
+}
+
+static void
+vips_exif_set_double( ExifData *ed, 
+	ExifEntry *entry, unsigned long component, void *data )
+{
+	double value = *((double *) data);
+
+	ExifByteOrder bo;
+	size_t sizeof_component;
+	size_t offset;
+
+	if( entry->components <= component ) {
+		VIPS_DEBUG_MSG( "vips_exif_set_int: too few components\n" );
+		return;
+	}
+
+	/* Wait until after the component check to make sure we cant get /0.
+	 */
+	bo = exif_data_get_byte_order( ed );
+	sizeof_component = entry->size / entry->components;
+	offset = component * sizeof_component;
+
+	VIPS_DEBUG_MSG( "vips_exif_set_double: %s = %g\n",
+		exif_tag_get_title( entry->tag ), value );
+
+	if( entry->format == EXIF_FORMAT_RATIONAL ) {
+		ExifRational rational;
+		unsigned int scale;
+
+		/* We scale up to fill uint32, then set that as the
+		 * denominator. Try to avoid generating 0.
+		 */
+		scale = (int) ((UINT_MAX - 1000) / value);
+		scale = scale == 0 ? 1 : scale;
+		rational.numerator = value * scale;
+		rational.denominator = scale;
+
+		exif_set_rational( entry->data + offset, bo, rational );
+	}
+	else if( entry->format == EXIF_FORMAT_SRATIONAL ) {
+		ExifSRational rational;
+		int scale;
+
+		scale = (int) ((INT_MAX - 1000) / value);
+		scale = scale == 0 ? 1 : scale;
+		rational.numerator = value * scale;
+		rational.denominator = scale;
+
+		exif_set_srational( entry->data + offset, bo, rational );
+	}
+}
+
+typedef void (*write_fn)( ExifData *ed, 
+	ExifEntry *entry, unsigned long component, void *data );
+
+/* Write a component in a tag everywhere it appears.
+ */
+static int
+write_tag( ExifData *ed, 
+	ExifTag tag, ExifFormat format, write_fn fn, void *data )
+{
+	int found;
+	int i;
+
+	found = 0;
+	for( i = 0; i < EXIF_IFD_COUNT; i++ ) {
+		ExifEntry *entry;
+
+		if( (entry = exif_content_get_entry( ed->ifd[i], tag )) &&
+			entry->format == format ) {
+			fn( ed, entry, 0, data );
+			found = 1;
+		}
+	}
+
+	if( !found ) {
+		/* There was no tag we could update ... make one in ifd[0].
+		 */
+		ExifEntry *entry;
+
+		entry = exif_entry_new();
+
+		/* tag must be set before calling exif_content_add_entry.
+		 */
+		entry->tag = tag; 
+
+		exif_content_add_entry( ed->ifd[0], entry );
+		exif_entry_initialize( entry, tag );
+		exif_entry_unref( entry );
+
+		fn( ed, entry, 0, data );
+	}
+
+	return( 0 );
+}
+
+/* This is different, we set the xres/yres from the vips header rather than
+ * from the exif tags on the image metadata.
+ */
+static int
+set_exif_resolution( ExifData *ed, VipsImage *im )
+{
+	double xres, yres;
+	int unit;
+
+	/* Always save as inches - more progs support it for read.
+	 */
+	xres = im->Xres * 25.4;
+	yres = im->Yres * 25.4;
+	unit = 2;
+
+	if( write_tag( ed, EXIF_TAG_X_RESOLUTION, EXIF_FORMAT_RATIONAL, 
+		vips_exif_set_double, (void *) &xres ) ||
+		write_tag( ed, EXIF_TAG_Y_RESOLUTION, EXIF_FORMAT_RATIONAL, 
+			vips_exif_set_double, (void *) &yres ) ||
+		write_tag( ed, EXIF_TAG_RESOLUTION_UNIT, EXIF_FORMAT_SHORT, 
+			vips_exif_set_int, (void *) &unit ) ) {
+		vips_error( "VipsFileSaveJpeg", 
+			"%s", _( "error setting JPEG resolution" ) );
+		return( -1 );
+	}
+
+	return( 0 );
+}
+
+/* See also vips_exif_to_s() ... keep in sync.
+ */
+static void
+vips_exif_from_s( ExifData *ed, ExifEntry *entry, const char *value )
+{
+	unsigned long i;
+	const char *p;
+
+	if( entry->format != EXIF_FORMAT_SHORT &&
+		entry->format != EXIF_FORMAT_SSHORT &&
+		entry->format != EXIF_FORMAT_LONG &&
+		entry->format != EXIF_FORMAT_SLONG &&
+		entry->format != EXIF_FORMAT_RATIONAL &&
+		entry->format != EXIF_FORMAT_SRATIONAL )
+		return;
+	if( entry->components >= 10 )
+		return;
+
+	/* Skip any leading spaces.
+	 */
+	p = value;
+	while( *p == ' ' )
+		p += 1;
+
+	for( i = 0; i < entry->components; i++ ) {
+		if( entry->format == EXIF_FORMAT_SHORT || 
+			entry->format == EXIF_FORMAT_SSHORT || 
+			entry->format == EXIF_FORMAT_LONG || 
+			entry->format == EXIF_FORMAT_SLONG ) {
+			int value = atof( p );
+
+			vips_exif_set_int( ed, entry, i, &value );
+		}
+		else if( entry->format == EXIF_FORMAT_RATIONAL || 
+			entry->format == EXIF_FORMAT_SRATIONAL ) {
+			double value = g_ascii_strtod( p, NULL );
+
+			vips_exif_set_double( ed, entry, i, &value );
+		}
+
+		/* Skip to the next set of spaces, then to the beginning of
+		 * the next item.
+		 */
+		while( *p && *p != ' ' )
+			p += 1;
+		while( *p == ' ' )
+			p += 1;
+		if( !*p )
+			break;
+	}
+}
+
+typedef struct _VipsExif {
+	VipsImage *image;
+	ExifData *ed;
+} VipsExif;
+
+static void
+vips_exif_update_entry( ExifEntry *entry, VipsExif *ve )
+{
+	char name[256];
+	char *value;
+
+	vips_snprintf( name, 256, "exif-%s", exif_tag_get_title( entry->tag ) );
+	if( !vips_image_get_string( ve->image, name, &value ) )
+		vips_exif_from_s( ve->ed, entry, value ); 
+}
+
+static void
+vips_exif_update_content( ExifContent *content, VipsExif *ve )
+{
+        exif_content_foreach_entry( content, 
+		(ExifContentForeachEntryFunc) vips_exif_update_entry, ve );
+}
+
+static void
+vips_exif_update( ExifData *ed, VipsImage *image )
+{
+	VipsExif ve;
+
+	VIPS_DEBUG_MSG( "vips_exif_update: \n" );
+
+	ve.image = image;
+	ve.ed = ed;
+	exif_data_foreach_content( ed, 
+		(ExifDataForeachContentFunc) vips_exif_update_content, &ve );
+}
+
+
+#endif /*HAVE_EXIF*/
+
+static int
+write_exif( VipsFileSaveJpeg *jpeg )
+{
+	VipsFileSave *save = (VipsFileSave *) jpeg;
+
+	unsigned char *data;
+	size_t data_length;
+	unsigned int idl;
+#ifdef HAVE_EXIF
+	ExifData *ed;
+
+	/* Either parse from the embedded EXIF, or if there's none, make
+	 * some fresh EXIF we can write the resolution to.
+	 */
+	if( vips_image_get_typeof( save->in, VIPS_META_EXIF_NAME ) ) {
+		if( vips_image_get_blob( save->in, VIPS_META_EXIF_NAME, 
+			(void *) &data, &data_length ) )
+			return( -1 );
+
+		if( !(ed = exif_data_new_from_data( data, data_length )) )
+			return( -1 );
+	}
+	else  {
+		ed = exif_data_new();
+
+		exif_data_set_option( ed, 
+			EXIF_DATA_OPTION_FOLLOW_SPECIFICATION );
+		exif_data_set_data_type( ed, EXIF_DATA_TYPE_COMPRESSED );
+		exif_data_set_byte_order( ed, EXIF_BYTE_ORDER_INTEL );
+	
+		/* Create the mandatory EXIF fields with default data.
+		 */
+		exif_data_fix( ed );
+	}
+
+	/* Update EXIF tags from the image metadata.
+	 */
+	vips_exif_update( ed, save->in );
+
+	/* Update EXIF resolution from the vips image header..
+	 */
+	if( set_exif_resolution( ed, save->in ) ) {
+		exif_data_free( ed );
+		return( -1 );
+	}
+
+	/* Reserialise and write. exif_data_save_data() returns an int for some
+	 * reason.
+	 */
+	exif_data_save_data( ed, &data, &idl );
+	if( !idl ) {
+		vips_error( "VipsFileSaveJpeg", 
+			"%s", _( "error saving EXIF" ) );
+		exif_data_free( ed );
+		return( -1 );
+	}
+	data_length = idl;
+
+#ifdef DEBUG
+	printf( "jpegsave: attaching %zd bytes of EXIF\n", data_length  );
+#endif /*DEBUG*/
+
+	exif_data_free( ed );
+	jpeg_write_marker( &jpeg->cinfo, JPEG_APP0 + 1, data, data_length );
+	free( data );
+#else /*!HAVE_EXIF*/
+	/* No libexif ... just copy the embedded EXIF over.
+	 */
+	if( vips_image_get_typeof( save->in, VIPS_META_EXIF_NAME ) ) {
+		if( vips_image_get_blob( save->in, VIPS_META_EXIF_NAME, 
+			(void *) &data, &data_length ) )
+			return( -1 );
+
+#ifdef DEBUG
+		printf( "jpegsave: attaching %zd bytes of EXIF\n", 
+			data_length  );
+#endif /*DEBUG*/
+
+		jpeg_write_marker( &jpeg->cinfo, JPEG_APP0 + 1, 
+			data, data_length );
+	}
+#endif /*!HAVE_EXIF*/
+
+	return( 0 );
+}
+
+static int
+write_xmp( VipsFileSaveJpeg *jpeg )
+{
+	VipsFileSave *save = (VipsFileSave *) jpeg;
+
+	unsigned char *data;
+	size_t data_length;
+
+	/* No libexif ... just copy the embedded EXIF over.
+	 */
+	if( vips_image_get_typeof( save->in, VIPS_META_XMP_NAME ) ) {
+		if( vips_image_get_blob( save->in, VIPS_META_XMP_NAME, 
+			(void *) &data, &data_length ) )
+			return( -1 );
+
+#ifdef DEBUG
+		printf( "jpegsave: attaching %zd bytes of XMP\n", 
+			data_length  );
+#endif /*DEBUG*/
+
+		jpeg_write_marker( &jpeg->cinfo, JPEG_APP0 + 1, 
+			data, data_length );
+	}
+
+	return( 0 );
+}
+
+/* ICC writer from lcms, slight tweaks.
+ */
+
+#define ICC_MARKER  (JPEG_APP0 + 2)     /* JPEG marker code for ICC */
+#define ICC_OVERHEAD_LEN  14            /* size of non-profile data in APP2 */
+#define MAX_BYTES_IN_MARKER  65533      /* maximum data len of a JPEG marker */
+#define MAX_DATA_BYTES_IN_MARKER  (MAX_BYTES_IN_MARKER - ICC_OVERHEAD_LEN)
+
+/*
+ * This routine writes the given ICC profile data into a JPEG file.
+ * It *must* be called AFTER calling jpeg_start_compress() and BEFORE
+ * the first call to jpeg_write_scanlines().
+ * (This ordering ensures that the APP2 marker(s) will appear after the
+ * SOI and JFIF or Adobe markers, but before all else.)
+ */
+
+static void
+write_profile_data (j_compress_ptr cinfo,
+                   const JOCTET *icc_data_ptr,
+                   unsigned int icc_data_len)
+{
+  unsigned int num_markers;     /* total number of markers we'll write */
+  int cur_marker = 1;           /* per spec, counting starts at 1 */
+  unsigned int length;          /* number of bytes to write in this marker */
+
+  /* rounding up will fail for length == 0 */
+  g_assert( icc_data_len > 0 );
+
+  /* Calculate the number of markers we'll need, rounding up of course */
+  num_markers = (icc_data_len + MAX_DATA_BYTES_IN_MARKER - 1) / 
+	  MAX_DATA_BYTES_IN_MARKER;
+
+  while (icc_data_len > 0) {
+    /* length of profile to put in this marker */
+    length = icc_data_len;
+    if (length > MAX_DATA_BYTES_IN_MARKER)
+      length = MAX_DATA_BYTES_IN_MARKER;
+    icc_data_len -= length;
+
+    /* Write the JPEG marker header (APP2 code and marker length) */
+    jpeg_write_m_header(cinfo, ICC_MARKER,
+                        (unsigned int) (length + ICC_OVERHEAD_LEN));
+
+    /* Write the marker identifying string "ICC_PROFILE" (null-terminated).
+     * We code it in this less-than-transparent way so that the code works
+     * even if the local character set is not ASCII.
+     */
+    jpeg_write_m_byte(cinfo, 0x49);
+    jpeg_write_m_byte(cinfo, 0x43);
+    jpeg_write_m_byte(cinfo, 0x43);
+    jpeg_write_m_byte(cinfo, 0x5F);
+    jpeg_write_m_byte(cinfo, 0x50);
+    jpeg_write_m_byte(cinfo, 0x52);
+    jpeg_write_m_byte(cinfo, 0x4F);
+    jpeg_write_m_byte(cinfo, 0x46);
+    jpeg_write_m_byte(cinfo, 0x49);
+    jpeg_write_m_byte(cinfo, 0x4C);
+    jpeg_write_m_byte(cinfo, 0x45);
+    jpeg_write_m_byte(cinfo, 0x0);
+
+    /* Add the sequencing info */
+    jpeg_write_m_byte(cinfo, cur_marker);
+    jpeg_write_m_byte(cinfo, (int) num_markers);
+
+    /* Add the profile data */
+    while (length--) {
+      jpeg_write_m_byte(cinfo, *icc_data_ptr);
+      icc_data_ptr++;
+    }
+    cur_marker++;
+  }
+}
+
+/* Write an ICC Profile from a file into the JPEG stream.
+ */
+static int
+write_profile_file( VipsFileSaveJpeg *jpeg, const char *profile )
+{
+	if( !(jpeg->profile_bytes = 
+		vips__file_read_name( profile, VIPS_ICC_DIR, 
+			&jpeg->profile_length )) ) 
+		return( -1 );
+	write_profile_data( &jpeg->cinfo, 
+		(JOCTET *) jpeg->profile_bytes, jpeg->profile_length );
+
+#ifdef DEBUG
+	printf( "jpegsave: attached profile \"%s\"\n", profile );
+#endif /*DEBUG*/
+
+	return( 0 );
+}
+
+static int
+write_profile_meta( VipsFileSaveJpeg *jpeg )
+{
+	VipsFileSave *save = (VipsFileSave *) jpeg;
+
+	void *data;
+	size_t data_length;
+
+	if( vips_image_get_blob( save->in, VIPS_META_ICC_NAME, 
+		&data, &data_length ) )
+		return( -1 );
+
+	write_profile_data( &jpeg->cinfo, data, data_length );
+
+#ifdef DEBUG
+	printf( "jpegsave: attached %zd byte profile from VIPS header\n",
+		data_length );
+#endif /*DEBUG*/
+
+	return( 0 );
+}
+
+static int
+write_jpeg_block( VipsRegion *region, VipsRect *area, void *a )
+{
+	VipsFileSaveJpeg *jpeg = (VipsFileSaveJpeg *) a;
+
+	int i;
+
+	for( i = 0; i < area->height; i++ )
+		jpeg->row_pointer[i] = (JSAMPROW) 
+			VIPS_REGION_ADDR( region, 0, area->top + i );
+
+	/* We are running in a background thread. We need to catch any
+	 * longjmp()s from jpeg_write_scanlines() here.
+	 */
+	if( setjmp( jpeg->eman.jmp ) ) 
+		return( -1 );
+
+	jpeg_write_scanlines( &jpeg->cinfo, jpeg->row_pointer, area->height );
+
+	return( 0 );
+}
+
+/* Write a VIPS image to a JPEG compress struct.
+ */
+static int
+vips_file_save_jpeg_write( VipsFileSaveJpeg *jpeg )
+{
+	VipsFileSave *save = (VipsFileSave *) jpeg;
+
+	VipsImage *in;
+	J_COLOR_SPACE space;
+
+	/* The image we'll be writing ... can change, see CMYK.
+	 */
+	in = save->ready;
+
+	/* Should have been converted for save.
+	 */
+        g_assert( in->BandFmt == VIPS_FORMAT_UCHAR );
+	g_assert( in->Coding == VIPS_CODING_NONE );
+        g_assert( in->Bands == 1 || in->Bands == 3 || in->Bands == 4 );
+
+        /* Check input image.
+         */
+	if( vips_image_pio_input( in ) )
+		return( -1 );
+
+	/* Set compression parameters.
+	 */
+        jpeg->cinfo.image_width = in->Xsize;
+        jpeg->cinfo.image_height = in->Ysize;
+	jpeg->cinfo.input_components = in->Bands;
+	if( in->Bands == 4 && 
+		in->Type == VIPS_INTERPRETATION_CMYK ) {
+		space = JCS_CMYK;
+		/* IJG always sets an Adobe marker, so we should invert CMYK.
+		 */
+		if( vips_invert( in, &jpeg->inverted, NULL ) ) 
+			return( -1 );
+		in = jpeg->inverted;
+	}
+	else if( in->Bands == 3 )
+		space = JCS_RGB;
+	else if( in->Bands == 1 )
+		space = JCS_GRAYSCALE;
+	else 
+		/* Use luminance compression for all channels.
+		 */
+		space = JCS_UNKNOWN;
+	jpeg->cinfo.in_color_space = space; 
+
+	/* Build VIPS output stuff now we know the image we'll be writing.
+	 */
+	if( !(jpeg->row_pointer = VIPS_ARRAY( NULL, in->Ysize, JSAMPROW )) )
+		return( -1 );
+
+	/* Rest to default. 
+	 */
+        jpeg_set_defaults( &jpeg->cinfo );
+        jpeg_set_quality( &jpeg->cinfo, jpeg->Q, TRUE );
+
+	/* Build compress tables.
+	 */
+	jpeg_start_compress( &jpeg->cinfo, TRUE );
+
+	/* Write any APP markers we need.
+	 */
+	if( write_exif( jpeg ) )
+		return( -1 );
+
+	if( write_xmp( jpeg ) )
+		return( -1 );
+
+	/* A profile supplied as an argument overrides an embedded profile.
+	 * "none" means don't attach a profile.
+	 */
+	if( jpeg->profile && 
+		strcmp( jpeg->profile, "none" ) != 0 &&
+		write_profile_file( jpeg, jpeg->profile ) )
+		return( -1 );
+	if( !jpeg->profile && 
+		vips_image_get_typeof( in, VIPS_META_ICC_NAME ) && 
+		write_profile_meta( jpeg ) )
+		return( -1 );
+
+	/* Write data. Note that the write function grabs the longjmp()!
+	 */
+	if( vips_sink_disc( in, write_jpeg_block, jpeg ) )
+		return( -1 );
+
+	/* We have to reinstate the setjmp() before we jpeg_finish_compress().
+	 */
+	if( setjmp( jpeg->eman.jmp ) ) 
+		return( -1 );
+
+	jpeg_finish_compress( &jpeg->cinfo );
+
+	return( 0 );
+}
+
+static int
+vips_file_save_jpeg_build( VipsObject *object )
+{
+	VipsFile *file = (VipsFile *) object;
 	VipsFileSaveJpeg *jpeg = (VipsFileSaveJpeg *) object;
 
 	if( VIPS_OBJECT_CLASS( vips_file_save_jpeg_parent_class )->
 		build( object ) )
 		return( -1 );
 
+	if( setjmp( jpeg->eman.jmp ) ) 
+		/* Here for longjmp() from vips__new_error_exit().
+		 */
+		return( -1 );
+
+	/* Can't do this in init, has to be after we've made the
+	 * setjmp().
+	 */
+        jpeg_create_compress( &jpeg->cinfo );
+
+	/* Make output.
+	 */
+        if( !(jpeg->eman.fp = vips__file_open_write( file->filename, FALSE )) ) 
+                return( -1 );
+        jpeg_stdio_dest( &jpeg->cinfo, jpeg->eman.fp );
+
+	/* Convert!
+	 */
+	if( vips_file_save_jpeg_write( jpeg ) ) 
+		return( -1 );
+
 	return( 0 );
 }
 
+#define UC VIPS_FORMAT_UCHAR
+
+/* Type promotion for save ... just always go to uchar.
+ */
+static int bandfmt_jpeg[10] = {
+/* UC  C   US  S   UI  I   F   X   D   DX */
+   UC, UC, UC, UC, UC, UC, UC, UC, UC, UC
+};
+
 static void
-vips_file_save_jpeg_class_init( VipsFileLoadJpegClass *class )
+vips_file_save_jpeg_class_init( VipsFileSaveJpegClass *class )
 {
 	GObjectClass *gobject_class = G_OBJECT_CLASS( class );
 	VipsObjectClass *object_class = (VipsObjectClass *) class;
 	VipsFileSaveClass *save_class = (VipsFileSaveClass *) class;
 
+	gobject_class->dispose = vips_file_save_jpeg_dispose;
+	gobject_class->finalize = vips_file_save_jpeg_finalize;
 	gobject_class->set_property = vips_object_set_property;
 	gobject_class->get_property = vips_object_get_property;
 
 	object_class->nickname = "jpegsave";
-	object_class->description = _( "save jpeg from file" );
+	object_class->description = _( "save jpeg to file" );
 	object_class->build = vips_file_save_jpeg_build;
 
+	save_class->saveable = VIPS_SAVEABLE_RGB_CMYK;
+	save_class->format_table = bandfmt_jpeg;
+
+	VIPS_ARG_INT( class, "Q", 10, 
+		_( "Q" ), 
+		_( "Q factor" ),
+		VIPS_ARGUMENT_OPTIONAL_INPUT,
+		G_STRUCT_OFFSET( VipsFileSaveJpeg, Q ),
+		1, 100, 75 );
+
+	VIPS_ARG_STRING( class, "profile", 11, 
+		_( "profile" ), 
+		_( "ICC profile to embed" ),
+		VIPS_ARGUMENT_OPTIONAL_INPUT,
+		G_STRUCT_OFFSET( VipsFileSaveJpeg, profile ),
+		NULL );
+
 }
 
 static void
 vips_file_save_jpeg_init( VipsFileSaveJpeg *jpeg )
 {
+	jpeg->Q = 75;
+        jpeg->cinfo.err = jpeg_std_error( &jpeg->eman.pub );
+	jpeg->eman.pub.error_exit = vips__new_error_exit;
+	jpeg->eman.pub.output_message = vips__new_output_message;
 }
 

libvips/include/vips/colour.h

@@ -42,6 +42,29 @@
 extern "C" {
 #endif /*__cplusplus*/
 
+struct im_col_display;
+
+int vips_LabQ2disp( VipsImage *in, VipsImage **out, 
+	struct im_col_display *disp, ... )
+	__attribute__((sentinel));
+int vips_rad2float( VipsImage *in, VipsImage **out, ... )
+	__attribute__((sentinel));
+int vips_LabS2LabQ( VipsImage *in, VipsImage **out, ... )
+	__attribute__((sentinel));
+int vips_LabQ2Lab( VipsImage *in, VipsImage **out, ... )
+	__attribute__((sentinel));
+int vips_LCh2Lab( VipsImage *in, VipsImage **out, ... )
+	__attribute__((sentinel));
+int vips_Yxy2Lab( VipsImage *in, VipsImage **out, ... )
+	__attribute__((sentinel));
+int vips_UCS2XYZ( VipsImage *in, VipsImage **out, ... )
+	__attribute__((sentinel));
+int vips_Lab2XYZ( VipsImage *in, VipsImage **out, ... )
+	__attribute__((sentinel));
+int vips_XYZ2disp( VipsImage *in, VipsImage **out, 
+	struct im_col_display *disp, ... )
+	__attribute__((sentinel));
+
 /* Areas under curves for Dxx. 2 degree observer.
  */
 #define IM_D93_X0 (89.7400)

libvips/include/vips/enumtypes.h

@@ -9,6 +9,8 @@ G_BEGIN_DECLS
 /* enumerations from "../../../libvips/include/vips/file.h" */
 GType vips_file_flags_get_type (void) G_GNUC_CONST;
 #define VIPS_TYPE_FILE_FLAGS (vips_file_flags_get_type())
+GType vips_saveable_get_type (void) G_GNUC_CONST;
+#define VIPS_TYPE_SAVEABLE (vips_saveable_get_type())
 /* enumerations from "../../../libvips/include/vips/arithmetic.h" */
 GType vips_operation_math_get_type (void) G_GNUC_CONST;
 #define VIPS_TYPE_OPERATION_MATH (vips_operation_math_get_type())

libvips/include/vips/file.h

@@ -201,7 +201,7 @@ typedef struct _VipsFileSave {
 
 	/* The image converted to a saveable format (eg. 8-bit RGB).
 	 */
-	VipsImage *in;
+	VipsImage *ready;
 
 } VipsFileSave;
 
@@ -216,7 +216,7 @@ typedef struct _VipsFileSaveClass {
 
 	/* How this format treats band formats.
 	 */
-	VipsBandFormat format_table[VIPS_FORMAT_LAST];
+	VipsBandFormat *format_table;
 } VipsFileSaveClass;
 
 GType vips_file_save_get_type( void );

libvips/iofuncs/enumtypes.c

@@ -23,6 +23,26 @@ vips_file_flags_get_type( void )
 
 	return( etype );
 }
+GType
+vips_saveable_get_type( void )
+{
+	static GType etype = 0;
+
+	if( etype == 0 ) {
+		static const GEnumValue values[] = {
+			{VIPS_SAVEABLE_RGB, "VIPS_SAVEABLE_RGB", "rgb"},
+			{VIPS_SAVEABLE_RGBA, "VIPS_SAVEABLE_RGBA", "rgba"},
+			{VIPS_SAVEABLE_RGB_CMYK, "VIPS_SAVEABLE_RGB_CMYK", "rgb-cmyk"},
+			{VIPS_SAVEABLE_ANY, "VIPS_SAVEABLE_ANY", "any"},
+			{VIPS_SAVEABLE_LAST, "VIPS_SAVEABLE_LAST", "last"},
+			{0, NULL, NULL}
+		};
+		
+		etype = g_enum_register_static( "VipsSaveable", values );
+	}
+
+	return( etype );
+}
 /* enumerations from "../../libvips/include/vips/conversion.h" */
 GType
 vips_extend_get_type( void )