[v3] gstreamer: Add sensor-config property
diff mbox series

Message ID 20260325094829.9119-1-david.plowman@raspberrypi.com
State Superseded
Headers show
Series
  • [v3] gstreamer: Add sensor-config property
Related show

Commit Message

David Plowman March 25, 2026, 9:47 a.m. UTC
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>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/300
---
 src/gstreamer/gstlibcamerasrc.cpp | 61 +++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

Comments

Umang Jain March 27, 2026, 3:44 p.m. UTC | #1
On 2026-03-25 15:17, David Plowman wrote:
> 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>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.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..482b4761 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, binY;
> +			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, xEvenInc, yOddInc, yEvenInc;
> +			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;

Please also look at https://patchwork.libcamera.org/patch/20080/#29567
reasoning as well.

Thanks,
Umang


> +		}
> +	}
> +
>  	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 = (GstStructure *)g_value_dup_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);
>  }
Nicolas Dufresne March 27, 2026, 3:59 p.m. UTC | #2
Le vendredi 27 mars 2026 à 21:14 +0530, Umang Jain a écrit :
> On 2026-03-25 15:17, David Plowman wrote:
> > 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>
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.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..482b4761 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, binY;
> > +			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, xEvenInc, yOddInc, yEvenInc;
> > +			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;
> 
> Please also look at https://patchwork.libcamera.org/patch/20080/#29567
> reasoning as well.

Let's cite it back, so we don't all have to follow a link:

From Laurent:
   Before we push this towards applications, I want the sensor
   configuration to be fully specified, including binning/skipping and the
   crop rectangles. Our current API to configure the sensor isn't good
   enough.
   
Which does not match with the current API anymore, since binning, skipping and
crop is in the API. Only thing I missed is that there is that analogCrop is
missing in the implementation here, I guess its easy to fix.

Is that all you'd like to see ? The one not being written at the one have have
defaults in the documentation (basically the configuration that you typically
want to leave disabled). I don't see that as not being fully specified though,
its just a short cut that disable == not in the GstStructure.

Nicolas

> 
> Thanks,
> Umang
> 
> 
> > +		}
> > +	}
> > +
> >  	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 = (GstStructure *)g_value_dup_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);
> >  }
David Plowman March 27, 2026, 4:27 p.m. UTC | #3
HI

Thanks for pointing that out, Umang.

Obviously it's basically the same thing. In the version I did I
required all of width, height and bit-depth to be present if you use
the option, which I think is reasonable seeing as the API forces that
on you anyway. I also left the binning/skipping optional on the
grounds that specifying them feels a bit onerous given that they're
all ignored (and are assigned default values anyway).

As Nicolas says, we could add the analogCrop. Make that optional for
the same reasons?

In gstreamer-world, would there be a "right way" to present that
analogCrop parameter in terms of syntax? (Asking for a friend who's
not so familiar with gstreamer!)

Thanks
David

On Fri, 27 Mar 2026 at 15:59, Nicolas Dufresne
<nicolas.dufresne@collabora.com> wrote:
>
> Le vendredi 27 mars 2026 à 21:14 +0530, Umang Jain a écrit :
> > On 2026-03-25 15:17, David Plowman wrote:
> > > 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>
> > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.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..482b4761 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, binY;
> > > +                   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, xEvenInc, yOddInc, yEvenInc;
> > > +                   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;
> >
> > Please also look at https://patchwork.libcamera.org/patch/20080/#29567
> > reasoning as well.
>
> Let's cite it back, so we don't all have to follow a link:
>
> From Laurent:
>    Before we push this towards applications, I want the sensor
>    configuration to be fully specified, including binning/skipping and the
>    crop rectangles. Our current API to configure the sensor isn't good
>    enough.
>
> Which does not match with the current API anymore, since binning, skipping and
> crop is in the API. Only thing I missed is that there is that analogCrop is
> missing in the implementation here, I guess its easy to fix.
>
> Is that all you'd like to see ? The one not being written at the one have have
> defaults in the documentation (basically the configuration that you typically
> want to leave disabled). I don't see that as not being fully specified though,
> its just a short cut that disable == not in the GstStructure.
>
> Nicolas
>
> >
> > Thanks,
> > Umang
> >
> >
> > > +           }
> > > +   }
> > > +
> > >     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 = (GstStructure *)g_value_dup_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);
> > >  }
Nicolas Dufresne March 27, 2026, 5:12 p.m. UTC | #4
Hi,

Le vendredi 27 mars 2026 à 16:27 +0000, David Plowman a écrit :
> In gstreamer-world, would there be a "right way" to present that
> analogCrop parameter in terms of syntax? (Asking for a friend who's
> not so familiar with gstreamer!)

If its cropped by the sensor, in an "analog way", its not something we care in
GStreamer. All we can do is add "analog-crop-x/y/width/height" (assuming this is
the the Rectangle class is) in the GstStructure for the configuration, leave it
unset by default.

Nicolas
David Plowman March 30, 2026, 8:30 a.m. UTC | #5
Thanks. I'll post a (final??) version that allows an optional analog
crop rectangle.

David

On Fri, 27 Mar 2026 at 17:12, Nicolas Dufresne
<nicolas.dufresne@collabora.com> wrote:
>
> Hi,
>
> Le vendredi 27 mars 2026 à 16:27 +0000, David Plowman a écrit :
> > In gstreamer-world, would there be a "right way" to present that
> > analogCrop parameter in terms of syntax? (Asking for a friend who's
> > not so familiar with gstreamer!)
>
> If its cropped by the sensor, in an "analog way", its not something we care in
> GStreamer. All we can do is add "analog-crop-x/y/width/height" (assuming this is
> the the Rectangle class is) in the GstStructure for the configuration, leave it
> unset by default.
>
> Nicolas

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index a7241e9b..482b4761 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, binY;
+			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, xEvenInc, yOddInc, yEvenInc;
+			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 = (GstStructure *)g_value_dup_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);
 }