Merge branch '238-implement-s3-sinks-pt-3' into 238-implement-s3-sink…

…s-pt-4

src/common/dimension.cpp

@@ -1,19 +1,37 @@
 #include "dimension.hh"
 
-#include <cmath>
-#include <thread>
-#include "utilities.hh"
-#include "zarr.hh"
-#include "platform.h"
+#include <algorithm>
 
 namespace zarr = acquire::sink::zarr;
 
+namespace {
+inline std::string
+trim(const std::string& s)
+{
+    // trim left
+    std::string trimmed = s;
+    trimmed.erase(trimmed.begin(),
+                  std::find_if(trimmed.begin(), trimmed.end(), [](char c) {
+                      return !std::isspace(c);
+                  }));
+
+    // trim right
+    trimmed.erase(std::find_if(trimmed.rbegin(),
+                               trimmed.rend(),
+                               [](char c) { return !std::isspace(c); })
+                    .base(),
+                  trimmed.end());
+
+    return trimmed;
+}
+} // namespace
+
 zarr::Dimension::Dimension(const std::string& name,
                            DimensionType kind,
                            uint32_t array_size_px,
                            uint32_t chunk_size_px,
                            uint32_t shard_size_chunks)
-  : name{ name }
+  : name{ trim(name) }
   , kind{ kind }
   , array_size_px{ array_size_px }
   , chunk_size_px{ chunk_size_px }
@@ -22,6 +40,7 @@ zarr::Dimension::Dimension(const std::string& name,
     EXPECT(kind < DimensionTypeCount, "Invalid dimension type.");
     EXPECT(!name.empty(), "Dimension name cannot be empty.");
 }
+
 zarr::Dimension::Dimension(const StorageDimension& dim)
   : Dimension(dim.name.str,
               dim.kind,
@@ -30,3 +49,31 @@ zarr::Dimension::Dimension(const StorageDimension& dim)
               dim.shard_size_chunks)
 {
 }
+
+#ifndef NO_UNIT_TESTS
+#ifdef _WIN32
+#define acquire_export __declspec(dllexport)
+#else
+#define acquire_export
+#endif // _WIN32
+
+extern "C" acquire_export int
+unit_test__trim()
+{
+    try {
+        EXPECT(trim("  foo") == "foo", "Failed to trim left.");
+        EXPECT(trim("foo  ") == "foo", "Failed to trim right.");
+        EXPECT(trim("  foo  ") == "foo", "Failed to trim both.");
+        EXPECT(trim("foo") == "foo", "Failed to trim either.");
+        EXPECT(trim("").empty(), "Failed to trim empty.");
+        return 1;
+    } catch (const std::exception& e) {
+        LOGE("Exception: %s", e.what());
+    } catch (...) {
+        LOGE("Unknown exception");
+    }
+
+    return 0;
+}
+
+#endif // NO_UNIT_TESTS

src/common/dimension.hh

@@ -1,6 +1,6 @@
-#ifndef H_ACQUIRE_STORAGE_ZARR_COMMON_DIMENSION_V0
-#define H_ACQUIRE_STORAGE_ZARR_COMMON_DIMENSION_V0
+#pragma once
 
+#include "macros.hh"
 #include "device/props/storage.h"
 
 #include <string>
@@ -14,6 +14,7 @@ struct Dimension
                        unsigned int array_size_px,
                        unsigned int chunk_size_px,
                        unsigned int shard_size_chunks);
+
     explicit Dimension(const StorageDimension& dim);
 
     const std::string name;
@@ -23,4 +24,3 @@ struct Dimension
     const unsigned int shard_size_chunks;
 };
 } // namespace acquire::sink::zarr
-#endif // H_ACQUIRE_STORAGE_ZARR_COMMON_DIMENSION_V0

src/common/macros.hh

