From 8e0de67f65969a76b4260e35f55200277b7446d5 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 30 Oct 2012 17:16:55 +0000 Subject: [PATCH] sync --- TODO | 31 ++++++++++++++++++++++++++++++ libvips/colour/colour.c | 34 ++++++++++++++++++++------------- libvips/colour/sRGB2XYZ.c | 12 ++++++++++++ libvips/conversion/sequential.c | 2 +- libvips/foreign/foreign.c | 32 ++++++++++++++++++++++++++++++- libvips/foreign/jpeg2vips.c | 19 ++++++++---------- libvips/iofuncs/generate.c | 31 +++++++++++++++++++++++++++++- libvips/iofuncs/image.c | 5 ++++- libvips/iofuncs/region.c | 5 +++-- libvips/iofuncs/sinkdisc.c | 6 ++---- libvips/iofuncs/sinkmemory.c | 5 +++-- 11 files changed, 146 insertions(+), 36 deletions(-) diff --git a/TODO b/TODO index 98919efc..73b56a9d 100644 --- a/TODO +++ b/TODO @@ -4,6 +4,37 @@ trim this down! +- seq is disabled in colour.c + + + +- try: + + $ valgrind --db-attach=yes vips sRGB2XYZ kylie110.jpg x2.v --vips-cache-trace --vips-concurrency=1 --vips-progress + + get: + + ==22629== Thread 4: + ==22629== Conditional jump or move depends on uninitialised value(s) + ==22629== at 0x4EDA287: vips_col_sRGB2XYZ (LabQ2sRGB.c:259) + ==22629== by 0x4EDB1DC: vips_sRGB2XYZ_line (sRGB2XYZ.c:78) + + + the .jpg is read to a mem buffer, we then process from that + + the mem buffer is created OK and we don't get unint errors if we read from + there during create + + the vips_sRGB2XYZ_line() is called with p pointing to the start of the mem + buffer and triggers the uninit + + therefore something during the rewind must be causing uninit to be set + + + + + + - add mono as a colourspace? also rad? diff --git a/libvips/colour/colour.c b/libvips/colour/colour.c index 7c4b0121..c1543462 100644 --- a/libvips/colour/colour.c +++ b/libvips/colour/colour.c @@ -65,20 +65,31 @@ vips_colour_gen( VipsRegion *or, /* Prepare all input regions and make buffer pointers. */ - for( i = 0; ir[i]; i++ ) { + for( i = 0; ir[i]; i++ ) if( vips_region_prepare( ir[i], r ) ) return( -1 ); - p[i] = (VipsPel *) VIPS_REGION_ADDR( ir[i], r->left, r->top ); - } - p[i] = NULL; - q = (VipsPel *) VIPS_REGION_ADDR( or, r->left, r->top ); + + int x; + int sum; + + printf( "vips_colour_gen: testing mem buffer #5 %p for uninit\n", + VIPS_REGION_ADDR( ir[0], r->left, r->top ) ); + sum = 0; + for( y = 0; y < r->height; y++ ) + for( x = 0; x < r->width; x++ ) + sum += + VIPS_REGION_ADDR( ir[0], x + r->left, y + r->top )[0] + + VIPS_REGION_ADDR( ir[0], x + r->left, y + r->top )[1] + + VIPS_REGION_ADDR( ir[0], x + r->left, y + r->top )[2]; + printf( "sum = %d\n", sum ); for( y = 0; y < r->height; y++ ) { - class->process_line( colour, q, p, r->width ); - for( i = 0; ir[i]; i++ ) - p[i] += VIPS_REGION_LSKIP( ir[i] ); - q += VIPS_REGION_LSKIP( or ); + p[i] = VIPS_REGION_ADDR( ir[i], r->left, r->top + y ); + p[i] = NULL; + q = VIPS_REGION_ADDR( or, r->left, r->top + y ); + + class->process_line( colour, q, p, r->width ); } return( 0 ); @@ -169,7 +180,7 @@ vips_colour_class_init( VipsColourClass *class ) vobject_class->description = _( "colour operations" ); vobject_class->build = vips_colour_build; - operation_class->flags = VIPS_OPERATION_SEQUENTIAL; + //operation_class->flags = VIPS_OPERATION_SEQUENTIAL; VIPS_ARG_IMAGE( class, "out", 100, _( "Output" ), @@ -761,7 +772,6 @@ vips_colour_convert_class_init( VipsColourConvertClass *class ) { GObjectClass *gobject_class = G_OBJECT_CLASS( class ); VipsObjectClass *vobject_class = VIPS_OBJECT_CLASS( class ); - VipsOperationClass *operation_class = VIPS_OPERATION_CLASS( class ); gobject_class->set_property = vips_object_set_property; gobject_class->get_property = vips_object_get_property; @@ -770,8 +780,6 @@ vips_colour_convert_class_init( VipsColourConvertClass *class ) vobject_class->description = _( "convert to a new colourspace" ); vobject_class->build = vips_colour_convert_build; - operation_class->flags = VIPS_OPERATION_SEQUENTIAL; - VIPS_ARG_IMAGE( class, "in", 1, _( "Output" ), _( "Output image" ), diff --git a/libvips/colour/sRGB2XYZ.c b/libvips/colour/sRGB2XYZ.c index a00fac73..8e5a8a2c 100644 --- a/libvips/colour/sRGB2XYZ.c +++ b/libvips/colour/sRGB2XYZ.c @@ -64,6 +64,18 @@ vips_sRGB2XYZ_line( VipsColour *colour, VipsPel *out, VipsPel **in, int width ) int i; + printf( "vips_sRGB2XYZ_line: out = %p, in = %p\n", + out, p ); + + int sum; + + printf( "vips_sRGB2XYZ_line: testing mem buffer #6 %p for uninit\n", + p ); + sum = 0; + for( i = 0; i < width; i++ ) + sum += p[0] + p[1] + p[2]; + printf( "sum = %d\n", sum ); + for( i = 0; i < width; i++ ) { int r = p[0]; int g = p[1]; diff --git a/libvips/conversion/sequential.c b/libvips/conversion/sequential.c index d74618e8..0d036a64 100644 --- a/libvips/conversion/sequential.c +++ b/libvips/conversion/sequential.c @@ -142,7 +142,7 @@ vips_sequential_generate( VipsRegion *or, * The stall can be cancelled by a signal on @ready. */ VIPS_DEBUG_MSG( "thread %p stalling for up to %gs ...\n", - STALL_TIME, g_thread_self() ); + g_thread_self(), STALL_TIME ); vips_g_cond_timed_wait( sequential->ready, sequential->lock, STALL_TIME * 1000000 ); VIPS_DEBUG_MSG( "thread %p awake again ...\n", diff --git a/libvips/foreign/foreign.c b/libvips/foreign/foreign.c index b73b1cf6..7b91b3b2 100644 --- a/libvips/foreign/foreign.c +++ b/libvips/foreign/foreign.c @@ -33,8 +33,8 @@ */ /* -#define DEBUG */ +#define DEBUG #ifdef HAVE_CONFIG_H #include @@ -42,6 +42,7 @@ #include #include +#include #include #include @@ -790,6 +791,19 @@ vips_foreign_load_start( VipsImage *out, void *a, void *b ) load->real, NULL ); } + if( load->real->dtype == VIPS_IMAGE_SETBUF ) { + int x, y; + int sum; + + printf( "testing mem buffer #3 %p for uninit\n", + VIPS_IMAGE_ADDR( load->real, 0, 0 ) ); + sum = 0; + for( y = 0; y < load->real->Ysize; y++ ) + for( x = 0; x < load->real->Xsize; x++ ) + sum += *VIPS_IMAGE_ADDR( load->real, x, y ); + printf( "sum = %d\n", sum ); + } + return( vips_region_new( load->real ) ); } @@ -813,6 +827,22 @@ vips_foreign_load_generate( VipsRegion *or, if( vips_region_region( or, ir, r, r->left, r->top ) ) return( -1 ); +{ + int x, y; + int sum; + + printf( "vips_foreign_load_generate: testing mem buffer #4 %p for uninit\n", + VIPS_REGION_ADDR( or, r->left, r->top ) ); + sum = 0; + for( y = 0; y < r->height; y++ ) + for( x = 0; x < r->width; x++ ) + sum += + VIPS_REGION_ADDR( or, x + r->left, y + r->top )[0] + + VIPS_REGION_ADDR( or, x + r->left, y + r->top )[1] + + VIPS_REGION_ADDR( or, x + r->left, y + r->top )[2]; + printf( "sum = %d\n", sum ); +} + return( 0 ); } diff --git a/libvips/foreign/jpeg2vips.c b/libvips/foreign/jpeg2vips.c index fb12ba49..d3e50b4e 100644 --- a/libvips/foreign/jpeg2vips.c +++ b/libvips/foreign/jpeg2vips.c @@ -842,7 +842,7 @@ read_jpeg_generate( VipsRegion *or, struct jpeg_decompress_struct *cinfo = &jpeg->cinfo; int sz = cinfo->output_width * cinfo->output_components; - JSAMPROW row_pointer[1]; + JSAMPROW row_pointer[8]; int y; #ifdef DEBUG @@ -871,21 +871,18 @@ read_jpeg_generate( VipsRegion *or, if( setjmp( jpeg->eman.jmp ) ) return( -1 ); - for( y = 0; y < r->height; y++ ) { - row_pointer[0] = (JSAMPLE *) + for( y = 0; y < r->height; y++ ) + row_pointer[y] = (JSAMPLE *) VIPS_REGION_ADDR( or, 0, r->top + y ); - /* No faster to read in groups and you have to loop - * anyway. So just read a line at a time. - */ - jpeg_read_scanlines( cinfo, &row_pointer[0], 1 ); + jpeg_read_scanlines( cinfo, &row_pointer[0], r->height ); - if( jpeg->invert_pels ) { - int x; + if( jpeg->invert_pels ) { + int x; + for( y = 0; y < r->height; y++ ) for( x = 0; x < sz; x++ ) - row_pointer[0][x] = 255 - row_pointer[0][x]; - } + row_pointer[y][x] = 255 - row_pointer[y][x]; } return( 0 ); diff --git a/libvips/iofuncs/generate.c b/libvips/iofuncs/generate.c index 65eb10d8..2a8fb7bc 100644 --- a/libvips/iofuncs/generate.c +++ b/libvips/iofuncs/generate.c @@ -586,6 +586,7 @@ write_vips( VipsRegion *region, VipsRect *area, void *a, void *b ) count = region->bpl * area->height; buf = VIPS_REGION_ADDR( region, 0, area->top ); + do { nwritten = write( region->im->fd, buf, count ); if( nwritten == (size_t) -1 ) @@ -699,9 +700,24 @@ vips_image_generate( VipsImage *image, if( image->dtype == VIPS_IMAGE_OPENOUT ) res = vips_sink_disc( image, (VipsRegionWrite) write_vips, NULL ); - else + else { + int x, y; + int sum; + res = vips_sink_memory( image ); + printf( "testing mem buffer %p for uninit\n", + VIPS_IMAGE_ADDR( image, 0, 0 ) ); + sum = 0; + for( y = 1; y < image->Ysize; y++ ) + for( x = 0; x < image->Xsize; x++ ) { + printf( "x = %d\n", + *VIPS_IMAGE_ADDR( image, x, y ) ); + sum += *VIPS_IMAGE_ADDR( image, x, y ); + } + printf( "sum = %d\n", sum ); + } + /* Error? */ if( res ) @@ -722,5 +738,18 @@ vips_image_generate( VipsImage *image, if( vips_image_written( image ) ) return( -1 ); + if( image->dtype == VIPS_IMAGE_SETBUF ) { + int x, y; + int sum; + + printf( "testing mem buffer #2 %p for uninit\n", + VIPS_IMAGE_ADDR( image, 0, 0 ) ); + sum = 0; + for( y = 0; y < image->Ysize; y++ ) + for( x = 0; x < image->Xsize; x++ ) + sum += *VIPS_IMAGE_ADDR( image, x, y ); + printf( "sum = %d\n", sum ); + } + return( 0 ); } diff --git a/libvips/iofuncs/image.c b/libvips/iofuncs/image.c index def5d7f1..bbe065ad 100644 --- a/libvips/iofuncs/image.c +++ b/libvips/iofuncs/image.c @@ -31,8 +31,8 @@ */ /* -#define VIPS_DEBUG */ +#define VIPS_DEBUG #ifdef HAVE_CONFIG_H #include @@ -1896,6 +1896,9 @@ vips_image_write_prepare( VipsImage *image ) VIPS_IMAGE_SIZEOF_IMAGE( image ))) ) return( -1 ); + printf( "vips_image_write_prepare: memory image at %p\n", + image->data ); + break; case VIPS_IMAGE_OPENOUT: diff --git a/libvips/iofuncs/region.c b/libvips/iofuncs/region.c index f192e115..417fe47e 100644 --- a/libvips/iofuncs/region.c +++ b/libvips/iofuncs/region.c @@ -869,8 +869,9 @@ vips_region_paint( VipsRegion *reg, VipsRect *r, int value ) vips_rect_intersectrect( r, ®->valid, &ovl ); if( !vips_rect_isempty( &ovl ) ) { VipsPel *q = VIPS_REGION_ADDR( reg, ovl.left, ovl.top ); - int wd = ovl.width * VIPS_IMAGE_SIZEOF_PEL( reg->im ); - int ls = VIPS_REGION_LSKIP( reg ); + size_t wd = ovl.width * VIPS_IMAGE_SIZEOF_PEL( reg->im ); + size_t ls = VIPS_REGION_LSKIP( reg ); + int y; for( y = 0; y < ovl.height; y++ ) { diff --git a/libvips/iofuncs/sinkdisc.c b/libvips/iofuncs/sinkdisc.c index d15e0cb2..37a3bae5 100644 --- a/libvips/iofuncs/sinkdisc.c +++ b/libvips/iofuncs/sinkdisc.c @@ -353,8 +353,7 @@ wbuffer_allocate_fn( VipsThreadState *state, void *a, gboolean *stop ) /* Position buf at the new y. */ if( wbuffer_position( write->buf, - //sink_base->y, sink_base->nlines ) ) - sink_base->y, sink_base->tile_height ) ) + sink_base->y, sink_base->nlines ) ) return( -1 ); } } @@ -484,8 +483,7 @@ vips_sink_disc( VipsImage *im, VipsRegionWrite write_fn, void *a ) result = 0; if( !write.buf || !write.buf_back || - //wbuffer_position( write.buf, 0, write.sink_base.nlines ) || - wbuffer_position( write.buf, 0, write.sink_base.tile_height ) || + wbuffer_position( write.buf, 0, write.sink_base.nlines ) || vips_threadpool_run( im, write_thread_state_new, wbuffer_allocate_fn, diff --git a/libvips/iofuncs/sinkmemory.c b/libvips/iofuncs/sinkmemory.c index 3b55d690..91c02229 100644 --- a/libvips/iofuncs/sinkmemory.c +++ b/libvips/iofuncs/sinkmemory.c @@ -324,7 +324,7 @@ vips_sink_memory( VipsImage *image ) SinkMemory memory; int result; - VIPS_DEBUG_MSG( "vips_sink_memory2:\n" ); + printf( "vips_sink_memory: writing to %p\n", image->data ); if( sink_memory_init( &memory, image ) ) return( -1 ); @@ -345,7 +345,8 @@ vips_sink_memory( VipsImage *image ) sink_memory_free( &memory ); - VIPS_DEBUG_MSG( "vips_sink_memory2: done\n" ); + printf( "vips_sink_memory: done\n" ); + VIPS_DEBUG_MSG( "vips_sink_memory: done\n" ); return( result ); }