[{"id":32694,"web_url":"https://patchwork.libcamera.org/comment/32694/","msgid":"<173399970168.32744.959157500575036608@ping.linuxembedded.co.uk>","date":"2024-12-12T10:35:01","subject":"Re: [PATCH v4 3/4] libcamera: controls: Add support for querying\n\tdirection information","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2024-12-12 05:24:37)\n> Add support to ControlId for querying direction information. This allows\n> applications to query whether a ControlId is meant for being set in\n> controls or to be returned in metadata or both. This also has a side\n> effect of properly encoding this information, as previously it was only\n> mentioned losely and inconsistently in the control id definition.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n> \n> ---\n> Changes in v4:\n> - add ControlId::direction()\n> \n> Changes in v3:\n> - make direction parameter to ControlId and Control consutrctors\n> mandatory\n> - this entails expanding the control serializer/deserializer for\n> V4L2Device and ControlSerializer to properly handle the direction\n> - also the constructor parameters have to be reordered, compared to\n> the last vesrion at least\n> \n> Changes in v2:\n> - simplify code\n> ---\n> include/libcamera/controls.h | 20 +++++++++-\n> include/libcamera/ipa/ipa_controls.h | 3 +-\n> src/libcamera/control_ids.cpp.in | 4 +-\n> src/libcamera/control_serializer.cpp | 8 +++-\n> src/libcamera/controls.cpp | 56 ++++++++++++++++++++++++++--\n> src/libcamera/v4l2_device.cpp | 10 ++++-\n> 6 files changed, 91 insertions(+), 10 deletions(-)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index b24336cc280f..7c7828ae5db0 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -17,6 +17,7 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -249,14 +250,25 @@ private:\n> class ControlId\n> {\n> public:\n> + enum class Direction {\n> + In = (1 << 0),\n> + Out = (1 << 1),\n> + };\n> +\n> + using DirectionFlags = Flags;\n> +\n> ControlId(unsigned int id, const std::string &name, const std::string &vendor,\n> - ControlType type, std::size_t size = 0,\n> + ControlType type, DirectionFlags direction,\n> + std::size_t size = 0,\n> const std::map &enumStrMap = {});\n> \n> unsigned int id() const { return id_; }\n> const std::string &name() const { return name_; }\n> const std::string &vendor() const { return vendor_; }\n> ControlType type() const { return type_; }\n> + DirectionFlags direction() const { return direction_; }\n> + bool isInput() const { return !!(direction_ & Direction::In); }\n> + bool isOutput() const { return !!(direction_ & Direction::Out); }\n> bool isArray() const { return size_ > 0; }\n> std::size_t size() const { return size_; }\n> const std::map &enumerators() const { return reverseMap_; }\n> @@ -268,11 +280,14 @@ private:\n> std::string name_;\n> std::string vendor_;\n> ControlType type_;\n> + DirectionFlags direction_;\n> std::size_t size_;\n> std::map enumStrMap_;\n> std::map reverseMap_;\n> };\n> \n> +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction)\n> +\n> static inline bool operator==(unsigned int lhs, const ControlId &rhs)\n> {\n> return lhs == rhs.id();\n> @@ -300,9 +315,10 @@ public:\n> using type = T;\n> \n> Control(unsigned int id, const char *name, const char *vendor,\n> + ControlId::DirectionFlags direction,\n> const std::map &enumStrMap = {})\n> : ControlId(id, name, vendor, details::control_type>::value,\n> - details::control_type>::size, enumStrMap)\n> + direction, details::control_type>::size, enumStrMap)\n> {\n> }\n> \n> diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h\n> index 5fd13394fcef..980668c86bcc 100644\n> --- a/include/libcamera/ipa/ipa_controls.h\n> +++ b/include/libcamera/ipa/ipa_controls.h\n> @@ -46,7 +46,8 @@ struct ipa_control_info_entry {\n> uint32_t id;\n> uint32_t type;\n> uint32_t offset;\n> - uint32_t padding[1];\n> + uint8_t direction;\n> + uint8_t padding[3];\n> };\n> \n> #ifdef __cplusplus\n> diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in\n> index afe9e2c96610..65668d486dbc 100644\n> --- a/src/libcamera/control_ids.cpp.in\n> +++ b/src/libcamera/control_ids.cpp.in\n> @@ -89,9 +89,9 @@ extern const std::map {{ctrl.name}}NameValueMap = {\n> { \"{{enum.name}}\", {{enum.name}} },\n> {%- endfor %}\n> };\n> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, \"{{ctrl.name}}\", \"{{vendor}}\", {{ctrl.name}}NameValueMap);\n> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, \"{{ctrl.name}}\", \"{{vendor}}\", {{ctrl.direction}}, {{ctrl.name}}NameValueMap);\n> {% else -%}\n> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, \"{{ctrl.name}}\", \"{{vendor}}\");\n> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, \"{{ctrl.name}}\", \"{{vendor}}\", {{ctrl.direction}});\n> {% endif -%}\n> {%- endfor %}\n> \n> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> index 0a5e8220a0ff..17834648c3f0 100644\n> --- a/src/libcamera/control_serializer.cpp\n> +++ b/src/libcamera/control_serializer.cpp\n> @@ -281,6 +281,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,\n> entry.id = id->id();\n> entry.type = id->type();\n> entry.offset = values.offset();\n> + entry.direction = static_cast(id->direction());\n> entries.write(&entry);\n> \n> store(info, values);\n> @@ -493,12 +494,17 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &\n> \n> /* If we're using a local id map, populate it. */\n> if (localIdMap) {\n> + ControlId::DirectionFlags flags{\n> + static_cast(entry->direction)\n> + };\n> +\n> /**\n> * \\todo Find a way to preserve the control name for\n> * debugging purpose.\n> */\n> controlIds_.emplace_back(std::make_unique(entry->id,\n> - \"\", \"local\", type));\n> + \"\", \"local\", type,\n> + flags));\n> (*localIdMap)[entry->id] = controlIds_.back().get();\n> }\n> \n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 65eeef2db9c2..6296aac6fff2 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -412,15 +412,16 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> * \\param[in] name The control name\n> * \\param[in] vendor The vendor name\n> * \\param[in] type The control data type\n> + * \\param[in] direction The direction of the control, if it can be used in Controls or Metadata\n> * \\param[in] size The size of the array control, or 0 if scalar control\n> * \\param[in] enumStrMap The map from enum names to values (optional)\n> */\n> ControlId::ControlId(unsigned int id, const std::string &name,\n> const std::string &vendor, ControlType type,\n> - std::size_t size,\n> + DirectionFlags direction, std::size_t size,\n> const std::map &enumStrMap)\n> - : id_(id), name_(name), vendor_(vendor), type_(type), size_(size),\n> - enumStrMap_(enumStrMap)\n> + : id_(id), name_(name), vendor_(vendor), type_(type),\n> + direction_(direction), size_(size), enumStrMap_(enumStrMap)\n> {\n> for (const auto &pair : enumStrMap_)\n> reverseMap_[pair.second] = pair.first;\n> @@ -450,6 +451,37 @@ ControlId::ControlId(unsigned int id, const std::string &name,\n> * \\return The control data type\n> */\n> \n> +/**\n> + * \\fn DirectionFlags direction() const\n> + * \\brief Return the direction that the control can be used in\n> + *\n> + * This is similar to \\sa isInput() and \\sa isOutput(), but returns the flags\n> + * direction instead of booleans for each direction.\n> + *\n> + * \\return The direction flags corresponding to if the control can be used as\n> + * an input control or as output metadata\n> + */\n> +\n> +/**\n> + * \\fn bool ControlId::isInput() const\n> + * \\brief Determine if the control is available to be used as an input control\n> + *\n> + * Controls can be used either as input in controls, or as output in metadata.\n> + * This function checks if the control is allowed to be used as the former.\n> + *\n> + * \\return True if the control can be used as an input control, false otherwise\n> + */\n> +\n> +/**\n> + * \\fn bool ControlId::isOutput() const\n> + * \\brief Determine if the control is available to be used in output metadata\n> + *\n> + * Controls can be used either as input in controls, or as output in metadata.\n> + * This function checks if the control is allowed to be used as the latter.\n> + *\n> + * \\return True if the control can be returned in output metadata, false otherwise\n> + */\n> +\n> /**\n> * \\fn bool ControlId::isArray() const\n> * \\brief Determine if the control is an array control\n> @@ -487,6 +519,22 @@ ControlId::ControlId(unsigned int id, const std::string &name,\n> * \\return True if \\a lhs.id() is equal to \\a rhs, false otherwise\n> */\n> \n> +/**\n> + * \\enum ControlId::Direction\n> + * \\brief The direction the control is capable of being passed from/to\n> + *\n> + * \\var ControlId::Direction::In\n> + * \\brief The control can be passed as input in controls\n> + *\n> + * \\var ControlId::Direction::Out\n> + * \\brief The control can be returned as output in metadata\n> + */\n> +\n> +/**\n> + * \\typedef ControlId::DirectionFlags\n> + * \\brief A wrapper for ControlId::Direction so that it can be used as flags\n> + */\n> +\n> /**\n> * \\class Control\n> * \\brief Describe a control and its intrinsic properties\n> @@ -520,6 +568,8 @@ ControlId::ControlId(unsigned int id, const std::string &name,\n> * \\param[in] id The control numerical ID\n> * \\param[in] name The control name\n> * \\param[in] vendor The vendor name\n> + * \\param[in] direction The direction of the control, if it can be used in\n> + * Controls or Metadata\n> * \\param[in] enumStrMap The map from enum names to values (optional)\n> *\n> * The control data type is automatically deduced from the template type T.\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 664b74afe6a1..2f65a43a0547 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -565,7 +565,15 @@ std::unique_ptr V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl &\n> const std::string name(static_cast(ctrl.name), len);\n> const ControlType type = v4l2CtrlType(ctrl.type);\n> \n> - return std::make_unique(ctrl.id, name, \"v4l2\", type);\n> + ControlId::DirectionFlags flags;\n> + if (ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY)\n> + flags = ControlId::Direction::Out;\n> + else if (ctrl.flags & V4L2_CTRL_FLAG_WRITE_ONLY)\n> + flags = ControlId::Direction::In;\n> + else\n> + flags = ControlId::Direction::In | ControlId::Direction::Out;\n> +\n\nAll my comments addressed, so \n\n\nReviewed-by: Kieran Bingham \n\n> + return std::make_unique(ctrl.id, name, \"v4l2\", type, flags);\n> }\n> \n> /**\n> -- \n> 2.39.2\n>","headers":{"Return-Path":"","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 BC892BD80A\n\tfor ;\n\tThu, 12 Dec 2024 10:35:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5C27E67ECC;\n\tThu, 12 Dec 2024 11:35:05 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 632C667E6D\n\tfor ;\n\tThu, 12 Dec 2024 11:35:04 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CC7EF229;\n\tThu, 12 Dec 2024 11:34:30 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"CWX/bfd2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733999670;\n\tbh=+QtdVqVN8hSF1Ba3t/dSRP2aFbAP6cexOJNZrM1UsGo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=CWX/bfd2PNLAiNXMhrVpvUtOoGKMnKGdLHoIBL8+qO2E+TcAq6wkP5ZP3Y4g6VizX\n\tEcPELxqmkIBbtfu3qPYNaIpgNQ43kdPUVYvfon6LedeCW9+zNnTAsq8tQzAPy6BZtf\n\t+SjOhEY+mpMNoffhGw+Fn9Ady0l4VE2QdXY2VBNg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241212052438.1547410-4-paul.elder@ideasonboard.com>","References":"<20241212052438.1547410-1-paul.elder@ideasonboard.com>\n\t<20241212052438.1547410-4-paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v4 3/4] libcamera: controls: Add support for querying\n\tdirection information","From":"Kieran Bingham ","Cc":"Paul Elder ,\n\tLaurent Pinchart ","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 12 Dec 2024 10:35:01 +0000","Message-ID":"<173399970168.32744.959157500575036608@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":32730,"web_url":"https://patchwork.libcamera.org/comment/32730/","msgid":"<20241215172702.GJ9975@pendragon.ideasonboard.com>","date":"2024-12-15T17:27:02","subject":"Re: [PATCH v4 3/4] libcamera: controls: Add support for querying\n\tdirection information","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Thu, Dec 12, 2024 at 02:24:37PM +0900, Paul Elder wrote:\n> Add support to ControlId for querying direction information. This allows\n> applications to query whether a ControlId is meant for being set in\n> controls or to be returned in metadata or both. This also has a side\n> effect of properly encoding this information, as previously it was only\n> mentioned losely and inconsistently in the control id definition.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n> \n> ---\n> Changes in v4:\n> - add ControlId::direction()\n> \n> Changes in v3:\n> - make direction parameter to ControlId and Control consutrctors\n> mandatory\n> - this entails expanding the control serializer/deserializer for\n> V4L2Device and ControlSerializer to properly handle the direction\n> - also the constructor parameters have to be reordered, compared to\n> the last vesrion at least\n> \n> Changes in v2:\n> - simplify code\n> ---\n> include/libcamera/controls.h | 20 +++++++++-\n> include/libcamera/ipa/ipa_controls.h | 3 +-\n> src/libcamera/control_ids.cpp.in | 4 +-\n> src/libcamera/control_serializer.cpp | 8 +++-\n> src/libcamera/controls.cpp | 56 ++++++++++++++++++++++++++--\n> src/libcamera/v4l2_device.cpp | 10 ++++-\n> 6 files changed, 91 insertions(+), 10 deletions(-)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index b24336cc280f..7c7828ae5db0 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -17,6 +17,7 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -249,14 +250,25 @@ private:\n> class ControlId\n> {\n> public:\n> +\tenum class Direction {\n> +\t\tIn = (1 << 0),\n> +\t\tOut = (1 << 1),\n> +\t};\n> +\n> +\tusing DirectionFlags = Flags;\n> +\n> \tControlId(unsigned int id, const std::string &name, const std::string &vendor,\n> -\t\t ControlType type, std::size_t size = 0,\n> +\t\t ControlType type, DirectionFlags direction,\n> +\t\t std::size_t size = 0,\n> \t\t const std::map &enumStrMap = {});\n> \n> \tunsigned int id() const { return id_; }\n> \tconst std::string &name() const { return name_; }\n> \tconst std::string &vendor() const { return vendor_; }\n> \tControlType type() const { return type_; }\n> +\tDirectionFlags direction() const { return direction_; }\n> +\tbool isInput() const { return !!(direction_ & Direction::In); }\n> +\tbool isOutput() const { return !!(direction_ & Direction::Out); }\n> \tbool isArray() const { return size_ > 0; }\n> \tstd::size_t size() const { return size_; }\n> \tconst std::map &enumerators() const { return reverseMap_; }\n> @@ -268,11 +280,14 @@ private:\n> \tstd::string name_;\n> \tstd::string vendor_;\n> \tControlType type_;\n> +\tDirectionFlags direction_;\n> \tstd::size_t size_;\n> \tstd::map enumStrMap_;\n> \tstd::map reverseMap_;\n> };\n> \n> +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction)\n> +\n> static inline bool operator==(unsigned int lhs, const ControlId &rhs)\n> {\n> \treturn lhs == rhs.id();\n> @@ -300,9 +315,10 @@ public:\n> \tusing type = T;\n> \n> \tControl(unsigned int id, const char *name, const char *vendor,\n> +\t\tControlId::DirectionFlags direction,\n> \t\tconst std::map &enumStrMap = {})\n> \t\t: ControlId(id, name, vendor, details::control_type>::value,\n> -\t\t\t details::control_type>::size, enumStrMap)\n> +\t\t\t direction, details::control_type>::size, enumStrMap)\n> \t{\n> \t}\n> \n> diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h\n> index 5fd13394fcef..980668c86bcc 100644\n> --- a/include/libcamera/ipa/ipa_controls.h\n> +++ b/include/libcamera/ipa/ipa_controls.h\n> @@ -46,7 +46,8 @@ struct ipa_control_info_entry {\n> \tuint32_t id;\n> \tuint32_t type;\n> \tuint32_t offset;\n> -\tuint32_t padding[1];\n> +\tuint8_t direction;\n> +\tuint8_t padding[3];\n> };\n> \n> #ifdef __cplusplus\n> diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in\n> index afe9e2c96610..65668d486dbc 100644\n> --- a/src/libcamera/control_ids.cpp.in\n> +++ b/src/libcamera/control_ids.cpp.in\n> @@ -89,9 +89,9 @@ extern const std::map {{ctrl.name}}NameValueMap = {\n> \t{ \"{{enum.name}}\", {{enum.name}} },\n> {%- endfor %}\n> };\n> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, \"{{ctrl.name}}\", \"{{vendor}}\", {{ctrl.name}}NameValueMap);\n> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, \"{{ctrl.name}}\", \"{{vendor}}\", {{ctrl.direction}}, {{ctrl.name}}NameValueMap);\n> {% else -%}\n> -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, \"{{ctrl.name}}\", \"{{vendor}}\");\n> +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, \"{{ctrl.name}}\", \"{{vendor}}\", {{ctrl.direction}});\n> {% endif -%}\n> {%- endfor %}\n> \n> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> index 0a5e8220a0ff..17834648c3f0 100644\n> --- a/src/libcamera/control_serializer.cpp\n> +++ b/src/libcamera/control_serializer.cpp\n> @@ -281,6 +281,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap,\n> \t\tentry.id = id->id();\n> \t\tentry.type = id->type();\n> \t\tentry.offset = values.offset();\n> +\t\tentry.direction = static_cast(id->direction());\n> \t\tentries.write(&entry);\n> \n> \t\tstore(info, values);\n> @@ -493,12 +494,17 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &\n> \n> \t\t/* If we're using a local id map, populate it. */\n> \t\tif (localIdMap) {\n\nShould there be a check here that entry->direction doesn't contain any\nunsupported flag ? If so, and if it does, I haven't thought about which\nerror handling strategy would be best, between making it a hard error,\nor logging a warning and clearing unsupported flags.\n\nWith that addressed (if needed),\n\nReviewed-by: Laurent Pinchart \n\n> +\t\t\tControlId::DirectionFlags flags{\n> +\t\t\t\tstatic_cast(entry->direction)\n> +\t\t\t};\n> +\n> \t\t\t/**\n> \t\t\t * \\todo Find a way to preserve the control name for\n> \t\t\t * debugging purpose.\n> \t\t\t */\n> \t\t\tcontrolIds_.emplace_back(std::make_unique(entry->id,\n> -\t\t\t\t\t\t\t\t\t \"\", \"local\", type));\n> +\t\t\t\t\t\t\t\t\t \"\", \"local\", type,\n> +\t\t\t\t\t\t\t\t\t flags));\n> \t\t\t(*localIdMap)[entry->id] = controlIds_.back().get();\n> \t\t}\n> \n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 65eeef2db9c2..6296aac6fff2 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -412,15 +412,16 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> * \\param[in] name The control name\n> * \\param[in] vendor The vendor name\n> * \\param[in] type The control data type\n> + * \\param[in] direction The direction of the control, if it can be used in Controls or Metadata\n> * \\param[in] size The size of the array control, or 0 if scalar control\n> * \\param[in] enumStrMap The map from enum names to values (optional)\n> */\n> ControlId::ControlId(unsigned int id, const std::string &name,\n> \t\t const std::string &vendor, ControlType type,\n> -\t\t std::size_t size,\n> +\t\t DirectionFlags direction, std::size_t size,\n> \t\t const std::map &enumStrMap)\n> -\t: id_(id), name_(name), vendor_(vendor), type_(type), size_(size),\n> -\t enumStrMap_(enumStrMap)\n> +\t: id_(id), name_(name), vendor_(vendor), type_(type),\n> +\t direction_(direction), size_(size), enumStrMap_(enumStrMap)\n> {\n> \tfor (const auto &pair : enumStrMap_)\n> \t\treverseMap_[pair.second] = pair.first;\n> @@ -450,6 +451,37 @@ ControlId::ControlId(unsigned int id, const std::string &name,\n> * \\return The control data type\n> */\n> \n> +/**\n> + * \\fn DirectionFlags direction() const\n> + * \\brief Return the direction that the control can be used in\n> + *\n> + * This is similar to \\sa isInput() and \\sa isOutput(), but returns the flags\n> + * direction instead of booleans for each direction.\n> + *\n> + * \\return The direction flags corresponding to if the control can be used as\n> + * an input control or as output metadata\n> + */\n> +\n> +/**\n> + * \\fn bool ControlId::isInput() const\n> + * \\brief Determine if the control is available to be used as an input control\n> + *\n> + * Controls can be used either as input in controls, or as output in metadata.\n> + * This function checks if the control is allowed to be used as the former.\n> + *\n> + * \\return True if the control can be used as an input control, false otherwise\n> + */\n> +\n> +/**\n> + * \\fn bool ControlId::isOutput() const\n> + * \\brief Determine if the control is available to be used in output metadata\n> + *\n> + * Controls can be used either as input in controls, or as output in metadata.\n> + * This function checks if the control is allowed to be used as the latter.\n> + *\n> + * \\return True if the control can be returned in output metadata, false otherwise\n> + */\n> +\n> /**\n> * \\fn bool ControlId::isArray() const\n> * \\brief Determine if the control is an array control\n> @@ -487,6 +519,22 @@ ControlId::ControlId(unsigned int id, const std::string &name,\n> * \\return True if \\a lhs.id() is equal to \\a rhs, false otherwise\n> */\n> \n> +/**\n> + * \\enum ControlId::Direction\n> + * \\brief The direction the control is capable of being passed from/to\n> + *\n> + * \\var ControlId::Direction::In\n> + * \\brief The control can be passed as input in controls\n> + *\n> + * \\var ControlId::Direction::Out\n> + * \\brief The control can be returned as output in metadata\n> + */\n> +\n> +/**\n> + * \\typedef ControlId::DirectionFlags\n> + * \\brief A wrapper for ControlId::Direction so that it can be used as flags\n> + */\n> +\n> /**\n> * \\class Control\n> * \\brief Describe a control and its intrinsic properties\n> @@ -520,6 +568,8 @@ ControlId::ControlId(unsigned int id, const std::string &name,\n> * \\param[in] id The control numerical ID\n> * \\param[in] name The control name\n> * \\param[in] vendor The vendor name\n> + * \\param[in] direction The direction of the control, if it can be used in\n> + * Controls or Metadata\n> * \\param[in] enumStrMap The map from enum names to values (optional)\n> *\n> * The control data type is automatically deduced from the template type T.\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 664b74afe6a1..2f65a43a0547 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -565,7 +565,15 @@ std::unique_ptr V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl &\n> \tconst std::string name(static_cast(ctrl.name), len);\n> \tconst ControlType type = v4l2CtrlType(ctrl.type);\n> \n> -\treturn std::make_unique(ctrl.id, name, \"v4l2\", type);\n> +\tControlId::DirectionFlags flags;\n> +\tif (ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY)\n> +\t\tflags = ControlId::Direction::Out;\n> +\telse if (ctrl.flags & V4L2_CTRL_FLAG_WRITE_ONLY)\n> +\t\tflags = ControlId::Direction::In;\n> +\telse\n> +\t\tflags = ControlId::Direction::In | ControlId::Direction::Out;\n> +\n> +\treturn std::make_unique(ctrl.id, name, \"v4l2\", type, flags);\n> }\n> \n> /**","headers":{"Return-Path":"","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 7577DC32F5\n\tfor ;\n\tSun, 15 Dec 2024 17:27:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C4CE67F07;\n\tSun, 15 Dec 2024 18:27:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D79467EEE\n\tfor ;\n\tSun, 15 Dec 2024 18:27:19 +0100 (CET)","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 31C3799F;\n\tSun, 15 Dec 2024 18:26:43 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"O8vvAcE2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734283603;\n\tbh=MrcLWtTospq2j3nypkOF6l04oU2EB2n2d/VgUy3VhXI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=O8vvAcE2e1tyAangwBXvwPNXsMP7aDKX2s83g4idTN2tpS3iuQxjp/SPxKRePkJSP\n\tQCZobZvuUGpME02JxHGiojai6cE7i6kC9rR70D+azMv4Qx4f0vENo2JWC+IU7VzEiX\n\to3uckMRX6hGs/7dZ7SoJ//7QgmIBDMCqK7p0bR7A=","Date":"Sun, 15 Dec 2024 19:27:02 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 3/4] libcamera: controls: Add support for querying\n\tdirection information","Message-ID":"<20241215172702.GJ9975@pendragon.ideasonboard.com>","References":"<20241212052438.1547410-1-paul.elder@ideasonboard.com>\n\t<20241212052438.1547410-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20241212052438.1547410-4-paul.elder@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]