- the pnm reader read maxval for ppm/pgm files and then ignored it,

          it's now validated (0 < maxval < 65536) and used to scale
          samples.  Note that binary ppm/pgm files (P6/P5) with maxval >
          255 result in an error, since I didn't want to add new features
          just yet, just get the code that's there working correctly.
          Thanks to Elthek on rhizo for reporting this and help in
          tracking it down.
        - added a bunch of tests for reading pnm files.

Changes

@@ -775,6 +775,14 @@ Revision history for Perl extension Imager.
           built from CVS on Win32)
         - Makefile.PL should now handle INCLUDE or LIB with spaces in them
           correctly on Win32.
+        - the pnm reader read maxval for ppm/pgm files and then ignored it,
+          it's now validated (0 < maxval < 65536) and used to scale
+          samples.  Note that binary ppm/pgm files (P6/P5) with maxval >
+          255 result in an error, since I didn't want to add new features
+          just yet, just get the code that's there working correctly.
+          Thanks to Elthek on rhizo for reporting this and help in 
+          tracking it down.
+        - added a bunch of tests for reading pnm files.
 
 =================================================================
 

MANIFEST

@@ -138,6 +138,12 @@ testimg/expected.gif
 testimg/gimpgrad        A GIMP gradient file
 testimg/junk.ppm
 testimg/loccmap.gif
+testimg/maxval.ppm
+testimg/maxval_0.ppm
+testimg/maxval_256.ppm
+testimg/maxval_4095_asc.ppm
+testimg/maxval_65536.ppm
+testimg/maxval_asc.ppm
 testimg/nocmap.gif
 testimg/palette.png
 testimg/palette_out.png

TODO

@@ -87,6 +87,7 @@ New Features:
    (handles TIFF and GIF)
 
 - write_multi() to save other multi-image types, (handles TIFF and GIF)
+  - pnm binary formats support multiple images per file
 
 - compose channels - build a new image based on channels from several
   images

pnm.c

@@ -229,11 +229,11 @@ i_readpnm_wiol(io_glue *ig, int length) {
   int type;
   int x, y, ch;
   int width, height, maxval, channels, pcount;
+  int rounder;
   char *cp;
   unsigned char *uc;
   mbuf buf;
   i_color val;
-  int mult;
 
   i_clear_error();
 
@@ -331,7 +331,25 @@ i_readpnm_wiol(io_glue *ig, int length) {
       mm_log((1, "i_readpnm: error reading maxval\n"));
       return NULL;
     }
+
+    if (maxval == 0) {
+      i_push_error(0, "maxval is zero - invalid pnm file");
+      mm_log((1, "i_readpnm: maxval is zero, invalid pnm file\n"));
+      return NULL;
+    }
+    else if (maxval > 65535) {
+      i_push_errorf(0, "maxval of %d is over 65535 - invalid pnm file", 
+		    maxval);
+      mm_log((1, "i_readpnm: maxval of %d is over 65535 - invalid pnm file\n"));
+      return NULL;
+    }
+    else if (type >= 4 && maxval > 255) {
+      i_push_errorf(0, "maxval of %d is over 255 - not currently supported by Imager for binary pnm", maxval);
+      mm_log((1, "i_readpnm: maxval of %d is over 255 - not currently supported by Imager for binary pnm\n", maxval));
+      return NULL;
+    }
   } else maxval=1;
