Skip to content

Commit

Permalink
Fix setting ROI (#86)
Browse files Browse the repository at this point in the history
closes #85 

adds test, updates changelog

Changes the ROI setting procedure so it's more robust:
1. Set offset to 0, so size can have full range of sensor.
2. Set the size
3. Set the offset

Sometimes the offset has to be divisible by 4. When setting a parameter
that is not appropriately divisible, the call just fails with an invalid
parameter which is not very helpful.

Fortunately, when running in "progressive" mode (the default for
light-sheet), there are fewer restrictions. I'm not exactly sure how to
query the restrictions, so the approach for this PR is to make sure
we're setting the mode early.
  • Loading branch information
nclack authored Aug 15, 2023
1 parent 8087240 commit 6c0af4a
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 13 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [0.1.6](https://github.com/acquire-project/acquire-driver-hdcam/compare/v0.1.5...v0.1.6) - 2023-08-10
## [0.1.6](https://github.com/acquire-project/acquire-driver-hdcam/compare/v0.1.5...v0.1.6) - 2023-08-15

- Fixes a bug where the output trigger source property was set when it shouldn't be.
- Fixes a bug where roi bound checking wasn't working.

## [0.1.5](https://github.com/acquire-project/acquire-driver-hdcam/compare/v0.1.4...v0.1.5) - 2023-07-28

Expand Down
35 changes: 25 additions & 10 deletions src/dcam.camera.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "dcam.camera.h"
#include "device/props/device.h"
#include "device/props/metadata.h"
#include "logger.h"

#include "dcam.error.h"
Expand Down Expand Up @@ -365,14 +366,14 @@ aq_dcam_get_metadata__inner(const struct Dcam4Camera* self,
is_ok &= READ_PROP_META(
&metadata->readout_direction, DCAM_IDPROP_READOUT_DIRECTION, 1.0f);

is_ok &=
READ_PROP_META(&metadata->offset.x, DCAM_IDPROP_SUBARRAYHPOS, 1.0f);
is_ok &=
READ_PROP_META(&metadata->offset.y, DCAM_IDPROP_SUBARRAYVPOS, 1.0f);
is_ok &=
READ_PROP_META(&metadata->shape.x, DCAM_IDPROP_SUBARRAYHSIZE, 1.0f);
is_ok &=
READ_PROP_META(&metadata->shape.y, DCAM_IDPROP_SUBARRAYVSIZE, 1.0f);
is_ok &=
READ_PROP_META(&metadata->offset.x, DCAM_IDPROP_SUBARRAYHPOS, 1.0f);
is_ok &=
READ_PROP_META(&metadata->offset.y, DCAM_IDPROP_SUBARRAYVPOS, 1.0f);

#undef READ_PROP_META

Expand Down Expand Up @@ -462,6 +463,11 @@ aq_dcam_set__inner(struct Dcam4Camera* self,

int is_ok = 1;

// Set readout speed and mode.
// It's important to do this first as it effects what settings are valid
// for downstream parameters (e.g. subarray, and exposure)
is_ok &= set_readout_speed(hdcam);

// pixel type
if (IS_CHANGED(pixel_type)) {
is_ok &= set_sample_type(hdcam, &props->pixel_type);
Expand Down Expand Up @@ -513,19 +519,28 @@ aq_dcam_set__inner(struct Dcam4Camera* self,
// roi
if (IS_CHANGED(offset.x) || IS_CHANGED(offset.y) || IS_CHANGED(shape.x) ||
IS_CHANGED(shape.y)) {
dcamprop_setvalue(hdcam, DCAM_IDPROP_SUBARRAYMODE, DCAMPROP_MODE__ON);
is_ok &=
prop_write(u32, hdcam, DCAM_IDPROP_SUBARRAYHPOS, &props->offset.x);
is_ok &=
prop_write(u32, hdcam, DCAM_IDPROP_SUBARRAYVPOS, &props->offset.y);
is_ok &= (dcamprop_setvalue(hdcam,
DCAM_IDPROP_SUBARRAYMODE,
DCAMPROP_MODE__ON) == DCAMERR_SUCCESS);

// temporarily set offset to (0,0) so we're free to change the shape
is_ok &= dcamprop_setvalue(hdcam, DCAM_IDPROP_SUBARRAYVPOS, 0.0f) ==
DCAMERR_SUCCESS;
is_ok &= dcamprop_setvalue(hdcam, DCAM_IDPROP_SUBARRAYHPOS, 0.0f) ==
DCAMERR_SUCCESS;

is_ok &=
prop_write(u32, hdcam, DCAM_IDPROP_SUBARRAYHSIZE, &props->shape.x);
is_ok &=
prop_write(u32, hdcam, DCAM_IDPROP_SUBARRAYVSIZE, &props->shape.y);

is_ok &=
prop_write(u32, hdcam, DCAM_IDPROP_SUBARRAYVPOS, &props->offset.y);
is_ok &=
prop_write(u32, hdcam, DCAM_IDPROP_SUBARRAYHPOS, &props->offset.x);
}

// exposure
is_ok &= set_readout_speed(hdcam);
if (IS_CHANGED(exposure_time_us) || IS_CHANGED(line_interval_us)) {
is_ok &= prop_write_scaled(f32,
hdcam,
Expand Down
5 changes: 3 additions & 2 deletions src/dcam.getset.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@ prop_write_u32(HDCAM hdcam,
const char* prop_name)
{
double v = *value;
DCAM(dcamprop_setgetvalue(hdcam, prop_id, &v, 0));
DCAMERR ecode;
DCAM(ecode = dcamprop_setgetvalue(hdcam, prop_id, &v, 0));
*value = (uint32_t)v;
return 1;
Error:
LOG("Failed to write %s", prop_name);
LOG("Failed to write %s.", prop_name);
return 0;
}

Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ else()
dcam-reset-on-fail
one-video-stream
set-output-trigger
set-roi
)

foreach(name ${tests})
Expand Down
99 changes: 99 additions & 0 deletions tests/set-roi.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
//! Test: Runs through setting up acquiring from a sub-rectangle of the sensor
#include "acquire.h"
#include "device/hal/device.manager.h"
#include "device/props/components.h"
#include "logger.h"
#include <stdio.h>
#include <string.h>

#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)) { \
ERR(__VA_ARGS__); \
goto Error; \
} \
} 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))

/// Check that a==b
/// example: `ASSERT_EQ(int,"%d",42,meaning_of_life())`
#define ASSERT_EQ(T, fmt, a, b) \
do { \
T a_ = (T)(a); \
T b_ = (T)(b); \
EXPECT(a_ == b_, "Expected %s==%s but " fmt "!=" fmt, #a, #b, a_, b_); \
} while (0)

#define countof(e) (sizeof(e) / sizeof(*(e)))

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);
}

int
setup(AcquireRuntime* runtime)
{
AcquireProperties props = {};

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

#define SIZED(s) s, sizeof(s) - 1
DEVOK(device_manager_select(dm,
DeviceKind_Camera,
SIZED("Hamamatsu C15440-20UP.*"),
&props.video[0].camera.identifier));
DEVOK(device_manager_select(dm,
DeviceKind_Storage,
SIZED("Trash"),
&props.video[0].storage.identifier));
#undef SIZED

props.video[0].max_frame_count = 10;
props.video[0].camera.settings.shape = { .x = 1700, .y = 512 };
props.video[0].camera.settings.offset = { .x = 304, .y = 896 };
OK(acquire_configure(runtime, &props));
// Expect that the roi is what we intended.

ASSERT_EQ(int, "%d", props.video[0].camera.settings.shape.x, 1700);
ASSERT_EQ(int, "%d", props.video[0].camera.settings.shape.y, 512);
ASSERT_EQ(int, "%d", props.video[0].camera.settings.offset.x, 304);
ASSERT_EQ(int, "%d", props.video[0].camera.settings.offset.y, 896);
return 1;
Error:
return 0;
}

int
main()
{
int ecode = 0;
AcquireRuntime* runtime = 0;
CHECK(runtime = acquire_init(reporter));
CHECK(setup(runtime));
OK(acquire_start(runtime));
OK(acquire_stop(runtime));
Finalize:
acquire_shutdown(runtime);
return ecode;
Error:
ecode = 1;
goto Finalize;
}

0 comments on commit 6c0af4a

Please sign in to comment.