From ded6f17fa8305fc862428f5b77184ef80ec0e2e1 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 14 Oct 2016 09:00:21 +0100 Subject: [PATCH] fix up new buffer.c and make getpoint() use the threading system --- ChangeLog | 1 + TODO | 4 ++++ libvips/arithmetic/getpoint.c | 35 +++++++++++------------------------ libvips/iofuncs/buffer.c | 24 +++++++++++++++--------- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/ChangeLog b/ChangeLog index af50726e..72ed989c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,5 @@ 13/10/16 started 8.5.0 +- rewritten buffer system is safer and frees memory earlier 27/9/16 started 8.4.2 - small doc improvements diff --git a/TODO b/TODO index b7721578..10b9f438 100644 --- a/TODO +++ b/TODO @@ -1,3 +1,7 @@ +- getpoint() doesn't use a worker, it just calls vips_region_prepare() + directly + + we should crop to a mem buffer instead - rework buffer.c, it's getting ugly diff --git a/libvips/arithmetic/getpoint.c b/libvips/arithmetic/getpoint.c index f3c27a0c..fb6c2218 100644 --- a/libvips/arithmetic/getpoint.c +++ b/libvips/arithmetic/getpoint.c @@ -19,6 +19,10 @@ * - redo as a class * 16/12/14 * - free the input region much earlier + * 14/10/16 + * - crop to a memory image rather than using a region ... this means we + * use workers to calculate pixels and therefore use per-thread caching + * in the revised buffer system */ /* @@ -86,9 +90,8 @@ vips_getpoint_build( VipsObject *object ) { VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object ); VipsGetpoint *getpoint = (VipsGetpoint *) object; + VipsImage **t = (VipsImage **) vips_object_local_array( object, 2 ); - VipsRect area; - VipsRegion *region; double *vector; int n; VipsArrayDouble *out_array; @@ -96,31 +99,15 @@ vips_getpoint_build( VipsObject *object ) if( VIPS_OBJECT_CLASS( vips_getpoint_parent_class )->build( object ) ) return( -1 ); - /* <0 ruled out already. - */ - if( getpoint->x >= getpoint->in->Xsize || - getpoint->y >= getpoint->in->Ysize ) { - vips_error( class->nickname, - "%s", _( "coordinates out of range" ) ); - return( -1 ); - } - - if( vips_check_coding_known( class->nickname, getpoint->in ) || - !(region = vips_region_new( getpoint->in )) ) + t[1] = vips_image_new_memory(); + if( vips_crop( getpoint->in, &t[0], + getpoint->x, getpoint->y, 1, 1, NULL ) || + vips_image_write( t[0], t[1] ) ) return( -1 ); - area.left = getpoint->x; - area.top = getpoint->y; - area.width = 1; - area.height = 1; - if( vips_region_prepare( region, &area ) || - !(vector = vips__ink_to_vector( class->nickname, getpoint->in, - VIPS_REGION_ADDR( region, area.left, area.top ), - &n )) ) { - VIPS_UNREF( region ); + if( !(vector = vips__ink_to_vector( class->nickname, + getpoint->in, VIPS_IMAGE_ADDR( t[1], 0, 0 ), &n )) ) return( -1 ); - } - VIPS_UNREF( region ); out_array = vips_array_double_new( vector, n ); g_object_set( object, diff --git a/libvips/iofuncs/buffer.c b/libvips/iofuncs/buffer.c index fa801bd1..8f50cb86 100644 --- a/libvips/iofuncs/buffer.c +++ b/libvips/iofuncs/buffer.c @@ -55,10 +55,10 @@ */ /* - */ #define DEBUG_VERBOSE #define DEBUG_CREATE #define DEBUG + */ #ifdef HAVE_CONFIG_H #include @@ -101,6 +101,7 @@ vips_buffer_print( VipsBuffer *buffer ) printf( "area.width = %d, ", buffer->area.width ); printf( "area.height = %d, ", buffer->area.height ); printf( "done = %d, ", buffer->done ); + printf( "cache = %p, ", buffer->cache ); printf( "buf = %p, ", buffer->buf ); printf( "bsize = %zd\n", buffer->bsize ); } @@ -210,6 +211,10 @@ vips_buffer_free( VipsBuffer *buffer ) g_mutex_unlock( vips__global_lock ); #endif /*DEBUG*/ + +#ifdef DEBUG_VERBOSE + printf( "vips_buffer_free: freeing buffer %p\n", buffer ); +#endif /*DEBUG_VERBOSE*/ } static void @@ -281,8 +286,8 @@ buffer_cache_new( VipsBufferThread *buffer_thread, VipsImage *im ) g_slist_prepend( vips__buffer_cache_all, cache ); g_mutex_unlock( vips__global_lock ); - printf( "buffer_cache_new: new cache %p for thread %p\n", - cache, g_thread_self() ); + printf( "buffer_cache_new: new cache %p for thread %p on image %p\n", + cache, g_thread_self(), im ); printf( "\t(%d caches now)\n", g_slist_length( vips__buffer_cache_all ) ); #endif /*DEBUG_CREATE*/ @@ -362,12 +367,6 @@ vips_buffer_done( VipsBuffer *buffer ) if( !buffer->done && (cache = buffer_cache_get( im )) ) { -#ifdef DEBUG_VERBOSE - printf( "vips_buffer_done: thread %p adding to cache %p\n", - g_thread_self(), cache ); - vips_buffer_print( buffer ); -#endif /*DEBUG_VERBOSE*/ - g_assert( !g_slist_find( cache->buffers, buffer ) ); g_assert( !buffer->cache ); @@ -375,6 +374,13 @@ vips_buffer_done( VipsBuffer *buffer ) buffer->cache = cache; cache->buffers = g_slist_prepend( cache->buffers, buffer ); + +#ifdef DEBUG_VERBOSE + printf( "vips_buffer_done: " + "thread %p adding buffer %p to cache %p\n", + g_thread_self(), buffer, cache ); + vips_buffer_print( buffer ); +#endif /*DEBUG_VERBOSE*/ } }