make write errors for jpegs cause errors at the top level

diff --git a/Changes b/Changes
index c038436d2851d84d6fa0470171118d8c234ce64a..83568e1530d3760f5d47e5d9e51003f050dc7cbe 100644 (file)
--- a/Changes
+++ b/Changes
@@ -508,6 +508,7 @@ Revision history for Perl extension Imager.
           hanging off the left of the character cell
         - the tt driver produces some artifacts when characters
           overlapped
+        - error handling for writing jpeg images
 
 =================================================================
 

diff --git a/Imager.pm b/Imager.pm
index 9c6354d8219e4b527b0b84f1f1d27da3ec26a5f2..dfa6bb70de23a96213aec3ebfd5dfbda37e06e64 100644 (file)
--- a/Imager.pm
+++ b/Imager.pm
@@ -1011,7 +1011,7 @@ sub write {
       $self->{DEBUG} && print "writing a png file\n";
     } elsif ( $input{type} eq 'jpeg' ) {
       if ( !i_writejpeg_wiol($self->{IMG}, $IO, $input{jpegquality})) {
-       $self->{ERRSTR}='unable to write jpeg image'; 
+        $self->{ERRSTR} = $self->_error_as_msg();
        return undef;
       }
       $self->{DEBUG} && print "writing a jpeg file\n";

diff --git a/TODO b/TODO
index 1d49135116b39b22d2d614e228994f2b79471092..82091c61cd2a0632f0834d3ca8f635b3d10bc8d6 100644 (file)
--- a/TODO
+++ b/TODO
@@ -40,8 +40,6 @@ New Features:
 
 - Finish antialiased filled polygon function.
 
-- freetype 2 support
-
 - advanced font layout (spacing, kerning, alignment) (sky)
 
 - font synthesis - synthesize a bold or slanted font from a normal font
@@ -99,6 +97,8 @@ Format specific issues:
 
 - read other format paletted images as paletted images
 
+- have jpeg.c write 2 or 4 channel images as 1 or 3 channels respectively
+
 
 Documentation:
 - Add to the documentation

diff --git a/jpeg.c b/jpeg.c
index 5796f0034fb0d328a4b83f2b25a62ee5a64ceb9e..86fc7cfbed4b6e86a77e3122bf0b509b1a2deaba 100644 (file)
--- a/jpeg.c
+++ b/jpeg.c
@@ -1,3 +1,25 @@
+/*
+=head1 NAME
+
+jpeg.c - implement saving and loading JPEG images
+
+=head1 SYNOPSIS
+
+  io_glue *ig;
+  if (!i_writejpeg_wiol(im, ig, quality)) {
+    .. error ..
+  }
+  im = i_readjpeg_wiol(ig, length, iptc_text, itlength);
+
+=head1 DESCRIPTION
+
+Reads and writes JPEG images
+
+=over
+
+=cut
+*/
+
 #include <stdio.h>
 #include <sys/stat.h>
 #ifndef _MSC_VER
@@ -8,6 +30,8 @@
 #include "iolayer.h"
 #include "image.h"
 #include "jpeglib.h"
+#include "jerror.h"
+#include <errno.h>
 
 #define JPEG_APP13       0xED    /* APP13 marker code */
 #define JPGS 1024
@@ -189,11 +213,12 @@ wiol_empty_output_buffer(j_compress_ptr cinfo) {
   ssize_t nbytes     = JPGS - dest->pub.free_in_buffer;
   */
 
-  mm_log((1,"wiol_emtpy_output_buffer(cinfo 0x%p)\n"));
+  mm_log((1,"wiol_empty_output_buffer(cinfo 0x%p)\n"));
   rc = dest->data->writecb(dest->data, dest->buffer, JPGS);
   
   if (rc != JPGS) { /* XXX: Should raise some jpeg error */
     mm_log((1, "wiol_empty_output_buffer: Error: nbytes = %d != rc = %d\n", JPGS, rc));
+    ERREXIT(cinfo, JERR_FILE_WRITE); 
   }
   dest->pub.free_in_buffer = JPGS;
   dest->pub.next_output_byte = dest->buffer;
@@ -203,11 +228,13 @@ wiol_empty_output_buffer(j_compress_ptr cinfo) {
 static void
 wiol_term_destination (j_compress_ptr cinfo) {
   wiol_dest_ptr dest = (wiol_dest_ptr) cinfo->dest;
-
+  size_t nbytes = JPGS - dest->pub.free_in_buffer;
   /* yes, this needs to flush the buffer */
   /* needs error handling */
-  dest->data->writecb(dest->data, dest->buffer, 
-                     JPGS - dest->pub.free_in_buffer);
+  if (dest->data->writecb(dest->data, dest->buffer, nbytes) != nbytes) {
+    ERREXIT(cinfo, JERR_FILE_WRITE);
+  }
+
 
   mm_log((1, "wiol_term_destination(cinfo %p)\n", cinfo));
   mm_log((1, "wiol_term_destination: dest %p\n", cinfo->dest));
@@ -239,10 +266,6 @@ jpeg_wiol_dest(j_compress_ptr cinfo, io_glue *ig) {
   dest->pub.next_output_byte    = dest->buffer;
 }
 
-
-
-
-
 LOCAL(unsigned int)
 jpeg_getc (j_decompress_ptr cinfo)
 /* Read next byte */
@@ -282,6 +305,8 @@ my_output_message (j_common_ptr cinfo) {
   /* Create the message */
   (*cinfo->err->format_message) (cinfo, buffer);
 
+  i_push_error(0, buffer);
+
   /* Send it to stderr, adding a newline */
   mm_log((1, "%s\n", buffer));
 }
@@ -303,15 +328,16 @@ my_error_exit (j_common_ptr cinfo) {
   /* Always display the message. */
   /* We could postpone this until after returning, if we chose. */
   (*cinfo->err->output_message) (cinfo);
-  
+
   /* Return control to the setjmp point */
   longjmp(myerr->setjmp_buffer, 1);
 }
 
+/*
+=item i_readjpeg_wiol(data, length, iptc_itext, itlength)
 
-
-
-
+=cut
+*/
 i_img*
 i_readjpeg_wiol(io_glue *data, int length, char** iptc_itext, int *itlength) {
   i_img *im;
@@ -323,6 +349,8 @@ i_readjpeg_wiol(io_glue *data, int length, char** iptc_itext, int *itlength) {
 
   mm_log((1,"i_readjpeg_wiol(data 0x%p, length %d,iptc_itext 0x%p)\n", data, iptc_itext));
 
+  i_clear_error();
+
   iptc_text = iptc_itext;
   cinfo.err = jpeg_std_error(&jerr.pub);
   jerr.pub.error_exit     = my_error_exit;
@@ -356,8 +384,11 @@ i_readjpeg_wiol(io_glue *data, int length, char** iptc_itext, int *itlength) {
   return im;
 }
 
+/*
+=item i_writejpeg_wiol(im, ig, qfactor)
 
-
+=cut
+*/
 
 undef_int
 i_writejpeg_wiol(i_img *im, io_glue *ig, int qfactor) {
@@ -366,20 +397,35 @@ i_writejpeg_wiol(i_img *im, io_glue *ig, int qfactor) {
   int quality;
 
   struct jpeg_compress_struct cinfo;
-  struct jpeg_error_mgr jerr;
+  struct my_error_mgr jerr;
 
   JSAMPROW row_pointer[1];     /* pointer to JSAMPLE row[s] */
   int row_stride;              /* physical row width in image buffer */
+  unsigned char * data = NULL;
 
   mm_log((1,"i_writejpeg(im %p, ig %p, qfactor %d)\n", im, ig, qfactor));
   
-  if (!(im->channels==1 || im->channels==3)) { fprintf(stderr,"Unable to write JPEG, improper colorspace.\n"); exit(3); }
-  quality = qfactor;
+  i_clear_error();
 
-  cinfo.err = jpeg_std_error(&jerr);
+  if (!(im->channels==1 || im->channels==3)) { 
+    i_push_error(0, "only 1 or 3 channels images can be saved as JPEG");
+    return 0;
+  }
+  quality = qfactor;
 
+  cinfo.err = jpeg_std_error(&jerr.pub);
+  jerr.pub.error_exit = my_error_exit;
+  jerr.pub.output_message = my_output_message;
+  
   jpeg_create_compress(&cinfo);
 
+  if (setjmp(jerr.setjmp_buffer)) {
+    jpeg_destroy_compress(&cinfo);
+    if (data)
+      myfree(data);
+    return 0;
+  }
+
   io_glue_commit_types(ig);
   jpeg_wiol_dest(&cinfo, ig);
 
@@ -416,7 +462,7 @@ i_writejpeg_wiol(i_img *im, io_glue *ig, int qfactor) {
     }
   }
   else {
-    unsigned char *data = mymalloc(im->xsize * im->channels);
+    data = mymalloc(im->xsize * im->channels);
     if (data) {
       while (cinfo.next_scanline < cinfo.image_height) {
         /* jpeg_write_scanlines expects an array of pointers to scanlines.
@@ -430,8 +476,8 @@ i_writejpeg_wiol(i_img *im, io_glue *ig, int qfactor) {
       }
     }
     else {
-      jpeg_finish_compress(&cinfo);
       jpeg_destroy_compress(&cinfo);
+      i_push_error(0, "out of memory");
       return 0; /* out of memory? */
     }
   }
@@ -444,3 +490,17 @@ i_writejpeg_wiol(i_img *im, io_glue *ig, int qfactor) {
 
   return(1);
 }
+
+/*
+=back
+
+=head1 AUTHOR
+
+Arnar M. Hrafnkelsson, addi@umich.edu
+
+=head1 SEE ALSO
+
+Imager(3)
+
+=cut
+*/

diff --git a/t/t101jpeg.t b/t/t101jpeg.t
index 00c09e6a5bd0a1ad97f0127c048c22cde6774c44..3b557684bca98688bae8eae6bbde55ba5d1ce954 100644 (file)
--- a/t/t101jpeg.t
+++ b/t/t101jpeg.t
@@ -1,6 +1,6 @@
 use Imager qw(:all);
 
-print "1..7\n";
+print "1..8\n";
 
 init_log("testout/t101jpeg.log",1);
 
@@ -18,7 +18,7 @@ i_conv($img,[0.1, 0.2, 0.4, 0.2, 0.1]);
 
 i_has_format("jpeg") && print "# has jpeg\n";
 if (!i_has_format("jpeg")) {
-  for (1..7) {
+  for (1..8) {
     print "ok $_ # skip no jpeg support\n";
   }
 } else {
@@ -57,4 +57,22 @@ if (!i_has_format("jpeg")) {
   print "# OO image difference $diff\n";
   $diff < 10000 or print "not ";
   print "ok 7\n";
+
+  # write failure test
+  open FH, "< testout/t101.jpg" or die "Cannot open testout/t101.jpg: $!";
+  binmode FH;
+  ok(8, !$imoo->write(fd=>fileno(FH), type=>'jpeg'), 'failure handling');
+  close FH;
+  print "# ",$imoo->errstr,"\n";
+}
+
+sub ok {
+  my ($num, $test, $msg) = @_;
+
+  if ($test) {
+    print "ok $num\n";
+  }
+  else {
+    print "not ok $num # $msg\n";
+  }
 }