From 261af58e00664489fb8cd4c57eff4d76da306765 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 1 Dec 2011 18:03:41 +0000 Subject: [PATCH] fix up byteswapping path (again) --- TODO | 10 ++-- libvips/format/im_analyze2vips.c | 4 +- libvips/include/vips/internal.h | 7 ++- libvips/include/vips/vips7compat.h | 4 -- libvips/iofuncs/util.c | 2 +- libvips/iofuncs/vips.c | 73 ++++++++++++++++++------------ 6 files changed, 55 insertions(+), 45 deletions(-) diff --git a/TODO b/TODO index f1d517d1..ff9942e8 100644 --- a/TODO +++ b/TODO @@ -1,3 +1,7 @@ +- get copy swap to use the glib byte order macros as well, faster? + + + - test vips_foreign_load_vips_get_flags(), sense inverted? test load/save jpeg buffer @@ -347,12 +351,6 @@ g++ -shared -nostdlib /usr/lib/gcc/x86_64-linux-gnu/4.4.3/../../../../lib/crti.o - look into G_GNUC_DEPRECATED for back compat in vips8 -- use - - http://library.gnome.org/devel/glib/stable/glib-Byte-Order-Macros.html - - for swapping ... they are asm macros so we should see a speedup - - can we use conv_sep to speed up the memuse benchmarks? - check mosaic1, global_balance, similarity etc. use of im__affine diff --git a/libvips/format/im_analyze2vips.c b/libvips/format/im_analyze2vips.c index 4fc4fcb1..54f7ceb3 100644 --- a/libvips/format/im_analyze2vips.c +++ b/libvips/format/im_analyze2vips.c @@ -332,14 +332,14 @@ read_header( const char *header ) case SHORT: p = &G_STRUCT_MEMBER( unsigned char, d, dsr_header[i].offset ); - im__read_2byte( 1, p, &p ); + vips__copy_2byte( TRUE, p, p ); break; case INT: case FLOAT: p = &G_STRUCT_MEMBER( unsigned char, d, dsr_header[i].offset ); - im__read_4byte( 1, p, &p ); + vips__copy_4byte( TRUE, p, p ); break; case BYTE: diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index fc8b7617..5754a254 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -103,10 +103,9 @@ int vips_remapfilerw( VipsImage * ); void vips__buffer_init( void ); -void vips__read_4byte( int msb_first, unsigned char *to, unsigned char **from ); -void vips__read_2byte( int msb_first, unsigned char *to, unsigned char **from ); -void vips__write_4byte( unsigned char **to, unsigned char *from ); -void vips__write_2byte( unsigned char **to, unsigned char *from ); +void vips__copy_4byte( int swap, unsigned char *to, unsigned char *from ); +void vips__copy_2byte( gboolean swap, unsigned char *to, unsigned char *from ); + guint32 vips__file_magic( const char *filename ); int vips__has_extension_block( VipsImage *im ); void *vips__read_extension_block( VipsImage *im, int *size ); diff --git a/libvips/include/vips/vips7compat.h b/libvips/include/vips/vips7compat.h index 7a43eb9c..afb191a3 100644 --- a/libvips/include/vips/vips7compat.h +++ b/libvips/include/vips/vips7compat.h @@ -450,10 +450,6 @@ size_t im_ref_string_get_length( const GValue *value ); #define im__open_image_read vips__open_image_read #define im_image_open_input vips_image_open_input #define im_image_open_output vips_image_open_output -#define im__read_4byte vips__read_4byte -#define im__read_2byte vips__read_2byte -#define im__write_4byte vips__write_4byte -#define im__write_2byte vips__write_2byte #define im__has_extension_block vips__has_extension_block #define im__read_extension_block vips__read_extension_block #define im__write_extension_block vips__write_extension_block diff --git a/libvips/iofuncs/util.c b/libvips/iofuncs/util.c index 7ab3611d..ec1703d0 100644 --- a/libvips/iofuncs/util.c +++ b/libvips/iofuncs/util.c @@ -852,7 +852,7 @@ vips__file_write( void *data, size_t size, size_t nmemb, FILE *stream ) if( (n = fwrite( data, size, nmemb, stream )) != nmemb ) { vips_error( "vips__file_write", - _( "writing error (%zd out of %zd blocks written) " + _( "write error (%zd out of %zd blocks written) " "... disc full?" ), n, nmemb ); return( -1 ); } diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 6c676021..fccf098f 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -47,6 +47,7 @@ /* #define DEBUG +#define SHOW_HEADER */ #ifdef HAVE_CONFIG_H @@ -170,32 +171,30 @@ image_pixel_length( VipsImage *image ) return( psize + image->sizeof_header ); } -/* Copy 2 and 4 bytes, swapping to and from native. +/* Copy 2 and 4 bytes, optionally swapping byte order. */ void -vips__copy_4byte( int msb_first, unsigned char *to, unsigned char *from ) +vips__copy_4byte( int swap, unsigned char *to, unsigned char *from ) { - guint32 out; + guint32 *in = (guint32 *) from; + guint32 *out = (guint32 *) to; - if( msb_first ) - out = from[0] << 24 | from[1] << 16 | from[2] << 8 | from[3]; + if( swap ) + *out = GUINT32_SWAP_LE_BE( *in ); else - out = from[3] << 24 | from[2] << 16 | from[1] << 8 | from[0]; - - *((guint32 *) to) = out; + *out = *in; } void -vips__copy_2byte( int msb_first, unsigned char *to, unsigned char *from ) +vips__copy_2byte( gboolean swap, unsigned char *to, unsigned char *from ) { - guint16 out; + guint16 *in = (guint16 *) from; + guint16 *out = (guint16 *) to; - if( msb_first ) - out = from[0] << 8 | from[1]; + if( swap ) + *out = GUINT16_SWAP_LE_BE( *in ); else - out = from[1] << 8 | from[0]; - - *((guint16 *) to) = out; + *out = *in; } guint32 @@ -216,7 +215,7 @@ vips__file_magic( const char *filename ) typedef struct _FieldIO { glong offset; int size; - void (*copy)( int msb_first, unsigned char *to, unsigned char *from ); + void (*copy)( gboolean swap, unsigned char *to, unsigned char *from ); } FieldIO; static FieldIO fields[] = { @@ -239,10 +238,19 @@ static FieldIO fields[] = { int vips__read_header_bytes( VipsImage *im, unsigned char *from ) { - int msb_first; + gboolean swap; int i; - vips__copy_4byte( 1, (unsigned char *) &im->magic, from ); +#ifdef SHOW_HEADER + printf( "vips__read_header_bytes: file bytes:\n" ); + for( i = 0; i < im->sizeof_header; i++ ) + printf( "%2d - 0x%02x\n", i, from[i] ); +#endif /*SHOW_HEADER*/ + + /* The magic number is always written MSB first, we may need to swap. + */ + vips__copy_4byte( !vips_amiMSBfirst(), + (unsigned char *) &im->magic, from ); from += 4; if( im->magic != VIPS_MAGIC_INTEL && im->magic != VIPS_MAGIC_SPARC ) { @@ -251,10 +259,13 @@ vips__read_header_bytes( VipsImage *im, unsigned char *from ) return( -1 ); } - msb_first = im->magic == VIPS_MAGIC_SPARC; + /* We need to swap for other fields if the file byte order is + * different from ours. + */ + swap = vips_amiMSBfirst() != (im->magic == VIPS_MAGIC_SPARC); for( i = 0; i < VIPS_NUMBER( fields ); i++ ) { - fields[i].copy( msb_first, + fields[i].copy( swap, &G_STRUCT_MEMBER( unsigned char, im, fields[i].offset ), from ); from += fields[i].size; @@ -270,22 +281,22 @@ vips__read_header_bytes( VipsImage *im, unsigned char *from ) int vips__write_header_bytes( VipsImage *im, unsigned char *to ) { - int msb_first; + /* Swap if the byte order we are asked to write the header in is + * different from ours. + */ + gboolean swap = vips_amiMSBfirst() != (im->magic == VIPS_MAGIC_SPARC); + int i; unsigned char *q; /* Always write the magic number MSB first. */ - to[0] = im->magic >> 24; - to[1] = im->magic >> 16; - to[2] = im->magic >> 8; - to[3] = im->magic; + vips__copy_4byte( !vips_amiMSBfirst(), + to, (unsigned char *) &im->magic ); q = to + 4; - msb_first = im->magic == VIPS_MAGIC_SPARC; - for( i = 0; i < VIPS_NUMBER( fields ); i++ ) { - fields[i].copy( msb_first, + fields[i].copy( swap, q, &G_STRUCT_MEMBER( unsigned char, im, fields[i].offset ) ); @@ -297,6 +308,12 @@ vips__write_header_bytes( VipsImage *im, unsigned char *to ) while( q - to < im->sizeof_header ) *q++ = 0; +#ifdef SHOW_HEADER + printf( "vips__write_header_bytes: file bytes:\n" ); + for( i = 0; i < im->sizeof_header; i++ ) + printf( "%2d - 0x%02x\n", i, to[i] ); +#endif /*SHOW_HEADER*/ + return( 0 ); }