Merge commit '0411e034e23801672354bda98700491cff8e4f73' into master_t…

…o_dev

Manual merge of the contents of #2713 onto dev, as they were omitted from 9896ccf.
Includes updating the commit hash for the "test_data" repository, which merges the changes to test data implemented as part of #2713 onto the dev branch version of those test data.

Conflicts:
	core/file/png.cpp
	core/file/png.h
	core/formats/png.cpp
	core/header.cpp
	core/image_io/png.cpp
	core/image_io/scratch.cpp
	core/image_io/tiff.cpp
	core/image_io/variable_scaling.cpp
	core/image_io/variable_scaling.h
	cpp/core/image_io/pipe.h
	cpp/core/image_io/png.h
	cpp/core/image_io/ram.cpp
	cpp/core/image_io/scratch.h
	testing/binaries/data
	testing/binaries/tests/mrconvert~0411e034e23801672354bda98700491cff8e4f73

cpp/core/file/png.cpp

@@ -30,8 +30,14 @@
 namespace MR::File::PNG {
 
 Reader::Reader(const std::string &filename)
-    : png_ptr(NULL), info_ptr(NULL), width(0), height(0), bit_depth(0), color_type(0), channels(0) {
-  FILE *infile = fopen(filename.c_str(), "rb");
+    : infile (fopen(filename.c_str(), "rb")),
+      png_ptr(NULL),
+      info_ptr(NULL),
+      width(0),
+      height(0),
+      bit_depth(0),
+      color_type(0),
+      channels(0) {
   unsigned char sig[8];
   if (fread(sig, 1, 8, infile) < 8)
     throw Exception("error reading from PNG file \"" + filename + "\"");
@@ -108,6 +114,10 @@ Reader::~Reader() {
     png_ptr = NULL;
     info_ptr = NULL;
   }
+  if (infile) {
+    fclose(infile);
+    infile = NULL;
+  }
 }
 
 void Reader::set_expand() {
@@ -138,6 +148,7 @@ Writer::Writer(const Header &H, const std::string &filename)
       bit_depth(0),
       filename(filename),
       data_type(H.datatype()),
+      multiplier (1.0),
       outfile(NULL) {
   if (Path::exists(filename) && !App::overwrite_files)
     throw Exception("output file \"" + filename + "\" already exists (use -force option to force overwrite)");
@@ -150,6 +161,9 @@ Writer::Writer(const Header &H, const std::string &filename)
     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()) {
@@ -195,17 +209,21 @@ Writer::Writer(const Header &H, const std::string &filename)
     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>::infinity(); break;
     break;
   case DataType::UInt16:
   case DataType::UInt32:
   case DataType::UInt64:
   case DataType::Float64:
     bit_depth = 16;
+    multiplier = std::numeric_limits<uint16_t>::infinity(); break;
     break;
   }
   // Detect cases where one axis has a size of 1, and hence represents the image plane
@@ -270,6 +288,10 @@ Writer::~Writer() {
     png_ptr = NULL;
     info_ptr = NULL;
   }
+  if (outfile) {
+    fclose(outfile);
+    outfile = NULL;
+  }
 }
 
 void Writer::save(uint8_t *data) {
@@ -287,9 +309,10 @@ void Writer::save(uint8_t *data) {
       row_pointers[row] = to_write + row * row_bytes;
     png_write_image(png_ptr, row_pointers.get());
     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];

cpp/core/file/png.h

@@ -47,6 +47,7 @@ class Reader {
   void load(uint8_t *);
 
 private:
+  FILE *infile;
   png_structp png_ptr;
   png_infop info_ptr;
   png_uint_32 width, height;
@@ -72,6 +73,7 @@ class Writer {
   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 *);
@@ -84,22 +86,10 @@ class Writer {
 template <typename T>
 void Writer::fill(uint8_t *in_ptr, uint8_t *out_ptr, const DataType data_type, const size_t num_elements) {
   auto fetch_func = __set_fetch_function<default_type>(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)))),
-                     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);                                                                          //
 };
 
 } // namespace MR::File::PNG

cpp/core/formats/png.cpp

@@ -63,16 +63,16 @@ std::unique_ptr<ImageIO::Base> PNG::read(Header &H) const {
   }
 
   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();
