PNG: Fix check for too few axes, memory, orientations

core/file/png.cpp

@@ -38,6 +38,7 @@ namespace MR
 
 
       Reader::Reader (const std::string& filename) :
+          infile (fopen (filename.c_str(), "rb")),
           png_ptr (NULL),
           info_ptr (NULL),
           width (0),
@@ -46,7 +47,6 @@ namespace MR
           color_type (0),
           channels (0)
       {
-        FILE* infile = fopen (filename.c_str(), "rb");
         unsigned char sig[8];
         if (fread (sig, 1, 8, infile) < 8)
           throw Exception ("error reading from PNG file \"" + filename + "\"");
@@ -130,6 +130,10 @@ namespace MR
           png_ptr = NULL;
           info_ptr = NULL;
         }
+        if (infile) {
+          fclose(infile);
+          infile = NULL;
+        }
       }
 
 
@@ -183,6 +187,7 @@ namespace MR
           bit_depth (0),
           filename (filename),
           data_type (H.datatype()),
+          multiplier (1.0),
           outfile (NULL)
       {
         if (Path::exists (filename) && !App::overwrite_files)
@@ -196,6 +201,8 @@ namespace MR
           throw Exception ("Unable to set jump buffer for PNG structure for image \"" + filename + "\"");
         }
         outfile = fopen (filename.c_str(), "wb");
+        if (!outfile)
+          throw Exception ("Unable to open PNG file for writing for image \"" + filename + "\": " + strerror (errno));
         png_init_io (png_ptr, outfile);
         png_set_compression_level (png_ptr, Z_DEFAULT_COMPRESSION);
         switch (H.ndim()) {
@@ -231,17 +238,23 @@ namespace MR
             png_destroy_write_struct (&png_ptr, &info_ptr);
             throw Exception ("Undefined data type in image \"" + H.name() + "\" for PNG writer");
           case DataType::Bit:
-            bit_depth = 1;
+            assert (false);
             break;
           case DataType::UInt8:
+            bit_depth = 8;
+            break;
           case DataType::Float32:
             bit_depth = 8;
+            multiplier = std::numeric_limits<uint8_t>::max(); break;
             break;
           case DataType::UInt16:
           case DataType::UInt32:
           case DataType::UInt64:
+            bit_depth = 16;
+            break;
           case DataType::Float64:
             bit_depth = 16;
+            multiplier = std::numeric_limits<uint16_t>::max(); break;
             break;
         }
         // Detect cases where one axis has a size of 1, and hence represents the image plane
@@ -302,6 +315,10 @@ namespace MR
           png_ptr = NULL;
           info_ptr = NULL;
         }
+        if (outfile) {
+          fclose(outfile);
+          outfile = NULL;
+        }
       }
 
 
@@ -323,10 +340,11 @@ namespace MR
             row_pointers[row] = to_write + row * row_bytes;
           png_write_image (png_ptr, row_pointers);
           png_write_end (png_ptr, info_ptr);
+          delete [] row_pointers;
         };
 
 
-        if (bit_depth == 1 || data_type == DataType::UInt8 || data_type == DataType::UInt16BE) {
+        if (data_type == DataType::UInt8 || data_type == DataType::UInt16BE) {
           finish (data);
         } else {
           uint8_t scratch[row_bytes * height];

core/file/png.h

@@ -54,6 +54,7 @@ namespace MR
           void load (uint8_t*);
 
         private:
+          FILE* infile;
           png_structp png_ptr;
           png_infop info_ptr;
           png_uint_32 width, height;
@@ -83,6 +84,7 @@ namespace MR
           int color_type, bit_depth;
           std::string filename;
           DataType data_type;
+          default_type multiplier;
           FILE* outfile;
 
           static void error_handler (png_struct_def*, const char*);
@@ -100,16 +102,8 @@ namespace MR
         std::function<default_type(const void*,size_t,default_type,default_type)> fetch_func;
         std::function<void(default_type,void*,size_t,default_type,default_type)> store_func;
         __set_fetch_store_functions<default_type> (fetch_func, store_func, data_type);
-        default_type multiplier = 1.0;
-        switch (data_type() & DataType::Type) {
-          case DataType::Float32: multiplier = std::numeric_limits<uint8_t>::max(); break;
-          case DataType::Float64: multiplier = std::numeric_limits<uint16_t>::max(); break;
-        }
-        for (size_t i = 0; i != num_elements; ++i) {
-          Raw::store_BE<T> (std::min (default_type(std::numeric_limits<T>::max()), std::max (0.0, std::round(multiplier * fetch_func (in_ptr, 0, 0.0, 1.0)))), out_ptr);
-          in_ptr += data_type.bytes();
-          out_ptr += sizeof(T);
-        }
+        for (size_t i = 0; i != num_elements; ++i)
+          Raw::store_BE<T> (std::min (default_type(std::numeric_limits<T>::max()), std::max (0.0, std::round(multiplier * fetch_func (in_ptr, i, 0.0, 1.0)))), out_ptr, i);
       };
 
 

core/formats/png.cpp

@@ -70,16 +70,16 @@ namespace MR
       }
 
       H.size(0) = png.get_width();
