From 9ba8d3208311dc191a57d9b6efa51ac60597f36a Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 24 Feb 2017 14:28:08 +0000 Subject: [PATCH 01/11] add expat to package deps --- TODO | 4 ++++ configure.ac | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/TODO b/TODO index 40c1ca70..e48e193b 100644 --- a/TODO +++ b/TODO @@ -1,3 +1,7 @@ +- added expat to required packages in configure.ac + + + - vips linecache has access there twice! $ vips linecache diff --git a/configure.ac b/configure.ac index 78d7acc5..b7a7cd8c 100644 --- a/configure.ac +++ b/configure.ac @@ -348,8 +348,9 @@ AC_CHECK_LIB(m,atan2,[AC_DEFINE(HAVE_ATAN2,1,[have atan2() in libm.])]) # have to have these # need glib 2.6 for GOption -PKG_CHECK_MODULES(REQUIRED, glib-2.0 >= 2.6 gmodule-2.0 libxml-2.0 gobject-2.0) -PACKAGES_USED="$PACKAGES_USED glib-2.0 libxml-2.0 gmodule-2.0 gobject-2.0" +PKG_CHECK_MODULES(REQUIRED, glib-2.0 >= 2.6 gmodule-2.0 gobject-2.0 + libxml-2.0 expat) +PACKAGES_USED="$PACKAGES_USED glib-2.0 gmodule-2.0 gobject-2.0 libxml-2.0 expat" # after 2.28 we have a monotonic timer PKG_CHECK_MODULES(MONOTONIC, glib-2.0 >= 2.28, From dbbe8b77c176ee571fceae229ee89c4350f7d3fd Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 24 Feb 2017 14:30:26 +0000 Subject: [PATCH 02/11] vipsheader could crash on bad field names we were not checking the return of vips_image_get_as_string() --- tools/vipsheader.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/vipsheader.c b/tools/vipsheader.c index 5d2892c4..f6114719 100644 --- a/tools/vipsheader.c +++ b/tools/vipsheader.c @@ -149,7 +149,8 @@ print_header( VipsImage *im, gboolean many ) else { char *str; - vips_image_get_as_string( im, main_option_field, &str ); + if( vips_image_get_as_string( im, main_option_field, &str ) ) + return( -1 ); printf( "%s\n", str ); g_free( str ); } From dc2b567ee21b7bb743c895527a4b3bdea126149f Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 24 Feb 2017 14:57:20 +0000 Subject: [PATCH 03/11] start looking at xml parse use in vips.c --- TODO | 42 ++++++++++++++++++++++++++++++++++++++++++ libvips/iofuncs/vips.c | 1 + 2 files changed, 43 insertions(+) diff --git a/TODO b/TODO index e48e193b..468b151b 100644 --- a/TODO +++ b/TODO @@ -1,5 +1,47 @@ - added expat to required packages in configure.ac + try moving iofuncs/vips.c to expat + + sample XML for us to parse + + $ vipsheader -f getext babe.v + + +
+ +
+ + jpegload + 0 + 4294966289/169093161 (25.399999998, Rational, 1 components, 8 bytes) + 4294966289/169093161 (25.399999998, Rational, 1 components, 8 bytes) + 2 (Inch, Short, 1 components, 2 bytes) + Exif Version 2.1 (Exif Version 2.1, Undefined, 4 components, 4 bytes) + FlashPix Version 1.0 (FlashPix Version 1.0, Undefined, 4 components, 4 bytes) + 65535 (Internal error (unknown value 65535), Short, 1 components, 2 bytes) + in + RXhpZgAATU0AKgAAAAgABAEaAAUAAAABAAAAPgEbAAUAAAABAAAARgEoAAMAAAABAAIAAIdpAAQA + AAABAAAATgAAAAD///wRChQoKf///BEKFCgpAAOQAAAHAAAABDAyMTCgAAAHAAAABDAxMDCgAQAD + AAAAAf//AAAAAAAA + + + + + currently works like this + + read_xml is called on image load + - loads extension bytes into memory + - calls libxml to parse it to an xmlDoc + - saves xmlDoc to VIPS_META_XML in header + + rebuild_header called next + - gets xmlDoc from header + - gets "header" node + calls rebuild_header_builtin() for every field node + - gets "meta" field + calls rebuild_header_meta() for every field node + + - vips linecache has access there twice! diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 749007ff..90b76225 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -78,6 +78,7 @@ #include #endif /*HAVE_IO_H*/ #include +#include #include #ifdef OS_WIN32 From 134ce0560ca605e42d3c5a60e472a07a5affad5e Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 25 Feb 2017 13:07:43 +0000 Subject: [PATCH 04/11] use expat for xml read we were using libxml for xml load, use expat instead, we get it for free with glib --- ChangeLog | 2 + libvips/include/vips/header.h | 8 - libvips/iofuncs/vips.c | 356 +++++++++++++++++----------------- 3 files changed, 182 insertions(+), 184 deletions(-) diff --git a/ChangeLog b/ChangeLog index a5f61e89..01e118fe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -34,6 +34,8 @@ - vips_region_shrink() knows about alpha, helps dzsave and tiffsave - vips_sequential() no longer stalls ahead threads, instead we constrain sinks more tightly ... this makes seq more versatile and more reliable +- use expat, not libxml, for XML load ... removes a required dependency, since + we get expat as part of glib 8/12/16 started 8.4.5 - allow libgsf-1.14.26 to help centos, thanks tdiprima diff --git a/libvips/include/vips/header.h b/libvips/include/vips/header.h index 1ae81011..98229c7f 100644 --- a/libvips/include/vips/header.h +++ b/libvips/include/vips/header.h @@ -77,14 +77,6 @@ extern "C" { */ #define VIPS_META_ICC_NAME "icc-profile-data" -/** - * VIPS_META_XML: - * - * The original XML that was used to code the metadata after reading a VIPS - * format file. - */ -#define VIPS_META_XML "xml-header" - /** * VIPS_META_IMAGEDESCRIPTION: * diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 90b76225..7c5342d9 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -17,6 +17,8 @@ * image.c ... this file now just does read / write to disc * 28/3/11 * - moved to vips_ namespace + * 25/2/17 + * - use expat for xml read */ /* @@ -435,9 +437,9 @@ vips__read_extension_block( VipsImage *im, int *size ) psize = image_pixel_length( im ); g_assert( im->file_length > 0 ); - if( im->file_length - psize > 10 * 1024 * 1024 ) { + if( im->file_length - psize > 100 * 1024 * 1024 ) { vips_error( "VipsImage", - "%s", _( "more than a 10 megabytes of XML? " + "%s", _( "more than 100 megabytes of XML? " "sufferin' succotash!" ) ); return( NULL ); } @@ -457,74 +459,108 @@ vips__read_extension_block( VipsImage *im, int *size ) return( buf ); } -/* Read everything after the pixels into memory. - - FIXME ... why can't we use xmlParserInputBufferCreateFd and parse - directly from the fd rather than having to read the stupid thing into - memory - - the libxml API docs are impossible to decipher - - */ -static xmlDoc * -read_xml( VipsImage *im ) +static int +parser_read_fd( XML_Parser parser, int fd ) { - void *buf; - int size; - xmlDoc *doc; - xmlNode *node; + const int chunk_size = 1024; - if( !(buf = vips__read_extension_block( im, &size )) ) - return( NULL ); - if( !(doc = xmlParseMemory( buf, size )) ) { - vips_free( buf ); - return( NULL ); - } - vips_free( buf ); - if( !(node = xmlDocGetRootElement( doc )) || - !node->nsDef || - !vips_isprefix( NAMESPACE, (char *) node->nsDef->href ) ) { - vips_error( "VipsImage", - "%s", _( "incorrect namespace in XML" ) ); - xmlFreeDoc( doc ); - return( NULL ); - } + ssize_t bytes_read; + + do { + void *buf; + + if( !(buf = XML_GetBuffer( parser, chunk_size )) ) { + vips_error( "VipsImage", + "%s", _( "unable to allocate read buffer" ) ); + return( -1 ); + } + bytes_read = read( fd, buf, chunk_size ); + if( bytes_read == (ssize_t) -1 ) { + vips_error( "VipsImage", + "%s", _( "read error while fetching XML" ) ); + return( -1 ); + } + + if( !XML_ParseBuffer( parser, bytes_read, bytes_read == 0 ) ) { + vips_error( "VipsImage", + "%s", _( "XML parse error" ) ); + return( -1 ); + } + } while( bytes_read > 0 ); + + return( 0 ); +} + +#define MAX_PARSE_ATTR (256) + +/* What we track during expat parse. + */ +typedef struct _VipsExpatParse { + VipsImage *image; + + /* TRUE for in header section. + */ + gboolean header; + + /* For the current node, the type and name. + */ + XML_Char type[MAX_PARSE_ATTR]; + XML_Char name[MAX_PARSE_ATTR]; + + /* Accumulate data here. + */ + char *data; + size_t current_size; + + gboolean error; +} VipsExpatParse; + +static void +parser_element_start_handler( void *user_data, + const XML_Char *name, const XML_Char **atts ) +{ + VipsExpatParse *vep = (VipsExpatParse *) user_data; + const XML_Char **p; #ifdef DEBUG - printf( "read_xml: namespace == %s\n", node->nsDef->href ); + printf( "parser_element_start: %s\n", name ); + for( p = atts; *p; p += 2 ) + printf( "%s = %s\n", p[0], p[1] ); #endif /*DEBUG*/ - return( doc ); + if( strcmp( name, "field" ) == 0 ) { + for( p = atts; *p; p += 2 ) { + if( strcmp( p[0], "name" ) == 0 ) + vips_strncpy( vep->name, p[1], MAX_PARSE_ATTR ); + if( strcmp( p[0], "type" ) == 0 ) + vips_strncpy( vep->type, p[1], MAX_PARSE_ATTR ); + } + + vep->current_size = 0; + } + else if( strcmp( name, "header" ) == 0 ) + vep->header = TRUE; + else if( strcmp( name, "meta" ) == 0 ) + vep->header = FALSE; + else if( strcmp( name, "root" ) == 0 ) { + for( p = atts; *p; p += 2 ) + if( strcmp( p[0], "xmlns" ) == 0 && + !vips_isprefix( NAMESPACE, p[1] ) ) { + vips_error( "VipsImage", "%s", + _( "incorrect namespace in XML" ) ); + vep->error = TRUE; + } + } } -/* Find the first child node with a name. - */ -static xmlNode * -get_node( xmlNode *base, const char *name ) +static void +parser_append( VipsExpatParse *vep, const XML_Char *data, int len ) { - xmlNode *i; + size_t new_size = vep->current_size + len; - for( i = base->children; i; i = i->next ) - if( strcmp( (char *) i->name, name ) == 0 ) - return( i ); - - return( NULL ); -} - -/* Read a string property to a buffer. TRUE for success. - */ -static int -get_sprop( xmlNode *xnode, const char *name, char *buf, int sz ) -{ - char *value = (char *) xmlGetProp( xnode, (xmlChar *) name ); - - if( !value ) - return( 0 ); - - vips_strncpy( buf, value, sz ); - VIPS_FREEF( xmlFree, value ); - - return( 1 ); + vep->data = g_realloc( vep->data, new_size ); + memcpy( vep->data + vep->current_size, data, len ); + vep->current_size = new_size; } /* Chop history into lines, add each one as a refstring. @@ -556,119 +592,76 @@ set_history( VipsImage *im, char *history ) im->history_list = g_slist_reverse( history_list ); } -/* Load header fields. - */ static int -rebuild_header_builtin( VipsImage *im, xmlNode *i ) +set_meta( VipsImage *image, GType gtype, const char *name, const char *data ) { - char name[256]; + GValue save_value = { 0 }; + GValue value = { 0 }; - if( get_sprop( i, "name", name, 256 ) ) { - if( strcmp( name, "Hist" ) == 0 ) { - char *history; + g_value_init( &save_value, VIPS_TYPE_SAVE_STRING ); + vips_value_set_save_string( &save_value, data ); - /* Have to take (another) copy, since we need to free - * with xmlFree(). + g_value_init( &value, gtype ); + if( !g_value_transform( &save_value, &value ) ) { + g_value_unset( &save_value ); + vips_error( "VipsImage", "%s", + _( "error transforming from save format" ) ); + return( -1 ); + } + + vips_image_set( image, name, &value ); + g_value_unset( &save_value ); + g_value_unset( &value ); + + return( 0 ); +} + +static void +parser_element_end_handler( void *user_data, const XML_Char *name ) +{ + VipsExpatParse *vep = (VipsExpatParse *) user_data; + +#ifdef DEBUG + printf( "parser_element_end_handler: %s\n", name ); +#endif /*DEBUG*/ + + if( strcmp( name, "field" ) == 0 ) { + parser_append( vep, "", 1 ); + +#ifdef DEBUG + printf( "parser_element_end_handler: %zd bytes\n", + vep->current_size ); +#endif /*DEBUG*/ + + if( vep->header ) { + if( strcmp( name, "Hist" ) == 0 ) + set_history( vep->image, vep->data ); + } + else { + GType gtype = g_type_from_name( vep->type ); + + /* Can we convert from VIPS_SAVE_STRING to type? */ - history = (char *) xmlNodeGetContent( i ); - set_history( im, history ); - xmlFree( history ); + if( gtype && + g_value_type_transformable( + VIPS_TYPE_SAVE_STRING, gtype ) && + set_meta( vep->image, + gtype, vep->name, vep->data ) ) + vep->error = TRUE; } } - - return( 0 ); } -/* Load meta fields. - */ -static int -rebuild_header_meta( VipsImage *im, xmlNode *i ) +static void +parser_data_handler( void *user_data, const XML_Char *data, int len ) { - char name[256]; - char type[256]; + VipsExpatParse *vep = (VipsExpatParse *) user_data; - if( get_sprop( i, "name", name, 256 ) && - get_sprop( i, "type", type, 256 ) ) { - GType gtype = g_type_from_name( type ); +#ifdef DEBUG + printf( "parser_data_handler: %d bytes\n", len ); +#endif /*DEBUG*/ - /* Can we convert from VIPS_SAVE_STRING to type? - */ - if( gtype && - g_value_type_transformable( - VIPS_TYPE_SAVE_STRING, gtype ) ) { - char *content; - GValue save_value = { 0 }; - GValue value = { 0 }; - - content = (char *) xmlNodeGetContent( i ); - g_value_init( &save_value, VIPS_TYPE_SAVE_STRING ); - vips_value_set_save_string( &save_value, content ); - xmlFree( content ); - - g_value_init( &value, gtype ); - if( !g_value_transform( &save_value, &value ) ) { - g_value_unset( &save_value ); - vips_error( "VipsImage", - "%s", _( "error transforming from " - "save format" ) ); - return( -1 ); - } - vips_image_set( im, name, &value ); - g_value_unset( &save_value ); - g_value_unset( &value ); - } - } - - return( 0 ); -} - -static xmlDoc * -get_xml( VipsImage *im ) -{ - if( vips_image_get_typeof( im, VIPS_META_XML ) ) { - xmlDoc *doc; - - if( vips_image_get_area( im, VIPS_META_XML, (void *) &doc ) ) - return( NULL ); - - return( doc ); - } - - return( NULL ); -} - -/* Rebuild header fields that depend on stuff saved in xml. - */ -static int -rebuild_header( VipsImage *im ) -{ - xmlDoc *doc; - - if( (doc = get_xml( im )) ) { - xmlNode *root; - xmlNode *block; - - if( !(root = xmlDocGetRootElement( doc )) ) - return( -1 ); - if( (block = get_node( root, "header" )) ) { - xmlNode *i; - - for( i = block->children; i; i = i->next ) - if( strcmp( (char *) i->name, "field" ) == 0 ) - if( rebuild_header_builtin( im, i ) ) - return( -1 ); - } - if( (block = get_node( root, "meta" )) ) { - xmlNode *i; - - for( i = block->children; i; i = i->next ) - if( strcmp( (char *) i->name, "field" ) == 0 ) - if( rebuild_header_meta( im, i ) ) - return( -1 ); - } - } - - return( 0 ); + parser_append( vep, data, len ); } /* Called at the end of vips open ... get any XML after the pixel data @@ -677,24 +670,35 @@ rebuild_header( VipsImage *im ) static int readhist( VipsImage *im ) { - /* Junk any old xml meta. - */ - if( vips_image_get_typeof( im, VIPS_META_XML ) ) - vips_image_set_area( im, VIPS_META_XML, NULL, NULL ); + XML_Parser parser; + VipsExpatParse vep; - if( vips__has_extension_block( im ) ) { - xmlDoc *doc; - - if( !(doc = read_xml( im )) ) - return( -1 ); - vips_image_set_area( im, VIPS_META_XML, - (VipsCallbackFn) xmlFreeDoc, doc ); - } - - if( rebuild_header( im ) ) + if( vips__seek( im->fd, image_pixel_length( im ) ) ) return( -1 ); - return( 0 ); + parser = XML_ParserCreate( "UTF-8" ); + + vep.image = im; + vep.data = NULL; + vep.current_size = 0; + vep.error = FALSE; + XML_SetUserData( parser, &vep ); + + XML_SetElementHandler( parser, + parser_element_start_handler, parser_element_end_handler ); + XML_SetCharacterDataHandler( parser, parser_data_handler ); + + if( parser_read_fd( parser, im->fd ) || + vep.error ) { + g_free( vep.data ); + XML_ParserFree( parser ); + return( -1 ); + } + + g_free( vep.data ); + XML_ParserFree( parser ); + + return( 0 ); } #define MAX_STRSIZE (32768) /* Max size of text for stack strings */ From e87654fcd96bdc093dcaae5e7d73295e986132f1 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 25 Feb 2017 17:28:48 +0000 Subject: [PATCH 05/11] use printf for xml write --- ChangeLog | 1 + libvips/iofuncs/vips.c | 297 +++++++++++++++++++---------------------- 2 files changed, 142 insertions(+), 156 deletions(-) diff --git a/ChangeLog b/ChangeLog index 01e118fe..a045cb33 100644 --- a/ChangeLog +++ b/ChangeLog @@ -36,6 +36,7 @@ more tightly ... this makes seq more versatile and more reliable - use expat, not libxml, for XML load ... removes a required dependency, since we get expat as part of glib +- use printf for xml write 8/12/16 started 8.4.5 - allow libgsf-1.14.26 to help centos, thanks tdiprima diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 7c5342d9..5244aa04 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -18,7 +18,7 @@ * 28/3/11 * - moved to vips_ namespace * 25/2/17 - * - use expat for xml read + * - use expat for xml read, printf for xml write */ /* @@ -79,7 +79,6 @@ #ifdef HAVE_IO_H #include #endif /*HAVE_IO_H*/ -#include #include #include @@ -493,11 +492,65 @@ parser_read_fd( XML_Parser parser, int fd ) #define MAX_PARSE_ATTR (256) +/* A memory buffer that expands as we write to it. + */ +typedef struct _Buffer { + char *data; + size_t current_size; +} Buffer; + +static void +buffer_init( Buffer *buffer ) +{ + buffer->data = NULL; + buffer->current_size = 0; +} + +static void +buffer_append( Buffer *buffer, const char *data, int len ) +{ + size_t new_size = buffer->current_size + len; + + buffer->data = g_realloc( buffer->data, new_size ); + memcpy( buffer->data + buffer->current_size, data, len ); + buffer->current_size = new_size; +} + +static void +buffer_appendf( Buffer *buffer, const char *fmt, ... ) +{ + va_list ap; + char line[256]; + + va_start( ap, fmt ); + (void) vips_vsnprintf( line, 256, fmt, ap ); + va_end( ap ); + + buffer_append( buffer, line, strlen( line ) ); +} + +static void +buffer_rewind( Buffer *buffer ) +{ + buffer->current_size = 0; +} + +static void +buffer_destroy( Buffer *buffer ) +{ + VIPS_FREE( buffer->data ); + buffer->current_size = 0; +} + /* What we track during expat parse. */ typedef struct _VipsExpatParse { VipsImage *image; + /* Set on error. + */ + gboolean error; + /* TRUE for in header section. */ gboolean header; @@ -509,10 +562,7 @@ typedef struct _VipsExpatParse { /* Accumulate data here. */ - char *data; - size_t current_size; - - gboolean error; + Buffer buffer; } VipsExpatParse; static void @@ -536,7 +586,7 @@ parser_element_start_handler( void *user_data, vips_strncpy( vep->type, p[1], MAX_PARSE_ATTR ); } - vep->current_size = 0; + buffer_rewind( &vep->buffer ); } else if( strcmp( name, "header" ) == 0 ) vep->header = TRUE; @@ -553,16 +603,6 @@ parser_element_start_handler( void *user_data, } } -static void -parser_append( VipsExpatParse *vep, const XML_Char *data, int len ) -{ - size_t new_size = vep->current_size + len; - - vep->data = g_realloc( vep->data, new_size ); - memcpy( vep->data + vep->current_size, data, len ); - vep->current_size = new_size; -} - /* Chop history into lines, add each one as a refstring. */ static void @@ -626,7 +666,7 @@ parser_element_end_handler( void *user_data, const XML_Char *name ) #endif /*DEBUG*/ if( strcmp( name, "field" ) == 0 ) { - parser_append( vep, "", 1 ); + buffer_append( &vep->buffer, "", 1 ); #ifdef DEBUG printf( "parser_element_end_handler: %zd bytes\n", @@ -635,7 +675,7 @@ parser_element_end_handler( void *user_data, const XML_Char *name ) if( vep->header ) { if( strcmp( name, "Hist" ) == 0 ) - set_history( vep->image, vep->data ); + set_history( vep->image, vep->buffer.data ); } else { GType gtype = g_type_from_name( vep->type ); @@ -646,7 +686,7 @@ parser_element_end_handler( void *user_data, const XML_Char *name ) g_value_type_transformable( VIPS_TYPE_SAVE_STRING, gtype ) && set_meta( vep->image, - gtype, vep->name, vep->data ) ) + gtype, vep->name, vep->buffer.data ) ) vep->error = TRUE; } } @@ -661,7 +701,7 @@ parser_data_handler( void *user_data, const XML_Char *data, int len ) printf( "parser_data_handler: %d bytes\n", len ); #endif /*DEBUG*/ - parser_append( vep, data, len ); + buffer_append( &vep->buffer, data, len ); } /* Called at the end of vips open ... get any XML after the pixel data @@ -679,8 +719,7 @@ readhist( VipsImage *im ) parser = XML_ParserCreate( "UTF-8" ); vep.image = im; - vep.data = NULL; - vep.current_size = 0; + buffer_init( &vep.buffer ); vep.error = FALSE; XML_SetUserData( parser, &vep ); @@ -690,116 +729,17 @@ readhist( VipsImage *im ) if( parser_read_fd( parser, im->fd ) || vep.error ) { - g_free( vep.data ); + buffer_destroy( &vep.buffer ); XML_ParserFree( parser ); return( -1 ); } - g_free( vep.data ); + buffer_destroy( &vep.buffer ); XML_ParserFree( parser ); return( 0 ); } -#define MAX_STRSIZE (32768) /* Max size of text for stack strings */ - -static int -set_prop( xmlNode *node, const char *name, const char *fmt, ... ) -{ - va_list ap; - char value[MAX_STRSIZE]; - - va_start( ap, fmt ); - (void) vips_vsnprintf( value, MAX_STRSIZE, fmt, ap ); - va_end( ap ); - - if( !xmlSetProp( node, (xmlChar *) name, (xmlChar *) value ) ) { - vips_error( "VipsImage", _( "unable to set property \"%s\" " - "to value \"%s\"." ), - name, value ); - return( -1 ); - } - - return( 0 ); -} - -static int -set_sprop( xmlNode *node, const char *name, const char *value ) -{ - if( value && set_prop( node, name, "%s", value ) ) - return( -1 ); - - return( 0 ); -} - -static int -set_field( xmlNode *node, - const char *name, const char *type, const char *content ) -{ - xmlNode *field; - - if( !(field = xmlNewChild( node, NULL, (xmlChar *) "field", NULL )) || - set_sprop( field, "type", type ) || - set_sprop( field, "name", name ) ) - return( -1 ); - xmlNodeSetContent( field, (xmlChar *) content ); - - return( 0 ); -} - -static void * -save_fields_meta( VipsMeta *meta, xmlNode *node ) -{ - GType type = G_VALUE_TYPE( &meta->value ); - - /* If we can transform to VIPS_TYPE_SAVE_STRING and back, we can save - * and restore. - */ - if( g_value_type_transformable( type, VIPS_TYPE_SAVE_STRING ) && - g_value_type_transformable( VIPS_TYPE_SAVE_STRING, type ) ) { - GValue save_value = { 0 }; - - g_value_init( &save_value, VIPS_TYPE_SAVE_STRING ); - if( !g_value_transform( &meta->value, &save_value ) ) { - vips_error( "VipsImage", "%s", - _( "error transforming to save format" ) ); - return( node ); - } - if( set_field( node, meta->name, g_type_name( type ), - vips_value_get_save_string( &save_value ) ) ) { - g_value_unset( &save_value ); - return( node ); - } - g_value_unset( &save_value ); - } - - return( NULL ); -} - -static int -save_fields( VipsImage *im, xmlNode *node ) -{ - xmlNode *this; - - /* Save header fields. - */ - if( !(this = xmlNewChild( node, NULL, (xmlChar *) "header", NULL )) ) - return( -1 ); - if( set_field( this, "Hist", - g_type_name( VIPS_TYPE_REF_STRING ), - vips_image_get_history( im ) ) ) - return( -1 ); - - if( !(this = xmlNewChild( node, NULL, (xmlChar *) "meta", NULL )) ) - return( -1 ); - if( im->meta_traverse && - vips_slist_map2( im->meta_traverse, - (VipsSListMap2Fn) save_fields_meta, this, NULL ) ) - return( -1 ); - - return( 0 ); -} - int vips__write_extension_block( VipsImage *im, void *buf, int size ) { @@ -828,52 +768,97 @@ vips__write_extension_block( VipsImage *im, void *buf, int size ) return( 0 ); } +static void * +build_xml_meta( VipsMeta *meta, Buffer *buffer ) +{ + GType type = G_VALUE_TYPE( &meta->value ); + + const char *str; + + /* If we can transform to VIPS_TYPE_SAVE_STRING and back, we can save + * and restore. + */ + if( g_value_type_transformable( type, VIPS_TYPE_SAVE_STRING ) && + g_value_type_transformable( VIPS_TYPE_SAVE_STRING, type ) ) { + GValue save_value = { 0 }; + + g_value_init( &save_value, VIPS_TYPE_SAVE_STRING ); + if( !g_value_transform( &meta->value, &save_value ) ) { + vips_error( "VipsImage", "%s", + _( "error transforming to save format" ) ); + return( buffer ); + } + + str = vips_value_get_save_string( &save_value ); + buffer_appendf( buffer, " ", + g_type_name( type ), meta->name ); + buffer_append( buffer, str, strlen( str ) ); + buffer_appendf( buffer, "\n" ); + + g_value_unset( &save_value ); + } + + return( NULL ); +} + +static char * +build_xml( VipsImage *image ) +{ + Buffer buffer; + const char *str; + + buffer_init( &buffer ); + + buffer_appendf( &buffer, "\n" ); + buffer_appendf( &buffer, "\n", + NAMESPACE, + VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION ); + buffer_appendf( &buffer, "
\n" ); + + str = vips_image_get_history( image ); + buffer_appendf( &buffer, " ", + g_type_name( VIPS_TYPE_REF_STRING ) ); + buffer_append( &buffer, str, strlen( str ) ); + buffer_appendf( &buffer, "\n" ); + + buffer_appendf( &buffer, "
\n" ); + buffer_appendf( &buffer, " \n" ); + + if( vips_slist_map2( image->meta_traverse, + (VipsSListMap2Fn) build_xml_meta, &buffer, NULL ) ) { + buffer_destroy( &buffer ); + return( NULL ); + } + + buffer_appendf( &buffer, " \n" ); + buffer_appendf( &buffer, "\n" ); + + return( buffer.data ); +} + /* Append XML to output fd. */ int -vips__writehist( VipsImage *im ) +vips__writehist( VipsImage *image ) { - xmlDoc *doc; - char namespace[256]; - char *dump; - int dump_size; + char *xml; - assert( im->dtype == VIPS_IMAGE_OPENOUT ); - assert( im->fd != -1 ); + assert( image->dtype == VIPS_IMAGE_OPENOUT ); + assert( image->fd != -1 ); - if( !(doc = xmlNewDoc( (xmlChar *) "1.0" )) ) + if( !(xml = build_xml( im )) ) return( -1 ); - vips_snprintf( namespace, 256, "%s/%d.%d.%d", - NAMESPACE, - VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION ); - if( !(doc->children = xmlNewDocNode( doc, - NULL, (xmlChar *) "root", NULL )) || - set_sprop( doc->children, "xmlns", namespace ) || - save_fields( im, doc->children ) ) { - vips_error( "VipsImage", "%s", _( "xml save error" ) ); - xmlFreeDoc( doc ); - return( -1 ); - } - - xmlDocDumpFormatMemory( doc, (xmlChar **) &dump, &dump_size, 1 ); - if( !dump ) { - vips_error( "VipsImage", "%s", _( "xml save error" ) ); - xmlFreeDoc( doc ); - return( -1 ); - } - xmlFreeDoc( doc ); - - if( vips__write_extension_block( im, dump, dump_size ) ) { - xmlFree( dump ); + if( vips__write_extension_block( image, xml, strlen( xml ) ) ) { + g_free( xml ); return( -1 ); } #ifdef DEBUG - printf( "vips__writehist: saved XML is: \"%s\"", dump ); + printf( "vips__writehist: saved XML is: \"%s\"", xml ); #endif /*DEBUG*/ - xmlFree( dump ); + g_free( xml ); return( 0 ); } From 5614330cc42651532bd4cbb1aa4daf319771d3c8 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 25 Feb 2017 18:10:42 +0000 Subject: [PATCH 06/11] remove libxml from xml save just some printfs now --- TODO | 149 ++++++++++++++++++++++++-------- configure.ac | 5 +- libvips/foreign/dzsave.c | 140 +----------------------------- libvips/foreign/tiff.c | 1 - libvips/foreign/vips2tiff.c | 5 +- libvips/include/vips/internal.h | 2 +- libvips/iofuncs/vips.c | 83 ++++++++++++++++-- 7 files changed, 198 insertions(+), 187 deletions(-) diff --git a/TODO b/TODO index 468b151b..e8841b03 100644 --- a/TODO +++ b/TODO @@ -1,47 +1,124 @@ -- added expat to required packages in configure.ac +- remove libxml from configure.ac + + we don't escape " inside xml attr strings, or < > inside xml content - try moving iofuncs/vips.c to expat + break Buffer out into a separate class that does this? VipsDbuf, for dynamic + buffer? or can it be a mode for VipsBuf? - sample XML for us to parse + buffer should expand by 30% on each fill to avoid lots of tiny realloc - $ vipsheader -f getext babe.v - - -
- -
- - jpegload - 0 - 4294966289/169093161 (25.399999998, Rational, 1 components, 8 bytes) - 4294966289/169093161 (25.399999998, Rational, 1 components, 8 bytes) - 2 (Inch, Short, 1 components, 2 bytes) - Exif Version 2.1 (Exif Version 2.1, Undefined, 4 components, 4 bytes) - FlashPix Version 1.0 (FlashPix Version 1.0, Undefined, 4 components, 4 bytes) - 65535 (Internal error (unknown value 65535), Short, 1 components, 2 bytes) - in - RXhpZgAATU0AKgAAAAgABAEaAAUAAAABAAAAPgEbAAUAAAABAAAARgEoAAMAAAABAAIAAIdpAAQA - AAABAAAATgAAAAD///wRChQoKf///BEKFCgpAAOQAAAHAAAABDAyMTCgAAAHAAAABDAxMDCgAQAD - AAAAAf//AAAAAAAA - - - + use for tiffsave_buffer etc. - currently works like this + have things like write-escaped-string as extra append operations - read_xml is called on image load - - loads extension bytes into memory - - calls libxml to parse it to an xmlDoc - - saves xmlDoc to VIPS_META_XML in header + need one that escapes ", a separate one that escapes < > - rebuild_header called next - - gets xmlDoc from header - - gets "header" node - calls rebuild_header_builtin() for every field node - - gets "meta" field - calls rebuild_header_meta() for every field node +- dzsave vips.properties is currently + + + + + width + 1450 + + + height + 2048 + + + bands + 3 + + + xoffset + 0 + + + yoffset + 0 + + + xres + 0.99999999990686772 + + + yres + 0.99999999990686772 + + + vips-loader + jpegload + + + jpeg-multiscan + 0 + + + exif-data + +RXhpZgAASUkqAAgAAAAGABIBAwABAAAAAQAAABoBBQABAAAAVgAAABsBBQABAAAAXgAAACgBAwAB +AAAAAgAAABMCAwABAAAAAQAAAGmHBAABAAAAZgAAAAAAAAAR/P//KSgUChH8//8pKBQKBgAAkAcA +BAAAADAyMTABkQcABAAAAAECAwAAoAcABAAAADAxMDABoAMAAQAAAP//AAACoAQAAQAAAKoFAAAD +oAQAAQAAAAAIAAAAAAAA + + + + resolution-unit + in + + + exif-ifd0-Orientation + 1 (Top-left, Short, 1 components, 2 bytes) + + + exif-ifd0-XResolution + 4294966289/169093161 (25.399999998, Rational, 1 components, 8 bytes) + + + exif-ifd0-YResolution + 4294966289/169093161 (25.399999998, Rational, 1 components, 8 bytes) + + + exif-ifd0-ResolutionUnit + 2 (Inch, Short, 1 components, 2 bytes) + + + exif-ifd0-YCbCrPositioning + 1 (Centred, Short, 1 components, 2 bytes) + + + exif-ifd2-ExifVersion + Exif Version 2.1 (Exif Version 2.1, Undefined, 4 components, 4 bytes) + + + exif-ifd2-ComponentsConfiguration + Y Cb Cr - (Y Cb Cr -, Undefined, 4 components, 4 bytes) + + + exif-ifd2-FlashPixVersion + FlashPix Version 1.0 (FlashPix Version 1.0, Undefined, 4 components, 4 bytes) + + + exif-ifd2-ColorSpace + 65535 (Internal error (unknown value 65535), Short, 1 components, 2 bytes) + + + exif-ifd2-PixelXDimension + 1450 (1450, Long, 1 components, 4 bytes) + + + exif-ifd2-PixelYDimension + 2048 (2048, Long, 1 components, 4 bytes) + + + orientation + 1 + + + + - vips linecache has access there twice! diff --git a/configure.ac b/configure.ac index b7a7cd8c..d81fd6bd 100644 --- a/configure.ac +++ b/configure.ac @@ -348,9 +348,8 @@ AC_CHECK_LIB(m,atan2,[AC_DEFINE(HAVE_ATAN2,1,[have atan2() in libm.])]) # have to have these # need glib 2.6 for GOption -PKG_CHECK_MODULES(REQUIRED, glib-2.0 >= 2.6 gmodule-2.0 gobject-2.0 - libxml-2.0 expat) -PACKAGES_USED="$PACKAGES_USED glib-2.0 gmodule-2.0 gobject-2.0 libxml-2.0 expat" +PKG_CHECK_MODULES(REQUIRED, glib-2.0 >= 2.6 gmodule-2.0 gobject-2.0 expat) +PACKAGES_USED="$PACKAGES_USED glib-2.0 gmodule-2.0 gobject-2.0 expat" # after 2.28 we have a monotonic timer PKG_CHECK_MODULES(MONOTONIC, glib-2.0 >= 2.28, diff --git a/libvips/foreign/dzsave.c b/libvips/foreign/dzsave.c index 46696af2..4beeeb31 100644 --- a/libvips/foreign/dzsave.c +++ b/libvips/foreign/dzsave.c @@ -152,144 +152,9 @@ #include #include -#include - #include #include -/* Track this during property save. - */ -typedef struct _WriteInfo { - const char *domain; - VipsImage *image; - xmlNode *node; -} WriteInfo; - -static int -set_prop( WriteInfo *info, - xmlNode *node, const char *name, const char *fmt, ... ) -{ - va_list ap; - char value[1024]; - - va_start( ap, fmt ); - (void) vips_vsnprintf( value, 1024, fmt, ap ); - va_end( ap ); - - if( !xmlSetProp( node, (xmlChar *) name, (xmlChar *) value ) ) { - vips_error( info->domain, - _( "unable to set property \"%s\" to value \"%s\"." ), - name, value ); - return( -1 ); - } - - return( 0 ); -} - -static xmlNode * -new_child( WriteInfo *info, xmlNode *parent, const char *name ) -{ - xmlNode *child; - - if( !(child = xmlNewChild( parent, NULL, (xmlChar *) name, NULL )) ) { - vips_error( info->domain, - _( "unable to set create node \"%s\"" ), name ); - return( NULL ); - } - - return( child ); -} - -static void * -write_vips_property( VipsImage *image, - const char *field, GValue *value, void *a ) -{ - WriteInfo *info = (WriteInfo *) a; - GType type = G_VALUE_TYPE( value ); - - if( g_value_type_transformable( type, VIPS_TYPE_SAVE_STRING ) ) { - GValue save_value = { 0 }; - xmlNode *property; - xmlNode *child; - - g_value_init( &save_value, VIPS_TYPE_SAVE_STRING ); - if( !g_value_transform( value, &save_value ) ) - return( image ); - - if( !(property = new_child( info, info->node, "property" )) ) - return( image ); - - if( !(child = new_child( info, property, "name" )) ) - return( image ); - xmlNodeSetContent( child, (xmlChar *) field ); - - if( !(child = new_child( info, property, "value" )) || - set_prop( info, child, "type", g_type_name( type ) ) ) - return( image ); - xmlNodeSetContent( child, - (xmlChar *) vips_value_get_save_string( &save_value ) ); - } - - return( NULL ); -} - -/* Pack up all the metadata from an image as XML. This called from vips2tiff - * as well. - * - * Free the result with xmlFree(). - */ -char * -vips__make_xml_metadata( const char *domain, VipsImage *image ) -{ - xmlDoc *doc; - GTimeVal now; - char *date; - WriteInfo info; - char *dump; - int dump_size; - - if( !(doc = xmlNewDoc( (xmlChar *) "1.0" )) ) { - vips_error( domain, "%s", _( "xml save error" ) ); - return( NULL ); - } - if( !(doc->children = xmlNewDocNode( doc, NULL, - (xmlChar *) "image", NULL )) ) { - vips_error( domain, "%s", _( "xml save error" ) ); - xmlFreeDoc( doc ); - return( NULL ); - } - - info.domain = domain; - info.image = image; - g_get_current_time( &now ); - date = g_time_val_to_iso8601( &now ); - if( set_prop( &info, doc->children, "xmlns", - "http://www.vips.ecs.soton.ac.uk/dzsave" ) || - set_prop( &info, doc->children, "date", date ) || - set_prop( &info, doc->children, "version", VIPS_VERSION ) ) { - g_free( date ); - xmlFreeDoc( doc ); - return( NULL ); - } - g_free( date ); - - if( !(info.node = new_child( &info, doc->children, "properties" )) || - vips_image_map( image, write_vips_property, &info ) ) { - xmlFreeDoc( doc ); - return( NULL ); - } - - xmlDocDumpFormatMemory( doc, (xmlChar **) &dump, &dump_size, 1 ); - if( !dump ) { - vips_error( domain, "%s", _( "xml save error" ) ); - xmlFreeDoc( doc ); - return( NULL ); - } - xmlFreeDoc( doc ); - - return( dump ); -} - #ifdef HAVE_GSF #include @@ -980,12 +845,11 @@ static int write_vips_meta( VipsForeignSaveDz *dz ) { VipsForeignSave *save = (VipsForeignSave *) dz; - VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( dz ); char *dump; GsfOutput *out; - if( !(dump = vips__make_xml_metadata( class->nickname, save->ready )) ) + if( !(dump = vips__xml_properties( save->ready )) ) return( -1 ); /* For deepzom the props must go inside the ${name}_files subdir, for @@ -1001,7 +865,7 @@ write_vips_meta( VipsForeignSaveDz *dz ) (void) gsf_output_close( out ); g_object_unref( out ); - xmlFree( dump ); + g_free( dump ); return( 0 ); } diff --git a/libvips/foreign/tiff.c b/libvips/foreign/tiff.c index dd9bdcd2..b083adc2 100644 --- a/libvips/foreign/tiff.c +++ b/libvips/foreign/tiff.c @@ -48,7 +48,6 @@ #include #endif /*HAVE_UNISTD_H*/ #include -#include #include #include diff --git a/libvips/foreign/vips2tiff.c b/libvips/foreign/vips2tiff.c index 8f344380..621e092a 100644 --- a/libvips/foreign/vips2tiff.c +++ b/libvips/foreign/vips2tiff.c @@ -218,7 +218,6 @@ #include #endif /*HAVE_UNISTD_H*/ #include -#include #include #include @@ -512,10 +511,10 @@ wtiff_embed_imagedescription( Wtiff *wtiff, TIFF *tif ) if( wtiff->properties ) { char *doc; - if( !(doc = vips__make_xml_metadata( "vips2tiff", wtiff->im )) ) + if( !(doc = vips__xml_properties( wtiff->im )) ) return( -1 ); TIFFSetField( tif, TIFFTAG_IMAGEDESCRIPTION, doc ); - xmlFree( doc ); + g_free( doc ); } else { const char *imagedescription; diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index 8329cf87..858bd11a 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -227,7 +227,7 @@ int vips_check_bands_3ormore( const char *domain, VipsImage *im ); int vips__byteswap_bool( VipsImage *in, VipsImage **out, gboolean swap ); -char *vips__make_xml_metadata( const char *domain, VipsImage *image ); +char *vips__xml_properties( VipsImage *image ); void vips__cairo2rgba( guint32 *buf, int n ); diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 5244aa04..970be1ef 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -141,7 +141,7 @@ /* Our XML namespace. */ -#define NAMESPACE "http://www.vips.ecs.soton.ac.uk/vips" +#define NAMESPACE_URI "http://www.vips.ecs.soton.ac.uk/" /* Open for read for image files. */ @@ -595,7 +595,7 @@ parser_element_start_handler( void *user_data, else if( strcmp( name, "root" ) == 0 ) { for( p = atts; *p; p += 2 ) if( strcmp( p[0], "xmlns" ) == 0 && - !vips_isprefix( NAMESPACE, p[1] ) ) { + !vips_isprefix( NAMESPACE_URI "/vips", p[1] ) ) { vips_error( "VipsImage", "%s", _( "incorrect namespace in XML" ) ); vep->error = TRUE; @@ -801,6 +801,8 @@ build_xml_meta( VipsMeta *meta, Buffer *buffer ) return( NULL ); } +/* Make the xml we append to vips images after the pixel data. + */ static char * build_xml( VipsImage *image ) { @@ -810,8 +812,8 @@ build_xml( VipsImage *image ) buffer_init( &buffer ); buffer_appendf( &buffer, "\n" ); - buffer_appendf( &buffer, "\n", - NAMESPACE, + buffer_appendf( &buffer, "\n", + NAMESPACE_URI, VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION ); buffer_appendf( &buffer, "
\n" ); @@ -836,6 +838,77 @@ build_xml( VipsImage *image ) return( buffer.data ); } +static void * +vips__xml_properties_meta( VipsImage *image, + const char *field, GValue *value, void *a ) +{ + Buffer *buffer = (Buffer *) a; + GType type = G_VALUE_TYPE( value ); + + const char *str; + + /* If we can transform to VIPS_TYPE_SAVE_STRING and back, we can save + * and restore. + */ + if( g_value_type_transformable( type, VIPS_TYPE_SAVE_STRING ) && + g_value_type_transformable( VIPS_TYPE_SAVE_STRING, type ) ) { + GValue save_value = { 0 }; + + g_value_init( &save_value, VIPS_TYPE_SAVE_STRING ); + if( !g_value_transform( value, &save_value ) ) { + vips_error( "VipsImage", "%s", + _( "error transforming to save format" ) ); + return( buffer ); + } + str = vips_value_get_save_string( &save_value ); + g_value_unset( &save_value ); + + buffer_appendf( buffer, " \n" ); + buffer_appendf( buffer, " %s\n", field ); + buffer_appendf( buffer, " ", + g_type_name( type ) ); + buffer_append( buffer, str, strlen( str ) ); + buffer_appendf( buffer, "\n" ); + buffer_appendf( buffer, " \n" ); + } + + return( NULL ); +} + +/* Make the xml we write to vips-properties in dzsave, or to TIFF. A simple + * dump of all vips metadata. Free with g_free(). + */ +char * +vips__xml_properties( VipsImage *image ) +{ + Buffer buffer; + GTimeVal now; + char *date; + + buffer_init( &buffer ); + + g_get_current_time( &now ); + date = g_time_val_to_iso8601( &now ); + buffer_appendf( &buffer, "\n" ); + buffer_appendf( &buffer, "\n", + NAMESPACE_URI, + date, + VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION ); + g_free( date ); + buffer_appendf( &buffer, " \n" ); + + if( vips_image_map( image, vips__xml_properties_meta, &buffer ) ) { + buffer_destroy( &buffer ); + return( NULL ); + } + + buffer_appendf( &buffer, " \n" ); + buffer_appendf( &buffer, "\n" ); + + return( buffer.data ); +} + /* Append XML to output fd. */ int @@ -846,7 +919,7 @@ vips__writehist( VipsImage *image ) assert( image->dtype == VIPS_IMAGE_OPENOUT ); assert( image->fd != -1 ); - if( !(xml = build_xml( im )) ) + if( !(xml = build_xml( image )) ) return( -1 ); if( vips__write_extension_block( image, xml, strlen( xml ) ) ) { From e1b9c789cb57586ad0f82fa7160d63a57c3861c8 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sun, 26 Feb 2017 17:37:46 +0000 Subject: [PATCH 07/11] add dbuf object dynamically expanding buffer also, escape "<>& appropriately when we write xml --- ChangeLog | 1 - TODO | 134 +------------------- libvips/include/vips/Makefile.am | 1 + libvips/include/vips/dbuf.h | 72 +++++++++++ libvips/include/vips/vips.h | 1 + libvips/iofuncs/Makefile.am | 1 + libvips/iofuncs/dbuf.c | 206 ++++++++++++++++++++++++++++++ libvips/iofuncs/vips.c | 210 +++++++++++++++---------------- 8 files changed, 388 insertions(+), 238 deletions(-) create mode 100644 libvips/include/vips/dbuf.h create mode 100644 libvips/iofuncs/dbuf.c diff --git a/ChangeLog b/ChangeLog index a045cb33..01e118fe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -36,7 +36,6 @@ more tightly ... this makes seq more versatile and more reliable - use expat, not libxml, for XML load ... removes a required dependency, since we get expat as part of glib -- use printf for xml write 8/12/16 started 8.4.5 - allow libgsf-1.14.26 to help centos, thanks tdiprima diff --git a/TODO b/TODO index e8841b03..156444ec 100644 --- a/TODO +++ b/TODO @@ -1,123 +1,7 @@ -- remove libxml from configure.ac - - we don't escape " inside xml attr strings, or < > inside xml content +- use VipsDbuf for tiffsave_buffer etc. - break Buffer out into a separate class that does this? VipsDbuf, for dynamic - buffer? or can it be a mode for VipsBuf? +- verify xml data against master for vips save and dzsave - buffer should expand by 30% on each fill to avoid lots of tiny realloc - - use for tiffsave_buffer etc. - - have things like write-escaped-string as extra append operations - - need one that escapes ", a separate one that escapes < > - -- dzsave vips.properties is currently - - - - - - - width - 1450 - - - height - 2048 - - - bands - 3 - - - xoffset - 0 - - - yoffset - 0 - - - xres - 0.99999999990686772 - - - yres - 0.99999999990686772 - - - vips-loader - jpegload - - - jpeg-multiscan - 0 - - - exif-data - -RXhpZgAASUkqAAgAAAAGABIBAwABAAAAAQAAABoBBQABAAAAVgAAABsBBQABAAAAXgAAACgBAwAB -AAAAAgAAABMCAwABAAAAAQAAAGmHBAABAAAAZgAAAAAAAAAR/P//KSgUChH8//8pKBQKBgAAkAcA -BAAAADAyMTABkQcABAAAAAECAwAAoAcABAAAADAxMDABoAMAAQAAAP//AAACoAQAAQAAAKoFAAAD -oAQAAQAAAAAIAAAAAAAA - - - - resolution-unit - in - - - exif-ifd0-Orientation - 1 (Top-left, Short, 1 components, 2 bytes) - - - exif-ifd0-XResolution - 4294966289/169093161 (25.399999998, Rational, 1 components, 8 bytes) - - - exif-ifd0-YResolution - 4294966289/169093161 (25.399999998, Rational, 1 components, 8 bytes) - - - exif-ifd0-ResolutionUnit - 2 (Inch, Short, 1 components, 2 bytes) - - - exif-ifd0-YCbCrPositioning - 1 (Centred, Short, 1 components, 2 bytes) - - - exif-ifd2-ExifVersion - Exif Version 2.1 (Exif Version 2.1, Undefined, 4 components, 4 bytes) - - - exif-ifd2-ComponentsConfiguration - Y Cb Cr - (Y Cb Cr -, Undefined, 4 components, 4 bytes) - - - exif-ifd2-FlashPixVersion - FlashPix Version 1.0 (FlashPix Version 1.0, Undefined, 4 components, 4 bytes) - - - exif-ifd2-ColorSpace - 65535 (Internal error (unknown value 65535), Short, 1 components, 2 bytes) - - - exif-ifd2-PixelXDimension - 1450 (1450, Long, 1 components, 4 bytes) - - - exif-ifd2-PixelYDimension - 2048 (2048, Long, 1 components, 4 bytes) - - - orientation - 1 - - - @@ -246,20 +130,6 @@ DEBUG:gi.overrides.Vips:assigning sequential output imagevec output blob -- fix up aconv - -- rewrite im_conv() etc. as vips_conv(), also the mosaicing functions - - finally finish --disable-deprecated option - - - -- can we use postbuild elsewhere? look at use of "preclose" / "written", etc. - - - -- test draw_mask on labq images - we probably need to unpack the ink back to double before blending diff --git a/libvips/include/vips/Makefile.am b/libvips/include/vips/Makefile.am index 69f51b28..a76a306e 100644 --- a/libvips/include/vips/Makefile.am +++ b/libvips/include/vips/Makefile.am @@ -6,6 +6,7 @@ pkginclude_HEADERS = \ deprecated.h \ arithmetic.h \ buf.h \ + dbuf.h \ colour.h \ conversion.h \ convolution.h \ diff --git a/libvips/include/vips/dbuf.h b/libvips/include/vips/dbuf.h new file mode 100644 index 00000000..c48908f7 --- /dev/null +++ b/libvips/include/vips/dbuf.h @@ -0,0 +1,72 @@ +/* A dynamic memory buffer that expands as you write. + */ + +/* + + 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 + + */ + +#ifndef VIPS_DBUF_H +#define VIPS_DBUF_H + +#ifdef __cplusplus +extern "C" { +#endif /*__cplusplus*/ + +#include + +/* A buffer in the process of being written to ... multiple calls to + * vips_dbuf_append add to it. + */ + +typedef struct _VipsDbuf { + /* All fields are private. + */ + /*< private >*/ + + /* The current base, and the size of the allocated memory area. + */ + char *data; + size_t max_size; + + /* And the write point. + */ + size_t write_point; +} VipsDbuf; + +void vips_dbuf_destroy( VipsDbuf *buf ); +void vips_dbuf_init( VipsDbuf *buf ); +gboolean vips_dbuf_append( VipsDbuf *dbuf, const char *data, size_t size ); +gboolean vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... ); +void vips_dbuf_rewind( VipsDbuf *dbuf ); +void vips_dbuf_destroy( VipsDbuf *dbuf ); +char *vips_dbuf_string( VipsDbuf *dbuf, size_t *size ); +char *vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ); + +#endif /*VIPS_DBUF_H*/ + +#ifdef __cplusplus +} +#endif /*__cplusplus*/ diff --git a/libvips/include/vips/vips.h b/libvips/include/vips/vips.h index 30be4706..75c6c3b4 100644 --- a/libvips/include/vips/vips.h +++ b/libvips/include/vips/vips.h @@ -106,6 +106,7 @@ extern "C" { #include #include +#include #include #include #include diff --git a/libvips/iofuncs/Makefile.am b/libvips/iofuncs/Makefile.am index cdf244a8..51b47f75 100644 --- a/libvips/iofuncs/Makefile.am +++ b/libvips/iofuncs/Makefile.am @@ -1,6 +1,7 @@ noinst_LTLIBRARIES = libiofuncs.la libiofuncs_la_SOURCES = \ + dbuf.c \ reorder.c \ vipsmarshal.h \ vipsmarshal.c \ diff --git a/libvips/iofuncs/dbuf.c b/libvips/iofuncs/dbuf.c new file mode 100644 index 00000000..36d22a59 --- /dev/null +++ b/libvips/iofuncs/dbuf.c @@ -0,0 +1,206 @@ +/* A dynamic memory buffer that expands as you write. + */ + +/* + + Copyright (C) 1991-2003 The National Gallery + + This library 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.1 of the License, or (at your option) any later version. + + This library 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 library; 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 DEBUG + */ + +#ifdef HAVE_CONFIG_H +#include +#endif /*HAVE_CONFIG_H*/ +#include + +#include + +#include + +/** + * vips_dbuf_init: + * @dbuf: the buffer + * + * Initialize a buffer. + */ +void +vips_dbuf_init( VipsDbuf *dbuf ) +{ + dbuf->data = NULL; + dbuf->max_size = 0; + dbuf->write_point = 0; +} + +/** + * vips_dbuf_append: + * @dbuf: the buffer + * @data: the data to append to the buffer + * @size: the size of the len to append + * + * Append len bytes from @data to the buffer. The buffer expands if necessary. + * + * Returns: %FALSE on out of memory, %TRUE otherwise. + */ +gboolean +vips_dbuf_append( VipsDbuf *dbuf, const char *data, size_t size ) +{ + size_t new_write_point = dbuf->write_point + size; + + if( new_write_point > dbuf->max_size ) { + size_t new_max_size = 3 * (16 + new_write_point) / 2; + + char *new_data; + + if( !(new_data = g_try_realloc( dbuf->data, new_max_size )) ) { + vips_error( "VipsDbuf", "%s", _( "out of memory" ) ); + return( FALSE ); + } + + dbuf->data = new_data; + dbuf->max_size = new_max_size; + } + + memcpy( dbuf->data + dbuf->write_point, data, size ); + dbuf->write_point = new_write_point; + + return( TRUE ); +} + +/** + * vips_dbuf_appendf: + * @dbuf: the buffer + * @fmt: printf()-style format string + * @...: arguments to format string + * + * Format the string and append to @dbuf. + * + * Returns: %FALSE on out of memory, %TRUE otherwise. + */ +gboolean +vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... ) +{ + va_list ap; + char *line; + + va_start( ap, fmt ); + line = g_strdup_vprintf( fmt, ap ); + va_end( ap ); + + if( vips_dbuf_append( dbuf, line, strlen( line ) ) ) { + g_free( line ); + return( FALSE ); + } + g_free( line ); + + return( TRUE ); +} + +/** + * vips_dbuf_rewind: + * @dbuf: the buffer + * + * Reset the buffer to empty. No memory is freed, just the write pointer is + * reset. + */ +void +vips_dbuf_rewind( VipsDbuf *dbuf ) +{ + dbuf->write_point = 0; +} + +/** + * vips_dbuf_destroy: + * @dbuf: the buffer + * + * Destroy a buffer. This frees any allocated memory. + */ +void +vips_dbuf_destroy( VipsDbuf *dbuf ) +{ + VIPS_FREE( dbuf->data ); + dbuf->max_size = 0; + dbuf->write_point = 0; +} + +/** + * vips_dbuf_steal: + * @dbuf: the buffer + * @size: (allow-none): optionally return length in bytes here + * + * Destroy a buffer, but rather than freeing memory, a pointer is returned. + * This must be freed with g_free(). + * + * A `\0` is appended, but not included in the character count. This is so the + * pointer can be safely treated as a C string. + * + * Returns: (transfer full): The pointer held by @dbuf. + */ +char * +vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ) +{ + char *data; + + vips_dbuf_append( dbuf, "", 1 ); + dbuf->write_point -= 1; + + data = dbuf->data; + + if( size ) + *size = dbuf->write_point; + + dbuf->data = NULL; + dbuf->max_size = 0; + dbuf->write_point = 0; + + return( data ); +} + +/** + * vips_dbuf_data: + * @dbuf: the buffer + * @size: (allow-none): optionally return length in bytes here + * + * Return a pointer to @dbuf's internal data. + * + * A `\0` is appended, but not included in the character count. This is so the + * pointer can be safely treated as a C string. + * + * Returns: (transfer none): The pointer held by @dbuf. + */ +char * +vips_dbuf_string( VipsDbuf *dbuf, size_t *size ) +{ + vips_dbuf_append( dbuf, "", 1 ); + dbuf->write_point -= 1; + + if( size ) + *size = dbuf->write_point; + + return( dbuf->data ); +} + + diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 970be1ef..1b564e84 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -49,8 +49,8 @@ */ /* -#define DEBUG #define SHOW_HEADER +#define DEBUG */ #ifdef HAVE_CONFIG_H @@ -492,56 +492,6 @@ parser_read_fd( XML_Parser parser, int fd ) #define MAX_PARSE_ATTR (256) -/* A memory buffer that expands as we write to it. - */ -typedef struct _Buffer { - char *data; - size_t current_size; -} Buffer; - -static void -buffer_init( Buffer *buffer ) -{ - buffer->data = NULL; - buffer->current_size = 0; -} - -static void -buffer_append( Buffer *buffer, const char *data, int len ) -{ - size_t new_size = buffer->current_size + len; - - buffer->data = g_realloc( buffer->data, new_size ); - memcpy( buffer->data + buffer->current_size, data, len ); - buffer->current_size = new_size; -} - -static void -buffer_appendf( Buffer *buffer, const char *fmt, ... ) -{ - va_list ap; - char line[256]; - - va_start( ap, fmt ); - (void) vips_vsnprintf( line, 256, fmt, ap ); - va_end( ap ); - - buffer_append( buffer, line, strlen( line ) ); -} - -static void -buffer_rewind( Buffer *buffer ) -{ - buffer->current_size = 0; -} - -static void -buffer_destroy( Buffer *buffer ) -{ - VIPS_FREE( buffer->data ); - buffer->current_size = 0; -} - /* What we track during expat parse. */ typedef struct _VipsExpatParse { @@ -562,7 +512,7 @@ typedef struct _VipsExpatParse { /* Accumulate data here. */ - Buffer buffer; + VipsDbuf dbuf; } VipsExpatParse; static void @@ -586,7 +536,7 @@ parser_element_start_handler( void *user_data, vips_strncpy( vep->type, p[1], MAX_PARSE_ATTR ); } - buffer_rewind( &vep->buffer ); + vips_dbuf_rewind( &vep->dbuf ); } else if( strcmp( name, "header" ) == 0 ) vep->header = TRUE; @@ -666,16 +616,10 @@ parser_element_end_handler( void *user_data, const XML_Char *name ) #endif /*DEBUG*/ if( strcmp( name, "field" ) == 0 ) { - buffer_append( &vep->buffer, "", 1 ); - -#ifdef DEBUG - printf( "parser_element_end_handler: %zd bytes\n", - vep->current_size ); -#endif /*DEBUG*/ - if( vep->header ) { if( strcmp( name, "Hist" ) == 0 ) - set_history( vep->image, vep->buffer.data ); + set_history( vep->image, + vips_dbuf_string( &vep->dbuf, NULL ) ); } else { GType gtype = g_type_from_name( vep->type ); @@ -686,7 +630,8 @@ parser_element_end_handler( void *user_data, const XML_Char *name ) g_value_type_transformable( VIPS_TYPE_SAVE_STRING, gtype ) && set_meta( vep->image, - gtype, vep->name, vep->buffer.data ) ) + gtype, vep->name, + vips_dbuf_string( &vep->dbuf, NULL ) ) ) vep->error = TRUE; } } @@ -701,7 +646,7 @@ parser_data_handler( void *user_data, const XML_Char *data, int len ) printf( "parser_data_handler: %d bytes\n", len ); #endif /*DEBUG*/ - buffer_append( &vep->buffer, data, len ); + vips_dbuf_append( &vep->dbuf, data, len ); } /* Called at the end of vips open ... get any XML after the pixel data @@ -719,7 +664,7 @@ readhist( VipsImage *im ) parser = XML_ParserCreate( "UTF-8" ); vep.image = im; - buffer_init( &vep.buffer ); + vips_dbuf_init( &vep.dbuf ); vep.error = FALSE; XML_SetUserData( parser, &vep ); @@ -729,12 +674,12 @@ readhist( VipsImage *im ) if( parser_read_fd( parser, im->fd ) || vep.error ) { - buffer_destroy( &vep.buffer ); + vips_dbuf_destroy( &vep.dbuf ); XML_ParserFree( parser ); return( -1 ); } - buffer_destroy( &vep.buffer ); + vips_dbuf_destroy( &vep.dbuf ); XML_ParserFree( parser ); return( 0 ); @@ -768,8 +713,59 @@ vips__write_extension_block( VipsImage *im, void *buf, int size ) return( 0 ); } +/* Append a string to a buffer, but escape " as \". + */ +static void +dbuf_append_quotes( VipsDbuf *dbuf, const char *str ) +{ + const char *p; + size_t len; + + for( p = str; *p; p += len ) { + len = strcspn( p, "\"" ); + + vips_dbuf_append( dbuf, p, len ); + if( p[len] == '"' ) + vips_dbuf_append( dbuf, "\\", 1 ); + } +} + +/* Append a string to a buffer, but escape &<>. + */ +static void +dbuf_append_amp( VipsDbuf *dbuf, const char *str ) +{ + const char *p; + size_t len; + + for( p = str; *p; p += len ) { + len = strcspn( p, "&<>" ); + + vips_dbuf_append( dbuf, p, len ); + switch( p[len] ) { + case '&': + vips_dbuf_append( dbuf, "&", 5 ); + len += 1; + break; + + case '<': + vips_dbuf_append( dbuf, "<", 4 ); + len += 1; + break; + + case '>': + vips_dbuf_append( dbuf, ">", 4 ); + len += 1; + break; + + default: + break; + } + } +} + static void * -build_xml_meta( VipsMeta *meta, Buffer *buffer ) +build_xml_meta( VipsMeta *meta, VipsDbuf *dbuf ) { GType type = G_VALUE_TYPE( &meta->value ); @@ -786,14 +782,16 @@ build_xml_meta( VipsMeta *meta, Buffer *buffer ) if( !g_value_transform( &meta->value, &save_value ) ) { vips_error( "VipsImage", "%s", _( "error transforming to save format" ) ); - return( buffer ); + return( meta ); } str = vips_value_get_save_string( &save_value ); - buffer_appendf( buffer, " ", - g_type_name( type ), meta->name ); - buffer_append( buffer, str, strlen( str ) ); - buffer_appendf( buffer, "\n" ); + vips_dbuf_appendf( dbuf, " name ); + vips_dbuf_appendf( dbuf, "\">" ); + dbuf_append_amp( dbuf, str ); + vips_dbuf_appendf( dbuf, "\n" ); g_value_unset( &save_value ); } @@ -806,43 +804,43 @@ build_xml_meta( VipsMeta *meta, Buffer *buffer ) static char * build_xml( VipsImage *image ) { - Buffer buffer; + VipsDbuf dbuf; const char *str; - buffer_init( &buffer ); + vips_dbuf_init( &dbuf ); - buffer_appendf( &buffer, "\n" ); - buffer_appendf( &buffer, "\n", + vips_dbuf_appendf( &dbuf, "\n" ); + vips_dbuf_appendf( &dbuf, "\n", NAMESPACE_URI, VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION ); - buffer_appendf( &buffer, "
\n" ); + vips_dbuf_appendf( &dbuf, "
\n" ); str = vips_image_get_history( image ); - buffer_appendf( &buffer, " ", + vips_dbuf_appendf( &dbuf, " ", g_type_name( VIPS_TYPE_REF_STRING ) ); - buffer_append( &buffer, str, strlen( str ) ); - buffer_appendf( &buffer, "\n" ); + dbuf_append_amp( &dbuf, str ); + vips_dbuf_appendf( &dbuf, "\n" ); - buffer_appendf( &buffer, "
\n" ); - buffer_appendf( &buffer, " \n" ); + vips_dbuf_appendf( &dbuf, "
\n" ); + vips_dbuf_appendf( &dbuf, " \n" ); if( vips_slist_map2( image->meta_traverse, - (VipsSListMap2Fn) build_xml_meta, &buffer, NULL ) ) { - buffer_destroy( &buffer ); + (VipsSListMap2Fn) build_xml_meta, &dbuf, NULL ) ) { + vips_dbuf_destroy( &dbuf ); return( NULL ); } - buffer_appendf( &buffer, " \n" ); - buffer_appendf( &buffer, "\n" ); + vips_dbuf_appendf( &dbuf, " \n" ); + vips_dbuf_appendf( &dbuf, "\n" ); - return( buffer.data ); + return( vips_dbuf_steal( &dbuf, NULL ) ); } static void * vips__xml_properties_meta( VipsImage *image, const char *field, GValue *value, void *a ) { - Buffer *buffer = (Buffer *) a; + VipsDbuf *dbuf = (VipsDbuf *) a; GType type = G_VALUE_TYPE( value ); const char *str; @@ -858,18 +856,20 @@ vips__xml_properties_meta( VipsImage *image, if( !g_value_transform( value, &save_value ) ) { vips_error( "VipsImage", "%s", _( "error transforming to save format" ) ); - return( buffer ); + return( dbuf ); } str = vips_value_get_save_string( &save_value ); g_value_unset( &save_value ); - buffer_appendf( buffer, " \n" ); - buffer_appendf( buffer, " %s\n", field ); - buffer_appendf( buffer, " ", + vips_dbuf_appendf( dbuf, " \n" ); + vips_dbuf_appendf( dbuf, " " ); + dbuf_append_amp( dbuf, field ); + vips_dbuf_appendf( dbuf, "\n" ); + vips_dbuf_appendf( dbuf, " ", g_type_name( type ) ); - buffer_append( buffer, str, strlen( str ) ); - buffer_appendf( buffer, "\n" ); - buffer_appendf( buffer, " \n" ); + dbuf_append_amp( dbuf, str ); + vips_dbuf_appendf( dbuf, "\n" ); + vips_dbuf_appendf( dbuf, " \n" ); } return( NULL ); @@ -881,32 +881,32 @@ vips__xml_properties_meta( VipsImage *image, char * vips__xml_properties( VipsImage *image ) { - Buffer buffer; + VipsDbuf dbuf; GTimeVal now; char *date; - buffer_init( &buffer ); + vips_dbuf_init( &dbuf ); g_get_current_time( &now ); date = g_time_val_to_iso8601( &now ); - buffer_appendf( &buffer, "\n" ); - buffer_appendf( &buffer, "\n" ); + vips_dbuf_appendf( &dbuf, "\n", NAMESPACE_URI, date, VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION ); g_free( date ); - buffer_appendf( &buffer, " \n" ); + vips_dbuf_appendf( &dbuf, " \n" ); - if( vips_image_map( image, vips__xml_properties_meta, &buffer ) ) { - buffer_destroy( &buffer ); + if( vips_image_map( image, vips__xml_properties_meta, &dbuf ) ) { + vips_dbuf_destroy( &dbuf ); return( NULL ); } - buffer_appendf( &buffer, " \n" ); - buffer_appendf( &buffer, "\n" ); + vips_dbuf_appendf( &dbuf, " \n" ); + vips_dbuf_appendf( &dbuf, "\n" ); - return( buffer.data ); + return( vips_dbuf_steal( &dbuf, NULL ) ); } /* Append XML to output fd. From 40294bb85c48b699880d7744bb5548839dcede34 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Mon, 27 Feb 2017 22:06:22 +0000 Subject: [PATCH 08/11] started png and jpg with dbuf rad and webp still to do, maybe tiff --- README.md | 6 +- TODO | 20 ++++ libvips/foreign/vips2jpeg.c | 200 ++++++------------------------------ libvips/foreign/vipspng.c | 49 ++------- libvips/include/vips/dbuf.h | 11 +- libvips/iofuncs/dbuf.c | 69 ++++++++++--- libvips/iofuncs/vips.c | 24 +++-- 7 files changed, 137 insertions(+), 242 deletions(-) diff --git a/README.md b/README.md index 4102e4b1..bfc373ea 100644 --- a/README.md +++ b/README.md @@ -46,8 +46,7 @@ Untar, then in the libvips directory you should just be able to do: $ ./configure Check the summary at the end of `configure` carefully. -libvips must have `build-essential`, `pkg-config`, `glib2.0-dev`, and -`libxml2-dev`. +libvips must have `build-essential`, `pkg-config`, `glib2.0-dev`. For the vips8 Python binding, you must have `gobject-introspection`, `python-gi-dev`, and `libgirepository1.0-dev`. @@ -145,8 +144,7 @@ Static analysis with: # Dependencies -libvips has to have `gettext`, `glib2.0-dev` and `libxml2-dev`. Other -dependencies are optional, see below. +libvips has to have `glib2.0-dev`. Other dependencies are optional, see below. # Optional dependencies diff --git a/TODO b/TODO index 156444ec..ed0c97b8 100644 --- a/TODO +++ b/TODO @@ -1,5 +1,25 @@ - use VipsDbuf for tiffsave_buffer etc. + $ grep -l save_buffer *.c + dzsave.c + jpegsave.c + pngsave.c + radsave.c + tiffsave.c + webpsave.c + + done png, jpg + + tiff needs seek ... perhaps add this to dbuf? + + dzsave uses gsf + + rad and webp left to do + + vips2jpeg term_destination() needs some kind of truncate call for dbuf + + + - verify xml data against master for vips save and dzsave diff --git a/libvips/foreign/vips2jpeg.c b/libvips/foreign/vips2jpeg.c index 404872b6..0f8f0365 100644 --- a/libvips/foreign/vips2jpeg.c +++ b/libvips/foreign/vips2jpeg.c @@ -82,6 +82,8 @@ * - turn off chroma subsample for Q > 90 * 7/11/16 * - move exif handling out to exif.c + * 27/2/17 + * - use dbuf for memory output */ /* @@ -670,112 +672,6 @@ vips__jpeg_write_file( VipsImage *in, return( 0 ); } -/* We can't predict how large the output buffer we need is, because we might - * need space for ICC profiles and stuff. So we write to a linked list of mem - * buffers and add a new one as they fill. - */ - -#define BUFFER_SIZE (10000) - -/* A buffer. - */ -typedef struct _Block { - j_compress_ptr cinfo; - - struct _Block *first; - struct _Block *next; - - JOCTET *data; /* Allocated area */ - size_t size; /* Max size */ - size_t used; /* How much has been used */ -} Block; - -static Block * -block_new( j_compress_ptr cinfo ) -{ - Block *block; - - block = (Block *) (*cinfo->mem->alloc_large) - ( (j_common_ptr) cinfo, JPOOL_IMAGE, sizeof( Block ) ); - - block->cinfo = cinfo; - block->first = block; - block->next = NULL; - block->data = (JOCTET *) (*cinfo->mem->alloc_large) - ( (j_common_ptr) cinfo, JPOOL_IMAGE, BUFFER_SIZE ); - block->size = BUFFER_SIZE; - block->used = 0; - - return( block ); -} - -static Block * -block_last( Block *block ) -{ - while( block->next ) - block = block->next; - - return( block ); -} - -static Block * -block_append( Block *block ) -{ - Block *new; - - g_assert( block ); - - new = block_new( block->cinfo ); - new->first = block->first; - block_last( block )->next = new; - - return( new ); -} - -static size_t -block_length( Block *block ) -{ - size_t len; - - len = 0; - for( block = block->first; block; block = block->next ) - len += block->used; - - return( len ); -} - -static void -block_copy( Block *block, void *dest ) -{ - JOCTET *p; - - p = dest; - for( block = block->first; block; block = block->next ) { - memcpy( p, block->data, block->used ); - p += block->used; - } -} - -#ifdef DEBUG -static void -block_print( Block *block ) -{ - int i; - - printf( "total length = %zd\n", block_length( block ) ); - printf( "set of blocks:\n" ); - - i = 0; - for( block = block->first; block; block = block->next ) { - printf( "%d) %p, first = %p, next = %p" - "\t data = %p, size = %zd, used = %zd\n", - i, block, block->first, block->next, - block->data, block->size, block->used ); - i += 1; - } -} -#endif /*DEBUG*/ - /* Just like the above, but we write to a memory buffer. * * A memory buffer for the compressed image. @@ -788,16 +684,36 @@ typedef struct { /* Private stuff during write. */ - /* Build the output area here in chunks. + /* Build the output area here. */ - Block *block; + VipsDbuf dbuf; - /* Copy the compressed area here. + /* Write the generated area here. */ void **obuf; /* Allocated buffer, and size */ size_t *olen; } OutputBuffer; +/* Buffer full method ... allocate a new output block. This is only called + * when the output area is exactly full. + */ +METHODDEF(boolean) +empty_output_buffer( j_compress_ptr cinfo ) +{ + OutputBuffer *buf = (OutputBuffer *) cinfo->dest; + + size_t size; + + vips_dbuf_allocate( &buf->dbuf, 10000 ); + buf->pub.next_output_byte = + (JOCTET *) vips_dbuf_get_write( &buf->dbuf, &size ); + buf->pub.free_in_buffer = size; + + /* TRUE means we've made some more space. + */ + return( 1 ); +} + /* Init dest method. */ METHODDEF(void) @@ -805,38 +721,8 @@ init_destination( j_compress_ptr cinfo ) { OutputBuffer *buf = (OutputBuffer *) cinfo->dest; - /* Allocate relative to the image we are writing .. freed when we junk - * this output. - */ - buf->block = block_new( cinfo ); - - /* Set buf pointers for library. - */ - buf->pub.next_output_byte = buf->block->data; - buf->pub.free_in_buffer = buf->block->size; -} - -/* Buffer full method ... allocate a new output block. - */ -METHODDEF(boolean) -empty_output_buffer( j_compress_ptr cinfo ) -{ - OutputBuffer *buf = (OutputBuffer *) cinfo->dest; - - /* Record how many bytes we used. empty_output_buffer() is always - * called when the buffer is exactly full. - */ - buf->block->used = buf->block->size; - - /* New block and reset. - */ - buf->block = block_append( buf->block ); - buf->pub.next_output_byte = buf->block->data; - buf->pub.free_in_buffer = buf->block->size; - - /* TRUE means we've made some more space. - */ - return( 1 ); + vips_dbuf_init( &buf->dbuf ); + empty_output_buffer( cinfo ); } /* Cleanup. Copy the set of blocks out as a big lump. @@ -846,35 +732,15 @@ term_destination( j_compress_ptr cinfo ) { OutputBuffer *buf = (OutputBuffer *) cinfo->dest; - size_t len; - void *obuf; + size_t size; - /* Record the number of bytes we wrote in the final buffer. - * pub.free_in_buffer is valid here. + /* This can be called before the output area fills. We need to chop + * size down to the number of bytes actually written. */ - buf->block->used = buf->block->size - buf->pub.free_in_buffer; + buf->dbuf.write_point = buf->dbuf.max_size - buf->pub.free_in_buffer; -#ifdef DEBUG - block_print( buf->block ); -#endif /*DEBUG*/ - - /* ... and we can count up our buffers now. - */ - len = block_length( buf->block ); - - /* Allocate and copy to the output area. - */ - if( !(obuf = vips_malloc( NULL, len )) ) - ERREXIT( cinfo, JERR_FILE_WRITE ); - else { - /* coverity doesn't know ERREXIT() does not return, so put - * this in an else. - */ - *(buf->obuf) = obuf; - *(buf->olen) = len; - - block_copy( buf->block, obuf ); - } + *(buf->obuf) = vips_dbuf_steal( &buf->dbuf, &size ); + *(buf->olen) = size; } /* Set dest to one of our objects. diff --git a/libvips/foreign/vipspng.c b/libvips/foreign/vipspng.c index 336d3a05..4de6fe3c 100644 --- a/libvips/foreign/vipspng.c +++ b/libvips/foreign/vipspng.c @@ -59,6 +59,8 @@ * - support --strip option * 17/1/17 * - invalidate operation on read error + * 27/2/17 + * - use dbuf for buffer output */ /* @@ -734,10 +736,7 @@ typedef struct { VipsImage *memory; FILE *fp; - - char *buf; - size_t len; - size_t alloc; + VipsDbuf dbuf; png_structp pPng; png_infop pInfo; @@ -749,7 +748,7 @@ write_finish( Write *write ) { VIPS_FREEF( fclose, write->fp ); VIPS_UNREF( write->memory ); - VIPS_FREE( write->buf ); + vips_dbuf_destroy( &write->dbuf ); if( write->pPng ) png_destroy_write_struct( &write->pPng, &write->pInfo ); } @@ -771,9 +770,7 @@ write_new( VipsImage *in ) write->in = in; write->memory = NULL; write->fp = NULL; - write->buf = NULL; - write->len = 0; - write->alloc = 0; + vips_dbuf_init( &write->dbuf ); g_signal_connect( in, "close", G_CALLBACK( write_destroy ), write ); @@ -1014,41 +1011,12 @@ vips__png_write( VipsImage *in, const char *filename, return( 0 ); } -static void -write_grow( Write *write, size_t grow_len ) -{ - size_t new_len = write->len + grow_len; - - if( new_len > write->alloc ) { - size_t proposed_alloc = (16 + write->alloc) * 3 / 2; - - write->alloc = VIPS_MAX( proposed_alloc, new_len ); - - /* Our result must be freedd with g_free(), so it's OK to use - * g_realloc(). - */ - write->buf = g_realloc( write->buf, write->alloc ); - - VIPS_DEBUG_MSG( "write_buf_grow: grown to %zd bytes\n", - write->alloc ); - } -} - static void user_write_data( png_structp png_ptr, png_bytep data, png_size_t length ) { Write *write = (Write *) png_get_io_ptr( png_ptr ); - char *write_start; - - write_grow( write, length ); - - write_start = write->buf + write->len; - memcpy( write_start, data, length ); - - write->len += length; - - g_assert( write->len <= write->alloc ); + vips_dbuf_append( &write->dbuf, data, length ); } int @@ -1073,10 +1041,7 @@ vips__png_write_buf( VipsImage *in, return( -1 ); } - *obuf = write->buf; - write->buf = NULL; - if( olen ) - *olen = write->len; + *obuf = vips_dbuf_steal( &write->dbuf, olen ); write_finish( write ); diff --git a/libvips/include/vips/dbuf.h b/libvips/include/vips/dbuf.h index c48908f7..a23fc44d 100644 --- a/libvips/include/vips/dbuf.h +++ b/libvips/include/vips/dbuf.h @@ -48,7 +48,7 @@ typedef struct _VipsDbuf { /* The current base, and the size of the allocated memory area. */ - char *data; + unsigned char *data; size_t max_size; /* And the write point. @@ -58,12 +58,15 @@ typedef struct _VipsDbuf { void vips_dbuf_destroy( VipsDbuf *buf ); void vips_dbuf_init( VipsDbuf *buf ); -gboolean vips_dbuf_append( VipsDbuf *dbuf, const char *data, size_t size ); +gboolean vips_dbuf_allocate( VipsDbuf *dbuf, size_t size ); +unsigned char *vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size ); +gboolean vips_dbuf_append( VipsDbuf *dbuf, + const unsigned char *data, size_t size ); gboolean vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... ); void vips_dbuf_rewind( VipsDbuf *dbuf ); void vips_dbuf_destroy( VipsDbuf *dbuf ); -char *vips_dbuf_string( VipsDbuf *dbuf, size_t *size ); -char *vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ); +unsigned char *vips_dbuf_string( VipsDbuf *dbuf, size_t *size ); +unsigned char *vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ); #endif /*VIPS_DBUF_H*/ diff --git a/libvips/iofuncs/dbuf.c b/libvips/iofuncs/dbuf.c index 36d22a59..823027fe 100644 --- a/libvips/iofuncs/dbuf.c +++ b/libvips/iofuncs/dbuf.c @@ -56,24 +56,23 @@ vips_dbuf_init( VipsDbuf *dbuf ) } /** - * vips_dbuf_append: + * vips_dbuf_allocate: * @dbuf: the buffer - * @data: the data to append to the buffer - * @size: the size of the len to append + * @size: the size to allocate * - * Append len bytes from @data to the buffer. The buffer expands if necessary. + * Make sure @dbuf has at least @size bytes available for writing. * * Returns: %FALSE on out of memory, %TRUE otherwise. */ gboolean -vips_dbuf_append( VipsDbuf *dbuf, const char *data, size_t size ) +vips_dbuf_allocate( VipsDbuf *dbuf, size_t size ) { size_t new_write_point = dbuf->write_point + size; if( new_write_point > dbuf->max_size ) { size_t new_max_size = 3 * (16 + new_write_point) / 2; - char *new_data; + unsigned char *new_data; if( !(new_data = g_try_realloc( dbuf->data, new_max_size )) ) { vips_error( "VipsDbuf", "%s", _( "out of memory" ) ); @@ -84,8 +83,50 @@ vips_dbuf_append( VipsDbuf *dbuf, const char *data, size_t size ) dbuf->max_size = new_max_size; } + return( TRUE ); +} + +/** + * vips_dbuf_get_write: + * @dbuf: the buffer + * @size: (allow-none): optionally return length in bytes here + * + * Return a pointer to an area you can write to, return length of area in + * @size. Use vips_dbuf_allocate() before this call to make the space. + * + * Returns: (transfer none): start of write area. + */ +unsigned char * +vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size ) +{ + unsigned char *data = dbuf->data + dbuf->write_point; + + if( size ) + *size = dbuf->max_size - dbuf->write_point; + + dbuf->write_point = dbuf->max_size; + + return( data ); +} + +/** + * vips_dbuf_append: + * @dbuf: the buffer + * @data: the data to append to the buffer + * @size: the size of the len to append + * + * Append len bytes from @data to the buffer. The buffer expands if necessary. + * + * Returns: %FALSE on out of memory, %TRUE otherwise. + */ +gboolean +vips_dbuf_append( VipsDbuf *dbuf, const unsigned char *data, size_t size ) +{ + if( !vips_dbuf_allocate( dbuf, size ) ) + return( FALSE ); + memcpy( dbuf->data + dbuf->write_point, data, size ); - dbuf->write_point = new_write_point; + dbuf->write_point += size; return( TRUE ); } @@ -110,7 +151,7 @@ vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... ) line = g_strdup_vprintf( fmt, ap ); va_end( ap ); - if( vips_dbuf_append( dbuf, line, strlen( line ) ) ) { + if( vips_dbuf_append( dbuf, (unsigned char *) line, strlen( line ) ) ) { g_free( line ); return( FALSE ); } @@ -159,12 +200,12 @@ vips_dbuf_destroy( VipsDbuf *dbuf ) * * Returns: (transfer full): The pointer held by @dbuf. */ -char * +unsigned char * vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ) { - char *data; + unsigned char *data; - vips_dbuf_append( dbuf, "", 1 ); + vips_dbuf_append( dbuf, (unsigned char *) "", 1 ); dbuf->write_point -= 1; data = dbuf->data; @@ -180,7 +221,7 @@ vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ) } /** - * vips_dbuf_data: + * vips_dbuf_string: * @dbuf: the buffer * @size: (allow-none): optionally return length in bytes here * @@ -191,10 +232,10 @@ vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ) * * Returns: (transfer none): The pointer held by @dbuf. */ -char * +unsigned char * vips_dbuf_string( VipsDbuf *dbuf, size_t *size ) { - vips_dbuf_append( dbuf, "", 1 ); + vips_dbuf_append( dbuf, (unsigned char *) "", 1 ); dbuf->write_point -= 1; if( size ) diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 1b564e84..e33a47ab 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -619,7 +619,8 @@ parser_element_end_handler( void *user_data, const XML_Char *name ) if( vep->header ) { if( strcmp( name, "Hist" ) == 0 ) set_history( vep->image, - vips_dbuf_string( &vep->dbuf, NULL ) ); + (char *) vips_dbuf_string( &vep->dbuf, + NULL ) ); } else { GType gtype = g_type_from_name( vep->type ); @@ -631,7 +632,8 @@ parser_element_end_handler( void *user_data, const XML_Char *name ) VIPS_TYPE_SAVE_STRING, gtype ) && set_meta( vep->image, gtype, vep->name, - vips_dbuf_string( &vep->dbuf, NULL ) ) ) + (char *) vips_dbuf_string( &vep->dbuf, + NULL ) ) ) vep->error = TRUE; } } @@ -646,7 +648,7 @@ parser_data_handler( void *user_data, const XML_Char *data, int len ) printf( "parser_data_handler: %d bytes\n", len ); #endif /*DEBUG*/ - vips_dbuf_append( &vep->dbuf, data, len ); + vips_dbuf_append( &vep->dbuf, (unsigned char *) data, len ); } /* Called at the end of vips open ... get any XML after the pixel data @@ -724,9 +726,9 @@ dbuf_append_quotes( VipsDbuf *dbuf, const char *str ) for( p = str; *p; p += len ) { len = strcspn( p, "\"" ); - vips_dbuf_append( dbuf, p, len ); + vips_dbuf_append( dbuf, (unsigned char *) p, len ); if( p[len] == '"' ) - vips_dbuf_append( dbuf, "\\", 1 ); + vips_dbuf_appendf( dbuf, "\\" ); } } @@ -741,20 +743,20 @@ dbuf_append_amp( VipsDbuf *dbuf, const char *str ) for( p = str; *p; p += len ) { len = strcspn( p, "&<>" ); - vips_dbuf_append( dbuf, p, len ); + vips_dbuf_append( dbuf, (unsigned char *) p, len ); switch( p[len] ) { case '&': - vips_dbuf_append( dbuf, "&", 5 ); + vips_dbuf_appendf( dbuf, "&" ); len += 1; break; case '<': - vips_dbuf_append( dbuf, "<", 4 ); + vips_dbuf_appendf( dbuf, "<" ); len += 1; break; case '>': - vips_dbuf_append( dbuf, ">", 4 ); + vips_dbuf_appendf( dbuf, ">" ); len += 1; break; @@ -833,7 +835,7 @@ build_xml( VipsImage *image ) vips_dbuf_appendf( &dbuf, " \n" ); vips_dbuf_appendf( &dbuf, "\n" ); - return( vips_dbuf_steal( &dbuf, NULL ) ); + return( (char *) vips_dbuf_steal( &dbuf, NULL ) ); } static void * @@ -906,7 +908,7 @@ vips__xml_properties( VipsImage *image ) vips_dbuf_appendf( &dbuf, " \n" ); vips_dbuf_appendf( &dbuf, "\n" ); - return( vips_dbuf_steal( &dbuf, NULL ) ); + return( (char *) vips_dbuf_steal( &dbuf, NULL ) ); } /* Append XML to output fd. From f2a178e98f164cde38f5295668e1a8f6a0550033 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 28 Feb 2017 13:40:34 +0000 Subject: [PATCH 09/11] move buf writers on top of dbuf tiff and webp not moved --- TODO | 22 ----- libvips/foreign/radiance.c | 134 +++++++-------------------- libvips/foreign/vips2jpeg.c | 8 +- libvips/include/vips/dbuf.h | 14 ++- libvips/iofuncs/dbuf.c | 179 ++++++++++++++++++++++++++++-------- libvips/iofuncs/vips.c | 6 +- 6 files changed, 196 insertions(+), 167 deletions(-) diff --git a/TODO b/TODO index ed0c97b8..e50960d6 100644 --- a/TODO +++ b/TODO @@ -1,25 +1,3 @@ -- use VipsDbuf for tiffsave_buffer etc. - - $ grep -l save_buffer *.c - dzsave.c - jpegsave.c - pngsave.c - radsave.c - tiffsave.c - webpsave.c - - done png, jpg - - tiff needs seek ... perhaps add this to dbuf? - - dzsave uses gsf - - rad and webp left to do - - vips2jpeg term_destination() needs some kind of truncate call for dbuf - - - - verify xml data against master for vips save and dzsave diff --git a/libvips/foreign/radiance.c b/libvips/foreign/radiance.c index d6cddeb2..69e80f59 100644 --- a/libvips/foreign/radiance.c +++ b/libvips/foreign/radiance.c @@ -17,6 +17,8 @@ * readers * 23/5/16 * - add buffer save functions + * 28/2/17 + * - use dbuf for buffer output */ /* @@ -1172,9 +1174,7 @@ typedef struct { char *filename; FILE *fout; - char *buf; - size_t len; - size_t alloc; + VipsDbuf dbuf; char format[256]; double expos; @@ -1189,7 +1189,7 @@ write_destroy( Write *write ) { VIPS_FREE( write->filename ); VIPS_FREEF( fclose, write->fout ); - VIPS_FREE( write->buf ); + vips_dbuf_destroy( &write->dbuf ); vips_free( write ); } @@ -1208,9 +1208,7 @@ write_new( VipsImage *in ) write->filename = NULL; write->fout = NULL; - write->buf = NULL; - write->len = 0; - write->alloc = 0; + vips_dbuf_init( &write->dbuf ); strcpy( write->format, COLRFMT ); write->expos = 1.0; @@ -1346,91 +1344,30 @@ vips__rad_save( VipsImage *in, const char *filename ) return( 0 ); } -static void -write_buf_grow( Write *write, size_t grow_len ) -{ - size_t new_len = write->len + grow_len; - - if( new_len > write->alloc ) { - size_t proposed_alloc = (16 + write->alloc) * 3 / 2; - - write->alloc = VIPS_MAX( proposed_alloc, new_len ); - - /* Our caller must free with g_free(), so we must use - * g_realloc(). - */ - write->buf = g_realloc( write->buf, write->alloc ); - - VIPS_DEBUG_MSG( "write_buf_grow: grown to %zd bytes\n", - write->alloc ); - } -} - -static void -bprintf( Write *write, const char *fmt, ... ) -{ - int length; - char *write_start; - va_list ap; - - /* Determine required size. - */ - va_start( ap, fmt ); - length = vsnprintf( NULL, 0, fmt, ap ); - va_end( ap ); - - write_buf_grow( write, length + 1 ); - - write_start = write->buf + write->len; - - va_start( ap, fmt ); - length = vsnprintf( write_start, length + 1, fmt, ap ); - va_end( ap ); - - write->len += length; - - g_assert( write->len <= write->alloc ); -} - -#define bputformat( write, s ) \ - bprintf( write, "%s%s\n", FMTSTR, s ) - -#define bputexpos( write, ex ) \ - bprintf( write, "%s%e\n", EXPOSSTR, ex ) - -#define bputcolcor( write, cc ) \ - bprintf( write, "%s %f %f %f\n", \ - COLCORSTR, (cc)[RED], (cc)[GRN], (cc)[BLU] ) - -#define bputaspect( write, pa ) \ - bprintf( write, "%s%f\n", ASPECTSTR, pa ) - -#define bputprims( write, p ) \ - bprintf( write, "%s %.4f %.4f %.4f %.4f %.4f %.4f %.4f %.4f\n", \ - PRIMARYSTR, \ - (p)[RED][CIEX], (p)[RED][CIEY], \ - (p)[GRN][CIEX], (p)[GRN][CIEY], \ - (p)[BLU][CIEX], (p)[BLU][CIEY], \ - (p)[WHT][CIEX], (p)[WHT][CIEY] ) - -#define bputsresolu( write, rs ) \ - bprintf( write, "%s", resolu2str( resolu_buf, rs ) ) - static int vips2rad_put_header_buf( Write *write ) { vips2rad_make_header( write ); - bprintf( write, "#?RADIANCE\n" ); - - bputformat( write, write->format ); - bputexpos( write, write->expos ); - bputcolcor( write, write->colcor ); - bprintf( write, "SOFTWARE=vips %s\n", vips_version_string() ); - bputaspect( write, write->aspect ); - bputprims( write, write->prims ); - bprintf( write, "\n" ); - bputsresolu( write, &write->rs ); + vips_dbuf_appendf( &write->dbuf, "#?RADIANCE\n" ); + vips_dbuf_appendf( &write->dbuf, "%s%s\n", FMTSTR, write->format ); + vips_dbuf_appendf( &write->dbuf, "%s%e\n", EXPOSSTR, write->expos ); + vips_dbuf_appendf( &write->dbuf, "%s %f %f %f\n", + COLCORSTR, + write->colcor[RED], write->colcor[GRN], write->colcor[BLU] ); + vips_dbuf_appendf( &write->dbuf, "SOFTWARE=vips %s\n", + vips_version_string() ); + vips_dbuf_appendf( &write->dbuf, "%s%f\n", ASPECTSTR, write->aspect ); + vips_dbuf_appendf( &write->dbuf, + "%s %.4f %.4f %.4f %.4f %.4f %.4f %.4f %.4f\n", + PRIMARYSTR, + write->prims[RED][CIEX], write->prims[RED][CIEY], + write->prims[GRN][CIEX], write->prims[GRN][CIEY], + write->prims[BLU][CIEX], write->prims[BLU][CIEY], + write->prims[WHT][CIEX], write->prims[WHT][CIEY] ); + vips_dbuf_appendf( &write->dbuf, "\n" ); + vips_dbuf_appendf( &write->dbuf, "%s", + resolu2str( resolu_buf, &write->rs ) ); return( 0 ); } @@ -1441,25 +1378,25 @@ static int scanline_write_buf( Write *write, COLR *scanline, int width ) { unsigned char *buffer; + size_t size; + int length; - write_buf_grow( write, MAX_LINE ); - buffer = (unsigned char *) write->buf + write->len; + vips_dbuf_allocate( &write->dbuf, MAX_LINE ); + buffer = vips_dbuf_get_write( &write->dbuf, &size ); if( width < MINELEN || width > MAXELEN ) { /* Write as a flat scanline. */ - memcpy( buffer, scanline, sizeof( COLR ) * width ); - write->len += sizeof( COLR ) * width; + length = sizeof( COLR ) * width; + memcpy( buffer, scanline, length ); } - else { - int length; - + else /* An RLE scanline. */ rle_scanline_write( scanline, width, buffer, &length ); - write->len += length; - } + + vips_dbuf_seek( &write->dbuf, length - size, SEEK_CUR ); return( 0 ); } @@ -1510,10 +1447,7 @@ vips__rad_save_buf( VipsImage *in, void **obuf, size_t *olen ) return( -1 ); } - *obuf = write->buf; - write->buf = NULL; - if( olen ) - *olen = write->len; + *obuf = vips_dbuf_steal( &write->dbuf, olen ); write_destroy( write ); diff --git a/libvips/foreign/vips2jpeg.c b/libvips/foreign/vips2jpeg.c index 0f8f0365..1c878986 100644 --- a/libvips/foreign/vips2jpeg.c +++ b/libvips/foreign/vips2jpeg.c @@ -734,10 +734,12 @@ term_destination( j_compress_ptr cinfo ) size_t size; - /* This can be called before the output area fills. We need to chop - * size down to the number of bytes actually written. + /* We probably won't have filled the area that was last allocated in + * empty_output_buffer(). Chop the data size down to the length that + * was actually written. */ - buf->dbuf.write_point = buf->dbuf.max_size - buf->pub.free_in_buffer; + vips_dbuf_seek( &buf->dbuf, -buf->pub.free_in_buffer, SEEK_END ); + vips_dbuf_truncate( &buf->dbuf ); *(buf->obuf) = vips_dbuf_steal( &buf->dbuf, &size ); *(buf->olen) = size; diff --git a/libvips/include/vips/dbuf.h b/libvips/include/vips/dbuf.h index a23fc44d..58cb5f47 100644 --- a/libvips/include/vips/dbuf.h +++ b/libvips/include/vips/dbuf.h @@ -49,11 +49,17 @@ typedef struct _VipsDbuf { /* The current base, and the size of the allocated memory area. */ unsigned char *data; - size_t max_size; + size_t allocated_size; - /* And the write point. + /* The size of the actual data that's been written. This will usually + * be <= allocated_size, but always >= write_point. + */ + size_t data_size; + + /* The write point. */ size_t write_point; + } VipsDbuf; void vips_dbuf_destroy( VipsDbuf *buf ); @@ -63,8 +69,10 @@ unsigned char *vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size ); gboolean vips_dbuf_append( VipsDbuf *dbuf, const unsigned char *data, size_t size ); gboolean vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... ); -void vips_dbuf_rewind( VipsDbuf *dbuf ); +void vips_dbuf_reset( VipsDbuf *dbuf ); void vips_dbuf_destroy( VipsDbuf *dbuf ); +gboolean vips_dbuf_seek( VipsDbuf *dbuf, off_t offset, int whence ); +void vips_dbuf_truncate( VipsDbuf *dbuf ); unsigned char *vips_dbuf_string( VipsDbuf *dbuf, size_t *size ); unsigned char *vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ); diff --git a/libvips/iofuncs/dbuf.c b/libvips/iofuncs/dbuf.c index 823027fe..f1b7b0e9 100644 --- a/libvips/iofuncs/dbuf.c +++ b/libvips/iofuncs/dbuf.c @@ -51,37 +51,74 @@ void vips_dbuf_init( VipsDbuf *dbuf ) { dbuf->data = NULL; - dbuf->max_size = 0; + dbuf->allocated_size = 0; + dbuf->data_size = 0; dbuf->write_point = 0; } +/** + * vips_dbuf_minimum_size: + * @dbuf: the buffer + * @size: the minimum size + * + * Make sure @dbuf is at least @size bytes. + * + * Returns: %FALSE on out of memory, %TRUE otherwise. + */ +gboolean +vips_dbuf_minimum_size( VipsDbuf *dbuf, size_t size ) +{ + if( size > dbuf->allocated_size ) { + size_t new_allocated_size = 3 * (16 + size) / 2; + + unsigned char *new_data; + + if( !(new_data = + g_try_realloc( dbuf->data, new_allocated_size )) ) { + vips_error( "VipsDbuf", "%s", _( "out of memory" ) ); + return( FALSE ); + } + + dbuf->data = new_data; + dbuf->allocated_size = new_allocated_size; + } + + return( TRUE ); +} + /** * vips_dbuf_allocate: * @dbuf: the buffer * @size: the size to allocate * - * Make sure @dbuf has at least @size bytes available for writing. + * Make sure @dbuf has at least @size bytes available after the write point. * * Returns: %FALSE on out of memory, %TRUE otherwise. */ gboolean vips_dbuf_allocate( VipsDbuf *dbuf, size_t size ) { - size_t new_write_point = dbuf->write_point + size; + return( vips_dbuf_minimum_size( dbuf, dbuf->write_point + size ) ); +} - if( new_write_point > dbuf->max_size ) { - size_t new_max_size = 3 * (16 + new_write_point) / 2; +/** + * vips_dbuf_null_terminate: + * @dbuf: the buffer + * + * Make sure the byte after the last data byte is `\0`. This extra byte is not + * included in the data size and the write point is not moved. + * + * This makes it safe to treat the dbuf contents as a C string. + * + * Returns: %FALSE on out of memory, %TRUE otherwise. + */ +static gboolean +vips_dbuf_null_terminate( VipsDbuf *dbuf ) +{ + if( !vips_dbuf_minimum_size( dbuf, dbuf->data_size + 1 ) ) + return( FALSE ); - unsigned char *new_data; - - if( !(new_data = g_try_realloc( dbuf->data, new_max_size )) ) { - vips_error( "VipsDbuf", "%s", _( "out of memory" ) ); - return( FALSE ); - } - - dbuf->data = new_data; - dbuf->max_size = new_max_size; - } + dbuf->data[dbuf->data_size] = 0; return( TRUE ); } @@ -92,21 +129,28 @@ vips_dbuf_allocate( VipsDbuf *dbuf, size_t size ) * @size: (allow-none): optionally return length in bytes here * * Return a pointer to an area you can write to, return length of area in - * @size. Use vips_dbuf_allocate() before this call to make the space. + * @size. Use vips_dbuf_allocate() before this call to set a minimum amount of + * space to have available. + * + * The write point moves to just beyond the returned block. Use + * vips_dbuf_seek() to move it back again. * * Returns: (transfer none): start of write area. */ unsigned char * vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size ) { - unsigned char *data = dbuf->data + dbuf->write_point; + unsigned char *write = dbuf->data + dbuf->write_point; + size_t length = dbuf->data + dbuf->allocated_size - write; + + memset( write, 0, length ); + dbuf->write_point = dbuf->allocated_size; + dbuf->data_size = dbuf->allocated_size; if( size ) - *size = dbuf->max_size - dbuf->write_point; + *size = length; - dbuf->write_point = dbuf->max_size; - - return( data ); + return( write ); } /** @@ -115,7 +159,7 @@ vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size ) * @data: the data to append to the buffer * @size: the size of the len to append * - * Append len bytes from @data to the buffer. The buffer expands if necessary. + * Append @size bytes from @data. @dbuf expands if necessary. * * Returns: %FALSE on out of memory, %TRUE otherwise. */ @@ -127,6 +171,7 @@ vips_dbuf_append( VipsDbuf *dbuf, const unsigned char *data, size_t size ) memcpy( dbuf->data + dbuf->write_point, data, size ); dbuf->write_point += size; + dbuf->data_size = VIPS_MAX( dbuf->data_size, dbuf->write_point ); return( TRUE ); } @@ -161,16 +206,17 @@ vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... ) } /** - * vips_dbuf_rewind: + * vips_dbuf_reset: * @dbuf: the buffer * - * Reset the buffer to empty. No memory is freed, just the write pointer is - * reset. + * Reset the buffer to empty. No memory is freed, just the data size and + * write point are reset. */ void -vips_dbuf_rewind( VipsDbuf *dbuf ) +vips_dbuf_reset( VipsDbuf *dbuf ) { dbuf->write_point = 0; + dbuf->data_size = 0; } /** @@ -182,9 +228,73 @@ vips_dbuf_rewind( VipsDbuf *dbuf ) void vips_dbuf_destroy( VipsDbuf *dbuf ) { + vips_dbuf_reset( dbuf ); + VIPS_FREE( dbuf->data ); - dbuf->max_size = 0; - dbuf->write_point = 0; + dbuf->allocated_size = 0; +} + +/** + * vips_dbuf_seek: + * @dbuf: the buffer + * @offset: how to move the write point + * @whence: from start, from end, from current + * + * Move the write point. @whence can be %SEEK_SET, %SEEK_CUR, %SEEK_END, with + * the usual meaning. + */ +gboolean +vips_dbuf_seek( VipsDbuf *dbuf, off_t offset, int whence ) +{ + off_t new_write_point; + + switch( whence ) { + case SEEK_SET: + new_write_point = offset; + break; + + case SEEK_END: + new_write_point = dbuf->data_size + offset; + break; + + case SEEK_CUR: + new_write_point = dbuf->write_point + offset; + break; + + default: + g_assert( 0 ); + new_write_point = dbuf->write_point; + break; + } + + if( new_write_point < 0 ) { + vips_error( "VipsDbuf", "%s", "negative seek" ); + return( FALSE ); + } + + /* Possibly need to grow the buffer + */ + if( !vips_dbuf_minimum_size( dbuf, new_write_point ) ) + return( FALSE ); + dbuf->write_point = new_write_point; + if( dbuf->data_size < dbuf->write_point ) { + memset( dbuf->data, 0, dbuf->write_point - dbuf->data_size ); + dbuf->data_size = dbuf->write_point; + } + + return( TRUE ); +} + +/** + * vips_dbuf_truncate: + * @dbuf: the buffer + * + * Truncate the data so that it ends at the write point. No memory is freed. + */ +void +vips_dbuf_truncate( VipsDbuf *dbuf ) +{ + dbuf->data_size = dbuf->write_point; } /** @@ -205,17 +315,15 @@ vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ) { unsigned char *data; - vips_dbuf_append( dbuf, (unsigned char *) "", 1 ); - dbuf->write_point -= 1; + vips_dbuf_null_terminate( dbuf ); data = dbuf->data; if( size ) - *size = dbuf->write_point; + *size = dbuf->data_size; dbuf->data = NULL; - dbuf->max_size = 0; - dbuf->write_point = 0; + vips_dbuf_destroy( dbuf ); return( data ); } @@ -235,11 +343,10 @@ vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ) unsigned char * vips_dbuf_string( VipsDbuf *dbuf, size_t *size ) { - vips_dbuf_append( dbuf, (unsigned char *) "", 1 ); - dbuf->write_point -= 1; + vips_dbuf_null_terminate( dbuf ); if( size ) - *size = dbuf->write_point; + *size = dbuf->data_size; return( dbuf->data ); } diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index e33a47ab..9b4c3935 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -536,7 +536,7 @@ parser_element_start_handler( void *user_data, vips_strncpy( vep->type, p[1], MAX_PARSE_ATTR ); } - vips_dbuf_rewind( &vep->dbuf ); + vips_dbuf_reset( &vep->dbuf ); } else if( strcmp( name, "header" ) == 0 ) vep->header = TRUE; @@ -545,7 +545,7 @@ parser_element_start_handler( void *user_data, else if( strcmp( name, "root" ) == 0 ) { for( p = atts; *p; p += 2 ) if( strcmp( p[0], "xmlns" ) == 0 && - !vips_isprefix( NAMESPACE_URI "/vips", p[1] ) ) { + !vips_isprefix( NAMESPACE_URI "vips", p[1] ) ) { vips_error( "VipsImage", "%s", _( "incorrect namespace in XML" ) ); vep->error = TRUE; @@ -812,7 +812,7 @@ build_xml( VipsImage *image ) vips_dbuf_init( &dbuf ); vips_dbuf_appendf( &dbuf, "\n" ); - vips_dbuf_appendf( &dbuf, "\n", + vips_dbuf_appendf( &dbuf, "\n", NAMESPACE_URI, VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION ); vips_dbuf_appendf( &dbuf, "
\n" ); From 8f47c75a85cd1795115c304b510859e8f9a53c1a Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 28 Feb 2017 16:44:12 +0000 Subject: [PATCH 10/11] tiff uses vipdbuf --- TODO | 5 +++ libvips/foreign/radiance.c | 18 ++++----- libvips/foreign/tiff.c | 52 ++++---------------------- libvips/foreign/vipspng.c | 2 +- libvips/include/vips/dbuf.h | 8 ++-- libvips/iofuncs/dbuf.c | 31 +++++++++++----- libvips/iofuncs/vips.c | 74 ++++++++++++++++++------------------- 7 files changed, 85 insertions(+), 105 deletions(-) diff --git a/TODO b/TODO index e50960d6..95423cd2 100644 --- a/TODO +++ b/TODO @@ -1,5 +1,10 @@ - verify xml data against master for vips save and dzsave +- curious + + $ vips tiffload_buffer images/sample.tif + (vips:18224): GLib-GObject-WARNING **: unable to set property 'buffer' of type 'VipsBlob' from value of type 'gchararray' + Trace/breakpoint trap (core dumped) diff --git a/libvips/foreign/radiance.c b/libvips/foreign/radiance.c index 69e80f59..cb62a4eb 100644 --- a/libvips/foreign/radiance.c +++ b/libvips/foreign/radiance.c @@ -1349,24 +1349,24 @@ vips2rad_put_header_buf( Write *write ) { vips2rad_make_header( write ); - vips_dbuf_appendf( &write->dbuf, "#?RADIANCE\n" ); - vips_dbuf_appendf( &write->dbuf, "%s%s\n", FMTSTR, write->format ); - vips_dbuf_appendf( &write->dbuf, "%s%e\n", EXPOSSTR, write->expos ); - vips_dbuf_appendf( &write->dbuf, "%s %f %f %f\n", + vips_dbuf_writef( &write->dbuf, "#?RADIANCE\n" ); + vips_dbuf_writef( &write->dbuf, "%s%s\n", FMTSTR, write->format ); + vips_dbuf_writef( &write->dbuf, "%s%e\n", EXPOSSTR, write->expos ); + vips_dbuf_writef( &write->dbuf, "%s %f %f %f\n", COLCORSTR, write->colcor[RED], write->colcor[GRN], write->colcor[BLU] ); - vips_dbuf_appendf( &write->dbuf, "SOFTWARE=vips %s\n", + vips_dbuf_writef( &write->dbuf, "SOFTWARE=vips %s\n", vips_version_string() ); - vips_dbuf_appendf( &write->dbuf, "%s%f\n", ASPECTSTR, write->aspect ); - vips_dbuf_appendf( &write->dbuf, + vips_dbuf_writef( &write->dbuf, "%s%f\n", ASPECTSTR, write->aspect ); + vips_dbuf_writef( &write->dbuf, "%s %.4f %.4f %.4f %.4f %.4f %.4f %.4f %.4f\n", PRIMARYSTR, write->prims[RED][CIEX], write->prims[RED][CIEY], write->prims[GRN][CIEX], write->prims[GRN][CIEY], write->prims[BLU][CIEX], write->prims[BLU][CIEY], write->prims[WHT][CIEX], write->prims[WHT][CIEY] ); - vips_dbuf_appendf( &write->dbuf, "\n" ); - vips_dbuf_appendf( &write->dbuf, "%s", + vips_dbuf_writef( &write->dbuf, "\n" ); + vips_dbuf_writef( &write->dbuf, "%s", resolu2str( resolu_buf, &write->rs ) ); return( 0 ); diff --git a/libvips/foreign/tiff.c b/libvips/foreign/tiff.c index b083adc2..232409c1 100644 --- a/libvips/foreign/tiff.c +++ b/libvips/foreign/tiff.c @@ -298,10 +298,7 @@ vips__tiff_openin_buffer( VipsImage *image, const void *data, size_t length ) */ typedef struct _VipsTiffOpenoutBuffer { - size_t position; - void *data; - size_t allocated; - size_t length; + VipsDbuf dbuf; /* On close, consolidate and write the output here. */ @@ -321,37 +318,12 @@ static tsize_t openout_buffer_write( thandle_t st, tdata_t data, tsize_t size ) { VipsTiffOpenoutBuffer *buffer = (VipsTiffOpenoutBuffer *) st; - size_t new_length = VIPS_MAX( buffer->position + size, buffer->length ); #ifdef DEBUG printf( "openout_buffer_write: %zd bytes\n", size ); #endif /*DEBUG*/ - /* We could make a chain of blocks, but libtiff does a lot of seeking - * during writes, so we'd have to handle writes being - * split over blocks, which would be annoying. - * - * Instead, go for exponential realloc: the number of reallocs grows as - * log(final size), and we never over allocate by more than 30%. - * - * realloc can be very, very slow on Windows, but maybe g_realloc() is - * smarter. - */ - if( new_length > buffer->allocated ) { - buffer->allocated = VIPS_MAX( (16 + buffer->allocated) * 3 / 2, - new_length ); - buffer->data = g_try_realloc( buffer->data, buffer->allocated ); - - if( buffer->data == NULL ) { - vips_error( "tiff memory write", - "%s", _( "Out of memory." ) ); - return( 0 ); - } - } - - memcpy( buffer->data + buffer->position, data, size ); - buffer->position += size; - buffer->length = new_length; + vips_dbuf_write( &buffer->dbuf, data, size ); return( size ); } @@ -361,8 +333,8 @@ openout_buffer_close( thandle_t st ) { VipsTiffOpenoutBuffer *buffer = (VipsTiffOpenoutBuffer *) st; - *(buffer->out_data) = buffer->data; - *(buffer->out_length) = buffer->length; + *(buffer->out_data) = vips_dbuf_steal( &buffer->dbuf, + buffer->out_length); return( 0 ); } @@ -377,16 +349,9 @@ openout_buffer_seek( thandle_t st, toff_t position, int whence ) position, whence ); #endif /*DEBUG*/ - if( whence == SEEK_SET ) - buffer->position = position; - else if( whence == SEEK_CUR ) - buffer->position += position; - else if( whence == SEEK_END ) - buffer->position = buffer->length + position; - else - g_assert_not_reached(); + vips_dbuf_seek( &buffer->dbuf, position, whence ); - return( buffer->position ); + return( vips_dbuf_tell( &buffer->dbuf ) ); } static toff_t @@ -429,10 +394,7 @@ vips__tiff_openout_buffer( VipsImage *image, #endif /*DEBUG*/ buffer = VIPS_NEW( image, VipsTiffOpenoutBuffer ); - buffer->position = 0; - buffer->data = NULL; - buffer->allocated = 0; - buffer->length = 0; + vips_dbuf_init( &buffer->dbuf ); buffer->out_data = out_data; buffer->out_length = out_length; diff --git a/libvips/foreign/vipspng.c b/libvips/foreign/vipspng.c index 4de6fe3c..33bcd21c 100644 --- a/libvips/foreign/vipspng.c +++ b/libvips/foreign/vipspng.c @@ -1016,7 +1016,7 @@ user_write_data( png_structp png_ptr, png_bytep data, png_size_t length ) { Write *write = (Write *) png_get_io_ptr( png_ptr ); - vips_dbuf_append( &write->dbuf, data, length ); + vips_dbuf_write( &write->dbuf, data, length ); } int diff --git a/libvips/include/vips/dbuf.h b/libvips/include/vips/dbuf.h index 58cb5f47..3234dbbe 100644 --- a/libvips/include/vips/dbuf.h +++ b/libvips/include/vips/dbuf.h @@ -37,8 +37,7 @@ extern "C" { #include -/* A buffer in the process of being written to ... multiple calls to - * vips_dbuf_append add to it. +/* A buffer in the process of being written to. */ typedef struct _VipsDbuf { @@ -66,13 +65,14 @@ void vips_dbuf_destroy( VipsDbuf *buf ); void vips_dbuf_init( VipsDbuf *buf ); gboolean vips_dbuf_allocate( VipsDbuf *dbuf, size_t size ); unsigned char *vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size ); -gboolean vips_dbuf_append( VipsDbuf *dbuf, +gboolean vips_dbuf_write( VipsDbuf *dbuf, const unsigned char *data, size_t size ); -gboolean vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... ); +gboolean vips_dbuf_writef( VipsDbuf *dbuf, const char *fmt, ... ); void vips_dbuf_reset( VipsDbuf *dbuf ); void vips_dbuf_destroy( VipsDbuf *dbuf ); gboolean vips_dbuf_seek( VipsDbuf *dbuf, off_t offset, int whence ); void vips_dbuf_truncate( VipsDbuf *dbuf ); +off_t vips_dbuf_tell( VipsDbuf *dbuf ); unsigned char *vips_dbuf_string( VipsDbuf *dbuf, size_t *size ); unsigned char *vips_dbuf_steal( VipsDbuf *dbuf, size_t *size ); diff --git a/libvips/iofuncs/dbuf.c b/libvips/iofuncs/dbuf.c index f1b7b0e9..2bf9db8a 100644 --- a/libvips/iofuncs/dbuf.c +++ b/libvips/iofuncs/dbuf.c @@ -154,17 +154,17 @@ vips_dbuf_get_write( VipsDbuf *dbuf, size_t *size ) } /** - * vips_dbuf_append: + * vips_dbuf_write: * @dbuf: the buffer - * @data: the data to append to the buffer - * @size: the size of the len to append + * @data: the data to write to the buffer + * @size: the size of the len to write * * Append @size bytes from @data. @dbuf expands if necessary. * * Returns: %FALSE on out of memory, %TRUE otherwise. */ gboolean -vips_dbuf_append( VipsDbuf *dbuf, const unsigned char *data, size_t size ) +vips_dbuf_write( VipsDbuf *dbuf, const unsigned char *data, size_t size ) { if( !vips_dbuf_allocate( dbuf, size ) ) return( FALSE ); @@ -177,17 +177,17 @@ vips_dbuf_append( VipsDbuf *dbuf, const unsigned char *data, size_t size ) } /** - * vips_dbuf_appendf: + * vips_dbuf_writef: * @dbuf: the buffer * @fmt: printf()-style format string * @...: arguments to format string * - * Format the string and append to @dbuf. + * Format the string and write to @dbuf. * * Returns: %FALSE on out of memory, %TRUE otherwise. */ gboolean -vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... ) +vips_dbuf_writef( VipsDbuf *dbuf, const char *fmt, ... ) { va_list ap; char *line; @@ -196,7 +196,7 @@ vips_dbuf_appendf( VipsDbuf *dbuf, const char *fmt, ... ) line = g_strdup_vprintf( fmt, ap ); va_end( ap ); - if( vips_dbuf_append( dbuf, (unsigned char *) line, strlen( line ) ) ) { + if( vips_dbuf_write( dbuf, (unsigned char *) line, strlen( line ) ) ) { g_free( line ); return( FALSE ); } @@ -278,7 +278,8 @@ vips_dbuf_seek( VipsDbuf *dbuf, off_t offset, int whence ) return( FALSE ); dbuf->write_point = new_write_point; if( dbuf->data_size < dbuf->write_point ) { - memset( dbuf->data, 0, dbuf->write_point - dbuf->data_size ); + memset( dbuf->data + dbuf->data_size, 0, + dbuf->write_point - dbuf->data_size ); dbuf->data_size = dbuf->write_point; } @@ -297,6 +298,18 @@ vips_dbuf_truncate( VipsDbuf *dbuf ) dbuf->data_size = dbuf->write_point; } +/** + * vips_dbuf_truncate: + * @dbuf: the buffer + * + * Truncate the data so that it ends at the write point. No memory is freed. + */ +off_t +vips_dbuf_tell( VipsDbuf *dbuf ) +{ + return( dbuf->write_point ); +} + /** * vips_dbuf_steal: * @dbuf: the buffer diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 9b4c3935..a92aa273 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -648,7 +648,7 @@ parser_data_handler( void *user_data, const XML_Char *data, int len ) printf( "parser_data_handler: %d bytes\n", len ); #endif /*DEBUG*/ - vips_dbuf_append( &vep->dbuf, (unsigned char *) data, len ); + vips_dbuf_write( &vep->dbuf, (unsigned char *) data, len ); } /* Called at the end of vips open ... get any XML after the pixel data @@ -718,7 +718,7 @@ vips__write_extension_block( VipsImage *im, void *buf, int size ) /* Append a string to a buffer, but escape " as \". */ static void -dbuf_append_quotes( VipsDbuf *dbuf, const char *str ) +dbuf_write_quotes( VipsDbuf *dbuf, const char *str ) { const char *p; size_t len; @@ -726,16 +726,16 @@ dbuf_append_quotes( VipsDbuf *dbuf, const char *str ) for( p = str; *p; p += len ) { len = strcspn( p, "\"" ); - vips_dbuf_append( dbuf, (unsigned char *) p, len ); + vips_dbuf_write( dbuf, (unsigned char *) p, len ); if( p[len] == '"' ) - vips_dbuf_appendf( dbuf, "\\" ); + vips_dbuf_writef( dbuf, "\\" ); } } /* Append a string to a buffer, but escape &<>. */ static void -dbuf_append_amp( VipsDbuf *dbuf, const char *str ) +dbuf_write_amp( VipsDbuf *dbuf, const char *str ) { const char *p; size_t len; @@ -743,20 +743,20 @@ dbuf_append_amp( VipsDbuf *dbuf, const char *str ) for( p = str; *p; p += len ) { len = strcspn( p, "&<>" ); - vips_dbuf_append( dbuf, (unsigned char *) p, len ); + vips_dbuf_write( dbuf, (unsigned char *) p, len ); switch( p[len] ) { case '&': - vips_dbuf_appendf( dbuf, "&" ); + vips_dbuf_writef( dbuf, "&" ); len += 1; break; case '<': - vips_dbuf_appendf( dbuf, "<" ); + vips_dbuf_writef( dbuf, "<" ); len += 1; break; case '>': - vips_dbuf_appendf( dbuf, ">" ); + vips_dbuf_writef( dbuf, ">" ); len += 1; break; @@ -788,12 +788,12 @@ build_xml_meta( VipsMeta *meta, VipsDbuf *dbuf ) } str = vips_value_get_save_string( &save_value ); - vips_dbuf_appendf( dbuf, " name ); - vips_dbuf_appendf( dbuf, "\">" ); - dbuf_append_amp( dbuf, str ); - vips_dbuf_appendf( dbuf, "\n" ); + dbuf_write_quotes( dbuf, meta->name ); + vips_dbuf_writef( dbuf, "\">" ); + dbuf_write_amp( dbuf, str ); + vips_dbuf_writef( dbuf, "\n" ); g_value_unset( &save_value ); } @@ -811,20 +811,20 @@ build_xml( VipsImage *image ) vips_dbuf_init( &dbuf ); - vips_dbuf_appendf( &dbuf, "\n" ); - vips_dbuf_appendf( &dbuf, "\n", + vips_dbuf_writef( &dbuf, "\n" ); + vips_dbuf_writef( &dbuf, "\n", NAMESPACE_URI, VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION ); - vips_dbuf_appendf( &dbuf, "
\n" ); + vips_dbuf_writef( &dbuf, "
\n" ); str = vips_image_get_history( image ); - vips_dbuf_appendf( &dbuf, " ", + vips_dbuf_writef( &dbuf, " ", g_type_name( VIPS_TYPE_REF_STRING ) ); - dbuf_append_amp( &dbuf, str ); - vips_dbuf_appendf( &dbuf, "\n" ); + dbuf_write_amp( &dbuf, str ); + vips_dbuf_writef( &dbuf, "\n" ); - vips_dbuf_appendf( &dbuf, "
\n" ); - vips_dbuf_appendf( &dbuf, " \n" ); + vips_dbuf_writef( &dbuf, "
\n" ); + vips_dbuf_writef( &dbuf, " \n" ); if( vips_slist_map2( image->meta_traverse, (VipsSListMap2Fn) build_xml_meta, &dbuf, NULL ) ) { @@ -832,8 +832,8 @@ build_xml( VipsImage *image ) return( NULL ); } - vips_dbuf_appendf( &dbuf, " \n" ); - vips_dbuf_appendf( &dbuf, "\n" ); + vips_dbuf_writef( &dbuf, " \n" ); + vips_dbuf_writef( &dbuf, "\n" ); return( (char *) vips_dbuf_steal( &dbuf, NULL ) ); } @@ -863,15 +863,15 @@ vips__xml_properties_meta( VipsImage *image, str = vips_value_get_save_string( &save_value ); g_value_unset( &save_value ); - vips_dbuf_appendf( dbuf, " \n" ); - vips_dbuf_appendf( dbuf, " " ); - dbuf_append_amp( dbuf, field ); - vips_dbuf_appendf( dbuf, "\n" ); - vips_dbuf_appendf( dbuf, " ", + vips_dbuf_writef( dbuf, " \n" ); + vips_dbuf_writef( dbuf, " " ); + dbuf_write_amp( dbuf, field ); + vips_dbuf_writef( dbuf, "\n" ); + vips_dbuf_writef( dbuf, " ", g_type_name( type ) ); - dbuf_append_amp( dbuf, str ); - vips_dbuf_appendf( dbuf, "\n" ); - vips_dbuf_appendf( dbuf, " \n" ); + dbuf_write_amp( dbuf, str ); + vips_dbuf_writef( dbuf, "\n" ); + vips_dbuf_writef( dbuf, " \n" ); } return( NULL ); @@ -891,22 +891,22 @@ vips__xml_properties( VipsImage *image ) g_get_current_time( &now ); date = g_time_val_to_iso8601( &now ); - vips_dbuf_appendf( &dbuf, "\n" ); - vips_dbuf_appendf( &dbuf, "\n" ); + vips_dbuf_writef( &dbuf, "\n", NAMESPACE_URI, date, VIPS_MAJOR_VERSION, VIPS_MINOR_VERSION, VIPS_MICRO_VERSION ); g_free( date ); - vips_dbuf_appendf( &dbuf, " \n" ); + vips_dbuf_writef( &dbuf, " \n" ); if( vips_image_map( image, vips__xml_properties_meta, &dbuf ) ) { vips_dbuf_destroy( &dbuf ); return( NULL ); } - vips_dbuf_appendf( &dbuf, " \n" ); - vips_dbuf_appendf( &dbuf, "\n" ); + vips_dbuf_writef( &dbuf, " \n" ); + vips_dbuf_writef( &dbuf, "\n" ); return( (char *) vips_dbuf_steal( &dbuf, NULL ) ); } From c05a4b67dc8defebbe0a71cf73d612f813ef29d2 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 28 Feb 2017 17:17:23 +0000 Subject: [PATCH 11/11] fix vips-properties.xml and we're done --- TODO | 10 ---------- libvips/iofuncs/vips.c | 3 ++- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/TODO b/TODO index 95423cd2..1645055c 100644 --- a/TODO +++ b/TODO @@ -1,13 +1,3 @@ -- verify xml data against master for vips save and dzsave - -- curious - - $ vips tiffload_buffer images/sample.tif - (vips:18224): GLib-GObject-WARNING **: unable to set property 'buffer' of type 'VipsBlob' from value of type 'gchararray' - Trace/breakpoint trap (core dumped) - - - - vips linecache has access there twice! $ vips linecache diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index a92aa273..c18c9742 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -861,7 +861,6 @@ vips__xml_properties_meta( VipsImage *image, return( dbuf ); } str = vips_value_get_save_string( &save_value ); - g_value_unset( &save_value ); vips_dbuf_writef( dbuf, " \n" ); vips_dbuf_writef( dbuf, " " ); @@ -872,6 +871,8 @@ vips__xml_properties_meta( VipsImage *image, dbuf_write_amp( dbuf, str ); vips_dbuf_writef( dbuf, "\n" ); vips_dbuf_writef( dbuf, " \n" ); + + g_value_unset( &save_value ); } return( NULL );