@@ -122,14 +122,14 @@ bool PNG::check(Header &H, size_t num_axes) const {
     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:
@@ -139,7 +139,7 @@ bool PNG::check(Header &H, size_t num_axes) const {
   // - 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) {
@@ -201,7 +201,7 @@ bool PNG::check(Header &H, size_t num_axes) const {
   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) {
@@ -215,9 +215,10 @@ bool PNG::check(Header &H, size_t num_axes) const {
 
   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;

cpp/core/header.cpp

@@ -726,12 +726,13 @@ concatenate(const std::vector<Header> &headers, const size_t axis_to_concat, con
   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) {

cpp/core/image_io/png.cpp

@@ -25,17 +25,19 @@
 namespace MR::ImageIO {
 
 void PNG::load(const Header &header, size_t) {
+  DEBUG (std::string("loading PNG image") + (files.size() > 1 ? "s" : "") + " \"" + header.name() + "\"");
   segsize = header.datatype().bytes() * voxel_count(header) * files.size();
   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) || png.get_height() != header.size(1) ||
@@ -57,19 +59,15 @@ void PNG::load(const Header &header, size_t) {
 }
 
 void PNG::unload(const Header &header) {
-  if (!addresses.empty()) {
-    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].reset();
   }
+  delete[] addresses[0].release();
 }
 
 } // namespace MR::ImageIO

cpp/core/image_io/ram.cpp

@@ -30,11 +30,4 @@ void RAM::load(const Header &header, size_t) {
   addresses[0].reset(new uint8_t[bytes_per_segment]);
 }
 
-void RAM::unload(const Header &header) {
-  if (!addresses.empty()) {
-    DEBUG("deleting RAM buffer for image \"" + header.name() + "\"...");
-    addresses[0].reset();
-  }
-}
-
 } // namespace MR::ImageIO

cpp/core/image_io/ram.h

@@ -26,7 +26,7 @@ class RAM : public Base {
 
 protected:
   virtual void load(const Header &, size_t);
-  virtual void unload(const Header &);
+  virtual void unload(const Header &) { }
 };
 
 } // namespace MR::ImageIO

cpp/core/image_io/scratch.cpp

@@ -34,11 +34,4 @@ void Scratch::load(const Header &header, size_t buffer_size) {
   }
 }
 
-void Scratch::unload(const Header &header) {
-  if (!addresses.empty()) {
-    DEBUG("deleting scratch buffer for image \"" + header.name() + "\"...");
-    addresses[0].reset();
-  }
-}
-
 } // namespace MR::ImageIO

cpp/core/image_io/scratch.h

@@ -28,7 +28,7 @@ class Scratch : public Base {
 
 protected:
   virtual void load(const Header &, size_t);
-  virtual void unload(const Header &);
+  virtual void unload(const Header &) { }
 };
 
 } // namespace MR::ImageIO

cpp/core/image_io/variable_scaling.cpp

@@ -56,6 +56,4 @@ void VariableScaling::load(const Header &header, size_t) {
   }
 }
 
-void VariableScaling::unload(const Header &header) {}
-
 } // namespace MR::ImageIO

cpp/core/image_io/variable_scaling.h

@@ -38,7 +38,7 @@ class VariableScaling : public Base {
 
 protected:
   virtual void load(const Header &, size_t);
-  virtual void unload(const Header &);
+  virtual void unload(const Header &) { }
 };
 
 } // namespace MR::ImageIO

testing/CMakeLists.txt

@@ -16,7 +16,7 @@ include(ExternalProject)
 ExternalProject_Add(BinariesTestData
     PREFIX ${CMAKE_CURRENT_BINARY_DIR}/binaries_data
     GIT_REPOSITORY ${mrtrix_binaries_data_url}
-    GIT_TAG 2169ebc06040a0b1380017f5f2a11d6380c69922 
+    GIT_TAG 9c5da1483bfb0ec9d592a055e20e4465d828cde9
     GIT_PROGRESS TRUE
     CONFIGURE_COMMAND ""
     BUILD_COMMAND ""
@@ -47,4 +47,4 @@ add_subdirectory(binaries)
 add_subdirectory(lib)
 add_subdirectory(scripts)
 add_subdirectory(tools)
-add_subdirectory(unit_tests)
+add_subdirectory(unit_tests)