Skip to content

Commit

Permalink
Align channel_write_map requests to 8 bytes (#45)
Browse files Browse the repository at this point in the history
* Define bytes_of_image() in components.h.

* Catch errors thrown in one-video-stream.cpp.

* Align VideoFrame struct to 64 bytes.

* wip

* Align to 8s when allocating VideoFrame.

* fix line endings in components.h

* Align to 8 when filtering as well.

* Apply suggestions from code review

Co-authored-by: Nathan Clack <[email protected]>

* Apply suggestions from code review

Co-authored-by: Nathan Clack <[email protected]>

* Add a semicolon.

---------

Co-authored-by: Nathan Clack <[email protected]>
  • Loading branch information
aliddell and nclack authored May 31, 2024
1 parent 078bd3f commit 7afe044
Show file tree
Hide file tree
Showing 8 changed files with 249 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ bytes_of_type(enum SampleType type)
return table[type];
}

size_t
bytes_of_image(const struct ImageShape* const shape)
{
return shape->strides.planes * bytes_of_type(shape->type);
}

//
// UNIT TESTS
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ extern "C"

const char* sample_type_as_string(enum SampleType type);
size_t bytes_of_type(enum SampleType type);
size_t bytes_of_image(const struct ImageShape* shape);

#ifdef __cplusplus
}
Expand Down
6 changes: 0 additions & 6 deletions acquire-driver-common/src/simcams/simulated.camera.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,6 @@ struct SimulatedCamera
struct Camera camera;
};

static size_t
bytes_of_image(const struct ImageShape* const shape)
{
return shape->strides.planes * bytes_of_type(shape->type);
}

static size_t
aligned_bytes_of_image(const struct ImageShape* const shape)
{
Expand Down
11 changes: 4 additions & 7 deletions acquire-video-runtime/src/runtime/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@ slice_size_bytes(const struct slice* slice)
return (uint8_t*)slice->end - (uint8_t*)slice->beg;
}

static size_t
bytes_of_image(const struct ImageShape* const shape)
{
return shape->strides.planes * bytes_of_type(shape->type);
}

