Update the chunking API to address some shortcomings and support shar…

…ding (#41) # Description The chunking parameters have been updated in the `StorageProperties` struct, allowing users to specify the number of actual planes per chunk, rather than setting the max bytes per chunk. Also, sharding properties have been added. ## Related issue(s) - See also: - acquire-project/acquire-driver-zarr#137 ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [X] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation - [ ] Other (please describe): # How has this been tested? Added a unit test to check sharding property setter convenience function works as expected. Updated the chunking property setter function. # Checklist - [X] I have performed a self-review of my own code. - [X] I have commented my code, particularly in hard-to-understand areas. - [X] I have updated the documentation, if necessary. - [X] I added tests for my changes, and/or the testing strategy is described above. - [X] I updated the [changelog](../CHANGELOG.md).
acquire-project · Nov 13, 2023 · 9298169 · 9298169
1 parent 983719d
commit 9298169
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 51 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Users can specify the full chunk size in width, height, and planes. 
+
+### Removed
+
+- `max_bytes_per_chunk` from `StorageProperties` chunking properties.
+
 ### Fixed
 
 - Removes 30-second timeout from `thread_join` on Windows. 
@@ -17,6 +25,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `acquire-device-properties`: A corresponding entry in `StoragePropertyMetadata`.
 - `acquire-device-properties`: A convenience function for enabling multiscale, similar to the function for
   setting chunking properties.
+- `acquire-device-properties`: A `struct storage_properties_sharding_s` member `shard_dims_chunks` of
+  `StorageProperties`. Users can now configure sharding properties where supported.
+- `acquire-device-properties`: A convenience function for setting sharding parameters.
 
 ## [0.1.3](https://github.com/acquire-project/acquire-core-libs/compare/v0.1.2...v0.1.3) - 2023-06-27
 

diff --git a/src/acquire-device-properties/device/props/storage.c b/src/acquire-device-properties/device/props/storage.c
@@ -76,18 +76,29 @@ storage_properties_set_external_metadata(struct StorageProperties* out,
 
 int
 storage_properties_set_chunking_props(struct StorageProperties* out,
-                                      uint32_t tile_width,
-                                      uint32_t tile_height,
-                                      uint32_t tile_planes,
-                                      uint64_t max_bytes_per_chunk)
+                                      uint32_t chunk_width,
+                                      uint32_t chunk_height,
+                                      uint32_t chunk_planes)
 {
     CHECK(out);
-    out->chunking.tile.width = tile_width;
-    out->chunking.tile.height = tile_height;
-    out->chunking.tile.planes = tile_planes;
-    // Use 16 MB default chunk size if 0 is passed in.
-    out->chunking.max_bytes_per_chunk =
-      max_bytes_per_chunk ? max_bytes_per_chunk : (1ULL << 24);
+    out->chunk_dims_px.width = chunk_width;
+    out->chunk_dims_px.height = chunk_height;
+    out->chunk_dims_px.planes = chunk_planes;
+    return 1;
+Error:
+    return 0;
+}
+
+int
+storage_properties_set_sharding_props(struct StorageProperties* out,
+                                      uint32_t shard_width,
+                                      uint32_t shard_height,
+                                      uint32_t shard_planes)
+{
+    CHECK(out);
+    out->shard_dims_chunks.width = shard_width;
+    out->shard_dims_chunks.height = shard_height;
+    out->shard_dims_chunks.planes = shard_planes;
     return 1;
 Error:
     return 0;
@@ -353,20 +364,40 @@ int
 unit_test__storage_properties_set_chunking_props()
 {
     struct StorageProperties props = { 0 };
-    CHECK(0 == props.chunking.tile.width);
-    CHECK(0 == props.chunking.tile.height);
-    CHECK(0 == props.chunking.tile.planes);
-    CHECK(0 == props.chunking.max_bytes_per_chunk);
+    CHECK(0 == props.chunk_dims_px.width);
+    CHECK(0 == props.chunk_dims_px.height);
+    CHECK(0 == props.chunk_dims_px.planes);
 
-    const uint32_t tile_width = 1, tile_height = 2, tile_planes = 3;
+    const uint32_t chunk_width = 1, chunk_height = 2, chunk_planes = 3;
     CHECK(storage_properties_set_chunking_props(
-      &props, tile_width, tile_height, tile_planes, 0));
+      &props, chunk_width, chunk_height, chunk_planes));
+
+    CHECK(chunk_width == props.chunk_dims_px.width);
+    CHECK(chunk_height == props.chunk_dims_px.height);
+    CHECK(chunk_planes == props.chunk_dims_px.planes);
+
+    storage_properties_destroy(&props);
+
+    return 1;
+Error:
+    return 0;
+}
+
+int
+unit_test__storage_properties_set_sharding_props()
+{
+    struct StorageProperties props = { 0 };
+    CHECK(0 == props.shard_dims_chunks.width);
+    CHECK(0 == props.shard_dims_chunks.height);
+    CHECK(0 == props.shard_dims_chunks.planes);
+
+    const uint32_t shard_width = 1, shard_height = 2, shard_planes = 3;
+    CHECK(storage_properties_set_sharding_props(
+      &props, shard_width, shard_height, shard_planes));
 
-    CHECK(tile_width == props.chunking.tile.width);
-    CHECK(tile_height == props.chunking.tile.height);
-    CHECK(tile_planes == props.chunking.tile.planes);
-    // This is the default value if you set bytes_per_chunk to 0
-    CHECK((1ULL << 24) == props.chunking.max_bytes_per_chunk);
+    CHECK(shard_width == props.shard_dims_chunks.width);
+    CHECK(shard_height == props.shard_dims_chunks.height);
+    CHECK(shard_planes == props.shard_dims_chunks.planes);
 
     storage_properties_destroy(&props);
 

diff --git a/src/acquire-device-properties/device/props/storage.h b/src/acquire-device-properties/device/props/storage.h
@@ -19,18 +19,17 @@ extern "C"
         uint32_t first_frame_id;
         struct PixelScale pixel_scale_um;
 
-        /// Parameters for chunking.
-        /// Tile dimensions, width, height, and planes, are in pixels and
-        /// determine how to break up frames. Together with
-        /// `max_bytes_per_chunk`, they determine the dimensions of a chunk.
+        /// Dimensions of chunks, in pixels.
         struct storage_properties_chunking_s
         {
-            uint64_t max_bytes_per_chunk;
-            struct storage_properties_chunking_tile_s
-            {
-                uint32_t width, height, planes;
-            } tile;
-        } chunking;
+            uint32_t width, height, planes;
+        } chunk_dims_px;
+
+        /// Dimensions of shards, in chunks.
+        struct storage_properties_sharding_s
+        {
+            uint32_t width, height, planes;
+        } shard_dims_chunks;
 
         /// Enable multiscale storage if true.
         uint8_t enable_multiscale;
