diff --git a/TODO b/TODO index 21c25639..c01db248 100644 --- a/TODO +++ b/TODO @@ -1,16 +1,3 @@ -- how do we maintain ordering? - - with very large numbers of thread seq is bound to break - - what guarantees sequentiality? tiles must be big enough that every thread - fits into one tile, and we must have exactly two tiles - - so ... tile height must scale with nthreads and image width - - is this nlines in get tile geometry? - - - - interlaced jpg needs massive memory, we should have two jpg read modes, like png diff --git a/libvips/conversion/sequential.c b/libvips/conversion/sequential.c index aa5e5e11..79e2e68e 100644 --- a/libvips/conversion/sequential.c +++ b/libvips/conversion/sequential.c @@ -78,6 +78,10 @@ vips_sequential_generate( VipsRegion *or, * position. */ if( r->top != sequential->y_pos ) { + printf( "vips_sequential_generate: error -- " + "at position %d in file, but position %d requested", + sequential->y_pos, r->top ); + vips_error( "VipsSequential", _( "non-sequential read --- " "at position %d in file, but position %d requested" ), diff --git a/libvips/conversion/tilecache.c b/libvips/conversion/tilecache.c index 588fe139..b9a0b87c 100644 --- a/libvips/conversion/tilecache.c +++ b/libvips/conversion/tilecache.c @@ -86,6 +86,7 @@ typedef struct _VipsTileCache { int tile_width; int tile_height; int max_tiles; + VipsCacheStrategy strategy; /* VipsTileCache. */ @@ -230,7 +231,7 @@ static Tile * tile_find( VipsTileCache *cache, VipsRegion *in, int x, int y ) { Tile *tile; - int oldest; + int oldest, topmost; GSList *p; /* In cache already? @@ -255,15 +256,35 @@ tile_find( VipsTileCache *cache, VipsRegion *in, int x, int y ) /* Reuse an old one. */ - oldest = cache->time; - tile = NULL; - for( p = cache->tiles; p; p = p->next ) { - Tile *t = (Tile *) p->data; + switch( cache->strategy ) { + case VIPS_CACHE_RANDOM: + oldest = cache->time; + tile = NULL; + for( p = cache->tiles; p; p = p->next ) { + Tile *t = (Tile *) p->data; - if( t->time < oldest ) { - oldest = t->time; - tile = t; + if( t->time < oldest ) { + oldest = t->time; + tile = t; + } } + break; + + case VIPS_CACHE_SEQUENTIAL: + topmost = cache->in->Ysize; + tile = NULL; + for( p = cache->tiles; p; p = p->next ) { + Tile *t = (Tile *) p->data; + + if( t->y < topmost ) { + topmost = t->y; + tile = t; + } + } + break; + + default: + g_assert( 0 ); } g_assert( tile ); @@ -424,6 +445,12 @@ vips_tile_cache_class_init( VipsTileCacheClass *class ) G_STRUCT_OFFSET( VipsTileCache, max_tiles ), -1, 1000000, 1000 ); + VIPS_ARG_ENUM( class, "strategy", 3, + _( "Strategy" ), + _( "Expected access pattern" ), + VIPS_ARGUMENT_OPTIONAL_INPUT, + G_STRUCT_OFFSET( VipsTileCache, strategy ), + VIPS_TYPE_CACHE_STRATEGY, VIPS_CACHE_RANDOM ); } static void @@ -432,6 +459,8 @@ vips_tile_cache_init( VipsTileCache *cache ) cache->tile_width = 128; cache->tile_height = 128; cache->max_tiles = 1000; + cache->strategy = VIPS_CACHE_RANDOM; + cache->time = 0; cache->ntiles = 0; cache->lock = g_mutex_new(); @@ -449,16 +478,23 @@ vips_tile_cache_init( VipsTileCache *cache ) * @tile_width: width of tiles in cache * @tile_height: height of tiles in cache * @max_tiles: maximum number of tiles to cache + * @strategy: hint expected access pattern #VipsCacheStrategy * * This operation behaves rather like vips_copy() between images * @in and @out, except that it keeps a cache of computed pixels. * This cache is made of up to @max_tiles tiles (a value of -1 * means any number of tiles), and each tile is of size @tile_width - * by @tile_height pixels. Each cache tile is made with a single call to - * vips_image_prepare(). + * by @tile_height pixels. + * + * Each cache tile is made with a single call to + * vips_image_prepare(). + * + * When the cache fills, a tile is chosen for reuse. If @strategy is + * #VIPS_CACHE_RANDOM, then the least-recently-used tile is reused. If + * @strategy is #VIPS_CACHE_SEQUENTIAL, the top-most tile is reused. * * By default, @tile_width and @tile_height are 128 pixels, and the operation - * will cache up to 1,000 tiles. + * will cache up to 1,000 tiles. @strategy defaults to #VIPS_CACHE_RANDOM. * * This is a lower-level operation than vips_image_cache() since it does no * subdivision and it single-threads its callee. It is suitable for caching diff --git a/libvips/foreign/foreign.c b/libvips/foreign/foreign.c index cadc42ca..a938efb4 100644 --- a/libvips/foreign/foreign.c +++ b/libvips/foreign/foreign.c @@ -932,6 +932,32 @@ vips_foreign_load_init( VipsForeignLoad *load ) load->disc = TRUE; } +int +vips_foreign_tilecache( VipsImage *in, VipsImage **out, int strip_height ) +{ + int tile_width; + int tile_height; + int nlines; + int nstrips; + + vips_get_tile_size( in, &tile_width, &tile_height, &nlines ); + + /* We need two buffers, each with enough strips to make a complete + * buffer. And double to be safe. + */ + nstrips = 2 * (1 + nlines / strip_height); + + if( vips_tilecache( in, out, + "tile_width", in->Xsize, + "tile_height", strip_height * nstrips, + "max_tiles", 2, + "strategy", VIPS_CACHE_SEQUENTIAL, + NULL ) ) + return( -1 ); + + return( 0 ); +} + /* Abstract base class for image savers. */ diff --git a/libvips/foreign/jpeg2vips.c b/libvips/foreign/jpeg2vips.c index 142734f5..33908ed3 100644 --- a/libvips/foreign/jpeg2vips.c +++ b/libvips/foreign/jpeg2vips.c @@ -837,8 +837,9 @@ read_jpeg_generate( VipsRegion *or, VipsRect *r = &or->valid; ReadJpeg *jpeg = (ReadJpeg *) a; struct jpeg_decompress_struct *cinfo = &jpeg->cinfo; + int sz = cinfo->output_width * cinfo->output_components; - JSAMPROW row_pointer[VIPS__TILE_HEIGHT]; + JSAMPROW row_pointer[1]; int y; #ifdef DEBUG @@ -853,12 +854,9 @@ read_jpeg_generate( VipsRegion *or, g_assert( r->width == or->im->Xsize ); g_assert( VIPS_RECT_BOTTOM( r ) <= or->im->Ysize ); - /* Tiles should always be on a 8-pixel boundary and exactly one block - * high. + /* Tiles should always be on a 8-pixel boundary. */ - g_assert( r->top % VIPS__TILE_HEIGHT == 0 ); - g_assert( r->height == - VIPS_MIN( VIPS__TILE_HEIGHT, or->im->Ysize - r->top ) ); + g_assert( r->top % 8 == 0 ); /* Here for longjmp() from vips__new_error_exit(). */ @@ -866,22 +864,20 @@ read_jpeg_generate( VipsRegion *or, return( -1 ); for( y = 0; y < r->height; y++ ) { - row_pointer[y] = (JSAMPLE *) + row_pointer[0] = (JSAMPLE *) VIPS_REGION_ADDR( or, 0, r->top + y ); /* No faster to read in groups and you have to loop - * anyway. + * anyway. So just read a line at a time. */ - jpeg_read_scanlines( cinfo, &row_pointer[y], 1 ); - } + jpeg_read_scanlines( cinfo, &row_pointer[0], 1 ); - if( jpeg->invert_pels ) { - int sz = cinfo->output_width * cinfo->output_components; - int x; + if( jpeg->invert_pels ) { + int x; - for( y = 0; y < r->height; y++ ) for( x = 0; x < sz; x++ ) - row_pointer[y][x] = 255 - row_pointer[y][x]; + row_pointer[0][x] = 255 - row_pointer[0][x]; + } } return( 0 ); @@ -919,11 +915,7 @@ read_jpeg_image( ReadJpeg *jpeg, VipsImage *out ) NULL, read_jpeg_generate, NULL, jpeg, NULL ) || vips_sequential( t[0], &t[1], NULL ) || - vips_tilecache( t[1], &t[2], - "tile_width", t[0]->Xsize, - "tile_height", VIPS__TILE_HEIGHT, - "max_tiles", 4, - NULL ) || + vips_foreign_tilecache( t[1], &t[2], 8 ) || vips_image_write( t[2], out ) ) return( -1 ); diff --git a/libvips/foreign/tiff2vips.c b/libvips/foreign/tiff2vips.c index 55f69de1..212d16c5 100644 --- a/libvips/foreign/tiff2vips.c +++ b/libvips/foreign/tiff2vips.c @@ -1266,10 +1266,11 @@ read_tilewise( ReadTiff *rtiff, VipsImage *out ) rtiff, NULL ) ) return( -1 ); + /* Copy to out, adding a cache. Enough tiles for two complete rows. */ - if( vips_tilecache( raw, &t, - "tile_width", rtiff->twidth, + if( vips_tilecache( raw, &t, + "tile_width", rtiff->twidth, "tile_height", rtiff->theight, "max_tiles", 2 * (1 + raw->Xsize / rtiff->twidth), NULL ) ) @@ -1291,8 +1292,7 @@ tiff2vips_stripwise_generate( VipsRegion *or, tdata_t tbuf = (tdata_t) b; VipsRect *r = &or->valid; - tdata_t dst; - tstrip_t strip; + int y; tsize_t length; #ifdef DEBUG @@ -1307,40 +1307,46 @@ tiff2vips_stripwise_generate( VipsRegion *or, g_assert( r->width == or->im->Xsize ); g_assert( VIPS_RECT_BOTTOM( r ) <= or->im->Ysize ); - /* Tiles should always be on a strip boundary and exactly one strip - * high. + /* Tiles should always be on a strip boundary. */ g_assert( r->top % rtiff->rows_per_strip == 0 ); - g_assert( r->height == - VIPS_MIN( rtiff->rows_per_strip, or->im->Ysize - r->top ) ); - /* Read directly into the image if we can. Otherwise, we must read - * to a temp buffer then unpack into the image. - */ - if( rtiff->memcpy ) - dst = VIPS_REGION_ADDR( or, 0, r->top ); - else - dst = tbuf; + for( y = 0; y < r->height; y += rtiff->rows_per_strip ) { + tdata_t dst; + tstrip_t strip; - strip = r->top / rtiff->rows_per_strip; + /* Read directly into the image if we can. Otherwise, we must + * read to a temp buffer then unpack into the image. + */ + if( rtiff->memcpy ) + dst = VIPS_REGION_ADDR( or, 0, r->top + y ); + else + dst = tbuf; - length = TIFFReadEncodedStrip( rtiff->tiff, strip, dst, (tsize_t) -1 ); - if( length == -1 ) { - vips_error( "tiff2vips", "%s", _( "read error" ) ); - return( -1 ); - } + strip = (r->top + y) / rtiff->rows_per_strip; - /* If necessary, unpack to destination. - */ - if( !rtiff->memcpy ) { - int y; + length = TIFFReadEncodedStrip( rtiff->tiff, + strip, dst, (tsize_t) -1 ); + if( length == -1 ) { + vips_error( "tiff2vips", "%s", _( "read error" ) ); + return( -1 ); + } - for( y = 0; y < r->height; y++ ) { - VipsPel *p = tbuf + y * rtiff->scanline_size; - VipsPel *q = VIPS_REGION_ADDR( or, 0, r->top ); + /* If necessary, unpack to destination. + */ + if( !rtiff->memcpy ) { + int height = VIPS_MIN( rtiff->rows_per_strip, + or->im->Ysize - (r->top + y) ); + int z; - rtiff->sfn( q, p, - or->im->Xsize, rtiff->client ); + for( z = 0; z < height; z++ ) { + VipsPel *p = tbuf + z * rtiff->scanline_size; + VipsPel *q = VIPS_REGION_ADDR( or, + 0, r->top + y + z ); + + rtiff->sfn( q, p, + or->im->Xsize, rtiff->client ); + } } } @@ -1350,7 +1356,7 @@ tiff2vips_stripwise_generate( VipsRegion *or, /* Stripwise reading. * * We could potentially read strips in any order, but this would give - * catastrophic performance for opertions like 90 degress rotate on a + * catastrophic performance for operations like 90 degress rotate on a * large image. Only offer sequential read. */ static int @@ -1397,21 +1403,12 @@ read_stripwise( ReadTiff *rtiff, VipsImage *out ) if( !(tbuf = vips_malloc( VIPS_OBJECT( out ), rtiff->strip_size )) ) return( -1 ); - /* Each tile in the cache is the size of a strip. Cache enough tiles - * for two lines of SMALLTILE. - */ if( vips_image_generate( t[0], NULL, tiff2vips_stripwise_generate, NULL, rtiff, tbuf ) || vips_sequential( t[0], &t[1], NULL ) || - vips_tilecache( t[1], &t[2], - "tile_width", t[0]->Xsize, - "tile_height", rtiff->rows_per_strip, - "max_tiles", - 1 + (2 * VIPS__TILE_HEIGHT) / - rtiff->rows_per_strip, - NULL ) || + vips_foreign_tilecache( t[1], &t[2], rtiff->rows_per_strip ) || vips_image_write( t[2], out ) ) return( -1 ); diff --git a/libvips/foreign/vipspng.c b/libvips/foreign/vipspng.c index 25aa62f1..a664a51a 100644 --- a/libvips/foreign/vipspng.c +++ b/libvips/foreign/vipspng.c @@ -422,11 +422,7 @@ vips__png_read( const char *name, VipsImage *out ) NULL, png2vips_generate, NULL, read, NULL ) || vips_sequential( t[0], &t[1], NULL ) || - vips_tilecache( t[1], &t[2], - "tile_width", t[0]->Xsize, - "tile_height", VIPS__TILE_HEIGHT, - "max_tiles", 2, - NULL ) || + vips_foreign_tilecache( t[1], &t[2], 16 ) || vips_image_write( t[2], out ) ) return( -1 ); } diff --git a/libvips/include/vips/conversion.h b/libvips/include/vips/conversion.h index bb46f6fb..1a957d96 100644 --- a/libvips/include/vips/conversion.h +++ b/libvips/include/vips/conversion.h @@ -39,11 +39,11 @@ extern "C" { /** * VipsExtend: - * @VIPS_EXTEND_BLACK; extend with black (all 0) pixels - * @VIPS_EXTEND_COPY; copy the image edges - * @VIPS_EXTEND_REPEAT; repeat the whole image - * @VIPS_EXTEND_MIRROR; mirror the whole image - * @VIPS_EXTEND_WHITE; extend with white (all bits set) pixels + * @VIPS_EXTEND_BLACK: extend with black (all 0) pixels + * @VIPS_EXTEND_COPY: copy the image edges + * @VIPS_EXTEND_REPEAT: repeat the whole image + * @VIPS_EXTEND_MIRROR: mirror the whole image + * @VIPS_EXTEND_WHITE: extend with white (all bits set) pixels * * See vips_embed(), vips_conv(), vips_affine() and so on. * @@ -78,8 +78,8 @@ typedef enum { /** * VipsDirection: - * @VIPS_DIRECTION_HORIZONTAL; left-right - * @VIPS_DIRECTION_VERTICAL; top-bottom + * @VIPS_DIRECTION_HORIZONTAL: left-right + * @VIPS_DIRECTION_VERTICAL: top-bottom * * See vips_flip(), vips_join() and so on. * @@ -96,9 +96,9 @@ typedef enum { /** * VipsAlign: - * @VIPS_ALIGN_LOW; align low coordinate edge - * @VIPS_ALIGN_CENTRE; align centre - * @VIPS_ALIGN_HIGH; align high coordinate edge + * @VIPS_ALIGN_LOW: align low coordinate edge + * @VIPS_ALIGN_CENTRE: align centre + * @VIPS_ALIGN_HIGH: align high coordinate edge * * See vips_join() and so on. * @@ -116,10 +116,10 @@ typedef enum { /** * VipsAngle: - * @VIPS_ANGLE_0; no rotate - * @VIPS_ANGLE_90; 90 degrees anti-clockwise - * @VIPS_ANGLE_180; 180 degree rotate - * @VIPS_ANGLE_270; 90 degrees clockwise + * @VIPS_ANGLE_0: no rotate + * @VIPS_ANGLE_90: 90 degrees anti-clockwise + * @VIPS_ANGLE_180: 180 degree rotate + * @VIPS_ANGLE_270: 90 degrees clockwise * * See vips_rot() and so on. * @@ -135,6 +135,24 @@ typedef enum { VIPS_ANGLE_LAST } VipsAngle; +/** + * VipsCacheStrategy: + * @VIPS_CACHE_RANDOM: expect random access + * @VIPS_CACHE_SEQUENTIAL: expect sequential access + * + * See vips_tilecache() and friends. + * + * Used to hint to caches about the expected access pattern. RANDOM might mean + * LRU eviction, SEQUENTIAL might mean top-most eviction. + * + * See also: vips_tilecache(). + */ +typedef enum { + VIPS_CACHE_RANDOM, + VIPS_CACHE_SEQUENTIAL, + VIPS_CACHE_LAST +} VipsCacheStrategy; + int vips_copy( VipsImage *in, VipsImage **out, ... ) __attribute__((sentinel)); int vips_tilecache( VipsImage *in, VipsImage **out, ... ) diff --git a/libvips/include/vips/enumtypes.h b/libvips/include/vips/enumtypes.h index aab6d7e8..e313bd2e 100644 --- a/libvips/include/vips/enumtypes.h +++ b/libvips/include/vips/enumtypes.h @@ -41,6 +41,8 @@ GType vips_align_get_type (void) G_GNUC_CONST; #define VIPS_TYPE_ALIGN (vips_align_get_type()) GType vips_angle_get_type (void) G_GNUC_CONST; #define VIPS_TYPE_ANGLE (vips_angle_get_type()) +GType vips_cache_strategy_get_type (void) G_GNUC_CONST; +#define VIPS_TYPE_CACHE_STRATEGY (vips_cache_strategy_get_type()) /* enumerations from "../../../libvips/include/vips/util.h" */ GType vips_token_get_type (void) G_GNUC_CONST; #define VIPS_TYPE_TOKEN (vips_token_get_type()) diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index 279f1fc0..86bc83d3 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -132,6 +132,8 @@ int vips__sizealike( VipsImage *in1, VipsImage *in2, int vips__bandalike( const char *domain, VipsImage *in1, VipsImage *in2, VipsImage **out1, VipsImage **out2 ); +int vips_foreign_tilecache( VipsImage *in, VipsImage **out, int strip_height ); + diff --git a/libvips/iofuncs/enumtypes.c b/libvips/iofuncs/enumtypes.c index dedb98b6..7bbaf637 100644 --- a/libvips/iofuncs/enumtypes.c +++ b/libvips/iofuncs/enumtypes.c @@ -184,6 +184,24 @@ vips_angle_get_type( void ) return( etype ); } +GType +vips_cache_strategy_get_type( void ) +{ + static GType etype = 0; + + if( etype == 0 ) { + static const GEnumValue values[] = { + {VIPS_CACHE_RANDOM, "VIPS_CACHE_RANDOM", "random"}, + {VIPS_CACHE_SEQUENTIAL, "VIPS_CACHE_SEQUENTIAL", "sequential"}, + {VIPS_CACHE_LAST, "VIPS_CACHE_LAST", "last"}, + {0, NULL, NULL} + }; + + etype = g_enum_register_static( "VipsCacheStrategy", values ); + } + + return( etype ); +} /* enumerations from "../../libvips/include/vips/arithmetic.h" */ GType vips_operation_math_get_type( void ) diff --git a/libvips/iofuncs/sinkdisc.c b/libvips/iofuncs/sinkdisc.c index 77baae0d..5902e059 100644 --- a/libvips/iofuncs/sinkdisc.c +++ b/libvips/iofuncs/sinkdisc.c @@ -348,6 +348,14 @@ wbuffer_allocate_fn( VipsThreadState *state, void *a, gboolean *stop ) return( 0 ); } + VIPS_DEBUG_MSG( "wbuffer_allocate_fn: " + "finished top = %d, height = %d\n", + write->buf->area.top, write->buf->area.height ); + + VIPS_DEBUG_MSG( "wbuffer_allocate_fn: " + "starting top = %d, height = %d\n", + sink_base->y, sink_base->nlines ); + /* Swap buffers. */ VIPS_SWAP( WriteBuffer *, write->buf, write->buf_back ); diff --git a/libvips/iofuncs/threadpool.c b/libvips/iofuncs/threadpool.c index e8874b57..e80db502 100644 --- a/libvips/iofuncs/threadpool.c +++ b/libvips/iofuncs/threadpool.c @@ -905,31 +905,34 @@ vips_get_tile_size( VipsImage *im, case VIPS_DEMAND_STYLE_SMALLTILE: *tile_width = vips__tile_width; *tile_height = vips__tile_height; - - /* Enough lines of tiles that we can expect to be able to keep - * nthr busy. Then double it. - */ - *nlines = *tile_height * - (1 + nthr / VIPS_MAX( 1, im->Xsize / *tile_width )) * 2; break; case VIPS_DEMAND_STYLE_ANY: case VIPS_DEMAND_STYLE_FATSTRIP: *tile_width = im->Xsize; *tile_height = vips__fatstrip_height; - *nlines = *tile_height * nthr * 2; break; case VIPS_DEMAND_STYLE_THINSTRIP: *tile_width = im->Xsize; *tile_height = vips__thinstrip_height; - *nlines = *tile_height * nthr * 2; break; default: g_assert( 0 ); } + /* We can't set nlines for the current demand style: a later bit of + * the pipeline might see a different hint and we need to synchronise + * buffer sizes everywhere. + * + * Pick the maximum buffer size we might possibly need. + */ + *nlines = vips__tile_height * + (1 + nthr / VIPS_MAX( 1, im->Xsize / vips__tile_width )) * 2; + *nlines = VIPS_MAX( *nlines, vips__fatstrip_height * nthr * 2 ); + *nlines = VIPS_MAX( *nlines, vips__thinstrip_height * nthr * 2 ); + /* We make this assumption in several places. */ g_assert( *nlines % *tile_height == 0 );