# HG changeset patch
# User Carnë Draug <carandraug@octave.org>
# Date 1389899556 0
#      Thu Jan 16 19:12:36 2014 +0000
# Node ID f311f25b152e43518423165cfb87fe049780dbca
# Parent  1e3aa3132f836c9bbab12e747fb3244b0d37c697
Fix input parsing for imageIO functions and make imread accept URL (bug #41234)

* image/private/imageIO.m: make the function now check existence of the file
and handle URLs. It may also modify the input that imread() and imfinfo()
got to remove format name/extension if it was given as separate argument. If
the file can't be found it will thrown an error (whatever function is set for
that format, it will never be called if the file can't be found).
* image/imfinfo.m, image/imread.m: fix for new correct call to imageIO().
* private/__imfinfo__.m, private/__imread__.m: remove parsing of input,
checking for existence of file, and handling of URL since that is now done
by imageIO().

diff -r 1e3aa3132f83 -r f311f25b152e scripts/image/imfinfo.m
--- a/scripts/image/imfinfo.m	Thu Jan 16 17:54:59 2014 +0000
+++ b/scripts/image/imfinfo.m	Thu Jan 16 19:12:36 2014 +0000
@@ -18,8 +18,8 @@
 
 ## -*- texinfo -*-
 ## @deftypefn  {Function File} {@var{info} =} imfinfo (@var{filename})
-## @deftypefnx {Function File} {@var{info} =} imfinfo (@var{filename}, @var{ext})
 ## @deftypefnx {Function File} {@var{info} =} imfinfo (@var{url})
+## @deftypefnx {Function File} {@var{info} =} imfinfo (@dots{}, @var{ext})
 ## Read image information from a file.
 ##
 ## @code{imfinfo} returns a structure containing information about the image
@@ -148,14 +148,14 @@
 
 ## Author: Soren Hauberg <hauberg@gmail.com>
 
-function info = imfinfo (varargin)
+function info = imfinfo (filename, varargin)
   if (nargin < 1 || nargin > 2)
     print_usage ();
-  elseif (! ischar (varargin{1}))
+  elseif (! ischar (filename))
     error ("imfinfo: FILENAME must be a string");
-  elseif (nargin > 1 && ! ischar (varargin{2}))
+  elseif (nargin > 1 && ! ischar (ext))
     error ("imfinfo: EXT must be a string");
   endif
-  info = imageIO (@__imfinfo__, "info", varargin, varargin{:});
+  info = imageIO ("imfinfo", @__imfinfo__, "info", filename, varargin{:});
 endfunction
 
diff -r 1e3aa3132f83 -r f311f25b152e scripts/image/imread.m
--- a/scripts/image/imread.m	Thu Jan 16 17:54:59 2014 +0000
+++ b/scripts/image/imread.m	Thu Jan 16 19:12:36 2014 +0000
@@ -23,16 +23,16 @@
 
 ## -*- texinfo -*-
 ## @deftypefn  {Function File} {[@var{img}, @var{map}, @var{alpha}] =} imread (@var{filename})
-## @deftypefnx {Function File} {[@dots{}] =} imread (@var{filename}, @var{ext})
 ## @deftypefnx {Function File} {[@dots{}] =} imread (@var{url})
+## @deftypefnx {Function File} {[@dots{}] =} imread (@dots{}, @var{ext})
 ## @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
 ## no file @var{filename}, and @var{ext} was specified, it will look for
-## a file named @var{filename} and extension @var{ext}, i.e., a file named
-## @var{filename}.@var{ext}.
+## a file with the extension @var{ext}.  Finally, it will attempt to download
+## and read an image from @var{url}.
 ##
 ## The size and class of the output depends on the
 ## format of the image.  A color image is returned as an
@@ -88,23 +88,14 @@
 ## Author: Stefan van der Walt <stefan@sun.ac.za>
 ## Author: Andy Adler
 
-function [img, varargout] = imread (varargin)
+function [img, varargout] = imread (filename, varargin)
   if (nargin < 1)
     print_usage ();
-  elseif (! ischar (varargin{1}))
+  elseif (! ischar (filename))
     error ("imread: FILENAME must be a string");
   endif
-  ## In case the file format was specified as a separate argument we
-  ## do this. imageIO() will ignore the second part if filename on its
-  ## own is enough. And if the second argument was a parameter name instead
-  ## of an extension, it is still going to be passed to the next function
-  ## since we are passing the whole function input as well.
-  filename = {varargin{1}};
-  if (nargin > 1 && ischar (varargin {2}))
-    filename{2} = varargin{2};
-  endif
 
-  [img, varargout{1:nargout-1}] = imageIO (@__imread__, "read", filename, varargin{:});
+  [img, varargout{1:nargout-1}] = imageIO ("imread", @__imread__, "read", filename, varargin{:});
 endfunction
 
 
diff -r 1e3aa3132f83 -r f311f25b152e scripts/image/private/__imfinfo__.m
--- a/scripts/image/private/__imfinfo__.m	Thu Jan 16 17:54:59 2014 +0000
+++ b/scripts/image/private/__imfinfo__.m	Thu Jan 16 19:12:36 2014 +0000
@@ -17,56 +17,22 @@
 ## along with Octave; see the file COPYING.  If not, see
 ## <http://www.gnu.org/licenses/>.
 
-## This function does al the work of imfinfo. It exists here as private
+## This function does all the work of imfinfo. It exists here as private
 ## function so that imfinfo can use other functions if imformats is
 ## configured to. It is also needed so that imformats can create a
 ## function handle for it.
 
 ## Author: Soren Hauberg <hauberg@gmail.com>
 
-function info = __imfinfo__ (filename, ext)
+function info = __imfinfo__ (filename)
 
-  if (nargin < 1 || nargin > 2)
+  if (nargin != 1)
     print_usage ("imfinfo");
+  elseif (! ischar (filename))
+    error ("imfinfo: FILENAME must be a string");
   endif
 
-  if (! ischar (filename))
-    error ("imfinfo: FILENAME must be a string");
-  elseif (nargin >= 2 && ! ischar (ext))
-    error ("imfinfo: EXT must be a string");
-  endif
-  filename = tilde_expand (filename);
-
-  delete_file = false;
-  unwind_protect
-
-    fn = file_in_path (IMAGE_PATH, filename);
-    if (isempty (fn))
-      ## We couldn't find the file so...
-      if (nargin >= 2)
-        ## try adding a possible file extesion
-        filename  = [filename "." ext];
-        fn        = file_in_path (IMAGE_PATH, filename);
-        if (isempty (fn))
-          error ("imfinfo: cannot find file %s", filename);
-        endif
-      else
-        ## try filename as an URL
-        [fn, status, msg] = urlwrite (filename, tmpnam ());
-        if (! status)
-          error ("imfinfo: cannot find or download %s: %s", filename, msg);
-        endif
-        delete_file = true;
-      endif
-    endif
-
-    info = __magick_finfo__ (fn);
-
-  unwind_protect_cleanup
-    if (delete_file)
-      unlink (fn);
-    endif
-  end_unwind_protect
+  info = __magick_finfo__ (filename);
 
 endfunction
 
diff -r 1e3aa3132f83 -r f311f25b152e scripts/image/private/__imread__.m
--- a/scripts/image/private/__imread__.m	Thu Jan 16 17:54:59 2014 +0000
+++ b/scripts/image/private/__imread__.m	Thu Jan 16 19:12:36 2014 +0000
@@ -44,18 +44,6 @@
   ## 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 >= offset + 1 && ischar (varargin{offset}))
-    ## if we can't find the file, check if the next input is the file extension
-    filename  = [filename "." varargin{offset}];
-    fn        = file_in_path (IMAGE_PATH, filename);
-    offset++;
-  endif
-  if (isempty (fn))
-    error ("imread: cannot find %s", filename);
-  endif
-
   ## It is possible for an file with multiple pages to have very different
   ## images on each page. Specifically, they may have different sizes. Because
   ## of this, we need to first find out the index of the images to read so
@@ -92,9 +80,9 @@
 
   ## Use information from the first image to be read to set defaults.
   if (ischar (options.index) && strcmpi (options.index, "all"))
-    info = __magick_ping__ (fn, 1);
+    info = __magick_ping__ (filename, 1);
   else
-    info = __magick_ping__ (fn, options.index(1));
+    info = __magick_ping__ (filename, options.index(1));
   endif
 
   ## Set default for options.
@@ -145,7 +133,7 @@
     endswitch
   endfor
 
-  [varargout{1:nargout}] = __magick_read__ (fn, options);
+  [varargout{1:nargout}] = __magick_read__ (filename, options);
 
 endfunction
 
diff -r 1e3aa3132f83 -r f311f25b152e scripts/image/private/imageIO.m
--- a/scripts/image/private/imageIO.m	Thu Jan 16 17:54:59 2014 +0000
+++ b/scripts/image/private/imageIO.m	Thu Jan 16 19:12:36 2014 +0000
@@ -16,54 +16,111 @@
 ## along with Octave; see the file COPYING.  If not, see
 ## <http://www.gnu.org/licenses/>.
 
-## 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.
+## This function the image input functions imread() and imfinfo() to the
+## functions that will actually be used, based on their format.  See below
+## on the details on how it identifies the format, and to what it defaults.
 ##
-## First argument is a function handle for the default imageIO function (what
-## to use if the file extension for the image file is not listed by imformats).
-## Second argument is the fieldname in the struct returned by imformats with a
-## function handle for the function to use. Third argument is a cell array, its
-## first element the filename, and the second, an optional file extension to
-## add to filename, if filename alone does not exist. All the others are the
-## original input arguments passed to the original imageIO function which will
-## be passed on to the destination function.
+## It will change the input arguments that were passed to imread() and
+## imfinfo().  It will change the filename to provide the absolute filepath
+## for the file, it will extract the possible format name from the rest of
+## the input arguments (in case there was one), and will give an error if
+## the file can't be found.
 ##
-## No input checking whatsoever is performed. That should be performed by the
-## function calling it.
+## Usage:
+##
+## func      - Function name to use on error message.
+## core_func - Function handle for the default function to use if we can't
+##             find the format in imformats.
+## fieldname - Name of the field in the struct returned by imformats that
+##             has the function to use.
+## filename  - Most likely the first input argument from the function that
+##             called this. May be missing the file extension which can be
+##             on varargin.
+## varargin  - Followed by all the OTHER arguments passed to imread and
+##             imfinfo.
 
-function varargout = imageIO (core_func, fieldname, filename, varargin)
+function varargout = imageIO (func, core_func, fieldname, filename, varargin)
 
-  ## It should not be this function job to check if the file exists or not.
-  ## However, we need to know the file extension to use with imformats and
-  ## that means we need to know the actual filename that will be used which
-  ## is dependent on whether a file exists.
-  ##
-  ## If a file named filename{1} exists, then that's it, we will use that
-  ## wether it has an extension or not. If it does not exist and we have
-  ## something in filename{2}, then we will consider it the file extension.
-  ## Note the detail that if we find a file using filename{1} only, then we
-  ## 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().
-  if (isscalar (filename) || ! isempty (file_in_path (IMAGE_PATH, filename{1})))
-    [~, ~, ext] = fileparts (filename{1});
-    if (! isempty (ext))
-      ## remove dot from extension
-      ext = ext(2:end);
-    endif
-  else
-    ext = filename{2};
+  ## First thing: figure out the filename and possibly download it.
+  ## The first attempt is to try the filename and see if it exists.  If it
+  ## does not, we try to add the next argument since the file extension can
+  ## be given as a separate argument.  If we still can't find the file, it
+  ## can be a URL.  Lastly, if we still didn't found a file, try adding the
+  ## extension to the URL
+
+  file_2_delete = false;  # will we have to remove the file in the end?
+  persistent abs_path = @(x) file_in_path (IMAGE_PATH, tilde_expand (x));
+
+  ## Filename was given with file extension
+  fn = abs_path (filename);
+  if (isempty (fn) && ! isempty (varargin))
+    ## Maybe if we add a file extension
+    fn = abs_path ([filename "." varargin{1}]);
   endif
 
-  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)))
-    [varargout{1:nargout}] = core_func (varargin{:});
-  else
-    [varargout{1:nargout}] = fmt.(fieldname) (varargin{:});
+  ## Maybe we have an URL
+  if (isempty (fn))
+    file_2_delete = true; # mark file for deletion
+    [fn, ~] = urlwrite (filename, tmpnam ());
+    ## Maybe the URL is missing the file extension
+    if (isempty (fn) && ! isempty (varargin))
+      [fn, ~] = urlwrite ([filename "." varargin{1}], tmpnam ());
+    endif
+
+    if (isempty (fn))
+      error ("%s: unable to find file %s", func, filename);
+    endif
   endif
