[libcamera-devel] [RFC PATCH v2 5/9] libcamera: Implement a ControlList
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jun 24 18:27:15 CEST 2019
Hi Kieran,
On Mon, Jun 24, 2019 at 04:48:05PM +0100, Kieran Bingham wrote:
> On 23/06/2019 16:35, Niklas Söderlund wrote:
> > On 2019-06-21 17:13:57 +0100, Kieran Bingham wrote:
> >> Define a ControlList which wraps the STL std::unordered_map to contain
> >> a list of associative pairs of ControlInfo and Value items.
> >>
> >> A ControlInfo (or represented by its ControlId) together with a Value
> >> provide a control item which can be interpreted by the internal IPA and
> >> PipelineHandler components.
> >>
> >> To pass a set of controls around, a ControlList can be added to a
> >> Request or stored locally to establish a set of default values for later
> >> reuse.
> >>
> >> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> >> ---
> >> include/libcamera/controls.h | 41 +++++++++++
> >> src/libcamera/controls.cpp | 128 +++++++++++++++++++++++++++++++++++
> >> 2 files changed, 169 insertions(+)
> >>
> >> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> >> index 95198d41c4cf..5835b840a31b 100644
> >> --- a/include/libcamera/controls.h
> >> +++ b/include/libcamera/controls.h
> >> @@ -4,6 +4,9 @@
> >> *
> >> * controls.h - Control handling
> >> */
> >> +
> >> +#include <unordered_map>
> >> +
> >> #ifndef __LIBCAMERA_CONTROLS_H__
> >> #define __LIBCAMERA_CONTROLS_H__
> >>
> >> @@ -53,6 +56,44 @@ private:
> >>
> >> std::ostream &operator<<(std::ostream &stream, const ControlInfo &value);
> >>
> >> +class ControlList
> >> +{
> >> +private:
> >> + struct ControlInfoHasher {
> >> + std::size_t operator()(const ControlInfo &ci) const
> >> + {
> >> + return std::hash<int>()(ci.id());
> >> + }
> >> + };
> >> +
> >> + typedef std::unordered_map<ControlInfo, Value, ControlInfoHasher> ControlListMap;
> >
> > Is this needed to go first or can it be moved down to the other private
> > section?
>
> It has to be defined before the usages below.
>
> >> +
> >> +public:
> >> + ControlList();
> >> +
> >> + using iterator = ControlListMap::iterator;
> >> + using const_iterator = ControlListMap::const_iterator;
> >> +
> >> + iterator begin() { return controls_.begin(); }
> >> + iterator end() { return controls_.end(); }
> >> + const_iterator begin() const { return controls_.begin(); }
> >> + const_iterator end() const { return controls_.end(); }
> >> +
> >> + bool contains(ControlId id) const { return controls_.count(id); };
> >
> > Would not return controls_.find(id) != controls_.end() be better?
>
> I thought this was elegant.
>
> Count == 0 = false,
> count > 0 = true...
But it's less efficient as it has to go through the whole list in all
cases.
> >> + bool empty() const { return controls_.empty(); }
> >> + size_t size() const { return controls_.size(); }
> >> + void reset() { controls_.clear(); }
> >> +
> >> + Value &operator[](ControlId id) { return controls_[id]; }
> >> +
> >> + void update(const ControlList &list);
> >> + void update(enum ControlId id, const Value &value);
> >> + void update(const ControlInfo &info, const Value &value);
> >> +
> >> +private:
> >> + ControlListMap controls_;
> >> +};
> >> +
> >> } /* namespace libcamera */
> >>
> >> #endif /* __LIBCAMERA_CONTROLS_H__ */
> >> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> >> index b1be46ddb55e..7c55afffe4ca 100644
> >> --- a/src/libcamera/controls.cpp
> >> +++ b/src/libcamera/controls.cpp
> >> @@ -157,4 +157,132 @@ std::ostream &operator<<(std::ostream &stream, const ControlInfo &info)
> >> return stream << info.toString();
> >> }
> >>
> >> +/**
> >> + * \class ControlList
> >> + * \brief Associates a list of ControlIds with their values.
> >> + *
> >> + * The ControlList stores a pair of ControlInfo and Value classes as an
> >> + * unordered map. Entries can be updated using array syntax such as
> >> + * myControlList[MyControlId] = myValue;
> >> + * or
> >> + * myNewValue = myControlList[MyControlId];
> >
> > I don't think this will update the value.
>
> No - it updates myNewValue. I'll just remove this. I was trying to put
> in that the the [] is the accessor.
>
> > I think you can drop the examples and the last sentence from the
> > documentation.
> >
> > With these issues addressed,
> >
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >
> >> + */
> >> +
> >> +/**
> >> + * \brief Construct a ControlList
> >> + */
> >> +ControlList::ControlList()
> >> +{
> >> +}
> >> +
> >> +/**
> >> + * \typedef ControlList::iterator
> >> + * \brief Iterator for the controls contained within the list.
> >> + */
> >> +
> >> +/**
> >> + * \typedef ControlList::const_iterator
> >> + * \brief Const iterator for the controls contained within the list.
> >> + */
> >> +
> >> +/**
> >> + * \fn iterator ControlList::begin()
> >> + * \brief Retrieve an iterator to the first Control in the list
> >> + * \return An iterator to the first Control in the list
> >> + */
> >> +
> >> +/**
> >> + * \fn iterator ControlList::end()
> >> + * \brief Retrieve an iterator to the next element after the last controls in
> >> + * the instance.
> >> + * \return An iterator to the element following the last control in the instance
> >> + */
> >> +
> >> +/**
> >> + * \fn const_iterator ControlList::begin() const
> >> + * \brief Retrieve a const_iterator to the first Control in the list
> >> + * \return A const_iterator to the first Control in the list
> >> + */
> >> +
> >> +/**
> >> + * \fn const_iterator ControlList::end() const
> >> + * \brief Retrieve a constant iterator pointing to an empty element after the
> >> + * \return A const iterator to the element following the last control in the
> >> + * instance
> >> + */
> >> +
> >> +/**
> >> + * \fn ControlList::contains()
> >> + * \brief Identify if the List contains a control with the specified ControlId
> >> + * \return True if the list contains a matching control, false otherwise
> >> + */
> >> +
> >> +/**
> >> + * \fn ControlList::empty()
> >> + * \brief Identify if the list is empty
> >> + * \return True if the list does not contain any control, false otherwise
> >> + */
> >> +
> >> +/**
> >> + * \fn ControlList::size()
> >> + * \brief Retrieve the number of controls in the list
> >> + * \return The number of Control entries stored in the list
> >> + */
> >> +
> >> +/**
> >> + * \fn ControlList::reset()
> >> + * \brief Removes all controls from the list
> >> + */
> >> +
> >> +/**
> >> + * \fn ControlList::operator[]()
> >> + * \brief Access the control with the given Id
> >> + * \param id The id of the control to access
> >> + * \return The Value of the control with a ControlId of \a Id
> >> + */
> >> +
> >> +/**
> >> + * \brief Update all Control values with the value from the given \a list
> >> + * \param list The list of controls to update or append to this list
> >> + *
> >> + * Update all controls in the ControlList, by the values given by \a list
> >> + * If the list already contains a control of this ID then it will be overwritten
> >> + */
> >> +void ControlList::update(const ControlList &list)
> >> +{
> >> + for (auto it : list) {
> >> + const ControlInfo &info = it.first;
> >> + const Value &value = it.second;
> >> +
> >> + controls_[info] = value;
> >> + }
> >> +}
> >> +
> >> +/**
> >> + * \brief Update a control value in the list
> >> + * \param id The ControlId of the control to update
> >> + * \param value The new value for the updated control
> >> + *
> >> + * Set the Control value in the list to the appropriate value
> >> + * If the list already contains a control of this ID then it will be overwritten
> >> + */
> >> +void ControlList::update(enum ControlId id, const Value &value)
> >> +{
> >> + controls_[id] = value;
> >> +}
> >> +
> >> +/**
> >> + * \brief Update a control value in the list.
> >> + * \param info The ControlInfo of the control to update
> >> + * \param value The new value for the updated control
> >> + *
> >> + * Set the Control value in the list to the appropriate value.
> >> + * If the list already contains a control of the Id matching the ControlInfo
> >> + * then it will be overwritten.
> >> + */
> >> +void ControlList::update(const ControlInfo &info, const Value &value)
> >> +{
> >> + controls_[info] = value;
> >> +}
> >> +
> >> } /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list