diff --git a/CHANGELOG.md b/CHANGELOG.md index a23faf2..9977a0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Users can specify the full chunk size in width, height, and planes. +- Users can specify the full chunk size in width, height, and planes. +- `acquire-device-hal`: `storage_open` no longer takes a `StorageProperties*` parameter. ### Removed @@ -17,7 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Removes 30-second timeout from `thread_join` on Windows. +- Removes 30-second timeout from `thread_join` on Windows. +- Memory leak in `copy_string`. +- Avoid an unnecessary call to `realloc`. ### Added @@ -28,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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. +- `acquire-device-hal`: `storage_start`, `storage_stop`, and `storage_set` functions. ## [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-hal/device/hal/storage.c b/src/acquire-device-hal/device/hal/storage.c index 35ddf74..adff42a 100644 --- a/src/acquire-device-hal/device/hal/storage.c +++ b/src/acquire-device-hal/device/hal/storage.c @@ -73,8 +73,7 @@ storage_validate(const struct DeviceManager* system, struct Storage* storage_open(const struct DeviceManager* system, - const struct DeviceIdentifier* identifier, - struct StorageProperties* settings) + const struct DeviceIdentifier* identifier) { struct Storage* self = 0; @@ -90,18 +89,39 @@ storage_open(const struct DeviceManager* system, self = containerof(device, struct Storage, device); } - if (self) { - self->state = self->set(self, settings); - CHECK(self->state == DeviceState_Armed); - self->state = self->start(self); - CHECK(self->state == DeviceState_Running); - } + // Check the required interface functions are non-null + CHECK(self->set != NULL); + CHECK(self->get != NULL); + CHECK(self->get_meta != NULL); + CHECK(self->start != NULL); + CHECK(self->append != NULL); + CHECK(self->stop != NULL); + CHECK(self->destroy != NULL); + CHECK(self->reserve_image_shape != NULL); + return self; Error: storage_close(self); return 0; } +enum DeviceStatusCode +storage_set(struct Storage* self, const struct StorageProperties* settings) +{ + CHECK(self); + CHECK(settings); + + self->state = self->set(self, settings); + EXPECT(DeviceState_Armed == self->state, + "Expected Armed. Got %s.", + device_state_as_string(self->state)); + + return Device_Ok; + +Error: + return Device_Err; +} + enum DeviceStatusCode storage_get(const struct Storage* self, struct StorageProperties* settings) { @@ -125,6 +145,47 @@ storage_get_meta(const struct Storage* self, return Device_Err; } +enum DeviceStatusCode +storage_start(struct Storage* self) +{ + CHECK(self); + CHECK(self->state == DeviceState_Armed); + + enum DeviceStatusCode status_code; + switch (self->state = self->start(self)) { + case DeviceState_Running: + status_code = Device_Ok; + break; + default: + status_code = Device_Err; + break; + } + + return status_code; +Error: + return Device_Err; +} + +enum DeviceStatusCode +storage_stop(struct Storage* self) +{ + enum DeviceStatusCode ecode = Device_Ok; + CHECK(self); + CHECK(self->stop); + if (self->state == DeviceState_Running) { + EXPECT((self->state = self->stop(self)) == DeviceState_Armed || + self->state == DeviceState_AwaitingConfiguration, + "Expected Armed or AwaitingConfiguration. Got state: %s.", + device_state_as_string(self->state)); + } + +Finalize: + return ecode; +Error: + ecode = Device_Err; + goto Finalize; +} + enum DeviceStatusCode storage_append(struct Storage* self, const struct VideoFrame* beg, @@ -145,25 +206,15 @@ storage_append(struct Storage* self, return Device_Err; } -enum DeviceStatusCode +void storage_close(struct Storage* self) { - enum DeviceStatusCode ecode = Device_Ok; - CHECK_SILENT(self); - self->state = self->stop(self); - EXPECT(self->state == DeviceState_Armed || - self->state == DeviceState_AwaitingConfiguration, - "Expected Armed or AwaitingConfiguration. Got state: %s.", - device_state_as_string(self->state)); -Finalize: - if (self) { - driver_close_device(&self->device); - self->state = DeviceState_Closed; - } - return ecode; -Error: - ecode = Device_Err; - goto Finalize; + CHECK(self); + storage_stop(self); + + driver_close_device(&self->device); + self->state = DeviceState_Closed; +Error:; } enum DeviceState diff --git a/src/acquire-device-hal/device/hal/storage.h b/src/acquire-device-hal/device/hal/storage.h index 157bb32..24e541c 100644 --- a/src/acquire-device-hal/device/hal/storage.h +++ b/src/acquire-device-hal/device/hal/storage.h @@ -15,9 +15,19 @@ extern "C" const struct DeviceIdentifier* identifier, const struct StorageProperties* settings); + /// @brief Open a storage device identified by @p identifier. + /// @param[in] system The device manager. + /// @return A pointer to the storage device, or NULL if the device could not + /// be opened. struct Storage* storage_open(const struct DeviceManager* system, - const struct DeviceIdentifier* identifier, - struct StorageProperties* settings); + const struct DeviceIdentifier* identifier); + + /// @brief Set the storage device properties. + /// @param[in] settings The properties to set. + /// @returns Device_Ok if the properties were set successfully, otherwise + /// Device_Err. + enum DeviceStatusCode storage_set(struct Storage* self, + const struct StorageProperties* settings); enum DeviceStatusCode storage_get(const struct Storage* self, struct StorageProperties* settings); @@ -26,6 +36,26 @@ extern "C" const struct Storage* self, struct StoragePropertyMetadata* meta); + /// @brief Start the storage device. + /// @details This function signals the storage device that data is about to + /// begin streaming to the storage. Any preparation the device needs to do + + /// should be handled here (e.g. opening a file or a network connection). + /// A successful call to this function will set self->state to + /// DeviceState_Running. + /// @returns Device_Ok if the device was started successfully, otherwise + /// Device_Err. + enum DeviceStatusCode storage_start(struct Storage* storage); + + /// @brief Stop the storage device. + /// @details This function signals the storage device that no more data is + /// forthcoming. The device is free to close any open files or network + /// connections. A successful call to this function will set self->state to + /// DeviceState_Armed. + /// @returns Device_Ok if the device was stopped successfully, otherwise + /// Device_Err. + enum DeviceStatusCode storage_stop(struct Storage* storage); + /// @brief Append data in `[beg,end)` to Storage /// @param[in] beg The beginning of the packet of frames to write. /// @param[in] end The end of the packet of frames to write. @@ -33,7 +63,10 @@ extern "C" const struct VideoFrame* beg, const struct VideoFrame* end); - enum DeviceStatusCode storage_close(struct Storage* self); + /// @brief Close the storage device. + /// @details The storage device is deallocated and any resources it was + /// using are freed. + void storage_close(struct Storage* self); enum DeviceState storage_get_state(const struct Storage* self); diff --git a/src/acquire-device-properties/device/props/storage.c b/src/acquire-device-properties/device/props/storage.c index e0d091d..bd0fd07 100644 --- a/src/acquire-device-properties/device/props/storage.c +++ b/src/acquire-device-properties/device/props/storage.c @@ -32,12 +32,19 @@ copy_string(struct String* dst, const struct String* src) // dst string pointer refers to caller-allocated memory. // Allocate a new string on the heap. CHECK(dst->str = malloc(src->nbytes)); // NOLINT + dst->nbytes = src->nbytes; dst->is_ref = 0; // mark as owned } CHECK(dst->is_ref == 0); if (src->nbytes > dst->nbytes) { - CHECK(dst->str = realloc(dst->str, src->nbytes)); + char* str = realloc(dst->str, src->nbytes); + if (!str) { + LOGE("Failed to allocate %llu bytes for string copy.", + (unsigned long long)src->nbytes); + goto Error; + } + dst->str = str; } dst->nbytes = src->nbytes;