| Message ID | 20190828011710.32128-7-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On 28/08/2019 17:20, Jacopo Mondi wrote: > 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@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() ? I think this refers to the fact that if you try to have a control in a ControlList which is not supported by the Controls on the Camera then it is not valid. > >> + * >> + * \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 ?? This is a ControlList, which stores a list of controls as controls_. But those Controls are required to be supported by the list of camera_->controls(), as the ControlInfo data is specific to a camera. IOW, you can't generate a controllist from one camera and apply it to another. -- Kieran > >> + >> + 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@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel >> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel
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. + * + * 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 + */ +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; + } + + return it->second; +} + /** * \fn ControlList::operator[](const ControlInfo *info) * \brief Access or insert the control specified by \a info
Allow the control values in a ControlList to be examined from a const environment. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- include/libcamera/controls.h | 1 + src/libcamera/controls.cpp | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+)