improve seek on pipes

There were a few issues in VipsSource around seeking on pipes. With this
patch, EOF detection is better, and pipe sources automatically turn into memory
sources when EOF is hit.

see https://github.com/libvips/libvips/issues/1829

ChangeLog

@@ -11,6 +11,7 @@
 - fix vips_fractsurf() typo [kleisauke]
 - better heif EOF detection [lovell]
 - fix gir build with g-o-i 1.66+ [László]
+- improve seek behaviour on pipes
 
 9/8/20 started 8.10.1
 - fix markdown -> xml conversion in doc generation

libvips/foreign/heifload.c

@@ -213,6 +213,13 @@ vips_foreign_load_heif_build( VipsObject *object )
 {
 	VipsForeignLoadHeif *heif = (VipsForeignLoadHeif *) object;
 
+#ifdef DEBUG
+	printf( "vips_foreign_load_heif_build:\n" );
+#endif /*DEBUG*/
+
+	if( vips_source_rewind( heif->source ) )
+		return( -1 );
+
 	if( !heif->ctx ) {
 		struct heif_error error;
 
@@ -225,6 +232,7 @@ vips_foreign_load_heif_build( VipsObject *object )
 		}
 	}
 
+
 	if( VIPS_OBJECT_CLASS( vips_foreign_load_heif_parent_class )->
 		build( object ) )
 		return( -1 );
@@ -281,7 +289,7 @@ vips_foreign_load_heif_get_flags( VipsForeignLoad *load )
 	return( VIPS_FOREIGN_SEQUENTIAL );
 }
 
-/* We've selcted the page. Try to select the associated thumbnail instead, 
+/* We've selected the page. Try to select the associated thumbnail instead, 
  * if we can.
  */
 static int
@@ -295,6 +303,10 @@ vips_foreign_load_heif_set_thumbnail( VipsForeignLoadHeif *heif )
 	double main_aspect;
 	double thumb_aspect;
 
+#ifdef DEBUG
+	printf( "vips_foreign_load_heif_set_thumbnail:\n" );
+#endif /*DEBUG*/
+
 	n_thumbs = heif_image_handle_get_list_of_thumbnail_IDs( 
 		heif->handle, thumb_ids, 1 );
 	if( n_thumbs == 0 )
