| Message ID | 20260324085612.14533-1-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Le mardi 24 mars 2026 à 08:53 +0000, David Plowman a écrit : > The sensor-config property may optionally be specified to give the > outputSize and bitDepth of the SensorConfiguration that is applied to > the camera configuration. For example, use > > libcamerasrc sensor-config="sensor/config,width=2304,height=1296,depth=10" > > to request the 10-bit 2304x1296 mode of a sensor. > > All three parameters, width, height and bit depth, must be present, or > it will issue a warning and revert to automatic mode selection (as if > there were no sensor-config at all). > > If a mode is requested that doesn't exist then camera configuration > will fail and the pipeline won't start. > > As future-proofing, the SensorConfiguration's binning and increment > parameters may be specified, though libcamera currently ignores them. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/300 > --- > src/gstreamer/gstlibcamerasrc.cpp | 61 +++++++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > index a7241e9b..c7c183dd 100644 > --- a/src/gstreamer/gstlibcamerasrc.cpp > +++ b/src/gstreamer/gstlibcamerasrc.cpp > @@ -141,6 +141,7 @@ struct _GstLibcameraSrc { > GstTask *task; > > gchar *camera_name; > + GstStructure *sensor_config; > > std::atomic<GstEvent *> pending_eos; > > @@ -152,6 +153,7 @@ struct _GstLibcameraSrc { > enum { > PROP_0, > PROP_CAMERA_NAME, > + PROP_SENSOR_CONFIG, > PROP_LAST > }; > > @@ -846,6 +848,44 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > } > g_assert(state->config_->size() == state->srcpads_.size()); > > + /* Apply optional sensor configuration. */ > + if (self->sensor_config) { > + gint w = 0, h = 0, depth = 0; > + if (!gst_structure_get_int(self->sensor_config, "width", &w) || > + !gst_structure_get_int(self->sensor_config, "height", &h) || > + !gst_structure_get_int(self->sensor_config, "depth", &depth) || > + w <= 0 || h <= 0 || depth <= 0) { > + GST_ELEMENT_WARNING(self, RESOURCE, SETTINGS, > + ("sensor-config requires non-zero width, height and depth" > + " fields, ignoring"), > + (nullptr)); > + } else { > + SensorConfiguration sensorCfg; > + sensorCfg.outputSize = Size(w, h); > + sensorCfg.bitDepth = depth; > + > + /* Optional binning (defaults to 1x1). */ > + gint binX = 0, binY = 0; It seems that you don't need to initialize these, leave it like this if the compiler complains of course. > + if (gst_structure_get_int(self->sensor_config, "bin-x", &binX) && binX > 0) > + sensorCfg.binning.binX = binX; > + if (gst_structure_get_int(self->sensor_config, "bin-y", &binY) && binY > 0) > + sensorCfg.binning.binY = binY; > + > + /* Optional skipping (defaults to 1 for all increments). */ > + gint xOddInc = 0, xEvenInc = 0, yOddInc = 0, yEvenInc = 0; Same. > + if (gst_structure_get_int(self->sensor_config, "x-odd-inc", &xOddInc) && xOddInc > 0) > + sensorCfg.skipping.xOddInc = xOddInc; > + if (gst_structure_get_int(self->sensor_config, "x-even-inc", &xEvenInc) && xEvenInc > 0) > + sensorCfg.skipping.xEvenInc = xEvenInc; > + if (gst_structure_get_int(self->sensor_config, "y-odd-inc", &yOddInc) && yOddInc > 0) > + sensorCfg.skipping.yOddInc = yOddInc; > + if (gst_structure_get_int(self->sensor_config, "y-even-inc", &yEvenInc) && yEvenInc > 0) > + sensorCfg.skipping.yEvenInc = yEvenInc; > + > + state->config_->sensorConfig = sensorCfg; > + } > + } > + > if (!gst_libcamera_src_negotiate(self)) { > state->initControls_.clear(); > GST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED); > @@ -934,6 +974,10 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id, > g_free(self->camera_name); > self->camera_name = g_value_dup_string(value); > break; > + case PROP_SENSOR_CONFIG: > + g_clear_pointer(&self->sensor_config, gst_structure_free); > + self->sensor_config = gst_structure_copy((GstStructure *)g_value_get_boxed(value)); self->sensor_config = (GstStructure *)g_value_dup_boxed(value); That also fixes the assert when you try to set it back to NULL, since g_value_dup_boxed() will simply return NULL where gst_structure_copy() uses a failsafe assert (still works, but looks ugly to see asserts). > + break; > default: > if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec)) > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); > @@ -953,6 +997,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value, > case PROP_CAMERA_NAME: > g_value_set_string(value, self->camera_name); > break; > + case PROP_SENSOR_CONFIG: > + g_value_set_boxed(value, self->sensor_config); > + break; > default: > if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec)) > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); > @@ -1040,6 +1087,7 @@ gst_libcamera_src_finalize(GObject *object) > g_clear_object(&self->task); > g_mutex_clear(&self->state->lock_); > g_free(self->camera_name); > + g_clear_pointer(&self->sensor_config, gst_structure_free); > delete self->state; > > return klass->finalize(object); > @@ -1162,6 +1210,19 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass) > | G_PARAM_STATIC_STRINGS)); > g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec); > > + spec = g_param_spec_boxed("sensor-config", "Sensor Config", > + "Desired sensor configuration as a GstStructure with mandatory " > + "fields width, height and depth, and optional fields bin-x, bin-y, " > + "x-odd-inc, x-even-inc, y-odd-inc, y-even-inc " > + "(e.g. \"sensor/config,width=2304,height=1296,depth=10\"). " > + "Leave unset to let the pipeline negotiate the sensor mode automatically.", > + GST_TYPE_STRUCTURE, > + (GParamFlags)(GST_PARAM_MUTABLE_READY > + | G_PARAM_CONSTRUCT > + | G_PARAM_READWRITE > + | G_PARAM_STATIC_STRINGS)); > + g_object_class_install_property(object_class, PROP_SENSOR_CONFIG, spec); > + > GstCameraControls::installProperties(object_class, PROP_LAST); > } > With these ajustements: Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> cheers, Nicolas
Hi Nicolas On Tue, 24 Mar 2026 at 19:44, Nicolas Dufresne <nicolas@ndufresne.ca> wrote: > > Le mardi 24 mars 2026 à 08:53 +0000, David Plowman a écrit : > > The sensor-config property may optionally be specified to give the > > outputSize and bitDepth of the SensorConfiguration that is applied to > > the camera configuration. For example, use > > > > libcamerasrc sensor-config="sensor/config,width=2304,height=1296,depth=10" > > > > to request the 10-bit 2304x1296 mode of a sensor. > > > > All three parameters, width, height and bit depth, must be present, or > > it will issue a warning and revert to automatic mode selection (as if > > there were no sensor-config at all). > > > > If a mode is requested that doesn't exist then camera configuration > > will fail and the pipeline won't start. > > > > As future-proofing, the SensorConfiguration's binning and increment > > parameters may be specified, though libcamera currently ignores them. > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/300 > > --- > > src/gstreamer/gstlibcamerasrc.cpp | 61 +++++++++++++++++++++++++++++++ > > 1 file changed, 61 insertions(+) > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > > index a7241e9b..c7c183dd 100644 > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > @@ -141,6 +141,7 @@ struct _GstLibcameraSrc { > > GstTask *task; > > > > gchar *camera_name; > > + GstStructure *sensor_config; > > > > std::atomic<GstEvent *> pending_eos; > > > > @@ -152,6 +153,7 @@ struct _GstLibcameraSrc { > > enum { > > PROP_0, > > PROP_CAMERA_NAME, > > + PROP_SENSOR_CONFIG, > > PROP_LAST > > }; > > > > @@ -846,6 +848,44 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, > > } > > g_assert(state->config_->size() == state->srcpads_.size()); > > > > + /* Apply optional sensor configuration. */ > > + if (self->sensor_config) { > > + gint w = 0, h = 0, depth = 0; > > + if (!gst_structure_get_int(self->sensor_config, "width", &w) || > > + !gst_structure_get_int(self->sensor_config, "height", &h) || > > + !gst_structure_get_int(self->sensor_config, "depth", &depth) || > > + w <= 0 || h <= 0 || depth <= 0) { > > + GST_ELEMENT_WARNING(self, RESOURCE, SETTINGS, > > + ("sensor-config requires non-zero width, height and depth" > > + " fields, ignoring"), > > + (nullptr)); > > + } else { > > + SensorConfiguration sensorCfg; > > + sensorCfg.outputSize = Size(w, h); > > + sensorCfg.bitDepth = depth; > > + > > + /* Optional binning (defaults to 1x1). */ > > + gint binX = 0, binY = 0; > > It seems that you don't need to initialize these, leave it like this if the > compiler complains of course. No, the compiler seems happy so I'll make the change. > > > + if (gst_structure_get_int(self->sensor_config, "bin-x", &binX) && binX > 0) > > + sensorCfg.binning.binX = binX; > > + if (gst_structure_get_int(self->sensor_config, "bin-y", &binY) && binY > 0) > > + sensorCfg.binning.binY = binY; > > + > > + /* Optional skipping (defaults to 1 for all increments). */ > > + gint xOddInc = 0, xEvenInc = 0, yOddInc = 0, yEvenInc = 0; > > Same. Same! > > > + if (gst_structure_get_int(self->sensor_config, "x-odd-inc", &xOddInc) && xOddInc > 0) > > + sensorCfg.skipping.xOddInc = xOddInc; > > + if (gst_structure_get_int(self->sensor_config, "x-even-inc", &xEvenInc) && xEvenInc > 0) > > + sensorCfg.skipping.xEvenInc = xEvenInc; > > + if (gst_structure_get_int(self->sensor_config, "y-odd-inc", &yOddInc) && yOddInc > 0) > > + sensorCfg.skipping.yOddInc = yOddInc; > > + if (gst_structure_get_int(self->sensor_config, "y-even-inc", &yEvenInc) && yEvenInc > 0) > > + sensorCfg.skipping.yEvenInc = yEvenInc; > > + > > + state->config_->sensorConfig = sensorCfg; > > + } > > + } > > + > > if (!gst_libcamera_src_negotiate(self)) { > > state->initControls_.clear(); > > GST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED); > > @@ -934,6 +974,10 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id, > > g_free(self->camera_name); > > self->camera_name = g_value_dup_string(value); > > break; > > + case PROP_SENSOR_CONFIG: > > + g_clear_pointer(&self->sensor_config, gst_structure_free); > > + self->sensor_config = gst_structure_copy((GstStructure *)g_value_get_boxed(value)); > > self->sensor_config = (GstStructure *)g_value_dup_boxed(value); > > > That also fixes the assert when you try to set it back to NULL, since > g_value_dup_boxed() will simply return NULL where gst_structure_copy() uses a > failsafe assert (still works, but looks ugly to see asserts). Good to know. v3 incoming... > > > > + break; > > default: > > if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec)) > > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); > > @@ -953,6 +997,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value, > > case PROP_CAMERA_NAME: > > g_value_set_string(value, self->camera_name); > > break; > > + case PROP_SENSOR_CONFIG: > > + g_value_set_boxed(value, self->sensor_config); > > + break; > > default: > > if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec)) > > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); > > @@ -1040,6 +1087,7 @@ gst_libcamera_src_finalize(GObject *object) > > g_clear_object(&self->task); > > g_mutex_clear(&self->state->lock_); > > g_free(self->camera_name); > > + g_clear_pointer(&self->sensor_config, gst_structure_free); > > delete self->state; > > > > return klass->finalize(object); > > @@ -1162,6 +1210,19 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass) > > | G_PARAM_STATIC_STRINGS)); > > g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec); > > > > + spec = g_param_spec_boxed("sensor-config", "Sensor Config", > > + "Desired sensor configuration as a GstStructure with mandatory " > > + "fields width, height and depth, and optional fields bin-x, bin-y, " > > + "x-odd-inc, x-even-inc, y-odd-inc, y-even-inc " > > + "(e.g. \"sensor/config,width=2304,height=1296,depth=10\"). " > > + "Leave unset to let the pipeline negotiate the sensor mode automatically.", > > + GST_TYPE_STRUCTURE, > > + (GParamFlags)(GST_PARAM_MUTABLE_READY > > + | G_PARAM_CONSTRUCT > > + | G_PARAM_READWRITE > > + | G_PARAM_STATIC_STRINGS)); > > + g_object_class_install_property(object_class, PROP_SENSOR_CONFIG, spec); > > + > > GstCameraControls::installProperties(object_class, PROP_LAST); > > } > > > > With these ajustements: > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Thanks David > > cheers, > Nicolas
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index a7241e9b..c7c183dd 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -141,6 +141,7 @@ struct _GstLibcameraSrc { GstTask *task; gchar *camera_name; + GstStructure *sensor_config; std::atomic<GstEvent *> pending_eos; @@ -152,6 +153,7 @@ struct _GstLibcameraSrc { enum { PROP_0, PROP_CAMERA_NAME, + PROP_SENSOR_CONFIG, PROP_LAST }; @@ -846,6 +848,44 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread, } g_assert(state->config_->size() == state->srcpads_.size()); + /* Apply optional sensor configuration. */ + if (self->sensor_config) { + gint w = 0, h = 0, depth = 0; + if (!gst_structure_get_int(self->sensor_config, "width", &w) || + !gst_structure_get_int(self->sensor_config, "height", &h) || + !gst_structure_get_int(self->sensor_config, "depth", &depth) || + w <= 0 || h <= 0 || depth <= 0) { + GST_ELEMENT_WARNING(self, RESOURCE, SETTINGS, + ("sensor-config requires non-zero width, height and depth" + " fields, ignoring"), + (nullptr)); + } else { + SensorConfiguration sensorCfg; + sensorCfg.outputSize = Size(w, h); + sensorCfg.bitDepth = depth; + + /* Optional binning (defaults to 1x1). */ + gint binX = 0, binY = 0; + if (gst_structure_get_int(self->sensor_config, "bin-x", &binX) && binX > 0) + sensorCfg.binning.binX = binX; + if (gst_structure_get_int(self->sensor_config, "bin-y", &binY) && binY > 0) + sensorCfg.binning.binY = binY; + + /* Optional skipping (defaults to 1 for all increments). */ + gint xOddInc = 0, xEvenInc = 0, yOddInc = 0, yEvenInc = 0; + if (gst_structure_get_int(self->sensor_config, "x-odd-inc", &xOddInc) && xOddInc > 0) + sensorCfg.skipping.xOddInc = xOddInc; + if (gst_structure_get_int(self->sensor_config, "x-even-inc", &xEvenInc) && xEvenInc > 0) + sensorCfg.skipping.xEvenInc = xEvenInc; + if (gst_structure_get_int(self->sensor_config, "y-odd-inc", &yOddInc) && yOddInc > 0) + sensorCfg.skipping.yOddInc = yOddInc; + if (gst_structure_get_int(self->sensor_config, "y-even-inc", &yEvenInc) && yEvenInc > 0) + sensorCfg.skipping.yEvenInc = yEvenInc; + + state->config_->sensorConfig = sensorCfg; + } + } + if (!gst_libcamera_src_negotiate(self)) { state->initControls_.clear(); GST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED); @@ -934,6 +974,10 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id, g_free(self->camera_name); self->camera_name = g_value_dup_string(value); break; + case PROP_SENSOR_CONFIG: + g_clear_pointer(&self->sensor_config, gst_structure_free); + self->sensor_config = gst_structure_copy((GstStructure *)g_value_get_boxed(value)); + break; default: if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec)) G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); @@ -953,6 +997,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value, case PROP_CAMERA_NAME: g_value_set_string(value, self->camera_name); break; + case PROP_SENSOR_CONFIG: + g_value_set_boxed(value, self->sensor_config); + break; default: if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec)) G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); @@ -1040,6 +1087,7 @@ gst_libcamera_src_finalize(GObject *object) g_clear_object(&self->task); g_mutex_clear(&self->state->lock_); g_free(self->camera_name); + g_clear_pointer(&self->sensor_config, gst_structure_free); delete self->state; return klass->finalize(object); @@ -1162,6 +1210,19 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass) | G_PARAM_STATIC_STRINGS)); g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec); + spec = g_param_spec_boxed("sensor-config", "Sensor Config", + "Desired sensor configuration as a GstStructure with mandatory " + "fields width, height and depth, and optional fields bin-x, bin-y, " + "x-odd-inc, x-even-inc, y-odd-inc, y-even-inc " + "(e.g. \"sensor/config,width=2304,height=1296,depth=10\"). " + "Leave unset to let the pipeline negotiate the sensor mode automatically.", + GST_TYPE_STRUCTURE, + (GParamFlags)(GST_PARAM_MUTABLE_READY + | G_PARAM_CONSTRUCT + | G_PARAM_READWRITE + | G_PARAM_STATIC_STRINGS)); + g_object_class_install_property(object_class, PROP_SENSOR_CONFIG, spec); + GstCameraControls::installProperties(object_class, PROP_LAST); }
The sensor-config property may optionally be specified to give the outputSize and bitDepth of the SensorConfiguration that is applied to the camera configuration. For example, use libcamerasrc sensor-config="sensor/config,width=2304,height=1296,depth=10" to request the 10-bit 2304x1296 mode of a sensor. All three parameters, width, height and bit depth, must be present, or it will issue a warning and revert to automatic mode selection (as if there were no sensor-config at all). If a mode is requested that doesn't exist then camera configuration will fail and the pipeline won't start. As future-proofing, the SensorConfiguration's binning and increment parameters may be specified, though libcamera currently ignores them. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/300 --- src/gstreamer/gstlibcamerasrc.cpp | 61 +++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)