[{"id":30849,"web_url":"https://patchwork.libcamera.org/comment/30849/","msgid":"<20240816000744.GE23911@pendragon.ideasonboard.com>","date":"2024-08-16T00:07:44","subject":"Re: [PATCH v2 3/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jaslo,\n\nThank you for the patch.\n\nOn Tue, Aug 13, 2024 at 02:25:07PM +0200, Jaslo Ziska wrote:\n> This commit implements gstreamer controls for the libcamera element by\n> generating the controls from the control_ids_*.yaml files using a new\n> gen-gst-controls.py script. The appropriate meson files are also changed\n> to automatically run the script when building.\n> \n> The gen-gst-controls.py script works similar to the gen-controls.py\n> script by parsing the control_ids_*.yaml files and generating C++ code\n> for each control.\n> For the controls to be used as gstreamer properties the type for each\n> control needs to be translated to the appropriate glib type and a\n> GEnumValue is generated for each enum control. Then a\n> g_object_install_property(), _get_property() and _set_property()\n> function is generated for each control.\n> The vendor controls get prefixed with \"$vendor-\" in the final gstreamer\n> property name.\n> \n> The C++ code generated by the gen-gst-controls.py script is written into\n> the template gstlibcamerasrc-controls.cpp.in file. The matching\n> gstlibcamerasrc-controls.h header defines the GstCameraControls class\n> which handles the installation of the gstreamer properties as well as\n> keeping track of the control values and setting and getting the\n> controls. The content of these functions is generated in the Python\n> script.\n> \n> Finally the libcamerasrc element itself is edited to make use of the new\n> GstCameraControls class. The way this works is by defining a PROP_LAST\n> enum variant which is passed to the installProperties() function so the\n> properties are defined with the appropriate offset. When getting or\n> setting a property PROP_LAST is subtracted from the requested property\n> to translate the control back into a libcamera::controls:: enum\n> variant.\n\nLooks good to me. I'll focus the review on the parts not related to the\nGStreamer internal APIs, and will let Nicolas bring his GStreamer\nexpertise.\n\n> Signed-off-by: Jaslo Ziska <jaslo@ziska.de>\n> ---\n>  src/gstreamer/gstlibcamera-controls.cpp.in | 296 +++++++++++++++++++++\n>  src/gstreamer/gstlibcamera-controls.h      |  43 +++\n>  src/gstreamer/gstlibcamerasrc.cpp          |  22 +-\n>  src/gstreamer/meson.build                  |  10 +\n>  utils/codegen/controls.py                  |   8 +\n>  utils/codegen/gen-gst-controls.py          | 151 +++++++++++\n>  utils/codegen/meson.build                  |   1 +\n>  7 files changed, 528 insertions(+), 3 deletions(-)\n>  create mode 100644 src/gstreamer/gstlibcamera-controls.cpp.in\n>  create mode 100644 src/gstreamer/gstlibcamera-controls.h\n>  create mode 100755 utils/codegen/gen-gst-controls.py\n> \n> diff --git a/src/gstreamer/gstlibcamera-controls.cpp.in b/src/gstreamer/gstlibcamera-controls.cpp.in\n> new file mode 100644\n> index 00000000..aab7ae24\n> --- /dev/null\n> +++ b/src/gstreamer/gstlibcamera-controls.cpp.in\n> @@ -0,0 +1,296 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Collabora Ltd.\n> + *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n\nI think you can attribute the copyright on this file to yourself.\n\n> + *\n> + * GStreamer Camera Controls\n> + *\n> + * This file is auto-generated. Do not edit.\n> + */\n> +\n> +#include <vector>\n> +\n> +#include <libcamera/control_ids.h>\n\nAnd\n\n#include <libcamera/controls.h>\n#include <libcamera/geometry.h>\n\nas you use classes defined in those headers below.\n\n> +\n> +#include \"gstlibcamera-controls.h\"\n> +\n> +using namespace libcamera;\n> +\n> +{% for vendor, ctrls in controls %}\n> +{%- for ctrl in ctrls if ctrl.is_enum %}\n> +static const GEnumValue {{ ctrl.name|snake_case }}_types[] = {\n> +{%- for enum in ctrl.enum_values %}\n> +\t{\n> +\t\tcontrols::{{ ctrl.namespace }}{{ enum.name }},\n> +\t\t{{ enum.description|format_description|indent('\\t\\t') }},\n> +\t\t\"{{ enum.gst_name }}\"\n> +\t},\n> +{%- endfor %}\n> +\t{0, NULL, NULL}\n> +};\n> +\n> +#define TYPE_{{ ctrl.name|snake_case|upper }} \\\n> +\t({{ ctrl.name|snake_case }}_get_type())\n> +static GType {{ ctrl.name|snake_case }}_get_type()\n> +{\n> +\tstatic GType {{ ctrl.name|snake_case }}_type = 0;\n> +\n> +\tif (!{{ ctrl.name|snake_case }}_type)\n> +\t\t{{ ctrl.name|snake_case }}_type =\n> +\t\t\tg_enum_register_static(\"{{ ctrl.name }}\",\n> +\t\t\t\t\t       {{ ctrl.name|snake_case }}_types);\n> +\n> +\treturn {{ ctrl.name|snake_case }}_type;\n> +}\n> +{% endfor %}\n> +{%- endfor %}\n> +\n> +void GstCameraControls::installProperties(GObjectClass *klass, int lastPropId)\n> +{\n> +{%- for vendor, ctrls in controls %}\n> +{%- for ctrl in ctrls %}\n> +\n> +{%- set spec %}\n\nI didn't know about set in jinja templates, interesting.\n\n> +{%- if ctrl.is_rectangle -%}\n> +gst_param_spec_array(\n> +{%- else -%}\n> +g_param_spec_{{ ctrl.gtype }}(\n> +{%- endif -%}\n> +{%- if ctrl.is_array %}\n> +\t\"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case }}-value\",\n> +\t\"{{ ctrl.name }} Value\",\n> +\t\"One {{ ctrl.name }} element value\",\n> +{%- else %}\n> +\t\"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case }}\",\n> +\t\"{{ ctrl.name }}\",\n> +\t{{ ctrl.description|format_description|indent('\\t') }},\n> +{%- endif %}\n> +{%- if ctrl.is_enum %}\n> +\tTYPE_{{ ctrl.name|snake_case|upper }},\n> +\t{{ ctrl.default }},\n> +{%- elif ctrl.is_rectangle %}\n> +\tg_param_spec_int(\n> +\t\t\"rectangle-value\",\n> +\t\t\"Rectangle Value\",\n> +\t\t\"One rectangle value, either x, y, width or height.\",\n> +\t\t{{ ctrl.min }}, {{ ctrl.max }}, {{ ctrl.default }},\n> +\t\t(GParamFlags) (GST_PARAM_CONTROLLABLE | G_PARAM_READWRITE |\n> +\t\t\t       G_PARAM_STATIC_STRINGS)\n> +\t),\n> +{%- elif ctrl.gtype == 'boolean' %}\n> +\t{{ ctrl.default }},\n> +{%- elif ctrl.gtype in ['float', 'int', 'int64', 'uchar'] %}\n> +\t{{ ctrl.min }}, {{ ctrl.max }}, {{ ctrl.default }},\n> +{%- endif %}\n> +\t(GParamFlags) (GST_PARAM_CONTROLLABLE | G_PARAM_READWRITE |\n> +\t\t       G_PARAM_STATIC_STRINGS)\n> +)\n> +{%- endset %}\n> +\n> +\tg_object_class_install_property(\n> +\t\tklass,\n> +\t\tlastPropId + controls::{{ ctrl.namespace }}{{ ctrl.name|snake_case|upper }},\n> +{%- if ctrl.is_array %}\n> +\t\tgst_param_spec_array(\n> +\t\t\t\"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case }}\",\n> +\t\t\t\"{{ ctrl.name }}\",\n> +\t\t\t{{ ctrl.description|format_description|indent('\\t\\t\\t') }},\n> +\t\t\t{{ spec|indent('\\t\\t\\t') }},\n> +\t\t\t(GParamFlags) (GST_PARAM_CONTROLLABLE |\n> +\t\t\t\t       G_PARAM_READWRITE |\n> +\t\t\t\t       G_PARAM_STATIC_STRINGS)\n> +\t\t)\n> +{%- else %}\n> +\t\t{{ spec|indent('\\t\\t') }}\n> +{%- endif %}\n> +\t);\n> +{%- endfor %}\n> +{%- endfor %}\n> +}\n> +\n> +bool GstCameraControls::getProperty(guint propId, GValue *value,\n> +\t\t\t\t    [[maybe_unused]] GParamSpec *pspec)\n> +{\n> +\tswitch (propId) {\n> +{%- for vendor, ctrls in controls %}\n> +{%- for ctrl in ctrls %}\n> +\n> +{%- set value_set %}\n> +{%- if ctrl.is_rectangle -%}\n> +Point top_left = val.topLeft();\n> +Size size = val.size();\n> +\n> +GValue x = G_VALUE_INIT;\n> +g_value_init(&x, G_TYPE_INT);\n> +g_value_set_int(&x, top_left.x);\n> +gst_value_array_append_and_take_value(&element, &x);\n> +\n> +GValue y = G_VALUE_INIT;\n> +g_value_init(&y, G_TYPE_INT);\n> +g_value_set_int(&y, top_left.y);\n> +gst_value_array_append_and_take_value(&element, &y);\n> +\n> +GValue width = G_VALUE_INIT;\n> +g_value_init(&width, G_TYPE_INT);\n> +g_value_set_int(&width, size.width);\n> +gst_value_array_append_and_take_value(&element, &width);\n> +\n> +GValue height = G_VALUE_INIT;\n> +g_value_init(&height, G_TYPE_INT);\n> +g_value_set_int(&x, size.height);\n> +gst_value_array_append_and_take_value(&element, &height);\n> +{%- else -%}\n> +g_value_set_{{ ctrl.gtype }}(&element, val);\n> +{%- endif -%}\n> +{%- endset %}\n> +\n> +\tcase controls::{{ ctrl.namespace }}{{ ctrl.name|snake_case|upper }}: {\n> +\t\tauto control = metadata_.get(controls::{{ ctrl.namespace }}{{ ctrl.name }});\n> +\t\tcontrol = control ? control :\n> +\t\t\t  controls_.get(controls::{{ ctrl.namespace }}{{ ctrl.name }});\n\n\t\tif (!control)\n\t\t\tcontrol = controls_.get(controls::{{ ctrl.namespace }}{{ ctrl.name }});\n\n> +\t\tif (!control) {\n> +\t\t\tGST_WARNING(\"Control '%s' is not available, default value will be returned\",\n> +\t\t\t\t    controls::{{ ctrl.namespace }}{{ ctrl.name }}.name().c_str());\n> +\t\t\treturn true;\n> +\t\t}\n> +\n> +{%- if ctrl.is_array %}\n> +\t\tfor (size_t i = 0; i < control->size(); ++i) {\n> +\t\t\tGValue element = G_VALUE_INIT;\n> +{%- if ctrl.is_rectangle %}\n> +\t\t\tg_value_init(&element, GST_TYPE_PARAM_ARRAY_LIST);\n> +{%- else %}\n> +\t\t\tg_value_init(&element, G_TYPE_{{ ctrl.gtype|upper }});\n> +{%- endif %}\n> +\t\t\tauto val = (*control)[i];\n> +\t\t\t{{ value_set|indent('\\t\\t\\t\\t') }}\n> +\t\t\tgst_value_array_append_and_take_value(value, &element);\n> +\t\t}\n> +{%- else %}\n> +\t\tGValue element = *value;\n> +\t\tauto val = *control;\n> +\t\t{{ value_set|indent('\\t\\t\\t') }}\n> +{%- endif %}\n> +\n> +\t\treturn true;\n> +\t}\n> +{%- endfor %}\n> +{%- endfor %}\n\nAdd a blank line here as there's one between all the cases above.\n\n> +\tdefault:\n> +\t\treturn false;\n> +\t}\n> +}\n> +\n> +bool GstCameraControls::setProperty(guint propId, const GValue *value,\n> +\t\t\t\t    [[maybe_unused]] GParamSpec *pspec)\n> +{\n> +\t// check whether the camera capabilities are available\n\nPlease use C-style comments, start sentences with a capital letter, and\nend them with a period.\n\n\t/* Check whether the camera capabilities are available. */\n\n> +\tif (!capabilities_.empty()) {\n> +\t\t// if so, check that the control is supported\n\n\t\t/* If so, check that the control is supported. */\n\nSame below\n\n> +\t\tconst ControlId *cid = capabilities_.idmap().at(propId);\n> +\t\tauto info = capabilities_.find(cid);\n> +\n> +\t\tif (info == capabilities_.end()) {\n> +\t\t\tGST_WARNING(\"Control '%s' is not supported by the camera and will be ignored\",\n> +\t\t\t\t    cid->name().c_str());\n> +\t\t\treturn true;\n> +\t\t}\n> +\t}\n> +\n> +\tswitch (propId) {\n> +{%- for vendor, ctrls in controls %}\n> +{%- for ctrl in ctrls %}\n> +\n> +{%- set value_get %}\n> +{%- if ctrl.is_rectangle -%}\n> +if (gst_value_array_get_size(element) != 4) {\n> +\tGST_ERROR(\"Rectangle must be an array of size 4\");\n\nMaybe print the name of the control to make debugging easier ?\n\n> +\treturn true;\n> +}\n> +\n> +const GValue *r;\n> +r = gst_value_array_get_value(element, 0);\n> +int x = g_value_get_int(r);\n> +r = gst_value_array_get_value(element, 1);\n> +int y = g_value_get_int(r);\n> +r = gst_value_array_get_value(element, 2);\n> +int w = g_value_get_int(r);\n> +r = gst_value_array_get_value(element, 3);\n> +int h = g_value_get_int(r);\n> +\n> +auto val = Rectangle(x, y, w, h);\n\nWould it make sense to move all this (including the size check) to a\nhelper function instead of duplicating the code for each rectangle\ncontrol ?\n\n> +{%- else -%}\n> +auto val = g_value_get_{{ ctrl.gtype }}(element);\n> +{%- endif -%}\n> +{%- endset %}\n> +\n> +\tcase controls::{{ ctrl.namespace }}{{ ctrl.name|snake_case|upper }}: {\n> +{%- if ctrl.is_array %}\n> +\t\tsize_t size = gst_value_array_get_size(value);\n> +{%- if ctrl.size != 0 %}\n> +\t\tif (size != {{ ctrl.size }}) {\n> +\t\t\tGST_ERROR(\"Incorrect array size, must be of size {{ ctrl.size }}\");\n> +\t\t\treturn true;\n> +\t\t}\n> +{%- endif %}\n> +\n> +\t\tstd::vector<{{ ctrl.element_type }}> values(size);\n> +\t\tfor (size_t i = 0; i < size; ++i) {\n> +\t\t\tconst GValue *element = gst_value_array_get_value(value, i);\n> +\t\t\t{{ value_get|indent('\\t\\t\\t') }}\n> +\t\t\tvalues[i] = val;\n> +\t\t}\n> +\t\tcontrols_.set(controls::{{ ctrl.namespace }}{{ ctrl.name }},\n> +{%- if ctrl.size == 0 %}\n> +\t\t\t      Span<const {{ ctrl.element_type }}>(values.data(), size));\n> +{%- else %}\n> +\t\t\t      Span<const {{ ctrl.element_type }}, {{ ctrl.size }}>(values.data(), {{ ctrl.size }}));\n> +{%- endif %}\n> +{%- else %}\n> +\t\tconst GValue *element = value;\n> +\t\t{{ value_get|indent('\\t\\t') }}\n> +\t\tcontrols_.set(controls::{{ ctrl.namespace }}{{ ctrl.name }}, val);\n> +{%- endif %}\n> +\t\treturn true;\n> +\t}\n> +{%- endfor %}\n> +{%- endfor %}\n\nAdd a blank line here too.\n\n> +\tdefault:\n> +\t\treturn false;\n> +\t}\n> +}\n> +\n> +void GstCameraControls::setCamera(const std::shared_ptr<libcamera::Camera> &cam)\n> +{\n> +\tcapabilities_ = cam->controls();\n\nIs there a need to make a copy of the ControlInfoMap ? Can't we retain a\nreference to the camera and use it to access the list of controls ?\n\n> +\n> +\t// check the controls which were set before the camera capabilities were known\n> +\tControlList new_controls;\n> +\tfor (auto control = controls_.begin(); control != controls_.end(); ++control) {\n\nsetCamera() is called in gst_libcamera_src_open(). Does that occur\nbefore or after gst_libcamera_src_set_property() ? If it occurs before,\ncontrols_ will be empty here.\n\n> +\t\tunsigned int id = control->first;\n> +\t\tControlValue value = control->second;\n> +\n> +\t\tconst ControlId *cid = capabilities_.idmap().at(id);\n> +\t\tauto info = capabilities_.find(cid);\n> +\n> +\t\t// only add controls which are supported\n> +\t\tif (info != capabilities_.end()) {\n> +\t\t\tnew_controls.set(id, value);\n> +\t\t} else {\n> +\t\t\tGST_WARNING(\"Control '%s' is not supported by the camera and will be ignored\",\n> +\t\t\t\t    cid->name().c_str());\n\nThe control should have been rejected by setProperty() in this case, and\nwon't end up in controls_ in that case, so I don't think this can occur.\n\n> +\t\t}\n\nYou can drop the curly braces.\n\n> +\t}\n> +\n> +\tcontrols_ = new_controls;\n> +}\n> +\n> +void GstCameraControls::applyControls(std::unique_ptr<libcamera::Request> &request)\n> +{\n> +\trequest->controls().merge(controls_);\n\nI think you want to clear controls_ here, they don't need to be set in\nevery request. That's problematic for getControl() though, as it relies\non controls_ accumulating the state of all controls ever set. You'll\nprobably need two variables, one that accumulates controls, and one that\ndoesn't.\n\n> +}\n> +\n> +void GstCameraControls::readMetadata(libcamera::Request *request)\n> +{\n> +\tmetadata_ = request->metadata();\n> +}\n\nGreat handling of white space, the generated code is very readable.\n\n> diff --git a/src/gstreamer/gstlibcamera-controls.h b/src/gstreamer/gstlibcamera-controls.h\n> new file mode 100644\n> index 00000000..89b616ad\n> --- /dev/null\n> +++ b/src/gstreamer/gstlibcamera-controls.h\n> @@ -0,0 +1,43 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Collabora Ltd.\n> + *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> + *\n> + * GStreamer Camera Controls\n> + */\n> +\n> +#pragma once\n> +\n> +#include <memory>\n> +\n> +#include <libcamera/camera.h>\n> +#include <libcamera/controls.h>\n> +#include <libcamera/request.h>\n> +\n> +#include \"gstlibcamerasrc.h\"\n> +\n> +namespace libcamera {\n> +\n> +class GstCameraControls\n> +{\n> +public:\n> +\tstatic void installProperties(GObjectClass *klass, int lastProp);\n> +\n> +\tbool getProperty(guint propId, GValue *value, GParamSpec *pspec);\n> +\tbool setProperty(guint propId, const GValue *value, GParamSpec *pspec);\n> +\n> +\tvoid setCamera(const std::shared_ptr<libcamera::Camera> &cam);\n> +\n> +\tvoid applyControls(std::unique_ptr<libcamera::Request> &request);\n> +\tvoid readMetadata(libcamera::Request *request);\n> +\n> +private:\n> +\t/* supported controls and limits of camera */\n> +\tControlInfoMap capabilities_;\n> +\t/* set of user modified controls */\n> +\tControlList controls_;\n> +\t/* metadata returned by the camera */\n> +\tControlList metadata_;\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 40b787c8..8efa25f4 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -37,10 +37,11 @@\n>  \n>  #include <gst/base/base.h>\n>  \n> +#include \"gstlibcamera-controls.h\"\n> +#include \"gstlibcamera-utils.h\"\n>  #include \"gstlibcameraallocator.h\"\n>  #include \"gstlibcamerapad.h\"\n>  #include \"gstlibcamerapool.h\"\n> -#include \"gstlibcamera-utils.h\"\n>  \n>  using namespace libcamera;\n>  \n> @@ -128,6 +129,7 @@ struct GstLibcameraSrcState {\n>  \n>  \tControlList initControls_;\n>  \tguint group_id_;\n> +\tGstCameraControls controls_;\n>  \n>  \tint queueRequest();\n>  \tvoid requestCompleted(Request *request);\n> @@ -153,6 +155,7 @@ struct _GstLibcameraSrc {\n>  enum {\n>  \tPROP_0,\n>  \tPROP_CAMERA_NAME,\n> +\tPROP_LAST\n>  };\n>  \n>  static void gst_libcamera_src_child_proxy_init(gpointer g_iface,\n> @@ -183,6 +186,9 @@ int GstLibcameraSrcState::queueRequest()\n>  \tif (!request)\n>  \t\treturn -ENOMEM;\n>  \n> +\t/* Apply controls */\n> +\tcontrols_.applyControls(request);\n> +\n>  \tstd::unique_ptr<RequestWrap> wrap =\n>  \t\tstd::make_unique<RequestWrap>(std::move(request));\n>  \n> @@ -226,6 +232,9 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n>  \n>  \t{\n>  \t\tGLibLocker locker(&lock_);\n> +\n> +\t\tcontrols_.readMetadata(request);\n> +\n>  \t\twrap = std::move(queuedRequests_.front());\n>  \t\tqueuedRequests_.pop();\n>  \t}\n> @@ -408,6 +417,8 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n>  \t\treturn false;\n>  \t}\n>  \n> +\tself->state->controls_.setCamera(cam);\n> +\n>  \tcam->requestCompleted.connect(self->state, &GstLibcameraSrcState::requestCompleted);\n>  \n>  \t/* No need to lock here, we didn't start our threads yet. */\n> @@ -722,6 +733,7 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,\n>  {\n>  \tGLibLocker lock(GST_OBJECT(object));\n>  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);\n> +\tGstLibcameraSrcState *state = self->state;\n>  \n>  \tswitch (prop_id) {\n>  \tcase PROP_CAMERA_NAME:\n> @@ -729,7 +741,8 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,\n>  \t\tself->camera_name = g_value_dup_string(value);\n>  \t\tbreak;\n>  \tdefault:\n> -\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> +\t\tif (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))\n> +\t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n>  \t\tbreak;\n>  \t}\n>  }\n> @@ -740,13 +753,15 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,\n>  {\n>  \tGLibLocker lock(GST_OBJECT(object));\n>  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);\n> +\tGstLibcameraSrcState *state = self->state;\n>  \n>  \tswitch (prop_id) {\n>  \tcase PROP_CAMERA_NAME:\n>  \t\tg_value_set_string(value, self->camera_name);\n>  \t\tbreak;\n>  \tdefault:\n> -\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> +\t\tif (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))\n> +\t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n>  \t\tbreak;\n>  \t}\n>  }\n> @@ -947,6 +962,7 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>  \t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n>  \tg_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);\n>  \n> +\tGstCameraControls::installProperties(object_class, PROP_LAST);\n>  }\n>  \n>  /* GstChildProxy implementation */\n> diff --git a/src/gstreamer/meson.build b/src/gstreamer/meson.build\n> index c2a01e7b..6b7e53b5 100644\n> --- a/src/gstreamer/meson.build\n> +++ b/src/gstreamer/meson.build\n> @@ -25,6 +25,16 @@ libcamera_gst_sources = [\n>      'gstlibcamerasrc.cpp',\n>  ]\n>  \n> +# Generate gstreamer control properties\n> +\n> +gen_gst_controls_template = files('gstlibcamera-controls.cpp.in')\n> +libcamera_gst_sources += custom_target('gstlibcamera-controls.cpp',\n> +                                       input : controls_files,\n> +                                       output : 'gstlibcamera-controls.cpp',\n> +                                       command : [gen_gst_controls, '-o', '@OUTPUT@',\n> +                                                  '-t', gen_gst_controls_template, '@INPUT@'],\n> +                                       env : py_build_env)\n> +\n>  libcamera_gst_cpp_args = [\n>      '-DVERSION=\"@0@\"'.format(libcamera_git_version),\n>      '-DPACKAGE=\"@0@\"'.format(meson.project_name()),\n> diff --git a/utils/codegen/controls.py b/utils/codegen/controls.py\n> index 7bafee59..03c77cc6 100644\n> --- a/utils/codegen/controls.py\n> +++ b/utils/codegen/controls.py\n> @@ -110,3 +110,11 @@ class Control(object):\n>              return f\"Span<const {typ}, {self.__size}>\"\n>          else:\n>              return f\"Span<const {typ}>\"\n> +\n> +    @property\n> +    def element_type(self):\n> +        return self.__data.get('type')\n\nI wonder if we shouldn't move the existing type property to\nextend_control() in generator scripts, and rename element_type to type.\nThis can be done on top, you don't have to address it.\n\n> +\n> +    @property\n> +    def size(self):\n> +        return self.__size\n> diff --git a/utils/codegen/gen-gst-controls.py b/utils/codegen/gen-gst-controls.py\n> new file mode 100755\n> index 00000000..5ac8981f\n> --- /dev/null\n> +++ b/utils/codegen/gen-gst-controls.py\n> @@ -0,0 +1,151 @@\n> +#!/usr/bin/env python3\n> +# SPDX-License-Identifier: GPL-2.0-or-later\n> +# Copyright (C) 2019, Google Inc.\n> +# Copyright (C) 2024, Jaslo Ziska\n> +#\n> +# Authors:\n> +# Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> +# Jaslo Ziska <jaslo@ziska.de>\n> +#\n> +# Generate gstreamer control properties from YAML\n> +\n> +import argparse\n> +import jinja2\n> +import re\n> +import sys\n> +import yaml\n> +\n> +from controls import Control\n> +\n> +\n> +def find_common_prefix(strings):\n> +    prefix = strings[0]\n> +\n> +    for string in strings[1:]:\n> +        while string[:len(prefix)] != prefix and prefix:\n> +            prefix = prefix[:len(prefix) - 1]\n> +        if not prefix:\n> +            break\n> +\n> +    return prefix\n> +\n> +\n> +def format_description(description):\n> +    # Substitute doxygen keywords \\sa (see also) and \\todo\n> +    description = re.sub(r'\\\\sa((?: \\w+)+)',\n> +                         lambda match: 'See also: ' + ', '.join(\n> +                             map(kebab_case, match.group(1).strip().split(' '))\n> +                         ) + '.', description)\n> +    description = re.sub(r'\\\\todo', 'Todo:', description)\n> +\n> +    description = description.strip().split('\\n')\n> +    return '\\n'.join([\n> +        '\"' + line.replace('\\\\', r'\\\\').replace('\"', r'\\\"') + ' \"' for line in description if line\n> +    ]).rstrip()\n> +\n> +\n> +def snake_case(s):\n> +    return ''.join([\n> +        c.isupper() and ('_' + c.lower()) or c for c in s\n> +    ]).strip('_')\n> +\n> +\n> +def kebab_case(s):\n> +    return snake_case(s).replace('_', '-')\n> +\n> +\n> +def extend_control(ctrl):\n> +    if ctrl.vendor != 'libcamera':\n> +        ctrl.namespace = f'{ctrl.vendor}::'\n> +        ctrl.vendor_prefix = f'{ctrl.vendor}-'\n> +    else:\n> +        ctrl.namespace = ''\n> +        ctrl.vendor_prefix = ''\n> +\n> +    ctrl.is_array = ctrl.size is not None\n> +\n> +    if ctrl.is_enum:\n> +        # remove common prefix from enum variant names\n\ns/# remove/# Remove/\n\n> +        prefix = find_common_prefix([enum.name for enum in ctrl.enum_values])\n> +        for enum in ctrl.enum_values:\n> +            enum.gst_name = kebab_case(enum.name.removeprefix(prefix))\n> +\n> +        ctrl.gtype = 'enum'\n> +        ctrl.default = '0'\n> +    elif ctrl.element_type == 'bool':\n> +        ctrl.gtype = 'boolean'\n> +        ctrl.default = 'false'\n> +    elif ctrl.element_type == 'float':\n> +        ctrl.gtype = 'float'\n> +        ctrl.default = '0'\n> +        ctrl.min = '-G_MAXFLOAT'\n> +        ctrl.max = 'G_MAXFLOAT'\n> +    elif ctrl.element_type == 'int32_t':\n> +        ctrl.gtype = 'int'\n> +        ctrl.default = '0'\n> +        ctrl.min = 'G_MININT'\n> +        ctrl.max = 'G_MAXINT'\n> +    elif ctrl.element_type == 'int64_t':\n> +        ctrl.gtype = 'int64'\n> +        ctrl.default = '0'\n> +        ctrl.min = 'G_MININT64'\n> +        ctrl.max = 'G_MAXINT64'\n> +    elif ctrl.element_type == 'uint8_t':\n> +        ctrl.gtype = 'uchar'\n> +        ctrl.default = '0'\n> +        ctrl.min = '0'\n> +        ctrl.max = 'G_MAXUINT8'\n> +    elif ctrl.element_type == 'Rectangle':\n> +        ctrl.is_rectangle = True\n> +        ctrl.default = '0'\n> +        ctrl.min = '0'\n> +        ctrl.max = 'G_MAXINT'\n> +    else:\n> +        raise RuntimeError(f'The type `{ctrl.element_type}` is unknown')\n> +\n> +    return ctrl\n> +\n> +\n> +def main(argv):\n> +    # Parse command line arguments\n> +    parser = argparse.ArgumentParser()\n> +    parser.add_argument('--output', '-o', metavar='file', type=str,\n> +                        help='Output file name. Defaults to standard output if not specified.')\n> +    parser.add_argument('--template', '-t', dest='template', type=str, required=True,\n> +                        help='Template file name.')\n> +    parser.add_argument('input', type=str, nargs='+',\n> +                        help='Input file name.')\n> +\n> +    args = parser.parse_args(argv[1:])\n> +\n> +    controls = {}\n> +    for input in args.input:\n> +        data = yaml.safe_load(open(input, 'rb').read())\n> +\n> +        vendor = data['vendor']\n> +        ctrls = controls.setdefault(vendor, [])\n> +\n> +        for ctrl in data['controls']:\n> +            ctrl = Control(*ctrl.popitem(), vendor)\n> +            ctrls.append(extend_control(ctrl))\n> +\n> +    data = {'controls': list(controls.items())}\n> +\n> +    env = jinja2.Environment()\n> +    env.filters['format_description'] = format_description\n> +    env.filters['snake_case'] = snake_case\n> +    env.filters['kebab_case'] = kebab_case\n> +    template = env.from_string(open(args.template, 'r', encoding='utf-8').read())\n> +    string = template.render(data)\n> +\n> +    if args.output:\n> +        with open(args.output, 'w', encoding='utf-8') as output:\n> +            output.write(string)\n> +    else:\n> +        sys.stdout.write(string)\n> +\n> +    return 0\n> +\n> +\n> +if __name__ == '__main__':\n> +    sys.exit(main(sys.argv))\n> diff --git a/utils/codegen/meson.build b/utils/codegen/meson.build\n> index adf33bba..904dd66d 100644\n> --- a/utils/codegen/meson.build\n> +++ b/utils/codegen/meson.build\n> @@ -11,6 +11,7 @@ py_modules += ['jinja2', 'yaml']\n>  \n>  gen_controls = files('gen-controls.py')\n>  gen_formats = files('gen-formats.py')\n> +gen_gst_controls = files('gen-gst-controls.py')\n>  gen_header = files('gen-header.sh')\n>  gen_ipa_pub_key = files('gen-ipa-pub-key.py')\n>  gen_tracepoints = files('gen-tp-header.py')","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 248EEC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 16 Aug 2024 00:08:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1D4FA633B3;\n\tFri, 16 Aug 2024 02:08:13 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A14F363369\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 16 Aug 2024 02:08:10 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8C4E3471;\n\tFri, 16 Aug 2024 02:07:11 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KKchjdGV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723766831;\n\tbh=BWri7hkdOUn4ZGbRaVWa3rUzxOSbY+FTBQGz5rm6xAU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KKchjdGVgM87vd59SFhr2C7N9VZRn5Z2xRNHfUXJPEz3TJb3Y81C6scaEQgGj3BAs\n\tm3dK7Dl1AHqd5xHDC6roEAroxiM4tJB2hPzzv8kMelMSYHHYkN2Ro0s+UPLvbeVoCS\n\tcWop/IR/wsS95K+VCjE3TT3hRUFCr1JeuVhDwWVk=","Date":"Fri, 16 Aug 2024 03:07:44 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jaslo Ziska <jaslo@ziska.de>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","Message-ID":"<20240816000744.GE23911@pendragon.ideasonboard.com>","References":"<20240813124722.22425-1-jaslo@ziska.de>\n\t<20240813124722.22425-4-jaslo@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240813124722.22425-4-jaslo@ziska.de>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30919,"web_url":"https://patchwork.libcamera.org/comment/30919/","msgid":"<87y14idw2q.fsf@ziska.de>","date":"2024-08-27T11:49:49","subject":"Re: [PATCH v2 3/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/people/173/","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"content":"Hi Laurent,\n\nthanks for the review.\n\nLaurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n> Hi Jaslo,\n>\n> Thank you for the patch.\n>\n> On Tue, Aug 13, 2024 at 02:25:07PM +0200, Jaslo Ziska wrote:\n>> This commit implements gstreamer controls for the libcamera \n>> element by\n>> generating the controls from the control_ids_*.yaml files using \n>> a new\n>> gen-gst-controls.py script. The appropriate meson files are \n>> also changed\n>> to automatically run the script when building.\n>> \n>> The gen-gst-controls.py script works similar to the \n>> gen-controls.py\n>> script by parsing the control_ids_*.yaml files and generating \n>> C++ code\n>> for each control.\n>> For the controls to be used as gstreamer properties the type \n>> for each\n>> control needs to be translated to the appropriate glib type and \n>> a\n>> GEnumValue is generated for each enum control. Then a\n>> g_object_install_property(), _get_property() and \n>> _set_property()\n>> function is generated for each control.\n>> The vendor controls get prefixed with \"$vendor-\" in the final \n>> gstreamer\n>> property name.\n>> \n>> The C++ code generated by the gen-gst-controls.py script is \n>> written into\n>> the template gstlibcamerasrc-controls.cpp.in file. The matching\n>> gstlibcamerasrc-controls.h header defines the GstCameraControls \n>> class\n>> which handles the installation of the gstreamer properties as \n>> well as\n>> keeping track of the control values and setting and getting the\n>> controls. The content of these functions is generated in the \n>> Python\n>> script.\n>> \n>> Finally the libcamerasrc element itself is edited to make use \n>> of the new\n>> GstCameraControls class. The way this works is by defining a \n>> PROP_LAST\n>> enum variant which is passed to the installProperties() \n>> function so the\n>> properties are defined with the appropriate offset. When \n>> getting or\n>> setting a property PROP_LAST is subtracted from the requested \n>> property\n>> to translate the control back into a libcamera::controls:: enum\n>> variant.\n>\n> Looks good to me. I'll focus the review on the parts not related \n> to the\n> GStreamer internal APIs, and will let Nicolas bring his \n> GStreamer\n> expertise.\n>\n>> Signed-off-by: Jaslo Ziska <jaslo@ziska.de>\n>> ---\n>>  src/gstreamer/gstlibcamera-controls.cpp.in | 296 \n>>  +++++++++++++++++++++\n>>  src/gstreamer/gstlibcamera-controls.h      |  43 +++\n>>  src/gstreamer/gstlibcamerasrc.cpp          |  22 +-\n>>  src/gstreamer/meson.build                  |  10 +\n>>  utils/codegen/controls.py                  |   8 +\n>>  utils/codegen/gen-gst-controls.py          | 151 +++++++++++\n>>  utils/codegen/meson.build                  |   1 +\n>>  7 files changed, 528 insertions(+), 3 deletions(-)\n>>  create mode 100644 src/gstreamer/gstlibcamera-controls.cpp.in\n>>  create mode 100644 src/gstreamer/gstlibcamera-controls.h\n>>  create mode 100755 utils/codegen/gen-gst-controls.py\n>> \n>> diff --git a/src/gstreamer/gstlibcamera-controls.cpp.in \n>> b/src/gstreamer/gstlibcamera-controls.cpp.in\n>> new file mode 100644\n>> index 00000000..aab7ae24\n>> --- /dev/null\n>> +++ b/src/gstreamer/gstlibcamera-controls.cpp.in\n>> @@ -0,0 +1,296 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2023, Collabora Ltd.\n>> + *     Author: Nicolas Dufresne \n>> <nicolas.dufresne@collabora.com>\n>\n> I think you can attribute the copyright on this file to \n> yourself.\n>\n>> + *\n>> + * GStreamer Camera Controls\n>> + *\n>> + * This file is auto-generated. Do not edit.\n>> + */\n>> +\n>> +#include <vector>\n>> +\n>> +#include <libcamera/control_ids.h>\n>\n> And\n>\n> #include <libcamera/controls.h>\n> #include <libcamera/geometry.h>\n>\n> as you use classes defined in those headers below.\n>\n>> +\n>> +#include \"gstlibcamera-controls.h\"\n>> +\n>> +using namespace libcamera;\n>> +\n>> +{% for vendor, ctrls in controls %}\n>> +{%- for ctrl in ctrls if ctrl.is_enum %}\n>> +static const GEnumValue {{ ctrl.name|snake_case }}_types[] = {\n>> +{%- for enum in ctrl.enum_values %}\n>> +\t{\n>> +\t\tcontrols::{{ ctrl.namespace }}{{ enum.name }},\n>> +\t\t{{ enum.description|format_description|indent('\\t\\t') \n>> }},\n>> +\t\t\"{{ enum.gst_name }}\"\n>> +\t},\n>> +{%- endfor %}\n>> +\t{0, NULL, NULL}\n>> +};\n>> +\n>> +#define TYPE_{{ ctrl.name|snake_case|upper }} \\\n>> +\t({{ ctrl.name|snake_case }}_get_type())\n>> +static GType {{ ctrl.name|snake_case }}_get_type()\n>> +{\n>> +\tstatic GType {{ ctrl.name|snake_case }}_type = 0;\n>> +\n>> +\tif (!{{ ctrl.name|snake_case }}_type)\n>> +\t\t{{ ctrl.name|snake_case }}_type =\n>> +\t\t\tg_enum_register_static(\"{{ ctrl.name }}\",\n>> +\t\t\t\t\t       {{ ctrl.name|snake_case }}_types);\n>> +\n>> +\treturn {{ ctrl.name|snake_case }}_type;\n>> +}\n>> +{% endfor %}\n>> +{%- endfor %}\n>> +\n>> +void GstCameraControls::installProperties(GObjectClass *klass, \n>> int lastPropId)\n>> +{\n>> +{%- for vendor, ctrls in controls %}\n>> +{%- for ctrl in ctrls %}\n>> +\n>> +{%- set spec %}\n>\n> I didn't know about set in jinja templates, interesting.\n>\n>> +{%- if ctrl.is_rectangle -%}\n>> +gst_param_spec_array(\n>> +{%- else -%}\n>> +g_param_spec_{{ ctrl.gtype }}(\n>> +{%- endif -%}\n>> +{%- if ctrl.is_array %}\n>> +\t\"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case \n>> }}-value\",\n>> +\t\"{{ ctrl.name }} Value\",\n>> +\t\"One {{ ctrl.name }} element value\",\n>> +{%- else %}\n>> +\t\"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case }}\",\n>> +\t\"{{ ctrl.name }}\",\n>> +\t{{ ctrl.description|format_description|indent('\\t') }},\n>> +{%- endif %}\n>> +{%- if ctrl.is_enum %}\n>> +\tTYPE_{{ ctrl.name|snake_case|upper }},\n>> +\t{{ ctrl.default }},\n>> +{%- elif ctrl.is_rectangle %}\n>> +\tg_param_spec_int(\n>> +\t\t\"rectangle-value\",\n>> +\t\t\"Rectangle Value\",\n>> +\t\t\"One rectangle value, either x, y, width or height.\",\n>> +\t\t{{ ctrl.min }}, {{ ctrl.max }}, {{ ctrl.default }},\n>> +\t\t(GParamFlags) (GST_PARAM_CONTROLLABLE | \n>> G_PARAM_READWRITE |\n>> +\t\t\t       G_PARAM_STATIC_STRINGS)\n>> +\t),\n>> +{%- elif ctrl.gtype == 'boolean' %}\n>> +\t{{ ctrl.default }},\n>> +{%- elif ctrl.gtype in ['float', 'int', 'int64', 'uchar'] %}\n>> +\t{{ ctrl.min }}, {{ ctrl.max }}, {{ ctrl.default }},\n>> +{%- endif %}\n>> +\t(GParamFlags) (GST_PARAM_CONTROLLABLE | G_PARAM_READWRITE \n>> |\n>> +\t\t       G_PARAM_STATIC_STRINGS)\n>> +)\n>> +{%- endset %}\n>> +\n>> +\tg_object_class_install_property(\n>> +\t\tklass,\n>> +\t\tlastPropId + controls::{{ ctrl.namespace }}{{ \n>> ctrl.name|snake_case|upper }},\n>> +{%- if ctrl.is_array %}\n>> +\t\tgst_param_spec_array(\n>> +\t\t\t\"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case \n>> }}\",\n>> +\t\t\t\"{{ ctrl.name }}\",\n>> +\t\t\t{{ \n>> ctrl.description|format_description|indent('\\t\\t\\t') }},\n>> +\t\t\t{{ spec|indent('\\t\\t\\t') }},\n>> +\t\t\t(GParamFlags) (GST_PARAM_CONTROLLABLE |\n>> +\t\t\t\t       G_PARAM_READWRITE |\n>> +\t\t\t\t       G_PARAM_STATIC_STRINGS)\n>> +\t\t)\n>> +{%- else %}\n>> +\t\t{{ spec|indent('\\t\\t') }}\n>> +{%- endif %}\n>> +\t);\n>> +{%- endfor %}\n>> +{%- endfor %}\n>> +}\n>> +\n>> +bool GstCameraControls::getProperty(guint propId, GValue \n>> *value,\n>> +\t\t\t\t    [[maybe_unused]] GParamSpec *pspec)\n>> +{\n>> +\tswitch (propId) {\n>> +{%- for vendor, ctrls in controls %}\n>> +{%- for ctrl in ctrls %}\n>> +\n>> +{%- set value_set %}\n>> +{%- if ctrl.is_rectangle -%}\n>> +Point top_left = val.topLeft();\n>> +Size size = val.size();\n>> +\n>> +GValue x = G_VALUE_INIT;\n>> +g_value_init(&x, G_TYPE_INT);\n>> +g_value_set_int(&x, top_left.x);\n>> +gst_value_array_append_and_take_value(&element, &x);\n>> +\n>> +GValue y = G_VALUE_INIT;\n>> +g_value_init(&y, G_TYPE_INT);\n>> +g_value_set_int(&y, top_left.y);\n>> +gst_value_array_append_and_take_value(&element, &y);\n>> +\n>> +GValue width = G_VALUE_INIT;\n>> +g_value_init(&width, G_TYPE_INT);\n>> +g_value_set_int(&width, size.width);\n>> +gst_value_array_append_and_take_value(&element, &width);\n>> +\n>> +GValue height = G_VALUE_INIT;\n>> +g_value_init(&height, G_TYPE_INT);\n>> +g_value_set_int(&x, size.height);\n>> +gst_value_array_append_and_take_value(&element, &height);\n>> +{%- else -%}\n>> +g_value_set_{{ ctrl.gtype }}(&element, val);\n>> +{%- endif -%}\n>> +{%- endset %}\n>> +\n>> +\tcase controls::{{ ctrl.namespace }}{{ \n>> ctrl.name|snake_case|upper }}: {\n>> +\t\tauto control = metadata_.get(controls::{{ \n>> ctrl.namespace }}{{ ctrl.name }});\n>> +\t\tcontrol = control ? control :\n>> +\t\t\t  controls_.get(controls::{{ ctrl.namespace }}{{ \n>> ctrl.name }});\n>\n> \t\tif (!control)\n> \t\t\tcontrol = controls_.get(controls::{{ ctrl.namespace \n> }}{{ ctrl.name }});\n>\n>> +\t\tif (!control) {\n>> +\t\t\tGST_WARNING(\"Control '%s' is not available, \n>> default value will be returned\",\n>> +\t\t\t\t    controls::{{ ctrl.namespace }}{{ ctrl.name \n>> }}.name().c_str());\n>> +\t\t\treturn true;\n>> +\t\t}\n>> +\n>> +{%- if ctrl.is_array %}\n>> +\t\tfor (size_t i = 0; i < control->size(); ++i) {\n>> +\t\t\tGValue element = G_VALUE_INIT;\n>> +{%- if ctrl.is_rectangle %}\n>> +\t\t\tg_value_init(&element, GST_TYPE_PARAM_ARRAY_LIST);\n>> +{%- else %}\n>> +\t\t\tg_value_init(&element, G_TYPE_{{ ctrl.gtype|upper \n>> }});\n>> +{%- endif %}\n>> +\t\t\tauto val = (*control)[i];\n>> +\t\t\t{{ value_set|indent('\\t\\t\\t\\t') }}\n>> +\t\t\tgst_value_array_append_and_take_value(value, \n>> &element);\n>> +\t\t}\n>> +{%- else %}\n>> +\t\tGValue element = *value;\n>> +\t\tauto val = *control;\n>> +\t\t{{ value_set|indent('\\t\\t\\t') }}\n>> +{%- endif %}\n>> +\n>> +\t\treturn true;\n>> +\t}\n>> +{%- endfor %}\n>> +{%- endfor %}\n>\n> Add a blank line here as there's one between all the cases \n> above.\n>\n>> +\tdefault:\n>> +\t\treturn false;\n>> +\t}\n>> +}\n>> +\n>> +bool GstCameraControls::setProperty(guint propId, const GValue \n>> *value,\n>> +\t\t\t\t    [[maybe_unused]] GParamSpec *pspec)\n>> +{\n>> +\t// check whether the camera capabilities are available\n>\n> Please use C-style comments, start sentences with a capital \n> letter, and\n> end them with a period.\n>\n> \t/* Check whether the camera capabilities are available. */\n>\n>> +\tif (!capabilities_.empty()) {\n>> +\t\t// if so, check that the control is supported\n>\n> \t\t/* If so, check that the control is supported. */\n>\n> Same below\n>\n>> +\t\tconst ControlId *cid = \n>> capabilities_.idmap().at(propId);\n>> +\t\tauto info = capabilities_.find(cid);\n>> +\n>> +\t\tif (info == capabilities_.end()) {\n>> +\t\t\tGST_WARNING(\"Control '%s' is not supported by the \n>> camera and will be ignored\",\n>> +\t\t\t\t    cid->name().c_str());\n>> +\t\t\treturn true;\n>> +\t\t}\n>> +\t}\n>> +\n>> +\tswitch (propId) {\n>> +{%- for vendor, ctrls in controls %}\n>> +{%- for ctrl in ctrls %}\n>> +\n>> +{%- set value_get %}\n>> +{%- if ctrl.is_rectangle -%}\n>> +if (gst_value_array_get_size(element) != 4) {\n>> +\tGST_ERROR(\"Rectangle must be an array of size 4\");\n>\n> Maybe print the name of the control to make debugging easier ?\n\nGood idea, I will add that. It might be a little problematic when \nextracting this part to a separate helper function as the Jinja \ncontext will then not be available anymore for the name but I will \ntry think of something (maybe passing the control name to the \nfunction?).\n\n>> +\treturn true;\n>> +}\n>> +\n>> +const GValue *r;\n>> +r = gst_value_array_get_value(element, 0);\n>> +int x = g_value_get_int(r);\n>> +r = gst_value_array_get_value(element, 1);\n>> +int y = g_value_get_int(r);\n>> +r = gst_value_array_get_value(element, 2);\n>> +int w = g_value_get_int(r);\n>> +r = gst_value_array_get_value(element, 3);\n>> +int h = g_value_get_int(r);\n>> +\n>> +auto val = Rectangle(x, y, w, h);\n>\n> Would it make sense to move all this (including the size check) \n> to a\n> helper function instead of duplicating the code for each \n> rectangle\n> control ?\n\nGood idea, that way I might also be able to drop the Jinja set \ndirective and use it directly.\n\n>> +{%- else -%}\n>> +auto val = g_value_get_{{ ctrl.gtype }}(element);\n>> +{%- endif -%}\n>> +{%- endset %}\n>> +\n>> +\tcase controls::{{ ctrl.namespace }}{{ \n>> ctrl.name|snake_case|upper }}: {\n>> +{%- if ctrl.is_array %}\n>> +\t\tsize_t size = gst_value_array_get_size(value);\n>> +{%- if ctrl.size != 0 %}\n>> +\t\tif (size != {{ ctrl.size }}) {\n>> +\t\t\tGST_ERROR(\"Incorrect array size, must be of size \n>> {{ ctrl.size }}\");\n>> +\t\t\treturn true;\n>> +\t\t}\n>> +{%- endif %}\n>> +\n>> +\t\tstd::vector<{{ ctrl.element_type }}> values(size);\n>> +\t\tfor (size_t i = 0; i < size; ++i) {\n>> +\t\t\tconst GValue *element = \n>> gst_value_array_get_value(value, i);\n>> +\t\t\t{{ value_get|indent('\\t\\t\\t') }}\n>> +\t\t\tvalues[i] = val;\n>> +\t\t}\n>> +\t\tcontrols_.set(controls::{{ ctrl.namespace }}{{ \n>> ctrl.name }},\n>> +{%- if ctrl.size == 0 %}\n>> +\t\t\t      Span<const {{ ctrl.element_type \n>> }}>(values.data(), size));\n>> +{%- else %}\n>> +\t\t\t      Span<const {{ ctrl.element_type }}, {{ \n>> ctrl.size }}>(values.data(), {{ ctrl.size }}));\n>> +{%- endif %}\n>> +{%- else %}\n>> +\t\tconst GValue *element = value;\n>> +\t\t{{ value_get|indent('\\t\\t') }}\n>> +\t\tcontrols_.set(controls::{{ ctrl.namespace }}{{ \n>> ctrl.name }}, val);\n>> +{%- endif %}\n>> +\t\treturn true;\n>> +\t}\n>> +{%- endfor %}\n>> +{%- endfor %}\n>\n> Add a blank line here too.\n>\n>> +\tdefault:\n>> +\t\treturn false;\n>> +\t}\n>> +}\n>> +\n>> +void GstCameraControls::setCamera(const \n>> std::shared_ptr<libcamera::Camera> &cam)\n>> +{\n>> +\tcapabilities_ = cam->controls();\n>\n> Is there a need to make a copy of the ControlInfoMap ? Can't we \n> retain a\n> reference to the camera and use it to access the list of \n> controls ?\n\nYou are right, I could just add a shared_ptr<Camera> member.\n\n>> +\n>> +\t// check the controls which were set before the camera \n>> capabilities were known\n>> +\tControlList new_controls;\n>> +\tfor (auto control = controls_.begin(); control != \n>> controls_.end(); ++control) {\n>\n> setCamera() is called in gst_libcamera_src_open(). Does that \n> occur\n> before or after gst_libcamera_src_set_property() ? If it occurs \n> before,\n> controls_ will be empty here.\n\nThis is a little complicated, gst_libcamera_src_set_property() can \nbe called before and after gst_libcamera_src_open(). So to be able \nto check the controls which were set before the pipeline is \nstarted I check the controls at this point because the camera \nbecomes available here.\n\n>> +\t\tunsigned int id = control->first;\n>> +\t\tControlValue value = control->second;\n>> +\n>> +\t\tconst ControlId *cid = capabilities_.idmap().at(id);\n>> +\t\tauto info = capabilities_.find(cid);\n>> +\n>> +\t\t// only add controls which are supported\n>> +\t\tif (info != capabilities_.end()) {\n>> +\t\t\tnew_controls.set(id, value);\n>> +\t\t} else {\n>> +\t\t\tGST_WARNING(\"Control '%s' is not supported by the \n>> camera and will be ignored\",\n>> +\t\t\t\t    cid->name().c_str());\n>\n> The control should have been rejected by setProperty() in this \n> case, and\n> won't end up in controls_ in that case, so I don't think this \n> can occur.\n\nBecause controls might be set before the camera is available the \nfirst line in setProperty() checks whether the camera capabilities \nare available and only checks and rejects the controls if they \nare. If the camera was not set when setProperty() is called the \ncontrol gets set unconditionally.\n\nThat is why the controls which were set before the camera became \navailable are checked at this point when the camera is set.\n\n>> +\t\t}\n>\n> You can drop the curly braces.\n>\n>> +\t}\n>> +\n>> +\tcontrols_ = new_controls;\n>> +}\n>> +\n>> +void \n>> GstCameraControls::applyControls(std::unique_ptr<libcamera::Request> \n>> &request)\n>> +{\n>> +\trequest->controls().merge(controls_);\n>\n> I think you want to clear controls_ here, they don't need to be \n> set in\n> every request. That's problematic for getControl() though, as it \n> relies\n> on controls_ accumulating the state of all controls ever set. \n> You'll\n> probably need two variables, one that accumulates controls, and \n> one that\n> doesn't.\n\nYou are right. I was mistaken about how the libcamera controls \nwork, for some reason I thought you had to repeatedly send the \ncontrol with every request. I will change this.\n\n>> +}\n>> +\n>> +void GstCameraControls::readMetadata(libcamera::Request \n>> *request)\n>> +{\n>> +\tmetadata_ = request->metadata();\n>> +}\n>\n> Great handling of white space, the generated code is very \n> readable.\n>\n>> diff --git a/src/gstreamer/gstlibcamera-controls.h \n>> b/src/gstreamer/gstlibcamera-controls.h\n>> new file mode 100644\n>> index 00000000..89b616ad\n>> --- /dev/null\n>> +++ b/src/gstreamer/gstlibcamera-controls.h\n>> @@ -0,0 +1,43 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2023, Collabora Ltd.\n>> + *     Author: Nicolas Dufresne \n>> <nicolas.dufresne@collabora.com>\n>> + *\n>> + * GStreamer Camera Controls\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include <memory>\n>> +\n>> +#include <libcamera/camera.h>\n>> +#include <libcamera/controls.h>\n>> +#include <libcamera/request.h>\n>> +\n>> +#include \"gstlibcamerasrc.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +class GstCameraControls\n>> +{\n>> +public:\n>> +\tstatic void installProperties(GObjectClass *klass, int \n>> lastProp);\n>> +\n>> +\tbool getProperty(guint propId, GValue *value, GParamSpec \n>> *pspec);\n>> +\tbool setProperty(guint propId, const GValue *value, \n>> GParamSpec *pspec);\n>> +\n>> +\tvoid setCamera(const std::shared_ptr<libcamera::Camera> \n>> &cam);\n>> +\n>> +\tvoid applyControls(std::unique_ptr<libcamera::Request> \n>> &request);\n>> +\tvoid readMetadata(libcamera::Request *request);\n>> +\n>> +private:\n>> +\t/* supported controls and limits of camera */\n>> +\tControlInfoMap capabilities_;\n>> +\t/* set of user modified controls */\n>> +\tControlList controls_;\n>> +\t/* metadata returned by the camera */\n>> +\tControlList metadata_;\n>> +};\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n>> b/src/gstreamer/gstlibcamerasrc.cpp\n>> index 40b787c8..8efa25f4 100644\n>> --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> @@ -37,10 +37,11 @@\n>>  \n>>  #include <gst/base/base.h>\n>>  \n>> +#include \"gstlibcamera-controls.h\"\n>> +#include \"gstlibcamera-utils.h\"\n>>  #include \"gstlibcameraallocator.h\"\n>>  #include \"gstlibcamerapad.h\"\n>>  #include \"gstlibcamerapool.h\"\n>> -#include \"gstlibcamera-utils.h\"\n>>  \n>>  using namespace libcamera;\n>>  \n>> @@ -128,6 +129,7 @@ struct GstLibcameraSrcState {\n>>  \n>>  \tControlList initControls_;\n>>  \tguint group_id_;\n>> +\tGstCameraControls controls_;\n>>  \n>>  \tint queueRequest();\n>>  \tvoid requestCompleted(Request *request);\n>> @@ -153,6 +155,7 @@ struct _GstLibcameraSrc {\n>>  enum {\n>>  \tPROP_0,\n>>  \tPROP_CAMERA_NAME,\n>> +\tPROP_LAST\n>>  };\n>>  \n>>  static void gst_libcamera_src_child_proxy_init(gpointer \n>>  g_iface,\n>> @@ -183,6 +186,9 @@ int GstLibcameraSrcState::queueRequest()\n>>  \tif (!request)\n>>  \t\treturn -ENOMEM;\n>>  \n>> +\t/* Apply controls */\n>> +\tcontrols_.applyControls(request);\n>> +\n>>  \tstd::unique_ptr<RequestWrap> wrap =\n>>  \t\tstd::make_unique<RequestWrap>(std::move(request));\n>>  \n>> @@ -226,6 +232,9 @@ \n>> GstLibcameraSrcState::requestCompleted(Request *request)\n>>  \n>>  \t{\n>>  \t\tGLibLocker locker(&lock_);\n>> +\n>> +\t\tcontrols_.readMetadata(request);\n>> +\n>>  \t\twrap = std::move(queuedRequests_.front());\n>>  \t\tqueuedRequests_.pop();\n>>  \t}\n>> @@ -408,6 +417,8 @@ gst_libcamera_src_open(GstLibcameraSrc \n>> *self)\n>>  \t\treturn false;\n>>  \t}\n>>  \n>> +\tself->state->controls_.setCamera(cam);\n>> +\n>>  \tcam->requestCompleted.connect(self->state, \n>>  &GstLibcameraSrcState::requestCompleted);\n>>  \n>>  \t/* No need to lock here, we didn't start our threads yet. \n>>  */\n>> @@ -722,6 +733,7 @@ gst_libcamera_src_set_property(GObject \n>> *object, guint prop_id,\n>>  {\n>>  \tGLibLocker lock(GST_OBJECT(object));\n>>  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);\n>> +\tGstLibcameraSrcState *state = self->state;\n>>  \n>>  \tswitch (prop_id) {\n>>  \tcase PROP_CAMERA_NAME:\n>> @@ -729,7 +741,8 @@ gst_libcamera_src_set_property(GObject \n>> *object, guint prop_id,\n>>  \t\tself->camera_name = g_value_dup_string(value);\n>>  \t\tbreak;\n>>  \tdefault:\n>> -\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, \n>> pspec);\n>> +\t\tif (!state->controls_.setProperty(prop_id - PROP_LAST, \n>> value, pspec))\n>> +\t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, \n>> pspec);\n>>  \t\tbreak;\n>>  \t}\n>>  }\n>> @@ -740,13 +753,15 @@ gst_libcamera_src_get_property(GObject \n>> *object, guint prop_id, GValue *value,\n>>  {\n>>  \tGLibLocker lock(GST_OBJECT(object));\n>>  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);\n>> +\tGstLibcameraSrcState *state = self->state;\n>>  \n>>  \tswitch (prop_id) {\n>>  \tcase PROP_CAMERA_NAME:\n>>  \t\tg_value_set_string(value, self->camera_name);\n>>  \t\tbreak;\n>>  \tdefault:\n>> -\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, \n>> pspec);\n>> +\t\tif (!state->controls_.getProperty(prop_id - PROP_LAST, \n>> value, pspec))\n>> +\t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, \n>> pspec);\n>>  \t\tbreak;\n>>  \t}\n>>  }\n>> @@ -947,6 +962,7 @@ \n>> gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n>>  \t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n>>  \tg_object_class_install_property(object_class, \n>>  PROP_CAMERA_NAME, spec);\n>>  \n>> +\tGstCameraControls::installProperties(object_class, \n>> PROP_LAST);\n>>  }\n>>  \n>>  /* GstChildProxy implementation */\n>> diff --git a/src/gstreamer/meson.build \n>> b/src/gstreamer/meson.build\n>> index c2a01e7b..6b7e53b5 100644\n>> --- a/src/gstreamer/meson.build\n>> +++ b/src/gstreamer/meson.build\n>> @@ -25,6 +25,16 @@ libcamera_gst_sources = [\n>>      'gstlibcamerasrc.cpp',\n>>  ]\n>>  \n>> +# Generate gstreamer control properties\n>> +\n>> +gen_gst_controls_template = \n>> files('gstlibcamera-controls.cpp.in')\n>> +libcamera_gst_sources += \n>> custom_target('gstlibcamera-controls.cpp',\n>> +                                       input : controls_files,\n>> +                                       output : \n>> 'gstlibcamera-controls.cpp',\n>> +                                       command : \n>> [gen_gst_controls, '-o', '@OUTPUT@',\n>> +                                                  '-t', \n>> gen_gst_controls_template, '@INPUT@'],\n>> +                                       env : py_build_env)\n>> +\n>>  libcamera_gst_cpp_args = [\n>>      '-DVERSION=\"@0@\"'.format(libcamera_git_version),\n>>      '-DPACKAGE=\"@0@\"'.format(meson.project_name()),\n>> diff --git a/utils/codegen/controls.py \n>> b/utils/codegen/controls.py\n>> index 7bafee59..03c77cc6 100644\n>> --- a/utils/codegen/controls.py\n>> +++ b/utils/codegen/controls.py\n>> @@ -110,3 +110,11 @@ class Control(object):\n>>              return f\"Span<const {typ}, {self.__size}>\"\n>>          else:\n>>              return f\"Span<const {typ}>\"\n>> +\n>> +    @property\n>> +    def element_type(self):\n>> +        return self.__data.get('type')\n>\n> I wonder if we shouldn't move the existing type property to\n> extend_control() in generator scripts, and rename element_type \n> to type.\n> This can be done on top, you don't have to address it.\n\nI'm fine with either, although just calling it type() is a little \nsimpler for the gen-gst-controls.py script.\n\n>> +\n>> +    @property\n>> +    def size(self):\n>> +        return self.__size\n>> diff --git a/utils/codegen/gen-gst-controls.py \n>> b/utils/codegen/gen-gst-controls.py\n>> new file mode 100755\n>> index 00000000..5ac8981f\n>> --- /dev/null\n>> +++ b/utils/codegen/gen-gst-controls.py\n>> @@ -0,0 +1,151 @@\n>> +#!/usr/bin/env python3\n>> +# SPDX-License-Identifier: GPL-2.0-or-later\n>> +# Copyright (C) 2019, Google Inc.\n>> +# Copyright (C) 2024, Jaslo Ziska\n>> +#\n>> +# Authors:\n>> +# Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>> +# Jaslo Ziska <jaslo@ziska.de>\n>> +#\n>> +# Generate gstreamer control properties from YAML\n>> +\n>> +import argparse\n>> +import jinja2\n>> +import re\n>> +import sys\n>> +import yaml\n>> +\n>> +from controls import Control\n>> +\n>> +\n>> +def find_common_prefix(strings):\n>> +    prefix = strings[0]\n>> +\n>> +    for string in strings[1:]:\n>> +        while string[:len(prefix)] != prefix and prefix:\n>> +            prefix = prefix[:len(prefix) - 1]\n>> +        if not prefix:\n>> +            break\n>> +\n>> +    return prefix\n>> +\n>> +\n>> +def format_description(description):\n>> +    # Substitute doxygen keywords \\sa (see also) and \\todo\n>> +    description = re.sub(r'\\\\sa((?: \\w+)+)',\n>> +                         lambda match: 'See also: ' + ', \n>> '.join(\n>> +                             map(kebab_case, \n>> match.group(1).strip().split(' '))\n>> +                         ) + '.', description)\n>> +    description = re.sub(r'\\\\todo', 'Todo:', description)\n>> +\n>> +    description = description.strip().split('\\n')\n>> +    return '\\n'.join([\n>> +        '\"' + line.replace('\\\\', r'\\\\').replace('\"', r'\\\"') + \n>> ' \"' for line in description if line\n>> +    ]).rstrip()\n>> +\n>> +\n>> +def snake_case(s):\n>> +    return ''.join([\n>> +        c.isupper() and ('_' + c.lower()) or c for c in s\n>> +    ]).strip('_')\n>> +\n>> +\n>> +def kebab_case(s):\n>> +    return snake_case(s).replace('_', '-')\n>> +\n>> +\n>> +def extend_control(ctrl):\n>> +    if ctrl.vendor != 'libcamera':\n>> +        ctrl.namespace = f'{ctrl.vendor}::'\n>> +        ctrl.vendor_prefix = f'{ctrl.vendor}-'\n>> +    else:\n>> +        ctrl.namespace = ''\n>> +        ctrl.vendor_prefix = ''\n>> +\n>> +    ctrl.is_array = ctrl.size is not None\n>> +\n>> +    if ctrl.is_enum:\n>> +        # remove common prefix from enum variant names\n>\n> s/# remove/# Remove/\n>\n>> +        prefix = find_common_prefix([enum.name for enum in \n>> ctrl.enum_values])\n>> +        for enum in ctrl.enum_values:\n>> +            enum.gst_name = \n>> kebab_case(enum.name.removeprefix(prefix))\n>> +\n>> +        ctrl.gtype = 'enum'\n>> +        ctrl.default = '0'\n>> +    elif ctrl.element_type == 'bool':\n>> +        ctrl.gtype = 'boolean'\n>> +        ctrl.default = 'false'\n>> +    elif ctrl.element_type == 'float':\n>> +        ctrl.gtype = 'float'\n>> +        ctrl.default = '0'\n>> +        ctrl.min = '-G_MAXFLOAT'\n>> +        ctrl.max = 'G_MAXFLOAT'\n>> +    elif ctrl.element_type == 'int32_t':\n>> +        ctrl.gtype = 'int'\n>> +        ctrl.default = '0'\n>> +        ctrl.min = 'G_MININT'\n>> +        ctrl.max = 'G_MAXINT'\n>> +    elif ctrl.element_type == 'int64_t':\n>> +        ctrl.gtype = 'int64'\n>> +        ctrl.default = '0'\n>> +        ctrl.min = 'G_MININT64'\n>> +        ctrl.max = 'G_MAXINT64'\n>> +    elif ctrl.element_type == 'uint8_t':\n>> +        ctrl.gtype = 'uchar'\n>> +        ctrl.default = '0'\n>> +        ctrl.min = '0'\n>> +        ctrl.max = 'G_MAXUINT8'\n>> +    elif ctrl.element_type == 'Rectangle':\n>> +        ctrl.is_rectangle = True\n>> +        ctrl.default = '0'\n>> +        ctrl.min = '0'\n>> +        ctrl.max = 'G_MAXINT'\n>> +    else:\n>> +        raise RuntimeError(f'The type `{ctrl.element_type}` is \n>> unknown')\n>> +\n>> +    return ctrl\n>> +\n>> +\n>> +def main(argv):\n>> +    # Parse command line arguments\n>> +    parser = argparse.ArgumentParser()\n>> +    parser.add_argument('--output', '-o', metavar='file', \n>> type=str,\n>> +                        help='Output file name. Defaults to \n>> standard output if not specified.')\n>> +    parser.add_argument('--template', '-t', dest='template', \n>> type=str, required=True,\n>> +                        help='Template file name.')\n>> +    parser.add_argument('input', type=str, nargs='+',\n>> +                        help='Input file name.')\n>> +\n>> +    args = parser.parse_args(argv[1:])\n>> +\n>> +    controls = {}\n>> +    for input in args.input:\n>> +        data = yaml.safe_load(open(input, 'rb').read())\n>> +\n>> +        vendor = data['vendor']\n>> +        ctrls = controls.setdefault(vendor, [])\n>> +\n>> +        for ctrl in data['controls']:\n>> +            ctrl = Control(*ctrl.popitem(), vendor)\n>> +            ctrls.append(extend_control(ctrl))\n>> +\n>> +    data = {'controls': list(controls.items())}\n>> +\n>> +    env = jinja2.Environment()\n>> +    env.filters['format_description'] = format_description\n>> +    env.filters['snake_case'] = snake_case\n>> +    env.filters['kebab_case'] = kebab_case\n>> +    template = env.from_string(open(args.template, 'r', \n>> encoding='utf-8').read())\n>> +    string = template.render(data)\n>> +\n>> +    if args.output:\n>> +        with open(args.output, 'w', encoding='utf-8') as \n>> output:\n>> +            output.write(string)\n>> +    else:\n>> +        sys.stdout.write(string)\n>> +\n>> +    return 0\n>> +\n>> +\n>> +if __name__ == '__main__':\n>> +    sys.exit(main(sys.argv))\n>> diff --git a/utils/codegen/meson.build \n>> b/utils/codegen/meson.build\n>> index adf33bba..904dd66d 100644\n>> --- a/utils/codegen/meson.build\n>> +++ b/utils/codegen/meson.build\n>> @@ -11,6 +11,7 @@ py_modules += ['jinja2', 'yaml']\n>>  \n>>  gen_controls = files('gen-controls.py')\n>>  gen_formats = files('gen-formats.py')\n>> +gen_gst_controls = files('gen-gst-controls.py')\n>>  gen_header = files('gen-header.sh')\n>>  gen_ipa_pub_key = files('gen-ipa-pub-key.py')\n>>  gen_tracepoints = files('gen-tp-header.py')\n\nBest regards\n\nJaslo","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3407FC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 27 Aug 2024 11:49:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B506F63460;\n\tTue, 27 Aug 2024 13:49:53 +0200 (CEST)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[85.215.255.23])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D46C61901\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Aug 2024 13:49:51 +0200 (CEST)","from archlinux by smtp.strato.de (RZmta 51.2.3 AUTH)\n\twith ESMTPSA id zec3f707RBnnhFe\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tTue, 27 Aug 2024 13:49:49 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ziska.de header.i=@ziska.de header.b=\"WV2xtflc\";\n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"2zZ/QClI\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1724759390; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=KMkkdR+mk/mdC5ynkpjJYPaNtkCDcyEznReho+BglyDJqHojXXNBvuORxz0sglrDs4\n\twOgXiOYD3gBQeuohf+jYhVeZKQQqA6wMEodAaUohyQhRAU6XDPKET+rd0YEeYtL97Z1q\n\tW5lYc935SgJWvPETiHGSvxiHq1faCxrLl3wehh4qnCjBAS7XJGETRwmoNeht8ArRlVWB\n\te4CUJPaOlhCn6oIGbfbAGS12oWhL5dODJVkzPuhIvVw6FFuqr0uZMR1W6cZ85cJM7izJ\n\tiwyOBbDXpShdGyRO3SaGgO189oqEdoICWqFFUPOi7YPzedOJOW8vzMSGiBKEc/gVq6bj\n\tT3JQ==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1724759390;\n\ts=strato-dkim-0002; d=strato.com;\n\th=Message-ID:Date:References:In-Reply-To:Subject:Cc:To:From:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=YGZIdccOQpQQxpBpFS1ESeTtZQTs1fepOp4QmMCwPCk=;\n\tb=taL58b5kPkPlJcEwycYTjTVbG//qziRxrzYioe6BeS8oNzG2FdET6i2xQPqqMnExZV\n\t81W7+y7oyXoWr3So57TLw/3CcD19dfGDxFBatoQTDywN/iX98ndMnG/jV5AhA5Ro4Rg1\n\tihFCUeKfzTRrLBDgABs2Tk2eqx0A4IfksaFGSW9KwJ7AG7AMltvjqTrpZDPi7fzgeTQ1\n\tQT9wyA7tt2ije1vVu+ckxN8gtdaxW1UEd7nMHjvxST+IKrMLXU3uGoidERsKM8/TB45t\n\tPapjAUWGeHZDsPE7y1BCyzx30SI6tDZWR7noEdccjyOzYM9NHXJMSYy2G+ubpPD2HHjZ\n\t++Bg==","ARC-Authentication-Results":"i=1; strato.com;\n    arc=none;\n    dkim=none","X-RZG-CLASS-ID":"mo00","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; t=1724759390;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=Message-ID:Date:References:In-Reply-To:Subject:Cc:To:From:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=YGZIdccOQpQQxpBpFS1ESeTtZQTs1fepOp4QmMCwPCk=;\n\tb=WV2xtflcImNe0gFa29nng+HiRF8yCXx+Q2HNKzrovIPIMtWtnU8BYDCTT8I9sgWf0U\n\tKYCM1/y3PF/u3ozeZr7rMpkszRSr94UAPWreR9Oovaa+RBGl/Gs4TZpoGr3CBt/BAuHG\n\tjLpoXgfoapNjUJ+CuPTsNlmpvUXFM/aqee6YwTsJKG+umZ7ergoD8OfLwKx0CWD9B1QV\n\tmCQaus+Ym0P030o7OueaA3CChoUF0Po/GU6Qv7JJQ9gb1T5HdTD5jrrEsMXvAPbr/3i7\n\tFYUlwtt2AaznN39p/cPsvffXqw+ApPgHzCtWMlFNTj6cTIfaZVwpwitWTffWeiiYpIeA\n\tkjEA==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1724759390;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=Message-ID:Date:References:In-Reply-To:Subject:Cc:To:From:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=YGZIdccOQpQQxpBpFS1ESeTtZQTs1fepOp4QmMCwPCk=;\n\tb=2zZ/QClI/Bm4D/ZsE2lh0m7t4N+u/ju3tQ74OM6/ASLg/ijhUWinFaIhTxc23BS4NC\n\tITtLC3JJuyO3BismoOCA=="],"X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusDqIM6Hizy8VdfzvKi4yoFC9cHgIq7BfJawKbGh7AKuSVfORBZmPIRJ15iJSlA\"","From":"Jaslo Ziska <jaslo@ziska.de>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","In-Reply-To":"<20240816000744.GE23911@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Fri, 16 Aug 2024 03:07:44 +0300\")","References":"<20240813124722.22425-1-jaslo@ziska.de>\n\t<20240813124722.22425-4-jaslo@ziska.de>\n\t<20240816000744.GE23911@pendragon.ideasonboard.com>","Date":"Tue, 27 Aug 2024 13:49:49 +0200","Message-ID":"<87y14idw2q.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30921,"web_url":"https://patchwork.libcamera.org/comment/30921/","msgid":"<20240827124308.GA19409@pendragon.ideasonboard.com>","date":"2024-08-27T12:43:08","subject":"Re: [PATCH v2 3/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Aug 27, 2024 at 01:49:49PM +0200, Jaslo Ziska wrote:\n> Laurent Pinchart writes:\n> > On Tue, Aug 13, 2024 at 02:25:07PM +0200, Jaslo Ziska wrote:\n> >> This commit implements gstreamer controls for the libcamera element by\n> >> generating the controls from the control_ids_*.yaml files using a new\n> >> gen-gst-controls.py script. The appropriate meson files are also changed\n> >> to automatically run the script when building.\n> >> \n> >> The gen-gst-controls.py script works similar to the gen-controls.py\n> >> script by parsing the control_ids_*.yaml files and generating C++ code\n> >> for each control.\n> >> For the controls to be used as gstreamer properties the type for each\n> >> control needs to be translated to the appropriate glib type and a\n> >> GEnumValue is generated for each enum control. Then a\n> >> g_object_install_property(), _get_property() and _set_property()\n> >> function is generated for each control.\n> >> The vendor controls get prefixed with \"$vendor-\" in the final gstreamer\n> >> property name.\n> >> \n> >> The C++ code generated by the gen-gst-controls.py script is written into\n> >> the template gstlibcamerasrc-controls.cpp.in file. The matching\n> >> gstlibcamerasrc-controls.h header defines the GstCameraControls class\n> >> which handles the installation of the gstreamer properties as well as\n> >> keeping track of the control values and setting and getting the\n> >> controls. The content of these functions is generated in the Python\n> >> script.\n> >> \n> >> Finally the libcamerasrc element itself is edited to make use of the new\n> >> GstCameraControls class. The way this works is by defining a PROP_LAST\n> >> enum variant which is passed to the installProperties() function so the\n> >> properties are defined with the appropriate offset. When getting or\n> >> setting a property PROP_LAST is subtracted from the requested property\n> >> to translate the control back into a libcamera::controls:: enum variant.\n> >\n> > Looks good to me. I'll focus the review on the parts not related to the\n> > GStreamer internal APIs, and will let Nicolas bring his GStreamer\n> > expertise.\n> >\n> >> Signed-off-by: Jaslo Ziska <jaslo@ziska.de>\n> >> ---\n> >>  src/gstreamer/gstlibcamera-controls.cpp.in | 296 +++++++++++++++++++++\n> >>  src/gstreamer/gstlibcamera-controls.h      |  43 +++\n> >>  src/gstreamer/gstlibcamerasrc.cpp          |  22 +-\n> >>  src/gstreamer/meson.build                  |  10 +\n> >>  utils/codegen/controls.py                  |   8 +\n> >>  utils/codegen/gen-gst-controls.py          | 151 +++++++++++\n> >>  utils/codegen/meson.build                  |   1 +\n> >>  7 files changed, 528 insertions(+), 3 deletions(-)\n> >>  create mode 100644 src/gstreamer/gstlibcamera-controls.cpp.in\n> >>  create mode 100644 src/gstreamer/gstlibcamera-controls.h\n> >>  create mode 100755 utils/codegen/gen-gst-controls.py\n> >> \n> >> diff --git a/src/gstreamer/gstlibcamera-controls.cpp.in \n> >> b/src/gstreamer/gstlibcamera-controls.cpp.in\n> >> new file mode 100644\n> >> index 00000000..aab7ae24\n> >> --- /dev/null\n> >> +++ b/src/gstreamer/gstlibcamera-controls.cpp.in\n> >> @@ -0,0 +1,296 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2023, Collabora Ltd.\n> >> + *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> >\n> > I think you can attribute the copyright on this file to \n> > yourself.\n> >\n> >> + *\n> >> + * GStreamer Camera Controls\n> >> + *\n> >> + * This file is auto-generated. Do not edit.\n> >> + */\n> >> +\n> >> +#include <vector>\n> >> +\n> >> +#include <libcamera/control_ids.h>\n> >\n> > And\n> >\n> > #include <libcamera/controls.h>\n> > #include <libcamera/geometry.h>\n> >\n> > as you use classes defined in those headers below.\n> >\n> >> +\n> >> +#include \"gstlibcamera-controls.h\"\n> >> +\n> >> +using namespace libcamera;\n> >> +\n> >> +{% for vendor, ctrls in controls %}\n> >> +{%- for ctrl in ctrls if ctrl.is_enum %}\n> >> +static const GEnumValue {{ ctrl.name|snake_case }}_types[] = {\n> >> +{%- for enum in ctrl.enum_values %}\n> >> +\t{\n> >> +\t\tcontrols::{{ ctrl.namespace }}{{ enum.name }},\n> >> +\t\t{{ enum.description|format_description|indent('\\t\\t') \n> >> }},\n> >> +\t\t\"{{ enum.gst_name }}\"\n> >> +\t},\n> >> +{%- endfor %}\n> >> +\t{0, NULL, NULL}\n> >> +};\n> >> +\n> >> +#define TYPE_{{ ctrl.name|snake_case|upper }} \\\n> >> +\t({{ ctrl.name|snake_case }}_get_type())\n> >> +static GType {{ ctrl.name|snake_case }}_get_type()\n> >> +{\n> >> +\tstatic GType {{ ctrl.name|snake_case }}_type = 0;\n> >> +\n> >> +\tif (!{{ ctrl.name|snake_case }}_type)\n> >> +\t\t{{ ctrl.name|snake_case }}_type =\n> >> +\t\t\tg_enum_register_static(\"{{ ctrl.name }}\",\n> >> +\t\t\t\t\t       {{ ctrl.name|snake_case }}_types);\n> >> +\n> >> +\treturn {{ ctrl.name|snake_case }}_type;\n> >> +}\n> >> +{% endfor %}\n> >> +{%- endfor %}\n> >> +\n> >> +void GstCameraControls::installProperties(GObjectClass *klass, \n> >> int lastPropId)\n> >> +{\n> >> +{%- for vendor, ctrls in controls %}\n> >> +{%- for ctrl in ctrls %}\n> >> +\n> >> +{%- set spec %}\n> >\n> > I didn't know about set in jinja templates, interesting.\n> >\n> >> +{%- if ctrl.is_rectangle -%}\n> >> +gst_param_spec_array(\n> >> +{%- else -%}\n> >> +g_param_spec_{{ ctrl.gtype }}(\n> >> +{%- endif -%}\n> >> +{%- if ctrl.is_array %}\n> >> +\t\"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case \n> >> }}-value\",\n> >> +\t\"{{ ctrl.name }} Value\",\n> >> +\t\"One {{ ctrl.name }} element value\",\n> >> +{%- else %}\n> >> +\t\"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case }}\",\n> >> +\t\"{{ ctrl.name }}\",\n> >> +\t{{ ctrl.description|format_description|indent('\\t') }},\n> >> +{%- endif %}\n> >> +{%- if ctrl.is_enum %}\n> >> +\tTYPE_{{ ctrl.name|snake_case|upper }},\n> >> +\t{{ ctrl.default }},\n> >> +{%- elif ctrl.is_rectangle %}\n> >> +\tg_param_spec_int(\n> >> +\t\t\"rectangle-value\",\n> >> +\t\t\"Rectangle Value\",\n> >> +\t\t\"One rectangle value, either x, y, width or height.\",\n> >> +\t\t{{ ctrl.min }}, {{ ctrl.max }}, {{ ctrl.default }},\n> >> +\t\t(GParamFlags) (GST_PARAM_CONTROLLABLE | \n> >> G_PARAM_READWRITE |\n> >> +\t\t\t       G_PARAM_STATIC_STRINGS)\n> >> +\t),\n> >> +{%- elif ctrl.gtype == 'boolean' %}\n> >> +\t{{ ctrl.default }},\n> >> +{%- elif ctrl.gtype in ['float', 'int', 'int64', 'uchar'] %}\n> >> +\t{{ ctrl.min }}, {{ ctrl.max }}, {{ ctrl.default }},\n> >> +{%- endif %}\n> >> +\t(GParamFlags) (GST_PARAM_CONTROLLABLE | G_PARAM_READWRITE \n> >> |\n> >> +\t\t       G_PARAM_STATIC_STRINGS)\n> >> +)\n> >> +{%- endset %}\n> >> +\n> >> +\tg_object_class_install_property(\n> >> +\t\tklass,\n> >> +\t\tlastPropId + controls::{{ ctrl.namespace }}{{ \n> >> ctrl.name|snake_case|upper }},\n> >> +{%- if ctrl.is_array %}\n> >> +\t\tgst_param_spec_array(\n> >> +\t\t\t\"{{ ctrl.vendor_prefix }}{{ ctrl.name|kebab_case \n> >> }}\",\n> >> +\t\t\t\"{{ ctrl.name }}\",\n> >> +\t\t\t{{ \n> >> ctrl.description|format_description|indent('\\t\\t\\t') }},\n> >> +\t\t\t{{ spec|indent('\\t\\t\\t') }},\n> >> +\t\t\t(GParamFlags) (GST_PARAM_CONTROLLABLE |\n> >> +\t\t\t\t       G_PARAM_READWRITE |\n> >> +\t\t\t\t       G_PARAM_STATIC_STRINGS)\n> >> +\t\t)\n> >> +{%- else %}\n> >> +\t\t{{ spec|indent('\\t\\t') }}\n> >> +{%- endif %}\n> >> +\t);\n> >> +{%- endfor %}\n> >> +{%- endfor %}\n> >> +}\n> >> +\n> >> +bool GstCameraControls::getProperty(guint propId, GValue \n> >> *value,\n> >> +\t\t\t\t    [[maybe_unused]] GParamSpec *pspec)\n> >> +{\n> >> +\tswitch (propId) {\n> >> +{%- for vendor, ctrls in controls %}\n> >> +{%- for ctrl in ctrls %}\n> >> +\n> >> +{%- set value_set %}\n> >> +{%- if ctrl.is_rectangle -%}\n> >> +Point top_left = val.topLeft();\n> >> +Size size = val.size();\n> >> +\n> >> +GValue x = G_VALUE_INIT;\n> >> +g_value_init(&x, G_TYPE_INT);\n> >> +g_value_set_int(&x, top_left.x);\n> >> +gst_value_array_append_and_take_value(&element, &x);\n> >> +\n> >> +GValue y = G_VALUE_INIT;\n> >> +g_value_init(&y, G_TYPE_INT);\n> >> +g_value_set_int(&y, top_left.y);\n> >> +gst_value_array_append_and_take_value(&element, &y);\n> >> +\n> >> +GValue width = G_VALUE_INIT;\n> >> +g_value_init(&width, G_TYPE_INT);\n> >> +g_value_set_int(&width, size.width);\n> >> +gst_value_array_append_and_take_value(&element, &width);\n> >> +\n> >> +GValue height = G_VALUE_INIT;\n> >> +g_value_init(&height, G_TYPE_INT);\n> >> +g_value_set_int(&x, size.height);\n> >> +gst_value_array_append_and_take_value(&element, &height);\n> >> +{%- else -%}\n> >> +g_value_set_{{ ctrl.gtype }}(&element, val);\n> >> +{%- endif -%}\n> >> +{%- endset %}\n> >> +\n> >> +\tcase controls::{{ ctrl.namespace }}{{ \n> >> ctrl.name|snake_case|upper }}: {\n> >> +\t\tauto control = metadata_.get(controls::{{ \n> >> ctrl.namespace }}{{ ctrl.name }});\n> >> +\t\tcontrol = control ? control :\n> >> +\t\t\t  controls_.get(controls::{{ ctrl.namespace }}{{ \n> >> ctrl.name }});\n> >\n> > \t\tif (!control)\n> > \t\t\tcontrol = controls_.get(controls::{{ ctrl.namespace \n> > }}{{ ctrl.name }});\n> >\n> >> +\t\tif (!control) {\n> >> +\t\t\tGST_WARNING(\"Control '%s' is not available, \n> >> default value will be returned\",\n> >> +\t\t\t\t    controls::{{ ctrl.namespace }}{{ ctrl.name \n> >> }}.name().c_str());\n> >> +\t\t\treturn true;\n> >> +\t\t}\n> >> +\n> >> +{%- if ctrl.is_array %}\n> >> +\t\tfor (size_t i = 0; i < control->size(); ++i) {\n> >> +\t\t\tGValue element = G_VALUE_INIT;\n> >> +{%- if ctrl.is_rectangle %}\n> >> +\t\t\tg_value_init(&element, GST_TYPE_PARAM_ARRAY_LIST);\n> >> +{%- else %}\n> >> +\t\t\tg_value_init(&element, G_TYPE_{{ ctrl.gtype|upper \n> >> }});\n> >> +{%- endif %}\n> >> +\t\t\tauto val = (*control)[i];\n> >> +\t\t\t{{ value_set|indent('\\t\\t\\t\\t') }}\n> >> +\t\t\tgst_value_array_append_and_take_value(value, \n> >> &element);\n> >> +\t\t}\n> >> +{%- else %}\n> >> +\t\tGValue element = *value;\n> >> +\t\tauto val = *control;\n> >> +\t\t{{ value_set|indent('\\t\\t\\t') }}\n> >> +{%- endif %}\n> >> +\n> >> +\t\treturn true;\n> >> +\t}\n> >> +{%- endfor %}\n> >> +{%- endfor %}\n> >\n> > Add a blank line here as there's one between all the cases \n> > above.\n> >\n> >> +\tdefault:\n> >> +\t\treturn false;\n> >> +\t}\n> >> +}\n> >> +\n> >> +bool GstCameraControls::setProperty(guint propId, const GValue \n> >> *value,\n> >> +\t\t\t\t    [[maybe_unused]] GParamSpec *pspec)\n> >> +{\n> >> +\t// check whether the camera capabilities are available\n> >\n> > Please use C-style comments, start sentences with a capital \n> > letter, and\n> > end them with a period.\n> >\n> > \t/* Check whether the camera capabilities are available. */\n> >\n> >> +\tif (!capabilities_.empty()) {\n> >> +\t\t// if so, check that the control is supported\n> >\n> > \t\t/* If so, check that the control is supported. */\n> >\n> > Same below\n> >\n> >> +\t\tconst ControlId *cid = \n> >> capabilities_.idmap().at(propId);\n> >> +\t\tauto info = capabilities_.find(cid);\n> >> +\n> >> +\t\tif (info == capabilities_.end()) {\n> >> +\t\t\tGST_WARNING(\"Control '%s' is not supported by the \n> >> camera and will be ignored\",\n> >> +\t\t\t\t    cid->name().c_str());\n> >> +\t\t\treturn true;\n> >> +\t\t}\n> >> +\t}\n> >> +\n> >> +\tswitch (propId) {\n> >> +{%- for vendor, ctrls in controls %}\n> >> +{%- for ctrl in ctrls %}\n> >> +\n> >> +{%- set value_get %}\n> >> +{%- if ctrl.is_rectangle -%}\n> >> +if (gst_value_array_get_size(element) != 4) {\n> >> +\tGST_ERROR(\"Rectangle must be an array of size 4\");\n> >\n> > Maybe print the name of the control to make debugging easier ?\n> \n> Good idea, I will add that. It might be a little problematic when \n> extracting this part to a separate helper function as the Jinja \n> context will then not be available anymore for the name but I will \n> try think of something (maybe passing the control name to the \n> function?).\n\nYes, passing the control name to the function seems fine.\n \n> >> +\treturn true;\n> >> +}\n> >> +\n> >> +const GValue *r;\n> >> +r = gst_value_array_get_value(element, 0);\n> >> +int x = g_value_get_int(r);\n> >> +r = gst_value_array_get_value(element, 1);\n> >> +int y = g_value_get_int(r);\n> >> +r = gst_value_array_get_value(element, 2);\n> >> +int w = g_value_get_int(r);\n> >> +r = gst_value_array_get_value(element, 3);\n> >> +int h = g_value_get_int(r);\n> >> +\n> >> +auto val = Rectangle(x, y, w, h);\n> >\n> > Would it make sense to move all this (including the size check) to a\n> > helper function instead of duplicating the code for each rectangle\n> > control ?\n> \n> Good idea, that way I might also be able to drop the Jinja set \n> directive and use it directly.\n> \n> >> +{%- else -%}\n> >> +auto val = g_value_get_{{ ctrl.gtype }}(element);\n> >> +{%- endif -%}\n> >> +{%- endset %}\n> >> +\n> >> +\tcase controls::{{ ctrl.namespace }}{{ \n> >> ctrl.name|snake_case|upper }}: {\n> >> +{%- if ctrl.is_array %}\n> >> +\t\tsize_t size = gst_value_array_get_size(value);\n> >> +{%- if ctrl.size != 0 %}\n> >> +\t\tif (size != {{ ctrl.size }}) {\n> >> +\t\t\tGST_ERROR(\"Incorrect array size, must be of size \n> >> {{ ctrl.size }}\");\n> >> +\t\t\treturn true;\n> >> +\t\t}\n> >> +{%- endif %}\n> >> +\n> >> +\t\tstd::vector<{{ ctrl.element_type }}> values(size);\n> >> +\t\tfor (size_t i = 0; i < size; ++i) {\n> >> +\t\t\tconst GValue *element = \n> >> gst_value_array_get_value(value, i);\n> >> +\t\t\t{{ value_get|indent('\\t\\t\\t') }}\n> >> +\t\t\tvalues[i] = val;\n> >> +\t\t}\n> >> +\t\tcontrols_.set(controls::{{ ctrl.namespace }}{{ \n> >> ctrl.name }},\n> >> +{%- if ctrl.size == 0 %}\n> >> +\t\t\t      Span<const {{ ctrl.element_type \n> >> }}>(values.data(), size));\n> >> +{%- else %}\n> >> +\t\t\t      Span<const {{ ctrl.element_type }}, {{ \n> >> ctrl.size }}>(values.data(), {{ ctrl.size }}));\n> >> +{%- endif %}\n> >> +{%- else %}\n> >> +\t\tconst GValue *element = value;\n> >> +\t\t{{ value_get|indent('\\t\\t') }}\n> >> +\t\tcontrols_.set(controls::{{ ctrl.namespace }}{{ \n> >> ctrl.name }}, val);\n> >> +{%- endif %}\n> >> +\t\treturn true;\n> >> +\t}\n> >> +{%- endfor %}\n> >> +{%- endfor %}\n> >\n> > Add a blank line here too.\n> >\n> >> +\tdefault:\n> >> +\t\treturn false;\n> >> +\t}\n> >> +}\n> >> +\n> >> +void GstCameraControls::setCamera(const \n> >> std::shared_ptr<libcamera::Camera> &cam)\n> >> +{\n> >> +\tcapabilities_ = cam->controls();\n> >\n> > Is there a need to make a copy of the ControlInfoMap ? Can't we \n> > retain a\n> > reference to the camera and use it to access the list of \n> > controls ?\n> \n> You are right, I could just add a shared_ptr<Camera> member.\n> \n> >> +\n> >> +\t// check the controls which were set before the camera \n> >> capabilities were known\n> >> +\tControlList new_controls;\n> >> +\tfor (auto control = controls_.begin(); control != \n> >> controls_.end(); ++control) {\n> >\n> > setCamera() is called in gst_libcamera_src_open(). Does that occur\n> > before or after gst_libcamera_src_set_property() ? If it occurs before,\n> > controls_ will be empty here.\n> \n> This is a little complicated, gst_libcamera_src_set_property() can \n> be called before and after gst_libcamera_src_open(). So to be able \n> to check the controls which were set before the pipeline is \n> started I check the controls at this point because the camera \n> becomes available here.\n\nMaybe a comment would be useful to explain this.\n\n> >> +\t\tunsigned int id = control->first;\n> >> +\t\tControlValue value = control->second;\n> >> +\n> >> +\t\tconst ControlId *cid = capabilities_.idmap().at(id);\n> >> +\t\tauto info = capabilities_.find(cid);\n> >> +\n> >> +\t\t// only add controls which are supported\n> >> +\t\tif (info != capabilities_.end()) {\n> >> +\t\t\tnew_controls.set(id, value);\n> >> +\t\t} else {\n> >> +\t\t\tGST_WARNING(\"Control '%s' is not supported by the camera and will be ignored\",\n> >> +\t\t\t\t    cid->name().c_str());\n> >\n> > The control should have been rejected by setProperty() in this case, and\n> > won't end up in controls_ in that case, so I don't think this can occur.\n> \n> Because controls might be set before the camera is available the \n> first line in setProperty() checks whether the camera capabilities \n> are available and only checks and rejects the controls if they \n> are. If the camera was not set when setProperty() is called the \n> control gets set unconditionally.\n> \n> That is why the controls which were set before the camera became \n> available are checked at this point when the camera is set.\n\nI see. I suppose there's no way to simplify this and requires the camera\nto be set before controls are set ?\n\n> >> +\t\t}\n> >\n> > You can drop the curly braces.\n> >\n> >> +\t}\n> >> +\n> >> +\tcontrols_ = new_controls;\n> >> +}\n> >> +\n> >> +void \n> >> GstCameraControls::applyControls(std::unique_ptr<libcamera::Request> \n> >> &request)\n> >> +{\n> >> +\trequest->controls().merge(controls_);\n> >\n> > I think you want to clear controls_ here, they don't need to be set in\n> > every request. That's problematic for getControl() though, as it relies\n> > on controls_ accumulating the state of all controls ever set. You'll\n> > probably need two variables, one that accumulates controls, and one that\n> > doesn't.\n> \n> You are right. I was mistaken about how the libcamera controls \n> work, for some reason I thought you had to repeatedly send the \n> control with every request. I will change this.\n> \n> >> +}\n> >> +\n> >> +void GstCameraControls::readMetadata(libcamera::Request \n> >> *request)\n> >> +{\n> >> +\tmetadata_ = request->metadata();\n> >> +}\n> >\n> > Great handling of white space, the generated code is very \n> > readable.\n> >\n> >> diff --git a/src/gstreamer/gstlibcamera-controls.h \n> >> b/src/gstreamer/gstlibcamera-controls.h\n> >> new file mode 100644\n> >> index 00000000..89b616ad\n> >> --- /dev/null\n> >> +++ b/src/gstreamer/gstlibcamera-controls.h\n> >> @@ -0,0 +1,43 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2023, Collabora Ltd.\n> >> + *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> >> + *\n> >> + * GStreamer Camera Controls\n> >> + */\n> >> +\n> >> +#pragma once\n> >> +\n> >> +#include <memory>\n> >> +\n> >> +#include <libcamera/camera.h>\n> >> +#include <libcamera/controls.h>\n> >> +#include <libcamera/request.h>\n> >> +\n> >> +#include \"gstlibcamerasrc.h\"\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +class GstCameraControls\n> >> +{\n> >> +public:\n> >> +\tstatic void installProperties(GObjectClass *klass, int lastProp);\n> >> +\n> >> +\tbool getProperty(guint propId, GValue *value, GParamSpec *pspec);\n> >> +\tbool setProperty(guint propId, const GValue *value, GParamSpec *pspec);\n> >> +\n> >> +\tvoid setCamera(const std::shared_ptr<libcamera::Camera> &cam);\n> >> +\n> >> +\tvoid applyControls(std::unique_ptr<libcamera::Request> &request);\n> >> +\tvoid readMetadata(libcamera::Request *request);\n> >> +\n> >> +private:\n> >> +\t/* supported controls and limits of camera */\n> >> +\tControlInfoMap capabilities_;\n> >> +\t/* set of user modified controls */\n> >> +\tControlList controls_;\n> >> +\t/* metadata returned by the camera */\n> >> +\tControlList metadata_;\n> >> +};\n> >> +\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n> >> b/src/gstreamer/gstlibcamerasrc.cpp\n> >> index 40b787c8..8efa25f4 100644\n> >> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> >> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> >> @@ -37,10 +37,11 @@\n> >>  \n> >>  #include <gst/base/base.h>\n> >>  \n> >> +#include \"gstlibcamera-controls.h\"\n> >> +#include \"gstlibcamera-utils.h\"\n> >>  #include \"gstlibcameraallocator.h\"\n> >>  #include \"gstlibcamerapad.h\"\n> >>  #include \"gstlibcamerapool.h\"\n> >> -#include \"gstlibcamera-utils.h\"\n> >>  \n> >>  using namespace libcamera;\n> >>  \n> >> @@ -128,6 +129,7 @@ struct GstLibcameraSrcState {\n> >>  \n> >>  \tControlList initControls_;\n> >>  \tguint group_id_;\n> >> +\tGstCameraControls controls_;\n> >>  \n> >>  \tint queueRequest();\n> >>  \tvoid requestCompleted(Request *request);\n> >> @@ -153,6 +155,7 @@ struct _GstLibcameraSrc {\n> >>  enum {\n> >>  \tPROP_0,\n> >>  \tPROP_CAMERA_NAME,\n> >> +\tPROP_LAST\n> >>  };\n> >>  \n> >>  static void gst_libcamera_src_child_proxy_init(gpointer g_iface,\n> >> @@ -183,6 +186,9 @@ int GstLibcameraSrcState::queueRequest()\n> >>  \tif (!request)\n> >>  \t\treturn -ENOMEM;\n> >>  \n> >> +\t/* Apply controls */\n> >> +\tcontrols_.applyControls(request);\n> >> +\n> >>  \tstd::unique_ptr<RequestWrap> wrap =\n> >>  \t\tstd::make_unique<RequestWrap>(std::move(request));\n> >>  \n> >> @@ -226,6 +232,9 @@ \n> >> GstLibcameraSrcState::requestCompleted(Request *request)\n> >>  \n> >>  \t{\n> >>  \t\tGLibLocker locker(&lock_);\n> >> +\n> >> +\t\tcontrols_.readMetadata(request);\n> >> +\n> >>  \t\twrap = std::move(queuedRequests_.front());\n> >>  \t\tqueuedRequests_.pop();\n> >>  \t}\n> >> @@ -408,6 +417,8 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> >>  \t\treturn false;\n> >>  \t}\n> >>  \n> >> +\tself->state->controls_.setCamera(cam);\n> >> +\n> >>  \tcam->requestCompleted.connect(self->state, \n> >>  &GstLibcameraSrcState::requestCompleted);\n> >>  \n> >>  \t/* No need to lock here, we didn't start our threads yet. \n> >>  */\n> >> @@ -722,6 +733,7 @@ gst_libcamera_src_set_property(GObject \n> >> *object, guint prop_id,\n> >>  {\n> >>  \tGLibLocker lock(GST_OBJECT(object));\n> >>  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);\n> >> +\tGstLibcameraSrcState *state = self->state;\n> >>  \n> >>  \tswitch (prop_id) {\n> >>  \tcase PROP_CAMERA_NAME:\n> >> @@ -729,7 +741,8 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,\n> >>  \t\tself->camera_name = g_value_dup_string(value);\n> >>  \t\tbreak;\n> >>  \tdefault:\n> >> -\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> >> +\t\tif (!state->controls_.setProperty(prop_id - PROP_LAST, value, pspec))\n> >> +\t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> >>  \t\tbreak;\n> >>  \t}\n> >>  }\n> >> @@ -740,13 +753,15 @@ gst_libcamera_src_get_property(GObject \n> >> *object, guint prop_id, GValue *value,\n> >>  {\n> >>  \tGLibLocker lock(GST_OBJECT(object));\n> >>  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(object);\n> >> +\tGstLibcameraSrcState *state = self->state;\n> >>  \n> >>  \tswitch (prop_id) {\n> >>  \tcase PROP_CAMERA_NAME:\n> >>  \t\tg_value_set_string(value, self->camera_name);\n> >>  \t\tbreak;\n> >>  \tdefault:\n> >> -\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> >> +\t\tif (!state->controls_.getProperty(prop_id - PROP_LAST, value, pspec))\n> >> +\t\t\tG_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);\n> >>  \t\tbreak;\n> >>  \t}\n> >>  }\n> >> @@ -947,6 +962,7 @@ \n> >> gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)\n> >>  \t\t\t\t\t\t\t     | G_PARAM_STATIC_STRINGS));\n> >>  \tg_object_class_install_property(object_class, \n> >>  PROP_CAMERA_NAME, spec);\n> >>  \n> >> +\tGstCameraControls::installProperties(object_class, PROP_LAST);\n> >>  }\n> >>  \n> >>  /* GstChildProxy implementation */\n> >> diff --git a/src/gstreamer/meson.build \n> >> b/src/gstreamer/meson.build\n> >> index c2a01e7b..6b7e53b5 100644\n> >> --- a/src/gstreamer/meson.build\n> >> +++ b/src/gstreamer/meson.build\n> >> @@ -25,6 +25,16 @@ libcamera_gst_sources = [\n> >>      'gstlibcamerasrc.cpp',\n> >>  ]\n> >>  \n> >> +# Generate gstreamer control properties\n> >> +\n> >> +gen_gst_controls_template = files('gstlibcamera-controls.cpp.in')\n> >> +libcamera_gst_sources += custom_target('gstlibcamera-controls.cpp',\n> >> +                                       input : controls_files,\n> >> +                                       output : 'gstlibcamera-controls.cpp',\n> >> +                                       command : [gen_gst_controls, '-o', '@OUTPUT@',\n> >> +                                                  '-t', gen_gst_controls_template, '@INPUT@'],\n> >> +                                       env : py_build_env)\n> >> +\n> >>  libcamera_gst_cpp_args = [\n> >>      '-DVERSION=\"@0@\"'.format(libcamera_git_version),\n> >>      '-DPACKAGE=\"@0@\"'.format(meson.project_name()),\n> >> diff --git a/utils/codegen/controls.py \n> >> b/utils/codegen/controls.py\n> >> index 7bafee59..03c77cc6 100644\n> >> --- a/utils/codegen/controls.py\n> >> +++ b/utils/codegen/controls.py\n> >> @@ -110,3 +110,11 @@ class Control(object):\n> >>              return f\"Span<const {typ}, {self.__size}>\"\n> >>          else:\n> >>              return f\"Span<const {typ}>\"\n> >> +\n> >> +    @property\n> >> +    def element_type(self):\n> >> +        return self.__data.get('type')\n> >\n> > I wonder if we shouldn't move the existing type property to\n> > extend_control() in generator scripts, and rename element_type to type.\n> > This can be done on top, you don't have to address it.\n> \n> I'm fine with either, although just calling it type() is a little \n> simpler for the gen-gst-controls.py script.\n> \n> >> +\n> >> +    @property\n> >> +    def size(self):\n> >> +        return self.__size\n> >> diff --git a/utils/codegen/gen-gst-controls.py \n> >> b/utils/codegen/gen-gst-controls.py\n> >> new file mode 100755\n> >> index 00000000..5ac8981f\n> >> --- /dev/null\n> >> +++ b/utils/codegen/gen-gst-controls.py\n> >> @@ -0,0 +1,151 @@\n> >> +#!/usr/bin/env python3\n> >> +# SPDX-License-Identifier: GPL-2.0-or-later\n> >> +# Copyright (C) 2019, Google Inc.\n> >> +# Copyright (C) 2024, Jaslo Ziska\n> >> +#\n> >> +# Authors:\n> >> +# Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >> +# Jaslo Ziska <jaslo@ziska.de>\n> >> +#\n> >> +# Generate gstreamer control properties from YAML\n> >> +\n> >> +import argparse\n> >> +import jinja2\n> >> +import re\n> >> +import sys\n> >> +import yaml\n> >> +\n> >> +from controls import Control\n> >> +\n> >> +\n> >> +def find_common_prefix(strings):\n> >> +    prefix = strings[0]\n> >> +\n> >> +    for string in strings[1:]:\n> >> +        while string[:len(prefix)] != prefix and prefix:\n> >> +            prefix = prefix[:len(prefix) - 1]\n> >> +        if not prefix:\n> >> +            break\n> >> +\n> >> +    return prefix\n> >> +\n> >> +\n> >> +def format_description(description):\n> >> +    # Substitute doxygen keywords \\sa (see also) and \\todo\n> >> +    description = re.sub(r'\\\\sa((?: \\w+)+)',\n> >> +                         lambda match: 'See also: ' + ', '.join(\n> >> +                             map(kebab_case, match.group(1).strip().split(' '))\n> >> +                         ) + '.', description)\n> >> +    description = re.sub(r'\\\\todo', 'Todo:', description)\n> >> +\n> >> +    description = description.strip().split('\\n')\n> >> +    return '\\n'.join([\n> >> +        '\"' + line.replace('\\\\', r'\\\\').replace('\"', r'\\\"') + ' \"' for line in description if line\n> >> +    ]).rstrip()\n> >> +\n> >> +\n> >> +def snake_case(s):\n> >> +    return ''.join([\n> >> +        c.isupper() and ('_' + c.lower()) or c for c in s\n> >> +    ]).strip('_')\n> >> +\n> >> +\n> >> +def kebab_case(s):\n> >> +    return snake_case(s).replace('_', '-')\n> >> +\n> >> +\n> >> +def extend_control(ctrl):\n> >> +    if ctrl.vendor != 'libcamera':\n> >> +        ctrl.namespace = f'{ctrl.vendor}::'\n> >> +        ctrl.vendor_prefix = f'{ctrl.vendor}-'\n> >> +    else:\n> >> +        ctrl.namespace = ''\n> >> +        ctrl.vendor_prefix = ''\n> >> +\n> >> +    ctrl.is_array = ctrl.size is not None\n> >> +\n> >> +    if ctrl.is_enum:\n> >> +        # remove common prefix from enum variant names\n> >\n> > s/# remove/# Remove/\n> >\n> >> +        prefix = find_common_prefix([enum.name for enum in ctrl.enum_values])\n> >> +        for enum in ctrl.enum_values:\n> >> +            enum.gst_name = kebab_case(enum.name.removeprefix(prefix))\n> >> +\n> >> +        ctrl.gtype = 'enum'\n> >> +        ctrl.default = '0'\n> >> +    elif ctrl.element_type == 'bool':\n> >> +        ctrl.gtype = 'boolean'\n> >> +        ctrl.default = 'false'\n> >> +    elif ctrl.element_type == 'float':\n> >> +        ctrl.gtype = 'float'\n> >> +        ctrl.default = '0'\n> >> +        ctrl.min = '-G_MAXFLOAT'\n> >> +        ctrl.max = 'G_MAXFLOAT'\n> >> +    elif ctrl.element_type == 'int32_t':\n> >> +        ctrl.gtype = 'int'\n> >> +        ctrl.default = '0'\n> >> +        ctrl.min = 'G_MININT'\n> >> +        ctrl.max = 'G_MAXINT'\n> >> +    elif ctrl.element_type == 'int64_t':\n> >> +        ctrl.gtype = 'int64'\n> >> +        ctrl.default = '0'\n> >> +        ctrl.min = 'G_MININT64'\n> >> +        ctrl.max = 'G_MAXINT64'\n> >> +    elif ctrl.element_type == 'uint8_t':\n> >> +        ctrl.gtype = 'uchar'\n> >> +        ctrl.default = '0'\n> >> +        ctrl.min = '0'\n> >> +        ctrl.max = 'G_MAXUINT8'\n> >> +    elif ctrl.element_type == 'Rectangle':\n> >> +        ctrl.is_rectangle = True\n> >> +        ctrl.default = '0'\n> >> +        ctrl.min = '0'\n> >> +        ctrl.max = 'G_MAXINT'\n> >> +    else:\n> >> +        raise RuntimeError(f'The type `{ctrl.element_type}` is unknown')\n> >> +\n> >> +    return ctrl\n> >> +\n> >> +\n> >> +def main(argv):\n> >> +    # Parse command line arguments\n> >> +    parser = argparse.ArgumentParser()\n> >> +    parser.add_argument('--output', '-o', metavar='file', type=str,\n> >> +                        help='Output file name. Defaults to standard output if not specified.')\n> >> +    parser.add_argument('--template', '-t', dest='template', type=str, required=True,\n> >> +                        help='Template file name.')\n> >> +    parser.add_argument('input', type=str, nargs='+',\n> >> +                        help='Input file name.')\n> >> +\n> >> +    args = parser.parse_args(argv[1:])\n> >> +\n> >> +    controls = {}\n> >> +    for input in args.input:\n> >> +        data = yaml.safe_load(open(input, 'rb').read())\n> >> +\n> >> +        vendor = data['vendor']\n> >> +        ctrls = controls.setdefault(vendor, [])\n> >> +\n> >> +        for ctrl in data['controls']:\n> >> +            ctrl = Control(*ctrl.popitem(), vendor)\n> >> +            ctrls.append(extend_control(ctrl))\n> >> +\n> >> +    data = {'controls': list(controls.items())}\n> >> +\n> >> +    env = jinja2.Environment()\n> >> +    env.filters['format_description'] = format_description\n> >> +    env.filters['snake_case'] = snake_case\n> >> +    env.filters['kebab_case'] = kebab_case\n> >> +    template = env.from_string(open(args.template, 'r', encoding='utf-8').read())\n> >> +    string = template.render(data)\n> >> +\n> >> +    if args.output:\n> >> +        with open(args.output, 'w', encoding='utf-8') as output:\n> >> +            output.write(string)\n> >> +    else:\n> >> +        sys.stdout.write(string)\n> >> +\n> >> +    return 0\n> >> +\n> >> +\n> >> +if __name__ == '__main__':\n> >> +    sys.exit(main(sys.argv))\n> >> diff --git a/utils/codegen/meson.build \n> >> b/utils/codegen/meson.build\n> >> index adf33bba..904dd66d 100644\n> >> --- a/utils/codegen/meson.build\n> >> +++ b/utils/codegen/meson.build\n> >> @@ -11,6 +11,7 @@ py_modules += ['jinja2', 'yaml']\n> >>  \n> >>  gen_controls = files('gen-controls.py')\n> >>  gen_formats = files('gen-formats.py')\n> >> +gen_gst_controls = files('gen-gst-controls.py')\n> >>  gen_header = files('gen-header.sh')\n> >>  gen_ipa_pub_key = files('gen-ipa-pub-key.py')\n> >>  gen_tracepoints = files('gen-tp-header.py')","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8474BC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 27 Aug 2024 12:43:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 59E2463460;\n\tTue, 27 Aug 2024 14:43:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2FF0D61901\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 27 Aug 2024 14:43:13 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BEBA59FF;\n\tTue, 27 Aug 2024 14:42:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"C4BwOem4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1724762526;\n\tbh=IJ2NTOYdldwsMSz+w+wvI4ChCFuEMvxyrA0djS2oE7Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=C4BwOem4XD38FAfoj8RHrek0jjLJssKgeTqqLJp2bpM0VOG8ksGUcodiz0oWJDFmc\n\tWSBTTR6zCuIDhzdYTCh4cwkuzf3SstgOuawJlVKhlDkMB41jfz7ip75Jn+2Ulkt+m1\n\tJLl9lP4qO+Z1KPFzWGO9VWXwQCRjwA45L8n1kQ4s=","Date":"Tue, 27 Aug 2024 15:43:08 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jaslo Ziska <jaslo@ziska.de>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 3/3] gstreamer: Generate controls from\n\tcontrol_ids_*.yaml files","Message-ID":"<20240827124308.GA19409@pendragon.ideasonboard.com>","References":"<20240813124722.22425-1-jaslo@ziska.de>\n\t<20240813124722.22425-4-jaslo@ziska.de>\n\t<20240816000744.GE23911@pendragon.ideasonboard.com>\n\t<87y14idw2q.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87y14idw2q.fsf@ziska.de>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]