@@ -0,0 +1,20 @@
+#pragma once
+
+#include "logger.h"
+#include <stdexcept>
+
+#define LOG(...) aq_logger(0, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__)
+#define LOGE(...) aq_logger(1, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__)
+#define EXPECT(e, ...)                                                         \
+    do {                                                                       \
+        if (!(e)) {                                                            \
+            LOGE(__VA_ARGS__);                                                 \
+            throw std::runtime_error("Expression was false: " #e);             \
+        }                                                                      \
+    } while (0)
+#define CHECK(e) EXPECT(e, "Expression evaluated as false:\n\t%s", #e)
+
+#define TRACE(...)
+
+#define containerof(ptr, T, V) ((T*)(((char*)(ptr)) - offsetof(T, V)))
+#define countof(e) (sizeof(e) / sizeof(*(e)))

src/common/thread.pool.cpp

@@ -4,15 +4,14 @@
 namespace zarr = acquire::sink::zarr;
 namespace common = zarr::common;
 
-common::ThreadPool::ThreadPool(size_t n_threads,
+common::ThreadPool::ThreadPool(unsigned int n_threads,
                                std::function<void(const std::string&)> err)
   : error_handler_{ err }
   , is_accepting_jobs_{ true }
 {
+    const unsigned int one = 1;
     n_threads = std::clamp(
-      n_threads,
-      (size_t)1,
-      (size_t)std::max(std::thread::hardware_concurrency(), (unsigned)1));
+      n_threads, one, std::max(std::thread::hardware_concurrency(), one));
 
     for (auto i = 0; i < n_threads; ++i) {
         threads_.emplace_back([this] { thread_worker_(); });
@@ -32,11 +31,12 @@ common::ThreadPool::~ThreadPool() noexcept
 void
 common::ThreadPool::push_to_job_queue(JobT&& job)
 {
-    std::unique_lock lock(jobs_mutex_);
-    CHECK(is_accepting_jobs_);
+    {
+        std::unique_lock lock(jobs_mutex_);
+        CHECK(is_accepting_jobs_);
 
-    jobs_.push(std::move(job));
-    lock.unlock();
+        jobs_.push(std::move(job));
+    }
 
     cv_.notify_one();
 }

src/common/thread.pool.hh

@@ -20,7 +20,7 @@ struct ThreadPool final
     // std::string& argument to the error handler is a diagnostic message from
     // the failing job and is logged to the error stream by the Zarr driver when
     // the next call to `append()` is made.
-    ThreadPool(size_t n_threads, std::function<void(const std::string&)> err);
+    ThreadPool(unsigned int n_threads, std::function<void(const std::string&)> err);
     ~ThreadPool() noexcept;
 
     void push_to_job_queue(JobT&& job);

src/common/utilities.cpp

@@ -166,28 +166,22 @@ common::bytes_per_chunk(const std::vector<Dimension>& dimensions,
     return n_bytes;
 }
 
-const char*
-common::sample_type_to_dtype(SampleType t)
-
-{
-    static const char* table[] = { "u1", "u2", "i1", "i2",
-                                   "f4", "u2", "u2", "u2" };
-    if (t < countof(table)) {
-        return table[t];
-    } else {
-        throw std::runtime_error("Invalid sample type.");
-    }
-}
-
 const char*
 common::sample_type_to_string(SampleType t) noexcept
 {
-    static const char* table[] = { "u8",  "u16", "i8",  "i16",
-                                   "f32", "u16", "u16", "u16" };
-    if (t < countof(table)) {
-        return table[t];
-    } else {
-        return "unrecognized pixel type";
+    switch (t) {
+        case SampleType_u8:
+            return "u8";
+        case SampleType_u16:
+            return "u16";
+        case SampleType_i8:
+            return "i8";
+        case SampleType_i16:
+            return "i16";
+        case SampleType_f32:
+            return "f32";
+        default:
+            return "unrecognized pixel type";
     }
 }
 
@@ -207,15 +201,11 @@ common::split_uri(const std::string& uri)
     auto end = uri.find_first_of(delim);
 
     std::vector<std::string> out;
-    while (end <= std::string::npos) {
+    while (end != std::string::npos) {
         if (end > begin) {
             out.emplace_back(uri.substr(begin, end - begin));
         }
 
-        if (end == std::string::npos) {
-            break;
-        }
-
         begin = end + 1;
         end = uri.find_first_of(delim, begin);
     }

src/common/utilities.hh

@@ -4,6 +4,7 @@
 #include "logger.h"
 #include "device/props/components.h"
 #include "device/props/storage.h"
+#include "macros.hh"
 #include "dimension.hh"
 
 #include <condition_variable>
@@ -15,23 +16,6 @@
 #include <stdexcept>
 #include <thread>
 
-#define LOG(...) aq_logger(0, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__)
-#define LOGE(...) aq_logger(1, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__)
-#define EXPECT(e, ...)                                                         \
-    do {                                                                       \
-        if (!(e)) {                                                            \
-            LOGE(__VA_ARGS__);                                                 \
-            throw std::runtime_error("Expression was false: " #e);             \
-        }                                                                      \
-    } while (0)
-#define CHECK(e) EXPECT(e, "Expression evaluated as false:\n\t%s", #e)
-
-// #define TRACE(...) LOG(__VA_ARGS__)
-#define TRACE(...)
-
-#define containerof(ptr, T, V) ((T*)(((char*)(ptr)) - offsetof(T, V)))
-#define countof(e) (sizeof(e) / sizeof(*(e)))
-
 namespace fs = std::filesystem;
 
 namespace acquire::sink::zarr {
@@ -96,13 +80,6 @@ size_t
 bytes_per_chunk(const std::vector<Dimension>& dimensions,
                 const SampleType& dtype);
 
-/// @brief Get the Zarr dtype for a given SampleType.
-/// @param t An enumerated sample type.
-/// @throw std::runtime_error if @par t is not a valid SampleType.
-/// @return A representation of the SampleType @par t expected by a Zarr reader.
-const char*
-sample_type_to_dtype(SampleType t);
-
 /// @brief Get a string representation of the SampleType enum.
 /// @param t An enumerated sample type.
 /// @return A human-readable representation of the SampleType @par t.

src/zarr.v2.cpp

@@ -4,6 +4,8 @@
 
 #include "nlohmann/json.hpp"
 
+#include <bit>
+
 namespace zarr = acquire::sink::zarr;
 
 namespace {
@@ -22,6 +24,33 @@ compressed_zarr_v2_init()
     }
     return nullptr;
 }
+
+std::string
+sample_type_to_dtype(SampleType t)
+
+{
+    const std::string dtype_prefix =
+      std::endian::native == std::endian::big ? ">" : "<";
+
+    switch (t) {
+        case SampleType_u8:
+            return dtype_prefix + "u1";
+        case SampleType_u10:
+        case SampleType_u12:
+        case SampleType_u14:
+        case SampleType_u16:
+            return dtype_prefix + "u2";
+        case SampleType_i8:
+            return dtype_prefix + "i1";
+        case SampleType_i16:
+            return dtype_prefix + "i2";
+        case SampleType_f32:
+            return dtype_prefix + "f4";
+        default:
+            throw std::runtime_error("Invalid SampleType: " +
+                                     std::to_string(static_cast<int>(t)));
+    }
+}
 } // end ::{anonymous} namespace
 
 /// ZarrV2
@@ -249,7 +278,7 @@ zarr::ZarrV2::write_array_metadata_(size_t level) const
     metadata["zarr_format"] = 2;
     metadata["shape"] = array_shape;
     metadata["chunks"] = chunk_shape;
-    metadata["dtype"] = common::sample_type_to_dtype(image_shape.type);
+    metadata["dtype"] = sample_type_to_dtype(image_shape.type);
     metadata["fill_value"] = 0;
     metadata["order"] = "C";
     metadata["filters"] = nullptr;

src/zarr.v3.cpp

@@ -24,6 +24,30 @@ compressed_zarr_v3_init()
     }
     return nullptr;
 }
+
+std::string
+sample_type_to_dtype(SampleType t)
+
+{
+    switch (t) {
+        case SampleType_u8:
+            return "uint8";
+        case SampleType_u10:
+        case SampleType_u12:
+        case SampleType_u14:
+        case SampleType_u16:
+            return "uint16";
+        case SampleType_i8:
+            return "int8";
+        case SampleType_i16:
+            return "int16";
+        case SampleType_f32:
+            return "float32";
+        default:
+            throw std::runtime_error("Invalid SampleType: " +
+                                     std::to_string(static_cast<int>(t)));
+    }
+}
 } // end ::{anonymous} namespace
 
 zarr::ZarrV3::ZarrV3(BloscCompressionParams&& compression_params)
@@ -175,7 +199,7 @@ zarr::ZarrV3::write_array_metadata_(size_t level) const
     });
 
     metadata["chunk_memory_layout"] = "C";
