Mercurial > octave-antonio
changeset 16984:997efb8d0b19
imread: implement options Index, Frames, and Info.
* imread.m: write documentation about options of param/key style.
* private/core_imread.m: write parsing of options in the param/key style.
Implement the option for Index, Frames, and Info. Also write parsing
for PixelRegion but comment out the code until it's implemented in
__magick_read__(). Add comments about, and deprecate, Octave's native
image format which is completely undocumented. Remove tests since they
are duplicated from the ones in imread.m.
* __magick_read__.cc: move input check to the m file calling it. Rewrite
parsing of options which are now in a non-optional struct (default values
are now set on the m function).
* private/imageIO.m: fix call to other functions with varargout with
multiple output values.
| author | Carnë Draug <carandraug@octave.org> |
|---|---|
| date | Sat, 13 Jul 2013 12:41:59 +0100 |
| parents | 4660d047955e |
| children | c9346014fed2 |
| files | libinterp/dldfcn/__magick_read__.cc scripts/image/imread.m scripts/image/private/core_imread.m scripts/image/private/imageIO.m |
| diffstat | 4 files changed, 194 insertions(+), 104 deletions(-) [+] |
line wrap: on
line diff
--- a/libinterp/dldfcn/__magick_read__.cc Sat Jul 13 01:33:17 2013 +0100 +++ b/libinterp/dldfcn/__magick_read__.cc Sat Jul 13 12:41:59 2013 +0100 @@ -1,5 +1,6 @@ /* +Copyright (C) 2013 Carnë Draug Copyright (C) 2002-2012 Andy Adler Copyright (C) 2008 Thomas L. Scofield Copyright (C) 2010 David Grundberg @@ -49,16 +50,11 @@ { octave_value_list output; - int rows = imvec[0].baseRows (); - int columns = imvec[0].baseColumns (); - int nframes = frameidx.length (); + const int rows = imvec[0].baseRows (); + const int columns = imvec[0].baseColumns (); + const int nframes = frameidx.length (); - dim_vector idim = dim_vector (); - idim.resize (4); - idim(0) = rows; - idim(1) = columns; - idim(2) = 1; - idim(3) = nframes; + const dim_vector idim = dim_vector (rows, columns, 1, nframes); Array<int> idx (dim_vector (4, 1)); @@ -200,16 +196,11 @@ T im; - int rows = imvec[0].baseRows (); - int columns = imvec[0].baseColumns (); - int nframes = frameidx.length (); + const int rows = imvec[0].baseRows (); + const int columns = imvec[0].baseColumns (); + const int nframes = frameidx.length (); - dim_vector idim = dim_vector (); - idim.resize (4); - idim(0) = rows; - idim(1) = columns; - idim(2) = 1; - idim(3) = nframes; + dim_vector idim = dim_vector (rows, columns, 1, nframes); Magick::ImageType type = imvec[0].type (); const int divisor = ((uint64_t (1) << QuantumDepth) - 1) / @@ -217,8 +208,8 @@ switch (type) { - case Magick::BilevelType: - case Magick::GrayscaleType: + case Magick::BilevelType: // Monochrome bi-level image + case Magick::GrayscaleType: // Grayscale image { im = T (idim); P *vec = im.fortran_vec (); @@ -246,7 +237,7 @@ } break; - case Magick::GrayscaleMatteType: + case Magick::GrayscaleMatteType: // Grayscale image with opacity { idim(2) = 2; im = T (idim); @@ -279,8 +270,8 @@ } break; - case Magick::PaletteType: - case Magick::TrueColorType: + case Magick::PaletteType: // Indexed color (palette) image + case Magick::TrueColorType: // Truecolor image { idim(2) = 3; im = T (idim); @@ -317,9 +308,9 @@ } break; - case Magick::PaletteMatteType: - case Magick::TrueColorMatteType: - case Magick::ColorSeparationType: + case Magick::PaletteMatteType: // Indexed color (palette) image with opacity + case Magick::TrueColorMatteType: // Truecolor image with opacity + case Magick::ColorSeparationType: // Cyan/Yellow/Magenta/Black (CYMK) image { idim(2) = 4; im = T (idim); @@ -361,7 +352,7 @@ break; default: - error ("__magick_read__: undefined ImageMagick image type"); + error ("__magick_read__: undefined Magick++ image type"); return retval; } @@ -399,12 +390,13 @@ DEFUN_DLD (__magick_read__, args, nargout, "-*- texinfo -*-\n\ -@deftypefn {Loadable Function} {@var{m} =} __magick_read__ (@var{fname}, @var{index})\n\ -@deftypefnx {Loadable Function} {[@var{m}, @var{colormap}] =} __magick_read__ (@var{fname}, @var{index})\n\ -@deftypefnx {Loadable Function} {[@var{m}, @var{colormap}, @var{alpha}] =} __magick_read__ (@var{fname}, @var{index})\n\ -Read images with ImageMagick++. In general you should not be using this\n\ -function. Instead use @code{imread}.\n\ -@seealso{imread}\n\ +@deftypefn {Loadable Function} {[@var{img}, @var{map}, @var{alpha}] =} __magick_read__ (@var{fname}, @var{options})\n\ +Read image with GraphicsMagick or ImageMagick.\n\ +\n\ +This is a private internal function not intended for direct use. Instead\n\ +use @code{imread}.\n\ +\n\ +@seealso{imfinfo, imformats, imread, imwrite}\n\ @end deftypefn") { octave_value_list output; @@ -415,34 +407,19 @@ maybe_initialize_magick (); - if (args.length () > 3 || args.length () < 1 || ! args(0).is_string () - || nargout > 3) + if (args.length () != 2 || ! args(0).is_string ()) { print_usage (); return output; } - Array<int> frameidx; - bool all_frames = false; - - if (args.length () == 2 && args(1).is_real_type ()) - frameidx = args(1).int_vector_value (); - else if (args.length () == 3 && args(1).is_string () - && args(1).string_value () == "frames") + octave_map options = args(1).map_value (); + if (error_state) { - if (args(2).is_string () && args(2).string_value () == "all") - all_frames = true; - else if (args(2).is_real_type ()) - frameidx = args(2).int_vector_value (); - } - else - { - frameidx = Array<int> (dim_vector (1, 1)); - frameidx(0) = 1; + error ("__magick_read__: OPTIONS must be a struct"); } std::vector<Magick::Image> imvec; - try { // Read a file into vector of image objects @@ -454,6 +431,8 @@ } catch (Magick::ErrorCoder& e) { + // FIXME: there's a WarningCoder and ErrorCoder. Shouldn't this + // exception cause an error? warning ("Magick++ coder error: %s", e.what ()); } catch (Magick::Exception& e) @@ -462,22 +441,34 @@ return output; } - int nframes = imvec.size (); - if (all_frames) + const int nframes = imvec.size (); + Array<int> frameidx; + + octave_value indexes = options.getfield ("index")(0); + if (indexes.is_string () && indexes.string_value () == "all") { frameidx = Array<int> (dim_vector (1, nframes)); - for (int i = 0; i < frameidx.length (); i++) - frameidx(i) = i; + for (int i = 0; i < nframes; i++) + { + frameidx(i) = i; + } } else { - for (int i = 0; i < frameidx.length (); i++) + frameidx = indexes.int_vector_value (); + if (error_state) { - frameidx(i) = frameidx(i) - 1; - - if (frameidx(i) >= nframes || frameidx(i) < 0) + error ("__magick_read__: invalid value for Index/Frame"); + } + // Fix indexex from base 1 to base 0, and at the same time, make + // sure none of the indexes is outside the range of image number. + int n = frameidx.nelem (); + for (int i = 0; i < n; i++) + { + frameidx(i)--; + if (frameidx(i) < 0 || frameidx(i) > nframes - 1) { - error ("__magick_read__: invalid INDEX vector"); + error ("imread: index/frames specified are outside the number of images"); return output; } } @@ -485,8 +476,14 @@ Magick::ClassType klass = imvec[0].classType (); + // PseudoClass: + // Image is composed of pixels which specify an index in a color palette. if (klass == Magick::PseudoClass && nargout > 1) - output = read_indexed_images (imvec, frameidx, (nargout == 3)); + { + output = read_indexed_images (imvec, frameidx, (nargout == 3)); + } + // If not PseudoClass then it must be DirectClass: Image is composed of + // pixels which represent literal color values. else { unsigned int depth = imvec[0].modulusDepth (); @@ -867,9 +864,12 @@ "-*- texinfo -*-\n\ @deftypefn {Loadable Function} {} __magick_write__ (@var{fname}, @var{fmt}, @var{img})\n\ @deftypefnx {Loadable Function} {} __magick_write__ (@var{fname}, @var{fmt}, @var{img}, @var{map})\n\ -Write images with ImageMagick++. In general you should not be using this\n\ -function. Instead use @code{imwrite}.\n\ -@seealso{imread}\n\ +Write image with GraphicsMagick or ImageMagick.\n\ +\n\ +This is a private internal function not intended for direct use. Instead\n\ +use @code{imwrite}.\n\ +\n\ +@seealso{imfinfo, imformats, imread, imwrite}\n\ @end deftypefn") { octave_value_list retval; @@ -1001,9 +1001,12 @@ DEFUN_DLD (__magick_finfo__, args, , "-*- texinfo -*-\n\ @deftypefn {Loadable Function} {} __magick_finfo__ (@var{fname})\n\ -Read image information with GraphicsMagick++. In general you should\n\ -not be using this function. Instead use @code{imfinfo}.\n\ -@seealso{imfinfo, imread}\n\ +Read image information with GraphicsMagick or ImageMagick.\n\ +\n\ +This is a private internal function not intended for direct use. Instead\n\ +use @code{imfinfo}.\n\ +\n\ +@seealso{imfinfo, imformats, imread, imwrite}\n\ @end deftypefn") { octave_value retval; @@ -1145,6 +1148,8 @@ "-*- texinfo -*-\n\ @deftypefn {Loadable Function} {} __magick_imformats__ (@var{formats})\n\ Fill formats info with GraphicsMagick CoderInfo.\n\ +\n\ +@seealso{imfinfo, imformats, imread, imwrite}\n\ @end deftypefn") { octave_value retval;
--- a/scripts/image/imread.m Sat Jul 13 01:33:17 2013 +0100 +++ b/scripts/image/imread.m Sat Jul 13 12:41:59 2013 +0100 @@ -1,3 +1,4 @@ +## Copyright (C) 2013 Carnë Draug ## Copyright (C) 2008-2012 Thomas L. Scofield ## Copyright (C) 2008 Kristian Rumberg ## Copyright (C) 2006 Thomas Weber @@ -25,6 +26,7 @@ ## @deftypefnx {Function File} {[@dots{}] =} imread (@var{filename}, @var{ext}) ## @deftypefnx {Function File} {[@dots{}] =} imread (@var{url}) ## @deftypefnx {Function File} {[@dots{}] =} imread (@dots{}, @var{idx}) +## @deftypefnx {Function File} {[@dots{}] =} imread (@dots{}, @var{param1}, @var{val1}, @dots{}) ## Read images from various file formats. ## ## Reads an image as a matrix from the file @var{filename}. If there is @@ -47,9 +49,25 @@ ## specifying the index of the images to read. By default, Octave ## will only read the first page. ## +## Depending on the file format, it is possible to configure the reading +## of images with @var{param}, @var{val} pairs. The following options +## are supported: +## +## @table @asis +## @item Frames or Index +## This is an alternative method to specify @var{idx}. When specifying it +## in this way, its value can also be the string "all". +## +## @item Info +## This option exists for @sc{Matlab} compatibility and has no effect. For +## maximum performance while reading multiple images from a single file, +## use the Index option. +## @end table +## ## @seealso{imwrite, imfinfo, imformats} ## @end deftypefn +## Author: Carnë Draug <carandraug@octave.org> ## Author: Thomas L. Scofield <scofield@calvin.edu> ## Author: Kristian Rumberg <kristianrumberg@gmail.com> ## Author: Thomas Weber <thomas.weber.mail@gmail.com> @@ -72,7 +90,7 @@ filename{2} = varargin{2}; endif - varargout{1:nargout} = imageIO (@core_imread, "read", filename, varargin{:}); + [varargout{1:nargout}] = imageIO (@core_imread, "read", filename, varargin{:}); endfunction %!testif HAVE_MAGICK
--- a/scripts/image/private/core_imread.m Sat Jul 13 01:33:17 2013 +0100 +++ b/scripts/image/private/core_imread.m Sat Jul 13 12:41:59 2013 +0100 @@ -41,32 +41,112 @@ error ("imread: FILENAME must be a string"); endif + ## keep track of the varargin offset we're looking at each moment + offset = 1; + filename = tilde_expand (filename); fn = file_in_path (IMAGE_PATH, filename); - if (isempty (fn) && nargin >= 2 && ischar (varargin{1})) + if (isempty (fn) && nargin >= offset + 1 && ischar (varargin{offset})) ## if we can't find the file, check if the next input is the file extension - filename = [filename "." varargin{1}]; + filename = [filename "." varargin{offset}]; fn = file_in_path (IMAGE_PATH, filename); + offset++; endif if (isempty (fn)) error ("imread: cannot find %s", filename); endif + ## set default for options + options = struct ("index", 1); + + ## Index is the only option that can be defined without the parameter/value + ## pair style. When defining it here, the string "all" is invalid though. + if (nargin >= offset + 1 && ! ischar (varargin{offset})) + if (! is_valid_index_option (options.index)) + error ("imread: IDX must be a numeric vector"); + endif + options.index = varargin{offset}; + offset++; + endif + + if (rem (numel (varargin) - offset + 1, 2) != 0) + error ("imread: no pair for all arguments (even number left)"); + endif + + for idx = offset:2:(numel (varargin) - offset + 1) + + switch (tolower (varargin{idx})) + + case {"frames", "index"}, + options.index = varargin{idx+1}; + if (! (is_valid_index_option (options.index)) && + ! (ischar (options.index) && strcmpi (options.index, "all"))) + error ("imread: value for %s must be a vector or the string `all'"); + endif + +## FIXME: commented until it's implemented in __magick_read__ +## case "pixelregion", +## options.region = varargin{idx+1}; +## if (! iscell (options.region) || numel (options.region) != 2) +## error ("imread: value for %s must be a 2 element cell array", +## varargin{idx}); +## endif +## for reg_idx = 1:2 +## if (numel (options.region{reg_idx}) == 3) +## ## do nothing +## elseif (numel (options.region{reg_idx}) == 2) +## options.region{reg_idx}(3) = options.region{reg_idx}(2); +## options.region{reg_idx}(2) = 1; +## else +## error ("imread: range for %s must be a 2 or 3 element vector", +## varargin{idx}); +## endif +## options.region{reg_idx} = floor (options.region{reg_idx}(1)): ... +## floor (options.region{reg_idx}(2)): ... +## floor (options.region{reg_idx}(3)); +## endfor + + case "info", + ## We ignore this option. This parameter exists in Matlab to + ## speed up the reading of multipage TIFF. It makes no difference + ## for us since we're already quite efficient. + + otherwise + error ("imread: invalid PARAMETER `%s'", varargin{idx}); + + endswitch + endfor + try - [varargout{1:nargout}] = __magick_read__ (fn, varargin{:}); + [varargout{1:nargout}] = __magick_read__ (fn, options); + catch + ## If we can't read it with Magick, maybe the image is in Octave's + ## native image format. This is from back before Octave had 'imread' + ## and 'imwrite'. Then we had the functions 'loadimage' and 'saveimage'. + ## + ## This "image format" seems to be any file that can be read with + ## load() and contains 2 variables. The variable named "map" is a + ## colormap and must exist whether the image is indexed or not. The + ## other variable must be named "img" or "X" for a "normal" or + ## indexed image. + ## + ## FIXME: this has been deprecated for the next major release (3.8 or 4.0). + ## If someone wants to revive this as yet another image format, a + ## separate Octave package can be written for it, that register the + ## format through imformats. magick_error = lasterr (); img_field = false; - x_field = false; + x_field = false; map_field = false; try vars = load (fn); if (isstruct (vars)) img_field = isfield (vars, "img"); - x_field = isfield (vars, "X"); + x_field = isfield (vars, "X"); map_field = isfield (vars, "map"); endif catch @@ -80,6 +160,11 @@ else varargout{1} = vars.X; endif + persistent warned = false; + if (! warned) + warning ("Octave's native image format has been deprecated."); + warned = true; + endif else error ("imread: invalid Octave image file format"); endif @@ -88,31 +173,13 @@ endfunction - -%!testif HAVE_MAGICK -%! vpng = [ ... -%! 137, 80, 78, 71, 13, 10, 26, 10, 0, 0, ... -%! 0, 13, 73, 72, 68, 82, 0, 0, 0, 3, ... -%! 0, 0, 0, 3, 8, 2, 0, 0, 0, 217, ... -%! 74, 34, 232, 0, 0, 0, 1, 115, 82, 71, ... -%! 66, 0, 174, 206, 28, 233, 0, 0, 0, 4, ... -%! 103, 65, 77, 65, 0, 0, 177, 143, 11, 252, ... -%! 97, 5, 0, 0, 0, 32, 99, 72, 82, 77, ... -%! 0, 0, 122, 38, 0, 0, 128, 132, 0, 0, ... -%! 250, 0, 0, 0, 128, 232, 0, 0, 117, 48, ... -%! 0, 0, 234, 96, 0, 0, 58, 152, 0, 0, ... -%! 23, 112, 156, 186, 81, 60, 0, 0, 0, 25, ... -%! 73, 68, 65, 84, 24, 87, 99, 96, 96, 96, ... -%! 248, 255, 255, 63, 144, 4, 81, 111, 101, 84, ... -%! 16, 28, 160, 16, 0, 197, 214, 13, 34, 74, ... -%! 117, 213, 17, 0, 0, 0, 0, 73, 69, 78, ... -%! 68, 174, 66, 96, 130]; -%! fid = fopen ("test.png", "wb"); -%! fwrite (fid, vpng); -%! fclose (fid); -%! A = imread ("test.png"); -%! delete ("test.png"); -%! assert (A(:,:,1), uint8 ([0, 255, 0; 255, 237, 255; 0, 255, 0])); -%! assert (A(:,:,2), uint8 ([0, 255, 0; 255, 28, 255; 0, 255, 0])); -%! assert (A(:,:,3), uint8 ([0, 255, 0; 255, 36, 255; 0, 255, 0])); - +## Tests if the value passed to the Index or Frames is valid. This option +## can be defined in two places, but only in one place can it also be the +## string "all" +function bool = is_valid_index_option (arg) + ## is the index option + bool = false; + if (isvector (arg) && isnumeric (arg) && isreal (arg)) + bool = true; + endif +endfunction
--- a/scripts/image/private/imageIO.m Sat Jul 13 01:33:17 2013 +0100 +++ b/scripts/image/private/imageIO.m Sat Jul 13 12:41:59 2013 +0100 @@ -61,8 +61,8 @@ ## When there is no match, fmt will be a 1x1 structure with no fields, ## so we can't just use `isempty (fmt)'. if (isempty (fieldnames (fmt))) - varargout{1:nargout} = core_func (varargin{:}); + [varargout{1:nargout}] = core_func (varargin{:}); else - varargout{1:nargout} = fmt.(fieldname) (varargin{:}); + [varargout{1:nargout}] = fmt.(fieldname) (varargin{:}); endif endfunction