[libcamera-devel] [PATCH v2 07/14] libcamera: controls: Allow read only access to control values
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Sep 4 20:23:25 CEST 2019
Hi Niklas,
Thank you for the patch.
On Fri, Aug 30, 2019 at 01:26:46AM +0200, Niklas Söderlund wrote:
> Allow the control values in a ControlList to be examined from a const
> environment.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/controls.h | 1 +
> src/libcamera/controls.cpp | 34 ++++++++++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index fbb3a62274c64259..eee5ef87b8fd2633 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -124,6 +124,7 @@ public:
> void clear() { controls_.clear(); }
>
> ControlValue &operator[](ControlId id);
> + const ControlValue &operator[](ControlId id) const;
> ControlValue &operator[](const ControlInfo *info) { return controls_[info]; }
>
> void update(const ControlList &list);
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 35861e27401d5241..12cbf778408ee8e7 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -521,6 +521,40 @@ ControlValue &ControlList::operator[](ControlId id)
> return controls_[&iter->second];
> }
>
> +/**
> + * \brief Access a control specified by \a id
s/a control/the control/ to match the non-const version.
> + * \param[in] id The control ID
> + *
> + * This method returns a reference to the control identified by \a id, if it
> + * exists or an empty control if it does not.
s/, if it exists/ if it exists,/
I would actually not use "exist" as thats ambiguous (where does it
exist, in the control list or in the camera ?).
* This method returns a const reference to the control identified by \a id. If
* no such controls is present in the list, a const reference to an empty
* control value is returned.
> + *
> + * The behaviour is undefined if the control \a id is not supported by the
> + * camera that the ControlList refers to.
> + *
> + * \return A reference to the value of the control identified by \a id
s/A reference/A const reference/
> + */
> +const ControlValue &ControlList::operator[](ControlId id) const
> +{
> + const ControlInfoMap &controls = camera_->controls();
> + static ControlValue empty;
> +
> + const auto iter = controls.find(id);
> + if (iter == controls.end()) {
> + LOG(Controls, Error)
> + << "Camera " << camera_->name()
> + << " does not support control " << id;
> + return empty;
> + }
This is common to the non-const version of the function, so I wonder if
it should be moved to a separate private method. It could be a bit
overkill.
> +
> + const auto it = controls_.find(&iter->second);
iter and it isn't a good idea, please use more explicit names.
> + if (it == controls_.end()) {
> + LOG(Controls, Error) << "list does not have control " << id;
> + return empty;
> + }
> +
> + return it->second;
> +}
> +
> /**
> * \fn ControlList::operator[](const ControlInfo *info)
> * \brief Access or insert the control specified by \a info
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list