[libcamera-devel] [PATCH 06/13] libcamera: controls: Allow read only access to control values

[Jacopo Mondi]

Hi Niklas,

On Wed, Aug 28, 2019 at 03:17:03AM +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 54efef1bb337e945..424fe515fa69f8d8 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -517,6 +517,40 @@ ControlValue &ControlList::operator[](ControlId id)
>  	return controls_[&iter->second];
>  }
>
> +/**
> + * \brief Access a control specified by \a id
> + * \param[in] id The control ID
> + *
> + * This method returns a reference to the control identified by \a id, if it
> + * exsists or an empty control if it does not.

s/exsists/exists

I would rather return nullptr, both here and in the existing version
of operator[], it would save creating the static empty control,
doesn't it?

> + *
> + * The behaviour is undefined if the control \a id is not supported by the
> + * camera that the ControlList refers to.

If the control is not in the InfoMap returned by camera_->controls()
won't iter be == controls.end() ?

> + *
> + * \return A reference to the value of the control identified by \a id
> + */
> +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;
> +	}
> +
> +	const auto it = controls_.find(&iter->second);
> +	if (it == controls_.end()) {
> +		LOG(Controls, Error) << "list does not have control " << id;
> +		return empty;
> +	}

Not on your patch, but why do we keep two lists of controls ??

> +
> +	return it->second;
> +}
> +
>  /**
>   * \fn ControlList::operator[](const ControlInfo *info)
>   * \brief Access or insert the control specified by \a info
> --
> 2.22.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190828/fd21d020/attachment.sig>