@@ -353,16 +365,16 @@ static int
 vips_foreign_load_heif_set_page( VipsForeignLoadHeif *heif, 
 	int page_no, gboolean thumbnail )
 {
-#ifdef DEBUG
-	printf( "vips_foreign_load_heif_set_page: %d, thumbnail = %d\n",
-		page_no, thumbnail );
-#endif /*DEBUG*/
-
 	if( !heif->handle ||
 		page_no != heif->page_no ||
 		thumbnail != heif->thumbnail_set ) {
 		struct heif_error error;
 
+#ifdef DEBUG
+		printf( "vips_foreign_load_heif_set_page: %d, thumbnail = %d\n",
+			page_no, thumbnail );
+#endif /*DEBUG*/
+
 		VIPS_FREEF( heif_image_handle_release, heif->handle );
 		VIPS_FREEF( heif_image_release, heif->img );
 		heif->data = NULL;
@@ -579,6 +591,10 @@ vips_foreign_load_heif_header( VipsForeignLoad *load )
 	heif_item_id primary_id;
 	int i;
 
+#ifdef DEBUG
+	printf( "vips_foreign_load_heif_header:\n" );
+#endif /*DEBUG*/
+
 	heif->n_top = heif_context_get_number_of_top_level_images( heif->ctx );
 	heif->id = VIPS_ARRAY( NULL, heif->n_top, heif_item_id );
 	heif_context_get_list_of_top_level_image_IDs( heif->ctx, 
@@ -940,9 +956,9 @@ vips_foreign_load_heif_seek( gint64 position, void *userdata )
 {
 	VipsForeignLoadHeif *heif = (VipsForeignLoadHeif *) userdata;
 
-	vips_source_seek( heif->source, position, SEEK_SET );
-
-	return( 0 );
+	/* Return 0 on success.
+	 */
+	return( vips_source_seek( heif->source, position, SEEK_SET ) == -1 );
 }
 
 /* libheif calls this to mean "I intend to read() to this position, please
@@ -1171,7 +1187,7 @@ vips_foreign_load_heif_source_build( VipsObject *object )
 		g_object_ref( heif->source );
 	}
 
-	if( VIPS_OBJECT_CLASS( vips_foreign_load_heif_file_parent_class )->
+	if( VIPS_OBJECT_CLASS( vips_foreign_load_heif_source_parent_class )->
 		build( object ) )
 		return( -1 );
 

libvips/include/vips/connection.h

@@ -159,7 +159,7 @@ typedef struct _VipsSource {
 	 * we rewind and try again, serve data from this until it runs out.
 	 *
 	 * If we need to force the whole pipe into memory, read everything to
-	 * this and put a copy pf the pointer in data.
+	 * this and put a copy of the pointer in data.
 	 */
 	GByteArray *header_bytes;
 
@@ -171,7 +171,7 @@ typedef struct _VipsSource {
 	 */
 	VipsBlob *blob;
 
-	/* If we mmaped the file, whet we need to unmmap on finalize.
+	/* If we mmaped the file, what we need to unmmap on finalize.
 	 */
 	void *mmap_baseaddr;
 	size_t mmap_length;

libvips/iofuncs/source.c

@@ -5,6 +5,8 @@
  *
  * 3/2/20
  * 	- add vips_pipe_read_limit_set()
+ * 3/10/20
+ * 	- improve behaviour with read and seek on pipes
  */
 
 /*
@@ -40,8 +42,8 @@
  */
 
 /*
-#define VIPS_DEBUG
 #define TEST_SANITY
+#define VIPS_DEBUG
  */
 
 #ifdef HAVE_CONFIG_H
@@ -759,34 +761,45 @@ vips_source_read( VipsSource *source, void *buffer, size_t length )
 	return( total_read );
 }
 
-/* Read to a position. -1 means read to end of source. Does not change 
- * read_position.
+#ifdef DEBUG
+static void
+vips_source_print( VipsSource *source )
+{
+	printf( "vips_source_print: %p\n", source );
+	printf( "  source->read_position = %zd\n", source->read_position );
+	printf( "  source->is_pipe = %d\n", source->is_pipe );
+	printf( "  source->length = %zd\n", source->length );
+	printf( "  source->data = %p\n", source->data );
+	printf( "  source->header_bytes = %p\n", source->header_bytes );
+	if( source->header_bytes ) 
+		printf( "  source->header_bytes->len = %d\n", 
+			source->header_bytes->len );
+	printf( "  source->sniff = %p\n", source->sniff );
+	if( source->sniff )
+		printf( "  source->sniff->len = %d\n", source->sniff->len );
+}
+#endif /*DEBUG*/
+
+/* Read to a position. 
+ *
+ * target == -1 means read to end of source -- useful for forcing a pipe into
+ * memory, for example. 
+ *
+ * If we hit EOF, set length on the pipe.
+ *
+ * read_position is left somewhere indeterminate.
  */
 static int
 vips_source_pipe_read_to_position( VipsSource *source, gint64 target )
 {
 	const char *nick = vips_connection_nick( VIPS_CONNECTION( source ) );
 
-	gint64 old_read_position;
 	unsigned char buffer[4096];
 
-	VIPS_DEBUG_MSG( "vips_source_pipe_read_position: %" G_GINT64_FORMAT 
-		"\n", target );
-
-	g_assert( !source->decode );
-	g_assert( source->header_bytes );
-	g_assert( source->is_pipe );
-
-	if( target != -1 &&
-		(target < 0 ||
-			(source->length != -1 && 
-			 target > source->length)) ) {
-		vips_error( nick, 
-			_( "bad read to %" G_GINT64_FORMAT ), target );
-		return( -1 );
-	}
-
-	old_read_position = source->read_position;
+	/* This is only useful for pipes (sources where we don't know the
+	 * length).
+	 */
+	g_assert( source->length == -1 );
 
 	while( target == -1 ||
 		source->read_position < target ) {
@@ -795,8 +808,13 @@ vips_source_pipe_read_to_position( VipsSource *source, gint64 target )
 		bytes_read = vips_source_read( source, buffer, 4096 );
 		if( bytes_read == -1 )
 			return( -1 );
-		if( bytes_read == 0 )
+
+		if( bytes_read == 0 ) {
+			/* No more bytes available, we must be at EOF.
+			 */
+			source->length = source->read_position;
 			break;
+		}
 
 		if( target == -1 &&
 			vips__pipe_read_limit != -1 &&
@@ -806,8 +824,6 @@ vips_source_pipe_read_to_position( VipsSource *source, gint64 target )
 		}
 	}
 
-	source->read_position = old_read_position;
-
 	return( 0 );
 }
 
@@ -1126,9 +1142,25 @@ vips_source_seek( VipsSource *source, gint64 offset, int whence )
 
 	/* For pipes, we have to fake seek by reading to that point.
 	 */
-	if( source->is_pipe &&
-		vips_source_pipe_read_to_position( source, new_pos ) )
-		return( -1 );
+	if( source->is_pipe ) {
+		if( vips_source_pipe_read_to_position( source, new_pos ) )
+			return( -1 );
+
+		/* We've hit EOF in the pipe, and we've got the whole thing 
+		 * buffered. Steal the header_bytes pointer and turn into a 
+		 * memory source.
+		 */
+		if( source->length != -1 &&
+			source->header_bytes ) {
+			source->length = source->header_bytes->len;
+			source->data = source->header_bytes->data;
+			source->is_pipe = FALSE;
+
+			/* TODO ... we could close more fds here.
+			 */
+			vips_source_minimise( source );
+		}
+	}
 
 	source->read_position = new_pos;