From 696ff2b24a6ad54b6189c69a216477e396635ae9 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 17 Oct 2019 13:16:12 +0100 Subject: [PATCH] fix up jpeg load and revise descriptor test --- libvips/foreign/jpeg2vips.c | 3 +- libvips/foreign/tiff2vips.c | 58 ++++++++++++++++++++--------------- libvips/foreign/vips2jpeg.c | 2 +- libvips/foreign/vipspng.c | 3 +- libvips/include/vips/stream.h | 2 +- libvips/iofuncs/stream.c | 30 ++++++++++++------ test/test_descriptors.sh | 19 +++--------- 7 files changed, 66 insertions(+), 51 deletions(-) diff --git a/libvips/foreign/jpeg2vips.c b/libvips/foreign/jpeg2vips.c index c98502cd..c29a1531 100644 --- a/libvips/foreign/jpeg2vips.c +++ b/libvips/foreign/jpeg2vips.c @@ -746,7 +746,8 @@ read_jpeg_generate( VipsRegion *or, /* In pixel decode mode. */ - vips_stream_input_decode( jpeg->input ); + if( vips_stream_input_decode( jpeg->input ) ) + return( -1 ); VIPS_GATE_START( "read_jpeg_generate: work" ); diff --git a/libvips/foreign/tiff2vips.c b/libvips/foreign/tiff2vips.c index ecf24463..c511da2b 100644 --- a/libvips/foreign/tiff2vips.c +++ b/libvips/foreign/tiff2vips.c @@ -300,7 +300,7 @@ typedef void (*scanline_process_fn)( struct _Rtiff *, typedef struct _Rtiff { /* Parameters. */ - char *filename; + VipsStreamInput *input; VipsImage *out; int page; int n; @@ -488,6 +488,7 @@ static void rtiff_free( Rtiff *rtiff ) { VIPS_FREEF( TIFFClose, rtiff->tiff ); + VIPS_UNREF( rtiff->input ); } static void @@ -496,15 +497,24 @@ rtiff_close_cb( VipsObject *object, Rtiff *rtiff ) rtiff_free( rtiff ); } +static void +rtiff_minimise_cb( VipsImage *image, Rtiff *rtiff ) +{ + if( rtiff->input ) + vips_stream_input_minimise( rtiff->input ); +} + static Rtiff * -rtiff_new( VipsImage *out, int page, int n, gboolean autorotate ) +rtiff_new( VipsStreamInput *input, VipsImage *out, + int page, int n, gboolean autorotate ) { Rtiff *rtiff; if( !(rtiff = VIPS_NEW( out, Rtiff )) ) return( NULL ); - rtiff->filename = NULL; + g_object_ref( input ); + rtiff->input = input; rtiff->out = out; rtiff->page = page; rtiff->n = n; @@ -521,11 +531,8 @@ rtiff_new( VipsImage *out, int page, int n, gboolean autorotate ) g_signal_connect( out, "close", G_CALLBACK( rtiff_close_cb ), rtiff ); - - /* Don't link to minimise. We need to be able to disconnect the - * underlying fd and we can't do that without making our own input - * handler for files. Implement this when we add input objects. - */ + g_signal_connect( out, "minimise", + G_CALLBACK( rtiff_minimise_cb ), rtiff ); if( rtiff->page < 0 || rtiff->page > 1000000 ) { vips_error( "tiff2vips", _( "bad page number %d" ), @@ -543,6 +550,9 @@ rtiff_new( VipsImage *out, int page, int n, gboolean autorotate ) return( NULL ); } + if( !(rtiff->tiff = vips__tiff_openin_stream( input )) ) + return( NULL ); + return( rtiff ); } @@ -1524,6 +1534,11 @@ rtiff_fill_region( VipsRegion *out, int x, y, z; + /* In pixel decode mode. + */ + if( vips_stream_input_decode( rtiff->input ) ) + return( -1 ); + /* Special case: we are filling a single tile exactly sized to match * the tiff tile and we have no repacking to do for this format. */ @@ -1850,6 +1865,11 @@ rtiff_stripwise_generate( VipsRegion *or, g_assert( r->height == VIPS_MIN( read_height, or->im->Ysize - r->top ) ); + /* In pixel decode mode. + */ + if( vips_stream_input_decode( rtiff->input ) ) + return( -1 ); + /* And check that y_pos is correct. It should be, since we are inside * a vips_sequential(). */ @@ -2400,20 +2420,6 @@ vips__istifftiled_stream( VipsStreamInput *input ) return( vips__testtiff_stream( input, TIFFIsTiled ) ); } -static Rtiff * -rtiff_new_stream( VipsStreamInput *input, VipsImage *out, - int page, int n, gboolean autorotate ) -{ - Rtiff *rtiff; - - if( !(rtiff = rtiff_new( out, page, n, autorotate )) || - !(rtiff->tiff = vips__tiff_openin_stream( input )) || - rtiff_header_read_all( rtiff ) ) - return( NULL ); - - return( rtiff ); -} - int vips__tiff_read_header_stream( VipsStreamInput *input, VipsImage *out, int page, int n, gboolean autorotate ) @@ -2422,7 +2428,8 @@ vips__tiff_read_header_stream( VipsStreamInput *input, VipsImage *out, vips__tiff_init(); - if( !(rtiff = rtiff_new_stream( input, out, page, n, autorotate )) ) + if( !(rtiff = rtiff_new( input, out, page, n, autorotate )) || + rtiff_header_read_all( rtiff ) ) return( -1 ); if( rtiff_set_header( rtiff, out ) ) @@ -2430,6 +2437,8 @@ vips__tiff_read_header_stream( VipsStreamInput *input, VipsImage *out, vips__tiff_read_header_orientation( rtiff, out ); + vips_stream_input_minimise( input ); + return( 0 ); } @@ -2446,7 +2455,8 @@ vips__tiff_read_stream( VipsStreamInput *input, VipsImage *out, vips__tiff_init(); - if( !(rtiff = rtiff_new_stream( input, out, page, n, autorotate )) ) + if( !(rtiff = rtiff_new( input, out, page, n, autorotate )) || + rtiff_header_read_all( rtiff ) ) return( -1 ); if( rtiff->header.tiled ) { diff --git a/libvips/foreign/vips2jpeg.c b/libvips/foreign/vips2jpeg.c index a1ab2240..6f53da29 100644 --- a/libvips/foreign/vips2jpeg.c +++ b/libvips/foreign/vips2jpeg.c @@ -158,8 +158,8 @@ vips__new_output_message( j_common_ptr cinfo ) vips_error( "VipsJpeg", _( "%s" ), buffer ); #ifdef DEBUG - printf( "vips__new_output_message: \"%s\"\n", buffer ); #endif /*DEBUG*/ + printf( "vips__new_output_message: \"%s\"\n", buffer ); /* This is run for things like file truncated. Signal invalidate to * force this op out of cache. diff --git a/libvips/foreign/vipspng.c b/libvips/foreign/vipspng.c index 8646bb5d..9b9c7d02 100644 --- a/libvips/foreign/vipspng.c +++ b/libvips/foreign/vipspng.c @@ -624,7 +624,8 @@ png2vips_generate( VipsRegion *or, /* In pixel decode mode. */ - vips_stream_input_decode( read->input ); + if( vips_stream_input_decode( read->input ) ) + return( -1 ); for( y = 0; y < r->height; y++ ) { png_bytep q = (png_bytep) VIPS_REGION_ADDR( or, 0, r->top + y ); diff --git a/libvips/include/vips/stream.h b/libvips/include/vips/stream.h index f54ef887..63f9b318 100644 --- a/libvips/include/vips/stream.h +++ b/libvips/include/vips/stream.h @@ -214,7 +214,7 @@ off_t vips_stream_input_seek( VipsStreamInput *input, off_t offset, int whence ); int vips_stream_input_rewind( VipsStreamInput *input ); void vips_stream_input_minimise( VipsStreamInput *input ); -void vips_stream_input_decode( VipsStreamInput *input ); +int vips_stream_input_decode( VipsStreamInput *input ); unsigned char *vips_stream_input_sniff( VipsStreamInput *input, size_t length ); #define VIPS_TYPE_STREAM_OUTPUT (vips_stream_output_get_type()) diff --git a/libvips/iofuncs/stream.c b/libvips/iofuncs/stream.c index e3f29547..c9b040fa 100644 --- a/libvips/iofuncs/stream.c +++ b/libvips/iofuncs/stream.c @@ -118,12 +118,16 @@ G_DEFINE_ABSTRACT_TYPE( VipsStream, vips_stream, VIPS_TYPE_OBJECT ); static void vips_stream_close( VipsStream *stream ) { + VIPS_DEBUG_MSG( "vips_stream_close:\n" ); + if( stream->close_descriptor >= 0 ) { + VIPS_DEBUG_MSG( " close()\n" ); close( stream->close_descriptor ); stream->close_descriptor = -1; } if( stream->tracked_descriptor >= 0 ) { + VIPS_DEBUG_MSG( " vips_tracked_close()\n" ); vips_tracked_close( stream->tracked_descriptor ); stream->tracked_descriptor = -1; } @@ -223,7 +227,8 @@ vips_stream_input_open( VipsStreamInput *input ) VIPS_DEBUG_MSG( "vips_stream_input_open: " "restoring read position %zd\n", input->read_position ); - new_pos = lseek( stream->descriptor, 0, SEEK_SET ); + new_pos = lseek( stream->descriptor, + input->read_position, SEEK_SET ); if( new_pos == -1 ) { vips_error_system( errno, STREAM_NAME( stream ), "%s", _( "unable to lseek()" ) ); @@ -298,11 +303,6 @@ vips_stream_input_read_real( VipsStreamInput *input, VIPS_DEBUG_MSG( "vips_stream_input_read_real:\n" ); - /* Make sure we are open, in case we've been minimised. - */ - if( vips_stream_input_open( input ) ) - return( -1 ); - if( input->blob ) { VipsArea *area = (VipsArea *) input->blob; ssize_t available = VIPS_MIN( length, @@ -375,6 +375,8 @@ vips_stream_input_minimise_real( VipsStreamInput *input ) { VipsStream *stream = VIPS_STREAM( input ); + VIPS_DEBUG_MSG( "vips_stream_input_minimise_real:\n" ); + if( stream->filename && stream->descriptor != -1 && input->seekable ) @@ -754,16 +756,26 @@ vips_stream_input_minimise( VipsStreamInput *input ) class->minimise( input ); } -void +int vips_stream_input_decode( VipsStreamInput *input ) { - if( !input->decode ) { - VIPS_DEBUG_MSG( "vips_stream_input_decode:\n" ); + VIPS_DEBUG_MSG( "vips_stream_input_decode:\n" ); + /* We have finished reading the header. We can discard the bytes we + * saved. + */ + if( !input->decode ) { input->decode = TRUE; VIPS_FREEF( g_byte_array_unref, input->header_bytes ); VIPS_FREEF( g_byte_array_unref, input->sniff ); } + + /* Make sure we are open, in case we've been minimised. + */ + if( vips_stream_input_open( input ) ) + return( -1 ); + + return( 0 ); } /** diff --git a/test/test_descriptors.sh b/test/test_descriptors.sh index 0d644949..1e5a7024 100755 --- a/test/test_descriptors.sh +++ b/test/test_descriptors.sh @@ -2,6 +2,9 @@ # test the various restartable loaders +# webp uses streans, but it needs to mmap the input, so you can't close() the +# fd on minimise + # set -x set -e @@ -11,22 +14,10 @@ if test_supported jpegload; then ./test_descriptors $image fi -if test_supported heifload; then - ./test_descriptors $test_images/Example1.heic -fi - -if test_supported gifload; then - ./test_descriptors $test_images/cogs.gif -fi - -if test_supported pdfload; then - ./test_descriptors $test_images/ISO_12233-reschart.pdf -fi - if test_supported pngload; then ./test_descriptors $test_images/sample.png fi -if test_supported webpload; then - ./test_descriptors $test_images/1.webp +if test_supported tiffload; then + ./test_descriptors $test_images/sample.tif fi