joshdoe / gst-plugins-vision

GStreamer plugins related to the field of machine vision
Other
134 stars 50 forks source link

pylonsrc: exposure can't be changed while plugin is live #47

Closed joshdoe closed 3 years ago

joshdoe commented 3 years ago

As I commented on #46 here, I was having issues changing the exposure while the camera was connected. I needed to change three things to make this work in my application:

  1. My camera had the feature name ExposureTimeAbs instead of ExposureTime, I'm not sure if this is associated with my specific camera (acA1920-25gm) or Pylon version (5).
  2. It seems there may be a typo in propAutoFeature which causes the ExposureTime not to be set.
  3. By default, auto exposure is off, however with the way things work in #46, it still refuses to set the exposure time.

The below diff fixes this for me, but I'd like to have the author of #46 comment (@mrstecklo).

diff --git a/sys/pylon/gstpylonsrc.c b/sys/pylon/gstpylonsrc.c
index e1c808f..36c143c 100644
--- a/sys/pylon/gstpylonsrc.c
+++ b/sys/pylon/gstpylonsrc.c
@@ -193,10 +193,10 @@ G_STATIC_ASSERT (AUTOF_NUM_LIMITED == GST_PYLONSRC_NUM_LIMITED_FEATURES);
 static const char *const featAutoFeature[AUTOF_NUM_FEATURES] =
     { "GainAuto", "ExposureAuto", "BalanceWhiteAuto" };
 static const GST_PYLONSRC_PROP propAutoFeature[AUTOF_NUM_FEATURES] =
-    { PROP_GAIN, PROP_EXPOSURE, PROP_AUTOWHITEBALANCE };
+    { PROP_AUTOGAIN, PROP_AUTOEXPOSURE, PROP_AUTOWHITEBALANCE };
 // Yes,there is no "WhiteBalance" feature, it is only used for logging
 static const char *const featManualFeature[AUTOF_NUM_FEATURES] =
-    { "Gain", "ExposureTime", "WhiteBalance" };
+    { "GainRaw", "ExposureTimeAbs", "WhiteBalance" };
 static const char *const featLimitedLower[AUTOF_NUM_LIMITED] =
     { "AutoGainLowerLimit", "AutoExposureTimeLowerLimit" };
 static const char *const featLimitedUpper[AUTOF_NUM_LIMITED] =
@@ -779,6 +779,21 @@ set_prop_implicitly (GObject * object, GST_PYLONSRC_PROP prop,
   }
 }

+// Use in gst_pylonsrc_set_property to set related string property
+static inline void
+set_string_prop_implicitly(GObject* object, GST_PYLONSRC_PROP prop,
+  GParamSpec* pspec, const gchar * str_val)
+{
+  GstPylonSrc* src = GST_PYLONSRC(object);
+  if (!is_prop_set(src, prop)) {
+    GValue val = G_VALUE_INIT;
+    g_value_init(&val, G_TYPE_STRING);
+    g_value_set_string(&val, str_val);
+    gst_pylonsrc_set_property(object, prop, &val, pspec);
+    g_value_unset(&val);
+  }
+}
+
 /* plugin's parameters/properties */
 void
 gst_pylonsrc_set_property (GObject * object, guint property_id,
@@ -934,9 +949,12 @@ gst_pylonsrc_set_property (GObject * object, guint property_id,
       break;
     case PROP_EXPOSURE:
       src->limitedFeature[AUTOF_EXPOSURE].manual = g_value_get_double (value);
+      // disable autoexposure unless set explicitly
+      set_string_prop_implicitly(object, PROP_AUTOEXPOSURE, pspec, "off");
       break;
     case PROP_GAIN:
       src->limitedFeature[AUTOF_GAIN].manual = g_value_get_double (value);
+      set_string_prop_implicitly(object, PROP_AUTOGAIN, pspec, "off");
       break;
     case PROP_BLACKLEVEL:
       src->blacklevel = g_value_get_double (value);
mrstecklo commented 3 years ago

Surely a typo in propAutoFeature. As I said ExposureTimeAbs and ExposureTime are probably equivalent (AcquisitionFrameRate and AcquisitionFrameRateAbs too), while GainRaw and Gain are not. Names are related with cameras, not Pylon version. We need to find a generic way for these duplicates.

If set_string_prop_implicitly() fixes setting exposure then it's a way to go. I'll take a closer look on why you could not set exposure with default auto exposure next week.

mrstecklo commented 3 years ago

Probably you were unable to set Exposure because of the typo. gst_pylonsrc_set_auto_feature resets flag for PROP_EXPOSURE and corresponding section of gst_pylonsrc_set_manual_feature is never invoked.

static _Bool
gst_pylonsrc_set_auto_feature (GstPylonSrc * src,
    GST_PYLONSRC_AUTOFEATURE feature)
{
  if (is_prop_implicit (src, propAutoFeature[feature])) {
    // Enable/disable automatic feature
    ...
    reset_prop (src, propAutoFeature[feature]);
  }

  return TRUE;
error:
  return FALSE;
}
static _Bool
gst_pylonsrc_set_manual_feature (GstPylonSrc * src,
    GST_PYLONSRC_AUTOFEATURE feature)
{
  if (feature >= AUTOF_NUM_LIMITED) {
    GST_WARNING_OBJECT (src,
        "Trying to set manual value for unsupported autofeature: %d",
        (int) feature);
  } else {
    // Configure exposure/gain
    if (is_prop_implicit (src, propManualFeature[feature])) {
      ...
      reset_prop (src, propManualFeature[feature]);
    }
  }

  return TRUE;

error:
  return FALSE;
}
mrstecklo commented 3 years ago

set_string_prop_implicitly() is still useful. As setting manual exposure/gain implies turning auto function off. Not sure about white balance though. There are plenty of manual features behind auto white balance. Those should all be adjusted before disabling auto. Better to leave as is

joshdoe commented 3 years ago

This should be handled by now.