| Message ID | 20191108205409.18845-6-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch. On 2019-11-08 22:53:50 +0200, Laurent Pinchart wrote: > In preparation for serialization, index the ControlList by unsigned int. > This will allow deserializing a ControlList without requiring external > information. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/controls.h | 26 +++++++--- > src/libcamera/camera_controls.cpp | 4 +- > src/libcamera/controls.cpp | 59 +++++++++-------------- > src/libcamera/include/camera_controls.h | 2 +- > src/libcamera/include/control_validator.h | 2 +- > src/libcamera/pipeline/uvcvideo.cpp | 4 +- > src/libcamera/pipeline/vimc.cpp | 4 +- > src/libcamera/v4l2_device.cpp | 23 ++++----- > 8 files changed, 60 insertions(+), 64 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 19075858fbba..f9979a466eaa 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -78,12 +78,22 @@ private: > ControlType type_; > }; > > -static inline bool operator==(const ControlId &lhs, const ControlId &rhs) > +static inline bool operator==(unsigned int lhs, const ControlId &rhs) > { > - return lhs.id() == rhs.id(); > + return lhs == rhs.id(); > } > > -static inline bool operator!=(const ControlId &lhs, const ControlId &rhs) > +static inline bool operator!=(unsigned int lhs, const ControlId &rhs) > +{ > + return !(lhs == rhs); > +} > + > +static inline bool operator==(const ControlId &lhs, unsigned int rhs) > +{ > + return lhs.id() == rhs; > +} > + > +static inline bool operator!=(const ControlId &lhs, unsigned int rhs) > { > return !(lhs == rhs); > } > @@ -175,7 +185,7 @@ private: > class ControlList > { > private: > - using ControlListMap = std::unordered_map<const ControlId *, ControlValue>; > + using ControlListMap = std::unordered_map<unsigned int, ControlValue>; > > public: > ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); > @@ -199,7 +209,7 @@ public: > template<typename T> > const T &get(const Control<T> &ctrl) const > { > - const ControlValue *val = find(ctrl); > + const ControlValue *val = find(ctrl.id()); > if (!val) { > static T t(0); > return t; > @@ -211,7 +221,7 @@ public: > template<typename T> > void set(const Control<T> &ctrl, const T &value) > { > - ControlValue *val = find(ctrl); > + ControlValue *val = find(ctrl.id()); > if (!val) > return; > > @@ -222,8 +232,8 @@ public: > void set(unsigned int id, const ControlValue &value); > > private: > - const ControlValue *find(const ControlId &id) const; > - ControlValue *find(const ControlId &id); > + const ControlValue *find(unsigned int id) const; > + ControlValue *find(unsigned int id); > > ControlValidator *validator_; > const ControlIdMap *idmap_; > diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp > index 341da56019f7..59dcede2ca36 100644 > --- a/src/libcamera/camera_controls.cpp > +++ b/src/libcamera/camera_controls.cpp > @@ -44,10 +44,10 @@ const std::string &CameraControlValidator::name() const > * \param[in] id The control ID > * \return True if the control is valid, false otherwise > */ > -bool CameraControlValidator::validate(const ControlId &id) const > +bool CameraControlValidator::validate(unsigned int id) const > { > const ControlInfoMap &controls = camera_->controls(); > - return controls.find(&id) != controls.end(); > + return controls.find(id) != controls.end(); > } > > } /* namespace libcamera */ > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 2c5c98633585..021d5f0990e0 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -259,16 +259,21 @@ bool ControlValue::operator==(const ControlValue &other) const > */ > > /** > - * \fn bool operator==(const ControlId &lhs, const ControlId &rhs) > - * \brief Compare two ControlId instances for equality > - * \param[in] lhs Left-hand side ControlId > + * \fn bool operator==(unsigned int lhs, const ControlId &rhs) > + * \brief Compare a ControlId with a control numerical ID > + * \param[in] lhs Left-hand side numerical ID > * \param[in] rhs Right-hand side ControlId > * > - * ControlId instances are compared based on the numerical ControlId::id() > - * only, as an object may not have two separate controls with the same > - * numerical ID. > + * \return True if \a lhs is equal to \a rhs.id(), false otherwise > + */ > + > +/** > + * \fn bool operator==(const ControlId &lhs, unsigned int rhs) > + * \brief Compare a ControlId with a control numerical ID > + * \param[in] lhs Left-hand side ControlId > + * \param[in] rhs Right-hand side numerical ID > * > - * \return True if \a lhs and \a rhs have equal control IDs, false otherwise > + * \return True if \a lhs.id() is equal to \a rhs, false otherwise > */ > > /** > @@ -655,7 +660,7 @@ ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator > */ > bool ControlList::contains(const ControlId &id) const > { > - return controls_.find(&id) != controls_.end(); > + return controls_.find(id.id()) != controls_.end(); > } > > /** > @@ -666,11 +671,7 @@ bool ControlList::contains(const ControlId &id) const > */ > bool ControlList::contains(unsigned int id) const > { > - const auto iter = idmap_->find(id); > - if (iter == idmap_->end()) > - return false; > - > - return contains(*iter->second); > + return controls_.find(id) != controls_.end(); > } > > /** > @@ -716,15 +717,7 @@ const ControlValue &ControlList::get(unsigned int id) const > { > static ControlValue zero; > > - const auto ctrl = idmap_->find(id); > - if (ctrl == idmap_->end()) { > - LOG(Controls, Error) > - << "Control " << utils::hex(id) > - << " is not supported"; > - return zero; > - } > - > - const ControlValue *val = find(*ctrl->second); > + const ControlValue *val = find(id); > if (!val) > return zero; > > @@ -745,27 +738,19 @@ const ControlValue &ControlList::get(unsigned int id) const > */ > void ControlList::set(unsigned int id, const ControlValue &value) > { > - const auto ctrl = idmap_->find(id); > - if (ctrl == idmap_->end()) { > - LOG(Controls, Error) > - << "Control 0x" << utils::hex(id) > - << " is not supported"; > - return; > - } > - > - ControlValue *val = find(*ctrl->second); > + ControlValue *val = find(id); > if (!val) > return; > > *val = value; > } > > -const ControlValue *ControlList::find(const ControlId &id) const > +const ControlValue *ControlList::find(unsigned int id) const > { > - const auto iter = controls_.find(&id); > + const auto iter = controls_.find(id); > if (iter == controls_.end()) { > LOG(Controls, Error) > - << "Control " << id.name() << " not found"; > + << "Control " << utils::hex(id) << " not found"; > > return nullptr; > } > @@ -773,16 +758,16 @@ const ControlValue *ControlList::find(const ControlId &id) const > return &iter->second; > } > > -ControlValue *ControlList::find(const ControlId &id) > +ControlValue *ControlList::find(unsigned int id) > { > if (validator_ && !validator_->validate(id)) { > LOG(Controls, Error) > - << "Control " << id.name() > + << "Control " << utils::hex(id) > << " is not valid for " << validator_->name(); > return nullptr; > } > > - return &controls_[&id]; > + return &controls_[id]; > } > > } /* namespace libcamera */ > diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h > index 998a2d155a44..265c1fe379db 100644 > --- a/src/libcamera/include/camera_controls.h > +++ b/src/libcamera/include/camera_controls.h > @@ -19,7 +19,7 @@ public: > CameraControlValidator(Camera *camera); > > const std::string &name() const override; > - bool validate(const ControlId &id) const override; > + bool validate(unsigned int id) const override; > > private: > Camera *camera_; > diff --git a/src/libcamera/include/control_validator.h b/src/libcamera/include/control_validator.h > index 3598b18f2f26..f1c9110b158f 100644 > --- a/src/libcamera/include/control_validator.h > +++ b/src/libcamera/include/control_validator.h > @@ -19,7 +19,7 @@ public: > virtual ~ControlValidator() {} > > virtual const std::string &name() const = 0; > - virtual bool validate(const ControlId &id) const = 0; > + virtual bool validate(unsigned int id) const = 0; > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 45448d6f8c05..522229241ffb 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -231,7 +231,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > ControlList controls(data->video_->controls()); > > for (auto it : request->controls()) { > - const ControlId &id = *it.first; > + unsigned int id = it.first; > ControlValue &value = it.second; > > if (id == controls::Brightness) { > @@ -250,7 +250,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > > for (const auto &ctrl : controls) > LOG(UVC, Debug) > - << "Setting control " << ctrl.first->name() > + << "Setting control " << utils::hex(ctrl.first) > << " to " << ctrl.second.toString(); > > int ret = data->video_->setControls(&controls); > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index e6ab6a085824..06664fed42e7 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -298,7 +298,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > ControlList controls(data->sensor_->controls()); > > for (auto it : request->controls()) { > - const ControlId &id = *it.first; > + unsigned int id = it.first; > ControlValue &value = it.second; > > if (id == controls::Brightness) > @@ -311,7 +311,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > > for (const auto &ctrl : controls) > LOG(VIMC, Debug) > - << "Setting control " << ctrl.first->name() > + << "Setting control " << utils::hex(ctrl.first) > << " to " << ctrl.second.toString(); > > int ret = data->sensor_->setControls(&controls); > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index a2b0d891b12f..0452f801029c 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -176,15 +176,15 @@ int V4L2Device::getControls(ControlList *ctrls) > > unsigned int i = 0; > for (const auto &ctrl : *ctrls) { > - const ControlId *id = ctrl.first; > - const auto iter = controls_.find(id->id()); > + unsigned int id = ctrl.first; > + const auto iter = controls_.find(id); > if (iter == controls_.end()) { > LOG(V4L2, Error) > - << "Control '" << id->name() << "' not found"; > + << "Control " << utils::hex(id) << " not found"; > return -EINVAL; > } > > - v4l2Ctrls[i].id = id->id(); > + v4l2Ctrls[i].id = id; > i++; > } > > @@ -250,19 +250,19 @@ int V4L2Device::setControls(ControlList *ctrls) > > unsigned int i = 0; > for (const auto &ctrl : *ctrls) { > - const ControlId *id = ctrl.first; > - const auto iter = controls_.find(id->id()); > + unsigned int id = ctrl.first; > + const auto iter = controls_.find(id); > if (iter == controls_.end()) { > LOG(V4L2, Error) > - << "Control '" << id->name() << "' not found"; > + << "Control " << utils::hex(id) << " not found"; > return -EINVAL; > } > > - v4l2Ctrls[i].id = id->id(); > + v4l2Ctrls[i].id = id; > > /* Set the v4l2_ext_control value for the write operation. */ > const ControlValue &value = ctrl.second; > - switch (id->type()) { > + switch (iter->first->type()) { > case ControlTypeInteger64: > v4l2Ctrls[i].value64 = value.get<int64_t>(); > break; > @@ -403,10 +403,11 @@ void V4L2Device::updateControls(ControlList *ctrls, > break; > > const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; > - const ControlId *id = ctrl.first; > + unsigned int id = ctrl.first; > ControlValue &value = ctrl.second; > > - switch (id->type()) { > + const auto iter = controls_.find(id); > + switch (iter->first->type()) { > case ControlTypeInteger64: > value.set<int64_t>(v4l2Ctrl->value64); > break; > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Laurent, On Fri, Nov 08, 2019 at 10:53:50PM +0200, Laurent Pinchart wrote: > In preparation for serialization, index the ControlList by unsigned int. > This will allow deserializing a ControlList without requiring external > information. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > include/libcamera/controls.h | 26 +++++++--- > src/libcamera/camera_controls.cpp | 4 +- > src/libcamera/controls.cpp | 59 +++++++++-------------- > src/libcamera/include/camera_controls.h | 2 +- > src/libcamera/include/control_validator.h | 2 +- > src/libcamera/pipeline/uvcvideo.cpp | 4 +- > src/libcamera/pipeline/vimc.cpp | 4 +- > src/libcamera/v4l2_device.cpp | 23 ++++----- > 8 files changed, 60 insertions(+), 64 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 19075858fbba..f9979a466eaa 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -78,12 +78,22 @@ private: > ControlType type_; > }; > > -static inline bool operator==(const ControlId &lhs, const ControlId &rhs) > +static inline bool operator==(unsigned int lhs, const ControlId &rhs) > { > - return lhs.id() == rhs.id(); > + return lhs == rhs.id(); > } > > -static inline bool operator!=(const ControlId &lhs, const ControlId &rhs) > +static inline bool operator!=(unsigned int lhs, const ControlId &rhs) > +{ > + return !(lhs == rhs); > +} > + > +static inline bool operator==(const ControlId &lhs, unsigned int rhs) > +{ > + return lhs.id() == rhs; > +} > + > +static inline bool operator!=(const ControlId &lhs, unsigned int rhs) > { > return !(lhs == rhs); > } > @@ -175,7 +185,7 @@ private: > class ControlList > { > private: > - using ControlListMap = std::unordered_map<const ControlId *, ControlValue>; > + using ControlListMap = std::unordered_map<unsigned int, ControlValue>; > > public: > ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); > @@ -199,7 +209,7 @@ public: > template<typename T> > const T &get(const Control<T> &ctrl) const > { > - const ControlValue *val = find(ctrl); > + const ControlValue *val = find(ctrl.id()); > if (!val) { > static T t(0); > return t; > @@ -211,7 +221,7 @@ public: > template<typename T> > void set(const Control<T> &ctrl, const T &value) > { > - ControlValue *val = find(ctrl); > + ControlValue *val = find(ctrl.id()); > if (!val) > return; > > @@ -222,8 +232,8 @@ public: > void set(unsigned int id, const ControlValue &value); > > private: > - const ControlValue *find(const ControlId &id) const; > - ControlValue *find(const ControlId &id); > + const ControlValue *find(unsigned int id) const; > + ControlValue *find(unsigned int id); > > ControlValidator *validator_; > const ControlIdMap *idmap_; > diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp > index 341da56019f7..59dcede2ca36 100644 > --- a/src/libcamera/camera_controls.cpp > +++ b/src/libcamera/camera_controls.cpp > @@ -44,10 +44,10 @@ const std::string &CameraControlValidator::name() const > * \param[in] id The control ID > * \return True if the control is valid, false otherwise > */ > -bool CameraControlValidator::validate(const ControlId &id) const > +bool CameraControlValidator::validate(unsigned int id) const > { > const ControlInfoMap &controls = camera_->controls(); > - return controls.find(&id) != controls.end(); > + return controls.find(id) != controls.end(); > } > > } /* namespace libcamera */ > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 2c5c98633585..021d5f0990e0 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -259,16 +259,21 @@ bool ControlValue::operator==(const ControlValue &other) const > */ > > /** > - * \fn bool operator==(const ControlId &lhs, const ControlId &rhs) > - * \brief Compare two ControlId instances for equality > - * \param[in] lhs Left-hand side ControlId > + * \fn bool operator==(unsigned int lhs, const ControlId &rhs) > + * \brief Compare a ControlId with a control numerical ID > + * \param[in] lhs Left-hand side numerical ID > * \param[in] rhs Right-hand side ControlId > * > - * ControlId instances are compared based on the numerical ControlId::id() > - * only, as an object may not have two separate controls with the same > - * numerical ID. > + * \return True if \a lhs is equal to \a rhs.id(), false otherwise > + */ > + > +/** > + * \fn bool operator==(const ControlId &lhs, unsigned int rhs) > + * \brief Compare a ControlId with a control numerical ID > + * \param[in] lhs Left-hand side ControlId > + * \param[in] rhs Right-hand side numerical ID > * > - * \return True if \a lhs and \a rhs have equal control IDs, false otherwise > + * \return True if \a lhs.id() is equal to \a rhs, false otherwise > */ > > /** > @@ -655,7 +660,7 @@ ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator > */ > bool ControlList::contains(const ControlId &id) const > { > - return controls_.find(&id) != controls_.end(); > + return controls_.find(id.id()) != controls_.end(); > } > > /** > @@ -666,11 +671,7 @@ bool ControlList::contains(const ControlId &id) const > */ > bool ControlList::contains(unsigned int id) const > { > - const auto iter = idmap_->find(id); > - if (iter == idmap_->end()) > - return false; > - > - return contains(*iter->second); > + return controls_.find(id) != controls_.end(); > } > > /** > @@ -716,15 +717,7 @@ const ControlValue &ControlList::get(unsigned int id) const > { > static ControlValue zero; > > - const auto ctrl = idmap_->find(id); > - if (ctrl == idmap_->end()) { > - LOG(Controls, Error) > - << "Control " << utils::hex(id) > - << " is not supported"; > - return zero; > - } > - > - const ControlValue *val = find(*ctrl->second); > + const ControlValue *val = find(id); > if (!val) > return zero; > > @@ -745,27 +738,19 @@ const ControlValue &ControlList::get(unsigned int id) const > */ > void ControlList::set(unsigned int id, const ControlValue &value) > { > - const auto ctrl = idmap_->find(id); > - if (ctrl == idmap_->end()) { > - LOG(Controls, Error) > - << "Control 0x" << utils::hex(id) > - << " is not supported"; > - return; > - } > - > - ControlValue *val = find(*ctrl->second); > + ControlValue *val = find(id); > if (!val) > return; > > *val = value; > } > > -const ControlValue *ControlList::find(const ControlId &id) const > +const ControlValue *ControlList::find(unsigned int id) const > { > - const auto iter = controls_.find(&id); > + const auto iter = controls_.find(id); > if (iter == controls_.end()) { > LOG(Controls, Error) > - << "Control " << id.name() << " not found"; > + << "Control " << utils::hex(id) << " not found"; > > return nullptr; > } > @@ -773,16 +758,16 @@ const ControlValue *ControlList::find(const ControlId &id) const > return &iter->second; > } > > -ControlValue *ControlList::find(const ControlId &id) > +ControlValue *ControlList::find(unsigned int id) > { > if (validator_ && !validator_->validate(id)) { > LOG(Controls, Error) > - << "Control " << id.name() > + << "Control " << utils::hex(id) > << " is not valid for " << validator_->name(); > return nullptr; > } > > - return &controls_[&id]; > + return &controls_[id]; > } > > } /* namespace libcamera */ > diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h > index 998a2d155a44..265c1fe379db 100644 > --- a/src/libcamera/include/camera_controls.h > +++ b/src/libcamera/include/camera_controls.h > @@ -19,7 +19,7 @@ public: > CameraControlValidator(Camera *camera); > > const std::string &name() const override; > - bool validate(const ControlId &id) const override; > + bool validate(unsigned int id) const override; > > private: > Camera *camera_; > diff --git a/src/libcamera/include/control_validator.h b/src/libcamera/include/control_validator.h > index 3598b18f2f26..f1c9110b158f 100644 > --- a/src/libcamera/include/control_validator.h > +++ b/src/libcamera/include/control_validator.h > @@ -19,7 +19,7 @@ public: > virtual ~ControlValidator() {} > > virtual const std::string &name() const = 0; > - virtual bool validate(const ControlId &id) const = 0; > + virtual bool validate(unsigned int id) const = 0; > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 45448d6f8c05..522229241ffb 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -231,7 +231,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > ControlList controls(data->video_->controls()); > > for (auto it : request->controls()) { > - const ControlId &id = *it.first; > + unsigned int id = it.first; > ControlValue &value = it.second; > > if (id == controls::Brightness) { > @@ -250,7 +250,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > > for (const auto &ctrl : controls) > LOG(UVC, Debug) > - << "Setting control " << ctrl.first->name() > + << "Setting control " << utils::hex(ctrl.first) > << " to " << ctrl.second.toString(); > > int ret = data->video_->setControls(&controls); > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index e6ab6a085824..06664fed42e7 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -298,7 +298,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > ControlList controls(data->sensor_->controls()); > > for (auto it : request->controls()) { > - const ControlId &id = *it.first; > + unsigned int id = it.first; > ControlValue &value = it.second; > > if (id == controls::Brightness) > @@ -311,7 +311,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > > for (const auto &ctrl : controls) > LOG(VIMC, Debug) > - << "Setting control " << ctrl.first->name() > + << "Setting control " << utils::hex(ctrl.first) > << " to " << ctrl.second.toString(); > > int ret = data->sensor_->setControls(&controls); > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index a2b0d891b12f..0452f801029c 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -176,15 +176,15 @@ int V4L2Device::getControls(ControlList *ctrls) > > unsigned int i = 0; > for (const auto &ctrl : *ctrls) { > - const ControlId *id = ctrl.first; > - const auto iter = controls_.find(id->id()); > + unsigned int id = ctrl.first; > + const auto iter = controls_.find(id); > if (iter == controls_.end()) { > LOG(V4L2, Error) > - << "Control '" << id->name() << "' not found"; > + << "Control " << utils::hex(id) << " not found"; > return -EINVAL; > } > > - v4l2Ctrls[i].id = id->id(); > + v4l2Ctrls[i].id = id; > i++; > } > > @@ -250,19 +250,19 @@ int V4L2Device::setControls(ControlList *ctrls) > > unsigned int i = 0; > for (const auto &ctrl : *ctrls) { > - const ControlId *id = ctrl.first; > - const auto iter = controls_.find(id->id()); > + unsigned int id = ctrl.first; > + const auto iter = controls_.find(id); > if (iter == controls_.end()) { > LOG(V4L2, Error) > - << "Control '" << id->name() << "' not found"; > + << "Control " << utils::hex(id) << " not found"; > return -EINVAL; > } > > - v4l2Ctrls[i].id = id->id(); > + v4l2Ctrls[i].id = id; > > /* Set the v4l2_ext_control value for the write operation. */ > const ControlValue &value = ctrl.second; > - switch (id->type()) { > + switch (iter->first->type()) { > case ControlTypeInteger64: > v4l2Ctrls[i].value64 = value.get<int64_t>(); > break; > @@ -403,10 +403,11 @@ void V4L2Device::updateControls(ControlList *ctrls, > break; > > const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; > - const ControlId *id = ctrl.first; > + unsigned int id = ctrl.first; > ControlValue &value = ctrl.second; > > - switch (id->type()) { > + const auto iter = controls_.find(id); > + switch (iter->first->type()) { > case ControlTypeInteger64: > value.set<int64_t>(v4l2Ctrl->value64); > break; > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > 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 19075858fbba..f9979a466eaa 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -78,12 +78,22 @@ private: ControlType type_; }; -static inline bool operator==(const ControlId &lhs, const ControlId &rhs) +static inline bool operator==(unsigned int lhs, const ControlId &rhs) { - return lhs.id() == rhs.id(); + return lhs == rhs.id(); } -static inline bool operator!=(const ControlId &lhs, const ControlId &rhs) +static inline bool operator!=(unsigned int lhs, const ControlId &rhs) +{ + return !(lhs == rhs); +} + +static inline bool operator==(const ControlId &lhs, unsigned int rhs) +{ + return lhs.id() == rhs; +} + +static inline bool operator!=(const ControlId &lhs, unsigned int rhs) { return !(lhs == rhs); } @@ -175,7 +185,7 @@ private: class ControlList { private: - using ControlListMap = std::unordered_map<const ControlId *, ControlValue>; + using ControlListMap = std::unordered_map<unsigned int, ControlValue>; public: ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); @@ -199,7 +209,7 @@ public: template<typename T> const T &get(const Control<T> &ctrl) const { - const ControlValue *val = find(ctrl); + const ControlValue *val = find(ctrl.id()); if (!val) { static T t(0); return t; @@ -211,7 +221,7 @@ public: template<typename T> void set(const Control<T> &ctrl, const T &value) { - ControlValue *val = find(ctrl); + ControlValue *val = find(ctrl.id()); if (!val) return; @@ -222,8 +232,8 @@ public: void set(unsigned int id, const ControlValue &value); private: - const ControlValue *find(const ControlId &id) const; - ControlValue *find(const ControlId &id); + const ControlValue *find(unsigned int id) const; + ControlValue *find(unsigned int id); ControlValidator *validator_; const ControlIdMap *idmap_; diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp index 341da56019f7..59dcede2ca36 100644 --- a/src/libcamera/camera_controls.cpp +++ b/src/libcamera/camera_controls.cpp @@ -44,10 +44,10 @@ const std::string &CameraControlValidator::name() const * \param[in] id The control ID * \return True if the control is valid, false otherwise */ -bool CameraControlValidator::validate(const ControlId &id) const +bool CameraControlValidator::validate(unsigned int id) const { const ControlInfoMap &controls = camera_->controls(); - return controls.find(&id) != controls.end(); + return controls.find(id) != controls.end(); } } /* namespace libcamera */ diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 2c5c98633585..021d5f0990e0 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -259,16 +259,21 @@ bool ControlValue::operator==(const ControlValue &other) const */ /** - * \fn bool operator==(const ControlId &lhs, const ControlId &rhs) - * \brief Compare two ControlId instances for equality - * \param[in] lhs Left-hand side ControlId + * \fn bool operator==(unsigned int lhs, const ControlId &rhs) + * \brief Compare a ControlId with a control numerical ID + * \param[in] lhs Left-hand side numerical ID * \param[in] rhs Right-hand side ControlId * - * ControlId instances are compared based on the numerical ControlId::id() - * only, as an object may not have two separate controls with the same - * numerical ID. + * \return True if \a lhs is equal to \a rhs.id(), false otherwise + */ + +/** + * \fn bool operator==(const ControlId &lhs, unsigned int rhs) + * \brief Compare a ControlId with a control numerical ID + * \param[in] lhs Left-hand side ControlId + * \param[in] rhs Right-hand side numerical ID * - * \return True if \a lhs and \a rhs have equal control IDs, false otherwise + * \return True if \a lhs.id() is equal to \a rhs, false otherwise */ /** @@ -655,7 +660,7 @@ ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator */ bool ControlList::contains(const ControlId &id) const { - return controls_.find(&id) != controls_.end(); + return controls_.find(id.id()) != controls_.end(); } /** @@ -666,11 +671,7 @@ bool ControlList::contains(const ControlId &id) const */ bool ControlList::contains(unsigned int id) const { - const auto iter = idmap_->find(id); - if (iter == idmap_->end()) - return false; - - return contains(*iter->second); + return controls_.find(id) != controls_.end(); } /** @@ -716,15 +717,7 @@ const ControlValue &ControlList::get(unsigned int id) const { static ControlValue zero; - const auto ctrl = idmap_->find(id); - if (ctrl == idmap_->end()) { - LOG(Controls, Error) - << "Control " << utils::hex(id) - << " is not supported"; - return zero; - } - - const ControlValue *val = find(*ctrl->second); + const ControlValue *val = find(id); if (!val) return zero; @@ -745,27 +738,19 @@ const ControlValue &ControlList::get(unsigned int id) const */ void ControlList::set(unsigned int id, const ControlValue &value) { - const auto ctrl = idmap_->find(id); - if (ctrl == idmap_->end()) { - LOG(Controls, Error) - << "Control 0x" << utils::hex(id) - << " is not supported"; - return; - } - - ControlValue *val = find(*ctrl->second); + ControlValue *val = find(id); if (!val) return; *val = value; } -const ControlValue *ControlList::find(const ControlId &id) const +const ControlValue *ControlList::find(unsigned int id) const { - const auto iter = controls_.find(&id); + const auto iter = controls_.find(id); if (iter == controls_.end()) { LOG(Controls, Error) - << "Control " << id.name() << " not found"; + << "Control " << utils::hex(id) << " not found"; return nullptr; } @@ -773,16 +758,16 @@ const ControlValue *ControlList::find(const ControlId &id) const return &iter->second; } -ControlValue *ControlList::find(const ControlId &id) +ControlValue *ControlList::find(unsigned int id) { if (validator_ && !validator_->validate(id)) { LOG(Controls, Error) - << "Control " << id.name() + << "Control " << utils::hex(id) << " is not valid for " << validator_->name(); return nullptr; } - return &controls_[&id]; + return &controls_[id]; } } /* namespace libcamera */ diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h index 998a2d155a44..265c1fe379db 100644 --- a/src/libcamera/include/camera_controls.h +++ b/src/libcamera/include/camera_controls.h @@ -19,7 +19,7 @@ public: CameraControlValidator(Camera *camera); const std::string &name() const override; - bool validate(const ControlId &id) const override; + bool validate(unsigned int id) const override; private: Camera *camera_; diff --git a/src/libcamera/include/control_validator.h b/src/libcamera/include/control_validator.h index 3598b18f2f26..f1c9110b158f 100644 --- a/src/libcamera/include/control_validator.h +++ b/src/libcamera/include/control_validator.h @@ -19,7 +19,7 @@ public: virtual ~ControlValidator() {} virtual const std::string &name() const = 0; - virtual bool validate(const ControlId &id) const = 0; + virtual bool validate(unsigned int id) const = 0; }; } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 45448d6f8c05..522229241ffb 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -231,7 +231,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) ControlList controls(data->video_->controls()); for (auto it : request->controls()) { - const ControlId &id = *it.first; + unsigned int id = it.first; ControlValue &value = it.second; if (id == controls::Brightness) { @@ -250,7 +250,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) for (const auto &ctrl : controls) LOG(UVC, Debug) - << "Setting control " << ctrl.first->name() + << "Setting control " << utils::hex(ctrl.first) << " to " << ctrl.second.toString(); int ret = data->video_->setControls(&controls); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index e6ab6a085824..06664fed42e7 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -298,7 +298,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) ControlList controls(data->sensor_->controls()); for (auto it : request->controls()) { - const ControlId &id = *it.first; + unsigned int id = it.first; ControlValue &value = it.second; if (id == controls::Brightness) @@ -311,7 +311,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) for (const auto &ctrl : controls) LOG(VIMC, Debug) - << "Setting control " << ctrl.first->name() + << "Setting control " << utils::hex(ctrl.first) << " to " << ctrl.second.toString(); int ret = data->sensor_->setControls(&controls); diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index a2b0d891b12f..0452f801029c 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -176,15 +176,15 @@ int V4L2Device::getControls(ControlList *ctrls) unsigned int i = 0; for (const auto &ctrl : *ctrls) { - const ControlId *id = ctrl.first; - const auto iter = controls_.find(id->id()); + unsigned int id = ctrl.first; + const auto iter = controls_.find(id); if (iter == controls_.end()) { LOG(V4L2, Error) - << "Control '" << id->name() << "' not found"; + << "Control " << utils::hex(id) << " not found"; return -EINVAL; } - v4l2Ctrls[i].id = id->id(); + v4l2Ctrls[i].id = id; i++; } @@ -250,19 +250,19 @@ int V4L2Device::setControls(ControlList *ctrls) unsigned int i = 0; for (const auto &ctrl : *ctrls) { - const ControlId *id = ctrl.first; - const auto iter = controls_.find(id->id()); + unsigned int id = ctrl.first; + const auto iter = controls_.find(id); if (iter == controls_.end()) { LOG(V4L2, Error) - << "Control '" << id->name() << "' not found"; + << "Control " << utils::hex(id) << " not found"; return -EINVAL; } - v4l2Ctrls[i].id = id->id(); + v4l2Ctrls[i].id = id; /* Set the v4l2_ext_control value for the write operation. */ const ControlValue &value = ctrl.second; - switch (id->type()) { + switch (iter->first->type()) { case ControlTypeInteger64: v4l2Ctrls[i].value64 = value.get<int64_t>(); break; @@ -403,10 +403,11 @@ void V4L2Device::updateControls(ControlList *ctrls, break; const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; - const ControlId *id = ctrl.first; + unsigned int id = ctrl.first; ControlValue &value = ctrl.second; - switch (id->type()) { + const auto iter = controls_.find(id); + switch (iter->first->type()) { case ControlTypeInteger64: value.set<int64_t>(v4l2Ctrl->value64); break;
In preparation for serialization, index the ControlList by unsigned int. This will allow deserializing a ControlList without requiring external information. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/controls.h | 26 +++++++--- src/libcamera/camera_controls.cpp | 4 +- src/libcamera/controls.cpp | 59 +++++++++-------------- src/libcamera/include/camera_controls.h | 2 +- src/libcamera/include/control_validator.h | 2 +- src/libcamera/pipeline/uvcvideo.cpp | 4 +- src/libcamera/pipeline/vimc.cpp | 4 +- src/libcamera/v4l2_device.cpp | 23 ++++----- 8 files changed, 60 insertions(+), 64 deletions(-)