-      H.stride(0) = -3;
+      H.stride(0) = -2;
 
       H.size(1) = png.get_height();
-      H.stride(1) = -4;
+      H.stride(1) = -3;
 
       H.size(2) = 1;
-      H.stride(2) = 1;
+      H.stride(2) = 4;
 
       if (H.ndim() == 4)
-        H.stride(3) = 2;
+        H.stride(3) = 1;
 
       H.spacing (0) = H.spacing (1) = H.spacing (2) = 1.0;
       H.transform().setIdentity();
@@ -132,14 +132,14 @@ namespace MR
       if (H.ndim() == 4 && H.size(3) > 4)
         throw Exception ("A 4D image written to PNG must have between one and four volumes (requested: " + str(H.size(3)) + ")");
 
-      // TODO After looping over axes via square-bracket notation,
+      // After looping over axes via square-bracket notation,
       //   there needs to be at least two axes with size greater than one
-      size_t unity_axes = 0;
+      size_t nonunity_axes = 0;
       for (size_t axis = 0; axis != H.ndim(); ++axis) {
-        if (H.size (axis) == 1)
-          ++unity_axes;
+        if (H.size (axis) > 1)
+          ++nonunity_axes;
       }
-      if (unity_axes - (H.ndim() - num_axes) < 2)
+      if (nonunity_axes - (H.ndim() - num_axes) < 2)
         throw Exception ("Too few (non-unity) image axes to support PNG export");
 
       // For 4D images, can support:
@@ -149,7 +149,7 @@ namespace MR
       // - 4 volumes (save as RGBA)
       // This needs to be compatible with NameParser used in Header::create():
       //   "num_axes" subtracts from H.ndim() however many instances of [] there are
-      size_t width_axis = 0, axis_to_zero = 3;
+      size_t axis_to_zero = 3;
       if (H.ndim() - num_axes > 1)
         throw Exception ("Cannot nominate more than one axis using square-bracket notation for PNG format");
       switch (num_axes) {
@@ -170,7 +170,6 @@ namespace MR
             axis_to_zero = 1;
           } else if (H.size(0) == 1) {
             axis_to_zero = 0;
-            width_axis = 1;
           } else {
             // If image is 3D, and all three axes have size greater than one, and we
             //   haven't used the square-bracket notation, we can't export genuine 3D data
@@ -192,8 +191,6 @@ namespace MR
           }
           if (axis < 0)
             throw Exception ("Cannot export 4D image to PNG format if all three spatial axes have size greater than 1 and square-bracket notation is not used");
-          if (!axis_to_zero)
-            width_axis = 1;
           break;
         default:
           throw Exception ("Cannot generate PNG file(s) from image with more than 4 axes");
