From dbd852211e28705351f2ef899e183d1dcccf0af0 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 5 Jun 2015 13:56:26 +0100 Subject: [PATCH] move bytw swapping out to vips_byteswap() it was an option to vips_copy(), ugly! --- ChangeLog | 1 + TODO | 30 +++- libvips/conversion/Makefile.am | 1 + libvips/conversion/bandmean.c | 2 +- libvips/conversion/byteswap.c | 242 ++++++++++++++++++++++++++++++ libvips/conversion/conversion.c | 2 + libvips/conversion/copy.c | 116 ++------------ libvips/deprecated/vips7compat.c | 4 +- libvips/foreign/analyze2vips.c | 2 +- libvips/foreign/ppm.c | 5 +- libvips/include/vips/conversion.h | 2 + libvips/include/vips/internal.h | 2 + libvips/iofuncs/image.c | 4 +- 13 files changed, 292 insertions(+), 121 deletions(-) create mode 100644 libvips/conversion/byteswap.c diff --git a/ChangeLog b/ChangeLog index 33ed2fdd..42db8a05 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,7 @@ - added VipsRefString as a thing that gi bindings can unpack - support "with Vips.Image as ...:" in Python - try to support DOS CSV and PPM files on linux +- add vips_byteswap(), remove byteswap from vips_copy() 7/5/15 started 8.0.3 - dzsave and tif pyr write could fail for some image dimensions, thanks Jonas diff --git a/TODO b/TODO index a08b55f2..50de4946 100644 --- a/TODO +++ b/TODO @@ -1,13 +1,18 @@ -- try: +- break up vips_copy(), it's become unwieldy - $ vips csvload ~/Desktop/NDVI_VGYRM-lut.txt x.v --skip 1 - $ vips copy x.v x2.v --width 256 --height 1 --bands 4 - Segmentation fault (core dumped) + - just let it make changes which don't change sizeof(pixel) - see commented-out test in test_conversion.py + - split swap out to vips_byteswap() + - split 1xN -> MxN out to vips_band_fold() / vips_band_unfold(), have an + option for fold direction -- does ruby need to unpack RefString as well? what about C++? + horizontal (default): M x N image, B bands -> (M * B) x N mono image + + vertical: M x N image, B bands -> M x (N * B) mono, copy image N times + vertically + + maybe leave direction option for later - how about something like vips_grid() which turns a tall thin one-band image into a much smaller many-band image? @@ -19,6 +24,19 @@ much faster to make a very tall, thin image and fold it up in a single operation + + +- try: + + $ vips csvload ~/Desktop/NDVI_VGYRM-lut.txt x.v --skip 1 + $ vips copy x.v x2.v --width 256 --height 1 --bands 4 + Segmentation fault (core dumped) + + see commented-out test in test_conversion.py + + +- does ruby need to unpack RefString as well? what about C++? + - are the mosaic functions calling vips_fastcor()? it must be very slow add vips_fastcor_direct() diff --git a/libvips/conversion/Makefile.am b/libvips/conversion/Makefile.am index b3d5575b..440f354e 100644 --- a/libvips/conversion/Makefile.am +++ b/libvips/conversion/Makefile.am @@ -9,6 +9,7 @@ libconversion_la_SOURCES = \ flatten.c \ premultiply.c \ unpremultiply.c \ + byteswap.c \ cache.c \ copy.c \ embed.c \ diff --git a/libvips/conversion/bandmean.c b/libvips/conversion/bandmean.c index e0c48b03..ad547f05 100644 --- a/libvips/conversion/bandmean.c +++ b/libvips/conversion/bandmean.c @@ -1,4 +1,4 @@ -/* im_bandmean.c +/* average image bands * * Author: Simon Goodall * Written on: 17/7/07 diff --git a/libvips/conversion/byteswap.c b/libvips/conversion/byteswap.c new file mode 100644 index 00000000..f2124018 --- /dev/null +++ b/libvips/conversion/byteswap.c @@ -0,0 +1,242 @@ +/* Swap image byte order. + * + * 5/6/15 + * - from copy.c + */ + +/* + + This file is part of VIPS. + + VIPS is free software; you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + 02110-1301 USA + + */ + +/* + + These files are distributed with VIPS - http://www.vips.ecs.soton.ac.uk + + */ + +/* +#define VIPS_DEBUG + */ + +#ifdef HAVE_CONFIG_H +#include +#endif /*HAVE_CONFIG_H*/ +#include + +#include +#include +#include +#include + +#include +#include +#include + +#include "pconversion.h" + +typedef struct _VipsByteswap { + VipsConversion parent_instance; + + /* The input image. + */ + VipsImage *in; + +} VipsByteswap; + +typedef VipsConversionClass VipsByteswapClass; + +G_DEFINE_TYPE( VipsByteswap, vips_byteswap, VIPS_TYPE_CONVERSION ); + +/* Swap pairs of bytes. + */ +static void +vips_byteswap_swap2( VipsPel *in, VipsPel *out, int width, VipsImage *im ) +{ + guint16 *p = (guint16 *) in; + guint16 *q = (guint16 *) out; + int sz = (VIPS_IMAGE_SIZEOF_PEL( im ) * width) / 2; + + int x; + + for( x = 0; x < sz; x++ ) + q[x] = GUINT16_SWAP_LE_BE( p[x] ); +} + +/* Swap 4- of bytes. + */ +static void +vips_byteswap_swap4( VipsPel *in, VipsPel *out, int width, VipsImage *im ) +{ + guint32 *p = (guint32 *) in; + guint32 *q = (guint32 *) out; + int sz = (VIPS_IMAGE_SIZEOF_PEL( im ) * width) / 4; + + int x; + + for( x = 0; x < sz; x++ ) + q[x] = GUINT32_SWAP_LE_BE( p[x] ); +} + +/* Swap 8- of bytes. + */ +static void +vips_byteswap_swap8( VipsPel *in, VipsPel *out, int width, VipsImage *im ) +{ + guint64 *p = (guint64 *) in; + guint64 *q = (guint64 *) out; + int sz = (VIPS_IMAGE_SIZEOF_PEL( im ) * width) / 8; + + int x; + + for( x = 0; x < sz; x++ ) + q[x] = GUINT64_SWAP_LE_BE( p[x] ); +} + +typedef void (*SwapFn)( VipsPel *in, VipsPel *out, int width, VipsImage *im ); + +static SwapFn vips_byteswap_swap_fn[] = { + NULL, /* VIPS_FORMAT_UCHAR = 0, */ + NULL, /* VIPS_FORMAT_CHAR = 1, */ + vips_byteswap_swap2, /* VIPS_FORMAT_USHORT = 2, */ + vips_byteswap_swap2, /* VIPS_FORMAT_SHORT = 3, */ + vips_byteswap_swap4, /* VIPS_FORMAT_UINT = 4, */ + vips_byteswap_swap4, /* VIPS_FORMAT_INT = 5, */ + vips_byteswap_swap4, /* VIPS_FORMAT_FLOAT = 6, */ + vips_byteswap_swap4, /* VIPS_FORMAT_COMPLEX = 7, */ + vips_byteswap_swap8, /* VIPS_FORMAT_DOUBLE = 8, */ + vips_byteswap_swap8 /* VIPS_FORMAT_DPCOMPLEX = 9, */ +}; + +/* Byteswap, turning bands into the x axis. + */ +static int +vips_byteswap_gen( VipsRegion *or, + void *seq, void *a, void *b, gboolean *stop ) +{ + VipsRegion *ir = (VipsRegion *) seq; + VipsImage *im = ir->im; + VipsRect *r = &or->valid; + SwapFn swap = vips_byteswap_swap_fn[im->BandFmt]; + + int y; + + if( vips_region_prepare( ir, r ) ) + return( -1 ); + + for( y = 0; y < r->height; y++ ) { + VipsPel *p = VIPS_REGION_ADDR( ir, r->left, r->top + y ); + VipsPel *q = VIPS_REGION_ADDR( or, r->left, r->top + y ); + + swap( p, q, r->width, im ); + } + + return( 0 ); +} + +static int +vips_byteswap_build( VipsObject *object ) +{ + VipsConversion *conversion = VIPS_CONVERSION( object ); + VipsByteswap *byteswap = (VipsByteswap *) object; + + if( VIPS_OBJECT_CLASS( vips_byteswap_parent_class )->build( object ) ) + return( -1 ); + + if( vips_image_pio_input( byteswap->in ) ) + return( -1 ); + + if( vips_image_pipelinev( conversion->out, + VIPS_DEMAND_STYLE_THINSTRIP, byteswap->in, NULL ) ) + return( -1 ); + + if( vips_image_generate( conversion->out, + vips_start_one, vips_byteswap_gen, vips_stop_one, + byteswap->in, byteswap ) ) + return( -1 ); + + return( 0 ); +} + +static void +vips_byteswap_class_init( VipsByteswapClass *class ) +{ + GObjectClass *gobject_class = G_OBJECT_CLASS( class ); + VipsObjectClass *vobject_class = VIPS_OBJECT_CLASS( class ); + VipsOperationClass *operation_class = VIPS_OPERATION_CLASS( class ); + + VIPS_DEBUG_MSG( "vips_byteswap_class_init\n" ); + + gobject_class->set_property = vips_object_set_property; + gobject_class->get_property = vips_object_get_property; + + vobject_class->nickname = "byteswap"; + vobject_class->description = _( "byteswap an image" ); + vobject_class->build = vips_byteswap_build; + + operation_class->flags = VIPS_OPERATION_SEQUENTIAL_UNBUFFERED; + + VIPS_ARG_IMAGE( class, "in", 1, + _( "Input" ), + _( "Input image" ), + VIPS_ARGUMENT_REQUIRED_INPUT, + G_STRUCT_OFFSET( VipsByteswap, in ) ); + +} + +static void +vips_byteswap_init( VipsByteswap *byteswap ) +{ +} + +/** + * vips_byteswap: + * @in: input image + * @out: output image + * @...: %NULL-terminated list of optional named arguments + * + * Swap the byte order in an image. + * + * See also: vips_rawload(). + * + * Returns: 0 on success, -1 on error. + */ +int +vips_byteswap( VipsImage *in, VipsImage **out, ... ) +{ + va_list ap; + int result; + + va_start( ap, out ); + result = vips_call_split( "byteswap", ap, in, out ); + va_end( ap ); + + return( result ); +} + +/* Convenience function: swap if @swap is %TRUE, otherwise copy. + */ +int +vips__byteswap_bool( VipsImage *in, VipsImage **out, gboolean swap ) +{ + if( swap ) + return( vips_byteswap( in, out, NULL ) ); + else + return( vips_copy( in, out, NULL ) ); +} diff --git a/libvips/conversion/conversion.c b/libvips/conversion/conversion.c index 4892c08f..b10f07de 100644 --- a/libvips/conversion/conversion.c +++ b/libvips/conversion/conversion.c @@ -248,6 +248,7 @@ vips_conversion_operation_init( void ) extern GType vips_zoom_get_type( void ); extern GType vips_subsample_get_type( void ); extern GType vips_msb_get_type( void ); + extern GType vips_byteswap_get_type( void ); #ifdef HAVE_PANGOFT2 extern GType vips_text_get_type( void ); #endif /*HAVE_PANGOFT2*/ @@ -289,6 +290,7 @@ vips_conversion_operation_init( void ) vips_zoom_get_type(); vips_subsample_get_type(); vips_msb_get_type(); + vips_byteswap_get_type(); #ifdef HAVE_PANGOFT2 vips_text_get_type(); #endif /*HAVE_PANGOFT2*/ diff --git a/libvips/conversion/copy.c b/libvips/conversion/copy.c index 7b946e86..dc6245e5 100644 --- a/libvips/conversion/copy.c +++ b/libvips/conversion/copy.c @@ -51,6 +51,8 @@ * - support bands -> width conversion * 4/6/15 * - support width -> bands conversion + * 5/6/15 + * - move byteswap out to vips_byteswap() */ /* @@ -107,12 +109,9 @@ typedef struct _VipsCopy { */ VipsImage *in; - /* Swap bytes on the way through. - */ - gboolean swap; - /* Fields we can optionally set on the way through. */ + gboolean swap; VipsInterpretation interpretation; double xres; double yres; @@ -130,66 +129,6 @@ typedef VipsConversionClass VipsCopyClass; G_DEFINE_TYPE( VipsCopy, vips_copy, VIPS_TYPE_CONVERSION ); -/* Swap pairs of bytes. - */ -static void -vips_copy_swap2( VipsPel *in, VipsPel *out, int width, VipsImage *im ) -{ - guint16 *p = (guint16 *) in; - guint16 *q = (guint16 *) out; - int sz = (VIPS_IMAGE_SIZEOF_PEL( im ) * width) / 2; - - int x; - - for( x = 0; x < sz; x++ ) - q[x] = GUINT16_SWAP_LE_BE( p[x] ); -} - -/* Swap 4- of bytes. - */ -static void -vips_copy_swap4( VipsPel *in, VipsPel *out, int width, VipsImage *im ) -{ - guint32 *p = (guint32 *) in; - guint32 *q = (guint32 *) out; - int sz = (VIPS_IMAGE_SIZEOF_PEL( im ) * width) / 4; - - int x; - - for( x = 0; x < sz; x++ ) - q[x] = GUINT32_SWAP_LE_BE( p[x] ); -} - -/* Swap 8- of bytes. - */ -static void -vips_copy_swap8( VipsPel *in, VipsPel *out, int width, VipsImage *im ) -{ - guint64 *p = (guint64 *) in; - guint64 *q = (guint64 *) out; - int sz = (VIPS_IMAGE_SIZEOF_PEL( im ) * width) / 8; - - int x; - - for( x = 0; x < sz; x++ ) - q[x] = GUINT64_SWAP_LE_BE( p[x] ); -} - -typedef void (*SwapFn)( VipsPel *in, VipsPel *out, int width, VipsImage *im ); - -static SwapFn vips_copy_swap_fn[] = { - NULL, /* VIPS_FORMAT_UCHAR = 0, */ - NULL, /* VIPS_FORMAT_CHAR = 1, */ - vips_copy_swap2, /* VIPS_FORMAT_USHORT = 2, */ - vips_copy_swap2, /* VIPS_FORMAT_SHORT = 3, */ - vips_copy_swap4, /* VIPS_FORMAT_UINT = 4, */ - vips_copy_swap4, /* VIPS_FORMAT_INT = 5, */ - vips_copy_swap4, /* VIPS_FORMAT_FLOAT = 6, */ - vips_copy_swap4, /* VIPS_FORMAT_COMPLEX = 7, */ - vips_copy_swap8, /* VIPS_FORMAT_DOUBLE = 8, */ - vips_copy_swap8 /* VIPS_FORMAT_DPCOMPLEX = 9, */ -}; - /* Copy, turning bands into the x axis. */ static int @@ -198,10 +137,7 @@ vips_copy_unbandize_gen( VipsRegion *or, { VipsRegion *ir = (VipsRegion *) seq; VipsImage *in = ir->im; - VipsImage *out = or->im; VipsRect *r = &or->valid; - VipsCopy *copy = (VipsCopy *) b; - SwapFn swap = vips_copy_swap_fn[copy->in->BandFmt]; int sze = VIPS_IMAGE_SIZEOF_ELEMENT( in ); VipsRect need; @@ -241,11 +177,7 @@ vips_copy_unbandize_gen( VipsRegion *or, (r->left + x) * sze; q = VIPS_REGION_ADDR( or, r->left + x, r->top + y ); - if( copy->swap && - swap ) - swap( p, q, 1, out ); - else - memcpy( q, p, sze ); + memcpy( q, p, sze ); } } @@ -265,8 +197,6 @@ vips_copy_bandize_gen( VipsRegion *or, VipsImage *in = ir->im; VipsImage *out = or->im; VipsRect *r = &or->valid; - VipsCopy *copy = (VipsCopy *) b; - SwapFn swap = vips_copy_swap_fn[copy->in->BandFmt]; int sze = VIPS_IMAGE_SIZEOF_ELEMENT( in ); VipsRect need; @@ -309,11 +239,7 @@ vips_copy_bandize_gen( VipsRegion *or, q = VIPS_REGION_ADDR( or, 0, r->left + x ); } - if( copy->swap && - swap ) - swap( p, q, 1, out ); - else - memcpy( q, p, out->Bands * sze ); + memcpy( q, p, out->Bands * sze ); } } @@ -327,32 +253,14 @@ vips_copy_gen( VipsRegion *or, void *seq, void *a, void *b, gboolean *stop ) { VipsRegion *ir = (VipsRegion *) seq; VipsRect *r = &or->valid; - VipsCopy *copy = (VipsCopy *) b; - SwapFn swap = vips_copy_swap_fn[copy->in->BandFmt]; /* Ask for input we need. */ if( vips_region_prepare( ir, r ) ) return( -1 ); - if( copy->swap && - swap ) { - int y; - - for( y = 0; y < r->height; y++ ) { - VipsPel *p = VIPS_REGION_ADDR( ir, - r->left, r->top + y ); - VipsPel *q = VIPS_REGION_ADDR( or, - r->left, r->top + y ); - - swap( p, q, r->width, copy->in ); - } - } - else - /* Nothing to do, just copy with pointers. - */ - if( vips_region_region( or, ir, r, r->left, r->top ) ) - return( -1 ); + if( vips_region_region( or, ir, r, r->left, r->top ) ) + return( -1 ); return( 0 ); } @@ -391,6 +299,10 @@ vips_copy_build( VipsObject *object ) if( vips_image_pio_input( copy->in ) ) return( -1 ); + if( copy->swap ) + vips_warn( class->nickname, "%s", + _( "copy swap is deprecated, use byteswap instead" ) ); + if( vips_image_pipelinev( conversion->out, VIPS_DEMAND_STYLE_THINSTRIP, copy->in, NULL ) ) return( -1 ); @@ -512,7 +424,7 @@ vips_copy_class_init( VipsCopyClass *class ) VIPS_ARG_BOOL( class, "swap", 2, _( "Swap" ), _( "Swap bytes in image between little and big-endian" ), - VIPS_ARGUMENT_OPTIONAL_INPUT, + VIPS_ARGUMENT_OPTIONAL_INPUT | VIPS_ARGUMENT_DEPRECATED, G_STRUCT_OFFSET( VipsCopy, swap ), FALSE ); @@ -612,7 +524,6 @@ vips_copy_init( VipsCopy *copy ) * @yres: set image yres * @xoffset: set image xoffset * @yoffset: set image yoffset - * @swap: swap byte order * * Copy an image, optionally modifying the header. VIPS copies images by * copying pointers, so this operation is instant, even for very large images. @@ -623,9 +534,6 @@ vips_copy_init( VipsCopy *copy ) * they are not set carefully. The operation will block changes which make the * image size grow, see VIPS_IMAGE_SIZEOF_IMAGE(). * - * Setting @swap to %TRUE will make vips_copy() swap the byte ordering of - * pixels according to the image's format. - * * You can use this operation to turn 1xN or Nx1 images with M bands into MxN * one band images. * diff --git a/libvips/deprecated/vips7compat.c b/libvips/deprecated/vips7compat.c index 612be800..272f812e 100644 --- a/libvips/deprecated/vips7compat.c +++ b/libvips/deprecated/vips7compat.c @@ -1137,9 +1137,7 @@ im_copy_swap( IMAGE *in, IMAGE *out ) { VipsImage *x; - if( vips_copy( in, &x, - "swap", TRUE, - NULL ) ) + if( vips_byteswap( in, &x, NULL ) ) return( -1 ); if( vips_image_write( x, out ) ) { g_object_unref( x ); diff --git a/libvips/foreign/analyze2vips.c b/libvips/foreign/analyze2vips.c index d92f295c..9489f639 100644 --- a/libvips/foreign/analyze2vips.c +++ b/libvips/foreign/analyze2vips.c @@ -584,7 +584,7 @@ vips__analyze_read( const char *filename, VipsImage *out ) } if( vips_copy( t[0], &t[1], "bands", bands, "format", fmt, NULL ) || - vips_copy( t[1], &t[2], "swap", !vips_amiMSBfirst(), NULL ) || + vips__byteswap_bool( t[1], &t[2], !vips_amiMSBfirst() ) || vips_image_write( t[2], out ) ) { g_object_unref( x ); return( -1 ); diff --git a/libvips/foreign/ppm.c b/libvips/foreign/ppm.c index 61935bf2..cb916e29 100644 --- a/libvips/foreign/ppm.c +++ b/libvips/foreign/ppm.c @@ -297,9 +297,8 @@ read_mmap( FILE *fp, const char *filename, int msb_first, VipsImage *out ) "format", out->BandFmt, "coding", out->Coding, NULL ) || - vips_copy( t[1], &t[2], - "swap", vips_amiMSBfirst() != msb_first, - NULL ) || + vips__byteswap_bool( t[1], &t[2], + vips_amiMSBfirst() != msb_first ) || vips_image_write( t[2], out ) ) { g_object_unref( x ); return( -1 ); diff --git a/libvips/include/vips/conversion.h b/libvips/include/vips/conversion.h index e8e336df..7fc66ad8 100644 --- a/libvips/include/vips/conversion.h +++ b/libvips/include/vips/conversion.h @@ -158,6 +158,8 @@ int vips_scale( VipsImage *in, VipsImage **out, ... ) __attribute__((sentinel)); int vips_msb( VipsImage *in, VipsImage **out, ... ) __attribute__((sentinel)); +int vips_byteswap( VipsImage *in, VipsImage **out, ... ) + __attribute__((sentinel)); int vips_bandjoin( VipsImage **in, VipsImage **out, int n, ... ) __attribute__((sentinel)); diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index ccd20fbd..c0bce202 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -221,6 +221,8 @@ int vips_check_coding_labq( const char *domain, VipsImage *im ); int vips_check_coding_rad( const char *domain, VipsImage *im ); int vips_check_bands_3ormore( const char *domain, VipsImage *im ); +int vips__byteswap_bool( VipsImage *in, VipsImage **out, gboolean swap ); + #ifdef __cplusplus } #endif /*__cplusplus*/ diff --git a/libvips/iofuncs/image.c b/libvips/iofuncs/image.c index 17dd962c..dd05d686 100644 --- a/libvips/iofuncs/image.c +++ b/libvips/iofuncs/image.c @@ -888,9 +888,7 @@ vips_image_build( VipsObject *object ) "v" )) ) return( -1 ); - if( vips_copy( t, &t2, - "swap", TRUE, - NULL ) ) { + if( vips_byteswap( t, &t2, NULL ) ) { g_object_unref( t ); return( -1 ); }