+  rounder = maxval / 2;
 
   if (!(cp = gnext(&buf)) || !misspace(*cp)) {
     i_push_error(0, "garbage in header, invalid PNM file");
@@ -350,11 +368,10 @@ i_readpnm_wiol(io_glue *ig, int length) {
   case 1: /* Ascii types */
   case 2:
   case 3:
-    mult = type == 1 ? 255 : 1;
     for(y=0;y<height;y++) for(x=0; x<width; x++) {
       for(ch=0; ch<channels; ch++) {
 	int t;
-	if (gnum(&buf, &t)) val.channel[ch] = t * mult;
+	if (gnum(&buf, &t)) val.channel[ch] = (t * 255 + rounder) / maxval;
 	else {
 	  mm_log((1,"i_readpnm: gnum() returned false in data\n"));
 	  return im;
@@ -385,7 +402,8 @@ i_readpnm_wiol(io_glue *ig, int length) {
   case 6: /* binary ppm */
     for(y=0;y<height;y++) for(x=0; x<width; x++) {
       for(ch=0; ch<channels; ch++) {
-	if ( (uc = (unsigned char*)gnext(&buf)) ) val.channel[ch] = *uc;
+	if ( (uc = (unsigned char*)gnext(&buf)) ) 
+	  val.channel[ch] = (*uc * 255 + rounder) / maxval;
 	else {
 	  mm_log((1,"i_readpnm: gnext() returned false in data\n"));
 	  return im;

quant.c

@@ -671,6 +671,7 @@ makemap_mediancut(i_quantize *quant, i_img **imgs, int count) {
       if (imgs[imgn]->channels > 2) {
         chan_count = 3;
         for (x = 0; x < imgs[imgn]->xsize; ++x) {
+	  printf("bumped entry %d\n", MED_CUT_INDEX(line[x]));
           ++colors[MED_CUT_INDEX(line[x])].count;
         }
       }

t/t104ppm.t

@@ -1,6 +1,9 @@
+#!perl -w
 use Imager ':all';
+require "t/testtools.pl";
+use strict;
 
-print "1..13\n";
+print "1..43\n";
 
 init_log("testout/t104ppm.log",1);
 
@@ -16,7 +19,7 @@ i_arc($img,75,75,30,0,361,$red);
 i_conv($img,[0.1, 0.2, 0.4, 0.2, 0.1]);
 
 my $fh = openimage(">testout/t104.ppm");
-$IO = Imager::io_new_fd(fileno($fh));
+my $IO = Imager::io_new_fd(fileno($fh));
 i_writeppm_wiol($img, $IO) 
   or die "Cannot write testout/t104.ppm\n";
 close($fh);
@@ -25,12 +28,12 @@ print "ok 1\n";
 
 $IO = Imager::io_new_bufchain();
 i_writeppm_wiol($img, $IO) or die "Cannot write to bufchain";
-$data = Imager::io_slurp($IO);
+my $data = Imager::io_slurp($IO);
 print "ok 2\n";
 
 $fh = openimage("testout/t104.ppm");
 $IO = Imager::io_new_fd( fileno($fh) );
-$cmpimg = i_readpnm_wiol($IO,-1) || die "Cannot read testout/t104.ppm\n";
+my $cmpimg = i_readpnm_wiol($IO,-1) || die "Cannot read testout/t104.ppm\n";
 close($fh);
 print "ok 3\n";
 
@@ -52,7 +55,7 @@ i_arc($gimg, 75, 75, 30, 0, 361, $white);
 
 open FH, "> testout/t104_gray.pgm" or die "Cannot create testout/t104_gray.pgm: $!\n";
 binmode FH;
-my $IO = Imager::io_new_fd(fileno(FH));
+$IO = Imager::io_new_fd(fileno(FH));
 i_writeppm_wiol($gimg, $IO) or print "not ";
 print "ok 6\n";
 close FH;
@@ -74,6 +77,100 @@ check_gray(11, Imager::i_get_pixel($ooim->{IMG}, 0, 1), 0);
 check_gray(12, Imager::i_get_pixel($ooim->{IMG}, 1, 0), 0);
 check_gray(13, Imager::i_get_pixel($ooim->{IMG}, 1, 1), 255);
 
+{
+  # https://rt.cpan.org/Ticket/Display.html?id=7465
+  # the pnm reader ignores the maxval that it reads from the pnm file
+  my $maxval = Imager->new;
+  $maxval->read(file=>"testimg/maxval.ppm") or print "not ";
+  print "ok 14 # read testimg/maxval.ppm\n";
+
+  # this image contains three pixels, with each sample from 0 to 63
+  # the pixels are (63, 63, 63), (32, 32, 32) and (31, 31, 0)
+
+  # check basic parameters
+  $maxval->getchannels == 3 or print "not ";
+  print "ok 15 # channel count\n";
+  $maxval->getwidth == 3 or print "not ";
+  print "ok 16 # width\n";
+  $maxval->getheight == 1 or print "not ";
+  print "ok 17 # height\n";
+
+  # check the pixels
+  my ($white, $grey, $green) = $maxval->getpixel('x'=>[0,1,2], 'y'=>[0,0,0])
+    or print "not ";
+  print "ok 18 # fetch pixels\n";
+  check_color(19, $white, 255, 255, 255, "white pixel");
+  check_color(20, $grey,  130, 130, 130, "grey  pixel");
+  check_color(21, $green, 125, 125, 0,   "green pixel");
+
+  # and do the same for ASCII images
+  my $maxval_asc = Imager->new;
+  $maxval_asc->read(file=>"testimg/maxval_asc.ppm") or print "not ";
+  print "ok 22 # read testimg/maxval_asc.ppm\n";
+
+  # this image contains three pixels, with each sample from 0 to 63
+  # the pixels are (63, 63, 63), (32, 32, 32) and (31, 31, 0)
+
+  # check basic parameters
+  $maxval_asc->getchannels == 3 or print "not ";
+  print "ok 23 # channel count\n";
+  $maxval_asc->getwidth == 3 or print "not ";
+  print "ok 24 # width\n";
+  $maxval_asc->getheight == 1 or print "not ";
+  print "ok 25 # height\n";
+
+  # check the pixels
+  my ($white_asc, $grey_asc, $green_asc) = $maxval_asc->getpixel('x'=>[0,1,2], 'y'=>[0,0,0])
+    or print "not ";
+  print "ok 26 # fetch pixels\n";
+  check_color(27, $white_asc, 255, 255, 255, "white asc pixel");
+  check_color(28, $grey_asc,  130, 130, 130, "grey  asc pixel");
+  check_color(29, $green_asc, 125, 125, 0,   "green asc pixel");
+}
+
+{ # previously we didn't validate maxval at all, make sure it's
+  # validated now
+  my $maxval0 = Imager->new;
+  $maxval0->read(file=>'testimg/maxval_0.ppm') and print "not ";
+  print "ok 30 # reading maxval 0 image\n";
+  print "# ", $maxval0->errstr, "\n";
+  $maxval0->errstr =~ /maxval is zero - invalid pnm file/
+    or print "not ";
+  print "ok 31 # error expected from reading maxval_0.ppm\n";
+
+  my $maxval65536 = Imager->new;
+  $maxval65536->read(file=>'testimg/maxval_65536.ppm') and print "not ";
+  print "ok 32 # reading maxval 65536 image\n";
+  print "# ",$maxval65536->errstr, "\n";
+  $maxval65536->errstr =~ /maxval of 65536 is over 65535 - invalid pnm file/
+    or print "not ";
+  print "ok 33 # error expected from reading maxval_65536.ppm\n";
+
+  # maxval of 256 is valid, but Imager can't handle it yet in binary files
+  my $maxval256 = Imager->new;
+  $maxval256->read(file=>'testimg/maxval_256.ppm') and print "not ";
+  print "ok 34 # reading maxval 256 image\n";
+  print "# ",$maxval256->errstr,"\n";
+  $maxval256->errstr =~ /maxval of 256 is over 255 - not currently supported by Imager/
+    or print "not ";
+  print "ok 35 # error expected from reading maxval_256.ppm\n";
+
+  # make sure we handle maxval > 255 for ascii
+  my $maxval4095asc = Imager->new;
+  okn(36, $maxval4095asc->read(file=>'testimg/maxval_4095_asc.ppm'),
+     "read maxval_4095_asc.ppm");
+  okn(37, $maxval4095asc->getchannels == 3, "channels");
+  okn(38, $maxval4095asc->getwidth == 3, "width");
+  okn(39, $maxval4095asc->getheight == 1, "height");
+
+  my ($white, $grey, $green) = $maxval4095asc->getpixel('x'=>[0,1,2], 'y'=>[0,0,0])
+    or print "not ";
+  print "ok 40 # fetch pixels\n";
+  check_color(41, $white, 255, 255, 255, "white 4095 pixel");
+  check_color(42, $grey,  128, 128, 128, "grey  4095 pixel");
+  check_color(43, $green, 127, 127, 0,   "green 4095 pixel");
+}
+
 sub openimage {
   my $fname = shift;
   local(*FH);
@@ -101,3 +198,15 @@ sub check_gray {
     print "not ok $num # $g doesn't match $gray\n";
   }
 }
+
+sub check_color {
+  my ($num, $c, $red, $green, $blue, $note) = @_;
+
+  my ($r, $g, $b) = $c->rgba;
+  if ($r == $red && $g == $green && $b == $blue) {
+    print "ok $num # $note\n";
+  }
+  else {
+    print "not ok $num # ($r, $g, $b) doesn't match ($red, $green, $blue)\n";
+  }
+}

testimg/maxval_4095_asc.ppm

@@ -0,0 +1,4 @@
+P3
+3 1
+4095
+4095 4095 4095  2048 2048 2048  2047 2047 0

testimg/maxval_asc.ppm

@@ -0,0 +1,4 @@
+P3
+3 1
+63
+63 63 63 32 32 32 31 31 0