From 1f3f20ee4e17f12bb8e3f6b0ddf8b7469fd33dec Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 13 May 2011 14:47:23 +0100 Subject: [PATCH] new cli working some final testing needed --- TODO | 20 +++++----- libvips/include/vips/object.h | 18 ++++++--- libvips/iofuncs/generate.c | 13 ++++--- libvips/iofuncs/object.c | 69 +++++++++++++++++++++++++---------- libvips/iofuncs/operation.c | 11 +++--- 5 files changed, 83 insertions(+), 48 deletions(-) diff --git a/TODO b/TODO index f78d107e..3f11ecfe 100644 --- a/TODO +++ b/TODO @@ -5,19 +5,14 @@ logic for things like create image from filename ... useful for object.c as well? -- try: + test booltest and imtest flags - $ vips add - VipsAdd (add), add two images - VipsOperation.add (left, right, out) - where: - left :: VipsImage - right :: VipsImage - out :: VipsImage - check_required: required construct param left to VipsAdd not set - check_required: required construct param right to VipsAdd not set - why don't we get an error for out not set? + + +- vips_object_set_argument_from_string() leaks output images, because it has + to, does this matter? + @@ -26,6 +21,9 @@ wrap new API for C++ +- add etc. should sizealike as well :( + + - matlab write diff --git a/libvips/include/vips/object.h b/libvips/include/vips/object.h index e701fcdf..692683b2 100644 --- a/libvips/include/vips/object.h +++ b/libvips/include/vips/object.h @@ -93,19 +93,25 @@ VIPS_ARGUMENT_OPTIONAL_OUTPUT Eg. the "width" of an image */ #define VIPS_ARGUMENT_REQUIRED_INPUT \ - (VIPS_ARGUMENT_INPUT | VIPS_ARGUMENT_REQUIRED | \ - VIPS_ARGUMENT_CONSTRUCT | VIPS_ARGUMENT_SET_ONCE) + (VIPS_ARGUMENT_INPUT | \ + VIPS_ARGUMENT_REQUIRED | \ + VIPS_ARGUMENT_CONSTRUCT | \ + VIPS_ARGUMENT_SET_ONCE) + +#define VIPS_ARGUMENT_REQUIRED_OUTPUT \ + (VIPS_ARGUMENT_OUTPUT | \ + VIPS_ARGUMENT_REQUIRED | \ + VIPS_ARGUMENT_CONSTRUCT | \ + VIPS_ARGUMENT_SET_ONCE) #define VIPS_ARGUMENT_OPTIONAL_INPUT \ (VIPS_ARGUMENT_INPUT | \ - VIPS_ARGUMENT_CONSTRUCT | VIPS_ARGUMENT_SET_ONCE) - -#define VIPS_ARGUMENT_REQUIRED_OUTPUT \ - (VIPS_ARGUMENT_OUTPUT | VIPS_ARGUMENT_REQUIRED | \ + VIPS_ARGUMENT_CONSTRUCT | \ VIPS_ARGUMENT_SET_ONCE) #define VIPS_ARGUMENT_OPTIONAL_OUTPUT \ (VIPS_ARGUMENT_OUTPUT | \ + VIPS_ARGUMENT_CONSTRUCT | \ VIPS_ARGUMENT_SET_ONCE) /* Keep one of these for every argument. diff --git a/libvips/iofuncs/generate.c b/libvips/iofuncs/generate.c index 79a77894..31b27fd4 100644 --- a/libvips/iofuncs/generate.c +++ b/libvips/iofuncs/generate.c @@ -498,7 +498,7 @@ vips_allocate_input_array( VipsImage *image, ... ) /* Fill array. */ va_start( ap, image ); - for( i = 0; i < n; i++ ) + for( i = 0; i < n; i++ ) ar[i] = va_arg( ap, VipsImage * ); va_end( ap ); ar[n] = NULL; @@ -603,6 +603,8 @@ vips_image_generate( VipsImage *image, { int res; + VIPS_DEBUG_MSG( "vips_image_generate: %p\n", image ); + g_assert( vips_object_sanity( VIPS_OBJECT( image ) ) ); if( !image->hint_set ) { @@ -626,7 +628,7 @@ vips_image_generate( VipsImage *image, image->start || image->stop ) { vips_error( "VipsImage", - "%s", _( "func already attached" ) ); + "%s", _( "generate() called twice" ) ); return( -1 ); } @@ -636,9 +638,8 @@ vips_image_generate( VipsImage *image, image->client1 = a; image->client2 = b; -#ifdef DEBUG_IO - printf( "vips_image_generate: attaching partial callbacks\n" ); -#endif /*DEBUG_IO*/ + VIPS_DEBUG_MSG( "vips_image_generate: " + "attaching partial callbacks\n" ); break; @@ -652,7 +653,7 @@ vips_image_generate( VipsImage *image, image->start || image->stop ) { vips_error( "VipsImage", - "%s", _( "func already attached" ) ); + "%s", _( "generate() called twice" ) ); return( -1 ); } diff --git a/libvips/iofuncs/object.c b/libvips/iofuncs/object.c index b1ee3b25..fe465493 100644 --- a/libvips/iofuncs/object.c +++ b/libvips/iofuncs/object.c @@ -45,6 +45,7 @@ #include #include +#include #ifdef WITH_DMALLOC #include @@ -748,11 +749,22 @@ vips_object_check_required( VipsObject *object, GParamSpec *pspec, { int *result = (int *) a; + VIPS_DEBUG_MSG( "vips_object_check_required: %s\n", + g_param_spec_get_name( pspec ) ); + VIPS_DEBUG_MSG( "\trequired: %d\n", + argument_class->flags & VIPS_ARGUMENT_REQUIRED ); + VIPS_DEBUG_MSG( "\tconstruct: %d\n", + argument_class->flags & VIPS_ARGUMENT_CONSTRUCT ); + VIPS_DEBUG_MSG( "\tassigned: %d\n", + argument_instance->assigned ); + if( (argument_class->flags & VIPS_ARGUMENT_REQUIRED) && (argument_class->flags & VIPS_ARGUMENT_CONSTRUCT) && !argument_instance->assigned ) { - vips_error( "check_required", - _( "required construct param %s to %s not set" ), + vips_error( "VipsObject", + /* used as eg. "parameter out to VipsAdd not set". + */ + _( "parameter %s to %s not set" ), g_param_spec_get_name( pspec ), G_OBJECT_TYPE_NAME( object ) ); *result = -1; @@ -986,8 +998,7 @@ vips_object_set_argument_from_string( VipsObject *object, pspec = g_object_class_find_property( G_OBJECT_CLASS( class ), name ); if( !pspec ) { - vips_error( "vips_object_set_argument_from_string", - _( "object %s has no argument %s" ), + vips_error( "VipsObject", _( "object %s has no argument %s" ), G_OBJECT_TYPE_NAME( object ), name ); return( -1 ); } @@ -1000,14 +1011,32 @@ vips_object_set_argument_from_string( VipsObject *object, if( G_IS_PARAM_SPEC_OBJECT( pspec ) && G_PARAM_SPEC_VALUE_TYPE( pspec ) == VIPS_TYPE_IMAGE ) { VipsImage *image; - char *mode; - mode = (argument_class->flags & VIPS_ARGUMENT_OUTPUT) ? - "w" : "r"; - if( !(image = vips_image_new_from_file( value, mode )) ) - return( -1 ); g_value_init( &gvalue, G_TYPE_OBJECT ); - g_value_set_object( &gvalue, image ); + + if( argument_class->flags & VIPS_ARGUMENT_OUTPUT ) { + if( !(image = vips_image_new_from_file( value, "w" )) ) + return( -1 ); + + /* When we set an output image on an object, the image + * will take a ref to the object. In other words, the + * operation will stay alive until we unref all of + * the output objects. + * + * For now, we leave the output image ref at 1 to + * keep it alive. Perhaps we should capture and + * return these refs somehow? + */ + g_value_set_object( &gvalue, image ); + } + else { + if( !(image = vips_image_new_from_file( value, "r" )) ) + return( -1 ); + + /* gvalue now owns the ref to image. + */ + g_value_take_object( &gvalue, image ); + } } else if( G_IS_PARAM_SPEC_BOOLEAN( pspec ) ) { gboolean b; @@ -1056,7 +1085,7 @@ vips_object_set_required( VipsObject *object, const char *value ) if( !(pspec = vips_argument_map( object, vips_object_set_required_test, NULL, NULL )) ) { - vips_error( "vips_object_set_required", + vips_error( "VipsObject", _( "no unset required arguments for %s" ), value ); return( -1 ); } @@ -1106,15 +1135,15 @@ vips_object_set_args( VipsObject *object, const char *p ) /* Now must be a , or a ). */ if( token != VIPS_TOKEN_RIGHT && token != VIPS_TOKEN_COMMA ) { - vips_error( "set_args", "%s", - _( "not , or ) after parameter" ) ); + vips_error( "VipsObject", + "%s", _( "not , or ) after parameter" ) ); return( -1 ); } } while( token != VIPS_TOKEN_RIGHT ); if( (p = vips__token_get( p, &token, string, PATH_MAX )) ) { - vips_error( "set_args", "%s", - _( "extra tokens after ')'" ) ); + vips_error( "VipsObject", + "%s", _( "extra tokens after ')'" ) ); return( -1 ); } @@ -1151,8 +1180,8 @@ vips_object_new_from_string_set( VipsObject *object, void *a, void *b ) if( (p = vips__token_get( p, &token, string, PATH_MAX )) ) { if( token == VIPS_TOKEN_LEFT && vips_object_set_args( object, p ) ) { - vips_error( "object_new", "%s", - _( "bad object arguments" ) ); + vips_error( "VipsObject", + "%s", _( "bad object arguments" ) ); return( object ); } } @@ -1358,7 +1387,7 @@ vips_class_map_concrete_all( GType type, VipsClassMap fn, void *a ) * classes. */ if( !(class = g_type_class_ref( type )) ) { - vips_error( "vips_class_map_concrete_all", + vips_error( "VipsObject", "%s", _( "unable to build class" ) ); return( NULL ); } @@ -1396,14 +1425,14 @@ vips_class_find( const char *basename, const char *nickname ) GType base; if( !(base = g_type_from_name( basename )) ) { - vips_error( "vips_class_find", + vips_error( "VipsObject", _( "base class \"%s\" not found" ), basename ); return( NULL ); } if( !(class = vips_class_map_concrete_all( base, (VipsClassMap) test_name, (void *) nickname )) ) { - vips_error( "vips_class_find", + vips_error( "VipsObject", _( "class \"%s\" not found" ), nickname ); return( NULL ); } diff --git a/libvips/iofuncs/operation.c b/libvips/iofuncs/operation.c index 0f28ebb3..a64001af 100644 --- a/libvips/iofuncs/operation.c +++ b/libvips/iofuncs/operation.c @@ -419,13 +419,14 @@ vips_call_options_add( VipsObject *object, !argument_instance->assigned ) { GOptionEntry entry[2]; - entry[0].long_name = pspec->name; - entry[0].short_name = pspec->name[0]; + entry[0].long_name = g_param_spec_get_name( pspec ); + entry[0].short_name = g_param_spec_get_name( pspec )[0]; entry[0].flags = G_OPTION_FLAG_OPTIONAL_ARG; entry[0].arg = G_OPTION_ARG_CALLBACK; entry[0].arg_data = (gpointer) vips_call_options_set; - entry[0].description = pspec->name; - entry[0].arg_description = pspec->name; + entry[0].description = g_param_spec_get_blurb( pspec ); + entry[0].arg_description = + g_type_name( G_PARAM_SPEC_VALUE_TYPE( pspec ) ); entry[1].long_name = NULL; @@ -468,7 +469,7 @@ vips_call_argv( VipsOperation *operation, int argc, char **argv ) /* Now set required args from the rest of the command-line. */ - for( i = 1; i < argc; i++ ) + for( i = 0; i < argc; i++ ) if( vips_call_argv_set_required( operation, argv[i] ) ) return( -1 );