- correct documentation of default of raw image interleave read
authorTony Cook <tony@develop=help.com>
Thu, 15 Jan 2009 11:43:47 +0000 (11:43 +0000)
committerTony Cook <tony@develop=help.com>
Thu, 15 Jan 2009 11:43:47 +0000 (11:43 +0000)
   parameter
   https://rt.cpan.org/Ticket/Display.html?id=42074

 - add raw_ prefix to raw read parameters, though the original names
   still work.

 - fail the read if an invalid raw_interleave parameter is supplied

 - warn if no interleave or raw_interleave parameter is supplied,
   since the documented default was wrong, and incompatible with the
   write format

 - for reading raw images, if raw_storechannels > raw_datachannels,
   set the extra channels in the image to 0

Changes
Imager.pm
Imager.xs
lib/Imager/Files.pod
raw.c
t/t103raw.t
t/t50basicoo.t

diff --git a/Changes b/Changes
index 854831e..46d28a9 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,5 +1,27 @@
 Imager release history.  Older releases can be found in Changes.old
 
+Imager 0.68 - unreleased
+===========
+
+Bug fixes:
+
+ - correct documentation of default of raw image interleave read
+   parameter
+   https://rt.cpan.org/Ticket/Display.html?id=42074
+
+ - add raw_ prefix to raw read parameters, though the original names
+   still work.
+
+ - fail the read if an invalid raw_interleave parameter is supplied
+
+ - warn if no interleave or raw_interleave parameter is supplied,
+   since the documented default was wrong, and incompatible with the
+   write format
+
+ - for reading raw images, if raw_storechannels > raw_datachannels,
+   set the extra channels in the image to 0
+
+
 Imager 0.67 - 12 Dec 2008
 ===========
 
index 7f0ba60..53df089 100644 (file)
--- a/Imager.pm
+++ b/Imager.pm
@@ -597,6 +597,14 @@ sub _valid_image {
   return;
 }
 
+# returns first defined parameter
+sub _first {
+  for (@_) {
+    return $_ if defined $_;
+  }
+  return undef;
+}
+
 #
 # Methods to be called on objects.
 #
