[v1,1/1] gstreamer: Add sensor-config property
diff mbox series

Message ID 20260323092237.3103-2-david.plowman@raspberrypi.com
State New
Headers show
Series
  • Add sensor config support to gstreamer
Related show

Commit Message

David Plowman March 23, 2026, 9:02 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=2304x1296/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.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/gstreamer/gstlibcamerasrc.cpp | 38 +++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Comments

Barnabás Pőcze March 23, 2026, 9:51 a.m. UTC | #1
Hi

2026. 03. 23. 10:02 keltezéssel, David Plowman írta:
> 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=2304x1296/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.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---

I think you can add `Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/300`.

And have you seen this previous attempt?
https://patchwork.libcamera.org/cover/18458/


Regards,
Barnabás Pőcze


>   src/gstreamer/gstlibcamerasrc.cpp | 38 +++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index a7241e9b..9eca65d1 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -141,6 +141,7 @@ struct _GstLibcameraSrc {
>   	GstTask *task;
> 
>   	gchar *camera_name;
> +	gchar *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,23 @@ 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) {
> +		unsigned int w = 0, h = 0, depth = 0;
> +		if (std::sscanf(self->sensor_config, "%ux%u/%u", &w, &h, &depth) == 3 && w && h && depth) {
> +			SensorConfiguration sensorCfg;
> +			sensorCfg.outputSize = Size(w, h);
> +			sensorCfg.bitDepth = depth;
> +			state->config_->sensorConfig = sensorCfg;
> +		} else {
> +			GST_ELEMENT_WARNING(self, RESOURCE, SETTINGS,
> +					    ("Invalid sensor-config \"%s\", expected \"WIDTHxHEIGHT/DEPTH\","
> +					     " ignoring",
> +					     self->sensor_config),
> +					    (nullptr));
> +		}
> +	}
> +
>   	if (!gst_libcamera_src_negotiate(self)) {
>   		state->initControls_.clear();
>   		GST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED);
> @@ -934,6 +953,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_free(self->sensor_config);
> +		self->sensor_config = g_value_dup_string(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 +976,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_string(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 +1066,7 @@ gst_libcamera_src_finalize(GObject *object)
>   	g_clear_object(&self->task);
>   	g_mutex_clear(&self->state->lock_);
>   	g_free(self->camera_name);
> +	g_free(self->sensor_config);
>   	delete self->state;
> 
>   	return klass->finalize(object);
> @@ -1162,6 +1189,17 @@ 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_string("sensor-config", "Sensor Config",
> +				   "Desired sensor configuration as \"WIDTHxHEIGHT/BITDEPTH\""
> +				   " (e.g. \"2304x1296/10\"). Leave unset to let the pipeline"
> +				   " negotiate the sensor mode automatically.",
> +				   nullptr,
> +				   (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);
>   }
> 
> --
> 2.47.3
>
David Plowman March 23, 2026, 10:07 a.m. UTC | #2
Hi Barnabas

On Mon, 23 Mar 2026 at 09:51, Barnabás Pőcze
<barnabas.pocze@ideasonboard.com> wrote:
>
> Hi
>
> 2026. 03. 23. 10:02 keltezéssel, David Plowman írta:
> > 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=2304x1296/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.
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
>
> I think you can add `Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/300`.

Ah, I hadn't seen that. Yes, I think it does close that issue, so I
can add that.

>
> And have you seen this previous attempt?
> https://patchwork.libcamera.org/cover/18458/

The discussion here is a little bit broader, and there's a wish, for
example, to be able to select a mode using framerate. That's more
difficult, of course, because we can't easily enumerate everything
about all the different modes. The intent here was simply to follow
the original SensorConfiguration design - you get the numbers exactly
right, or it fails. For Pi users that's not so bad because it's easy
enough to look up what you want ("rpicam-hello --list-cameras"), but
more generally there remains a wider problem here.

Thanks for the info!
David

>
>
> Regards,
> Barnabás Pőcze
>
>
> >   src/gstreamer/gstlibcamerasrc.cpp | 38 +++++++++++++++++++++++++++++++
> >   1 file changed, 38 insertions(+)
> >
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index a7241e9b..9eca65d1 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -141,6 +141,7 @@ struct _GstLibcameraSrc {
> >       GstTask *task;
> >
> >       gchar *camera_name;
> > +     gchar *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,23 @@ 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) {
> > +             unsigned int w = 0, h = 0, depth = 0;
> > +             if (std::sscanf(self->sensor_config, "%ux%u/%u", &w, &h, &depth) == 3 && w && h && depth) {
> > +                     SensorConfiguration sensorCfg;
> > +                     sensorCfg.outputSize = Size(w, h);
> > +                     sensorCfg.bitDepth = depth;
> > +                     state->config_->sensorConfig = sensorCfg;
> > +             } else {
> > +                     GST_ELEMENT_WARNING(self, RESOURCE, SETTINGS,
> > +                                         ("Invalid sensor-config \"%s\", expected \"WIDTHxHEIGHT/DEPTH\","
> > +                                          " ignoring",
> > +                                          self->sensor_config),
> > +                                         (nullptr));
> > +             }
> > +     }
> > +
> >       if (!gst_libcamera_src_negotiate(self)) {
> >               state->initControls_.clear();
> >               GST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED);
> > @@ -934,6 +953,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_free(self->sensor_config);
> > +             self->sensor_config = g_value_dup_string(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 +976,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_string(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 +1066,7 @@ gst_libcamera_src_finalize(GObject *object)
> >       g_clear_object(&self->task);
> >       g_mutex_clear(&self->state->lock_);
> >       g_free(self->camera_name);
> > +     g_free(self->sensor_config);
> >       delete self->state;
> >
> >       return klass->finalize(object);
> > @@ -1162,6 +1189,17 @@ 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_string("sensor-config", "Sensor Config",
> > +                                "Desired sensor configuration as \"WIDTHxHEIGHT/BITDEPTH\""
> > +                                " (e.g. \"2304x1296/10\"). Leave unset to let the pipeline"
> > +                                " negotiate the sensor mode automatically.",
> > +                                nullptr,
> > +                                (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);
> >   }
> >
> > --
> > 2.47.3
> >
>
Nicolas Dufresne March 23, 2026, 12:56 p.m. UTC | #3
Hi David,

Le lundi 23 mars 2026 à 09:02 +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=2304x1296/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).

Any reason to not allow setting the binning and the skipping too ? 

> 
> If a mode is requested that doesn't exist then camera configuration
> will fail and the pipeline won't start.

That I'm ok with, as we discussed in past thread, the SensorConfig is not an
equivalent of the mode enumeration hacks from the libcamera-app (and my previous
attempts). Its a one time configuration for users that knows what they are
doing.

> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 38 +++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> b/src/gstreamer/gstlibcamerasrc.cpp
> index a7241e9b..9eca65d1 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -141,6 +141,7 @@ struct _GstLibcameraSrc {
>  	GstTask *task;
>  
>  	gchar *camera_name;
> +	gchar *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,23 @@ 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) {
> +		unsigned int w = 0, h = 0, depth = 0;
> +		if (std::sscanf(self->sensor_config, "%ux%u/%u", &w, &h,
> &depth) == 3 && w && h && depth) {
> +			SensorConfiguration sensorCfg;
> +			sensorCfg.outputSize = Size(w, h);
> +			sensorCfg.bitDepth = depth;
> +			state->config_->sensorConfig = sensorCfg;
> +		} else {
> +			GST_ELEMENT_WARNING(self, RESOURCE, SETTINGS,
> +					    ("Invalid sensor-config \"%s\",
> expected \"WIDTHxHEIGHT/DEPTH\","
> +					     " ignoring",
> +					     self->sensor_config),
> +					    (nullptr));
> +		}
> +	}
> +
>  	if (!gst_libcamera_src_negotiate(self)) {
>  		state->initControls_.clear();
>  		GST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED);
> @@ -934,6 +953,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_free(self->sensor_config);
> +		self->sensor_config = g_value_dup_string(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 +976,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_string(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 +1066,7 @@ gst_libcamera_src_finalize(GObject *object)
>  	g_clear_object(&self->task);
>  	g_mutex_clear(&self->state->lock_);
>  	g_free(self->camera_name);
> +	g_free(self->sensor_config);
>  	delete self->state;
>  
>  	return klass->finalize(object);
> @@ -1162,6 +1189,17 @@ 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_string("sensor-config", "Sensor Config",
> +				   "Desired sensor configuration as
> \"WIDTHxHEIGHT/BITDEPTH\""
> +				   " (e.g. \"2304x1296/10\"). Leave unset to
> let the pipeline"
> +				   " negotiate the sensor mode
> automatically.",
> +				   nullptr,
> +				   (GParamFlags)(GST_PARAM_MUTABLE_READY
> +						 | G_PARAM_CONSTRUCT
> +						 | G_PARAM_READWRITE
> +						 | G_PARAM_STATIC_STRINGS));

My recommendation would be to stay away from string parsing. As it is today, if
a new configuration parameter is added (and it is already missing binning and 
skipping parameters), maintaining backward compatibility will be a nightmare.

I'd like to suggest to use a GstStructure instead. This allow for key/value
pairs. You can make width/height/depth mandatory, while allowing for defaults
values on binning and skipping and future additions. Defaults to any future
additions will be required anyway to maintain backward compatibility at
libcamera level.

The GstStructure have full serialization support, the only odd part is the
structure name, which we can either ignore or fixate. So typically:

  sensor/config,width=X,height=Y,depth=Z

gst_util_set_object_arg() (or when used inside gst-launch for testing) can be
used to pass this as strings, but this is optional.

cheers,
Nicolas

> +	g_object_class_install_property(object_class, PROP_SENSOR_CONFIG,
> spec);
> +
>  	GstCameraControls::installProperties(object_class, PROP_LAST);
>  }
>
David Plowman March 23, 2026, 4:12 p.m. UTC | #4
Hi Nicolas

Thanks for the comments!

On Mon, 23 Mar 2026 at 12:56, Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Hi David,
>
> Le lundi 23 mars 2026 à 09:02 +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=2304x1296/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).
>
> Any reason to not allow setting the binning and the skipping too ?

I guess because they're not used and I'm a bit lazy! But I can add
them, and it sounds like it might be easier if I switch over to this
GstStructure.

>
> >
> > If a mode is requested that doesn't exist then camera configuration
> > will fail and the pipeline won't start.
>
> That I'm ok with, as we discussed in past thread, the SensorConfig is not an
> equivalent of the mode enumeration hacks from the libcamera-app (and my previous
> attempts). Its a one time configuration for users that knows what they are
> doing.
>
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  src/gstreamer/gstlibcamerasrc.cpp | 38 +++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> > b/src/gstreamer/gstlibcamerasrc.cpp
> > index a7241e9b..9eca65d1 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -141,6 +141,7 @@ struct _GstLibcameraSrc {
> >       GstTask *task;
> >
> >       gchar *camera_name;
> > +     gchar *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,23 @@ 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) {
> > +             unsigned int w = 0, h = 0, depth = 0;
> > +             if (std::sscanf(self->sensor_config, "%ux%u/%u", &w, &h,
> > &depth) == 3 && w && h && depth) {
> > +                     SensorConfiguration sensorCfg;
> > +                     sensorCfg.outputSize = Size(w, h);
> > +                     sensorCfg.bitDepth = depth;
> > +                     state->config_->sensorConfig = sensorCfg;
> > +             } else {
> > +                     GST_ELEMENT_WARNING(self, RESOURCE, SETTINGS,
> > +                                         ("Invalid sensor-config \"%s\",
> > expected \"WIDTHxHEIGHT/DEPTH\","
> > +                                          " ignoring",
> > +                                          self->sensor_config),
> > +                                         (nullptr));
> > +             }
> > +     }
> > +
> >       if (!gst_libcamera_src_negotiate(self)) {
> >               state->initControls_.clear();
> >               GST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED);
> > @@ -934,6 +953,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_free(self->sensor_config);
> > +             self->sensor_config = g_value_dup_string(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 +976,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_string(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 +1066,7 @@ gst_libcamera_src_finalize(GObject *object)
> >       g_clear_object(&self->task);
> >       g_mutex_clear(&self->state->lock_);
> >       g_free(self->camera_name);
> > +     g_free(self->sensor_config);
> >       delete self->state;
> >
> >       return klass->finalize(object);
> > @@ -1162,6 +1189,17 @@ 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_string("sensor-config", "Sensor Config",
> > +                                "Desired sensor configuration as
> > \"WIDTHxHEIGHT/BITDEPTH\""
> > +                                " (e.g. \"2304x1296/10\"). Leave unset to
> > let the pipeline"
> > +                                " negotiate the sensor mode
> > automatically.",
> > +                                nullptr,
> > +                                (GParamFlags)(GST_PARAM_MUTABLE_READY
> > +                                              | G_PARAM_CONSTRUCT
> > +                                              | G_PARAM_READWRITE
> > +                                              | G_PARAM_STATIC_STRINGS));
>
> My recommendation would be to stay away from string parsing. As it is today, if
> a new configuration parameter is added (and it is already missing binning and
> skipping parameters), maintaining backward compatibility will be a nightmare.
>
> I'd like to suggest to use a GstStructure instead. This allow for key/value
> pairs. You can make width/height/depth mandatory, while allowing for defaults
> values on binning and skipping and future additions. Defaults to any future
> additions will be required anyway to maintain backward compatibility at
> libcamera level.
>
> The GstStructure have full serialization support, the only odd part is the
> structure name, which we can either ignore or fixate. So typically:
>
>   sensor/config,width=X,height=Y,depth=Z
>
> gst_util_set_object_arg() (or when used inside gst-launch for testing) can be
> used to pass this as strings, but this is optional.

OK, that sounds good. I'll look that up and post a v2.

Thanks!
David

>
> cheers,
> Nicolas
>
> > +     g_object_class_install_property(object_class, PROP_SENSOR_CONFIG,
> > spec);
> > +
> >       GstCameraControls::installProperties(object_class, PROP_LAST);
> >  }
> >

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index a7241e9b..9eca65d1 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -141,6 +141,7 @@  struct _GstLibcameraSrc {
 	GstTask *task;
 
 	gchar *camera_name;
+	gchar *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,23 @@  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) {
+		unsigned int w = 0, h = 0, depth = 0;
+		if (std::sscanf(self->sensor_config, "%ux%u/%u", &w, &h, &depth) == 3 && w && h && depth) {
+			SensorConfiguration sensorCfg;
+			sensorCfg.outputSize = Size(w, h);
+			sensorCfg.bitDepth = depth;
+			state->config_->sensorConfig = sensorCfg;
+		} else {
+			GST_ELEMENT_WARNING(self, RESOURCE, SETTINGS,
+					    ("Invalid sensor-config \"%s\", expected \"WIDTHxHEIGHT/DEPTH\","
+					     " ignoring",
+					     self->sensor_config),
+					    (nullptr));
+		}
+	}
+
 	if (!gst_libcamera_src_negotiate(self)) {
 		state->initControls_.clear();
 		GST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED);
@@ -934,6 +953,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_free(self->sensor_config);
+		self->sensor_config = g_value_dup_string(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 +976,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_string(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 +1066,7 @@  gst_libcamera_src_finalize(GObject *object)
 	g_clear_object(&self->task);
 	g_mutex_clear(&self->state->lock_);
 	g_free(self->camera_name);
+	g_free(self->sensor_config);
 	delete self->state;
 
 	return klass->finalize(object);
@@ -1162,6 +1189,17 @@  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_string("sensor-config", "Sensor Config",
+				   "Desired sensor configuration as \"WIDTHxHEIGHT/BITDEPTH\""
+				   " (e.g. \"2304x1296/10\"). Leave unset to let the pipeline"
+				   " negotiate the sensor mode automatically.",
+				   nullptr,
+				   (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);
 }