Merge branch '8.6'

ChangeLog

@@ -5,6 +5,10 @@
 - vips_sink_screen() keeps a ref to the input image ... stops a rare race
 - fix a minor accidental ABI break in 8.6.0 -> 8.6.1 [remicollet]
 - fix read of plane-separate TIFFs with large strips [remicollet]
+- fix a C++ warning in composite.cpp [lovell]
+- remove number of images limit in composite
+- composite allows 1 mode ... reused for all joins
+- fix race in vips_sink() for seq read 
 
 10/12/17 started 8.6.1
 - fix mmap window new/free cycling 

libvips/conversion/composite.cpp

@@ -4,6 +4,9 @@
  * 	- from bandjoin.c
  * 30/11/17
  * 	- add composite2 class, to make a nice CLI interface
+ * 30/1/18
+ * 	- remove number of images limit
+ * 	- allow one mode ... reused for all joins
  */
 
 /*
@@ -53,10 +56,6 @@
 
 #include "pconversion.h"
 
-/* Maximum number of input images -- why not?
- */
-#define MAX_INPUT_IMAGES (64)
-
 /* Maximum number of image bands.
  */
 #define MAX_BANDS (64)
@@ -80,7 +79,7 @@ typedef struct _VipsCompositeBase {
 	 */
 	VipsArrayImage *in;
 
-	/* For N input images, N - 1 blend modes.
+	/* For N input images, 1 blend mode or N - 1 blend modes.
 	 */
 	VipsArrayInt *mode;
 
@@ -141,6 +140,88 @@ vips_composite_base_dispose( GObject *gobject )
 	G_OBJECT_CLASS( vips_composite_base_parent_class )->dispose( gobject );
 }
 
+/* Our sequence value.
+ */
+typedef struct {
+	VipsCompositeBase *composite;
+
+	/* Set of input regions.
+	 */
+	VipsRegion **ir;
+
+	/* For each input, an input pointer.
+	 */
+	VipsPel **p;
+
+} VipsCompositeSequence;
+
+static int
+vips_composite_stop( void *vseq, void *a, void *b )
+{
+	VipsCompositeSequence *seq = (VipsCompositeSequence *) vseq;
+
+        if( seq->ir ) {
+		int i;
+
+		for( i = 0; seq->ir[i]; i++ )
+			VIPS_UNREF( seq->ir[i] );
+		VIPS_FREE( seq->ir );
+	}
+
+	VIPS_FREE( seq->p );
+
+	VIPS_FREE( seq );
+
+	return( 0 );
+}
+
+static void *
+vips_composite_start( VipsImage *out, void *a, void *b )
+{
+	VipsImage **in = (VipsImage **) a;
+	VipsCompositeBase *composite = (VipsCompositeBase *) b;
+
+	VipsCompositeSequence *seq;
+	int i, n;
+
+	if( !(seq = VIPS_NEW( NULL, VipsCompositeSequence )) )
+		return( NULL );
+
+	seq->composite = composite;
+	seq->ir = NULL;
+	seq->p = NULL;
+
+	/* How many images?
+	 */
+	for( n = 0; in[n]; n++ )
+		;
+
+	/* Alocate space for region array.
+	 */
+	if( !(seq->ir = VIPS_ARRAY( NULL, n + 1, VipsRegion * )) ) {
+		vips_composite_stop( seq, NULL, NULL );
+		return( NULL );
+	}
+
+	/* Create a set of regions.
+	 */
+	for( i = 0; i < n; i++ )
+		if( !(seq->ir[i] = vips_region_new( in[i] )) ) {
+			vips_composite_stop( seq, NULL, NULL );
+			return( NULL );
+		}
+	seq->ir[n] = NULL;
+
+	/* Input pointers.
+	 */
+	if( !(seq->p = VIPS_ARRAY( NULL, n + 1, VipsPel * )) ) {
+		vips_composite_stop( seq, NULL, NULL );
+		return( NULL );
+	}
+
+	return( seq );
+}
+
 /* For each of the supported interpretations, the maximum value of each band.
  */
 static int
