Fix LU-15: Broken ROI image

This fix contains two changes: 1. We check that passed ROI requests are
multiples of possible ROI steps as provided by the camera. If this is not the
case, the request is ignored and a warning issued. 2. We added two new base
properties ROI_WIDTH_MULTIPLIER and ROI_HEIGHT_MULTIPLIER that expose this
information to client programs.

4 files changed, 62 insertions, 7 deletions

diff --git a/src/cameras/uca-pco-camera.c b/src/cameras/uca-pco-camera.c
index 52246ae..87167cf 100644
--- a/src/cameras/uca-pco-camera.c
+++ b/src/cameras/uca-pco-camera.c
@@ -128,6 +128,8 @@ static gint base_overrideables[] = {
     PROP_ROI_Y,
     PROP_ROI_WIDTH,
     PROP_ROI_HEIGHT,
+    PROP_ROI_WIDTH_MULTIPLIER,
+    PROP_ROI_HEIGHT_MULTIPLIER,
     PROP_HAS_STREAMING,
     PROP_HAS_CAMRAM_RECORDING,
     0
@@ -165,6 +167,7 @@ struct _UcaPcoCameraPrivate {
     guint16 binning_h, binning_v;
     guint16 roi_x, roi_y;
     guint16 roi_width, roi_height;
+    guint16 roi_horizontal_steps, roi_vertical_steps;
     GValueArray *horizontal_binnings;
     GValueArray *vertical_binnings;
     GValueArray *pixelrates;
@@ -329,6 +332,8 @@ UcaPcoCamera *uca_pco_camera_new(GError **error)
 
     guint16 roi[4];
     pco_get_roi(priv->pco, roi);
+    pco_get_roi_steps(priv->pco, &priv->roi_horizontal_steps, &priv->roi_vertical_steps);
+
     priv->roi_x = roi[0] - 1;
     priv->roi_y = roi[1] - 1;
     priv->roi_width = roi[2] - roi[0] + 1;
@@ -464,7 +469,12 @@ static void uca_pco_camera_start_recording(UcaCamera *camera, GError **error)
      * All parameters are valid. Now, set them on the camera.
      */
     guint16 roi[4] = { priv->roi_x + 1, priv->roi_y + 1, priv->roi_x + priv->roi_width, priv->roi_y + priv->roi_height };
-    pco_set_roi(priv->pco, roi);
+
+    if (pco_set_roi(priv->pco, roi) != PCO_NOERROR) {
+        g_set_error(error, UCA_PCO_CAMERA_ERROR, UCA_PCO_CAMERA_ERROR_LIBPCO_GENERAL,
+                "Could not set ROI via pco_set_roi()");
+        return;
+    }
 
     g_object_get(G_OBJECT(camera), "transfer-asynchronously", &transfer_async, NULL);
 
@@ -635,11 +645,25 @@ static void uca_pco_camera_set_property(GObject *object, guint property_id, cons
             break;
 
         case PROP_ROI_WIDTH:
-            priv->roi_width = g_value_get_uint(value);
+            {
+                guint width = g_value_get_uint(value);
+                
+                if (width % priv->roi_horizontal_steps)
+                    g_warning("ROI width %i is not a multiple of %i", width, priv->roi_horizontal_steps);
+                else
+                    priv->roi_width = width;
+            }
             break;
 
         case PROP_ROI_HEIGHT:
-            priv->roi_height = g_value_get_uint(value);
+            {
+                guint height = g_value_get_uint(value);
+                
+                if (height % priv->roi_vertical_steps)
+                    g_warning("ROI height %i is not a multiple of %i", height, priv->roi_vertical_steps);
+                else
+                    priv->roi_height = height;
+            }
             break;
 
         case PROP_SENSOR_HORIZONTAL_BINNING:
@@ -1048,6 +1072,14 @@ static void uca_pco_camera_get_property(GObject *object, guint property_id, GVal
             g_value_set_uint(value, priv->roi_height);
             break;
 
+        case PROP_ROI_WIDTH_MULTIPLIER:
+            g_value_set_uint(value, priv->roi_horizontal_steps);
+            break;
+
+        case PROP_ROI_HEIGHT_MULTIPLIER:
+            g_value_set_uint(value, priv->roi_vertical_steps);
+            break;
+
         case PROP_NAME: 
             {
                 gchar *name = NULL;
diff --git a/src/uca-camera.c b/src/uca-camera.c
index 9fa1c2c..1bf4e75 100644
--- a/src/uca-camera.c
+++ b/src/uca-camera.c
@@ -97,6 +97,8 @@ const gchar *uca_camera_props[N_BASE_PROPERTIES] = {
     "roi-y0",
     "roi-width",
     "roi-height",
+    "roi-width-multiplier",
+    "roi-height-multiplier",
     "has-streaming",
     "has-camram-recording",
     "transfer-asynchronously",
@@ -262,6 +264,20 @@ static void uca_camera_class_init(UcaCameraClass *klass)
             1, G_MAXUINT, 1,
             G_PARAM_READWRITE);
 
+    camera_properties[PROP_ROI_WIDTH_MULTIPLIER] =
+        g_param_spec_uint(uca_camera_props[PROP_ROI_WIDTH_MULTIPLIER],
+            "Horizontal ROI multiplier",
+            "Minimum possible step size of horizontal ROI",
+            1, G_MAXUINT, 1,
+            G_PARAM_READABLE);
+
+    camera_properties[PROP_ROI_HEIGHT_MULTIPLIER] =
+        g_param_spec_uint(uca_camera_props[PROP_ROI_HEIGHT_MULTIPLIER],
+            "Vertical ROI multiplier",
+            "Minimum possible step size of vertical ROI",
+            1, G_MAXUINT, 1,
+            G_PARAM_READABLE);
+
     camera_properties[PROP_EXPOSURE_TIME] =
         g_param_spec_double(uca_camera_props[PROP_EXPOSURE_TIME],
             "Exposure time in seconds",
diff --git a/src/uca-camera.h b/src/uca-camera.h
index e2887a0..8462b3d 100644
--- a/src/uca-camera.h
+++ b/src/uca-camera.h
@@ -66,6 +66,8 @@ enum {
     PROP_ROI_Y,
     PROP_ROI_WIDTH,
     PROP_ROI_HEIGHT,
+    PROP_ROI_WIDTH_MULTIPLIER,
+    PROP_ROI_HEIGHT_MULTIPLIER,
     PROP_HAS_STREAMING,
     PROP_HAS_CAMRAM_RECORDING,
 
diff --git a/test/grab.c b/test/grab.c
index c4b3f3c..a5f1cf5 100644
--- a/test/grab.c
+++ b/test/grab.c
@@ -36,7 +36,7 @@ int main(int argc, char *argv[])
     GError *error = NULL;
     (void) signal(SIGINT, sigint_handler);
     guint sensor_width, sensor_height, sensor_width_extended, sensor_height_extended;
-    guint roi_width, roi_height, roi_x, roi_y;
+    guint roi_width, roi_height, roi_x, roi_y, roi_width_multiplier, roi_height_multiplier;
     guint bits, sensor_rate;
     gchar *name;
 
@@ -62,13 +62,15 @@ int main(int argc, char *argv[])
             "roi-x0", 0,
             "roi-y0", 0,
             "sensor-extended", FALSE,
-            "roi-width", sensor_width,
+            "roi-width", 1000,
             "roi-height", sensor_height,
             NULL);
 
     g_object_get(G_OBJECT(camera),
             "roi-width", &roi_width,
             "roi-height", &roi_height,
+            "roi-width-multiplier", &roi_width_multiplier,
+            "roi-height-multiplier", &roi_height_multiplier,
             "roi-x0", &roi_x,
             "roi-y0", &roi_y,
             "sensor-bitdepth", &bits,
@@ -78,10 +80,13 @@ int main(int argc, char *argv[])
     g_print("Camera: %s\n", name);
     g_free(name);
 
-    g_print("Sensor: %ix%i px (extended: %ix%i), ROI %ix%i @ (%i, %i) and %i Hz\n", 
+    g_print("Sensor: %ix%i px (extended: %ix%i) @ %i Hz\n", 
             sensor_width, sensor_height, 
             sensor_width_extended, sensor_height_extended,
-            roi_width, roi_height, roi_x, roi_y, sensor_rate);
+            sensor_rate);
+
+    g_print("ROI: %ix%i @ (%i, %i), steps: %i, %i\n", 
+            roi_width, roi_height, roi_x, roi_y, roi_width_multiplier, roi_height_multiplier);
 
     const int pixel_size = bits == 8 ? 1 : 2;
     gpointer buffer = g_malloc0(roi_width * roi_height * pixel_size);