@@ -1423,19 +1431,26 @@ sub read {
   }
 
   if ( $input{'type'} eq 'raw' ) {
-    my %params=(datachannels=>3,storechannels=>3,interleave=>1,%input);
-
-    if ( !($params{xsize} && $params{ysize}) ) {
-      $self->{ERRSTR}='missing xsize or ysize parameter for raw';
+    unless ( $input{xsize} && $input{ysize} ) {
+      $self->_set_error('missing xsize or ysize parameter for raw');
       return undef;
     }
 
+    my $interleave = _first($input{raw_interleave}, $input{interleave});
+    unless (defined $interleave) {
+      my @caller = caller;
+      warn "read(type => 'raw') $caller[2] line $caller[1]: supply interleave or raw_interleave for future compatibility\n";
+      $interleave = 1;
+    }
+    my $data_ch = _first($input{raw_datachannels}, $input{datachannels}, 3);
+    my $store_ch = _first($input{raw_storechannels}, $input{storechannels}, 3);
+
     $self->{IMG} = i_readraw_wiol( $IO,
-                                  $params{xsize},
-                                  $params{ysize},
-                                  $params{datachannels},
-                                  $params{storechannels},
-                                  $params{interleave});
+                                  $input{xsize},
+                                  $input{ysize},
+                                  $data_ch,
+                                  $store_ch,
+                                  $interleave);
     if ( !defined($self->{IMG}) ) {
       $self->{ERRSTR}=$self->_error_as_msg();
       return undef;
index ec861ef..2de3747 100644 (file)
--- a/Imager.xs
+++ b/Imager.xs
@@ -3518,8 +3518,6 @@ DSO_call(handle,func_index,hv)
               if (SvTYPE(hv)!=SVt_PVHV) croak("Imager: Parameter 2 must be a reference to a hash\n");
               DSO_call( (DSO_handle *)handle,func_index,hv);
 
-
-
 SV *
 i_get_pixel(im, x, y)
        Imager::ImgRaw im
index eea79d8..45a68a8 100644 (file)
@@ -1058,13 +1058,62 @@ values for each pixel you could do:
             storechannels=>3)
     or die "Cannot read raw image\n";
 
-Normally the raw image is expected to have the value for channel 1
-immediately following channel 0 and channel 2 immediately following
-channel 1 for each pixel.  If your input image has all the channel 0
-values for the first line of the image, followed by all the channel 1
-values for the first line and so on, you can use the interleave option:
+Read parameters:
 
-  $img->read(file=>'foo.raw', xsize=100, ysize=>100, interleave=>1)
+=over
+
+=item *
+
+raw_interleave - controls the ordering of samples within the image.
+Default: 1.  Alternatively and historically spelled C<interleave>.
+Possible values:
+
+=over
+
+=item *
+
+0 - samples are pixel by pixel, so all samples for the first pixel,
+then all samples for the second pixel and so on.  eg. for a four pixel
+scanline the channels would be laid out as:
+
+  012012012012
+
+=item *
+
+1 - samples are line by line, so channel 0 for the entire scanline is
+followed by channel 1 for the entire scanline and so on.  eg. for a
+four pixel scanline the channels would be laid out as:
+
+  000011112222
+
+This is the default.
+
+=back
+
+Unfortunately, historically, the default C<raw_interleave> for read
+has been 1, while writing only supports the C<raw_interleave> = 0
+format.
+
+For future compatibility, you should always supply the
+C<raw_interleave> (or C<interleave>) parameter.  As of 0.68, Imager
+will warn if you attempt to read a raw image without a
+C<raw_interleave> parameter.
+
+=item *
+
+raw_storechannels - the number of channels to store in the image.
+Range: 1 to 4.  Default: 3.  Alternatively and historically spelled
+C<storechannels>.
+
+=item *
+
+raw_datachannels - the number of channels to read from the file.
+Range: 1 or more.  Default: 3.  Alternatively and historically spelled
+C<datachannels>.
+
+=back
+
+  $img->read(file=>'foo.raw', xsize=100, ysize=>100, raw_interleave=>1)
     or die "Cannot read raw image\n";
 
 =head2 PNG
diff --git a/raw.c b/raw.c
index efdf44a..912aaf9 100644 (file)
--- a/raw.c
+++ b/raw.c
@@ -23,7 +23,7 @@
           intrl: interlace flag,
                        0 = sample interleaving
                        1 = line interleaving
-                       2 = image interleaving
+                       2 = image interleaving (not implemented)
 
 */
 
@@ -41,12 +41,17 @@ interleave(unsigned char *inbuffer,unsigned char *outbuffer,int rowsize,int chan
 static
 void
 expandchannels(unsigned char *inbuffer, unsigned char *outbuffer, 
-              int chunks, int datachannels, int storechannels) {
-  int ch,i;
-  if (inbuffer == outbuffer) return; /* Check if data is already in expanded format */
-  for(ch=0; ch<chunks; ch++) 
-    for (i=0; i<storechannels; i++) 
-      outbuffer[ch*storechannels+i] = inbuffer[ch*datachannels+i];
+              int xsize, int datachannels, int storechannels) {
+  int x, ch;
+  int copy_chans = storechannels > datachannels ? datachannels : storechannels;
+  if (inbuffer == outbuffer)
+    return; /* Check if data is already in expanded format */
+  for(x = 0; x < xsize; x++) {
+    for (ch = 0; ch < copy_chans; ch++) 
+      outbuffer[x*storechannels+ch] = inbuffer[x*datachannels+ch];
+    for (; ch < storechannels; ch++)
+      outbuffer[x*storechannels+ch] = 0;
+  }
 }
 
 i_img *
@@ -65,6 +70,15 @@ i_readraw_wiol(io_glue *ig, int x, int y, int datachannels, int storechannels, i
   io_glue_commit_types(ig);
   mm_log((1, "i_readraw(ig %p,x %d,y %d,datachannels %d,storechannels %d,intrl %d)\n",
          ig, x, y, datachannels, storechannels, intrl));
+
+  if (intrl != 0 && intrl != 1) {
+    i_push_error(0, "raw_interleave must be 0 or 1");
+    return NULL;
+  }
+  if (storechannels < 1 || storechannels > 4) {
+    i_push_error(0, "raw_storechannels must be between 1 and 4");
+    return NULL;
+  }
   
   im = i_img_empty_ch(NULL,x,y,storechannels);
   if (!im)
index 0aa53a9..5f55514 100644 (file)
@@ -1,9 +1,12 @@
 #!perl -w
 use strict;
-use Test::More tests => 25;
+use Test::More tests => 47;
 use Imager qw(:all);
+use Imager::Test qw/is_color3 is_color4/;
 init_log("testout/t103raw.log",1);
 
+$| = 1;
+
 my $green=i_color_new(0,255,0,255);
 my $blue=i_color_new(0,0,255,255);
 my $red=i_color_new(255,0,0,255);
@@ -167,12 +170,12 @@ SKIP:
   close RAW;
 
   # should get an error reading an empty file
-  ok(!$im->read(file => 'testout/t103_empty.raw', xsize => 50, ysize=>50, type=>'raw'),
+  ok(!$im->read(file => 'testout/t103_empty.raw', xsize => 50, ysize=>50, type=>'raw', interleave => 1),
      'read an empty file');
   is($im->errstr, 'premature end of file', "check message");
   open RAW, "> testout/t103_empty.raw"
     or die "Cannot create testout/t103_empty.raw: $!";
-  ok(!$im->read(fh => \*RAW, , xsize => 50, ysize=>50, type=>'raw'),
+  ok(!$im->read(fh => \*RAW, , xsize => 50, ysize=>50, type=>'raw', interleave => 1),
      'read a file open for write');
   cmp_ok($im->errstr, '=~', '^error reading file: read\(\) failure', "check message");
   
@@ -184,6 +187,90 @@ SKIP:
   ok(grep($_ eq 'raw', Imager->write_types), "check raw in write types");
 }
 
+
+{ # OO no interleave warning
+  my $im = Imager->new;
+  my $msg;
+  local $SIG{__WARN__} = sub { $msg = "@_" };
+  ok($im->read(file => "testout/t103_line_int.raw", xsize => 4, ysize => 4,
+              type => "raw"),
+     "read without interleave parameter")
+    or print "# ", $im->errstr, "\n";
+  ok($msg, "should have warned");
+  like($msg, qr/interleave/, "check warning is ok");
+  # check we got the right value
+  is_color3($im->getpixel(x => 0, y => 0), 0x00, 0x11, 0x22,
+           "check the image was read correctly");
+
+  # check no warning if either is supplied
+  $im = Imager->new;
+  undef $msg;
+  ok($im->read(file => "testout/t103_base.raw", xsize => 4, ysize => 4, type => "raw", interleave => 0), 
+     "read with interleave 0");
+  is($msg, undef, "no warning");
+  is_color3($im->getpixel(x => 0, y => 0), 0x00, 0x11, 0x22,
+           "check read non-interleave");
+
+  $im = Imager->new;
+  undef $msg;
+  ok($im->read(file => "testout/t103_base.raw", xsize => 4, ysize => 4, type => "raw", raw_interleave => 0), 
+     "read with raw_interleave 0");
+  is($msg, undef, "no warning");
+  is_color3($im->getpixel(x => 1, y => 0), 0x01, 0x12, 0x23,
+           "check read non-interleave");
+
+  # make sure set to 1 is sane
+  $im = Imager->new;
+  undef $msg;
+  ok($im->read(file => "testout/t103_line_int.raw", xsize => 4, ysize => 4, type => "raw", raw_interleave => 1), 
+     "read with raw_interleave 1");
+  is($msg, undef, "no warning");
+  is_color3($im->getpixel(x => 2, y => 0), 0x02, 0x13, 0x24,
+           "check read interleave = 1");
+}
+
+{ # invalid interleave error handling
+  my $im = Imager->new;
+  ok(!$im->read(file => "testout/t103_base.raw", raw_interleave => 2, type => "raw", xsize => 4, ysize => 4),
+     "invalid interleave");
+  is($im->errstr, "raw_interleave must be 0 or 1", "check message");
+}
+
+{ # store/data channel behaviour
+  my $im = Imager->new;
+  ok($im->read(file => "testout/t103_3to4.raw", xsize => 4, ysize => 4, 
+              raw_datachannels => 4, raw_interleave => 0, type => "raw"),
+     "read 4 channel file as 3 channels")
+    or print "# ", $im->errstr, "\n";
+  is_color3($im->getpixel(x => 2, y => 1), 0x12, 0x23, 0x34,
+           "check read correctly");
+}
+
+{ # should fail to read with storechannels > 4
+  my $im = Imager->new;
+  ok(!$im->read(file => "testout/t103_line_int.raw", type => "raw",
+               raw_interleave => 1, xsize => 4, ysize => 4,
+               raw_storechannels => 5),
+     "read with large storechannels");
+  is($im->errstr, "raw_storechannels must be between 1 and 4", 
+     "check error message");
+}
+
+{ # should zero spare channels if storechannels > datachannels
+  my $im = Imager->new;
+  ok($im->read(file => "testout/t103_base.raw", type => "raw",
+               raw_interleave => 0, xsize => 4, ysize => 4,
+               raw_storechannels => 4),
+     "read with storechannels > datachannels");
+  is($im->getchannels, 4, "should have 4 channels");
+  is_color4($im->getpixel(x => 2, y => 1), 0x12, 0x23, 0x34, 0x00,
+           "check last channel zeroed");
+}
+
+unlink(qw(testout/t103_base.raw testout/t103_3to4.raw
+          testout/t103_line_int.raw testout/t103_img_int.raw))
+  unless $ENV{IMAGER_KEEP_FILES};
+
 sub read_test {
   my ($in, $xsize, $ysize, $data, $store, $intrl, $base) = @_;
   open FH, $in or die "Cannot open $in: $!";
index ad45eaa..a53bb83 100644 (file)
@@ -42,7 +42,7 @@ my $img = Imager->new();
 my %files;
 @files{@types} = ({ file => "testout/t101.jpg"  },
                  { file => "testout/t102.png"  },
-                 { file => "testout/t103.raw", xsize=>150, ysize=>150, type=>'raw'},
+                 { file => "testout/t103.raw", xsize=>150, ysize=>150, type=>'raw', interleave => 0},
                  { file => "testout/t104.ppm"  },
                  { file => "testout/t105.gif"  },
                  { file => "testout/t106.tiff" },