@@ -696,7 +777,8 @@ template <typename T, gint64 min_T, gint64 max_T>
 static void 
 vips_combine_pixels( VipsCompositeBase *composite, VipsPel *q, VipsPel **p )
 {
-	VipsBlendMode *m = (VipsBlendMode *) composite->mode->area.data;
+	VipsBlendMode *mode = (VipsBlendMode *) composite->mode->area.data;
+	int n_mode = composite->mode->area.n;
 	int n = composite->n;
 	int bands = composite->bands;
 	T * restrict tq = (T * restrict) q;
@@ -715,8 +797,11 @@ vips_combine_pixels( VipsCompositeBase *composite, VipsPel *q, VipsPel **p )
 		for( int b = 0; b < bands; b++ )
 			B[b] *= aB;
 
-	for( int i = 1; i < n; i++ ) 
+	for( int i = 1; i < n; i++ ) {
-		vips_composite_base_blend<T>( composite, m[i - 1], B, tp[i] ); 
+		VipsBlendMode m = n_mode == 1 ? mode[0] : mode[i - 1];
+
+		vips_composite_base_blend<T>( composite, m, B, tp[i] ); 
+	}
 
 	/* Unpremultiply, if necessary.
 	 */
@@ -755,7 +840,8 @@ template <typename T, gint64 min_T, gint64 max_T>
 static void 
 vips_combine_pixels3( VipsCompositeBase *composite, VipsPel *q, VipsPel **p )
 {
-	VipsBlendMode *m = (VipsBlendMode *) composite->mode->area.data;
+	VipsBlendMode *mode = (VipsBlendMode *) composite->mode->area.data;
+	int n_mode = composite->mode->area.n;
 	int n = composite->n;
 	T * restrict tq = (T * restrict) q;
 	T ** restrict tp = (T ** restrict) p;
@@ -778,8 +864,11 @@ vips_combine_pixels3( VipsCompositeBase *composite, VipsPel *q, VipsPel **p )
 		B[3] = aB;
 	}
 
-	for( int i = 1; i < n; i++ ) 
+	for( int i = 1; i < n; i++ ) {
-		vips_composite_base_blend3<T>( composite, m[i - 1], B, tp[i] ); 
+		VipsBlendMode m = n_mode == 1 ? mode[0] : mode[i - 1];
+
+		vips_composite_base_blend3<T>( composite, m, B, tp[i] ); 
+	}
 
 	/* Unpremultiply, if necessary.
 	 */
@@ -815,47 +904,46 @@ vips_combine_pixels3( VipsCompositeBase *composite, VipsPel *q, VipsPel **p )
 
 static int
 vips_composite_base_gen( VipsRegion *output_region,
-	void *seq, void *a, void *b, gboolean *stop )
+	void *vseq, void *a, void *b, gboolean *stop )
 {
-	VipsRegion **input_regions = (VipsRegion **) seq;
+	VipsCompositeSequence *seq = (VipsCompositeSequence *) vseq;
 	VipsCompositeBase *composite = (VipsCompositeBase *) b;
 	VipsRect *r = &output_region->valid;
 	int ps = VIPS_IMAGE_SIZEOF_PEL( output_region->im );
 
-	if( vips_reorder_prepare_many( output_region->im, input_regions, r ) )
+	if( vips_reorder_prepare_many( output_region->im, seq->ir, r ) )
 		return( -1 );
 
 	VIPS_GATE_START( "vips_composite_base_gen: work" );
 
 	for( int y = 0; y < r->height; y++ ) {
-		VipsPel *p[MAX_INPUT_IMAGES];
 		VipsPel *q;
 
 		for( int i = 0; i < composite->n; i++ )
-			p[i] = VIPS_REGION_ADDR( input_regions[i],
+			seq->p[i] = VIPS_REGION_ADDR( seq->ir[i],
 				r->left, r->top + y );
-		p[composite->n] = NULL;
+		seq->p[composite->n] = NULL;
 		q = VIPS_REGION_ADDR( output_region, r->left, r->top + y );
 
 		for( int x = 0; x < r->width; x++ ) {
-			switch( input_regions[0]->im->BandFmt ) {
+			switch( seq->ir[0]->im->BandFmt ) {
 			case VIPS_FORMAT_UCHAR: 	
 #ifdef HAVE_VECTOR_ARITH
 				if( composite->bands == 3 ) 
 					vips_combine_pixels3
 						<unsigned char, 0, UCHAR_MAX>
-						( composite, q, p );
+						( composite, q, seq->p );
 				else
 #endif 
 					vips_combine_pixels
 						<unsigned char, 0, UCHAR_MAX>
-						( composite, q, p );
+						( composite, q, seq->p );
 				break;
 
 			case VIPS_FORMAT_CHAR: 		
 				vips_combine_pixels
 					<signed char, SCHAR_MIN, SCHAR_MAX>
-					( composite, q, p );
+					( composite, q, seq->p );
 				break; 
 
 			case VIPS_FORMAT_USHORT: 	
@@ -863,30 +951,30 @@ vips_composite_base_gen( VipsRegion *output_region,
 				if( composite->bands == 3 ) 
 					vips_combine_pixels3
 						<unsigned short, 0, USHRT_MAX>
-						( composite, q, p );
+						( composite, q, seq->p );
 				else
 #endif 
 					vips_combine_pixels
 						<unsigned short, 0, USHRT_MAX>
-						( composite, q, p );
+						( composite, q, seq->p );
 				break; 
 
 			case VIPS_FORMAT_SHORT: 	
 				vips_combine_pixels
 					<signed short, SHRT_MIN, SHRT_MAX>
-					( composite, q, p );
+					( composite, q, seq->p );
 				break; 
 
 			case VIPS_FORMAT_UINT: 		
 				vips_combine_pixels
 					<unsigned int, 0, UINT_MAX>
-					( composite, q, p );
+					( composite, q, seq->p );
 				break; 
 
 			case VIPS_FORMAT_INT: 		
 				vips_combine_pixels
 					<signed int, INT_MIN, INT_MAX>
-					( composite, q, p );
+					( composite, q, seq->p );
 				break; 
 
 			case VIPS_FORMAT_FLOAT:
@@ -894,18 +982,18 @@ vips_composite_base_gen( VipsRegion *output_region,
 				if( composite->bands == 3 ) 
 					vips_combine_pixels3
 						<float, 0, USHRT_MAX>
-						( composite, q, p );
+						( composite, q, seq->p );
 				else
 #endif 
 					vips_combine_pixels
 						<float, 0, 0>
-						( composite, q, p );
+						( composite, q, seq->p );
 				break;
 
 			case VIPS_FORMAT_DOUBLE:
 				vips_combine_pixels
 					<double, 0, 0>
-					( composite, q, p );
+					( composite, q, seq->p );
 				break;
 
 			default:
@@ -914,7 +1002,7 @@ vips_composite_base_gen( VipsRegion *output_region,
 			}
 
 			for( int i = 0; i < composite->n; i++ )
-				p[i] += ps;
+				seq->p[i] += ps;
 			q += ps;
 		}
 	}
@@ -948,14 +1036,14 @@ vips_composite_base_build( VipsObject *object )
 		vips_error( klass->nickname, "%s", _( "no input images" ) );
 		return( -1 );
 	}
-	if( composite->mode->area.n != composite->n - 1 ) {
+	if( composite->mode->area.n != composite->n - 1 &&
-		vips_error( klass->nickname,
+		composite->mode->area.n != 1 ) {
-			_( "for %d input images there must be %d blend modes" ),
+		vips_error( klass->nickname, _( "must be 1 or %d blend modes" ),
-			composite->n, composite->n - 1 );
+			composite->n - 1 );
 		return( -1 );
 	}
 	mode = (VipsBlendMode *) composite->mode->area.data;
-	for( int i = 0; i < composite->n - 1; i++ ) {
+	for( int i = 0; i < composite->mode->area.n; i++ ) {
 		if( mode[i] < 0 ||
 			mode[i] >= VIPS_BLEND_MODE_LAST ) {
 			vips_error( klass->nickname,
@@ -991,12 +1079,6 @@ vips_composite_base_build( VipsObject *object )
 			break;
 		}
 
-	if( composite->n > MAX_INPUT_IMAGES ) {
-		vips_error( klass->nickname,
-			"%s", _( "too many input images" ) );
-		return( -1 );
-	}
-
 	/* Transform to compositing space. It defaults to sRGB or B_W, usually 
 	 * 8 bit, but 16 bit if any inputs are 16 bit.
 	 */
@@ -1086,7 +1168,9 @@ vips_composite_base_build( VipsObject *object )
 		return( -1 );
 
 	if( vips_image_generate( conversion->out,
-		vips_start_many, vips_composite_base_gen, vips_stop_many,
+		vips_composite_start, 
+		vips_composite_base_gen, 
+		vips_composite_stop,
 		in, composite ) )
 		return( -1 );
 

libvips/iofuncs/sink.c

@@ -50,6 +50,19 @@
 
 #include "sink.h"
 
+/* A part of the image we are scanning. 
+ *
+ * We can't let any threads fall too far behind as that would mess up seq
+ * image sources. Keep track of two areas moving down the image, and stall if
+ * the previous area still has active threads. 
+ */
+typedef struct _SinkArea {
+	struct _Sink *sink;
+
+	VipsRect rect;		/* Part of image this area covers */
+        VipsSemaphore n_thread;	/* Number of threads scanning this area */
+} SinkArea;
+
 /* Per-call state.
  */
 typedef struct _Sink {
@@ -67,6 +80,13 @@ typedef struct _Sink {
 	VipsStopFn stop_fn;
 	void *a;
 	void *b;
+
+	/* We are current scanning area, we'll delay starting a new
+	 * area if old_area (the previous position) hasn't completed. 
+	 */
+	SinkArea *area;
+	SinkArea *old_area;
+
 } Sink;
 
 /* Our per-thread state.
@@ -82,6 +102,11 @@ typedef struct _SinkThreadState {
 	 * parent_object.reg, it's defined on the outer image.
 	 */
 	VipsRegion *reg;
+
+	/* The area we were allocated from. 
+	 */
+        SinkArea *area;
+
 } SinkThreadState;
 
 typedef struct _SinkThreadStateClass {
@@ -91,6 +116,133 @@ typedef struct _SinkThreadStateClass {
 
 G_DEFINE_TYPE( SinkThreadState, sink_thread_state, VIPS_TYPE_THREAD_STATE );
 
+static void
+sink_area_free( SinkArea *area )
+{
+	vips_semaphore_destroy( &area->n_thread );
+	vips_free( area );
+}
+
+static SinkArea *
+sink_area_new( Sink *sink )
+{
+	SinkArea *area;
+
+	if( !(area = VIPS_NEW( NULL, SinkArea )) )
+		return( NULL );
+	area->sink = sink;
+	vips_semaphore_init( &area->n_thread, 0, "n_thread" );
+
+	return( area );
+}
+
+/* Move an area to a position.
+ */
+static void 
+sink_area_position( SinkArea *area, int top, int height )
+{
+	Sink *sink = area->sink;
+
+	VipsRect all, rect;
+
+	all.left = 0;
+	all.top = 0;
+	all.width = sink->sink_base.im->Xsize;
+	all.height = sink->sink_base.im->Ysize;
+
+	rect.left = 0;
+	rect.top = top;
+	rect.width = sink->sink_base.im->Xsize;
+	rect.height = height;
+
+	vips_rect_intersectrect( &all, &rect, &area->rect );
+}
+
+/* Our VipsThreadpoolAllocate function ... move the thread to the next tile
+ * that needs doing. If we fill the current area, we block until the previous
+ * area is finished, then swap areas. 
+ *
+ * If all tiles are done, we return FALSE to end iteration.
+ */
+static gboolean
+sink_area_allocate_fn( VipsThreadState *state, void *a, gboolean *stop )
+{
+	SinkThreadState *wstate = (SinkThreadState *) state;
+	Sink *sink = (Sink *) a;
+	SinkBase *sink_base = (SinkBase *) sink;
+
+	VipsRect image;
+	VipsRect tile;
+
+	VIPS_DEBUG_MSG( "sink_area_allocate_fn: %p\n", g_thread_self() );
+
+	/* Is the state x/y OK? New line or maybe new buffer or maybe even 
+	 * all done.
+	 */
+	if( sink_base->x >= sink->area->rect.width ) {
+		sink_base->x = 0;
+		sink_base->y += sink_base->tile_height;
+
+		if( sink_base->y >= VIPS_RECT_BOTTOM( &sink->area->rect ) ) {
+			/* Block until the previous area is done.
+			 */
+			if( sink->area->rect.top > 0 ) 
+				vips_semaphore_downn( 
+					&sink->old_area->n_thread, 0 );
+
+			/* End of image?
+			 */
+			if( sink_base->y >= sink_base->im->Ysize ) {
+				*stop = TRUE;
+				return( 0 );
+			}
+
+			/* Swap buffers.
+			 */
+			VIPS_SWAP( SinkArea *, 
+				sink->area, sink->old_area );
+
+			/* Position buf at the new y.
+			 */
+			sink_area_position( sink->area, 
+				sink_base->y, sink_base->n_lines );
+		}
+	}
+
+	/* x, y and buf are good: save params for thread.
+	 */
+	image.left = 0;
+	image.top = 0;
+	image.width = sink_base->im->Xsize;
+	image.height = sink_base->im->Ysize;
+	tile.left = sink_base->x;
+	tile.top = sink_base->y;
+	tile.width = sink_base->tile_width;
+	tile.height = sink_base->tile_height;
+	vips_rect_intersectrect( &image, &tile, &state->pos );
+
+	/* The thread needs to know which area it's writing to.
+	 */
+	wstate->area = sink->area;
+
+	VIPS_DEBUG_MSG( "  %p allocated %d x %d:\n", 
+		g_thread_self(), state->pos.left, state->pos.top );
+
+	/* Add to the number of writers on the area.
+	 */
+	vips_semaphore_upn( &sink->area->n_thread, -1 );
+
+	/* Move state on.
+	 */
+	sink_base->x += sink_base->tile_width;
+
+	/* Add the number of pixels we've just allocated to progress.
+	 */
+	sink_base->processed += state->pos.width * state->pos.height;
+
+	return( 0 );
+}
+
 /* Call a thread's stop function. 
  */
 static int
@@ -194,6 +346,8 @@ vips_sink_thread_state_new( VipsImage *im, void *a )
 static void
 sink_free( Sink *sink )
 {
+	VIPS_FREEF( sink_area_free, sink->area );
+	VIPS_FREEF( sink_area_free, sink->old_area );
 	VIPS_FREEF( g_object_unref, sink->t );
 }
 
@@ -233,7 +387,12 @@ sink_init( Sink *sink,
 	sink->a = a;
 	sink->b = b;
 
+	sink->area = NULL;
+	sink->old_area = NULL;
+
 	if( !(sink->t = vips_image_new()) ||
+		!(sink->area = sink_area_new( sink )) ||
+		!(sink->old_area = sink_area_new( sink )) ||
 		vips_image_write( sink->sink_base.im, sink->t ) ) {
 		sink_free( sink );
 		return( -1 );
@@ -242,69 +401,25 @@ sink_init( Sink *sink,
 	return( 0 );
 }
 
-int 
-vips_sink_base_allocate( VipsThreadState *state, void *a, gboolean *stop )
-{
-	SinkBase *sink_base = (SinkBase *) a;
-
-	VipsRect image, tile;
-
-	/* Has work requested early termination?
-	 */
-	if( state->stop ) {
-		*stop = TRUE;
-
-		return( 0 );
-	}
-
-	/* Is the state x/y OK? New line or maybe all done.
-	 */
-	if( sink_base->x >= sink_base->im->Xsize ) {
-		sink_base->x = 0;
-		sink_base->y += sink_base->tile_height;
-
-		if( sink_base->y >= sink_base->im->Ysize ) {
-			*stop = TRUE;
-
-			return( 0 );
-		}
-	}
-
-	/* x, y and buf are good: save params for thread.
-	 */
-	image.left = 0;
-	image.top = 0;
-	image.width = sink_base->im->Xsize;
-	image.height = sink_base->im->Ysize;
-	tile.left = sink_base->x;
-	tile.top = sink_base->y;
-	tile.width = sink_base->tile_width;
-	tile.height = sink_base->tile_height;
-	vips_rect_intersectrect( &image, &tile, &state->pos );
-
-	/* Move state on.
-	 */
-	sink_base->x += sink_base->tile_width;
-
-	/* Add the number of pixels we've just allocated to progress.
-	 */
-	sink_base->processed += state->pos.width * state->pos.height;
-
-	return( 0 );
-}
-
 static int 
 sink_work( VipsThreadState *state, void *a )
 {
 	SinkThreadState *sstate = (SinkThreadState *) state;
 	Sink *sink = (Sink *) a;
+	SinkArea *area = sstate->area;
 
-	if( vips_region_prepare( sstate->reg, &state->pos ) ||
+	int result;
-		sink->generate_fn( sstate->reg, sstate->seq, 
-			sink->a, sink->b, &state->stop ) ) 
-		return( -1 );
 
-	return( 0 );
+	result = vips_region_prepare( sstate->reg, &state->pos );
+	if( !result )
+		result = sink->generate_fn( sstate->reg, sstate->seq,
+			sink->a, sink->b, &state->stop );
+
+	/* Tell the allocator we're done.
+	 */
+	vips_semaphore_upn( &area->n_thread, 1 );
+
+	return( result );
 }
 
 int 
@@ -376,9 +491,10 @@ vips_sink_tile( VipsImage *im,
 	 */
 	vips_image_preeval( im );
 
+	sink_area_position( sink.area, 0, sink.sink_base.n_lines );
 	result = vips_threadpool_run( im, 
 		vips_sink_thread_state_new,
-		vips_sink_base_allocate, 
+		sink_area_allocate_fn, 
 		sink_work, 
 		vips_sink_base_progress, 
 		&sink );