From 25c3c49d1c34e7e618b74eaf60db258f816fcd72 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 1 Mar 2016 11:31:54 +0000 Subject: [PATCH] better handling of cast+shift for non-int formats before, im.cast(uchar, shift = true) where im was float and tagged as rgb16 would not shift the image, since it's unclear how much to shift a float type by now we do two casts: first, we guess the numeric range from the interpretation, so rgb16 would be ushort, so we cast float->ushort; second, we cast to the target type and do the shift on the way see https://github.com/jcupitt/libvips/issues/397 thanks apacheark --- ChangeLog | 2 + TODO | 37 ++++++---------- libvips/conversion/cast.c | 28 +++++++++--- libvips/include/vips/header.h | 1 + libvips/iofuncs/header.c | 80 +++++++++++++++++++++++++++++++++++ 5 files changed, 118 insertions(+), 30 deletions(-) diff --git a/ChangeLog b/ChangeLog index acb81ccf..25a2ac29 100644 --- a/ChangeLog +++ b/ChangeLog @@ -15,6 +15,8 @@ - faster hist_find (Lovell Fuller) - webpload has a @shrink parameter for shrink-on-load - vipsthumbnail knows about webp shrink-on-load +- more vips_resize() tuning, a bit quicker now +- better behaviour for vips_cast() shift of non-int types (thanks apacheark) 27/1/16 started 8.2.3 - fix a crash with SPARC byte-order labq vips images diff --git a/TODO b/TODO index 91c0e2ee..c0c870c7 100644 --- a/TODO +++ b/TODO @@ -1,33 +1,18 @@ +better handling of cast+shift for non-int formats -- use the incremental webp decoding api to support seq for webp images - - https://developers.google.com/speed/webp/docs/api#decodingadvancedapi +before, im.cast(uchar, shift = true) where im was float and tagged as +rgb16 would not shift the image, since it's unclear how much to shift a float +type by -- try +now we do two casts: first, we guess the numeric range from the +interpretation, so rgb16 would be ushort, so we cast float->ushort; +second, we cast to the target type and do the shift on the way - $ vips sharpen babe.jpg x.v - horrible posterization, looks like too few bits - - $ vips colourspace babe.jpg x.v rgb16 - $ vips sharpen x.v x2.v - $ vips colourspace x2.v x3.v srgb - - no better, strange - -- we have a buffer reserve list on each image ... could we junk this on - "minimise"? +- python bandrank should work like bandjoin - try SEQ_UNBUFFERED on jpg source, get out of order error? -- add reduceh > 3 warning for wtc shrink to 1280x1280 - - or was it 1703? - -- colour ops are doing cast + extract_band before every op ... for something - like colourspace, which can potentially do many ops in a row, this is a LOT - of casting and extracting - - could load pdf thumbnails? - new vips_reduce: @@ -69,6 +54,12 @@ +- use the incremental webp decoding api to support seq for webp images + + https://developers.google.com/speed/webp/docs/api#decodingadvancedapi + + doesn't seem to be possible + - does ruby need to unpack RefString as well? what about C++? - are the mosaic functions calling vips_fastcor()? it must be very slow diff --git a/libvips/conversion/cast.c b/libvips/conversion/cast.c index f4f61b0f..4650f1db 100644 --- a/libvips/conversion/cast.c +++ b/libvips/conversion/cast.c @@ -49,6 +49,8 @@ * - cast to uint now removes <0 values * 11/2/15 * - add @shift option + * 1/3/16 + * - better behaviour for shift of non-int types (thanks apacheark) */ /* @@ -375,10 +377,8 @@ vips_cast_gen( VipsRegion *or, void *vseq, void *a, void *b, VipsCast *cast = (VipsCast *) b; VipsConversion *conversion = (VipsConversion *) b; VipsRect *r = &or->valid; - int le = r->left; - int to = r->top; - int bo = VIPS_RECT_BOTTOM( r ); int sz = VIPS_REGION_N_ELEMENTS( or ); + int x, y; if( vips_region_prepare( ir, r ) ) @@ -386,11 +386,11 @@ vips_cast_gen( VipsRegion *or, void *vseq, void *a, void *b, VIPS_GATE_START( "vips_cast_gen: work" ); - for( y = to; y < bo; y++ ) { - VipsPel *in = VIPS_REGION_ADDR( ir, le, y ); - VipsPel *out = VIPS_REGION_ADDR( or, le, y ); + for( y = 0; y < r->height; y++ ) { + VipsPel *in = VIPS_REGION_ADDR( ir, r->left, r->top + y ); + VipsPel *out = VIPS_REGION_ADDR( or, r->left, r->top + y ); - switch( cast->in->BandFmt ) { + switch( ir->im->BandFmt ) { case VIPS_FORMAT_UCHAR: BAND_SWITCH_INNER( unsigned char, VIPS_INT_INT, @@ -495,6 +495,20 @@ vips_cast_build( VipsObject *object ) return( -1 ); in = t[0]; + /* If @shift is on but we're not in an int format and we're going to + * an int format, we need to cast to int first. For example, what + * about a float image tagged as rgb16 being cast to uint8? We need + * to cast to ushort before we do the final cast to uint8. + */ + if( cast->shift && + !vips_band_format_isint( in->BandFmt ) && + vips_band_format_isint( cast->format ) ) { + if( vips_cast( in, &t[1], + vips_image_guess_format( in ), NULL ) ) + return( -1 ); + in = t[1]; + } + if( vips_image_pipelinev( conversion->out, VIPS_DEMAND_STYLE_THINSTRIP, in, NULL ) ) return( -1 ); diff --git a/libvips/include/vips/header.h b/libvips/include/vips/header.h index 43bd0f3d..8831f933 100644 --- a/libvips/include/vips/header.h +++ b/libvips/include/vips/header.h @@ -114,6 +114,7 @@ int vips_image_get_width( const VipsImage *image ); int vips_image_get_height( const VipsImage *image ); int vips_image_get_bands( const VipsImage *image ); VipsBandFormat vips_image_get_format( const VipsImage *image ); +VipsBandFormat vips_image_guess_format( const VipsImage *image ); VipsCoding vips_image_get_coding( const VipsImage *image ); VipsInterpretation vips_image_get_interpretation( const VipsImage *image ); VipsInterpretation vips_image_guess_interpretation( const VipsImage *image ); diff --git a/libvips/iofuncs/header.c b/libvips/iofuncs/header.c index 155b9606..5e913759 100644 --- a/libvips/iofuncs/header.c +++ b/libvips/iofuncs/header.c @@ -386,6 +386,86 @@ vips_image_get_format( const VipsImage *image ) return( image->BandFmt ); } +/** + * vips_image_guess_format: + * @image: image to guess for + * + * Return the #VipsBandFormat for an image, guessing a sane value if + * the set value looks crazy. + * + * For example, for a float image tagged as rgb16, we'd return ushort. + * + * Returns: a sensible #VipsBandFormat for the image. + */ +VipsBandFormat +vips_image_guess_format( const VipsImage *image ) +{ + VipsBandFormat format; + + switch( image->Type ) { + case VIPS_INTERPRETATION_B_W: + case VIPS_INTERPRETATION_HISTOGRAM: + case VIPS_INTERPRETATION_MULTIBAND: + format = image->BandFmt; + break; + + case VIPS_INTERPRETATION_FOURIER: + if( image->BandFmt == VIPS_FORMAT_DOUBLE || + image->BandFmt == VIPS_FORMAT_DPCOMPLEX ) + format = VIPS_FORMAT_DPCOMPLEX; + else + format = VIPS_FORMAT_COMPLEX; + break; + + case VIPS_INTERPRETATION_sRGB: + format = VIPS_FORMAT_UCHAR; + break; + + case VIPS_INTERPRETATION_XYZ: + case VIPS_INTERPRETATION_LAB: + case VIPS_INTERPRETATION_RGB: + case VIPS_INTERPRETATION_CMC: + case VIPS_INTERPRETATION_LCH: + case VIPS_INTERPRETATION_HSV: + case VIPS_INTERPRETATION_scRGB: + case VIPS_INTERPRETATION_YXY: + format = VIPS_FORMAT_FLOAT; + break; + + case VIPS_INTERPRETATION_CMYK: + if( image->BandFmt != VIPS_FORMAT_USHORT ) + format = VIPS_FORMAT_UCHAR; + else + format = image->BandFmt; + break; + + case VIPS_INTERPRETATION_LABQ: + format = VIPS_FORMAT_UCHAR; + break; + + case VIPS_INTERPRETATION_LABS: + format = VIPS_FORMAT_SHORT; + break; + + case VIPS_INTERPRETATION_GREY16: + case VIPS_INTERPRETATION_RGB16: + format = VIPS_FORMAT_USHORT; + break; + + case VIPS_INTERPRETATION_MATRIX: + if( image->BandFmt != VIPS_FORMAT_DOUBLE ) + format = VIPS_FORMAT_FLOAT; + else + format = image->BandFmt; + break; + + default: + g_assert_not_reached(); + } + + return( format ); +} + /** * vips_image_get_coding: * @image: image to get from