From b1b3c6e9de55d27b1cc6672af20c3fc45d42efde Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Wed, 30 Apr 2014 14:39:50 +0100 Subject: [PATCH 1/4] support 1/2/4 bit palette tiff images with alpha --- ChangeLog | 1 + libvips/foreign/tiff2vips.c | 41 +++++++++++++++++++------------------ 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index e28eef50..10c0ffc7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,7 @@ - added vips_tiffload_buffer() - added vips_foreign_load_buffer(), vips_foreign_save_buffer() - added vips_object_set_from_string() +- support 1/2/4 bit palette tiff images with alpha 6/3/14 started 7.38.6 - grey ramp minimum was wrong diff --git a/libvips/foreign/tiff2vips.c b/libvips/foreign/tiff2vips.c index 19ec69e0..adb09bdd 100644 --- a/libvips/foreign/tiff2vips.c +++ b/libvips/foreign/tiff2vips.c @@ -146,6 +146,8 @@ * - palette images can have an alpha * 22/4/14 * - add read from buffer + * 30/4/14 + * - 1/2/4 bit palette images can have alpha */ /* @@ -707,12 +709,13 @@ typedef struct { gboolean mono; } PaletteRead; -/* Per-scanline process function for palette images. +/* 1/2/4 bit samples with an 8-bit palette. */ static void palette_line_bit( ReadTiff *rtiff, VipsPel *q, VipsPel *p, int n, void *client ) { PaletteRead *read = (PaletteRead *) client; + int samples = rtiff->samples_per_pixel; int bit; VipsPel data; @@ -720,7 +723,7 @@ palette_line_bit( ReadTiff *rtiff, VipsPel *q, VipsPel *p, int n, void *client ) bit = 0; data = 0; - for( x = 0; x < n; x++ ) { + for( x = 0; x < n * samples; x++ ) { int i; if( bit <= 0 ) { @@ -732,20 +735,25 @@ palette_line_bit( ReadTiff *rtiff, VipsPel *q, VipsPel *p, int n, void *client ) data <<= rtiff->bits_per_sample; bit -= rtiff->bits_per_sample; - if( read->mono ) { - q[0] = read->red8[i]; - q += 1; - } - else { - q[0] = read->red8[i]; - q[1] = read->green8[i]; - q[2] = read->blue8[i]; - q += 3; + /* The first band goes through the LUT, subsequent bands are + * left-justified and copied. + */ + if( x % samples == 0 ) { + if( read->mono ) + *q++ = read->red8[i]; + else { + q[0] = read->red8[i]; + q[1] = read->green8[i]; + q[2] = read->blue8[i]; + q += 3; + } } + else + *q++ = i << (8 - rtiff->bits_per_sample); } } -/* The tiff is 8-bit and can have an alpha. +/* 8-bit samples with an 8-bit palette. */ static void palette_line8( ReadTiff *rtiff, VipsPel *q, VipsPel *p, int n, @@ -777,7 +785,7 @@ palette_line8( ReadTiff *rtiff, VipsPel *q, VipsPel *p, int n, } } -/* 16-bit tiff and can have an alpha. +/* 16-bit samples with 16-bit data in the palette. */ static void palette_line16( ReadTiff *rtiff, VipsPel *q, VipsPel *p, int n, @@ -827,13 +835,6 @@ parse_palette( ReadTiff *rtiff, VipsImage *out ) return( -1 ); len = 1 << rtiff->bits_per_sample; - if( rtiff->bits_per_sample < 8 && - rtiff->samples_per_pixel > 1 ) { - vips_error( "tiff2vips", "%s", _( "can't have an alpha for " - "palette images less than 8 bits per sample" ) ); - return( -1 ); - } - if( !(read = VIPS_NEW( out, PaletteRead )) || !(read->red8 = VIPS_ARRAY( out, len, VipsPel )) || !(read->green8 = VIPS_ARRAY( out, len, VipsPel )) || From 7819fde047a3025cabad2a2ee4f66b595432a69e Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 2 May 2014 20:23:29 +0100 Subject: [PATCH 2/4] don't cache vips_system() --- libvips/iofuncs/system.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libvips/iofuncs/system.c b/libvips/iofuncs/system.c index 1082ec1c..335c5f8c 100644 --- a/libvips/iofuncs/system.c +++ b/libvips/iofuncs/system.c @@ -206,6 +206,7 @@ vips_system_class_init( VipsSystemClass *class ) { GObjectClass *gobject_class = G_OBJECT_CLASS( class ); VipsObjectClass *vobject_class = VIPS_OBJECT_CLASS( class ); + VipsOperationClass *operation_class = VIPS_OPERATION_CLASS( class ); gobject_class->dispose = vips_system_dispose; gobject_class->set_property = vips_object_set_property; @@ -215,6 +216,10 @@ vips_system_class_init( VipsSystemClass *class ) vobject_class->description = _( "run an external command" ); vobject_class->build = vips_system_build; + /* Commands can have side-effects, so don't cache them. + */ + operation_class->flags = VIPS_OPERATION_NOCACHE; + VIPS_ARG_BOXED( class, "in", 0, _( "Input" ), _( "Array of input images" ), From c81a12ee00e9f20717da09d2bd9b85ad310a6abe Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 3 May 2014 18:04:25 +0100 Subject: [PATCH 3/4] vips_system() now uses g_spawn_command_line_sync() helps stop stray command windows appearing on Windows, better error msg too --- ChangeLog | 1 + TODO | 5 --- libvips/include/vips/util.h | 2 ++ libvips/iofuncs/system.c | 65 +++++++++++++++++++++++++------------ libvips/iofuncs/util.c | 11 +++++++ 5 files changed, 58 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index 10c0ffc7..a1037bde 100644 --- a/ChangeLog +++ b/ChangeLog @@ -26,6 +26,7 @@ - added vips_foreign_load_buffer(), vips_foreign_save_buffer() - added vips_object_set_from_string() - support 1/2/4 bit palette tiff images with alpha +- vips_system() now uses g_spawn_command_line_sync() 6/3/14 started 7.38.6 - grey ramp minimum was wrong diff --git a/TODO b/TODO index 4b86a3b1..4a80ac3a 100644 --- a/TODO +++ b/TODO @@ -4,12 +4,7 @@ vips_foreign_load_new_from_foreign_sub(), but it splits on ':' ... argh! deprecate this thing and stop ':' split - - - -- support 1/2/4 bit palette tiff with alpha - - can we use postbuild elsewhere? look at use of "preclose" / "written", etc. diff --git a/libvips/include/vips/util.h b/libvips/include/vips/util.h index 4bd3f069..dc559de4 100644 --- a/libvips/include/vips/util.h +++ b/libvips/include/vips/util.h @@ -186,6 +186,8 @@ gboolean vips_ispostfix( const char *a, const char *b ); gboolean vips_isprefix( const char *a, const char *b ); char *vips_break_token( char *str, const char *brk ); +void vips__chomp( char *str ); + int vips_vsnprintf( char *str, size_t size, const char *format, va_list ap ); int vips_snprintf( char *str, size_t size, const char *format, ... ) __attribute__((format(printf, 3, 4))); diff --git a/libvips/iofuncs/system.c b/libvips/iofuncs/system.c index 335c5f8c..8c414b80 100644 --- a/libvips/iofuncs/system.c +++ b/libvips/iofuncs/system.c @@ -17,6 +17,9 @@ * 4/6/13 * - redo as a class * - input and output images are now optional + * 3/5/14 + * - switch to g_spawn_command_line_sync() from popen() ... helps stop + * stray command-windows on Windows */ /* @@ -119,8 +122,10 @@ vips_system_build( VipsObject *object ) char line[VIPS_PATH_MAX]; char txt[VIPS_PATH_MAX]; VipsBuf buf = VIPS_BUF_STATIC( txt ); - char *p; + char *std_output; + char *std_error; int result; + GError *error = NULL; if( VIPS_OBJECT_CLASS( vips_system_parent_class )->build( object ) ) return( -1 ); @@ -165,30 +170,48 @@ vips_system_build( VipsObject *object ) cmd, VIPS_PATH_MAX, system->out_name ) ) return( -1 ); - /* Swap all "%%" in the string for a single "%". We need this for - * compatibility with older printf-based vips_system()s which - * needed a double %%. - */ - for( p = cmd; *p; p++ ) - if( p[0] == '%' && - p[1] == '%' ) - memmove( p, p + 1, strlen( p ) ); + if( !g_spawn_command_line_sync( cmd, + &std_output, &std_error, &result, &error ) ) { + if( error ) { + vips_error( class->nickname, "%s", error->message ); + g_error_free( error ); + } + if( std_error ) { + vips__chomp( std_error ); + if( strcmp( std_error, "" ) != 0 ) + vips_error( class->nickname, + "error output: %s", std_error ); + VIPS_FREE( std_error ); + } + if( std_output ) { + vips__chomp( std_output ); + if( strcmp( std_output, "" ) != 0 ) + vips_error( class->nickname, + "output: %s", std_output ); + VIPS_FREE( std_output ); + } + vips_error_system( result, class->nickname, + "%s", _( "command failed" ) ); - if( !(fp = vips_popenf( "%s", "r", cmd )) ) - return( -1 ); - - while( fgets( line, VIPS_PATH_MAX, fp ) ) - if( !vips_buf_appends( &buf, line ) ) - break; - - g_object_set( system, "log", vips_buf_all( &buf ), NULL ); - - if( (result = pclose( fp )) ) { - vips_error( class->nickname, - _( "command failed: \"%s\"" ), system->cmd_format ); return( -1 ); } + g_assert( !result ); + + if( std_error ) { + vips__chomp( std_error ); + if( strcmp( std_error, "" ) != 0 ) + vips_warn( class->nickname, + _( "stderr output: %s" ), std_error ); + } + if( std_output ) { + vips__chomp( std_output ); + g_object_set( system, "log", std_output, NULL ); + } + + VIPS_FREE( std_output ); + VIPS_FREE( std_error ); + if( system->out_name ) { VipsImage *out; diff --git a/libvips/iofuncs/util.c b/libvips/iofuncs/util.c index c8840010..5a408152 100644 --- a/libvips/iofuncs/util.c +++ b/libvips/iofuncs/util.c @@ -1206,6 +1206,17 @@ vips_mkdirf( const char *name, ... ) return( 0 ); } +/* Chop off any trailing whitespace. + */ +void +vips__chomp( char *str ) +{ + char *p; + + for( p = str + strlen( str ); p > str && isspace( p[-1] ); p-- ) + p[-1] = '\0'; +} + /* Break a command-line argument into tokens separated by whitespace. * * Strings can't be adjacent, so "hello world" (without quotes) is a single From 42931c86ca3e24dc3359d2198a34081a9ba58efc Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 3 May 2014 19:58:27 +0100 Subject: [PATCH 4/4] oops, put "%%" squash back in vips_system() still need this afetr all --- libvips/iofuncs/system.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libvips/iofuncs/system.c b/libvips/iofuncs/system.c index 8c414b80..15b34ff6 100644 --- a/libvips/iofuncs/system.c +++ b/libvips/iofuncs/system.c @@ -122,6 +122,7 @@ vips_system_build( VipsObject *object ) char line[VIPS_PATH_MAX]; char txt[VIPS_PATH_MAX]; VipsBuf buf = VIPS_BUF_STATIC( txt ); + char *p; char *std_output; char *std_error; int result; @@ -170,6 +171,15 @@ vips_system_build( VipsObject *object ) cmd, VIPS_PATH_MAX, system->out_name ) ) return( -1 ); + /* Swap all "%%" in the string for a single "%". We need this for + * compatibility with older printf-based vips_system()s which + * needed a double %%. + */ + for( p = cmd; *p; p++ ) + if( p[0] == '%' && + p[1] == '%' ) + memmove( p, p + 1, strlen( p ) ); + if( !g_spawn_command_line_sync( cmd, &std_output, &std_error, &result, &error ) ) { if( error ) {