Merge branch '8.9'

ChangeLog

@@ -7,6 +7,10 @@
 - libtiff LOGLUV images load and save as libvips XYZ 
 - add gifload_source
 
+31/1/19 started 8.9.2
+- fix a deadlock with --vips-leak [DarthSim]
+- better gifload behaviour for DISPOSAL_UNSPECIFIED [DarthSim]
+
 20/6/19 started 8.9.1
 - don't use the new source loaders for new_from_file or new_from_buffer, it
   will break the loader priority system

libvips/foreign/gifload.c

@@ -37,6 +37,9 @@
  * 30/1/19
  * 	- rework on top of VipsSource
  * 	- add gifload_source
+ * 31/1/20
+ * 	- treat DISPOSAL_UNSPECIFIED as _DO_NOT, since that's what many GIFs
+ * 	  in the wild appear to do
  */
 
 /*
@@ -847,8 +850,12 @@ vips_foreign_load_gif_render_line( VipsForeignLoadGif *gif,
 			/* In DISPOSE_DO_NOT mode, the previous frame shows
 			 * through (ie. we do nothing). In all other modes,
 			 * it's just transparent.
+			 *
+			 * Many GIFs use DISPOSAL_UNSPECIFIED to mean DO_NOT,
+			 * so use that for previous frame as well.
 			 */
-			if( gif->dispose != DISPOSE_DO_NOT )
+			if( gif->dispose != DISPOSE_DO_NOT &&
+				gif->dispose != DISPOSAL_UNSPECIFIED )
 				iq[x] = 0;
 		}
 		else
@@ -971,6 +978,20 @@ vips_foreign_load_gif_render( VipsForeignLoadGif *gif )
 	return( 0 );
 }
 
+#ifdef VIPS_DEBUG
+static const char *
+dispose2str( int dispose )
+{
+	switch( dispose ) {
+	case DISPOSAL_UNSPECIFIED: return( "DISPOSAL_UNSPECIFIED" );
+	case DISPOSE_DO_NOT: return( "DISPOSE_DO_NOT" );
+	case DISPOSE_BACKGROUND: return( "DISPOSE_BACKGROUND" );
+	case DISPOSE_PREVIOUS: return( "DISPOSE_PREVIOUS" );
+	default: return( "<unknown>" );
+	}
+}
+#endif /*VIPS_DEBUG*/
+
 static int
 vips_foreign_load_gif_extension( VipsForeignLoadGif *gif )
 {
@@ -993,15 +1014,18 @@ vips_foreign_load_gif_extension( VipsForeignLoadGif *gif )
 		 * is being set.
 		 */
 		gif->transparency = -1;
-		if( extension[1] & 0x1 )
+		if( extension[1] & 0x1 ) {
 			gif->transparency = extension[4];
+			VIPS_DEBUG_MSG( "vips_foreign_load_gif_extension: "
+				"transparency = %d\n", gif->transparency );
+		}
 
 		/* Set the current dispose mode. This is read during frame load
 		 * to set the meaning of background and transparent pixels.
 		 */
 		gif->dispose = (extension[1] >> 2) & 0x7;
 		VIPS_DEBUG_MSG( "vips_foreign_load_gif_extension: "
-			"dispose = %d\n", gif->dispose );
+			"dispose = %s\n", dispose2str( gif->dispose ) );
 	}
 
 	while( extension != NULL )

libvips/include/vips/private.h

@@ -192,6 +192,8 @@ int vips__view_image( struct _VipsImage *image );
  */
 extern int _vips__argument_id;
 
+void vips__meta_init( void );
+
 #ifdef __cplusplus
 }
 #endif /*__cplusplus*/

libvips/iofuncs/header.c

@@ -36,6 +36,8 @@
  * 	- add vips_image_get_n_pages()
  * 20/6/19
  * 	- add vips_image_get/set_array_int()
+ * 31/1/19
+ * 	- lock for metadata changes
  */
 
 /*
@@ -130,6 +132,11 @@
  * these types, it can be copied between images efficiently.
  */
 
+/* Use in various small places where we need a mutex and it's not worth 
+ * making a private one.
+ */
+static GMutex *vips__meta_lock = NULL;
+
 /* We have to keep the gtype as a string, since we statically init this.
  */
 typedef struct _HeaderField {
@@ -934,10 +941,10 @@ meta_cp( VipsImage *dst, const VipsImage *src )
 		/* We lock with vips_image_set() to stop races in highly-
 		 * threaded applications.
 		 */
-		g_mutex_lock( vips__global_lock );
+		g_mutex_lock( vips__meta_lock );
 		vips_slist_map2( src->meta_traverse,
 			(VipsSListMap2Fn) meta_cp_field, dst, NULL );
-		g_mutex_unlock( vips__global_lock );
+		g_mutex_unlock( vips__meta_lock );
 	}
 
 	return( 0 );
@@ -1025,13 +1032,6 @@ vips_image_set( VipsImage *image, const char *name, GValue *value )
 	g_assert( name );
 	g_assert( value );
 
-        /* If this image is shared, block metadata changes.
-         */
-        if( G_OBJECT( image )->ref_count > 1 ) {
-                g_warning( "can't set metadata \"%s\" on shared image", name );
-                return;
-        }
-
 	meta_init( image );
 
 	/* We lock between modifying metadata and copying metadata between
@@ -1041,9 +1041,9 @@ vips_image_set( VipsImage *image, const char *name, GValue *value )
 	 * metadata copy on another -- this can lead to crashes in
 	 * highly-threaded applications.
 	 */
-	g_mutex_lock( vips__global_lock );
+	g_mutex_lock( vips__meta_lock );
 	(void) meta_new( image, name, value );
-	g_mutex_unlock( vips__global_lock );
+	g_mutex_unlock( vips__meta_lock );
 
 	/* If we're setting an EXIF data block, we need to automatically expand 
 	 * out all the tags. This will set things like xres/yres too.
@@ -1240,14 +1240,6 @@ vips_image_remove( VipsImage *image, const char *name )
 
 	result = FALSE;
 
-        /* If this image is shared, block metadata changes. 
-         */
-        if( G_OBJECT( image )->ref_count > 1 ) {
-                g_warning( "can't remove metadata \"%s\" on shared image", 
-			name );
-                return( result );
-        }
-
 	if( image->meta ) {
 		/* We lock between modifying metadata and copying metadata 
 		 * between images, see meta_cp().
@@ -1256,9 +1248,9 @@ vips_image_remove( VipsImage *image, const char *name )
 		 * racing with metadata copy on another -- this can lead to 
 		 * crashes in highly-threaded applications.
 		 */
-		g_mutex_lock( vips__global_lock );
+		g_mutex_lock( vips__meta_lock );
 		result = g_hash_table_remove( image->meta, name );
-		g_mutex_unlock( vips__global_lock );
+		g_mutex_unlock( vips__meta_lock );
 	}
 
 	return( result );
@@ -2027,3 +2019,12 @@ vips_image_get_history( VipsImage *image )
 
 	return( image->Hist ? image->Hist : "" );
 }
+
+/* Called during vips_init().
+ */
+void
+vips__meta_init( void )
+{
+	if( !vips__meta_lock ) 
+		vips__meta_lock = vips_g_mutex_new();
+}

libvips/iofuncs/init.c

@@ -382,6 +382,7 @@ vips_init( const char *argv0 )
 
 	vips__threadpool_init();
 	vips__buffer_init();
+	vips__meta_init();
 
 	/* This does an unsynchronised static hash table init on first call --
 	 * we have to make sure we do this single-threaded. See: