Permalink
Browse files

- 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
  • Loading branch information...
1 parent c3be83f commit 500888dae6ac345f223bc472079aa5ab9550fdd5 Tony Cook committed Jan 15, 2009
Showing with 213 additions and 28 deletions.
  1. +22 −0 Changes
  2. +24 −9 Imager.pm
  3. +0 −2 Imager.xs
  4. +55 −6 lib/Imager/Files.pod
  5. +21 −7 raw.c
  6. +90 −3 t/t103raw.t
  7. +1 −1 t/t50basicoo.t
View
22 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
===========
View
@@ -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;
View
@@ -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
View
@@ -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
View
28 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)
View
@@ -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: $!";
View
@@ -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" },

0 comments on commit 500888d

Please sign in to comment.