static int
assert_consistent_shape(const struct VideoFrame* acc,
const struct VideoFrame* in)
Expand Down Expand Up @@ -113,8 +107,11 @@ process_data(struct video_filter_s* self,
if (!*accumulator) {
struct ImageShape shape = in->shape;
shape.type = SampleType_f32;
size_t bytes_of_accumulator =

const size_t nbytes =
bytes_of_image(&shape) + sizeof(struct VideoFrame);
const size_t bytes_of_accumulator = 8*((nbytes+7)/8);

*accumulator = (struct VideoFrame*)channel_write_map(
self->out, bytes_of_accumulator);
if (*accumulator) {
Expand Down
12 changes: 4 additions & 8 deletions acquire-video-runtime/src/runtime/source.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,6 @@
} while (0)
#define CHECK(e) EXPECT(e, "Expression evaluated as false:\n\t%s", #e)

static size_t
bytes_of_image(const struct ImageShape* const shape)
{
return shape->strides.planes * bytes_of_type(shape->type);
}

static int
check_frame_id(uint8_t stream_id,
uint64_t iframe,
Expand Down Expand Up @@ -66,6 +60,8 @@ video_source_thread(struct video_source_s* self)

size_t sz = bytes_of_image(&info.shape);
size_t nbytes = sizeof(struct VideoFrame) + sz;
// padding to 8-byte aligned size
const size_t nbytes_aligned = 8*((nbytes+7)/8);

struct channel* channel =
(self->enable_filter) ? self->to_filter : self->to_sink;
Expand All @@ -76,7 +72,7 @@ video_source_thread(struct video_source_s* self)
last_stream = channel;

struct VideoFrame* im =
(struct VideoFrame*)channel_write_map(channel, nbytes);
(struct VideoFrame*)channel_write_map(channel, nbytes_aligned);
if (im) {
CHECK(camera_get_frame(self->camera, im->data, &sz, &info) ==
Device_Ok);
Expand All @@ -88,7 +84,7 @@ video_source_thread(struct video_source_s* self)
last_hardware_frame_id = info.hardware_frame_id;
*im = (struct VideoFrame){
.shape = info.shape,
.bytes_of_frame = nbytes,
.bytes_of_frame = nbytes_aligned,
.frame_id = iframe,
.hardware_frame_id = info.hardware_frame_id,
.timestamps.hardware = info.hardware_timestamp,
Expand Down
1 change: 1 addition & 0 deletions acquire-video-runtime/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ else ()
zero-config-start
filter-video-average
repeat-start-no-monitor
aligned-videoframe-pointers
)

foreach (name ${tests})
Expand Down
190 changes: 190 additions & 0 deletions acquire-video-runtime/tests/aligned-videoframe-pointers.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
/// @file aligned-videoframe-pointers.cpp
/// Test that VideoFrame pointers are aligned at 8 bytes.

#include "acquire.h"
#include "device/hal/device.manager.h"
#include "platform.h"
#include "logger.h"

#include <cstdio>
#include <stdexcept>

void
reporter(int is_error,
const char* file,
int line,
const char* function,
const char* msg)
{
fprintf(is_error ? stderr : stdout,
"%s%s(%d) - %s: %s\n",
is_error ? "ERROR " : "",
file,
line,
function,
msg);
}

/// Helper for passing size static strings as function args.
/// For a function: `f(char*,size_t)` use `f(SIZED("hello"))`.
/// Expands to `f("hello",5)`.
#define SIZED(str) str, sizeof(str)

#define L (aq_logger)
#define LOG(...) L(0, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__)
#define ERR(...) L(1, __FILE__, __LINE__, __FUNCTION__, __VA_ARGS__)
#define EXPECT(e, ...) \
do { \
if (!(e)) { \
char buf[1 << 8] = { 0 }; \
ERR(__VA_ARGS__); \
snprintf(buf, sizeof(buf) - 1, __VA_ARGS__); \
throw std::runtime_error(buf); \
} \
} while (0)
#define CHECK(e) EXPECT(e, "Expression evaluated as false: %s", #e)
#define DEVOK(e) CHECK(Device_Ok == (e))
#define OK(e) CHECK(AcquireStatus_Ok == (e))

void
configure(AcquireRuntime* runtime)
{
CHECK(runtime);
const DeviceManager* dm = acquire_device_manager(runtime);
CHECK(dm);

AcquireProperties props = {};
OK(acquire_get_configuration(runtime, &props));

// configure camera
DEVOK(device_manager_select(dm,
DeviceKind_Camera,
SIZED("simulated.*empty.*") - 1,
&props.video[0].camera.identifier));

// These settings are chosen to exercise the 8-byte alignment constraint.
props.video[0].camera.settings.binning = 1;
props.video[0].camera.settings.pixel_type = SampleType_u8;
props.video[0].camera.settings.shape = {
.x = 33,
.y = 47,
};

// configure acquisition
props.video[0].max_frame_count = 10;

// configure storage
DEVOK(device_manager_select(dm,
DeviceKind_Storage,
SIZED("trash") - 1,
&props.video[0].storage.identifier));
storage_properties_init(
&props.video[0].storage.settings, 0, nullptr, 0, nullptr, 0, { 0 }, 0);

OK(acquire_configure(runtime, &props));
storage_properties_destroy(&props.video[0].storage.settings);
}

static size_t
align_up(size_t n, size_t align)
{
return (n + align - 1) & ~(align - 1);
}

void
acquire(AcquireRuntime* runtime)
{
CHECK(runtime);

AcquireProperties props = {};
OK(acquire_get_configuration(runtime, &props));

const auto next = [](VideoFrame* cur) -> VideoFrame* {
return (VideoFrame*)(((uint8_t*)cur) +
align_up(cur->bytes_of_frame, 8));
};

const auto consumed_bytes = [](const VideoFrame* const cur,
const VideoFrame* const end) -> size_t {
return (uint8_t*)end - (uint8_t*)cur;
};

struct clock clock
{};
// expected time to acquire frames + 100%
static double time_limit_ms = props.video[0].max_frame_count * 1000.0 * 3.0;
clock_init(&clock);
clock_shift_ms(&clock, time_limit_ms);
OK(acquire_start(runtime));
{
uint64_t nframes = 0;
while (nframes < props.video[0].max_frame_count) {
struct clock throttle
{};
clock_init(&throttle);
EXPECT(clock_cmp_now(&clock) < 0,
"Timeout at %f ms",
clock_toc_ms(&clock) + time_limit_ms);

VideoFrame *beg, *end, *cur;
OK(acquire_map_read(runtime, 0, &beg, &end));

for (cur = beg; cur < end; cur = next(cur)) {
LOG("stream %d counting frame w id %d", 0, cur->frame_id);

const size_t unpadded_bytes =
bytes_of_image(&cur->shape) + sizeof(*cur);
const size_t nbytes_aligned = 8*((unpadded_bytes+7)/8);

// check data is correct
CHECK(bytes_of_image(&cur->shape) == 33 * 47);
CHECK(cur->bytes_of_frame == nbytes_aligned);
CHECK(cur->frame_id == nframes);
CHECK(cur->shape.dims.width ==
props.video[0].camera.settings.shape.x);
CHECK(cur->shape.dims.height ==
props.video[0].camera.settings.shape.y);

// check pointer is aligned
CHECK((size_t)cur % 8 == 0);
++nframes;
}

{
const auto n = (uint32_t)consumed_bytes(beg, end);
CHECK(n % 8 == 0);
OK(acquire_unmap_read(runtime, 0, n));
if (n)
LOG("stream %d consumed bytes %d", 0, n);
}
clock_sleep_ms(&throttle, 100.0f);

LOG("stream %d nframes %d. remaining time %f s",
0,
nframes,
-1e-3 * clock_toc_ms(&clock));
}

CHECK(nframes == props.video[0].max_frame_count);
}

OK(acquire_stop(runtime));
}

int
main()
{
int retval = 1;
AcquireRuntime* runtime = acquire_init(reporter);

try {
configure(runtime);
acquire(runtime);
retval = 0;
} catch (const std::exception& e) {
ERR("Exception: %s", e.what());
}

acquire_shutdown(runtime);
return retval;
}
54 changes: 43 additions & 11 deletions acquire-video-runtime/tests/one-video-stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "acquire.h"
#include "device/hal/device.manager.h"
#include "device/props/components.h"
#include "platform.h"
#include "logger.h"

Expand All @@ -25,6 +26,12 @@ reporter(int is_error,
msg);
}

static size_t
bytes_of_frame(const VideoFrame* frame)
{
return sizeof(*frame) + bytes_of_image(&frame->shape);
}

/// Helper for passing size static strings as function args.
/// For a function: `f(char*,size_t)` use `f(SIZED("hello"))`.
/// Expands to `f("hello",5)`.
Expand All @@ -46,13 +53,12 @@ reporter(int is_error,
#define DEVOK(e) CHECK(Device_Ok == (e))
#define OK(e) CHECK(AcquireStatus_Ok == (e))

int
main()
void
configure(AcquireRuntime* runtime)
{

auto runtime = acquire_init(reporter);
auto dm = acquire_device_manager(runtime);
CHECK(runtime);

const DeviceManager* dm = acquire_device_manager(runtime);
CHECK(dm);

AcquireProperties props = {};
Expand Down Expand Up @@ -84,18 +90,27 @@ main()
props.video[0].max_frame_count = 10;

OK(acquire_configure(runtime, &props));
storage_properties_destroy(&props.video[0].storage.settings);
}

void
acquire(AcquireRuntime* runtime)
{
CHECK(runtime);

AcquireProperties props = {};
OK(acquire_get_configuration(runtime, &props));

const auto next = [](VideoFrame* cur) -> VideoFrame* {
return (VideoFrame*)(((uint8_t*)cur) + cur->bytes_of_frame);
return (VideoFrame*)(((uint8_t*)cur) + bytes_of_frame(cur));
};

const auto consumed_bytes = [](const VideoFrame* const cur,
const VideoFrame* const end) -> size_t {
return (uint8_t*)end - (uint8_t*)cur;
};

struct clock clock
{};
struct clock clock = {};
// expected time to acquire frames + 100%
static double time_limit_ms =
(props.video[0].max_frame_count / 6.0) * 1000.0 * 2.0;
Expand Down Expand Up @@ -137,9 +152,26 @@ main()

CHECK(nframes == props.video[0].max_frame_count);
}
storage_properties_destroy(&props.video[0].storage.settings);

OK(acquire_stop(runtime));
OK(acquire_shutdown(runtime));
return 0;
}

int
main()
{
int retval = 1;
AcquireRuntime* runtime = acquire_init(reporter);

try {
configure(runtime);
acquire(runtime);
retval = 0;
} catch (const std::exception& e) {
ERR("Failed to configure runtime: %s", e.what());
} catch (...) {
ERR("Failed to configure runtime: unknown error");
}

acquire_shutdown(runtime);
return retval;
}

0 comments on commit 7afe044

Please sign in to comment.