+
+  ## unwind_protect block because we may have a file to remove in the end
+  unwind_protect
+
+    ## When guessing the format to use, we first check if the second
+    ## argument is a format defined in imformats.  If so, we remove it
+    ## from the rest of arguments before passing them on.  If not, we
+    ## try to guess the format from the file extension.  Finally, if
+    ## we still don't know the format, we use the Octave core functions
+    ## which is the same for all formats.
+    foo = []; # the function we will use
+
+    ## We check if the call to imformats (ext) worked using
+    ## "isempty (fieldnames (fmt))" because when it fails, the returned
+    ## struct is not considered empty.
+
+    ## try the second input argument
+    if (! isempty (varargin) && ischar (varargin{1}))
+      fmt = imformats (varargin{1});
+      if (! isempty (fieldnames (fmt)))
+        foo = fmt.(fieldname);
+        varargin(1) = []; # remove format name from arguments
+      endif
+    endif
+
+    ## try extension from file name
+    if (isempty (foo))
+      [~, ~, ext] = fileparts (fn);
+      if (! isempty (ext))
+        ## remove dot from extension
+        ext = ext(2:end);
+      endif
+      fmt = imformats (ext);
+      if (! isempty (fieldnames (fmt)))
+        foo = fmt.(fieldname);
+      endif
+    endif
+
+    ## use the core function
+    if (isempty (foo))
+      foo = core_func;
+    endif
+
+    [varargout{1:nargout}] = foo (fn, varargin{:});
+
+  unwind_protect_cleanup
+    if (file_2_delete)
+      unlink (fn);
+    endif
+  end_unwind_protect
+
 endfunction
 
