Merge pull request #1552 from wix-playground/master

Add force subsample argument to JPEG save

libvips/foreign/jpegsave.c

@@ -73,10 +73,17 @@ typedef struct _VipsForeignSaveJpeg {
 	 */
 	gboolean interlace;
 
-	/* Disable chroma subsampling. 
+	/* Deprecated: Disable chroma subsampling. Use subsample_mode instead.
 	 */
 	gboolean no_subsample;
 
+    /* Select chroma subsampling mode:
+     * auto will disable subsampling for Q >= 90
+     * on will always enable subsampling
+     * off will always disable subsampling
+     */
+    VipsForeignJpegSubsample subsample_mode;
+
 	/* Apply trellis quantisation to each 8x8 block.
 	 */
 	gboolean trellis_quant;
@@ -110,6 +117,24 @@ static int bandfmt_jpeg[10] = {
    UC, UC, UC, UC, UC, UC, UC, UC, UC, UC
 };
 
+static int
+vips_foreign_save_jpeg_build( VipsObject *object )
+{
+    VipsForeignSaveJpeg *jpeg = (VipsForeignSaveJpeg *) object;
+
+    if( VIPS_OBJECT_CLASS( vips_foreign_save_jpeg_parent_class )->
+        build( object ) )
+        return( -1 );
+
+    /* no_subsample is deprecated, but we retain backwards compatibility
+     * new code should use subsample_mode
+     */
+    if( vips_object_argument_isset(object, "no_subsample") )
+        jpeg->subsample_mode = jpeg->no_subsample ? VIPS_FOREIGN_JPEG_SUBSAMPLE_OFF : VIPS_FOREIGN_JPEG_SUBSAMPLE_AUTO;
+
+    return( 0 );
+}
+
 static void
 vips_foreign_save_jpeg_class_init( VipsForeignSaveJpegClass *class )
 {
@@ -123,6 +148,7 @@ vips_foreign_save_jpeg_class_init( VipsForeignSaveJpegClass *class )
 
 	object_class->nickname = "jpegsave_base";
 	object_class->description = _( "save jpeg" );
+    object_class->build = vips_foreign_save_jpeg_build;
 
 	foreign_class->suffs = vips__jpeg_suffs;
 
@@ -162,7 +188,7 @@ vips_foreign_save_jpeg_class_init( VipsForeignSaveJpegClass *class )
 	VIPS_ARG_BOOL( class, "no_subsample", 14,
 		_( "No subsample" ),
 		_( "Disable chroma subsample" ),
-		VIPS_ARGUMENT_OPTIONAL_INPUT,
+		VIPS_ARGUMENT_OPTIONAL_INPUT | VIPS_ARGUMENT_DEPRECATED,
 		G_STRUCT_OFFSET( VipsForeignSaveJpeg, no_subsample ),
 		FALSE );
 
@@ -194,6 +220,13 @@ vips_foreign_save_jpeg_class_init( VipsForeignSaveJpegClass *class )
 		G_STRUCT_OFFSET( VipsForeignSaveJpeg, quant_table ),
 		0, 8, 0 );
 
+    VIPS_ARG_ENUM( class, "subsample_mode", 19,
+            _( "Subsample mode" ),
+            _( "Select chroma subsample operation mode" ),
+            VIPS_ARGUMENT_OPTIONAL_INPUT,
+            G_STRUCT_OFFSET( VipsForeignSaveJpeg, subsample_mode ),
+            VIPS_TYPE_FOREIGN_JPEG_SUBSAMPLE,
+            VIPS_FOREIGN_JPEG_SUBSAMPLE_AUTO );
 }
 
 static void
