From: Tony Cook Date: Thu, 15 Jan 2009 11:43:47 +0000 (+0000) Subject: - correct documentation of default of raw image interleave read X-Git-Tag: Imager-0.71~39 X-Git-Url: http://git.imager.perl.org/imager.git/commitdiff_plain/500888dae6ac345f223bc472079aa5ab9550fdd5 - 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 --- diff --git a/Changes b/Changes index 854831e0..46d28a94 100644 --- 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 =========== diff --git a/Imager.pm b/Imager.pm index 7f0ba60e..53df0895 100644 --- 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; diff --git a/Imager.xs b/Imager.xs index ec861ef9..2de3747f 100644 --- 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 diff --git a/lib/Imager/Files.pod b/lib/Imager/Files.pod index eea79d82..45a68a85 100644 --- a/lib/Imager/Files.pod +++ b/lib/Imager/Files.pod @@ -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. +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 for read +has been 1, while writing only supports the C = 0 +format. + +For future compatibility, you should always supply the +C (or C) parameter. As of 0.68, Imager +will warn if you attempt to read a raw image without a +C parameter. + +=item * + +raw_storechannels - the number of channels to store in the image. +Range: 1 to 4. Default: 3. Alternatively and historically spelled +C. + +=item * + +raw_datachannels - the number of channels to read from the file. +Range: 1 or more. Default: 3. Alternatively and historically spelled +C. + +=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 efdf44ae..912aaf90 100644 --- 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 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) diff --git a/t/t103raw.t b/t/t103raw.t index 0aa53a94..5f55514c 100644 --- a/t/t103raw.t +++ b/t/t103raw.t @@ -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: $!"; diff --git a/t/t50basicoo.t b/t/t50basicoo.t index ad45eaa9..a53bb83e 100644 --- a/t/t50basicoo.t +++ b/t/t50basicoo.t @@ -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" },