@@ -209,7 +206,7 @@ namespace MR
       H.stride(1) = -3;
       H.spacing(1) = 1.0;
       if (H.ndim() > 2) {
-        H.stride(2) = 4;
+        H.stride(2) = -4;
         H.spacing(2) = 1.0;
       }
       if (H.ndim() > 3) {
@@ -223,9 +220,10 @@ namespace MR
 
       H.transform().setIdentity();
 
-      if (H.datatype() == DataType::Bit && H.size (width_axis) % 8) {
-        WARN ("Cannot write bitwise PNG image with width not a factor of 8; will instead write with 8-bit depth");
+      if (H.datatype() == DataType::Bit) {
+        WARN ("Cannot write bitwise PNG images; will instead write with 8-bit depth");
         H.datatype() = DataType::UInt8;
+        H.intensity_scale() = 1.0 / 255.0;
       }
 
       return true;

core/header.cpp

@@ -785,12 +785,13 @@ namespace MR
     Header result (headers[0]);
 
     if (axis_to_concat >= result.ndim()) {
+      Stride::symbolise (result);
       result.ndim() = axis_to_concat + 1;
       result.size(axis_to_concat) = 1;
+      result.stride(axis_to_concat) = axis_to_concat+1;
+      Stride::actualise (result);
     }
 
-    result.stride (axis_to_concat) = result.ndim()+1;
-
     for (size_t axis = 0; axis != result.ndim(); ++axis) {
       if (axis != axis_to_concat && result.size (axis) <= 1) {
         for (const auto& H : headers) {

core/image_io/png.cpp

@@ -29,17 +29,14 @@ namespace MR
 
     void PNG::load (const Header& header, size_t)
     {
-      segsize = header.datatype().bytes() * voxel_count (header) * files.size();
+      DEBUG (std::string("loading PNG image") + (files.size() > 1 ? "s" : "") + " \"" + header.name() + "\"");
       addresses.resize (1);
-      addresses[0].reset (new uint8_t [segsize]);
+      segsize = (header.datatype().bits() * voxel_count (header) + 7) / 8;
+      addresses[0].reset (new uint8_t[segsize]);
       if (is_new) {
         memset (addresses[0].get(), 0x00, segsize);
-        DEBUG ("allocated memory for PNG image \"" + header.name() + "\"");
       } else {
-        DEBUG (std::string("loading PNG image") + (files.size() > 1 ? "s" : "") + " \"" + header.name() + "\"");
-        size_t slice_bytes = (header.datatype().bits() * header.size(0) * header.size(1) + 7) / 8;
-        if (header.ndim() == 4)
-          slice_bytes *= header.size (3);
+        const size_t slice_bytes = (header.datatype().bits() * header.size(0) * header.size(1) * (header.ndim() == 4 ? header.size(3) : 1) + 7) / 8;
         for (size_t i = 0; i != files.size(); ++i) {
           File::PNG::Reader png (files[i].name);
           if (png.get_width() != header.size(0) ||
@@ -59,19 +56,15 @@ namespace MR
 
     void PNG::unload (const Header& header)
     {
-      if (addresses.size()) {
-        if (writable) {
-          size_t slice_bytes = (header.datatype().bits() * header.size(0) * header.size(1) + 7) / 8;
-          if (header.ndim() == 4)
-            slice_bytes *= header.size (3);
-          for (size_t i = 0; i != files.size(); i++) {
-            File::PNG::Writer png (header, files[i].name);
-            png.save (addresses[0].get() + (i * slice_bytes));
-          }
+      assert (addresses.size() == 1);
+      if (writable) {
+        const size_t slice_bytes = (header.datatype().bits() * header.size(0) * header.size(1) * (header.ndim() == 4 ? header.size(3) : 1) + 7) / 8;
+        for (size_t i = 0; i != files.size(); i++) {
+          File::PNG::Writer png (header, files[i].name);
+          png.save (addresses[0].get() + (i * slice_bytes));
         }
-        DEBUG ("deleting buffer for PNG image \"" + header.name() + "\"...");
-        addresses[0].release();
       }
+      delete[] addresses[0].release();
     }
 
   }

core/image_io/ram.cpp

@@ -36,14 +36,6 @@ namespace MR
     }
 
 
-    void RAM::unload (const Header& header)
-    {
-      if (addresses.size()) {
-        DEBUG ("deleting RAM buffer for image \"" + header.name() + "\"...");
-        delete [] addresses[0];
-      }
-    }
-
   }
 }
 

core/image_io/ram.h

@@ -32,7 +32,7 @@ namespace MR
 
       protected:
         virtual void load (const Header&, size_t);
-        virtual void unload (const Header&);
+        virtual void unload (const Header&) { }
     };
 
   }

core/image_io/scratch.cpp

@@ -38,15 +38,6 @@ namespace MR
       }
     }
 
-
-    void Scratch::unload (const Header& header)
-    {
-      if (addresses.size()) {
-        DEBUG ("deleting scratch buffer for image \"" + header.name() + "\"...");
-        addresses[0].reset();
-      }
-    }
-
   }
 }
 

core/image_io/scratch.h

@@ -34,7 +34,7 @@ namespace MR
 
       protected:
         virtual void load (const Header&, size_t);
-        virtual void unload (const Header&);
+        virtual void unload (const Header&) { }
     };
 
   }

core/image_io/tiff.cpp

@@ -63,15 +63,6 @@ namespace MR
 
     }
 
-
-    void TIFF::unload (const Header& header)
-    {
-      if (addresses.size()) {
-        DEBUG ("deleting buffer for TIFF image \"" + header.name() + "\"...");
-        addresses[0].release();
-      }
-    }
-
   }
 }
 

core/image_io/tiff.h

@@ -30,11 +30,11 @@ namespace MR
     class TIFF : public Base
     { MEMALIGN (TIFF)
       public:
-        TIFF (const Header& header) : Base (header) { } 
+        TIFF (const Header& header) : Base (header) { }
 
       protected:
         virtual void load (const Header&, size_t);
-        virtual void unload (const Header&);
+        virtual void unload (const Header&) { }
     };
 
   }

core/image_io/variable_scaling.cpp

@@ -61,8 +61,6 @@ namespace MR
 
     }
 
-    void VariableScaling::unload (const Header& header) { }
-
   }
 }
 

core/image_io/variable_scaling.h

@@ -44,7 +44,7 @@ namespace MR
 
       protected:
         virtual void load (const Header&, size_t);
-        virtual void unload (const Header&);
+        virtual void unload (const Header&) { }
     };
 
   }