[PATCH v2 3/3] gstreamer: Generate controls from control_ids_*.yaml files

Laurent Pinchart laurent.pinchart at ideasonboard.com
Tue Aug 27 14:43:08 CEST 2024


On Tue, Aug 27, 2024 at 01:49:49PM +0200, Jaslo Ziska wrote:
> Laurent Pinchart writes:
> > On Tue, Aug 13, 2024 at 02:25:07PM +0200, Jaslo Ziska wrote:
> >> This commit implements gstreamer controls for the libcamera element by
> >> generating the controls from the control_ids_*.yaml files using a new
> >> gen-gst-controls.py script. The appropriate meson files are also changed
> >> to automatically run the script when building.
> >> 
> >> The gen-gst-controls.py script works similar to the gen-controls.py
> >> script by parsing the control_ids_*.yaml files and generating C++ code
> >> for each control.
> >> For the controls to be used as gstreamer properties the type for each
> >> control needs to be translated to the appropriate glib type and a
> >> GEnumValue is generated for each enum control. Then a
> >> g_object_install_property(), _get_property() and _set_property()
> >> function is generated for each control.
> >> The vendor controls get prefixed with "$vendor-" in the final gstreamer
> >> property name.
> >> 
> >> The C++ code generated by the gen-gst-controls.py script is written into
> >> the template gstlibcamerasrc-controls.cpp.in file. The matching
> >> gstlibcamerasrc-controls.h header defines the GstCameraControls class
> >> which handles the installation of the gstreamer properties as well as
> >> keeping track of the control values and setting and getting the
> >> controls. The content of these functions is generated in the Python
> >> script.
> >> 
> >> Finally the libcamerasrc element itself is edited to make use of the new
> >> GstCameraControls class. The way this works is by defining a PROP_LAST
> >> enum variant which is passed to the installProperties() function so the
> >> properties are defined with the appropriate offset. When getting or
> >> setting a property PROP_LAST is subtracted from the requested property
> >> to translate the control back into a libcamera::controls:: enum variant.
> >
> > Looks good to me. I'll focus the review on the parts not related to the
> > GStreamer internal APIs, and will let Nicolas bring his GStreamer
> > expertise.
> >
> >> Signed-off-by: Jaslo Ziska <jaslo at ziska.de>
> >> ---
> >>  src/gstreamer/gstlibcamera-controls.cpp.in | 296 +++++++++++++++++++++
> >>  src/gstreamer/gstlibcamera-controls.h      |  43 +++
> >>  src/gstreamer/gstlibcamerasrc.cpp          |  22 +-
> >>  src/gstreamer/meson.build                  |  10 +
> >>  utils/codegen/controls.py                  |   8 +
> >>  utils/codegen/gen-gst-controls.py          | 151 +++++++++++
> >>  utils/codegen/meson.build                  |   1 +
> >>  7 files changed, 528 insertions(+), 3 deletions(-)
> >>  create mode 100644 src/gstreamer/gstlibcamera-controls.cpp.in
> >>  create mode 100644 src/gstreamer/gstlibcamera-controls.h
> >>  create mode 100755 utils/codegen/gen-gst-controls.py
> >> 
> >> diff --git a/src/gstreamer/gstlibcamera-controls.cpp.in 
> >> b/src/gstreamer/gstlibcamera-controls.cpp.in
> >> new file mode 100644
> >> index 00000000..aab7ae24
> >> --- /dev/null
> >> +++ b/src/gstreamer/gstlibcamera-controls.cpp.in
> >> @@ -0,0 +1,296 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2023, Collabora Ltd.
> >> + *     Author: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> >
> > I think you can attribute the copyright on this file to 
> > yourself.
> >
> >> + *
> >> + * GStreamer Camera Controls
> >> + *
> >> + * This file is auto-generated. Do not edit.
> >> + */
> >> +
> >> +#include <vector>
> >> +
> >> +#include <libcamera/control_ids.h>
> >
> > And
> >
> > #include <libcamera/controls.h>
> > #include <libcamera/geometry.h>
> >
> > as you use classes defined in those headers below.
> >
> >> +
> >> +#include "gstlibcamera-controls.h"
> >> +
> >> +using namespace libcamera;
> >> +
> >> +{% for vendor, ctrls in controls %}
> >> +{%- for ctrl in ctrls if ctrl.is_enum %}
> >> +static const GEnumValue {{ ctrl.name|snake_case }}_types[] = {
> >> +{%- for enum in ctrl.enum_values %}
> >> +	{
> >> +		controls::{{ ctrl.namespace }}{{ enum.name }},
> >> +		{{ enum.description|format_description|indent('\t\t') 
> >> }},
> >> +		"{{ enum.gst_name }}"
> >> +	},
> >> +{%- endfor %}
> >> +	{0, NULL, NULL}
> >> +};
> >> +
> >> +#define TYPE_{{ ctrl.name|snake_case|upper }} \
> >> +	({{ ctrl.name|snake_case }}_get_type())
> >> +static GType {{ ctrl.name|snake_case }}_get_type()
> >> +{
> >> +	static GType {{ ctrl.name|snake_case }}_type = 0;
> >> +
> >> +	if (!{{ ctrl.name|snake_case }}_type)
> >> +		{{ ctrl.name|snake_case }}_type =
> >> +			g_enum_register_static("{{ ctrl.name }}",
> >> +					       {{ ctrl.name|snake_case }}_types);
> >> +
> >> +	return {{ ctrl.name|snake_case }}_type;
> >> +}
> >> +{% endfor %}
> >> +{%- endfor %}
> >> +
> >> +void GstCameraControls::installProperties(GObjectClass *klass, 
> >> int lastPropId)
> >> +{
> >> +{%- for vendor, ctrls in controls %}
> >> +{%- for ctrl in ctrls %}
> >> +
> >> +{%- set spec %}
> >
> > I didn't know about set in jinja templates, interesting.
> >
> >> +{%- if ctrl.is_rectangle -%}
> >> +gst_param_spec_array(
> >> +{%- else -%}
> >> +g_param_spec_{{ ctrl.gtype }}(
> >> +{%- endif -%}
> >> +{%- if ctrl.is_array %}
> >> +	"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case 
> >> }}-value",
> >> +	"{{ ctrl.name }} Value",
> >> +	"One {{ ctrl.name }} element value",
> >> +{%- else %}
> >> +	"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case }}",
> >> +	"{{ ctrl.name }}",
> >> +	{{ ctrl.description|format_description|indent('\t') }},
> >> +{%- endif %}
> >> +{%- if ctrl.is_enum %}
> >> +	TYPE_{{ ctrl.name|snake_case|upper }},
> >> +	{{ ctrl.default }},
> >> +{%- elif ctrl.is_rectangle %}
> >> +	g_param_spec_int(
> >> +		"rectangle-value",
> >> +		"Rectangle Value",
> >> +		"One rectangle value, either x, y, width or height.",
> >> +		{{ ctrl.min }}, {{ ctrl.max }}, {{ ctrl.default }},
> >> +		(GParamFlags) (GST_PARAM_CONTROLLABLE | 
> >> G_PARAM_READWRITE |
> >> +			       G_PARAM_STATIC_STRINGS)
> >> +	),
> >> +{%- elif ctrl.gtype == 'boolean' %}
> >> +	{{ ctrl.default }},
> >> +{%- elif ctrl.gtype in ['float', 'int', 'int64', 'uchar'] %}
> >> +	{{ ctrl.min }}, {{ ctrl.max }}, {{ ctrl.default }},
> >> +{%- endif %}
> >> +	(GParamFlags) (GST_PARAM_CONTROLLABLE | G_PARAM_READWRITE 
> >> |
> >> +		       G_PARAM_STATIC_STRINGS)
> >> +)
> >> +{%- endset %}
> >> +
> >> +	g_object_class_install_property(
> >> +		klass,
> >> +		lastPropId + controls::{{ ctrl.namespace }}{{ 
> >> ctrl.name|snake_case|upper }},
> >> +{%- if ctrl.is_array %}
> >> +		gst_param_spec_array(
> >> +			"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case 
> >> }}",
> >> +			"{{ ctrl.name }}",
> >> +			{{ 
> >> ctrl.description|format_description|indent('\t\t\t') }},
> >> +			{{ spec|indent('\t\t\t') }},
> >> +			(GParamFlags) (GST_PARAM_CONTROLLABLE |
> >> +				       G_PARAM_READWRITE |
> >> +				       G_PARAM_STATIC_STRINGS)
> >> +		)
> >> +{%- else %}
> >> +		{{ spec|indent('\t\t') }}
> >> +{%- endif %}
> >> +	);
> >> +{%- endfor %}
> >> +{%- endfor %}
> >> +}
> >> +
> >> +bool GstCameraControls::getProperty(guint propId, GValue 
> >> *value,
> >> +				    [[maybe_unused]] GParamSpec *pspec)
> >> +{
> >> +	switch (propId) {
> >> +{%- for vendor, ctrls in controls %}
> >> +{%- for ctrl in ctrls %}
> >> +
> >> +{%- set value_set %}
> >> +{%- if ctrl.is_rectangle -%}
> >> +Point top_left = val.topLeft();
> >> +Size size = val.size();
> >> +
> >> +GValue x = G_VALUE_INIT;
> >> +g_value_init(&x, G_TYPE_INT);
> >> +g_value_set_int(&x, top_left.x);
> >> +gst_value_array_append_and_take_value(&element, &x);
> >> +
> >> +GValue y = G_VALUE_INIT;
> >> +g_value_init(&y, G_TYPE_INT);
> >> +g_value_set_int(&y, top_left.y);
> >> +gst_value_array_append_and_take_value(&element, &y);
> >> +
> >> +GValue width = G_VALUE_INIT;
> >> +g_value_init(&width, G_TYPE_INT);
> >> +g_value_set_int(&width, size.width);
> >> +gst_value_array_append_and_take_value(&element, &width);
> >> +
> >> +GValue height = G_VALUE_INIT;
> >> +g_value_init(&height, G_TYPE_INT);
> >> +g_value_set_int(&x, size.height);
> >> +gst_value_array_append_and_take_value(&element, &height);
> >> +{%- else -%}
> >> +g_value_set_{{ ctrl.gtype }}(&element, val);
> >> +{%- endif -%}
> >> +{%- endset %}
> >> +
> >> +	case controls::{{ ctrl.namespace }}{{ 
> >> ctrl.name|snake_case|upper }}: {
> >> +		auto control = metadata_.get(controls::{{ 
> >> ctrl.namespace }}{{ ctrl.name }});
> >> +		control = control ? control :
> >> +			  controls_.get(controls::{{ ctrl.namespace }}{{ 
> >> ctrl.name }});
> >
> > 		if (!control)
> > 			control = controls_.get(controls::{{ ctrl.namespace 
> > }}{{ ctrl.name }});
> >
> >> +		if (!control) {
> >> +			GST_WARNING("Control '%s' is not available, 
> >> default value will be returned",
> >> +				    controls::{{ ctrl.namespace }}{{ ctrl.name 
> >> }}.name().c_str());
> >> +			return true;
> >> +		}
> >> +
> >> +{%- if ctrl.is_array %}
> >> +		for (size_t i = 0; i < control->size(); ++i) {
> >> +			GValue element = G_VALUE_INIT;
> >> +{%- if ctrl.is_rectangle %}
> >> +			g_value_init(&element, GST_TYPE_PARAM_ARRAY_LIST);
> >> +{%- else %}
> >> +			g_value_init(&element, G_TYPE_{{ ctrl.gtype|upper 
> >> }});
> >> +{%- endif %}
> >> +			auto val = (*control)[i];
> >> +			{{ value_set|indent('\t\t\t\t') }}
> >> +			gst_value_array_append_and_take_value(value, 
> >> &element);
> >> +		}
> >> +{%- else %}
> >> +		GValue element = *value;
> >> +		auto val = *control;
> >> +		{{ value_set|indent('\t\t\t') }}
> >> +{%- endif %}
> >> +
> >> +		return true;
> >> +	}
> >> +{%- endfor %}
> >> +{%- endfor %}
> >
> > Add a blank line here as there's one between all the cases 
> > above.
> >
> >> +	default:
> >> +		return false;
> >> +	}
> >> +}
> >> +
> >> +bool GstCameraControls::setProperty(guint propId, const GValue 
> >> *value,
> >> +				    [[maybe_unused]] GParamSpec *pspec)
> >> +{
> >> +	// check whether the camera capabilities are available
> >
> > Please use C-style comments, start sentences with a capital 
> > letter, and
> > end them with a period.
> >
> > 	/* Check whether the camera capabilities are available. */
> >
> >> +	if (!capabilities_.empty()) {
> >> +		// if so, check that the control is supported
> >
> > 		/* If so, check that the control is supported. */
> >
> > Same below
> >
> >> +		const ControlId *cid = 
> >> capabilities_.idmap().at(propId);
> >> +		auto info = capabilities_.find(cid);
> >> +
> >> +		if (info == capabilities_.end()) {
> >> +			GST_WARNING("Control '%s' is not supported by the 
> >> camera and will be ignored",
> >> +				    cid->name().c_str());
> >> +			return true;
> >> +		}
> >> +	}
> >> +
> >> +	switch (propId) {
> >> +{%- for vendor, ctrls in controls %}
> >> +{%- for ctrl in ctrls %}
> >> +
> >> +{%- set value_get %}
> >> +{%- if ctrl.is_rectangle -%}
> >> +if (gst_value_array_get_size(element) != 4) {
> >> +	GST_ERROR("Rectangle must be an array of size 4");
> >
> > Maybe print the name of the control to make debugging easier ?
> 
> Good idea, I will add that. It might be a little problematic when 
> extracting this part to a separate helper function as the Jinja 
> context will then not be available anymore for the name but I will 
> try think of something (maybe passing the control name to the 
> function?).

Yes, passing the control name to the function seems fine.
 
> >> +	return true;
> >> +}
> >> +
> >> +const GValue *r;
> >> +r = gst_value_array_get_value(element, 0);
> >> +int x = g_value_get_int(r);
> >> +r = gst_value_array_get_value(element, 1);
> >> +int y = g_value_get_int(r);
> >> +r = gst_value_array_get_value(element, 2);
> >> +int w = g_value_get_int(r);
> >> +r = gst_value_array_get_value(element, 3);
> >> +int h = g_value_get_int(r);
> >> +
> >> +auto val = Rectangle(x, y, w, h);
> >
> > Would it make sense to move all this (including the size check) to a
> > helper function instead of duplicating the code for each rectangle
> > control ?
> 
> Good idea, that way I might also be able to drop the Jinja set 
> directive and use it directly.
> 
> >> +{%- else -%}
> >> +auto val = g_value_get_{{ ctrl.gtype }}(element);
> >> +{%- endif -%}
> >> +{%- endset %}
> >> +
> >> +	case controls::{{ ctrl.namespace }}{{ 
> >> ctrl.name|snake_case|upper }}: {
> >> +{%- if ctrl.is_array %}
> >> +		size_t size = gst_value_array_get_size(value);
> >> +{%- if ctrl.size != 0 %}
> >> +		if (size != {{ ctrl.size }}) {
> >> +			GST_ERROR("Incorrect array size, must be of size 
> >> {{ ctrl.size }}");
> >> +			return true;
> >> +		}
> >> +{%- endif %}
> >> +
> >> +		std::vector<{{ ctrl.element_type }}> values(size);
> >> +		for (size_t i = 0; i < size; ++i) {
> >> +			const GValue *element = 
> >> gst_value_array_get_value(value, i);
> >> +			{{ value_get|indent('\t\t\t') }}
> >> +			values[i] = val;
> >> +		}
> >> +		controls_.set(controls::{{ ctrl.namespace }}{{ 
> >> ctrl.name }},
> >> +{%- if ctrl.size == 0 %}
> >> +			      Span<const {{ ctrl.element_type 
> >> }}>(values.data(), size));
> >> +{%- else %}
> >> +			      Span<const {{ ctrl.element_type }}, {{ 
> >> ctrl.size }}>(values.data(), {{ ctrl.size }}));
> >> +{%- endif %}
> >> +{%- else %}
> >> +		const GValue *element = value;
> >> +		{{ value_get|indent('\t\t') }}
> >> +		controls_.set(controls::{{ ctrl.namespace }}{{ 
> >> ctrl.name }}, val);
> >> +{%- endif %}
> >> +		return true;
> >> +	}
> >> +{%- endfor %}
> >> +{%- endfor %}
> >
> > Add a blank line here too.
> >
> >> +	default:
> >> +		return false;
> >> +	}
> >> +}
> >> +
> >> +void GstCameraControls::setCamera(const 
> >> std::shared_ptr<libcamera::Camera> &cam)
> >> +{
> >> +	capabilities_ = cam->controls();
> >
> > Is there a need to make a copy of the ControlInfoMap ? Can't we 
> > retain a
> > reference to the camera and use it to access the list of 
> > controls ?
> 
> You are right, I could just add a shared_ptr<Camera> member.
> 
> >> +
> >> +	// check the controls which were set before the camera 
> >> capabilities were known
> >> +	ControlList new_controls;
> >> +	for (auto control = controls_.begin(); control != 
> >> controls_.end(); ++control) {
> >
> > setCamera() is called in gst_libcamera_src_open(). Does that occur
> > before or after gst_libcamera_src_set_property() ? If it occurs before,
> > controls_ will be empty here.
> 
> This is a little complicated, gst_libcamera_src_set_property() can 
> be called before and after gst_libcamera_src_open(). So to be able 
> to check the controls which were set before the pipeline is 
> started I check the controls at this point because the camera 
> becomes available here.

Maybe a comment would be useful to explain this.

> >> +		unsigned int id = control->first;
> >> +		ControlValue value = control->second;
> >> +
> >> +		const ControlId *cid = capabilities_.idmap().at(id);
> >> +		auto info = capabilities_.find(cid);
> >> +
> >> +		// only add controls which are supported
> >> +		if (info != capabilities_.end()) {
> >> +			new_controls.set(id, value);
> >> +		} else {
> >> +			GST_WARNING("Control '%s' is not supported by the camera and will be ignored",
> >> +				    cid->name().c_str());
> >
> > The control should have been rejected by setProperty() in this case, and
> > won't end up in controls_ in that case, so I don't think this can occur.
> 
> Because controls might be set before the camera is available the 
> first line in setProperty() checks whether the camera capabilities 
> are available and only checks and rejects the controls if they 
> are. If the camera was not set when setProperty() is called the 
> control gets set unconditionally.
> 
> That is why the controls which were set before the camera became 
> available are checked at this point when the camera is set.

I see. I suppose there's no way to simplify this and requires the camera
to be set before controls are set ?

> >> +		}
> >
> > You can drop the curly braces.
> >
> >> +	}
> >> +
> >> +	controls_ = new_controls;
> >> +}
> >> +
> >> +void 
> >> GstCameraControls::applyControls(std::unique_ptr<libcamera::Request> 
> >> &request)
> >> +{
> >> +	request->controls().merge(controls_);
> >
> > I think you want to clear controls_ here, they don't need to be set in
> > every request. That's problematic for getControl() though, as it relies
> > on controls_ accumulating the state of all controls ever set. You'll
> > probably need two variables, one that accumulates controls, and one that
> > doesn't.
> 
> You are right. I was mistaken about how the libcamera controls 
> work, for some reason I thought you had to repeatedly send the 
> control with every request. I will change this.
> 
> >> +}
> >> +
> >> +void GstCameraControls::readMetadata(libcamera::Request 
> >> *request)
> >> +{
> >> +	metadata_ = request->metadata();
> >> +}
> >
> > Great handling of white space, the generated code is very 
> > readable.
> >
> >> diff --git a/src/gstreamer/gstlibcamera-controls.h 
> >> b/src/gstreamer/gstlibcamera-controls.h
> >> new file mode 100644
> >> index 00000000..89b616ad
> >> --- /dev/null
> >> +++ b/src/gstreamer/gstlibcamera-controls.h
> >> @@ -0,0 +1,43 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2023, Collabora Ltd.
> >> + *     Author: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> >> + *
> >> + * GStreamer Camera Controls
> >> + */
> >> +
> >> +#pragma once
> >> +
> >> +#include <memory>
> >> +
> >> +#include <libcamera/camera.h>
> >> +#include <libcamera/controls.h>
> >> +#include <libcamera/request.h>
> >> +
> >> +#include "gstlibcamerasrc.h"
> >> +
> >> +namespace libcamera {
> >> +
> >> +class GstCameraControls
> >> +{
> >> +public:
> >> +	static void installProperties(GObjectClass *klass, int lastProp);
> >> +
> >> +	bool getProperty(guint propId, GValue *value, GParamSpec *pspec);
> >> +	bool setProperty(guint propId, const GValue *value, GParamSpec *pspec);
> >> +
> >> +	void setCamera(const std::shared_ptr<libcamera::Camera> &cam);
> >> +
> >> +	void applyControls(std::unique_ptr<libcamera::Request> &request);
> >> +	void readMetadata(libcamera::Request *request);
> >> +
> >> +private:
> >> +	/* supported controls and limits of camera */
> >> +	ControlInfoMap capabilities_;
> >> +	/* set of user modified controls */
> >> +	ControlList controls_;
> >> +	/* metadata returned by the camera */
> >> +	ControlList metadata_;
> >> +};
> >> +
> >> +} /* namespace libcamera */
> >> diff --git a/src/gstreamer/gstlibcamerasrc.cpp 
> >> b/src/gstreamer/gstlibcamerasrc.cpp
> >> index 40b787c8..8efa25f4 100644
> >> --- a/src/gstreamer/gstlibcamerasrc.cpp
> >> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> >> @@ -37,10 +37,11 @@
> >>  
> >>  #include <gst/base/base.h>
> >>  
> >> +#include "gstlibcamera-controls.h"
> >> +#include "gstlibcamera-utils.h"
> >>  #include "gstlibcameraallocator.h"
> >>  #include "gstlibcamerapad.h"
> >>  #include "gstlibcamerapool.h"
> >> -#include "gstlibcamera-utils.h"
> >>  
> >>  using namespace libcamera;
> >>  
> >> @@ -128,6 +129,7 @@ struct GstLibcameraSrcState {
> >>  
> >>  	ControlList initControls_;
> >>  	guint group_id_;
> >> +	GstCameraControls controls_;
> >>  
> >>  	int queueRequest();
> >>  	void requestCompleted(Request *request);
> >> @@ -153,6 +155,7 @@ struct _GstLibcameraSrc {
> >>  enum {
> >>  	PROP_0,
> >>  	PROP_CAMERA_NAME,
> >> +	PROP_LAST
> >>  };
> >>  
> >>  static void gst_libcamera_src_child_proxy_init(gpointer g_iface,
> >> @@ -183,6 +186,9 @@ int GstLibcameraSrcState::queueRequest()
> >>  	if (!request)
> >>  		return -ENOMEM;
> >>  
> >> +	/* Apply controls */
> >> +	controls_.applyControls(request);
> >> +
> >>  	std::unique_ptr<RequestWrap> wrap =
> >>  		std::make_unique<RequestWrap>(std::move(request));
> >>  
> >> @@ -226,6 +232,9 @@ 
> >> GstLibcameraSrcState::requestCompleted(Request *request)
> >>  
> >>  	{
> >>  		GLibLocker locker(&lock_);
> >> +
> >> +		controls_.readMetadata(request);
> >> +
> >>  		wrap = std::move(queuedRequests_.front());
> >>  		queuedRequests_.pop();
> >>  	}
> >> @@ -408,6 +417,8 @@ gst_libcamera_src_open(GstLibcameraSrc *self)
> >>  		return false;
> >>  	}
> >>  
> >> +	self->state->controls_.setCamera(cam);
> >> +
> >>  	cam->requestCompleted.connect(self->state, 
> >>  &GstLibcameraSrcState::requestCompleted);
> >>  
> >>  	/* No need to lock here, we didn't start our threads yet. 
> >>  */
> >> @@ -722,6 +733,7 @@ gst_libcamera_src_set_property(GObject 
> >> *object, guint prop_id,
> >>  {
> >>  	GLibLocker lock(GST_OBJECT(object));
> >>  	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);
> >> +	GstLibcameraSrcState *state = self->state;
> >>  
> >>  	switch (prop_id) {
> >>  	case PROP_CAMERA_NAME:
> >> @@ -729,7 +741,8 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
> >>  		self->camera_name = g_value_dup_string(value);
> >>  		break;
> >>  	default:
> >> -		G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> >> +		if (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))
> >> +			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> >>  		break;
> >>  	}
> >>  }
> >> @@ -740,13 +753,15 @@ gst_libcamera_src_get_property(GObject 
> >> *object, guint prop_id, GValue *value,
> >>  {
> >>  	GLibLocker lock(GST_OBJECT(object));
> >>  	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);
> >> +	GstLibcameraSrcState *state = self->state;
> >>  
> >>  	switch (prop_id) {
> >>  	case PROP_CAMERA_NAME:
> >>  		g_value_set_string(value, self->camera_name);
> >>  		break;
> >>  	default:
> >> -		G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> >> +		if (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))
> >> +			G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> >>  		break;
> >>  	}
> >>  }
> >> @@ -947,6 +962,7 @@ 
> >> gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> >>  							     | G_PARAM_STATIC_STRINGS));
> >>  	g_object_class_install_property(object_class, 
> >>  PROP_CAMERA_NAME, spec);
> >>  
> >> +	GstCameraControls::installProperties(object_class, PROP_LAST);
> >>  }
> >>  
> >>  /* GstChildProxy implementation */
> >> diff --git a/src/gstreamer/meson.build 
> >> b/src/gstreamer/meson.build
> >> index c2a01e7b..6b7e53b5 100644
> >> --- a/src/gstreamer/meson.build
> >> +++ b/src/gstreamer/meson.build
> >> @@ -25,6 +25,16 @@ libcamera_gst_sources = [
> >>      'gstlibcamerasrc.cpp',
> >>  ]
> >>  
> >> +# Generate gstreamer control properties
> >> +
> >> +gen_gst_controls_template = files('gstlibcamera-controls.cpp.in')
> >> +libcamera_gst_sources += custom_target('gstlibcamera-controls.cpp',
> >> +                                       input : controls_files,
> >> +                                       output : 'gstlibcamera-controls.cpp',
> >> +                                       command : [gen_gst_controls, '-o', '@OUTPUT@',
> >> +                                                  '-t', gen_gst_controls_template, '@INPUT@'],
> >> +                                       env : py_build_env)
> >> +
> >>  libcamera_gst_cpp_args = [
> >>      '-DVERSION="@0@"'.format(libcamera_git_version),
> >>      '-DPACKAGE="@0@"'.format(meson.project_name()),
> >> diff --git a/utils/codegen/controls.py 
> >> b/utils/codegen/controls.py
> >> index 7bafee59..03c77cc6 100644
> >> --- a/utils/codegen/controls.py
> >> +++ b/utils/codegen/controls.py
> >> @@ -110,3 +110,11 @@ class Control(object):
> >>              return f"Span<const {typ}, {self.__size}>"
> >>          else:
> >>              return f"Span<const {typ}>"
> >> +
> >> +    @property
> >> +    def element_type(self):
> >> +        return self.__data.get('type')
> >
> > I wonder if we shouldn't move the existing type property to
> > extend_control() in generator scripts, and rename element_type to type.
> > This can be done on top, you don't have to address it.
> 
> I'm fine with either, although just calling it type() is a little 
> simpler for the gen-gst-controls.py script.
> 
> >> +
> >> +    @property
> >> +    def size(self):
> >> +        return self.__size
> >> diff --git a/utils/codegen/gen-gst-controls.py 
> >> b/utils/codegen/gen-gst-controls.py
> >> new file mode 100755
> >> index 00000000..5ac8981f
> >> --- /dev/null
> >> +++ b/utils/codegen/gen-gst-controls.py
> >> @@ -0,0 +1,151 @@
> >> +#!/usr/bin/env python3
> >> +# SPDX-License-Identifier: GPL-2.0-or-later
> >> +# Copyright (C) 2019, Google Inc.
> >> +# Copyright (C) 2024, Jaslo Ziska
> >> +#
> >> +# Authors:
> >> +# Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >> +# Jaslo Ziska <jaslo at ziska.de>
> >> +#
> >> +# Generate gstreamer control properties from YAML
> >> +
> >> +import argparse
> >> +import jinja2
> >> +import re
> >> +import sys
> >> +import yaml
> >> +
> >> +from controls import Control
> >> +
> >> +
> >> +def find_common_prefix(strings):
> >> +    prefix = strings[0]
> >> +
> >> +    for string in strings[1:]:
> >> +        while string[:len(prefix)] != prefix and prefix:
> >> +            prefix = prefix[:len(prefix) - 1]
> >> +        if not prefix:
> >> +            break
> >> +
> >> +    return prefix
> >> +
> >> +
> >> +def format_description(description):
> >> +    # Substitute doxygen keywords \sa (see also) and \todo
> >> +    description = re.sub(r'\\sa((?: \w+)+)',
> >> +                         lambda match: 'See also: ' + ', '.join(
> >> +                             map(kebab_case, match.group(1).strip().split(' '))
> >> +                         ) + '.', description)
> >> +    description = re.sub(r'\\todo', 'Todo:', description)
> >> +
> >> +    description = description.strip().split('\n')
> >> +    return '\n'.join([
> >> +        '"' + line.replace('\\', r'\\').replace('"', r'\"') + ' "' for line in description if line
> >> +    ]).rstrip()
> >> +
> >> +
> >> +def snake_case(s):
> >> +    return ''.join([
> >> +        c.isupper() and ('_' + c.lower()) or c for c in s
> >> +    ]).strip('_')
> >> +
> >> +
> >> +def kebab_case(s):
> >> +    return snake_case(s).replace('_', '-')
> >> +
> >> +
> >> +def extend_control(ctrl):
> >> +    if ctrl.vendor != 'libcamera':
> >> +        ctrl.namespace = f'{ctrl.vendor}::'
> >> +        ctrl.vendor_prefix = f'{ctrl.vendor}-'
> >> +    else:
> >> +        ctrl.namespace = ''
> >> +        ctrl.vendor_prefix = ''
> >> +
> >> +    ctrl.is_array = ctrl.size is not None
> >> +
> >> +    if ctrl.is_enum:
> >> +        # remove common prefix from enum variant names
> >
> > s/# remove/# Remove/
> >
> >> +        prefix = find_common_prefix([enum.name for enum in ctrl.enum_values])
> >> +        for enum in ctrl.enum_values:
> >> +            enum.gst_name = kebab_case(enum.name.removeprefix(prefix))
> >> +
> >> +        ctrl.gtype = 'enum'
> >> +        ctrl.default = '0'
> >> +    elif ctrl.element_type == 'bool':
> >> +        ctrl.gtype = 'boolean'
> >> +        ctrl.default = 'false'
> >> +    elif ctrl.element_type == 'float':
> >> +        ctrl.gtype = 'float'
> >> +        ctrl.default = '0'
> >> +        ctrl.min = '-G_MAXFLOAT'
> >> +        ctrl.max = 'G_MAXFLOAT'
> >> +    elif ctrl.element_type == 'int32_t':
> >> +        ctrl.gtype = 'int'
> >> +        ctrl.default = '0'
> >> +        ctrl.min = 'G_MININT'
> >> +        ctrl.max = 'G_MAXINT'
> >> +    elif ctrl.element_type == 'int64_t':
> >> +        ctrl.gtype = 'int64'
> >> +        ctrl.default = '0'
> >> +        ctrl.min = 'G_MININT64'
> >> +        ctrl.max = 'G_MAXINT64'
> >> +    elif ctrl.element_type == 'uint8_t':
> >> +        ctrl.gtype = 'uchar'
> >> +        ctrl.default = '0'
> >> +        ctrl.min = '0'
> >> +        ctrl.max = 'G_MAXUINT8'
> >> +    elif ctrl.element_type == 'Rectangle':
> >> +        ctrl.is_rectangle = True
> >> +        ctrl.default = '0'
> >> +        ctrl.min = '0'
> >> +        ctrl.max = 'G_MAXINT'
> >> +    else:
> >> +        raise RuntimeError(f'The type `{ctrl.element_type}` is unknown')
> >> +
> >> +    return ctrl
> >> +
> >> +
> >> +def main(argv):
> >> +    # Parse command line arguments
> >> +    parser = argparse.ArgumentParser()
> >> +    parser.add_argument('--output', '-o', metavar='file', type=str,
> >> +                        help='Output file name. Defaults to standard output if not specified.')
> >> +    parser.add_argument('--template', '-t', dest='template', type=str, required=True,
> >> +                        help='Template file name.')
> >> +    parser.add_argument('input', type=str, nargs='+',
> >> +                        help='Input file name.')
> >> +
> >> +    args = parser.parse_args(argv[1:])
> >> +
> >> +    controls = {}
> >> +    for input in args.input:
> >> +        data = yaml.safe_load(open(input, 'rb').read())
> >> +
> >> +        vendor = data['vendor']
> >> +        ctrls = controls.setdefault(vendor, [])
> >> +
> >> +        for ctrl in data['controls']:
> >> +            ctrl = Control(*ctrl.popitem(), vendor)
> >> +            ctrls.append(extend_control(ctrl))
> >> +
> >> +    data = {'controls': list(controls.items())}
> >> +
> >> +    env = jinja2.Environment()
> >> +    env.filters['format_description'] = format_description
> >> +    env.filters['snake_case'] = snake_case
> >> +    env.filters['kebab_case'] = kebab_case
> >> +    template = env.from_string(open(args.template, 'r', encoding='utf-8').read())
> >> +    string = template.render(data)
> >> +
> >> +    if args.output:
> >> +        with open(args.output, 'w', encoding='utf-8') as output:
> >> +            output.write(string)
> >> +    else:
> >> +        sys.stdout.write(string)
> >> +
> >> +    return 0
> >> +
> >> +
> >> +if __name__ == '__main__':
> >> +    sys.exit(main(sys.argv))
> >> diff --git a/utils/codegen/meson.build 
> >> b/utils/codegen/meson.build
> >> index adf33bba..904dd66d 100644
> >> --- a/utils/codegen/meson.build
> >> +++ b/utils/codegen/meson.build
> >> @@ -11,6 +11,7 @@ py_modules += ['jinja2', 'yaml']
> >>  
> >>  gen_controls = files('gen-controls.py')
> >>  gen_formats = files('gen-formats.py')
> >> +gen_gst_controls = files('gen-gst-controls.py')
> >>  gen_header = files('gen-header.sh')
> >>  gen_ipa_pub_key = files('gen-ipa-pub-key.py')
> >>  gen_tracepoints = files('gen-tp-header.py')

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list