@@ -228,9 +261,9 @@ vips_foreign_save_jpeg_target_build( VipsObject *object )
 
 	if( vips__jpeg_write_target( save->ready, target->target,
 		jpeg->Q, jpeg->profile, jpeg->optimize_coding, 
-		jpeg->interlace, save->strip, jpeg->no_subsample,
+		jpeg->interlace, save->strip, jpeg->trellis_quant,
-		jpeg->trellis_quant, jpeg->overshoot_deringing,
+		jpeg->overshoot_deringing, jpeg->optimize_scans,
-		jpeg->optimize_scans, jpeg->quant_table ) )
+		jpeg->quant_table, jpeg->subsample_mode ) )
 		return( -1 );
 
 	return( 0 );
@@ -256,7 +289,6 @@ vips_foreign_save_jpeg_target_class_init(
 		VIPS_ARGUMENT_REQUIRED_INPUT, 
 		G_STRUCT_OFFSET( VipsForeignSaveJpegTarget, target ),
 		VIPS_TYPE_TARGET );
-
 }
 
 static void
@@ -295,9 +327,9 @@ vips_foreign_save_jpeg_file_build( VipsObject *object )
 		return( -1 );
 	if( vips__jpeg_write_target( save->ready, target,
 		jpeg->Q, jpeg->profile, jpeg->optimize_coding, 
-		jpeg->interlace, save->strip, jpeg->no_subsample,
+		jpeg->interlace, save->strip, jpeg->trellis_quant,
-		jpeg->trellis_quant, jpeg->overshoot_deringing,
+		jpeg->overshoot_deringing, jpeg->optimize_scans,
-		jpeg->optimize_scans, jpeg->quant_table ) ) {
+		jpeg->quant_table, jpeg->subsample_mode ) ) {
 		VIPS_UNREF( target );
 		return( -1 );
 	}
@@ -365,9 +397,9 @@ vips_foreign_save_jpeg_buffer_build( VipsObject *object )
 
 	if( vips__jpeg_write_target( save->ready, target,
 		jpeg->Q, jpeg->profile, jpeg->optimize_coding, 
-		jpeg->interlace, save->strip, jpeg->no_subsample,
+		jpeg->interlace, save->strip, jpeg->trellis_quant,
-		jpeg->trellis_quant, jpeg->overshoot_deringing,
+		jpeg->overshoot_deringing, jpeg->optimize_scans,
-		jpeg->optimize_scans, jpeg->quant_table ) ) {
+		jpeg->quant_table, jpeg->subsample_mode ) ) {
 		VIPS_UNREF( target );
 		return( -1 );
 	}
@@ -438,9 +470,9 @@ vips_foreign_save_jpeg_mime_build( VipsObject *object )
 
 	if( vips__jpeg_write_target( save->ready, target,
 		jpeg->Q, jpeg->profile, jpeg->optimize_coding, 
-		jpeg->interlace, save->strip, jpeg->no_subsample,
+		jpeg->interlace, save->strip, jpeg->trellis_quant,
-		jpeg->trellis_quant, jpeg->overshoot_deringing,
+		jpeg->overshoot_deringing, jpeg->optimize_scans,
-		jpeg->optimize_scans, jpeg->quant_table ) ) {
+		jpeg->quant_table, jpeg->subsample_mode ) ) {
 		VIPS_UNREF( target );
 		return( -1 );
 	}

libvips/foreign/pforeign.h

@@ -164,9 +164,9 @@ extern const char *vips__jpeg_suffs[];
 int vips__jpeg_write_target( VipsImage *in, VipsTarget *target,
 	int Q, const char *profile, 
 	gboolean optimize_coding, gboolean progressive, gboolean strip,
-	gboolean no_subsample, gboolean trellis_quant,
+	gboolean trellis_quant, gboolean overshoot_deringing,
-	gboolean overshoot_deringing, gboolean optimize_scans, 
+	gboolean optimize_scans, int quant_table,
-	int quant_table );
+	VipsForeignJpegSubsample subsample_mode );
 
 int vips__jpeg_read_source( VipsSource *source, VipsImage *out,
 	gboolean header_only, int shrink, int fail, gboolean autorotate );

libvips/foreign/vips2jpeg.c

