diff --git a/TODO b/TODO index 4d6a44bf..32131197 100644 --- a/TODO +++ b/TODO @@ -1,55 +1,3 @@ -- 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 ); - } - -argh we have ->stop (a bool) and ->stop (a function) - -rename the function as stop_fn - -sstate is the "state" member of VipsThread - -which is created by the ->start member of the thread - -which is set by vips_threadpool_run() - -which is vips_thread_state_new(), see vips_sink_memory() - -which creates a VipsThreadState - - -so .. move ->stop from SinkThreadState into the base class, VipsThreadState - - - - don't do vips_image_write() at the end of a pipeline, instead try: diff --git a/libvips/conversion/insert.c b/libvips/conversion/insert.c index 4bc3120a..d179754d 100644 --- a/libvips/conversion/insert.c +++ b/libvips/conversion/insert.c @@ -57,8 +57,8 @@ */ /* - */ #define VIPS_DEBUG + */ #ifdef HAVE_CONFIG_H #include diff --git a/libvips/include/vips/threadpool.h b/libvips/include/vips/threadpool.h index a4fd67a5..a6eae612 100644 --- a/libvips/include/vips/threadpool.h +++ b/libvips/include/vips/threadpool.h @@ -79,6 +79,10 @@ typedef struct _VipsThreadState { VipsRect pos; int x, y; + /* Set in work to get the allocate to signal stop. + */ + gboolean stop; + /* The client data passed to the enclosing vips_threadpool_run(). */ void *a; diff --git a/libvips/iofuncs/sink.c b/libvips/iofuncs/sink.c index 3a73d6f6..1f05ed91 100644 --- a/libvips/iofuncs/sink.c +++ b/libvips/iofuncs/sink.c @@ -81,10 +81,6 @@ typedef struct _SinkThreadState { * parent_object.reg, it's defined on the outer image. */ VipsRegion *reg; - - /* Set this in work to get the allocate to signal stop. - */ - gboolean stop; } SinkThreadState; typedef struct _SinkThreadStateClass { @@ -184,11 +180,10 @@ sink_thread_state_init( SinkThreadState *state ) { state->seq = NULL; state->reg = NULL; - state->stop = FALSE; } -static VipsThreadState * -sink_thread_state_new( VipsImage *im, void *a ) +VipsThreadState * +vips_sink_thread_state_new( VipsImage *im, void *a ) { return( VIPS_THREAD_STATE( vips_object_new( sink_thread_state_get_type(), @@ -247,14 +242,13 @@ sink_init( Sink *sink, int vips_sink_base_allocate( VipsThreadState *state, void *a, gboolean *stop ) { - SinkThreadState *sstate = (SinkThreadState *) state; SinkBase *sink_base = (SinkBase *) a; VipsRect image, tile; /* Has work requested early termination? */ - if( sstate->stop ) { + if( state->stop ) { *stop = TRUE; return( 0 ); @@ -300,7 +294,7 @@ sink_work( VipsThreadState *state, void *a ) if( vips_region_prepare( sstate->reg, &state->pos ) || sink->generate_fn( sstate->reg, sstate->seq, - sink->a, sink->b, &sstate->stop ) ) + sink->a, sink->b, &state->stop ) ) return( -1 ); return( 0 ); @@ -379,7 +373,7 @@ vips_sink_tile( VipsImage *im, vips_image_preeval( im ); result = vips_threadpool_run( im, - sink_thread_state_new, + vips_sink_thread_state_new, vips_sink_base_allocate, sink_work, vips_sink_base_progress, diff --git a/libvips/iofuncs/sink.h b/libvips/iofuncs/sink.h index 5eb9339e..1df31684 100644 --- a/libvips/iofuncs/sink.h +++ b/libvips/iofuncs/sink.h @@ -58,6 +58,7 @@ typedef struct _SinkBase { /* Some function we can share. */ void vips_sink_base_init( SinkBase *sink_base, VipsImage *image ); +VipsThreadState *vips_sink_thread_state_new( VipsImage *im, void *a ); int vips_sink_base_allocate( VipsThreadState *state, void *a, gboolean *stop ); int vips_sink_base_progress( void *a ); diff --git a/libvips/iofuncs/sinkmemory.c b/libvips/iofuncs/sinkmemory.c index 93188a16..595f701e 100644 --- a/libvips/iofuncs/sinkmemory.c +++ b/libvips/iofuncs/sinkmemory.c @@ -51,35 +51,35 @@ /* Per-call state. */ -typedef struct _Sink { +typedef struct _SinkMemory { SinkBase sink_base; /* A big region for the image memory. All the threads write to this. */ VipsRegion *all; -} Sink; +} SinkMemory; static void -sink_free( Sink *sink ) +sink_memory_free( SinkMemory *memory ) { - VIPS_UNREF( sink->all ); + VIPS_UNREF( memory->all ); } static int -sink_init( Sink *sink, VipsImage *im ) +sink_memory_init( SinkMemory *memory, VipsImage *im ) { VipsRect all; - vips_sink_base_init( &sink->sink_base, im ); + vips_sink_base_init( &memory->sink_base, im ); all.left = 0; all.top = 0; all.width = im->Xsize; all.height = im->Ysize; - if( !(sink->all = vips_region_new( im )) || - vips_region_image( sink->all, &all ) ) { - sink_free( sink ); + if( !(memory->all = vips_region_new( im )) || + vips_region_image( memory->all, &all ) ) { + sink_memory_free( memory ); return( -1 ); } @@ -87,19 +87,19 @@ sink_init( Sink *sink, VipsImage *im ) } static int -sink_work( VipsThreadState *state, void *a ) +sink_memory_work( VipsThreadState *state, void *a ) { - Sink *sink = (Sink *) a; + SinkMemory *memory = (SinkMemory *) a; - VIPS_DEBUG_MSG( "sink_work: %p " + VIPS_DEBUG_MSG( "sink_memory_work: %p " "left = %d, top = %d, width = %d, height = %d\n", - sink, + memory, state->pos.left, state->pos.top, state->pos.width, state->pos.height ); - if( vips_region_prepare_to( state->reg, sink->all, + if( vips_region_prepare_to( state->reg, memory->all, &state->pos, state->pos.left, state->pos.top ) ) return( -1 ); @@ -109,7 +109,7 @@ sink_work( VipsThreadState *state, void *a ) state->pos.left, state->pos.top ); int i; - VIPS_DEBUG_MSG( "sink_work: %p\n", sink ); + VIPS_DEBUG_MSG( "sink_memory_work: %p\n", memory ); for( i = 0; i < VIPS_IMAGE_SIZEOF_PEL( state->reg->im ); i++ ) printf( "\t%d) %02x\n", i, p[i] ); } @@ -132,7 +132,7 @@ sink_work( VipsThreadState *state, void *a ) int vips_sink_memory( VipsImage *image ) { - Sink sink; + SinkMemory memory; int result; VIPS_DEBUG_MSG( "vips_sink_memory: %p\n", image ); @@ -144,7 +144,7 @@ vips_sink_memory( VipsImage *image ) */ image->Bbits = vips_format_sizeof( image->BandFmt ) << 3; - if( sink_init( &sink, image ) ) + if( sink_memory_init( &memory, image ) ) return( -1 ); vips_image_preeval( image ); @@ -152,13 +152,13 @@ vips_sink_memory( VipsImage *image ) result = vips_threadpool_run( image, vips_thread_state_new, vips_sink_base_allocate, - sink_work, + sink_memory_work, vips_sink_base_progress, - &sink ); + &memory ); vips_image_posteval( image ); - sink_free( &sink ); + sink_memory_free( &memory ); return( result ); } diff --git a/libvips/iofuncs/threadpool.c b/libvips/iofuncs/threadpool.c index db11cffb..e8874b57 100644 --- a/libvips/iofuncs/threadpool.c +++ b/libvips/iofuncs/threadpool.c @@ -287,6 +287,7 @@ vips_thread_state_init( VipsThreadState *state ) VIPS_DEBUG_MSG( "vips_thread_state_init:\n" ); state->reg = NULL; + state->stop = FALSE; } void *