| Message ID | 20241127085017.2192069-4-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Wed, Nov 27, 2024 at 05:50:16PM +0900, Paul Elder wrote: > Add support to ControlId for querying direction information. This allows > applications to query whether a ControlId is meant for being set in > controls or to be returned in metadata or both. This also has a side > effect of properly encoding this information, as previously it was only > mentioned losely and inconsistently in the control id definition. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v2: > - simplify code > --- > include/libcamera/controls.h | 20 ++++++++++++++- > src/libcamera/control_ids.cpp.in | 4 +-- > src/libcamera/controls.cpp | 43 ++++++++++++++++++++++++++++++-- > 3 files changed, 62 insertions(+), 5 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 3cfe2de5973c..74a566a05db4 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -17,6 +17,7 @@ > #include <vector> > > #include <libcamera/base/class.h> > +#include <libcamera/base/flags.h> > #include <libcamera/base/span.h> > > #include <libcamera/geometry.h> > @@ -235,8 +236,18 @@ private: > class ControlId > { > public: > + enum class Direction { > + In = (1 << 0), > + Out = (1 << 1), > + }; > + > + using DirectionFlags = Flags<Direction>; > + > ControlId(unsigned int id, const std::string &name, const std::string &vendor, > ControlType type, std::size_t size = 0, > + DirectionFlags direction = > + static_cast<DirectionFlags>(Direction::In) | > + static_cast<DirectionFlags>(Direction::Out), I think I would still prefer not having a default value here, as all controls should have a direction. This will require setting the direction explicitly when creating ControlId instances for V4L2 controls or in the control serializer, but I think that's better, as it will force us to look at how we handle those instead of forgetting about them because they don't appear in this patch. > const std::map<std::string, int32_t> &enumStrMap = {}); > > unsigned int id() const { return id_; } > @@ -245,6 +256,8 @@ public: > ControlType type() const { return type_; } > bool isArray() const { return size_ > 0; } > std::size_t size() const { return size_; } > + bool isInput() const { return !!(direction_ & Direction::In); } > + bool isOutput() const { return !!(direction_ & Direction::Out); } > const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; } > > private: > @@ -255,10 +268,13 @@ private: > std::string vendor_; > ControlType type_; > std::size_t size_; > + DirectionFlags direction_; > std::map<std::string, int32_t> enumStrMap_; > std::map<int32_t, std::string> reverseMap_; > }; > > +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction) > + > static inline bool operator==(unsigned int lhs, const ControlId &rhs) > { > return lhs == rhs.id(); > @@ -286,9 +302,11 @@ public: > using type = T; > > Control(unsigned int id, const char *name, const char *vendor, > + ControlId::DirectionFlags direction = ControlId::Direction::In > + | ControlId::Direction::Out, > const std::map<std::string, int32_t> &enumStrMap = {}) > : ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value, > - details::control_type<std::remove_cv_t<T>>::size, enumStrMap) > + details::control_type<std::remove_cv_t<T>>::size, direction, enumStrMap) > { > } > > diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in > index afe9e2c96610..65668d486dbc 100644 > --- a/src/libcamera/control_ids.cpp.in > +++ b/src/libcamera/control_ids.cpp.in > @@ -89,9 +89,9 @@ extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = { > { "{{enum.name}}", {{enum.name}} }, > {%- endfor %} > }; > -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.name}}NameValueMap); > +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}, {{ctrl.name}}NameValueMap); > {% else -%} > -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}"); > +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}); > {% endif -%} > {%- endfor %} > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 2efecf0fc52b..f221a7e621ab 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -397,14 +397,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen > * \param[in] vendor The vendor name > * \param[in] type The control data type > * \param[in] size The size of the array control, or 0 if scalar control > + * \param[in] direction The direction of the control, if it can be used in Controls or Metadata > * \param[in] enumStrMap The map from enum names to values (optional) > */ > ControlId::ControlId(unsigned int id, const std::string &name, > const std::string &vendor, ControlType type, > - std::size_t size, > + std::size_t size, DirectionFlags direction, > const std::map<std::string, int32_t> &enumStrMap) > : id_(id), name_(name), vendor_(vendor), type_(type), size_(size), > - enumStrMap_(enumStrMap) > + direction_(direction), enumStrMap_(enumStrMap) > { > for (const auto &pair : enumStrMap_) > reverseMap_[pair.second] = pair.first; > @@ -440,6 +441,26 @@ ControlId::ControlId(unsigned int id, const std::string &name, > * \return True if the control is an array control, false otherwise > */ > > +/** > + * \fn bool ControlId::isInput() const > + * \brief Determine if the control is available to be used as an input control > + * > + * Controls can be used either as input as a control, or as output in metadata. Maybe "as input in controls" to match the "in metadata" ? > + * This function checks if the control is allowed to be used as the former. > + * > + * \return True if the control can be used as an input control, false otherwise > + */ > + > +/** > + * \fn bool ControlId::isOutput() const > + * \brief Determine if the control is available to be used in output metadata > + * > + * Controls can be used either as input as a control, or as output in metadata. > + * This function checks if the control is allowed to be used as the latter. > + * > + * \return True if the control can be returned in output metadata, false otherwise > + */ > + > /** > * \fn std::size_t ControlId::size() const > * \brief Retrieve the size of the control if it is an array control > @@ -471,6 +492,22 @@ ControlId::ControlId(unsigned int id, const std::string &name, > * \return True if \a lhs.id() is equal to \a rhs, false otherwise > */ > > +/** > + * \enum ControlId::Direction > + * \brief The direction that a control of the ControlId is capable of being passed from/to This sounds a bit weird. > + * > + * \var ControlId::Direction::In > + * \brief The control can be passed as input in controls > + * > + * \var ControlId::Direction::Out > + * \brief The control can be returned as output in metadata > + */ > + > +/** > + * \typedef ControlId::DirectionFlags > + * \brief A wrapper for ControlId::Direction so that it can be used as flags > + */ > + > /** > * \class Control > * \brief Describe a control and its intrinsic properties > @@ -504,6 +541,8 @@ ControlId::ControlId(unsigned int id, const std::string &name, > * \param[in] id The control numerical ID > * \param[in] name The control name > * \param[in] vendor The vendor name > + * \param[in] direction The direction of the control, if it can be used in > + * Controls or Metadata > * \param[in] enumStrMap The map from enum names to values (optional) > * > * The control data type is automatically deduced from the template type T.
Quoting Laurent Pinchart (2024-11-27 13:55:12) > Hi Paul, > > Thank you for the patch. > > On Wed, Nov 27, 2024 at 05:50:16PM +0900, Paul Elder wrote: > > Add support to ControlId for querying direction information. This allows > > applications to query whether a ControlId is meant for being set in > > controls or to be returned in metadata or both. This also has a side > > effect of properly encoding this information, as previously it was only > > mentioned losely and inconsistently in the control id definition. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > Changes in v2: > > - simplify code > > --- > > include/libcamera/controls.h | 20 ++++++++++++++- > > src/libcamera/control_ids.cpp.in | 4 +-- > > src/libcamera/controls.cpp | 43 ++++++++++++++++++++++++++++++-- > > 3 files changed, 62 insertions(+), 5 deletions(-) > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > > index 3cfe2de5973c..74a566a05db4 100644 > > --- a/include/libcamera/controls.h > > +++ b/include/libcamera/controls.h > > @@ -17,6 +17,7 @@ > > #include <vector> > > > > #include <libcamera/base/class.h> > > +#include <libcamera/base/flags.h> > > #include <libcamera/base/span.h> > > > > #include <libcamera/geometry.h> > > @@ -235,8 +236,18 @@ private: > > class ControlId > > { > > public: > > + enum class Direction { > > + In = (1 << 0), > > + Out = (1 << 1), > > + }; > > + > > + using DirectionFlags = Flags<Direction>; > > + > > ControlId(unsigned int id, const std::string &name, const std::string &vendor, > > ControlType type, std::size_t size = 0, > > + DirectionFlags direction = > > + static_cast<DirectionFlags>(Direction::In) | > > + static_cast<DirectionFlags>(Direction::Out), > > I think I would still prefer not having a default value here, as all > controls should have a direction. This will require setting the > direction explicitly when creating ControlId instances for V4L2 controls > or in the control serializer, but I think that's better, as it will > force us to look at how we handle those instead of forgetting about them > because they don't appear in this patch. Does that mean we'll be able to use this to convey 'ReadOnly' controls from V4L2 ? I assume we can construct controls from V4L2 accordingly with this now? and superseed https://patchwork.libcamera.org/project/libcamera/list/?series=4064 and https://patchwork.libcamera.org/project/libcamera/list/?series=3654 with this implementation? -- Kieran > > > const std::map<std::string, int32_t> &enumStrMap = {}); > > > > unsigned int id() const { return id_; } > > @@ -245,6 +256,8 @@ public: > > ControlType type() const { return type_; } > > bool isArray() const { return size_ > 0; } > > std::size_t size() const { return size_; } > > + bool isInput() const { return !!(direction_ & Direction::In); } > > + bool isOutput() const { return !!(direction_ & Direction::Out); } > > const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; } > > > > private: > > @@ -255,10 +268,13 @@ private: > > std::string vendor_; > > ControlType type_; > > std::size_t size_; > > + DirectionFlags direction_; > > std::map<std::string, int32_t> enumStrMap_; > > std::map<int32_t, std::string> reverseMap_; > > }; > > > > +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction) > > + > > static inline bool operator==(unsigned int lhs, const ControlId &rhs) > > { > > return lhs == rhs.id(); > > @@ -286,9 +302,11 @@ public: > > using type = T; > > > > Control(unsigned int id, const char *name, const char *vendor, > > + ControlId::DirectionFlags direction = ControlId::Direction::In > > + | ControlId::Direction::Out, > > const std::map<std::string, int32_t> &enumStrMap = {}) > > : ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value, > > - details::control_type<std::remove_cv_t<T>>::size, enumStrMap) > > + details::control_type<std::remove_cv_t<T>>::size, direction, enumStrMap) > > { > > } > > > > diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in > > index afe9e2c96610..65668d486dbc 100644 > > --- a/src/libcamera/control_ids.cpp.in > > +++ b/src/libcamera/control_ids.cpp.in > > @@ -89,9 +89,9 @@ extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = { > > { "{{enum.name}}", {{enum.name}} }, > > {%- endfor %} > > }; > > -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.name}}NameValueMap); > > +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}, {{ctrl.name}}NameValueMap); > > {% else -%} > > -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}"); > > +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}); > > {% endif -%} > > {%- endfor %} > > > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > > index 2efecf0fc52b..f221a7e621ab 100644 > > --- a/src/libcamera/controls.cpp > > +++ b/src/libcamera/controls.cpp > > @@ -397,14 +397,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen > > * \param[in] vendor The vendor name > > * \param[in] type The control data type > > * \param[in] size The size of the array control, or 0 if scalar control > > + * \param[in] direction The direction of the control, if it can be used in Controls or Metadata > > * \param[in] enumStrMap The map from enum names to values (optional) > > */ > > ControlId::ControlId(unsigned int id, const std::string &name, > > const std::string &vendor, ControlType type, > > - std::size_t size, > > + std::size_t size, DirectionFlags direction, > > const std::map<std::string, int32_t> &enumStrMap) > > : id_(id), name_(name), vendor_(vendor), type_(type), size_(size), > > - enumStrMap_(enumStrMap) > > + direction_(direction), enumStrMap_(enumStrMap) > > { > > for (const auto &pair : enumStrMap_) > > reverseMap_[pair.second] = pair.first; > > @@ -440,6 +441,26 @@ ControlId::ControlId(unsigned int id, const std::string &name, > > * \return True if the control is an array control, false otherwise > > */ > > > > +/** > > + * \fn bool ControlId::isInput() const > > + * \brief Determine if the control is available to be used as an input control > > + * > > + * Controls can be used either as input as a control, or as output in metadata. > > Maybe "as input in controls" to match the "in metadata" ? > > > + * This function checks if the control is allowed to be used as the former. > > + * > > + * \return True if the control can be used as an input control, false otherwise > > + */ > > + > > +/** > > + * \fn bool ControlId::isOutput() const > > + * \brief Determine if the control is available to be used in output metadata > > + * > > + * Controls can be used either as input as a control, or as output in metadata. > > + * This function checks if the control is allowed to be used as the latter. > > + * > > + * \return True if the control can be returned in output metadata, false otherwise > > + */ > > + > > /** > > * \fn std::size_t ControlId::size() const > > * \brief Retrieve the size of the control if it is an array control > > @@ -471,6 +492,22 @@ ControlId::ControlId(unsigned int id, const std::string &name, > > * \return True if \a lhs.id() is equal to \a rhs, false otherwise > > */ > > > > +/** > > + * \enum ControlId::Direction > > + * \brief The direction that a control of the ControlId is capable of being passed from/to > > This sounds a bit weird. > > > + * > > + * \var ControlId::Direction::In > > + * \brief The control can be passed as input in controls > > + * > > + * \var ControlId::Direction::Out > > + * \brief The control can be returned as output in metadata > > + */ > > + > > +/** > > + * \typedef ControlId::DirectionFlags > > + * \brief A wrapper for ControlId::Direction so that it can be used as flags > > + */ > > + > > /** > > * \class Control > > * \brief Describe a control and its intrinsic properties > > @@ -504,6 +541,8 @@ ControlId::ControlId(unsigned int id, const std::string &name, > > * \param[in] id The control numerical ID > > * \param[in] name The control name > > * \param[in] vendor The vendor name > > + * \param[in] direction The direction of the control, if it can be used in > > + * Controls or Metadata > > * \param[in] enumStrMap The map from enum names to values (optional) > > * > > * The control data type is automatically deduced from the template type T. > > -- > Regards, > > Laurent Pinchart
On Wed, Dec 04, 2024 at 11:46:39AM +0000, Kieran Bingham wrote: > Quoting Laurent Pinchart (2024-11-27 13:55:12) > > Hi Paul, > > > > Thank you for the patch. > > > > On Wed, Nov 27, 2024 at 05:50:16PM +0900, Paul Elder wrote: > > > Add support to ControlId for querying direction information. This allows > > > applications to query whether a ControlId is meant for being set in > > > controls or to be returned in metadata or both. This also has a side > > > effect of properly encoding this information, as previously it was only > > > mentioned losely and inconsistently in the control id definition. > > > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > > > --- > > > Changes in v2: > > > - simplify code > > > --- > > > include/libcamera/controls.h | 20 ++++++++++++++- > > > src/libcamera/control_ids.cpp.in | 4 +-- > > > src/libcamera/controls.cpp | 43 ++++++++++++++++++++++++++++++-- > > > 3 files changed, 62 insertions(+), 5 deletions(-) > > > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > > > index 3cfe2de5973c..74a566a05db4 100644 > > > --- a/include/libcamera/controls.h > > > +++ b/include/libcamera/controls.h > > > @@ -17,6 +17,7 @@ > > > #include <vector> > > > > > > #include <libcamera/base/class.h> > > > +#include <libcamera/base/flags.h> > > > #include <libcamera/base/span.h> > > > > > > #include <libcamera/geometry.h> > > > @@ -235,8 +236,18 @@ private: > > > class ControlId > > > { > > > public: > > > + enum class Direction { > > > + In = (1 << 0), > > > + Out = (1 << 1), > > > + }; > > > + > > > + using DirectionFlags = Flags<Direction>; > > > + > > > ControlId(unsigned int id, const std::string &name, const std::string &vendor, > > > ControlType type, std::size_t size = 0, > > > + DirectionFlags direction = > > > + static_cast<DirectionFlags>(Direction::In) | > > > + static_cast<DirectionFlags>(Direction::Out), > > > > I think I would still prefer not having a default value here, as all > > controls should have a direction. This will require setting the > > direction explicitly when creating ControlId instances for V4L2 controls > > or in the control serializer, but I think that's better, as it will > > force us to look at how we handle those instead of forgetting about them > > because they don't appear in this patch. > > Does that mean we'll be able to use this to convey 'ReadOnly' controls > from V4L2 ? > > I assume we can construct controls from V4L2 accordingly with this now? > and superseed > https://patchwork.libcamera.org/project/libcamera/list/?series=4064 and > https://patchwork.libcamera.org/project/libcamera/list/?series=3654 with > this implementation? We may need a few adjustements, but I think it's worth checking. I like when new APIs have a positive unexpected impact on other parts of libcamera, it feels clean and right :-) > > > const std::map<std::string, int32_t> &enumStrMap = {}); > > > > > > unsigned int id() const { return id_; } > > > @@ -245,6 +256,8 @@ public: > > > ControlType type() const { return type_; } > > > bool isArray() const { return size_ > 0; } > > > std::size_t size() const { return size_; } > > > + bool isInput() const { return !!(direction_ & Direction::In); } > > > + bool isOutput() const { return !!(direction_ & Direction::Out); } > > > const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; } > > > > > > private: > > > @@ -255,10 +268,13 @@ private: > > > std::string vendor_; > > > ControlType type_; > > > std::size_t size_; > > > + DirectionFlags direction_; > > > std::map<std::string, int32_t> enumStrMap_; > > > std::map<int32_t, std::string> reverseMap_; > > > }; > > > > > > +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction) > > > + > > > static inline bool operator==(unsigned int lhs, const ControlId &rhs) > > > { > > > return lhs == rhs.id(); > > > @@ -286,9 +302,11 @@ public: > > > using type = T; > > > > > > Control(unsigned int id, const char *name, const char *vendor, > > > + ControlId::DirectionFlags direction = ControlId::Direction::In > > > + | ControlId::Direction::Out, > > > const std::map<std::string, int32_t> &enumStrMap = {}) > > > : ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value, > > > - details::control_type<std::remove_cv_t<T>>::size, enumStrMap) > > > + details::control_type<std::remove_cv_t<T>>::size, direction, enumStrMap) > > > { > > > } > > > > > > diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in > > > index afe9e2c96610..65668d486dbc 100644 > > > --- a/src/libcamera/control_ids.cpp.in > > > +++ b/src/libcamera/control_ids.cpp.in > > > @@ -89,9 +89,9 @@ extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = { > > > { "{{enum.name}}", {{enum.name}} }, > > > {%- endfor %} > > > }; > > > -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.name}}NameValueMap); > > > +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}, {{ctrl.name}}NameValueMap); > > > {% else -%} > > > -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}"); > > > +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}); > > > {% endif -%} > > > {%- endfor %} > > > > > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > > > index 2efecf0fc52b..f221a7e621ab 100644 > > > --- a/src/libcamera/controls.cpp > > > +++ b/src/libcamera/controls.cpp > > > @@ -397,14 +397,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen > > > * \param[in] vendor The vendor name > > > * \param[in] type The control data type > > > * \param[in] size The size of the array control, or 0 if scalar control > > > + * \param[in] direction The direction of the control, if it can be used in Controls or Metadata > > > * \param[in] enumStrMap The map from enum names to values (optional) > > > */ > > > ControlId::ControlId(unsigned int id, const std::string &name, > > > const std::string &vendor, ControlType type, > > > - std::size_t size, > > > + std::size_t size, DirectionFlags direction, > > > const std::map<std::string, int32_t> &enumStrMap) > > > : id_(id), name_(name), vendor_(vendor), type_(type), size_(size), > > > - enumStrMap_(enumStrMap) > > > + direction_(direction), enumStrMap_(enumStrMap) > > > { > > > for (const auto &pair : enumStrMap_) > > > reverseMap_[pair.second] = pair.first; > > > @@ -440,6 +441,26 @@ ControlId::ControlId(unsigned int id, const std::string &name, > > > * \return True if the control is an array control, false otherwise > > > */ > > > > > > +/** > > > + * \fn bool ControlId::isInput() const > > > + * \brief Determine if the control is available to be used as an input control > > > + * > > > + * Controls can be used either as input as a control, or as output in metadata. > > > > Maybe "as input in controls" to match the "in metadata" ? > > > > > + * This function checks if the control is allowed to be used as the former. > > > + * > > > + * \return True if the control can be used as an input control, false otherwise > > > + */ > > > + > > > +/** > > > + * \fn bool ControlId::isOutput() const > > > + * \brief Determine if the control is available to be used in output metadata > > > + * > > > + * Controls can be used either as input as a control, or as output in metadata. > > > + * This function checks if the control is allowed to be used as the latter. > > > + * > > > + * \return True if the control can be returned in output metadata, false otherwise > > > + */ > > > + > > > /** > > > * \fn std::size_t ControlId::size() const > > > * \brief Retrieve the size of the control if it is an array control > > > @@ -471,6 +492,22 @@ ControlId::ControlId(unsigned int id, const std::string &name, > > > * \return True if \a lhs.id() is equal to \a rhs, false otherwise > > > */ > > > > > > +/** > > > + * \enum ControlId::Direction > > > + * \brief The direction that a control of the ControlId is capable of being passed from/to > > > > This sounds a bit weird. > > > > > + * > > > + * \var ControlId::Direction::In > > > + * \brief The control can be passed as input in controls > > > + * > > > + * \var ControlId::Direction::Out > > > + * \brief The control can be returned as output in metadata > > > + */ > > > + > > > +/** > > > + * \typedef ControlId::DirectionFlags > > > + * \brief A wrapper for ControlId::Direction so that it can be used as flags > > > + */ > > > + > > > /** > > > * \class Control > > > * \brief Describe a control and its intrinsic properties > > > @@ -504,6 +541,8 @@ ControlId::ControlId(unsigned int id, const std::string &name, > > > * \param[in] id The control numerical ID > > > * \param[in] name The control name > > > * \param[in] vendor The vendor name > > > + * \param[in] direction The direction of the control, if it can be used in > > > + * Controls or Metadata > > > * \param[in] enumStrMap The map from enum names to values (optional) > > > * > > > * The control data type is automatically deduced from the template type T.
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 3cfe2de5973c..74a566a05db4 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -17,6 +17,7 @@ #include <vector> #include <libcamera/base/class.h> +#include <libcamera/base/flags.h> #include <libcamera/base/span.h> #include <libcamera/geometry.h> @@ -235,8 +236,18 @@ private: class ControlId { public: + enum class Direction { + In = (1 << 0), + Out = (1 << 1), + }; + + using DirectionFlags = Flags<Direction>; + ControlId(unsigned int id, const std::string &name, const std::string &vendor, ControlType type, std::size_t size = 0, + DirectionFlags direction = + static_cast<DirectionFlags>(Direction::In) | + static_cast<DirectionFlags>(Direction::Out), const std::map<std::string, int32_t> &enumStrMap = {}); unsigned int id() const { return id_; } @@ -245,6 +256,8 @@ public: ControlType type() const { return type_; } bool isArray() const { return size_ > 0; } std::size_t size() const { return size_; } + bool isInput() const { return !!(direction_ & Direction::In); } + bool isOutput() const { return !!(direction_ & Direction::Out); } const std::map<int32_t, std::string> &enumerators() const { return reverseMap_; } private: @@ -255,10 +268,13 @@ private: std::string vendor_; ControlType type_; std::size_t size_; + DirectionFlags direction_; std::map<std::string, int32_t> enumStrMap_; std::map<int32_t, std::string> reverseMap_; }; +LIBCAMERA_FLAGS_ENABLE_OPERATORS(ControlId::Direction) + static inline bool operator==(unsigned int lhs, const ControlId &rhs) { return lhs == rhs.id(); @@ -286,9 +302,11 @@ public: using type = T; Control(unsigned int id, const char *name, const char *vendor, + ControlId::DirectionFlags direction = ControlId::Direction::In + | ControlId::Direction::Out, const std::map<std::string, int32_t> &enumStrMap = {}) : ControlId(id, name, vendor, details::control_type<std::remove_cv_t<T>>::value, - details::control_type<std::remove_cv_t<T>>::size, enumStrMap) + details::control_type<std::remove_cv_t<T>>::size, direction, enumStrMap) { } diff --git a/src/libcamera/control_ids.cpp.in b/src/libcamera/control_ids.cpp.in index afe9e2c96610..65668d486dbc 100644 --- a/src/libcamera/control_ids.cpp.in +++ b/src/libcamera/control_ids.cpp.in @@ -89,9 +89,9 @@ extern const std::map<std::string, {{ctrl.type}}> {{ctrl.name}}NameValueMap = { { "{{enum.name}}", {{enum.name}} }, {%- endfor %} }; -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.name}}NameValueMap); +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}, {{ctrl.name}}NameValueMap); {% else -%} -extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}"); +extern const Control<{{ctrl.type}}> {{ctrl.name}}({{ctrl.name|snake_case|upper}}, "{{ctrl.name}}", "{{vendor}}", {{ctrl.direction}}); {% endif -%} {%- endfor %} diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 2efecf0fc52b..f221a7e621ab 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -397,14 +397,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen * \param[in] vendor The vendor name * \param[in] type The control data type * \param[in] size The size of the array control, or 0 if scalar control + * \param[in] direction The direction of the control, if it can be used in Controls or Metadata * \param[in] enumStrMap The map from enum names to values (optional) */ ControlId::ControlId(unsigned int id, const std::string &name, const std::string &vendor, ControlType type, - std::size_t size, + std::size_t size, DirectionFlags direction, const std::map<std::string, int32_t> &enumStrMap) : id_(id), name_(name), vendor_(vendor), type_(type), size_(size), - enumStrMap_(enumStrMap) + direction_(direction), enumStrMap_(enumStrMap) { for (const auto &pair : enumStrMap_) reverseMap_[pair.second] = pair.first; @@ -440,6 +441,26 @@ ControlId::ControlId(unsigned int id, const std::string &name, * \return True if the control is an array control, false otherwise */ +/** + * \fn bool ControlId::isInput() const + * \brief Determine if the control is available to be used as an input control + * + * Controls can be used either as input as a control, or as output in metadata. + * This function checks if the control is allowed to be used as the former. + * + * \return True if the control can be used as an input control, false otherwise + */ + +/** + * \fn bool ControlId::isOutput() const + * \brief Determine if the control is available to be used in output metadata + * + * Controls can be used either as input as a control, or as output in metadata. + * This function checks if the control is allowed to be used as the latter. + * + * \return True if the control can be returned in output metadata, false otherwise + */ + /** * \fn std::size_t ControlId::size() const * \brief Retrieve the size of the control if it is an array control @@ -471,6 +492,22 @@ ControlId::ControlId(unsigned int id, const std::string &name, * \return True if \a lhs.id() is equal to \a rhs, false otherwise */ +/** + * \enum ControlId::Direction + * \brief The direction that a control of the ControlId is capable of being passed from/to + * + * \var ControlId::Direction::In + * \brief The control can be passed as input in controls + * + * \var ControlId::Direction::Out + * \brief The control can be returned as output in metadata + */ + +/** + * \typedef ControlId::DirectionFlags + * \brief A wrapper for ControlId::Direction so that it can be used as flags + */ + /** * \class Control * \brief Describe a control and its intrinsic properties @@ -504,6 +541,8 @@ ControlId::ControlId(unsigned int id, const std::string &name, * \param[in] id The control numerical ID * \param[in] name The control name * \param[in] vendor The vendor name + * \param[in] direction The direction of the control, if it can be used in + * Controls or Metadata * \param[in] enumStrMap The map from enum names to values (optional) * * The control data type is automatically deduced from the template type T.
Add support to ControlId for querying direction information. This allows applications to query whether a ControlId is meant for being set in controls or to be returned in metadata or both. This also has a side effect of properly encoding this information, as previously it was only mentioned losely and inconsistently in the control id definition. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v2: - simplify code --- include/libcamera/controls.h | 20 ++++++++++++++- src/libcamera/control_ids.cpp.in | 4 +-- src/libcamera/controls.cpp | 43 ++++++++++++++++++++++++++++++-- 3 files changed, 62 insertions(+), 5 deletions(-)