@@ -479,8 +479,9 @@ write_jpeg_block( VipsRegion *region, VipsRect *area, void *a )
 static int
 write_vips( Write *write, int qfac, const char *profile, 
 	gboolean optimize_coding, gboolean progressive, gboolean strip, 
-	gboolean no_subsample, gboolean trellis_quant,
+	gboolean trellis_quant, gboolean overshoot_deringing,
-	gboolean overshoot_deringing, gboolean optimize_scans, int quant_table )
+	gboolean optimize_scans, int quant_table,
+    VipsForeignJpegSubsample subsample_mode)
 {
 	VipsImage *in;
 	J_COLOR_SPACE space;
@@ -627,17 +628,24 @@ write_vips( Write *write, int qfac, const char *profile,
 	if( progressive ) 
 		jpeg_simple_progression( &write->cinfo ); 
 
-	/* Turn off chroma subsampling. Follow IM and do it automatically for
+	switch ( subsample_mode ) {
-	 * high Q. 
+    case VIPS_FOREIGN_JPEG_SUBSAMPLE_ON:
-	 */
+            break;
-	if( no_subsample ||
+    case VIPS_FOREIGN_JPEG_SUBSAMPLE_AUTO:
-		qfac >= 90 ) { 
+            /* Turn off chroma subsampling. Follow IM and do it automatically for
-		int i;
+             * high Q.
-
+             */
-		for( i = 0; i < in->Bands; i++ ) { 
+            if( qfac < 90 )
-			write->cinfo.comp_info[i].h_samp_factor = 1;
+                break;
-			write->cinfo.comp_info[i].v_samp_factor = 1;
+    case VIPS_FOREIGN_JPEG_SUBSAMPLE_OFF:
-		}
+    default:
+        {
+            int i;
+            for ( i = 0; i < in->Bands; i++ ) {
+                write->cinfo.comp_info[i].h_samp_factor = 1;
+                write->cinfo.comp_info[i].v_samp_factor = 1;
+            }
+        }
 	}
 
 	/* Don't write the APP0 JFIF headers if we are stripping.
@@ -774,8 +782,9 @@ int
 vips__jpeg_write_target( VipsImage *in, VipsTarget *target,
 	int Q, const char *profile, 
 	gboolean optimize_coding, gboolean progressive,
-	gboolean strip, gboolean no_subsample, gboolean trellis_quant,
+	gboolean strip, gboolean trellis_quant,
-	gboolean overshoot_deringing, gboolean optimize_scans, int quant_table )
+    gboolean overshoot_deringing, gboolean optimize_scans,
+    int quant_table, VipsForeignJpegSubsample subsample_mode)
 {
 	Write *write;
 
@@ -800,9 +809,9 @@ vips__jpeg_write_target( VipsImage *in, VipsTarget *target,
 	/* Convert! Write errors come back here as an error return.
 	 */
 	if( write_vips( write, 
-		Q, profile, optimize_coding, progressive, strip, no_subsample,
+		Q, profile, optimize_coding, progressive, strip,
 		trellis_quant, overshoot_deringing, optimize_scans, 
-		quant_table ) ) {
+		quant_table, subsample_mode ) ) {
 		write_destroy( write );
 		return( -1 );
 	}

libvips/include/vips/enumtypes.h

@@ -58,6 +58,8 @@ GType vips_foreign_flags_get_type (void) G_GNUC_CONST;
 #define VIPS_TYPE_FOREIGN_FLAGS (vips_foreign_flags_get_type())
 GType vips_saveable_get_type (void) G_GNUC_CONST;
 #define VIPS_TYPE_SAVEABLE (vips_saveable_get_type())
+GType vips_foreign_jpeg_subsample_get_type (void) G_GNUC_CONST;
+#define VIPS_TYPE_FOREIGN_JPEG_SUBSAMPLE (vips_foreign_jpeg_subsample_get_type())
 GType vips_foreign_webp_preset_get_type (void) G_GNUC_CONST;
 #define VIPS_TYPE_FOREIGN_WEBP_PRESET (vips_foreign_webp_preset_get_type())
 GType vips_foreign_tiff_compression_get_type (void) G_GNUC_CONST;

libvips/include/vips/foreign.h

@@ -364,6 +364,18 @@ int vips_vipssave( VipsImage *in, const char *filename, ... )
 int vips_openslideload( const char *filename, VipsImage **out, ... )
 	__attribute__((sentinel));
 
+/**
+ * VipsForeignJpegSubsample:
+ * @VIPS_FOREIGN_JPEG_SUBSAMPLE_AUTO: default preset
+ * @VIPS_FOREIGN_JPEG_SUBSAMPLE_ON: always perform subsampling
+ * @VIPS_FOREIGN_JPEG_SUBSAMPLE_OFF: never perform subsampling
+ */
+typedef enum {
+    VIPS_FOREIGN_JPEG_SUBSAMPLE_AUTO,
+    VIPS_FOREIGN_JPEG_SUBSAMPLE_ON,
+    VIPS_FOREIGN_JPEG_SUBSAMPLE_OFF
+} VipsForeignJpegSubsample;
+
 int vips_jpegload( const char *filename, VipsImage **out, ... )
 	__attribute__((sentinel));
 int vips_jpegload_buffer( void *buf, size_t len, VipsImage **out, ... )

libvips/iofuncs/enumtypes.c

@@ -499,6 +499,24 @@ vips_saveable_get_type( void )
 	return( etype );
 }
 GType
