From de6d0c37a339799240c9f3c8543925023eefb6f2 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Mon, 11 Sep 2023 15:47:20 +0200 Subject: [PATCH 1/8] PNG: Fix check for too few axes Fixes erroneous check put in place in #2535 (3acf6cc). --- core/formats/png.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/formats/png.cpp b/core/formats/png.cpp index 43313e219a..e04e2dbb21 100644 --- a/core/formats/png.cpp +++ b/core/formats/png.cpp @@ -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: From 76d7007ab1d4715600106ca49dda02bf3f879715 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 27 Sep 2023 23:48:51 +1000 Subject: [PATCH 2/8] PNG: Multiple changes - Fix failure to return dynamically allocated memory (two separate instances). - Fix orientation of exported PNGs in some use cases. - Fix excessive memory allocation for images spread across multiple PNG files. - When reading from multiple input PNG images, allocate memory for read buffers on a per-slice basis. --- core/file/png.cpp | 1 + core/formats/png.cpp | 2 +- core/image_io/png.cpp | 39 +++++++++++++++++++-------------------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/core/file/png.cpp b/core/file/png.cpp index 8785f7f19e..5acb3d4b41 100644 --- a/core/file/png.cpp +++ b/core/file/png.cpp @@ -323,6 +323,7 @@ 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; }; diff --git a/core/formats/png.cpp b/core/formats/png.cpp index e04e2dbb21..3e5b2654a9 100644 --- a/core/formats/png.cpp +++ b/core/formats/png.cpp @@ -209,7 +209,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) { diff --git a/core/image_io/png.cpp b/core/image_io/png.cpp index d295d2112b..d0bf383096 100644 --- a/core/image_io/png.cpp +++ b/core/image_io/png.cpp @@ -29,17 +29,15 @@ namespace MR void PNG::load (const Header& header, size_t) { - segsize = header.datatype().bytes() * voxel_count (header) * files.size(); - addresses.resize (1); - addresses[0].reset (new uint8_t [segsize]); + DEBUG (std::string("loading PNG image") + (files.size() > 1 ? "s" : "") + " \"" + header.name() + "\""); if (is_new) { + segsize = (header.datatype().bits() * voxel_count (header) + 7) / 8; + addresses.resize (1); + addresses[0].reset (new uint8_t[segsize]); 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); + segsize = (header.datatype().bits() * header.size(0) * header.size(1) * (header.ndim() == 4 ? header.size(3) : 1) + 7) / 8; + addresses.resize (files.size()); for (size_t i = 0; i != files.size(); ++i) { File::PNG::Reader png (files[i].name); if (png.get_width() != header.size(0) || @@ -51,7 +49,8 @@ namespace MR e.push_back ("File \"" + files[i].name + ": " + str(png.get_width()) + "x" + str(png.get_height()) + " x " + str(png.get_bitdepth()) + "(->" + str(png.get_output_bitdepth()) + ") bits, " + str(png.get_channels()) + " channels"); throw e; } - png.load (addresses[0].get() + (i * slice_bytes)); + addresses[i].reset (new uint8_t[segsize]); + png.load (addresses[i].get()); } } } @@ -59,18 +58,18 @@ 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)); - } + if (writable) { + assert (addresses.size() == 1); + 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(); + } else { + assert (files.size() == addresses.size()); + for (size_t i = 0; i != files.size(); i++) + delete[] addresses[i].release(); } } From 386db1f78ecfda2fbab9da6dc79dc0cf9b03315d Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 4 Oct 2023 13:02:22 +1100 Subject: [PATCH 3/8] Multiple changes to PNG handling - Do not attempt to support writing bitwise images, even if the number of pixels in the width direction is a factor of 8. - Fix intensity scaling of bitwise images such that values of 0 and 255 are used in the output uint8 image. Add test data and tests for verifying operation of PNG export. --- core/file/png.cpp | 11 +++++++++-- core/file/png.h | 13 +++---------- core/formats/png.cpp | 14 ++++++++------ testing/binaries/data | 2 +- testing/binaries/tests/mrconvert | 7 ++++++- 5 files changed, 27 insertions(+), 20 deletions(-) diff --git a/core/file/png.cpp b/core/file/png.cpp index 5acb3d4b41..8033f4326c 100644 --- a/core/file/png.cpp +++ b/core/file/png.cpp @@ -183,6 +183,7 @@ namespace MR bit_depth (0), filename (filename), data_type (H.datatype()), + multiplier (1.0), outfile (NULL) { if (Path::exists (filename) && !App::overwrite_files) @@ -231,17 +232,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::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::max(); break; break; } // Detect cases where one axis has a size of 1, and hence represents the image plane @@ -327,7 +334,7 @@ namespace MR }; - 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]; diff --git a/core/file/png.h b/core/file/png.h index 7a1d6b59ec..c0a5e65e69 100644 --- a/core/file/png.h +++ b/core/file/png.h @@ -83,6 +83,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 +101,8 @@ namespace MR std::function fetch_func; std::function store_func; __set_fetch_store_functions (fetch_func, store_func, data_type); - default_type multiplier = 1.0; - switch (data_type() & DataType::Type) { - case DataType::Float32: multiplier = std::numeric_limits::max(); break; - case DataType::Float64: multiplier = std::numeric_limits::max(); break; - } - for (size_t i = 0; i != num_elements; ++i) { - Raw::store_BE (std::min (default_type(std::numeric_limits::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 (std::min (default_type(std::numeric_limits::max()), std::max (0.0, std::round(multiplier * fetch_func (in_ptr, i, 0.0, 1.0)))), out_ptr, i); }; diff --git a/core/formats/png.cpp b/core/formats/png.cpp index 3e5b2654a9..54e74c42e7 100644 --- a/core/formats/png.cpp +++ b/core/formats/png.cpp @@ -149,7 +149,8 @@ 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 width_axis = 0; + 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 +171,7 @@ namespace MR axis_to_zero = 1; } else if (H.size(0) == 1) { axis_to_zero = 0; - width_axis = 1; + //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 +193,8 @@ 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; + // if (!axis_to_zero) + // width_axis = 1; break; default: throw Exception ("Cannot generate PNG file(s) from image with more than 4 axes"); @@ -223,9 +224,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; diff --git a/testing/binaries/data b/testing/binaries/data index e5646b6b5a..68869a7fe0 160000 --- a/testing/binaries/data +++ b/testing/binaries/data @@ -1 +1 @@ -Subproject commit e5646b6b5a5311df287610c1b0ea4e13388d6740 +Subproject commit 68869a7fe0780c7b1e9577ba68466d5b407a3fe8 diff --git a/testing/binaries/tests/mrconvert b/testing/binaries/tests/mrconvert index 8d3f6d2f96..b17770093e 100644 --- a/testing/binaries/tests/mrconvert +++ b/testing/binaries/tests/mrconvert @@ -16,4 +16,9 @@ mrconvert mrcat/voxel[].mih - | testing_diff_header -keyval - mrcat/all_axis0.mi mrconvert mrcat/all_axis3.mif tmp-[].mif -force && testing_diff_header -keyval tmp-0.mif mrcat/voxel1.mih && testing_diff_header -keyval tmp-1.mif mrcat/voxel2.mih && testing_diff_header -keyval tmp-2.mif mrcat/voxel3.mih && testing_diff_header -keyval tmp-3.mif mrcat/voxel4.mih && testing_diff_header -keyval tmp-4.mif mrcat/voxel5.mih && testing_diff_header -keyval tmp-5.mif mrcat/voxel6.mih mrconvert dwi.mif tmp-[]-[].mif -force && testing_diff_image dwi.mif tmp-[]-[].mif mrconvert dwi.mif -coord 3 1:2:end -axes 0:2,-1,3 - | testing_diff_image - mrconvert/dwi_select_axes.mif - +mrinfo template.mif.gz -transform > tmp.txt && mrconvert template.mif.gz tmp[].png -force && mrconvert tmp[].png -vox 2.5 - | mrtransform - -replace tmp.txt - | mrcalc - 255 -div - | testing_diff_image - $(mrcalc template.mif.gz 1.0 -min -) -abs 0.002 +rm -f tmpaxial*.png && mrconvert template.mif.gz -coord 2 9,19,29,39,49 tmpaxial[].png && testing_diff_image tmpaxial[].png mrconvert/axial[].png +rm -f tmpcoronal*.png && mrconvert template.mif.gz -coord 1 17,32,47,62,77 -axes 0,2,1 tmpcoronal[].png && testing_diff_image tmpcoronal[].png mrconvert/coronal[].png +rm -f tmpsagittal*.png && mrconvert template.mif.gz -coord 0 27,47,67 -axes 1,2,0 tmpsagittal[].png && testing_diff_image tmpsagittal[].png mrconvert/sagittal[].png +rm -f tmpmask*.png && mrconvert mask.mif tmpmask[].png && testing_diff_image tmpmask[].png mrconvert/mask[].png +rm -f tmptissues*.png && mrconvert dwi2fod/msmt/tissues.mif tmptissues[].png && testing_diff_image tmptissues[].png mrconvert/tissues[].png From 99572da78f918a54fa6caf2c185782c6ff774814 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Thu, 5 Oct 2023 14:25:33 +1100 Subject: [PATCH 4/8] PNG read: Revert to single memory block Partial regression of 76d7007ab1d4715600106ca49dda02bf3f879715. --- core/image_io/png.cpp | 20 +++++++------------- testing/binaries/tests/mrconvert | 2 +- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/core/image_io/png.cpp b/core/image_io/png.cpp index d0bf383096..25cbcf4ef5 100644 --- a/core/image_io/png.cpp +++ b/core/image_io/png.cpp @@ -30,14 +30,13 @@ namespace MR void PNG::load (const Header& header, size_t) { DEBUG (std::string("loading PNG image") + (files.size() > 1 ? "s" : "") + " \"" + header.name() + "\""); + addresses.resize (1); + segsize = (header.datatype().bits() * voxel_count (header) + 7) / 8; + addresses[0].reset (new uint8_t[segsize]); if (is_new) { - segsize = (header.datatype().bits() * voxel_count (header) + 7) / 8; - addresses.resize (1); - addresses[0].reset (new uint8_t[segsize]); memset (addresses[0].get(), 0x00, segsize); } else { - segsize = (header.datatype().bits() * header.size(0) * header.size(1) * (header.ndim() == 4 ? header.size(3) : 1) + 7) / 8; - addresses.resize (files.size()); + 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) || @@ -49,8 +48,7 @@ namespace MR e.push_back ("File \"" + files[i].name + ": " + str(png.get_width()) + "x" + str(png.get_height()) + " x " + str(png.get_bitdepth()) + "(->" + str(png.get_output_bitdepth()) + ") bits, " + str(png.get_channels()) + " channels"); throw e; } - addresses[i].reset (new uint8_t[segsize]); - png.load (addresses[i].get()); + png.load (addresses[0].get() + (i * slice_bytes)); } } } @@ -58,19 +56,15 @@ namespace MR void PNG::unload (const Header& header) { + assert (addresses.size() == 1); if (writable) { - assert (addresses.size() == 1); 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)); } - delete[] addresses[0].release(); - } else { - assert (files.size() == addresses.size()); - for (size_t i = 0; i != files.size(); i++) - delete[] addresses[i].release(); } + delete[] addresses[0].release(); } } diff --git a/testing/binaries/tests/mrconvert b/testing/binaries/tests/mrconvert index b17770093e..4b9460dcce 100644 --- a/testing/binaries/tests/mrconvert +++ b/testing/binaries/tests/mrconvert @@ -8,7 +8,7 @@ mrconvert mrconvert/in.mif tmp.nii && testing_diff_image tmp.nii mrconvert/in.m mrconvert mrconvert/in.mif -datatype float32 tmp.nii.gz -force && testing_diff_image tmp.nii.gz mrconvert/in.mif mrconvert mrconvert/in.mif -strides 3,2,1 tmp.mgh && testing_diff_image tmp.mgh mrconvert/in.mif mrconvert mrconvert/in.mif -strides 1,3,2 -datatype int16 tmp.mgz && testing_diff_image tmp.mgz mrconvert/in.mif -mrconvert mrconvert/in.mif tmp-[].png && echo -e "1 0 0 0\n0 1 0 0\n0 0 1 0\n" > tmp.txt && testing_diff_image tmp-[].png $(mrcalc mrconvert/in.mif 0 -max - | mrtransform - -replace tmp.txt -) +mrconvert mrconvert/in.mif tmp-[].png && echo -e "1 0 0 0\n0 1 0 0\n0 0 1 0\n" > tmp.txt && testing_diff_image tmp-[].png $(mrcalc mrconvert/in.mif 0 -max - | mrtransform - -replace tmp.txt -) mrconvert unit_warp.mif tmp-[].png -datatype uint8 -force && echo -e "1 0 0 0\n0 1 0 0\n0 0 1 0\n" > tmp.txt && testing_diff_image tmp-[].png $(mrcalc unit_warp.mif 0 -max -round - | mrtransform - -replace tmp.txt - | mrconvert - -vox 1,1,1 -) mrconvert dwi.mif tmp-[].mif -force && testing_diff_image dwi.mif tmp-[].mif mrconvert dwi.mif -coord 3 0:2:end tmp1.mif -force && mrconvert tmp-[0:2:66].mif tmp2.mif && testing_diff_header -keyval tmp1.mif tmp2.mif && testing_diff_image tmp1.mif tmp2.mif From 1c80b0c6d08df54b9cc3c48cf7274b057d655743 Mon Sep 17 00:00:00 2001 From: J-Donald Tournier Date: Mon, 9 Oct 2023 10:49:00 +0100 Subject: [PATCH 5/8] PNG image handling: add check for failure to create output file --- core/file/png.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/file/png.cpp b/core/file/png.cpp index 8033f4326c..261b159712 100644 --- a/core/file/png.cpp +++ b/core/file/png.cpp @@ -197,6 +197,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()) { From 1cc476d796e4fe16192e3c612839f3da296609bb Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Thu, 15 Feb 2024 17:31:56 +1100 Subject: [PATCH 6/8] PNG handling fixes - Fix erroneous modification of strides upon header concatenation; this replicates the fix included in 41c02e9 as part of #2806, but is necessary for the suite of fixes here to work. Note that this affects mrcat in addition to multi-image format handling. - Fix setting of strides upon PNG read. - Expand testing of PNG handling. Evaluation of read and write functionality is performed separately, with the reference data fully visible and verifiable within the test data repository. Some previous tests bundled read and write testing together, which obscured the erroneous intermediate interpretation. --- core/formats/png.cpp | 8 ++++---- core/header.cpp | 5 +++-- testing/binaries/data | 2 +- testing/binaries/tests/mrconvert | 19 ++++++++++++------- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/core/formats/png.cpp b/core/formats/png.cpp index 54e74c42e7..7aa972e96d 100644 --- a/core/formats/png.cpp +++ b/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(); diff --git a/core/header.cpp b/core/header.cpp index ad5b49acf8..48240f20af 100644 --- a/core/header.cpp +++ b/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) { diff --git a/testing/binaries/data b/testing/binaries/data index 68869a7fe0..f65db971d0 160000 --- a/testing/binaries/data +++ b/testing/binaries/data @@ -1 +1 @@ -Subproject commit 68869a7fe0780c7b1e9577ba68466d5b407a3fe8 +Subproject commit f65db971d019675874751c2111877531e8bb5544 diff --git a/testing/binaries/tests/mrconvert b/testing/binaries/tests/mrconvert index 4b9460dcce..e012f18e92 100644 --- a/testing/binaries/tests/mrconvert +++ b/testing/binaries/tests/mrconvert @@ -8,8 +8,8 @@ mrconvert mrconvert/in.mif tmp.nii && testing_diff_image tmp.nii mrconvert/in.m mrconvert mrconvert/in.mif -datatype float32 tmp.nii.gz -force && testing_diff_image tmp.nii.gz mrconvert/in.mif mrconvert mrconvert/in.mif -strides 3,2,1 tmp.mgh && testing_diff_image tmp.mgh mrconvert/in.mif mrconvert mrconvert/in.mif -strides 1,3,2 -datatype int16 tmp.mgz && testing_diff_image tmp.mgz mrconvert/in.mif -mrconvert mrconvert/in.mif tmp-[].png && echo -e "1 0 0 0\n0 1 0 0\n0 0 1 0\n" > tmp.txt && testing_diff_image tmp-[].png $(mrcalc mrconvert/in.mif 0 -max - | mrtransform - -replace tmp.txt -) -mrconvert unit_warp.mif tmp-[].png -datatype uint8 -force && echo -e "1 0 0 0\n0 1 0 0\n0 0 1 0\n" > tmp.txt && testing_diff_image tmp-[].png $(mrcalc unit_warp.mif 0 -max -round - | mrtransform - -replace tmp.txt - | mrconvert - -vox 1,1,1 -) +rm -f tmp-*.png && mrconvert mrconvert/in.mif tmp-[].png && testing_diff_image tmp-[].png $(mrcalc mrconvert/in.mif 0 -max - | mrtransform - -replace identity.txt -) +rm -f tmp-*.png && mrconvert unit_warp.mif tmp-[].png -datatype uint8 && testing_diff_image tmp-[].png $(mrcalc unit_warp.mif 0 -max -round - | mrtransform - -replace identity.txt - | mrconvert - -vox 1,1,1 -) mrconvert dwi.mif tmp-[].mif -force && testing_diff_image dwi.mif tmp-[].mif mrconvert dwi.mif -coord 3 0:2:end tmp1.mif -force && mrconvert tmp-[0:2:66].mif tmp2.mif && testing_diff_header -keyval tmp1.mif tmp2.mif && testing_diff_image tmp1.mif tmp2.mif mrconvert mrcat/voxel[].mih - | testing_diff_header -keyval - mrcat/all_axis0.mif @@ -17,8 +17,13 @@ mrconvert mrcat/all_axis3.mif tmp-[].mif -force && testing_diff_header -keyval t mrconvert dwi.mif tmp-[]-[].mif -force && testing_diff_image dwi.mif tmp-[]-[].mif mrconvert dwi.mif -coord 3 1:2:end -axes 0:2,-1,3 - | testing_diff_image - mrconvert/dwi_select_axes.mif mrinfo template.mif.gz -transform > tmp.txt && mrconvert template.mif.gz tmp[].png -force && mrconvert tmp[].png -vox 2.5 - | mrtransform - -replace tmp.txt - | mrcalc - 255 -div - | testing_diff_image - $(mrcalc template.mif.gz 1.0 -min -) -abs 0.002 -rm -f tmpaxial*.png && mrconvert template.mif.gz -coord 2 9,19,29,39,49 tmpaxial[].png && testing_diff_image tmpaxial[].png mrconvert/axial[].png -rm -f tmpcoronal*.png && mrconvert template.mif.gz -coord 1 17,32,47,62,77 -axes 0,2,1 tmpcoronal[].png && testing_diff_image tmpcoronal[].png mrconvert/coronal[].png -rm -f tmpsagittal*.png && mrconvert template.mif.gz -coord 0 27,47,67 -axes 1,2,0 tmpsagittal[].png && testing_diff_image tmpsagittal[].png mrconvert/sagittal[].png -rm -f tmpmask*.png && mrconvert mask.mif tmpmask[].png && testing_diff_image tmpmask[].png mrconvert/mask[].png -rm -f tmptissues*.png && mrconvert dwi2fod/msmt/tissues.mif tmptissues[].png && testing_diff_image tmptissues[].png mrconvert/tissues[].png +rm -f tmpaxial*.png && mrconvert template.mif.gz -coord 2 9,19,29,39,49 tmpaxial[].png && testing_diff_image tmpaxial[].png mrconvert/pngaxial[].png +mrconvert mrconvert/pngaxial[].png - | testing_diff_image - mrconvert/pngaxial.mif.gz +rm -f tmpcoronal*.png && mrconvert template.mif.gz -coord 1 17,32,47,62,77 -axes 0,2,1 tmpcoronal[].png && testing_diff_image tmpcoronal[].png mrconvert/pngcoronal[].png +mrconvert mrconvert/pngcoronal[].png - | testing_diff_image - $(mrconvert mrconvert/pngcoronal.mif.gz -axes 0,2,1 -) +rm -f tmpsagittal*.png && mrconvert template.mif.gz -coord 0 27,47,67 -axes 1,2,0 tmpsagittal[].png && testing_diff_image tmpsagittal[].png mrconvert/pngsagittal[].png +mrconvert mrconvert/pngsagittal[].png - | testing_diff_image - $(mrconvert mrconvert/pngsagittal.mif.gz -axes 1,2,0 -) +rm -f tmpmask*.png && mrconvert mask.mif tmpmask[].png && testing_diff_image tmpmask[].png mrconvert/pngmask[].png +mrconvert mrconvert/pngmask[].png - | testing_diff_image - mrconvert/pngmask.mif.gz +rm -f tmptissues*.png && mrconvert dwi2fod/msmt/tissues.mif tmprgb[].png && testing_diff_image tmprgb[].png mrconvert/pngrgb[].png +mrconvert mrconvert/pngrgb[].png - | testing_diff_image - $(mrconvert dwi2fod/msmt/tissues.mif -vox 1,1,1 - | mrtransform - -replace identity.txt - | mrcalc - 255 -mult -round 255 -min -) From a35d52f3e9fa99c1082d91c84b54e68829fdf869 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 20 Nov 2024 21:49:47 +1100 Subject: [PATCH 7/8] ImageIO::PNG: Remove commented code --- core/formats/png.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/formats/png.cpp b/core/formats/png.cpp index 7aa972e96d..9e82686ea2 100644 --- a/core/formats/png.cpp +++ b/core/formats/png.cpp @@ -149,7 +149,6 @@ 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; 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"); @@ -171,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 @@ -193,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"); From 0411e034e23801672354bda98700491cff8e4f73 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 20 Nov 2024 23:25:51 +1100 Subject: [PATCH 8/8] ImageIO: Changes to memory freeing - Omit explicit deletion of ImageIO::Base data where unnecessary; this can be handled by ImageIO::Base::close(). - FIx additional memory leaks in ImageIO::PNG. --- core/file/png.cpp | 12 ++++++++++-- core/file/png.h | 1 + core/image_io/ram.cpp | 8 -------- core/image_io/ram.h | 2 +- core/image_io/scratch.cpp | 9 --------- core/image_io/scratch.h | 2 +- core/image_io/tiff.cpp | 9 --------- core/image_io/tiff.h | 4 ++-- core/image_io/variable_scaling.cpp | 2 -- core/image_io/variable_scaling.h | 2 +- 10 files changed, 16 insertions(+), 35 deletions(-) diff --git a/core/file/png.cpp b/core/file/png.cpp index 261b159712..47049cf6b3 100644 --- a/core/file/png.cpp +++ b/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; + } } @@ -311,6 +315,10 @@ namespace MR png_ptr = NULL; info_ptr = NULL; } + if (outfile) { + fclose(outfile); + outfile = NULL; + } } @@ -332,7 +340,7 @@ 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; + delete [] row_pointers; }; diff --git a/core/file/png.h b/core/file/png.h index c0a5e65e69..87e14b969d 100644 --- a/core/file/png.h +++ b/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; diff --git a/core/image_io/ram.cpp b/core/image_io/ram.cpp index acd3b01868..b145127c09 100644 --- a/core/image_io/ram.cpp +++ b/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]; - } - } - } } diff --git a/core/image_io/ram.h b/core/image_io/ram.h index 87dac3245e..21ef01d521 100644 --- a/core/image_io/ram.h +++ b/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&) { } }; } diff --git a/core/image_io/scratch.cpp b/core/image_io/scratch.cpp index 7f5b8809aa..a4413f0986 100644 --- a/core/image_io/scratch.cpp +++ b/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(); - } - } - } } diff --git a/core/image_io/scratch.h b/core/image_io/scratch.h index 59cc7610cd..336709dd9b 100644 --- a/core/image_io/scratch.h +++ b/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&) { } }; } diff --git a/core/image_io/tiff.cpp b/core/image_io/tiff.cpp index 7246567722..d4443d06ff 100644 --- a/core/image_io/tiff.cpp +++ b/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(); - } - } - } } diff --git a/core/image_io/tiff.h b/core/image_io/tiff.h index 2b5a67760d..d0eec7dea0 100644 --- a/core/image_io/tiff.h +++ b/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&) { } }; } diff --git a/core/image_io/variable_scaling.cpp b/core/image_io/variable_scaling.cpp index 24fc0fa96a..967d61784f 100644 --- a/core/image_io/variable_scaling.cpp +++ b/core/image_io/variable_scaling.cpp @@ -61,8 +61,6 @@ namespace MR } - void VariableScaling::unload (const Header& header) { } - } } diff --git a/core/image_io/variable_scaling.h b/core/image_io/variable_scaling.h index 871cb666e4..5af2b8ed55 100644 --- a/core/image_io/variable_scaling.h +++ b/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&) { } }; }