changeset 17240:d757c98636d8

Use first image to be read rather than first image in file to set defaults. * __magick_read__.cc (read_indexed_images, read_images, __magick_read__): use the index for the first image that will be actually read to infer the bitdepth and type of image, instead of the first image in the file. * private/__imread__.m: separate finding the image index from the rest of all other options since we need to know it before calling imfinfo which will be used to set defaults.
author Carnë Draug <carandraug@octave.org>
date Wed, 14 Aug 2013 01:54:34 +0100
parents d6467d6dfb83
children 03a666018e0f
files libinterp/dldfcn/__magick_read__.cc scripts/image/private/__imread__.m
diffstat 2 files changed, 61 insertions(+), 36 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/dldfcn/__magick_read__.cc	Tue Aug 13 18:41:41 2013 +0100
+++ b/libinterp/dldfcn/__magick_read__.cc	Wed Aug 14 01:54:34 2013 +0100
@@ -91,6 +91,11 @@
   const octave_idx_type nRows = region["row_out"];
   const octave_idx_type nCols = region["col_out"];
 
+  // imvec has all of the pages of a file, even the ones we are not
+  // interested in. We will use the first image that we will be actually
+  // reading to get information about the image.
+  const octave_idx_type def_elem = frameidx(0);
+
   T img       = T (dim_vector (nRows, nCols, 1, nFrames));
   P* img_fvec = img.fortran_vec ();
 
@@ -128,7 +133,7 @@
   // Do we need to get the colormap to interpret the image and alpha channel?
   if (nargout > 1)
     {
-      const octave_idx_type mapsize = imvec[0].colorMapSize ();
+      const octave_idx_type mapsize = imvec[def_elem].colorMapSize ();
       Matrix cmap                   = Matrix (mapsize, 3);
 
       // In theory, it should be possible for each frame of an image to
@@ -136,12 +141,12 @@
       // return the colormap of the first frame.
 
       // only get alpha channel if it exists and was requested as output
-      if (imvec[0].matte () && nargout >= 3)
+      if (imvec[def_elem].matte () && nargout >= 3)
         {
           Matrix amap = Matrix (mapsize, 1);
           for (octave_idx_type i = 0; i < mapsize; i++)
             {
-              const Magick::ColorRGB c = imvec[0].colorMap (i);
+              const Magick::ColorRGB c = imvec[def_elem].colorMap (i);
               cmap(i,0) = c.red   ();
               cmap(i,1) = c.green ();
               cmap(i,2) = c.blue  ();
@@ -168,7 +173,7 @@
         {
           for (octave_idx_type i = 0; i < mapsize; i++)
             {
-              const Magick::ColorRGB c = imvec[0].colorMap (i);
+              const Magick::ColorRGB c = imvec[def_elem].colorMap (i);
               cmap(i,0) = c.red   ();
               cmap(i,1) = c.green ();
               cmap(i,2) = c.blue  ();
@@ -202,6 +207,11 @@
   const octave_idx_type nCols = region["col_out"];
   T img;
 
+  // imvec has all of the pages of a file, even the ones we are not
+  // interested in. We will use the first image that we will be actually
+  // reading to get information about the image.
+  const octave_idx_type def_elem = frameidx(0);
+
   const octave_idx_type row_start  = region["row_start"];
   const octave_idx_type col_start  = region["col_start"];
   const octave_idx_type row_shift  = region["row_shift"];
@@ -228,10 +238,10 @@
   // TODO in the next release of GraphicsMagick, MaxRGB should be replaced
   //      with QuantumRange since MaxRGB is already deprecated in ImageMagick.
   double divisor;
-  if (imvec[0].depth () == 32)
+  if (imvec[def_elem].depth () == 32)
     divisor = std::numeric_limits<uint32_t>::max ();
   else
-    divisor = MaxRGB / ((uint64_t (1) << imvec[0].depth ()) - 1);
+    divisor = MaxRGB / ((uint64_t (1) << imvec[def_elem].depth ()) - 1);
 
   // FIXME: this workaround should probably be fixed in GM by creating a
   //        new ImageType BilevelMatteType
@@ -240,8 +250,8 @@
   // black (1 color), and have a second channel set for transparency (2nd
   // color). Its type will be bilevel since there is no BilevelMatte. The
   // only way to check for this seems to be by checking matte ().
-  Magick::ImageType type = imvec[0].type ();
-  if (type == Magick::BilevelType && imvec[0].matte ())
+  Magick::ImageType type = imvec[def_elem].type ();
+  if (type == Magick::BilevelType && imvec[def_elem].matte ())
     type = Magick::GrayscaleMatteType;
 
   // FIXME: ImageType is the type being used to represent the image in memory
@@ -658,8 +668,8 @@
         }
     }
 
-  const Magick::ClassType klass = imvec[0].classType ();
-  const octave_idx_type depth   = imvec[0].depth ();
+  const Magick::ClassType klass = imvec[frameidx(0)].classType ();
+  const octave_idx_type depth   = imvec[frameidx(0)].depth ();
 
   // Magick::ClassType
   // PseudoClass:
--- a/scripts/image/private/__imread__.m	Tue Aug 13 18:41:41 2013 +0100
+++ b/scripts/image/private/__imread__.m	Wed Aug 14 01:54:34 2013 +0100
@@ -56,37 +56,50 @@
     error ("imread: cannot find %s", filename);
   endif
 
-  try
-    info = imfinfo (fn)(1);
-    ## set default for options
-    options = struct ("index",       1,
-                      "region", {{1:1:info.Height 1:1:info.Width}});
+  ## 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
+  ## we can set up defaults for things such as PixelRegion later on.
+  options = struct ("index", 1);  # default image index
+
+  ## 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.
+  ## Also, for matlab compatibility, if index is defined both as an option here
+  ## and parameter/value pair, silently ignore the first.
+  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
 
-    ## 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++;
+  if (rem (numel (varargin) - offset + 1, 2) != 0)
+    error ("imread: no pair for all arguments (odd number left over)");
+  endif
+
+  ## Check key/value options.
+  indexes = find (cellfun (@(x) ischar (x) ...
+                                && any (strcmpi (x, {"frames", "index"})),
+                           varargin));
+  if (indexes)
+    options.index = varargin{indexes+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
+  endif
 
-    if (rem (numel (varargin) - offset + 1, 2) != 0)
-      error ("imread: no pair for all arguments (even number left)");
-    endif
+  try
+    ## Use information from the first image to be read to set defaults.
+    info = imfinfo (fn)(options.index(1));
+
+    ## Set default for options.
+    options.region = {1:1:info.Height 1:1:info.Width};
 
     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
-
         case "pixelregion",
           options.region = varargin{idx+1};
           if (! iscell (options.region) || numel (options.region) != 2)
@@ -115,8 +128,10 @@
 
         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.
+          ## speed up the reading of multipage TIFF by passing a structure
+          ## that contains information about the start on the file of each
+          ## page.  We can't control it through GraphicsMagic but at least
+          ## we allow to load multiple pages with one command.
 
         otherwise
           error ("imread: invalid PARAMETER `%s'", varargin{idx});