From 111a82c06e586692559530f7e0b9c6a8e5c1a72f Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Wed, 25 Sep 2013 12:57:36 +0100
Subject: [PATCH] possible fix for tiff write assert fail

---
 TODO                        | 12 ++++++------
 libvips/foreign/vips2tiff.c |  5 +++--
 libvips/iofuncs/region.c    |  3 ++-
 libvips/iofuncs/sink.c      |  8 +++-----
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/TODO b/TODO
index eb9fa11e..476b6b84 100644
--- a/TODO
+++ b/TODO
@@ -33,15 +33,15 @@
 
   that means we've not taken ownership
 
-  so it's probably ->strip or ->copy in dzsave, since we don't pass ownership
-  in a strict manner
+  ooo crash shows up in gdb on work PC
 
-  add take_ownership to dzsave and see if the ->NULL on the thread changes
-
-  vips_region_buffer() is called directly by dzsave ... could it be one of
-  those?
+  vips_region_buffer() is being called from find_tile() in vips2tiff.c
 
+  vips_sink_tile() calls the work function from many threads with
+  start/gen/stop, so we have to be careful when moving regions between threads
 
+  find_tile() now tags regions as unowned after vips_region_buffer(), is that
+  enough? also, benchmark
 
 
 
diff --git a/libvips/foreign/vips2tiff.c b/libvips/foreign/vips2tiff.c
index 8faac8c5..dec5da22 100644
--- a/libvips/foreign/vips2tiff.c
+++ b/libvips/foreign/vips2tiff.c
@@ -697,7 +697,7 @@ find_new_tile( PyramidLayer *layer )
 {
 	int i;
 
-	/* Exisiting buffer we have finished with? 
+	/* Existing buffer we have finished with? 
 	 */
 	for( i = 0; i < MAX_LAYER_BUFFER; i++ )
 		if( layer->tiles[i].bits == PYR_ALL ) 
@@ -710,7 +710,7 @@ find_new_tile( PyramidLayer *layer )
 			if( !(layer->tiles[i].tile = 
 				vips_region_new( layer->tw->im )) )
 				return( -1 );
-			vips__region_no_ownership( layer->tiles[i].tile );
+
 			return( i );
 		}
 
@@ -749,6 +749,7 @@ find_tile( PyramidLayer *layer, VipsRect *pos )
 		return( -1 );
 	if( vips_region_buffer( layer->tiles[i].tile, pos ) )
 		return( -1 );
+	vips__region_no_ownership( layer->tiles[i].tile );
 	layer->tiles[i].bits = PYR_NONE;
 
 	/* Do any quadrants of this tile fall entirely outside the image? 
diff --git a/libvips/iofuncs/region.c b/libvips/iofuncs/region.c
index 070afb28..33c1f6b3 100644
--- a/libvips/iofuncs/region.c
+++ b/libvips/iofuncs/region.c
@@ -373,7 +373,8 @@ vips__region_take_ownership( VipsRegion *region )
 		 * Not sure if this will ever happen: if it does, we'll 
 		 * need to dup the buffer.
 		 */
-		g_assert( !region->buffer || region->buffer->ref_count == 1 );
+		g_assert( !region->buffer || 
+			region->buffer->ref_count == 1 );
 
 		region->thread = g_thread_self();
 	}
diff --git a/libvips/iofuncs/sink.c b/libvips/iofuncs/sink.c
index e871675d..5399f53b 100644
--- a/libvips/iofuncs/sink.c
+++ b/libvips/iofuncs/sink.c
@@ -338,11 +338,9 @@ vips_sink_base_progress( void *a )
  * Loops over an image. @generate is called for every pixel in the image, with
  * the @reg argument being a region of pixels for processing. 
  *
- * Each set of
- * pixels is @tile_width by @tile_height pixels (less at the image edges). 
- * This is handy for things like
- * writing a tiled TIFF image, where tiles have to be generated with a certain
- * size.
+ * Each set of pixels is @tile_width by @tile_height pixels (less at the 
+ * image edges). This is handy for things like writing a tiled TIFF image, 
+ * where tiles have to be generated with a certain size.
  *
  * See also: vips_sink(), vips_get_tile_size().
  *