@@ -39,21 +38,26 @@ extern "C"
     struct StoragePropertyMetadata
     {
         /// Metadata for chunking.
-        /// Indicates whether chunking is supported, and if so, bounds on the
-        /// dimensions of tiles and the maximum number of bytes per chunk.
+        /// Indicates whether chunking is supported, and if so, bounds on what
+        /// the dimensions (in px) of the chunks are.
         struct storage_property_metadata_chunking_s
         {
-            uint8_t supported;
-            struct Property max_bytes_per_chunk;
-            struct storage_property_metadata_chunk_dim_s
-            {
-                struct Property width, height, planes;
-            } tile;
-        } chunking;
+            uint8_t is_supported;
+            struct Property width, height, planes;
+        } chunk_dims_px;
+
+        /// Metadata for sharding.
+        /// Indicates whether sharding is supported, and if so, bounds on what
+        /// the dimensions (in chunks) of the shards are.
+        struct storage_property_metadata_sharding_s
+        {
+            uint8_t is_supported;
+            struct Property width, height, planes;
+        } shard_dims_chunks;
 
         struct storage_property_metadata_multiscale_s
         {
-            uint8_t supported;
+            uint8_t is_supported;
         } multiscale;
     };
 
@@ -111,19 +115,32 @@ extern "C"
                                                  size_t bytes_of_metadata);
 
     /// @brief Set chunking properties for `out`.
-    /// Convenience function to set tiling/chunking properties in a single call.
+    /// Convenience function to set chunking properties in a single call.
     /// @returns 1 on success, otherwise 0
     /// @param[in, out] out The storage properties to change.
-    /// @param[in] tile_width The width, in px, of a tile.
-    /// @param[in] tile_height The height, in px, of a tile.
-    /// @param[in] tile_planes The number of @p tile_width x @p tile_height
-    ///            planes in a single tile.
-    /// @param[in] max_bytes_per_chunk The maximum size, in bytes, of a chunk.
+    /// @param[in] chunk_width The width, in px, of a chunk.
+    /// @param[in] chunk_height The height, in px, of a chunk.
+    /// @param[in] chunk_planes The number of @p chunk_width x @p chunk_height
+    ///            planes in a single chunk.
     int storage_properties_set_chunking_props(struct StorageProperties* out,
-                                              uint32_t tile_width,
-                                              uint32_t tile_height,
-                                              uint32_t tile_planes,
-                                              uint64_t max_bytes_per_chunk);
+                                              uint32_t chunk_width,
+                                              uint32_t chunk_height,
+                                              uint32_t chunk_planes);
+
+    /// @brief Set sharding properties for `out`.
+    /// Convenience function to set sharding properties in a single call.
+    /// @returns 1 on success, otherwise 0
+    /// @param[in, out] out The storage properties to change.
+    /// @param[in] shard_width The number of chunks in a shard along the x
+    ///            dimension.
+    /// @param[in] shard_height The number of chunks in a shard along the y
+    ///            dimension.
+    /// @param[in] shard_planes The number of chunks in a shard along the append
+    ///            dimension.
+    int storage_properties_set_sharding_props(struct StorageProperties* out,
+                                              uint32_t shard_width,
+                                              uint32_t shard_height,
+                                              uint32_t shard_planes);
 
     /// @brief Set multiscale properties for `out`.
     /// Convenience function to enable multiscale.

diff --git a/tests/unit-tests.cpp b/tests/unit-tests.cpp
@@ -68,6 +68,7 @@ extern "C"
     int unit_test__storage__storage_property_string_check();
     int unit_test__storage__copy_string();
     int unit_test__storage_properties_set_chunking_props();
+    int unit_test__storage_properties_set_sharding_props();
     int unit_test__device_state_as_string__is_defined_for_all();
     int unit_test__device_kind_as_string__is_defined_for_all();
     int unit_test__sample_type_as_string__is_defined_for_all();
@@ -93,6 +94,7 @@ main()
         CASE(unit_test__storage__storage_property_string_check),
         CASE(unit_test__storage__copy_string),
         CASE(unit_test__storage_properties_set_chunking_props),
+        CASE(unit_test__storage_properties_set_sharding_props),
         CASE(unit_test__device_state_as_string__is_defined_for_all),
         CASE(unit_test__device_kind_as_string__is_defined_for_all),
         CASE(unit_test__sample_type_as_string__is_defined_for_all),