Mercurial > hg > octave-lyh
changeset 16948:ee2166121a28
imwrite: clean up finding of filename and its connection to imformats.
* private/imwrite_filename.m: new private function to find which
argument is which from the args of imwrite(), specially what
is going to be the actual filename.
* imwrite.m: delegate finding of file format to the new private
function imwrite_filename(). No longer makes use of the private
function imageIO() so that we can issue errors and warnings.
* private/core_imwrite.m: delegate finding the order of the initial
input arguments to the new private function imwrite_filename().
* private/imageIO.m: made simpler since imwrite() won't be using it
anymore. It would need some extra checks for imwrite() only
which defeats the purpose of having it common with the other
image IO functions.
author | Carnë Draug <carandraug@octave.org> |
---|---|
date | Thu, 11 Jul 2013 02:07:10 +0100 |
parents | 24bb7dc754ed |
children | 1eb5e5f0ee13 |
files | scripts/image/imwrite.m scripts/image/private/core_imwrite.m scripts/image/private/imageIO.m scripts/image/private/imwrite_filename.m |
diffstat | 4 files changed, 97 insertions(+), 62 deletions(-) [+] |
line wrap: on
line diff
--- a/scripts/image/imwrite.m +++ b/scripts/image/imwrite.m @@ -1,4 +1,5 @@ ## Copyright (C) 2008-2012 John W. Eaton +## Copyright (C) 2013 Carnë Draug ## ## This file is part of Octave. ## @@ -45,25 +46,23 @@ if (nargin < 2) print_usage (); endif + [filename, ext] = imwrite_filename (varargin{2:end}); - ## This input checking is a bit convoluted to support the multiple - ## ways the function can be called. Basically, after the image we - ## can have the filename or a colormap. If we have a colormap, then - ## the filename becomes the third argument. After that, we may have - ## the optional file extension. - if (ischar (varargin{2})) - filename_idx = 2; - elseif (nargin >= 3 && iscolormap (varargin{2}) && ! ischar (varargin{3})) - filename_idx = 3; + fmt = imformats (ext); + ## 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))) + if (isempty (ext)) + error ("imwrite: no extension found for %s to identify the image format", + filename); + endif + warning ("imwrite: unlisted image format %s (see imformats). Trying to save anyway.", + ext); + core_imwrite (varargin{:}); else - error ("imwrite: no FILENAME specified"); - endif - filename = {varargin{filename_idx}}; - if (nargin > filename_idx + 1 && ischar (varargin {filename_idx + 1})) - filename{2} = varargin{filename_idx + 1}; + fmt.write (varargin{:}); endif - imageIO (@core_imwrite, "write", filename, varargin{:}); endfunction %% Test input validation
--- a/scripts/image/private/core_imwrite.m +++ b/scripts/image/private/core_imwrite.m @@ -27,43 +27,15 @@ print_usage ("imwrite"); endif - map = []; - fmt = ""; + [filename, ext, map, param_list] = imwrite_filename (varargin{2:end}); - offset = 1; - if (isnumeric (varargin{1})) - map = varargin{1}; - if (! iscolormap (map)) - error ("imwrite: invalid COLORMAP"); - endif - offset = 2; - endif - if (offset <= length (varargin) && ischar (varargin{offset})) - filename = varargin{offset}; - offset++; - if (rem (length (varargin) - offset, 2) == 0 && ischar (varargin{offset})) - fmt = varargin{offset}; - offset++; - endif + if (isempty (options)) + has_param_list = false; else - print_usage ("imwrite"); - endif - if (offset < length (varargin)) has_param_list = true; - for ii = offset:2:(length (varargin) - 1) - options.(varargin{ii}) = varargin{ii + 1}; + for ii = 1:2:(length (param_list)) + options.(param_list{ii}) = param_list{ii + 1}; endfor - else - has_param_list = false; - endif - - filename = tilde_expand (filename); - - if (isempty (fmt)) - [~, ~, fmt] = fileparts (filename); - if (! isempty (fmt)) - fmt = fmt(2:end); - endif endif if (isempty (img)) @@ -95,9 +67,9 @@ error ("imwrite: %s: invalid class for truecolor image", img_class); endif if (has_param_list) - __magick_write__ (filename, fmt, img, options); + __magick_write__ (filename, ext, img, options); else - __magick_write__ (filename, fmt, img); + __magick_write__ (filename, ext, img); endif else if (any (strcmp (img_class, {"uint8", "uint16", "double"}))) @@ -121,11 +93,11 @@ tmp = uint8 (cat (3, r, g, b) * 255); if (has_param_list) - __magick_write__ (filename, fmt, tmp, options); - ## __magick_write__ (filename, fmt, img, map, options); + __magick_write__ (filename, ext, tmp, options); + ## __magick_write__ (filename, ext, img, map, options); else - __magick_write__ (filename, fmt, tmp); - ## __magick_write__ (filename, fmt, img, map); + __magick_write__ (filename, ext, tmp); + ## __magick_write__ (filename, ext, img, map); endif endif
--- a/scripts/image/private/imageIO.m +++ b/scripts/image/private/imageIO.m @@ -16,8 +16,8 @@ ## along with Octave; see the file COPYING. If not, see ## <http://www.gnu.org/licenses/>. -## This function simply connects the function that calls it to all -## other imageIO functions. It does it by checking the file extension +## This function simply connects imread and imfinfo() to the function +## to be used based on their format. It does it by checking the file extension ## of the file and redirecting to the appropriate function after checking ## with imformats. ## @@ -47,15 +47,11 @@ ## should completely ignore filename{2}. It won't even be used by ## imformats() at all, even if filename{1} has no extension to use with ## imformats(). - ## - ## To further complicate things, when we are going to be writing a - ## file, whether the file exists or not does not matter. - if (isscalar (filename) || (strcmp (fieldname, "write") && - ! isempty (file_in_path (IMAGE_PATH, filename{1})))) + if (isscalar (filename) || ! isempty (file_in_path (IMAGE_PATH, filename{1}))) [~, ~, ext] = fileparts (filename{1}); if (! isempty (ext)) ## remove dot from extension - ext(1) = []; + ext = ext(2:end); endif else ext = filename{2};
new file mode 100644 --- /dev/null +++ b/scripts/image/private/imwrite_filename.m @@ -0,0 +1,68 @@ +## Copyright (C) 2013 Carnë Draug +## +## This file is part of Octave. +## +## Octave is free software; you can redistribute it and/or modify it +## under the terms of the GNU General Public License as published by +## the Free Software Foundation; either version 3 of the License, or (at +## your option) any later version. +## +## Octave is distributed in the hope that it will be useful, but +## WITHOUT ANY WARRANTY; without even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +## General Public License for more details. +## +## You should have received a copy of the GNU General Public License +## along with Octave; see the file COPYING. If not, see +## <http://www.gnu.org/licenses/>. + +## The input check for imwrite needs to be done twice, once when imwrite +## is called the first time to find where the filename is, and a second +## time by core_imwrite after imformats decides what function to use. +## Because a user can, and is encouraged to, get a function handle to +## core_imwrite, the input check is also done there. +## In addition, the input check for imwrite is not that straightforward +## in order to support the multiple ways the function can be called, +## and interpretations of Matlab documentation. +## +## Anyway, this will only do the input check until it finds the filename +## to be used, the only part that imwrite actually needs. + +function [filename, ext, cmap, options] = imwrite_filename (varargin) + + ## First, we check if the first argument is a colormap or a filename. + cmap = []; + if (ischar (varargin{1})) + filename_idx = 1; + elseif (numel (varargin) >= 2 && iscolormap (varargin{1}) && ischar (varargin{2})) + filename_idx = 2; + cmap = varargin{1}; + else + error ("imwrite: no FILENAME specified"); + endif + filename = tilde_expand (varargin{filename_idx}); + + ## Next, we get the file extension. + ## if we have an odd number of leftover arguments, and the next argument + ## is a string, we consider it the file extension. Otherwise we will + ## extract what we can from the previously found filename. + options_idx = filename_idx + 1; + if (numel (varargin) > filename_idx && + rem (length (varargin) - filename_idx, 2) != 0 && + ischar (varargin{filename_idx + 1})) + ext = varargin{filename_idx + 1}; + filename = [filename "." ext]; + options_idx++; + else + [~, ~, ext] = fileparts (filename); + if (! isempty (ext)) + ## remove dot from extension + ext = ext(2:end); + endif + endif + + ## After all the work finding where the filename was, we might as well + ## send the leftovers list (they should be in key value pairs) + options = varargin(options_idx:end); + +endfunction