diff --git a/ChangeLog b/ChangeLog index fbd6b7a6..e6748150 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ +<<<<<<< HEAD 21/9/18 started 8.8.0 - much faster smartcrop [lovell] - add low/high to smartcrop [jcupitt] @@ -32,6 +33,9 @@ - move vips_image_set_kill() and iskilled() to the public API - remove old c++ and python interfaces +31/3/19 started 8.7.5 +- better buffer sizing in tiff reader [omira-sch] + 4/1/19 started 8.7.4 - magickload with magick6 API did not chain exceptions correctly causing a memory leak under some conditions [kleisauke] diff --git a/libvips/foreign/tiff2vips.c b/libvips/foreign/tiff2vips.c index 47a0bdbb..78777574 100644 --- a/libvips/foreign/tiff2vips.c +++ b/libvips/foreign/tiff2vips.c @@ -183,6 +183,8 @@ * - check for non-byte-multiple bits_per_sample [HongxuChen] * 16/8/18 * - shut down the input file as soon as we can [kleisauke] + * 28/3/19 omira-sch + * - better buffer sizing */ /* @@ -253,20 +255,29 @@ typedef struct _RtiffHeader { */ gboolean tiled; - /* Fields for tiled images. + /* Fields for tiled images, as returned by libtiff. */ uint32 tile_width; uint32 tile_height; + tsize_t tile_size; - /* Fields for strip images. - * - * If read_scanlinewise is TRUE, the strips are too large to read in a - * single lump and we need to use the scanline API. + /* Fields for strip images, as returned by libtiff. */ uint32 rows_per_strip; tsize_t strip_size; int number_of_strips; + + /* If read_scanlinewise is TRUE, the strips are too large to read in a + * single lump and we will use the scanline API. + */ gboolean read_scanlinewise; + + /* Strip read geometry. These are the read params as they will be used + * to read, with either TIFFReadScanline() or TIFFReadEncodedStrip(), + * not as they are stored in the file. + */ + uint32 read_rows_per_strip; + tsize_t read_strip_size; } RtiffHeader; /* Scanline-type process function. @@ -1262,7 +1273,12 @@ rtiff_parse_copy( Rtiff *rtiff, VipsImage *out ) rtiff->sfn = rtiff_memcpy_line; rtiff->client = out; - rtiff->memcpy = TRUE; + + /* We expand YCBCR images to RGB using JPEGCOLORMODE_RGB, and this + * means we need a slightly larger read buffer for the edge pixels. In + * turn, this means we can't just memcpy to libvips regions. + */ + rtiff->memcpy = photometric_interpretation != PHOTOMETRIC_YCBCR; return( 0 ); } @@ -1297,14 +1313,6 @@ rtiff_pick_reader( Rtiff *rtiff ) if( photometric_interpretation == PHOTOMETRIC_PALETTE ) return( rtiff_parse_palette ); - if( photometric_interpretation == PHOTOMETRIC_YCBCR ) { - /* Sometimes JPEG in TIFF images are tagged as YCBCR. Ask - * libtiff to convert to RGB for us. - */ - TIFFSetField( rtiff->tiff, - TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB ); - } - return( rtiff_parse_copy ); } @@ -1317,6 +1325,10 @@ rtiff_set_header( Rtiff *rtiff, VipsImage *out ) uint32 data_length; void *data; + /* Request YCbCr expansion. + */ + TIFFSetField( rtiff->tiff, TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB ); + out->Xsize = rtiff->header.width; out->Ysize = rtiff->header.height * rtiff->n; @@ -1408,19 +1420,6 @@ rtiff_set_header( Rtiff *rtiff, VipsImage *out ) return( 0 ); } -/* The size of the buffer written by TIFFReadTile(). We can't use - * TIFFTileSize() since that ignores the setting of TIFFTAG_JPEGCOLORMODE. If - * this pseudo tag has been set and the tile is encoded with YCbCr, the tile - * is returned with chrominance upsampled. - * - * This seems not to happen for old-style jpeg-compressed tiles. - */ -static size_t -rtiff_tile_size( Rtiff *rtiff ) -{ - return( TIFFTileRowSize( rtiff->tiff ) * rtiff->header.tile_height ); -} - /* Allocate a tile buffer. Have one of these for each thread so we can unpack * to vips in parallel. */ @@ -1428,11 +1427,9 @@ static void * rtiff_seq_start( VipsImage *out, void *a, void *b ) { Rtiff *rtiff = (Rtiff *) a; - tsize_t size; tdata_t *buf; - size = rtiff_tile_size( rtiff ); - if( !(buf = vips_malloc( NULL, size )) ) + if( !(buf = vips_malloc( NULL, rtiff->header.tile_size )) ) return( NULL ); return( (void *) buf ); @@ -1501,7 +1498,7 @@ rtiff_fill_region( VipsRegion *out, /* Sizeof a line of bytes in the TIFF tile. */ - int tls = rtiff_tile_size( rtiff ) / tile_height; + int tls = rtiff->header.tile_size / tile_height; /* Sizeof a pel in the TIFF file. This won't work for formats which * are <1 byte per pel, like onebit :-( Fortunately, it's only used @@ -1714,12 +1711,10 @@ rtiff_read_tilewise( Rtiff *rtiff, VipsImage *out ) * match the tifftile size. */ if( rtiff->memcpy ) { - size_t vips_tile_size; - - vips_tile_size = VIPS_IMAGE_SIZEOF_PEL( t[0] ) * + size_t vips_tile_size = VIPS_IMAGE_SIZEOF_PEL( t[0] ) * tile_width * tile_height; - if( rtiff_tile_size( rtiff ) != vips_tile_size ) { + if( rtiff->header.tile_size != vips_tile_size ) { vips_error( "tiff2vips", "%s", _( "unsupported tiff image type" ) ); return( -1 ); @@ -1761,15 +1756,15 @@ static int rtiff_strip_read_interleaved( Rtiff *rtiff, tstrip_t strip, tdata_t buf ) { int samples_per_pixel = rtiff->header.samples_per_pixel; - int rows_per_strip = rtiff->header.rows_per_strip; + int read_rows_per_strip = rtiff->header.read_rows_per_strip; int bits_per_sample = rtiff->header.bits_per_sample; - int strip_y = strip * rows_per_strip; + int strip_y = strip * read_rows_per_strip; if( rtiff->header.separate ) { int page_width = rtiff->header.width; int page_height = rtiff->header.height; - int strips_per_plane = 1 + (page_height - 1) / rows_per_strip; - int strip_height = VIPS_MIN( rows_per_strip, + int strips_per_plane = 1 + (page_height - 1) / read_rows_per_strip; + int strip_height = VIPS_MIN( read_rows_per_strip, page_height - strip_y ); int pels_per_strip = page_width * strip_height; int bytes_per_sample = bits_per_sample >> 3; @@ -1809,7 +1804,7 @@ rtiff_stripwise_generate( VipsRegion *or, void *seq, void *a, void *b, gboolean *stop ) { Rtiff *rtiff = (Rtiff *) a; - int rows_per_strip = rtiff->header.rows_per_strip; + int read_rows_per_strip = rtiff->header.read_rows_per_strip; int page_height = rtiff->header.height; tsize_t scanline_size = TIFFScanlineSize( rtiff->tiff ); VipsRect *r = &or->valid; @@ -1837,7 +1832,7 @@ rtiff_stripwise_generate( VipsRegion *or, * strip in the image. */ g_assert( r->height == - VIPS_MIN( rows_per_strip, or->im->Ysize - r->top ) ); + VIPS_MIN( read_rows_per_strip, or->im->Ysize - r->top ) ); /* And check that y_pos is correct. It should be, since we are inside * a vips_sequential(). @@ -1861,7 +1856,7 @@ rtiff_stripwise_generate( VipsRegion *or, /* Strip number. */ - tstrip_t strip_no = y_page / rows_per_strip; + tstrip_t strip_no = y_page / read_rows_per_strip; VipsRect image, page, strip, hit; @@ -1879,9 +1874,9 @@ rtiff_stripwise_generate( VipsRegion *or, page.height = page_height; strip.left = 0; - strip.top = page.top + strip_no * rows_per_strip; + strip.top = page.top + strip_no * read_rows_per_strip; strip.width = rtiff->out->Xsize; - strip.height = rows_per_strip; + strip.height = read_rows_per_strip; /* Clip strip against page and image ... the final strip will * be smaller. @@ -1991,6 +1986,10 @@ rtiff_read_stripwise( Rtiff *rtiff, VipsImage *out ) rtiff->header.strip_size ); printf( "rtiff_read_stripwise: header.number_of_strips = %d\n", rtiff->header.number_of_strips ); + printf( "rtiff_read_stripwise: header.read_rows_per_strip = %u\n", + rtiff->header.read_rows_per_strip ); + printf( "rtiff_read_stripwise: header.read_strip_size = %zd\n", + rtiff->header.read_strip_size ); #endif /*DEBUG*/ /* Double check: in memcpy mode, the vips linesize should exactly @@ -2022,7 +2021,7 @@ rtiff_read_stripwise( Rtiff *rtiff, VipsImage *out ) */ if( rtiff->header.separate ) { if( !(rtiff->plane_buf = vips_malloc( VIPS_OBJECT( out ), - rtiff->header.strip_size )) ) + rtiff->header.read_strip_size )) ) return( -1 ); } @@ -2039,14 +2038,13 @@ rtiff_read_stripwise( Rtiff *rtiff, VipsImage *out ) rtiff->n > 1 ) { tsize_t size; - size = rtiff->header.strip_size; + size = rtiff->header.read_strip_size; if( rtiff->header.separate ) size *= rtiff->header.samples_per_pixel; if( !(rtiff->contig_buf = vips_malloc( VIPS_OBJECT( out ), size )) ) return( -1 ); - } /* rows_per_strip can be very large if this is a separate plane image, @@ -2057,7 +2055,7 @@ rtiff_read_stripwise( Rtiff *rtiff, VipsImage *out ) NULL, rtiff_stripwise_generate, NULL, rtiff, NULL ) || vips_sequential( t[0], &t[1], - "tile_height", rtiff->header.rows_per_strip, + "tile_height", rtiff->header.read_rows_per_strip, NULL ) || rtiff_autorotate( rtiff, t[1], &t[2] ) || rtiff_unpremultiply( rtiff, t[2], &t[3] ) || @@ -2075,6 +2073,12 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header ) uint16 extra_samples_count; uint16 *extra_samples_types; + /* We want to always expand subsampled YCBCR images to full RGB. We + * need to set this pseudo tag early, since it affects the value you + * get from TIFFStripSize() etc. + */ + TIFFSetField( rtiff->tiff, TIFFTAG_JPEGCOLORMODE, JPEGCOLORMODE_RGB ); + if( !tfget32( rtiff->tiff, TIFFTAG_IMAGEWIDTH, &header->width ) || !tfget32( rtiff->tiff, TIFFTAG_IMAGELENGTH, &header->height ) || !tfget16( rtiff->tiff, @@ -2088,11 +2092,11 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header ) /* Arbitrary sanity-checking limits. */ - if( header->width <= 0 || - header->width > VIPS_MAX_COORD || - header->height <= 0 || + if( header->width <= 0 || + header->width > VIPS_MAX_COORD || + header->height <= 0 || header->height > VIPS_MAX_COORD ) { - vips_error( "tiff2vips", + vips_error( "tiff2vips", "%s", _( "width/height out of range" ) ); return( -1 ); } @@ -2130,11 +2134,26 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header ) TIFFTAG_TILELENGTH, &header->tile_height ) ) return( -1 ); + /* Arbitrary sanity-checking limits. + */ + if( header->tile_width <= 0 || + header->tile_width > 10000 || + header->tile_height <= 0 || + header->tile_height > 10000 ) { + vips_error( "tiff2vips", + "%s", _( "tile size out of range" ) ); + return( -1 ); + } + + header->tile_size = TIFFTileSize( rtiff->tiff ); + /* Stop some compiler warnings. */ header->rows_per_strip = 0; header->strip_size = 0; header->number_of_strips = 0; + header->read_rows_per_strip = 0; + header->read_strip_size = 0; } else { if( !tfget32( rtiff->tiff, @@ -2142,15 +2161,6 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header ) return( -1 ); header->strip_size = TIFFStripSize( rtiff->tiff ); header->number_of_strips = TIFFNumberOfStrips( rtiff->tiff ); - header->read_scanlinewise = FALSE; - - /* rows_per_strip can be 2 ** 32 - 1, meaning the whole image. - * Clip this down to height to avoid confusing vips. - * - * And it musn't be zero. - */ - header->rows_per_strip = - VIPS_CLIP( 1, header->rows_per_strip, header->height ); /* libtiff has two strip-wise readers. TIFFReadEncodedStrip() * decompresses an entire strip to memory. It's fast, but it @@ -2166,16 +2176,32 @@ rtiff_header_read( Rtiff *rtiff, RtiffHeader *header ) */ if( header->rows_per_strip > 128 && !header->separate ) { - header->rows_per_strip = 1; - header->strip_size = TIFFScanlineSize( rtiff->tiff ); - header->number_of_strips = header->height; header->read_scanlinewise = TRUE; + header->read_rows_per_strip = 1; + header->read_strip_size = + TIFFScanlineSize( rtiff->tiff ); + } + else { + header->read_scanlinewise = FALSE; + + /* rows_per_strip can be 2 ** 32 - 1, meaning the + * whole image. Clip this down to height to avoid + * confusing vips. + * + * And it musn't be zero. + */ + header->read_rows_per_strip = + VIPS_CLIP( 1, + header->rows_per_strip, + header->height ); + header->read_strip_size = header->strip_size; } /* Stop some compiler warnings. */ header->tile_width = 0; header->tile_height = 0; + header->tile_size = 0; } TIFFGetFieldDefaulted( rtiff->tiff, TIFFTAG_EXTRASAMPLES, @@ -2207,8 +2233,8 @@ rtiff_header_equal( RtiffHeader *h1, RtiffHeader *h2 ) return( 0 ); } else { - if( h1->rows_per_strip != h2->rows_per_strip || - h1->strip_size != h2->strip_size || + if( h1->read_rows_per_strip != h2->read_rows_per_strip || + h1->read_strip_size != h2->read_strip_size || h1->number_of_strips != h2->number_of_strips ) return( 0 ); }