+vips_foreign_jpeg_subsample_get_type( void )
+{
+	static GType etype = 0;
+
+	if( etype == 0 ) {
+		static const GEnumValue values[] = {
+			{VIPS_FOREIGN_JPEG_SUBSAMPLE_AUTO, "VIPS_FOREIGN_JPEG_SUBSAMPLE_AUTO", "auto"},
+			{VIPS_FOREIGN_JPEG_SUBSAMPLE_ON, "VIPS_FOREIGN_JPEG_SUBSAMPLE_ON", "on"},
+			{VIPS_FOREIGN_JPEG_SUBSAMPLE_OFF, "VIPS_FOREIGN_JPEG_SUBSAMPLE_OFF", "off"},
+			{0, NULL, NULL}
+		};
+		
+		etype = g_enum_register_static( "VipsForeignJpegSubsample", values );
+	}
+
+	return( etype );
+}
+GType
 vips_foreign_webp_preset_get_type( void )
 {
 	static GType etype = 0;

test/test-suite/test_foreign.py

@@ -254,6 +254,31 @@ class TestForeign:
             # format area at the end
             assert y.startswith("hello world")
 
+    @skip_if_no("jpegload")
+    def test_jpegsave(self):
+        im = pyvips.Image.new_from_file(JPEG_FILE)
+
+        q10 = im.jpegsave_buffer(Q=10)
+        q10_subsample_auto = im.jpegsave_buffer(Q=10, subsample_mode="auto")
+        q10_subsample_on = im.jpegsave_buffer(Q=10, subsample_mode="on")
+        q10_subsample_off = im.jpegsave_buffer(Q=10, subsample_mode="off")
+        
+        q90 = im.jpegsave_buffer(Q=90)
+        q90_subsample_auto = im.jpegsave_buffer(Q=90, subsample_mode="auto")
+        q90_subsample_on = im.jpegsave_buffer(Q=90, subsample_mode="on")
+        q90_subsample_off = im.jpegsave_buffer(Q=90, subsample_mode="off")
+
+        # higher Q should mean a bigger buffer
+        assert len(q90) > len(q10)
+        
+        assert len(q10_subsample_auto) == len(q10) 
+        assert len(q10_subsample_on) == len(q10_subsample_auto)
+        assert len(q10_subsample_off) > len(q10)    
+        
+        assert len(q90_subsample_auto) == len(q90) 
+        assert len(q90_subsample_on) < len(q90) 
+        assert len(q90_subsample_off) == len(q90_subsample_auto)
+
     @skip_if_no("jpegload")
     def test_truncated(self):
         # This should open (there's enough there for the header)