| Message ID | 20191013232755.3292-5-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch. On 2019-10-14 02:27:50 +0300, Laurent Pinchart wrote: > To bring the libcamera and V4L2 control info maps closer, index the > latter by ControlId * like the former. As V4L2ControlInfoMap is widely > indexed by V4L2 numerical IDs, add accessors based on numerical IDs. > > This allows complete removal of the ControId pointer from the > V4L2ControlInfo, as the ControId is accessible as the key when iterating > over the map. A handful of users have to be modified to adapt to the > change. > > The controlInfo argument from V4L2Device::updateControls() can also be > removed as it itsn't used anymore. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/v4l2_controls.h | 40 +++++++++------ > src/libcamera/include/v4l2_device.h | 1 - > src/libcamera/pipeline/uvcvideo.cpp | 2 +- > src/libcamera/pipeline/vimc.cpp | 2 +- > src/libcamera/v4l2_controls.cpp | 71 +++++++++++++++++++++------ > src/libcamera/v4l2_device.cpp | 25 +++------- > 6 files changed, 92 insertions(+), 49 deletions(-) > > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h > index 4d7ac1a133c7..ca7217501256 100644 > --- a/src/libcamera/include/v4l2_controls.h > +++ b/src/libcamera/include/v4l2_controls.h > @@ -28,31 +28,41 @@ public: > class V4L2ControlInfo > { > public: > - V4L2ControlInfo(const V4L2ControlId &id, > - const struct v4l2_query_ext_ctrl &ctrl); > + V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl); > > - const ControlId &id() const { return *id_; } > const ControlRange &range() const { return range_; } > > private: > - const V4L2ControlId *id_; > ControlRange range_; > }; > > -class V4L2ControlInfoMap : private std::map<unsigned int, V4L2ControlInfo> > +class V4L2ControlInfoMap : private std::map<const ControlId *, V4L2ControlInfo> > { > public: > - V4L2ControlInfoMap &operator=(std::map<unsigned int, V4L2ControlInfo> &&info); > + V4L2ControlInfoMap &operator=(std::map<const ControlId *, V4L2ControlInfo> &&info); > > - using std::map<unsigned int, V4L2ControlInfo>::begin; > - using std::map<unsigned int, V4L2ControlInfo>::cbegin; > - using std::map<unsigned int, V4L2ControlInfo>::end; > - using std::map<unsigned int, V4L2ControlInfo>::cend; > - using std::map<unsigned int, V4L2ControlInfo>::at; > - using std::map<unsigned int, V4L2ControlInfo>::empty; > - using std::map<unsigned int, V4L2ControlInfo>::size; > - using std::map<unsigned int, V4L2ControlInfo>::count; > - using std::map<unsigned int, V4L2ControlInfo>::find; > + using std::map<const ControlId *, V4L2ControlInfo>::key_type; > + using std::map<const ControlId *, V4L2ControlInfo>::mapped_type; > + using std::map<const ControlId *, V4L2ControlInfo>::value_type; > + using std::map<const ControlId *, V4L2ControlInfo>::size_type; > + using std::map<const ControlId *, V4L2ControlInfo>::iterator; > + using std::map<const ControlId *, V4L2ControlInfo>::const_iterator; > + > + using std::map<const ControlId *, V4L2ControlInfo>::begin; > + using std::map<const ControlId *, V4L2ControlInfo>::cbegin; > + using std::map<const ControlId *, V4L2ControlInfo>::end; > + using std::map<const ControlId *, V4L2ControlInfo>::cend; > + using std::map<const ControlId *, V4L2ControlInfo>::at; > + using std::map<const ControlId *, V4L2ControlInfo>::empty; > + using std::map<const ControlId *, V4L2ControlInfo>::size; > + using std::map<const ControlId *, V4L2ControlInfo>::count; > + using std::map<const ControlId *, V4L2ControlInfo>::find; > + > + mapped_type &at(unsigned int key); > + const mapped_type &at(unsigned int key) const; > + size_type count(unsigned int key) const; > + iterator find(unsigned int key); > + const_iterator find(unsigned int key) const; > > const ControlIdMap &idmap() const { return idmap_; } > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > index 5a5b85827f23..f30b1c2cde34 100644 > --- a/src/libcamera/include/v4l2_device.h > +++ b/src/libcamera/include/v4l2_device.h > @@ -45,7 +45,6 @@ protected: > private: > void listControls(); > void updateControls(ControlList *ctrls, > - const V4L2ControlInfo **controlInfo, > const struct v4l2_ext_control *v4l2Ctrls, > unsigned int count); > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 547ad5ca4e55..4d76b5fd9347 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -340,7 +340,7 @@ int UVCCameraData::init(MediaEntity *entity) > const V4L2ControlInfo &info = ctrl.second; > const ControlId *id; > > - switch (info.id().id()) { > + switch (ctrl.first->id()) { > case V4L2_CID_BRIGHTNESS: > id = &controls::Brightness; > break; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 53d00360eb6e..78c0fe5ae509 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -416,7 +416,7 @@ int VimcCameraData::init(MediaDevice *media) > const V4L2ControlInfo &info = ctrl.second; > const ControlId *id; > > - switch (info.id().id()) { > + switch (ctrl.first->id()) { > case V4L2_CID_BRIGHTNESS: > id = &controls::Brightness; > break; > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp > index 12c4fb271ba5..9a5e4830db91 100644 > --- a/src/libcamera/v4l2_controls.cpp > +++ b/src/libcamera/v4l2_controls.cpp > @@ -122,12 +122,9 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl) > > /** > * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl > - * \param[in] id The V4L2 control ID > * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel > */ > -V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > - const struct v4l2_query_ext_ctrl &ctrl) > - : id_(&id) > +V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) > { > if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64) > range_ = ControlRange(static_cast<int64_t>(ctrl.minimum), > @@ -137,12 +134,6 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > static_cast<int32_t>(ctrl.maximum)); > } > > -/** > - * \fn V4L2ControlInfo::id() > - * \brief Retrieve the control ID > - * \return The V4L2 control ID > - */ > - > /** > * \fn V4L2ControlInfo::range() > * \brief Retrieve the control value range > @@ -151,7 +142,7 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > > /** > * \class V4L2ControlInfoMap > - * \brief A map of control ID to V4L2ControlInfo > + * \brief A map of controlID to V4L2ControlInfo > */ > > /** > @@ -165,17 +156,69 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > * > * \return The populated V4L2ControlInfoMap > */ > -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<unsigned int, V4L2ControlInfo> &&info) > +V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<const ControlId *, V4L2ControlInfo> &&info) > { > - std::map<unsigned int, V4L2ControlInfo>::operator=(std::move(info)); > + std::map<const ControlId *, V4L2ControlInfo>::operator=(std::move(info)); > > idmap_.clear(); > for (const auto &ctrl : *this) > - idmap_[ctrl.first] = &ctrl.second.id(); > + idmap_[ctrl.first->id()] = ctrl.first; > > return *this; > } > > +/** > + * \brief Access specified element by numerical ID > + * \param[in] id The numerical ID > + * \return A reference to the element whose ID is equal to \a id > + */ > +V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) > +{ > + return at(idmap_.at(id)); > +} > + > +/** > + * \brief Access specified element by numerical ID > + * \param[in] id The numerical ID > + * \return A const reference to the element whose ID is equal to \a id > + */ > +const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const > +{ > + return at(idmap_.at(id)); > +} > + > +/** > + * \brief Count the number of elements matching a numerical ID > + * \param[in] id The numerical ID > + * \return The number of elements matching the numerical \a id > + */ > +V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const > +{ > + return count(idmap_.at(id)); > +} > + > +/** > + * \brief Find the element matching a numerical ID > + * \param[in] id The numerical ID > + * \return An iterator pointing to the element matching the numerical \a id, or > + * end() if no such element exists > + */ > +V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id) > +{ > + return find(idmap_.at(id)); > +} > + > +/** > + * \brief Find the element matching a numerical ID > + * \param[in] id The numerical ID > + * \return A const iterator pointing to the element matching the numerical > + * \a id, or end() if no such element exists > + */ > +V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const > +{ > + return find(idmap_.at(id)); > +} > + > /** > * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const > * \brief Retrieve the ControlId map > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 144a60b4fe93..4bb7d5950f3a 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -171,7 +171,6 @@ int V4L2Device::getControls(ControlList *ctrls) > if (count == 0) > return 0; > > - const V4L2ControlInfo *controlInfo[count]; > struct v4l2_ext_control v4l2Ctrls[count]; > memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > > @@ -185,10 +184,7 @@ int V4L2Device::getControls(ControlList *ctrls) > return -EINVAL; > } > > - const V4L2ControlInfo *info = &iter->second; > - controlInfo[i] = info; > v4l2Ctrls[i].id = id->id(); > - > i++; > } > > @@ -215,7 +211,7 @@ int V4L2Device::getControls(ControlList *ctrls) > ret = errorIdx; > } > > - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > + updateControls(ctrls, v4l2Ctrls, count); > > return ret; > } > @@ -249,7 +245,6 @@ int V4L2Device::setControls(ControlList *ctrls) > if (count == 0) > return 0; > > - const V4L2ControlInfo *controlInfo[count]; > struct v4l2_ext_control v4l2Ctrls[count]; > memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > > @@ -263,13 +258,11 @@ int V4L2Device::setControls(ControlList *ctrls) > return -EINVAL; > } > > - const V4L2ControlInfo *info = &iter->second; > - controlInfo[i] = info; > v4l2Ctrls[i].id = id->id(); > > /* Set the v4l2_ext_control value for the write operation. */ > const ControlValue &value = ctrl.second; > - switch (info->id().type()) { > + switch (id->type()) { > case ControlTypeInteger64: > v4l2Ctrls[i].value64 = value.get<int64_t>(); > break; > @@ -308,7 +301,7 @@ int V4L2Device::setControls(ControlList *ctrls) > ret = errorIdx; > } > > - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > + updateControls(ctrls, v4l2Ctrls, count); > > return ret; > } > @@ -349,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > */ > void V4L2Device::listControls() > { > - std::map<unsigned int, V4L2ControlInfo> ctrls; > + std::map<const ControlId *, V4L2ControlInfo> ctrls; > struct v4l2_query_ext_ctrl ctrl = {}; > > /* \todo Add support for menu and compound controls. */ > @@ -388,8 +381,8 @@ void V4L2Device::listControls() > > controlIds_.emplace_back(utils::make_unique<V4L2ControlId>(ctrl)); > ctrls.emplace(std::piecewise_construct, > - std::forward_as_tuple(ctrl.id), > - std::forward_as_tuple(*controlIds_.back().get(), ctrl)); > + std::forward_as_tuple(controlIds_.back().get()), > + std::forward_as_tuple(ctrl)); > } > > controls_ = std::move(ctrls); > @@ -399,12 +392,10 @@ void V4L2Device::listControls() > * \brief Update the value of the first \a count V4L2 controls in \a ctrls using > * values in \a v4l2Ctrls > * \param[inout] ctrls List of V4L2 controls to update > - * \param[in] controlInfo List of V4L2 control information > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > * \param[in] count The number of controls to update > */ > void V4L2Device::updateControls(ControlList *ctrls, > - const V4L2ControlInfo **controlInfo, > const struct v4l2_ext_control *v4l2Ctrls, > unsigned int count) > { > @@ -414,10 +405,10 @@ void V4L2Device::updateControls(ControlList *ctrls, > break; > > const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; > - const V4L2ControlInfo *info = controlInfo[i]; > + const ControlId *id = ctrl.first; > ControlValue &value = ctrl.second; > > - switch (info->id().type()) { > + switch (id->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 Mon, Oct 14, 2019 at 02:27:50AM +0300, Laurent Pinchart wrote: > To bring the libcamera and V4L2 control info maps closer, index the > latter by ControlId * like the former. As V4L2ControlInfoMap is widely > indexed by V4L2 numerical IDs, add accessors based on numerical IDs. > > This allows complete removal of the ControId pointer from the > V4L2ControlInfo, as the ControId is accessible as the key when iterating > over the map. A handful of users have to be modified to adapt to the > change. > > The controlInfo argument from V4L2Device::updateControls() can also be > removed as it itsn't used anymore. Great! I had this in my early serialization work and I really wanted to see the v4l2_device control handling simplified > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/include/v4l2_controls.h | 40 +++++++++------ > src/libcamera/include/v4l2_device.h | 1 - > src/libcamera/pipeline/uvcvideo.cpp | 2 +- > src/libcamera/pipeline/vimc.cpp | 2 +- > src/libcamera/v4l2_controls.cpp | 71 +++++++++++++++++++++------ > src/libcamera/v4l2_device.cpp | 25 +++------- > 6 files changed, 92 insertions(+), 49 deletions(-) > > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h > index 4d7ac1a133c7..ca7217501256 100644 > --- a/src/libcamera/include/v4l2_controls.h > +++ b/src/libcamera/include/v4l2_controls.h > @@ -28,31 +28,41 @@ public: > class V4L2ControlInfo > { > public: > - V4L2ControlInfo(const V4L2ControlId &id, > - const struct v4l2_query_ext_ctrl &ctrl); > + V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl); > > - const ControlId &id() const { return *id_; } > const ControlRange &range() const { return range_; } > > private: > - const V4L2ControlId *id_; > ControlRange range_; > }; > > -class V4L2ControlInfoMap : private std::map<unsigned int, V4L2ControlInfo> > +class V4L2ControlInfoMap : private std::map<const ControlId *, V4L2ControlInfo> > { > public: > - V4L2ControlInfoMap &operator=(std::map<unsigned int, V4L2ControlInfo> &&info); > + V4L2ControlInfoMap &operator=(std::map<const ControlId *, V4L2ControlInfo> &&info); > > - using std::map<unsigned int, V4L2ControlInfo>::begin; > - using std::map<unsigned int, V4L2ControlInfo>::cbegin; > - using std::map<unsigned int, V4L2ControlInfo>::end; > - using std::map<unsigned int, V4L2ControlInfo>::cend; > - using std::map<unsigned int, V4L2ControlInfo>::at; > - using std::map<unsigned int, V4L2ControlInfo>::empty; > - using std::map<unsigned int, V4L2ControlInfo>::size; > - using std::map<unsigned int, V4L2ControlInfo>::count; > - using std::map<unsigned int, V4L2ControlInfo>::find; > + using std::map<const ControlId *, V4L2ControlInfo>::key_type; > + using std::map<const ControlId *, V4L2ControlInfo>::mapped_type; > + using std::map<const ControlId *, V4L2ControlInfo>::value_type; > + using std::map<const ControlId *, V4L2ControlInfo>::size_type; > + using std::map<const ControlId *, V4L2ControlInfo>::iterator; > + using std::map<const ControlId *, V4L2ControlInfo>::const_iterator; > + > + using std::map<const ControlId *, V4L2ControlInfo>::begin; > + using std::map<const ControlId *, V4L2ControlInfo>::cbegin; > + using std::map<const ControlId *, V4L2ControlInfo>::end; > + using std::map<const ControlId *, V4L2ControlInfo>::cend; > + using std::map<const ControlId *, V4L2ControlInfo>::at; > + using std::map<const ControlId *, V4L2ControlInfo>::empty; > + using std::map<const ControlId *, V4L2ControlInfo>::size; > + using std::map<const ControlId *, V4L2ControlInfo>::count; > + using std::map<const ControlId *, V4L2ControlInfo>::find; > + > + mapped_type &at(unsigned int key); > + const mapped_type &at(unsigned int key) const; > + size_type count(unsigned int key) const; > + iterator find(unsigned int key); > + const_iterator find(unsigned int key) const; > > const ControlIdMap &idmap() const { return idmap_; } > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > index 5a5b85827f23..f30b1c2cde34 100644 > --- a/src/libcamera/include/v4l2_device.h > +++ b/src/libcamera/include/v4l2_device.h > @@ -45,7 +45,6 @@ protected: > private: > void listControls(); > void updateControls(ControlList *ctrls, > - const V4L2ControlInfo **controlInfo, > const struct v4l2_ext_control *v4l2Ctrls, > unsigned int count); > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 547ad5ca4e55..4d76b5fd9347 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -340,7 +340,7 @@ int UVCCameraData::init(MediaEntity *entity) > const V4L2ControlInfo &info = ctrl.second; > const ControlId *id; > > - switch (info.id().id()) { > + switch (ctrl.first->id()) { > case V4L2_CID_BRIGHTNESS: > id = &controls::Brightness; > break; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 53d00360eb6e..78c0fe5ae509 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -416,7 +416,7 @@ int VimcCameraData::init(MediaDevice *media) > const V4L2ControlInfo &info = ctrl.second; > const ControlId *id; > > - switch (info.id().id()) { > + switch (ctrl.first->id()) { > case V4L2_CID_BRIGHTNESS: > id = &controls::Brightness; > break; > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp > index 12c4fb271ba5..9a5e4830db91 100644 > --- a/src/libcamera/v4l2_controls.cpp > +++ b/src/libcamera/v4l2_controls.cpp > @@ -122,12 +122,9 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl) > > /** > * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl > - * \param[in] id The V4L2 control ID > * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel > */ > -V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > - const struct v4l2_query_ext_ctrl &ctrl) > - : id_(&id) > +V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) > { > if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64) > range_ = ControlRange(static_cast<int64_t>(ctrl.minimum), > @@ -137,12 +134,6 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > static_cast<int32_t>(ctrl.maximum)); > } > > -/** > - * \fn V4L2ControlInfo::id() > - * \brief Retrieve the control ID > - * \return The V4L2 control ID > - */ > - > /** > * \fn V4L2ControlInfo::range() > * \brief Retrieve the control value range > @@ -151,7 +142,7 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > > /** > * \class V4L2ControlInfoMap > - * \brief A map of control ID to V4L2ControlInfo > + * \brief A map of controlID to V4L2ControlInfo > */ > > /** > @@ -165,17 +156,69 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > * > * \return The populated V4L2ControlInfoMap > */ > -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<unsigned int, V4L2ControlInfo> &&info) > +V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<const ControlId *, V4L2ControlInfo> &&info) > { > - std::map<unsigned int, V4L2ControlInfo>::operator=(std::move(info)); > + std::map<const ControlId *, V4L2ControlInfo>::operator=(std::move(info)); > > idmap_.clear(); > for (const auto &ctrl : *this) > - idmap_[ctrl.first] = &ctrl.second.id(); > + idmap_[ctrl.first->id()] = ctrl.first; > > return *this; > } > > +/** > + * \brief Access specified element by numerical ID > + * \param[in] id The numerical ID > + * \return A reference to the element whose ID is equal to \a id > + */ > +V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) > +{ > + return at(idmap_.at(id)); > +} > + > +/** > + * \brief Access specified element by numerical ID > + * \param[in] id The numerical ID > + * \return A const reference to the element whose ID is equal to \a id > + */ > +const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const > +{ > + return at(idmap_.at(id)); > +} > + > +/** > + * \brief Count the number of elements matching a numerical ID > + * \param[in] id The numerical ID > + * \return The number of elements matching the numerical \a id > + */ > +V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const > +{ > + return count(idmap_.at(id)); > +} Can we have multiple elements indexed by the same id ? > + > +/** > + * \brief Find the element matching a numerical ID > + * \param[in] id The numerical ID > + * \return An iterator pointing to the element matching the numerical \a id, or > + * end() if no such element exists > + */ > +V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id) > +{ > + return find(idmap_.at(id)); > +} > + > +/** > + * \brief Find the element matching a numerical ID > + * \param[in] id The numerical ID > + * \return A const iterator pointing to the element matching the numerical > + * \a id, or end() if no such element exists > + */ > +V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const > +{ > + return find(idmap_.at(id)); > +} > + > /** > * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const > * \brief Retrieve the ControlId map > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 144a60b4fe93..4bb7d5950f3a 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -171,7 +171,6 @@ int V4L2Device::getControls(ControlList *ctrls) > if (count == 0) > return 0; > > - const V4L2ControlInfo *controlInfo[count]; > struct v4l2_ext_control v4l2Ctrls[count]; > memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > > @@ -185,10 +184,7 @@ int V4L2Device::getControls(ControlList *ctrls) > return -EINVAL; > } > > - const V4L2ControlInfo *info = &iter->second; > - controlInfo[i] = info; > v4l2Ctrls[i].id = id->id(); > - > i++; > } > > @@ -215,7 +211,7 @@ int V4L2Device::getControls(ControlList *ctrls) > ret = errorIdx; > } > > - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > + updateControls(ctrls, v4l2Ctrls, count); > > return ret; > } > @@ -249,7 +245,6 @@ int V4L2Device::setControls(ControlList *ctrls) > if (count == 0) > return 0; > > - const V4L2ControlInfo *controlInfo[count]; > struct v4l2_ext_control v4l2Ctrls[count]; > memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > > @@ -263,13 +258,11 @@ int V4L2Device::setControls(ControlList *ctrls) > return -EINVAL; > } > > - const V4L2ControlInfo *info = &iter->second; > - controlInfo[i] = info; > v4l2Ctrls[i].id = id->id(); > > /* Set the v4l2_ext_control value for the write operation. */ > const ControlValue &value = ctrl.second; > - switch (info->id().type()) { > + switch (id->type()) { > case ControlTypeInteger64: > v4l2Ctrls[i].value64 = value.get<int64_t>(); > break; > @@ -308,7 +301,7 @@ int V4L2Device::setControls(ControlList *ctrls) > ret = errorIdx; > } > > - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > + updateControls(ctrls, v4l2Ctrls, count); > > return ret; > } > @@ -349,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > */ > void V4L2Device::listControls() > { > - std::map<unsigned int, V4L2ControlInfo> ctrls; > + std::map<const ControlId *, V4L2ControlInfo> ctrls; > struct v4l2_query_ext_ctrl ctrl = {}; > > /* \todo Add support for menu and compound controls. */ > @@ -388,8 +381,8 @@ void V4L2Device::listControls() > > controlIds_.emplace_back(utils::make_unique<V4L2ControlId>(ctrl)); Not in this patch but I just noticed: doesn't this create a V4L2ControlId while we should instead forward the argument to its constructor ? Well, I think the unique_ptr<> here plays a role, and I cannot suggest a way around it... Anyway, the change is good and desirable: Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > ctrls.emplace(std::piecewise_construct, > - std::forward_as_tuple(ctrl.id), > - std::forward_as_tuple(*controlIds_.back().get(), ctrl)); > + std::forward_as_tuple(controlIds_.back().get()), > + std::forward_as_tuple(ctrl)); > } > > controls_ = std::move(ctrls); > @@ -399,12 +392,10 @@ void V4L2Device::listControls() > * \brief Update the value of the first \a count V4L2 controls in \a ctrls using > * values in \a v4l2Ctrls > * \param[inout] ctrls List of V4L2 controls to update > - * \param[in] controlInfo List of V4L2 control information > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > * \param[in] count The number of controls to update > */ > void V4L2Device::updateControls(ControlList *ctrls, > - const V4L2ControlInfo **controlInfo, > const struct v4l2_ext_control *v4l2Ctrls, > unsigned int count) > { > @@ -414,10 +405,10 @@ void V4L2Device::updateControls(ControlList *ctrls, > break; > > const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; > - const V4L2ControlInfo *info = controlInfo[i]; > + const ControlId *id = ctrl.first; > ControlValue &value = ctrl.second; > > - switch (info->id().type()) { > + switch (id->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 Jacopo, On Tue, Oct 15, 2019 at 05:16:06PM +0200, Jacopo Mondi wrote: > On Mon, Oct 14, 2019 at 02:27:50AM +0300, Laurent Pinchart wrote: > > To bring the libcamera and V4L2 control info maps closer, index the > > latter by ControlId * like the former. As V4L2ControlInfoMap is widely > > indexed by V4L2 numerical IDs, add accessors based on numerical IDs. > > > > This allows complete removal of the ControId pointer from the > > V4L2ControlInfo, as the ControId is accessible as the key when iterating > > over the map. A handful of users have to be modified to adapt to the > > change. > > > > The controlInfo argument from V4L2Device::updateControls() can also be > > removed as it itsn't used anymore. > > Great! I had this in my early serialization work and I really wanted > to see the v4l2_device control handling simplified > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/include/v4l2_controls.h | 40 +++++++++------ > > src/libcamera/include/v4l2_device.h | 1 - > > src/libcamera/pipeline/uvcvideo.cpp | 2 +- > > src/libcamera/pipeline/vimc.cpp | 2 +- > > src/libcamera/v4l2_controls.cpp | 71 +++++++++++++++++++++------ > > src/libcamera/v4l2_device.cpp | 25 +++------- > > 6 files changed, 92 insertions(+), 49 deletions(-) > > > > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h > > index 4d7ac1a133c7..ca7217501256 100644 > > --- a/src/libcamera/include/v4l2_controls.h > > +++ b/src/libcamera/include/v4l2_controls.h > > @@ -28,31 +28,41 @@ public: > > class V4L2ControlInfo > > { > > public: > > - V4L2ControlInfo(const V4L2ControlId &id, > > - const struct v4l2_query_ext_ctrl &ctrl); > > + V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl); > > > > - const ControlId &id() const { return *id_; } > > const ControlRange &range() const { return range_; } > > > > private: > > - const V4L2ControlId *id_; > > ControlRange range_; > > }; > > > > -class V4L2ControlInfoMap : private std::map<unsigned int, V4L2ControlInfo> > > +class V4L2ControlInfoMap : private std::map<const ControlId *, V4L2ControlInfo> > > { > > public: > > - V4L2ControlInfoMap &operator=(std::map<unsigned int, V4L2ControlInfo> &&info); > > + V4L2ControlInfoMap &operator=(std::map<const ControlId *, V4L2ControlInfo> &&info); > > > > - using std::map<unsigned int, V4L2ControlInfo>::begin; > > - using std::map<unsigned int, V4L2ControlInfo>::cbegin; > > - using std::map<unsigned int, V4L2ControlInfo>::end; > > - using std::map<unsigned int, V4L2ControlInfo>::cend; > > - using std::map<unsigned int, V4L2ControlInfo>::at; > > - using std::map<unsigned int, V4L2ControlInfo>::empty; > > - using std::map<unsigned int, V4L2ControlInfo>::size; > > - using std::map<unsigned int, V4L2ControlInfo>::count; > > - using std::map<unsigned int, V4L2ControlInfo>::find; > > + using std::map<const ControlId *, V4L2ControlInfo>::key_type; > > + using std::map<const ControlId *, V4L2ControlInfo>::mapped_type; > > + using std::map<const ControlId *, V4L2ControlInfo>::value_type; > > + using std::map<const ControlId *, V4L2ControlInfo>::size_type; > > + using std::map<const ControlId *, V4L2ControlInfo>::iterator; > > + using std::map<const ControlId *, V4L2ControlInfo>::const_iterator; > > + > > + using std::map<const ControlId *, V4L2ControlInfo>::begin; > > + using std::map<const ControlId *, V4L2ControlInfo>::cbegin; > > + using std::map<const ControlId *, V4L2ControlInfo>::end; > > + using std::map<const ControlId *, V4L2ControlInfo>::cend; > > + using std::map<const ControlId *, V4L2ControlInfo>::at; > > + using std::map<const ControlId *, V4L2ControlInfo>::empty; > > + using std::map<const ControlId *, V4L2ControlInfo>::size; > > + using std::map<const ControlId *, V4L2ControlInfo>::count; > > + using std::map<const ControlId *, V4L2ControlInfo>::find; > > + > > + mapped_type &at(unsigned int key); > > + const mapped_type &at(unsigned int key) const; > > + size_type count(unsigned int key) const; > > + iterator find(unsigned int key); > > + const_iterator find(unsigned int key) const; > > > > const ControlIdMap &idmap() const { return idmap_; } > > > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > > index 5a5b85827f23..f30b1c2cde34 100644 > > --- a/src/libcamera/include/v4l2_device.h > > +++ b/src/libcamera/include/v4l2_device.h > > @@ -45,7 +45,6 @@ protected: > > private: > > void listControls(); > > void updateControls(ControlList *ctrls, > > - const V4L2ControlInfo **controlInfo, > > const struct v4l2_ext_control *v4l2Ctrls, > > unsigned int count); > > > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > > index 547ad5ca4e55..4d76b5fd9347 100644 > > --- a/src/libcamera/pipeline/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo.cpp > > @@ -340,7 +340,7 @@ int UVCCameraData::init(MediaEntity *entity) > > const V4L2ControlInfo &info = ctrl.second; > > const ControlId *id; > > > > - switch (info.id().id()) { > > + switch (ctrl.first->id()) { > > case V4L2_CID_BRIGHTNESS: > > id = &controls::Brightness; > > break; > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > > index 53d00360eb6e..78c0fe5ae509 100644 > > --- a/src/libcamera/pipeline/vimc.cpp > > +++ b/src/libcamera/pipeline/vimc.cpp > > @@ -416,7 +416,7 @@ int VimcCameraData::init(MediaDevice *media) > > const V4L2ControlInfo &info = ctrl.second; > > const ControlId *id; > > > > - switch (info.id().id()) { > > + switch (ctrl.first->id()) { > > case V4L2_CID_BRIGHTNESS: > > id = &controls::Brightness; > > break; > > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp > > index 12c4fb271ba5..9a5e4830db91 100644 > > --- a/src/libcamera/v4l2_controls.cpp > > +++ b/src/libcamera/v4l2_controls.cpp > > @@ -122,12 +122,9 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl) > > > > /** > > * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl > > - * \param[in] id The V4L2 control ID > > * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel > > */ > > -V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > > - const struct v4l2_query_ext_ctrl &ctrl) > > - : id_(&id) > > +V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) > > { > > if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64) > > range_ = ControlRange(static_cast<int64_t>(ctrl.minimum), > > @@ -137,12 +134,6 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > > static_cast<int32_t>(ctrl.maximum)); > > } > > > > -/** > > - * \fn V4L2ControlInfo::id() > > - * \brief Retrieve the control ID > > - * \return The V4L2 control ID > > - */ > > - > > /** > > * \fn V4L2ControlInfo::range() > > * \brief Retrieve the control value range > > @@ -151,7 +142,7 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > > > > /** > > * \class V4L2ControlInfoMap > > - * \brief A map of control ID to V4L2ControlInfo > > + * \brief A map of controlID to V4L2ControlInfo > > */ > > > > /** > > @@ -165,17 +156,69 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, > > * > > * \return The populated V4L2ControlInfoMap > > */ > > -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<unsigned int, V4L2ControlInfo> &&info) > > +V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<const ControlId *, V4L2ControlInfo> &&info) > > { > > - std::map<unsigned int, V4L2ControlInfo>::operator=(std::move(info)); > > + std::map<const ControlId *, V4L2ControlInfo>::operator=(std::move(info)); > > > > idmap_.clear(); > > for (const auto &ctrl : *this) > > - idmap_[ctrl.first] = &ctrl.second.id(); > > + idmap_[ctrl.first->id()] = ctrl.first; > > > > return *this; > > } > > > > +/** > > + * \brief Access specified element by numerical ID > > + * \param[in] id The numerical ID > > + * \return A reference to the element whose ID is equal to \a id > > + */ > > +V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) > > +{ > > + return at(idmap_.at(id)); > > +} > > + > > +/** > > + * \brief Access specified element by numerical ID > > + * \param[in] id The numerical ID > > + * \return A const reference to the element whose ID is equal to \a id > > + */ > > +const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const > > +{ > > + return at(idmap_.at(id)); > > +} > > + > > +/** > > + * \brief Count the number of elements matching a numerical ID > > + * \param[in] id The numerical ID > > + * \return The number of elements matching the numerical \a id > > + */ > > +V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const > > +{ > > + return count(idmap_.at(id)); > > +} > > Can we have multiple elements indexed by the same id ? That would require a V4L2 device to expose multiple controls with the same ID. Highly unlikely given that the kernel API won't support this :-) > > + > > +/** > > + * \brief Find the element matching a numerical ID > > + * \param[in] id The numerical ID > > + * \return An iterator pointing to the element matching the numerical \a id, or > > + * end() if no such element exists > > + */ > > +V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id) > > +{ > > + return find(idmap_.at(id)); > > +} > > + > > +/** > > + * \brief Find the element matching a numerical ID > > + * \param[in] id The numerical ID > > + * \return A const iterator pointing to the element matching the numerical > > + * \a id, or end() if no such element exists > > + */ > > +V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const > > +{ > > + return find(idmap_.at(id)); > > +} > > + > > /** > > * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const > > * \brief Retrieve the ControlId map > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 144a60b4fe93..4bb7d5950f3a 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -171,7 +171,6 @@ int V4L2Device::getControls(ControlList *ctrls) > > if (count == 0) > > return 0; > > > > - const V4L2ControlInfo *controlInfo[count]; > > struct v4l2_ext_control v4l2Ctrls[count]; > > memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > > > > @@ -185,10 +184,7 @@ int V4L2Device::getControls(ControlList *ctrls) > > return -EINVAL; > > } > > > > - const V4L2ControlInfo *info = &iter->second; > > - controlInfo[i] = info; > > v4l2Ctrls[i].id = id->id(); > > - > > i++; > > } > > > > @@ -215,7 +211,7 @@ int V4L2Device::getControls(ControlList *ctrls) > > ret = errorIdx; > > } > > > > - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > > + updateControls(ctrls, v4l2Ctrls, count); > > > > return ret; > > } > > @@ -249,7 +245,6 @@ int V4L2Device::setControls(ControlList *ctrls) > > if (count == 0) > > return 0; > > > > - const V4L2ControlInfo *controlInfo[count]; > > struct v4l2_ext_control v4l2Ctrls[count]; > > memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > > > > @@ -263,13 +258,11 @@ int V4L2Device::setControls(ControlList *ctrls) > > return -EINVAL; > > } > > > > - const V4L2ControlInfo *info = &iter->second; > > - controlInfo[i] = info; > > v4l2Ctrls[i].id = id->id(); > > > > /* Set the v4l2_ext_control value for the write operation. */ > > const ControlValue &value = ctrl.second; > > - switch (info->id().type()) { > > + switch (id->type()) { > > case ControlTypeInteger64: > > v4l2Ctrls[i].value64 = value.get<int64_t>(); > > break; > > @@ -308,7 +301,7 @@ int V4L2Device::setControls(ControlList *ctrls) > > ret = errorIdx; > > } > > > > - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > > + updateControls(ctrls, v4l2Ctrls, count); > > > > return ret; > > } > > @@ -349,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > > */ > > void V4L2Device::listControls() > > { > > - std::map<unsigned int, V4L2ControlInfo> ctrls; > > + std::map<const ControlId *, V4L2ControlInfo> ctrls; > > struct v4l2_query_ext_ctrl ctrl = {}; > > > > /* \todo Add support for menu and compound controls. */ > > @@ -388,8 +381,8 @@ void V4L2Device::listControls() > > > > controlIds_.emplace_back(utils::make_unique<V4L2ControlId>(ctrl)); > > Not in this patch but I just noticed: doesn't this create a > V4L2ControlId while we should instead forward the argument to its > constructor ? Well, I think the unique_ptr<> here plays a role, and I > cannot suggest a way around it... controlIds_ store instance of std::unique_ptr<>, so it doesn't construct V4L2ControlId instances. This should, as far as I can tell, create a single V4L2ControlId. The fact that ControlId copies are now impossible is also a hint that no copy is taking place. > Anyway, the change is good and desirable: > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > ctrls.emplace(std::piecewise_construct, > > - std::forward_as_tuple(ctrl.id), > > - std::forward_as_tuple(*controlIds_.back().get(), ctrl)); > > + std::forward_as_tuple(controlIds_.back().get()), > > + std::forward_as_tuple(ctrl)); > > } > > > > controls_ = std::move(ctrls); > > @@ -399,12 +392,10 @@ void V4L2Device::listControls() > > * \brief Update the value of the first \a count V4L2 controls in \a ctrls using > > * values in \a v4l2Ctrls > > * \param[inout] ctrls List of V4L2 controls to update > > - * \param[in] controlInfo List of V4L2 control information > > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > > * \param[in] count The number of controls to update > > */ > > void V4L2Device::updateControls(ControlList *ctrls, > > - const V4L2ControlInfo **controlInfo, > > const struct v4l2_ext_control *v4l2Ctrls, > > unsigned int count) > > { > > @@ -414,10 +405,10 @@ void V4L2Device::updateControls(ControlList *ctrls, > > break; > > > > const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; > > - const V4L2ControlInfo *info = controlInfo[i]; > > + const ControlId *id = ctrl.first; > > ControlValue &value = ctrl.second; > > > > - switch (info->id().type()) { > > + switch (id->type()) { > > case ControlTypeInteger64: > > value.set<int64_t>(v4l2Ctrl->value64); > > break;
Hi Laurent, On Tue, Oct 15, 2019 at 06:31:18PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > > > +/** > > > + * \brief Count the number of elements matching a numerical ID > > > + * \param[in] id The numerical ID > > > + * \return The number of elements matching the numerical \a id > > > + */ > > > +V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const > > > +{ > > > + return count(idmap_.at(id)); > > > +} > > > > Can we have multiple elements indexed by the same id ? > > That would require a V4L2 device to expose multiple controls with the > same ID. Highly unlikely given that the kernel API won't support this > :-) > That's why I asked :) What does a count() operation does then if not making sure the control is present or not ? Thanks j > > > + > > > +/** > > > + * \brief Find the element matching a numerical ID > > > + * \param[in] id The numerical ID > > > + * \return An iterator pointing to the element matching the numerical \a id, or > > > + * end() if no such element exists > > > + */ > > > +V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id) > > > +{ > > > + return find(idmap_.at(id)); > > > +} > > > + > > > +/** > > > + * \brief Find the element matching a numerical ID > > > + * \param[in] id The numerical ID > > > + * \return A const iterator pointing to the element matching the numerical > > > + * \a id, or end() if no such element exists > > > + */ > > > +V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const > > > +{ > > > + return find(idmap_.at(id)); > > > +} > > > + > > > /** > > > * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const > > > * \brief Retrieve the ControlId map > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > > index 144a60b4fe93..4bb7d5950f3a 100644 > > > --- a/src/libcamera/v4l2_device.cpp > > > +++ b/src/libcamera/v4l2_device.cpp > > > @@ -171,7 +171,6 @@ int V4L2Device::getControls(ControlList *ctrls) > > > if (count == 0) > > > return 0; > > > > > > - const V4L2ControlInfo *controlInfo[count]; > > > struct v4l2_ext_control v4l2Ctrls[count]; > > > memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > > > > > > @@ -185,10 +184,7 @@ int V4L2Device::getControls(ControlList *ctrls) > > > return -EINVAL; > > > } > > > > > > - const V4L2ControlInfo *info = &iter->second; > > > - controlInfo[i] = info; > > > v4l2Ctrls[i].id = id->id(); > > > - > > > i++; > > > } > > > > > > @@ -215,7 +211,7 @@ int V4L2Device::getControls(ControlList *ctrls) > > > ret = errorIdx; > > > } > > > > > > - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > > > + updateControls(ctrls, v4l2Ctrls, count); > > > > > > return ret; > > > } > > > @@ -249,7 +245,6 @@ int V4L2Device::setControls(ControlList *ctrls) > > > if (count == 0) > > > return 0; > > > > > > - const V4L2ControlInfo *controlInfo[count]; > > > struct v4l2_ext_control v4l2Ctrls[count]; > > > memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > > > > > > @@ -263,13 +258,11 @@ int V4L2Device::setControls(ControlList *ctrls) > > > return -EINVAL; > > > } > > > > > > - const V4L2ControlInfo *info = &iter->second; > > > - controlInfo[i] = info; > > > v4l2Ctrls[i].id = id->id(); > > > > > > /* Set the v4l2_ext_control value for the write operation. */ > > > const ControlValue &value = ctrl.second; > > > - switch (info->id().type()) { > > > + switch (id->type()) { > > > case ControlTypeInteger64: > > > v4l2Ctrls[i].value64 = value.get<int64_t>(); > > > break; > > > @@ -308,7 +301,7 @@ int V4L2Device::setControls(ControlList *ctrls) > > > ret = errorIdx; > > > } > > > > > > - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > > > + updateControls(ctrls, v4l2Ctrls, count); > > > > > > return ret; > > > } > > > @@ -349,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > > > */ > > > void V4L2Device::listControls() > > > { > > > - std::map<unsigned int, V4L2ControlInfo> ctrls; > > > + std::map<const ControlId *, V4L2ControlInfo> ctrls; > > > struct v4l2_query_ext_ctrl ctrl = {}; > > > > > > /* \todo Add support for menu and compound controls. */ > > > @@ -388,8 +381,8 @@ void V4L2Device::listControls() > > > > > > controlIds_.emplace_back(utils::make_unique<V4L2ControlId>(ctrl)); > > > > Not in this patch but I just noticed: doesn't this create a > > V4L2ControlId while we should instead forward the argument to its > > constructor ? Well, I think the unique_ptr<> here plays a role, and I > > cannot suggest a way around it... > > controlIds_ store instance of std::unique_ptr<>, so it doesn't construct > V4L2ControlId instances. This should, as far as I can tell, create a > single V4L2ControlId. The fact that ControlId copies are now impossible > is also a hint that no copy is taking place. > > > Anyway, the change is good and desirable: > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > > > ctrls.emplace(std::piecewise_construct, > > > - std::forward_as_tuple(ctrl.id), > > > - std::forward_as_tuple(*controlIds_.back().get(), ctrl)); > > > + std::forward_as_tuple(controlIds_.back().get()), > > > + std::forward_as_tuple(ctrl)); > > > } > > > > > > controls_ = std::move(ctrls); > > > @@ -399,12 +392,10 @@ void V4L2Device::listControls() > > > * \brief Update the value of the first \a count V4L2 controls in \a ctrls using > > > * values in \a v4l2Ctrls > > > * \param[inout] ctrls List of V4L2 controls to update > > > - * \param[in] controlInfo List of V4L2 control information > > > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > > > * \param[in] count The number of controls to update > > > */ > > > void V4L2Device::updateControls(ControlList *ctrls, > > > - const V4L2ControlInfo **controlInfo, > > > const struct v4l2_ext_control *v4l2Ctrls, > > > unsigned int count) > > > { > > > @@ -414,10 +405,10 @@ void V4L2Device::updateControls(ControlList *ctrls, > > > break; > > > > > > const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; > > > - const V4L2ControlInfo *info = controlInfo[i]; > > > + const ControlId *id = ctrl.first; > > > ControlValue &value = ctrl.second; > > > > > > - switch (info->id().type()) { > > > + switch (id->type()) { > > > case ControlTypeInteger64: > > > value.set<int64_t>(v4l2Ctrl->value64); > > > break; > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Tue, Oct 15, 2019 at 07:37:27PM +0200, Jacopo Mondi wrote: > On Tue, Oct 15, 2019 at 06:31:18PM +0300, Laurent Pinchart wrote: > >>> +/** > >>> + * \brief Count the number of elements matching a numerical ID > >>> + * \param[in] id The numerical ID > >>> + * \return The number of elements matching the numerical \a id > >>> + */ > >>> +V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const > >>> +{ > >>> + return count(idmap_.at(id)); > >>> +} > >> > >> Can we have multiple elements indexed by the same id ? > > > > That would require a V4L2 device to expose multiple controls with the > > same ID. Highly unlikely given that the kernel API won't support this > > :-) > > That's why I asked :) What does a count() operation does then if not > making sure the control is present or not ? On a std::map(), count() always returns 0 or 1. > >>> + > >>> +/** > >>> + * \brief Find the element matching a numerical ID > >>> + * \param[in] id The numerical ID > >>> + * \return An iterator pointing to the element matching the numerical \a id, or > >>> + * end() if no such element exists > >>> + */ > >>> +V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id) > >>> +{ > >>> + return find(idmap_.at(id)); > >>> +} > >>> + > >>> +/** > >>> + * \brief Find the element matching a numerical ID > >>> + * \param[in] id The numerical ID > >>> + * \return A const iterator pointing to the element matching the numerical > >>> + * \a id, or end() if no such element exists > >>> + */ > >>> +V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const > >>> +{ > >>> + return find(idmap_.at(id)); > >>> +} > >>> + > >>> /** > >>> * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const > >>> * \brief Retrieve the ControlId map > >>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > >>> index 144a60b4fe93..4bb7d5950f3a 100644 > >>> --- a/src/libcamera/v4l2_device.cpp > >>> +++ b/src/libcamera/v4l2_device.cpp > >>> @@ -171,7 +171,6 @@ int V4L2Device::getControls(ControlList *ctrls) > >>> if (count == 0) > >>> return 0; > >>> > >>> - const V4L2ControlInfo *controlInfo[count]; > >>> struct v4l2_ext_control v4l2Ctrls[count]; > >>> memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > >>> > >>> @@ -185,10 +184,7 @@ int V4L2Device::getControls(ControlList *ctrls) > >>> return -EINVAL; > >>> } > >>> > >>> - const V4L2ControlInfo *info = &iter->second; > >>> - controlInfo[i] = info; > >>> v4l2Ctrls[i].id = id->id(); > >>> - > >>> i++; > >>> } > >>> > >>> @@ -215,7 +211,7 @@ int V4L2Device::getControls(ControlList *ctrls) > >>> ret = errorIdx; > >>> } > >>> > >>> - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > >>> + updateControls(ctrls, v4l2Ctrls, count); > >>> > >>> return ret; > >>> } > >>> @@ -249,7 +245,6 @@ int V4L2Device::setControls(ControlList *ctrls) > >>> if (count == 0) > >>> return 0; > >>> > >>> - const V4L2ControlInfo *controlInfo[count]; > >>> struct v4l2_ext_control v4l2Ctrls[count]; > >>> memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); > >>> > >>> @@ -263,13 +258,11 @@ int V4L2Device::setControls(ControlList *ctrls) > >>> return -EINVAL; > >>> } > >>> > >>> - const V4L2ControlInfo *info = &iter->second; > >>> - controlInfo[i] = info; > >>> v4l2Ctrls[i].id = id->id(); > >>> > >>> /* Set the v4l2_ext_control value for the write operation. */ > >>> const ControlValue &value = ctrl.second; > >>> - switch (info->id().type()) { > >>> + switch (id->type()) { > >>> case ControlTypeInteger64: > >>> v4l2Ctrls[i].value64 = value.get<int64_t>(); > >>> break; > >>> @@ -308,7 +301,7 @@ int V4L2Device::setControls(ControlList *ctrls) > >>> ret = errorIdx; > >>> } > >>> > >>> - updateControls(ctrls, controlInfo, v4l2Ctrls, count); > >>> + updateControls(ctrls, v4l2Ctrls, count); > >>> > >>> return ret; > >>> } > >>> @@ -349,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > >>> */ > >>> void V4L2Device::listControls() > >>> { > >>> - std::map<unsigned int, V4L2ControlInfo> ctrls; > >>> + std::map<const ControlId *, V4L2ControlInfo> ctrls; > >>> struct v4l2_query_ext_ctrl ctrl = {}; > >>> > >>> /* \todo Add support for menu and compound controls. */ > >>> @@ -388,8 +381,8 @@ void V4L2Device::listControls() > >>> > >>> controlIds_.emplace_back(utils::make_unique<V4L2ControlId>(ctrl)); > >> > >> Not in this patch but I just noticed: doesn't this create a > >> V4L2ControlId while we should instead forward the argument to its > >> constructor ? Well, I think the unique_ptr<> here plays a role, and I > >> cannot suggest a way around it... > > > > controlIds_ store instance of std::unique_ptr<>, so it doesn't construct > > V4L2ControlId instances. This should, as far as I can tell, create a > > single V4L2ControlId. The fact that ControlId copies are now impossible > > is also a hint that no copy is taking place. > > > >> Anyway, the change is good and desirable: > >> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > >> > >>> ctrls.emplace(std::piecewise_construct, > >>> - std::forward_as_tuple(ctrl.id), > >>> - std::forward_as_tuple(*controlIds_.back().get(), ctrl)); > >>> + std::forward_as_tuple(controlIds_.back().get()), > >>> + std::forward_as_tuple(ctrl)); > >>> } > >>> > >>> controls_ = std::move(ctrls); > >>> @@ -399,12 +392,10 @@ void V4L2Device::listControls() > >>> * \brief Update the value of the first \a count V4L2 controls in \a ctrls using > >>> * values in \a v4l2Ctrls > >>> * \param[inout] ctrls List of V4L2 controls to update > >>> - * \param[in] controlInfo List of V4L2 control information > >>> * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > >>> * \param[in] count The number of controls to update > >>> */ > >>> void V4L2Device::updateControls(ControlList *ctrls, > >>> - const V4L2ControlInfo **controlInfo, > >>> const struct v4l2_ext_control *v4l2Ctrls, > >>> unsigned int count) > >>> { > >>> @@ -414,10 +405,10 @@ void V4L2Device::updateControls(ControlList *ctrls, > >>> break; > >>> > >>> const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; > >>> - const V4L2ControlInfo *info = controlInfo[i]; > >>> + const ControlId *id = ctrl.first; > >>> ControlValue &value = ctrl.second; > >>> > >>> - switch (info->id().type()) { > >>> + switch (id->type()) { > >>> case ControlTypeInteger64: > >>> value.set<int64_t>(v4l2Ctrl->value64); > >>> break;
diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h index 4d7ac1a133c7..ca7217501256 100644 --- a/src/libcamera/include/v4l2_controls.h +++ b/src/libcamera/include/v4l2_controls.h @@ -28,31 +28,41 @@ public: class V4L2ControlInfo { public: - V4L2ControlInfo(const V4L2ControlId &id, - const struct v4l2_query_ext_ctrl &ctrl); + V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl); - const ControlId &id() const { return *id_; } const ControlRange &range() const { return range_; } private: - const V4L2ControlId *id_; ControlRange range_; }; -class V4L2ControlInfoMap : private std::map<unsigned int, V4L2ControlInfo> +class V4L2ControlInfoMap : private std::map<const ControlId *, V4L2ControlInfo> { public: - V4L2ControlInfoMap &operator=(std::map<unsigned int, V4L2ControlInfo> &&info); + V4L2ControlInfoMap &operator=(std::map<const ControlId *, V4L2ControlInfo> &&info); - using std::map<unsigned int, V4L2ControlInfo>::begin; - using std::map<unsigned int, V4L2ControlInfo>::cbegin; - using std::map<unsigned int, V4L2ControlInfo>::end; - using std::map<unsigned int, V4L2ControlInfo>::cend; - using std::map<unsigned int, V4L2ControlInfo>::at; - using std::map<unsigned int, V4L2ControlInfo>::empty; - using std::map<unsigned int, V4L2ControlInfo>::size; - using std::map<unsigned int, V4L2ControlInfo>::count; - using std::map<unsigned int, V4L2ControlInfo>::find; + using std::map<const ControlId *, V4L2ControlInfo>::key_type; + using std::map<const ControlId *, V4L2ControlInfo>::mapped_type; + using std::map<const ControlId *, V4L2ControlInfo>::value_type; + using std::map<const ControlId *, V4L2ControlInfo>::size_type; + using std::map<const ControlId *, V4L2ControlInfo>::iterator; + using std::map<const ControlId *, V4L2ControlInfo>::const_iterator; + + using std::map<const ControlId *, V4L2ControlInfo>::begin; + using std::map<const ControlId *, V4L2ControlInfo>::cbegin; + using std::map<const ControlId *, V4L2ControlInfo>::end; + using std::map<const ControlId *, V4L2ControlInfo>::cend; + using std::map<const ControlId *, V4L2ControlInfo>::at; + using std::map<const ControlId *, V4L2ControlInfo>::empty; + using std::map<const ControlId *, V4L2ControlInfo>::size; + using std::map<const ControlId *, V4L2ControlInfo>::count; + using std::map<const ControlId *, V4L2ControlInfo>::find; + + mapped_type &at(unsigned int key); + const mapped_type &at(unsigned int key) const; + size_type count(unsigned int key) const; + iterator find(unsigned int key); + const_iterator find(unsigned int key) const; const ControlIdMap &idmap() const { return idmap_; } diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h index 5a5b85827f23..f30b1c2cde34 100644 --- a/src/libcamera/include/v4l2_device.h +++ b/src/libcamera/include/v4l2_device.h @@ -45,7 +45,6 @@ protected: private: void listControls(); void updateControls(ControlList *ctrls, - const V4L2ControlInfo **controlInfo, const struct v4l2_ext_control *v4l2Ctrls, unsigned int count); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 547ad5ca4e55..4d76b5fd9347 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -340,7 +340,7 @@ int UVCCameraData::init(MediaEntity *entity) const V4L2ControlInfo &info = ctrl.second; const ControlId *id; - switch (info.id().id()) { + switch (ctrl.first->id()) { case V4L2_CID_BRIGHTNESS: id = &controls::Brightness; break; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 53d00360eb6e..78c0fe5ae509 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -416,7 +416,7 @@ int VimcCameraData::init(MediaDevice *media) const V4L2ControlInfo &info = ctrl.second; const ControlId *id; - switch (info.id().id()) { + switch (ctrl.first->id()) { case V4L2_CID_BRIGHTNESS: id = &controls::Brightness; break; diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp index 12c4fb271ba5..9a5e4830db91 100644 --- a/src/libcamera/v4l2_controls.cpp +++ b/src/libcamera/v4l2_controls.cpp @@ -122,12 +122,9 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl) /** * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl - * \param[in] id The V4L2 control ID * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the kernel */ -V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, - const struct v4l2_query_ext_ctrl &ctrl) - : id_(&id) +V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl) { if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64) range_ = ControlRange(static_cast<int64_t>(ctrl.minimum), @@ -137,12 +134,6 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, static_cast<int32_t>(ctrl.maximum)); } -/** - * \fn V4L2ControlInfo::id() - * \brief Retrieve the control ID - * \return The V4L2 control ID - */ - /** * \fn V4L2ControlInfo::range() * \brief Retrieve the control value range @@ -151,7 +142,7 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, /** * \class V4L2ControlInfoMap - * \brief A map of control ID to V4L2ControlInfo + * \brief A map of controlID to V4L2ControlInfo */ /** @@ -165,17 +156,69 @@ V4L2ControlInfo::V4L2ControlInfo(const V4L2ControlId &id, * * \return The populated V4L2ControlInfoMap */ -V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<unsigned int, V4L2ControlInfo> &&info) +V4L2ControlInfoMap &V4L2ControlInfoMap::operator=(std::map<const ControlId *, V4L2ControlInfo> &&info) { - std::map<unsigned int, V4L2ControlInfo>::operator=(std::move(info)); + std::map<const ControlId *, V4L2ControlInfo>::operator=(std::move(info)); idmap_.clear(); for (const auto &ctrl : *this) - idmap_[ctrl.first] = &ctrl.second.id(); + idmap_[ctrl.first->id()] = ctrl.first; return *this; } +/** + * \brief Access specified element by numerical ID + * \param[in] id The numerical ID + * \return A reference to the element whose ID is equal to \a id + */ +V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) +{ + return at(idmap_.at(id)); +} + +/** + * \brief Access specified element by numerical ID + * \param[in] id The numerical ID + * \return A const reference to the element whose ID is equal to \a id + */ +const V4L2ControlInfoMap::mapped_type &V4L2ControlInfoMap::at(unsigned int id) const +{ + return at(idmap_.at(id)); +} + +/** + * \brief Count the number of elements matching a numerical ID + * \param[in] id The numerical ID + * \return The number of elements matching the numerical \a id + */ +V4L2ControlInfoMap::size_type V4L2ControlInfoMap::count(unsigned int id) const +{ + return count(idmap_.at(id)); +} + +/** + * \brief Find the element matching a numerical ID + * \param[in] id The numerical ID + * \return An iterator pointing to the element matching the numerical \a id, or + * end() if no such element exists + */ +V4L2ControlInfoMap::iterator V4L2ControlInfoMap::find(unsigned int id) +{ + return find(idmap_.at(id)); +} + +/** + * \brief Find the element matching a numerical ID + * \param[in] id The numerical ID + * \return A const iterator pointing to the element matching the numerical + * \a id, or end() if no such element exists + */ +V4L2ControlInfoMap::const_iterator V4L2ControlInfoMap::find(unsigned int id) const +{ + return find(idmap_.at(id)); +} + /** * \fn const ControlIdMap &V4L2ControlInfoMap::idmap() const * \brief Retrieve the ControlId map diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 144a60b4fe93..4bb7d5950f3a 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -171,7 +171,6 @@ int V4L2Device::getControls(ControlList *ctrls) if (count == 0) return 0; - const V4L2ControlInfo *controlInfo[count]; struct v4l2_ext_control v4l2Ctrls[count]; memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); @@ -185,10 +184,7 @@ int V4L2Device::getControls(ControlList *ctrls) return -EINVAL; } - const V4L2ControlInfo *info = &iter->second; - controlInfo[i] = info; v4l2Ctrls[i].id = id->id(); - i++; } @@ -215,7 +211,7 @@ int V4L2Device::getControls(ControlList *ctrls) ret = errorIdx; } - updateControls(ctrls, controlInfo, v4l2Ctrls, count); + updateControls(ctrls, v4l2Ctrls, count); return ret; } @@ -249,7 +245,6 @@ int V4L2Device::setControls(ControlList *ctrls) if (count == 0) return 0; - const V4L2ControlInfo *controlInfo[count]; struct v4l2_ext_control v4l2Ctrls[count]; memset(v4l2Ctrls, 0, sizeof(v4l2Ctrls)); @@ -263,13 +258,11 @@ int V4L2Device::setControls(ControlList *ctrls) return -EINVAL; } - const V4L2ControlInfo *info = &iter->second; - controlInfo[i] = info; v4l2Ctrls[i].id = id->id(); /* Set the v4l2_ext_control value for the write operation. */ const ControlValue &value = ctrl.second; - switch (info->id().type()) { + switch (id->type()) { case ControlTypeInteger64: v4l2Ctrls[i].value64 = value.get<int64_t>(); break; @@ -308,7 +301,7 @@ int V4L2Device::setControls(ControlList *ctrls) ret = errorIdx; } - updateControls(ctrls, controlInfo, v4l2Ctrls, count); + updateControls(ctrls, v4l2Ctrls, count); return ret; } @@ -349,7 +342,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp) */ void V4L2Device::listControls() { - std::map<unsigned int, V4L2ControlInfo> ctrls; + std::map<const ControlId *, V4L2ControlInfo> ctrls; struct v4l2_query_ext_ctrl ctrl = {}; /* \todo Add support for menu and compound controls. */ @@ -388,8 +381,8 @@ void V4L2Device::listControls() controlIds_.emplace_back(utils::make_unique<V4L2ControlId>(ctrl)); ctrls.emplace(std::piecewise_construct, - std::forward_as_tuple(ctrl.id), - std::forward_as_tuple(*controlIds_.back().get(), ctrl)); + std::forward_as_tuple(controlIds_.back().get()), + std::forward_as_tuple(ctrl)); } controls_ = std::move(ctrls); @@ -399,12 +392,10 @@ void V4L2Device::listControls() * \brief Update the value of the first \a count V4L2 controls in \a ctrls using * values in \a v4l2Ctrls * \param[inout] ctrls List of V4L2 controls to update - * \param[in] controlInfo List of V4L2 control information * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver * \param[in] count The number of controls to update */ void V4L2Device::updateControls(ControlList *ctrls, - const V4L2ControlInfo **controlInfo, const struct v4l2_ext_control *v4l2Ctrls, unsigned int count) { @@ -414,10 +405,10 @@ void V4L2Device::updateControls(ControlList *ctrls, break; const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; - const V4L2ControlInfo *info = controlInfo[i]; + const ControlId *id = ctrl.first; ControlValue &value = ctrl.second; - switch (info->id().type()) { + switch (id->type()) { case ControlTypeInteger64: value.set<int64_t>(v4l2Ctrl->value64); break;
To bring the libcamera and V4L2 control info maps closer, index the latter by ControlId * like the former. As V4L2ControlInfoMap is widely indexed by V4L2 numerical IDs, add accessors based on numerical IDs. This allows complete removal of the ControId pointer from the V4L2ControlInfo, as the ControId is accessible as the key when iterating over the map. A handful of users have to be modified to adapt to the change. The controlInfo argument from V4L2Device::updateControls() can also be removed as it itsn't used anymore. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/include/v4l2_controls.h | 40 +++++++++------ src/libcamera/include/v4l2_device.h | 1 - src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/v4l2_controls.cpp | 71 +++++++++++++++++++++------ src/libcamera/v4l2_device.cpp | 25 +++------- 6 files changed, 92 insertions(+), 49 deletions(-)