diff --git a/ChangeLog b/ChangeLog index 43a7c465..7000264f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -14,7 +14,6 @@ - VipsMin stops search early if it can - C API supports optional output args - switch back to int-valued operations -- fix up VipsPool - add the operation cache - fallback vips_init() - vips_tracked_malloc() tracks allocation size and can report total mem usage @@ -29,6 +28,7 @@ - added VipsArea as a public struct - added array members and arguments - added nary +- remove VipsPool, vips_object_local_array() is much better 12/10/11 started 7.26.6 - NOCACHE was not being set correctly on OS X causing performance diff --git a/TODO b/TODO index afa4670b..f51454a3 100644 --- a/TODO +++ b/TODO @@ -1,16 +1,60 @@ +- strange + +$ valgrind vips embed Gugg_coloured.jpg x.jpg 100 100 5000 5000 --extend +mirror +==32146== Memcheck, a memory error detector +==32146== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al. +==32146== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright +info +==32146== Command: vips embed Gugg_coloured.jpg x.jpg 100 100 5000 5000 +--extend mirror +==32146== +==32146== Thread 2: +==32146== Conditional jump or move depends on uninitialised value(s) +==32146== at 0x4F8ED7D: vips_sink_base_allocate (sink.c:257) +==32146== by 0x4F99BC2: vips_thread_allocate (threadpool.c:450) +==32146== by 0x4F99CB5: vips_thread_work_unit (threadpool.c:508) +==32146== by 0x4F99DF0: vips_thread_main_loop (threadpool.c:547) +==32146== by 0x55852B5: ??? (in +/lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.0) +==32146== by 0x5818EFB: start_thread (pthread_create.c:304) +==32146== by 0x5B0F89C: clone (clone.S:112) +==32146== + +it's the 'stop' member of SinkThreadState + + /* Has work requested early termination? + */ + if( sstate->stop ) { + *stop = TRUE; + + return( 0 ); + } + + + - don't do vips_image_write() at the end of a pipeline, instead try: - g_object_set( object, "output", t ); + g_object_set( object, "out", t, NULL ); ie. replace the output object - in cache.c, we must not include the hash of the output objects, I guess we + this doesn't work, I wonder why ... do we need to add an extra ref to t? + + + + +- in cache.c, we must not include the hash of the output objects, I guess we don't +- get rid of vipspool + + + - vipsimage should be cached too, eg. diff --git a/libvips/conversion/embed.c b/libvips/conversion/embed.c index 63c8880d..0d474b3d 100644 --- a/libvips/conversion/embed.c +++ b/libvips/conversion/embed.c @@ -325,84 +325,14 @@ vips_embed_gen( VipsRegion *or, void *seq, void *a, void *b, gboolean *stop ) return( 0 ); } -static int -vips_embed_repeat( VipsPool *pool, VipsImage *in, VipsImage **out, - int x, int y, int width, int height ) -{ - VipsPoolContext *context = vips_pool_context_new( pool ); - - /* Clock arithmetic: we want negative x/y to wrap around - * nicely. - */ - const int nx = x < 0 ? - -x % in->Xsize : in->Xsize - x % in->Xsize; - const int ny = y < 0 ? - -y % in->Ysize : in->Ysize - y % in->Ysize; - - if( - vips_replicate( in, &VIPS_VI( 1 ), - width / in->Xsize + 2, - height / in->Ysize + 2, NULL ) || - vips_extract_area( VIPS_VI( 1 ), out, - nx, ny, width, height, NULL ) ) - return( -1 ); - - return( 0 ); -} - -static int -vips_embed_mirror( VipsPool *pool, VipsImage *in, VipsImage **out, - int x, int y, int width, int height ) -{ - VipsPoolContext *context = vips_pool_context_new( pool ); - - /* As repeat, but the tiles are twice the size because of - * mirroring. - */ - const int w2 = in->Xsize * 2; - const int h2 = in->Ysize * 2; - - const int nx = x < 0 ? -x % w2 : w2 - x % w2; - const int ny = y < 0 ? -y % h2 : h2 - y % h2; - - if( - /* Make a 2x2 mirror tile. - */ - vips_flip( in, &VIPS_VI( 1 ), - VIPS_DIRECTION_HORIZONTAL, NULL ) || - vips_join( in, VIPS_VI( 1 ), &VIPS_VI( 2 ), - VIPS_DIRECTION_HORIZONTAL, NULL ) || - vips_flip( VIPS_VI( 2 ), &VIPS_VI( 3 ), - VIPS_DIRECTION_VERTICAL, NULL ) || - vips_join( VIPS_VI( 2 ), VIPS_VI( 3 ), &VIPS_VI( 4 ), - VIPS_DIRECTION_VERTICAL, NULL ) || - - /* Repeat, then cut out the centre. - */ - vips_replicate( VIPS_VI( 4 ), &VIPS_VI( 5 ), - width / VIPS_VI( 4 )->Xsize + 2, - height / VIPS_VI( 4 )->Ysize + 2, NULL ) || - vips_extract_area( VIPS_VI( 5 ), &VIPS_VI( 6 ), - nx, ny, width, height, NULL ) || - - /* Overwrite the centre with the in, much faster - * for centre pixels. - */ - vips_insert( VIPS_VI( 6 ), in, out, - x, y, NULL ) ) - return( -1 ); - - return( 0 ); -} - static int vips_embed_build( VipsObject *object ) { VipsConversion *conversion = VIPS_CONVERSION( object ); VipsEmbed *embed = (VipsEmbed *) object; + VipsImage **t = (VipsImage **) vips_object_local_array( object, 7 ); VipsRect want; - VipsPool *pool; if( VIPS_OBJECT_CLASS( vips_embed_parent_class )->build( object ) ) return( -1 ); @@ -419,30 +349,72 @@ vips_embed_build( VipsObject *object ) vips_image_pio_output( conversion->out ) ) return( -1 ); - pool = vips_pool_new( "VipsEmbed" ); - vips_object_local( object, pool ); - switch( embed->extend ) { case VIPS_EXTEND_REPEAT: { - VipsPoolContext *context = vips_pool_context_new( pool ); + /* Clock arithmetic: we want negative x/y to wrap around + * nicely. + */ + const int nx = embed->x < 0 ? + -embed->x % embed->in->Xsize : + embed->in->Xsize - embed->x % embed->in->Xsize; + const int ny = embed->y < 0 ? + -embed->y % embed->in->Ysize : + embed->in->Ysize - embed->y % embed->in->Ysize; - if( vips_embed_repeat( pool, embed->in, &VIPS_VI( 1 ), - embed->x, embed->y, embed->width, embed->height ) || - vips_image_write( VIPS_VI( 1 ), conversion->out ) ) + if( vips_replicate( embed->in, &t[0], + embed->width / embed->in->Xsize + 2, + embed->height / embed->in->Ysize + 2, NULL ) || + vips_extract_area( t[0], &t[1], + nx, ny, embed->width, embed->height, NULL ) || + vips_image_write( t[1], conversion->out ) ) return( -1 ); -} +} break; case VIPS_EXTEND_MIRROR: { - VipsPoolContext *context = vips_pool_context_new( pool ); + /* As repeat, but the tiles are twice the size because of + * mirroring. + */ + const int w2 = embed->in->Xsize * 2; + const int h2 = embed->in->Ysize * 2; - if( vips_embed_mirror( pool, embed->in, &VIPS_VI( 1 ), - embed->x, embed->y, embed->width, embed->height ) || - vips_image_write( VIPS_VI( 1 ), conversion->out ) ) - return( -1 ); + const int nx = embed->x < 0 ? + -embed->x % w2 : w2 - embed->x % w2; + const int ny = embed->y < 0 ? + -embed->y % h2 : h2 - embed->y % h2; + + + if( + /* Make a 2x2 mirror tile. + */ + vips_flip( embed->in, &t[0], + VIPS_DIRECTION_HORIZONTAL, NULL ) || + vips_join( embed->in, t[0], &t[1], + VIPS_DIRECTION_HORIZONTAL, NULL ) || + vips_flip( t[1], &t[2], + VIPS_DIRECTION_VERTICAL, NULL ) || + vips_join( t[1], t[2], &t[3], + VIPS_DIRECTION_VERTICAL, NULL ) || + + /* Repeat, then cut out the centre. + */ + vips_replicate( t[3], &t[4], + embed->width / t[3]->Xsize + 2, + embed->height / t[3]->Ysize + 2, NULL ) || + vips_extract_area( t[4], &t[5], + nx, ny, embed->width, embed->height, NULL ) || + + /* Overwrite the centre with the in, much faster + * for centre pixels. + */ + vips_insert( t[5], embed->in, &t[6], + embed->x, embed->y, NULL ) || + + vips_image_write( t[6], conversion->out ) ) + return( -1 ); } break; diff --git a/libvips/include/vips/Makefile.am b/libvips/include/vips/Makefile.am index ca16e4e4..95a15e1a 100644 --- a/libvips/include/vips/Makefile.am +++ b/libvips/include/vips/Makefile.am @@ -24,7 +24,6 @@ pkginclude_HEADERS = \ interpolate.h \ intl.h \ mask.h \ - pool.h \ memory.h \ morphology.h \ mosaicing.h \ diff --git a/libvips/include/vips/pool.h b/libvips/include/vips/pool.h deleted file mode 100644 index 6c4587e8..00000000 --- a/libvips/include/vips/pool.h +++ /dev/null @@ -1,92 +0,0 @@ -/* manage pools of objects for unreffing - */ - -/* - - Copyright (C) 1991-2003 The National Gallery - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 2 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program; if not, write to the Free Software - Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - - */ - -/* - - These files are distributed with VIPS - http://www.vips.ecs.soton.ac.uk - - */ - -#ifndef VIPS_POOL_H -#define VIPS_POOL_H - -#ifdef __cplusplus -extern "C" { -#endif /*__cplusplus*/ - -/* A pool context ... just an array of GObject pointers. - */ -typedef GPtrArray VipsPoolContext; - -/* A VipsPool is really just a hash table, but we make it a VipsObject so we - * can put pools into other pools. - */ - -#define VIPS_TYPE_POOL (vips_pool_get_type()) -#define VIPS_POOL( obj ) \ - (G_TYPE_CHECK_INSTANCE_CAST( (obj), VIPS_TYPE_POOL, VipsPool )) -#define VIPS_POOL_CLASS( klass ) \ - (G_TYPE_CHECK_CLASS_CAST( (klass), VIPS_TYPE_POOL, VipsPoolClass)) -#define VIPS_IS_POOL( obj ) \ - (G_TYPE_CHECK_INSTANCE_TYPE( (obj), VIPS_TYPE_POOL )) -#define VIPS_IS_POOL_CLASS( klass ) \ - (G_TYPE_CHECK_CLASS_TYPE( (klass), VIPS_TYPE_POOL )) -#define VIPS_POOL_GET_CLASS( obj ) \ - (G_TYPE_INSTANCE_GET_CLASS( (obj), VIPS_TYPE_POOL, VipsPoolClass )) - -typedef struct _VipsPool { - VipsObject parent_object; - - /* A table of all the contexts we've seen. - */ - GHashTable *contexts; - - /* Track a name for debugging. - */ - const char *name; -} VipsPool; - -typedef struct _VipsPoolClass { - VipsObjectClass parent_class; -} VipsPoolClass; - -GType vips_pool_get_type( void ); - -VipsPool *vips_pool_new( const char *name ); -VipsPoolContext *vips_pool_context_new( VipsPool *pool ); -GObject **vips_pool_context_object( VipsPoolContext *context, int n ); - -/* Save some typing. This assumes you have a (VipsPoolContext *) called - * "context" in scope. - */ -#define VIPS_VI( N ) \ - (*((VipsImage **) vips_pool_context_object( context, (N) ))) - - -#ifdef __cplusplus -} -#endif /*__cplusplus*/ - -#endif /*VIPS_POOL_H*/ - - diff --git a/libvips/include/vips/vips.h b/libvips/include/vips/vips.h index 5b77b3a5..6da9abb1 100644 --- a/libvips/include/vips/vips.h +++ b/libvips/include/vips/vips.h @@ -98,7 +98,6 @@ extern "C" { #include #include #include -#include #include #include diff --git a/libvips/iofuncs/Makefile.am b/libvips/iofuncs/Makefile.am index 0d8a76a7..b28294c3 100644 --- a/libvips/iofuncs/Makefile.am +++ b/libvips/iofuncs/Makefile.am @@ -4,7 +4,6 @@ libiofuncs_la_SOURCES = \ type.c \ enumtypes.c \ object.c \ - pool.c \ base64.h \ base64.c \ error.c \ diff --git a/libvips/iofuncs/pool.c b/libvips/iofuncs/pool.c deleted file mode 100644 index e195c6cd..00000000 --- a/libvips/iofuncs/pool.c +++ /dev/null @@ -1,219 +0,0 @@ -/* manage pools of objects for unreffing - */ - -/* - - Copyright (C) 1991-2003 The National Gallery - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 2 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program; if not, write to the Free Software - Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - - */ - -/* - - These files are distributed with VIPS - http://www.vips.ecs.soton.ac.uk - - */ - -/* -#define VIPS_DEBUG - */ - -#ifdef HAVE_CONFIG_H -#include -#endif /*HAVE_CONFIG_H*/ -#include - -#include -#include - -#include -#include -#include - -/* - - Here's how to handle ref counts when calling vips operations: - - int - thing( VipsImage *in1, VipsImage *in2, VipsImage **out ) - { - VipsImage *t; - - if( vips_add( in1, in2, &t, NULL ) ) - return( -1 ); - if( vips_add( in1, t, out, NULL ) ) { - g_object_unref( t ); - return( -1 ); - } - g_object_unref( t ); - - return( 0 ); - } - - The first vips_add() call returns (via the reference argument) a new - VipsImage in variable t. The second vips_add() uses this as an input and - takes the ref count up to two. After calling the second vips_add() we have - to drop t to avoid leaks. We also have to drop t if the second vips_add() - fails. - - VipsPool provides a nicer way to track the objects that you create and free - them safely. The above function would become: - - int - thing( VipsPool *pool, VipsImage *in1, VipsImage *in2, VipsImage **out ) - { - VipsPoolContext *context = vips_pool_context_new( pool ); - - if( vips_add( in1, in2, &VIPS_VI( 1 ), NULL ) || - vips_add( in1, VIPS_VI( 1 ), out, NULL ) ) - return( -1 ); - - return( 0 ); - } - - vips_pool_context_new() creates a new context to hold a set of temporary - objects. You can get a pointer to a temporary image object with the - macro VIPS_VI() (this assumes there is a variable called "context" in scope). - Temporary objects are numbered from zero. - - Our caller will (eventually) call g_object_unref() on the pool and this - will in turn unref all objects in the pool. - - */ - -G_DEFINE_TYPE( VipsPool, vips_pool, VIPS_TYPE_OBJECT ); - -static void -vips_pool_dispose( GObject *gobject ) -{ - VipsPool *pool = VIPS_POOL( gobject ); - -#ifdef VIPS_DEBUG - VIPS_DEBUG_MSG( "vips_pool_dispose: " ); - vips_object_print( VIPS_OBJECT( gobject ) ); -#endif /*VIPS_DEBUG*/ - - VIPS_FREEF( g_hash_table_unref, pool->contexts ); - - G_OBJECT_CLASS( vips_pool_parent_class )->dispose( gobject ); -} - -static void -vips_pool_context_print( VipsPoolContext *context, VipsPoolContext *value, - VipsBuf *buf ) -{ - int i, n; - - n = 0; - for( i = 0; i < context->len; i++ ) - if( g_ptr_array_index( context, i ) ) - n += 1; - - vips_buf_appendf( buf, "VipsPoolContext %p, size %d, %d objects\n", - context, context->len, n ); - - for( i = 0; i < context->len; i++ ) - if( g_ptr_array_index( context, i ) ) - vips_buf_appendf( buf, "\t%p\n", - g_ptr_array_index( context, i ) ) ; -} - -static void -vips_pool_print( VipsObject *object, VipsBuf *buf ) -{ - VipsPool *pool = VIPS_POOL( object ); - - if( pool->contexts ) { - int size = g_hash_table_size( pool->contexts ); - - vips_buf_appendf( buf, "%d contexts\n", size ); - if( size > 0 ) - g_hash_table_foreach( pool->contexts, - (GHFunc) vips_pool_context_print, buf ); - } - - VIPS_OBJECT_CLASS( vips_pool_parent_class )->print( object, buf ); -} - -static void -vips_pool_class_init( VipsPoolClass *class ) -{ - GObjectClass *gobject_class = G_OBJECT_CLASS( class ); - VipsObjectClass *vobject_class = VIPS_OBJECT_CLASS( class ); - - gobject_class->dispose = vips_pool_dispose; - - vobject_class->print = vips_pool_print; -} - -static void -vips_pool_init( VipsPool *pool ) -{ - pool->contexts = g_hash_table_new_full( g_direct_hash, g_direct_equal, - NULL, (GDestroyNotify) g_ptr_array_unref ); -} - -VipsPool * -vips_pool_new( const char *name ) -{ - VipsPool *pool; - - vips_check_init(); - - pool = VIPS_POOL( g_object_new( VIPS_TYPE_POOL, NULL ) ); - - pool->name = name; - - if( vips_object_build( VIPS_OBJECT( pool ) ) ) { - VIPS_UNREF( pool ); - return( NULL ); - } - - return( pool ); -} - -static void -vips_pool_context_element_free( GObject *object ) -{ - if( object ) - g_object_unref( object ); -} - -VipsPoolContext * -vips_pool_context_new( VipsPool *pool ) -{ - VipsPoolContext *context; - - /* g_object_unref() hates unreffing NULL. - */ - context = g_ptr_array_new_with_free_func( - (GDestroyNotify) vips_pool_context_element_free ); - g_hash_table_insert( pool->contexts, context, context ); - - return( context ); -} - -GObject ** -vips_pool_context_object( VipsPoolContext *context, int n ) -{ - g_assert( n >= 0 ); - - if( n >= context->len ) - g_ptr_array_set_size( context, n + 10 ); - - return( (GObject **) &g_ptr_array_index( context, n ) ); -} -