From 6edf57eed9c6a9844c6d465e3bc631545d1edd15 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 17 Jul 2010 15:57:22 +0000 Subject: [PATCH] fixes from Tim Elliott --- ChangeLog | 3 +++ libvips/format/im_vips2jpeg.c | 23 ++++++++++++++++------- libvips/iofuncs/sinkdisc.c | 9 ++++++++- libvips/iofuncs/threadpool.c | 10 ++++++++-- 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index cfae5002..1d82fa61 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,9 @@ necessary - oops vipsthumbnail sharpening was turning off for integer shrinks, thanks Nicolas +- im_vips2jpeg() could fail for very small images (thanks Tim) +- threadpool wasn't stopping on allocate errors (thanks Tim) +- vips_sink_disc() could block if allocate failed (thanks Tim) 12/5/10 started 7.22.1 - fix a problem with tiff pyramid write and >1cpu, thanks Ruven diff --git a/libvips/format/im_vips2jpeg.c b/libvips/format/im_vips2jpeg.c index 9c8f58b7..33ef98bc 100644 --- a/libvips/format/im_vips2jpeg.c +++ b/libvips/format/im_vips2jpeg.c @@ -31,6 +31,10 @@ * - allow "none" for profile, meaning don't embed one * 4/2/10 * - gtkdoc + * 17/7/10 + * - use g_assert() + * - allow space for the header in init_destination(), helps writing very + * small JPEGs (thanks Tim Elliott) */ /* @@ -106,7 +110,6 @@ im_vips2mimejpeg( IMAGE *in, int qfac ) #include #include #include -#include #ifdef HAVE_EXIF #ifdef UNTAGGED_EXIF @@ -442,7 +445,7 @@ write_profile_data (j_compress_ptr cinfo, unsigned int length; /* number of bytes to write in this marker */ /* rounding up will fail for length == 0 */ - assert( icc_data_len > 0 ); + g_assert( icc_data_len > 0 ); /* Calculate the number of markers we'll need, rounding up of course */ num_markers = (icc_data_len + MAX_DATA_BYTES_IN_MARKER - 1) / @@ -563,9 +566,9 @@ write_vips( Write *write, int qfac, const char *profile ) /* Should have been converted for save. */ - assert( in->BandFmt == IM_BANDFMT_UCHAR ); - assert( in->Coding == IM_CODING_NONE ); - assert( in->Bands == 1 || in->Bands == 3 || in->Bands == 4 ); + g_assert( in->BandFmt == IM_BANDFMT_UCHAR ); + g_assert( in->Coding == IM_CODING_NONE ); + g_assert( in->Bands == 1 || in->Bands == 3 || in->Bands == 4 ); /* Check input image. */ @@ -794,8 +797,14 @@ METHODDEF(void) init_destination( j_compress_ptr cinfo ) { OutputBuffer *buf = (OutputBuffer *) cinfo->dest; - int mx = cinfo->image_width * cinfo->image_height * - cinfo->input_components * sizeof( JOCTET ); + + /* Allocate an extra 1,000 bytes for header overhead on small images. + * + * TODO: use empty_output_buffer() instead in order to accomodate + * headers larger than 1,000 bytes. + */ + int mx = cinfo->image_width * cinfo->image_height * + cinfo->input_components * sizeof( JOCTET ) + 1000; /* Allocate relative to the image we are writing .. freed when we junk * this output. diff --git a/libvips/iofuncs/sinkdisc.c b/libvips/iofuncs/sinkdisc.c index 304be2cc..d2c032d5 100644 --- a/libvips/iofuncs/sinkdisc.c +++ b/libvips/iofuncs/sinkdisc.c @@ -5,6 +5,8 @@ * - move on top of VipsThreadpool, instead of im_threadgroup_t * 23/6/10 * - better buffer handling for single-line images + * 17/7/10 + * - we could get stuck if allocate failed (thanks Tim) */ /* @@ -529,8 +531,13 @@ vips_sink_disc( VipsImage *im, VipsRegionWrite write_fn, void *a ) * We can't just free the buffers (which will wait for the bg threads * to finish), since the bg thread might see the kill before it gets a * chance to write. + * + * If the pool exited with an error, write.buf might not have been + * started (if the allocate failed), and in any case, we don't care if + * the final write went through or not. */ - im_semaphore_down( &write.buf->done ); + if( !result ) + im_semaphore_down( &write.buf->done ); im__end_eval( im ); diff --git a/libvips/iofuncs/threadpool.c b/libvips/iofuncs/threadpool.c index ed6ff5b7..c090b925 100644 --- a/libvips/iofuncs/threadpool.c +++ b/libvips/iofuncs/threadpool.c @@ -10,6 +10,9 @@ * 11/5/10 * - argh, stopping many threads could sometimes leave allocated work * undone + * 17/7/10 + * - set pool->error whenever we set thr->error, lets us catch allocate + * errors (thanks Tim) */ /* @@ -348,7 +351,7 @@ typedef struct _VipsThreadpool { /*< private >*/ VipsImage *im; /* Image we are calculating */ - /* STart a thread, do a unit of work (runs in parallel) and allocate + /* Start a thread, do a unit of work (runs in parallel) and allocate * a unit of work (serial). Plus the mutex we use to serialize work * allocation. */ @@ -507,6 +510,7 @@ vips_thread_work_unit( VipsThread *thr ) if( vips_thread_allocate( thr ) ) { thr->error = TRUE; + pool->error = TRUE; g_mutex_unlock( pool->allocate_lock ); return; } @@ -522,8 +526,10 @@ vips_thread_work_unit( VipsThread *thr ) /* Process a work unit. */ - if( vips_thread_work( thr ) ) + if( vips_thread_work( thr ) ) { thr->error = TRUE; + pool->error = TRUE; + } } #ifdef HAVE_THREADS