-    metadata["data_type"] = common::sample_type_to_dtype(image_shape.type);
+    metadata["data_type"] = sample_type_to_dtype(image_shape.type);
     metadata["extensions"] = json::array();
     metadata["fill_value"] = 0;
     metadata["shape"] = array_shape;

tests/multiscales-metadata.cpp

@@ -182,6 +182,10 @@ verify_layer(const LayerTestCase& test_case)
     std::ifstream f(zarray_path);
     json zarray = json::parse(f);
 
+    const std::string dtype =
+      std::endian::native == std::endian::little ? "<u1" : ">u1";
+    CHECK(dtype == zarray["dtype"].get<std::string>());
+
     const auto shape = zarray["shape"];
     ASSERT_EQ(int, "%d", frames_per_layer, shape[0]);
     ASSERT_EQ(int, "%d", layer_frame_height, shape[1]);

tests/repeat-start.cpp

@@ -187,7 +187,9 @@ validate(AcquireRuntime* runtime)
     ASSERT_EQ(int, "%d", 32, chunk_shape[3]);
 
     CHECK("C" == metadata["chunk_memory_layout"]);
-    CHECK("u1" == metadata["data_type"]);
+
+    CHECK("uint8" == metadata["data_type"].get<std::string>());
+
     CHECK(metadata["extensions"].empty());
 
     const auto array_shape = metadata["shape"];

tests/unit-tests.cpp

@@ -82,6 +82,7 @@ main()
     };
     const std::vector<testcase> tests{
 #define CASE(e) { .name = #e, .test = (int (*)())lib_load(&lib, #e) }
+        CASE(unit_test__trim),
         CASE(unit_test__average_frame),
         CASE(unit_test__thread_pool__push_to_job_queue),
         CASE(unit_test__sink_creator__create_chunk_file_sinks),