| Message ID | 20190620153144.5394-3-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Thu, Jun 20, 2019 at 05:31:40PM +0200, Jacopo Mondi wrote: > Enumerate all the valid controls a device supports at open() time. > A control is valid only if its type is supported. > > Store the control informations in a map inside the device to save s/informations/information/ https://en.wiktionary.org/wiki/informations "Most senses of the word “information” are uncountable. The legal sense, referring to court filings, is one that does form a plural." > querying the control when setting or getting its value from the device > and provide an operation to retrieve information by control ID. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/include/v4l2_device.h | 9 ++++ > src/libcamera/v4l2_device.cpp | 72 +++++++++++++++++++++++++++++ > 2 files changed, 81 insertions(+) > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > index 75f23a05b903..91a72fcecbcc 100644 > --- a/src/libcamera/include/v4l2_device.h > +++ b/src/libcamera/include/v4l2_device.h > @@ -7,18 +7,23 @@ > #ifndef __LIBCAMERA_V4L2_DEVICE_H__ > #define __LIBCAMERA_V4L2_DEVICE_H__ > > +#include <map> > #include <string> > > #include "log.h" > > namespace libcamera { > > +class V4L2ControlInfo; > + > class V4L2Device : protected Loggable > { > public: > void close(); > bool isOpen() const { return fd_ != -1; } > > + const V4L2ControlInfo *getControlInfo(unsigned int id) const; > + > const std::string &deviceNode() const { return deviceNode_; } > > protected: > @@ -32,6 +37,10 @@ protected: > int fd() { return fd_; } > > private: > + void listControls(); > + int validateControlType(const V4L2ControlInfo *info); > + > + std::map<unsigned int, V4L2ControlInfo> controls_; > std::string deviceNode_; > int fd_; > }; > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 99621a724b96..b113ff77e3cf 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -13,6 +13,7 @@ > #include <unistd.h> > > #include "log.h" > +#include "v4l2_controls.h" > > /** > * \file v4l2_device.h > @@ -82,6 +83,8 @@ int V4L2Device::open(unsigned int flags) > > fd_ = ret; > > + listControls(); > + > return 0; > } > > @@ -107,6 +110,21 @@ void V4L2Device::close() > * \return True if the V4L2 device node is open, false otherwise > */ > > +/** > + * \brief Retrieve information about a control > + * \param[in] id The control ID > + * \return A pointer to the V4L2ControlInfo for control \a id, or nullptr > + * if the device doesn't have such a control > + */ > +const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const > +{ > + auto it = controls_.find(id); > + if (it == controls_.end()) > + return nullptr; > + > + return &it->second; > +} > + > /** > * \brief Perform an IOCTL system call on the device node > * \param[in] request The IOCTL request code > @@ -137,4 +155,58 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > * \return The V4L2 device file descriptor, -1 if the device node is not open > */ > > +/* > + * \brief List and store information about all controls supported by the > + * V4L2 device > + */ > +void V4L2Device::listControls() > +{ > + struct v4l2_query_ext_ctrl ctrl = {}; > + > + /* \todo Add support for menu and compound controls. */ > + ctrl.id = V4L2_CTRL_FLAG_NEXT_CTRL; > + while (ioctl(VIDIOC_QUERY_EXT_CTRL, &ctrl) == 0) { > + if (ctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS || > + ctrl.flags & V4L2_CTRL_FLAG_DISABLED) { > + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; > + continue; > + } > + > + V4L2ControlInfo info(ctrl); > + if (validateControlType(&info)) > + continue; > + > + controls_.insert(std::pair<unsigned int, V4L2ControlInfo> > + (ctrl.id, info)); Anything wrong with controls_[ctrl.id] = info; ? > + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; > + } > +} > + > +/* > + * \brief Make sure the control type is supported > + * \param[in] info The V4L2ControlInfo to inspect type of > + * \return 0 on success or a negative error code otherwise > + * \retval -EINVAL The control type is not supported > + */ > +int V4L2Device::validateControlType(const V4L2ControlInfo *info) As this method has a single user, how about inlining it ? V4L2Device::listControls() isn't big. > +{ > + /* \todo Support compound controls. */ > + switch (info->type()) { > + case V4L2_CTRL_TYPE_INTEGER: > + case V4L2_CTRL_TYPE_BOOLEAN: > + case V4L2_CTRL_TYPE_MENU: > + case V4L2_CTRL_TYPE_BUTTON: > + case V4L2_CTRL_TYPE_INTEGER64: > + case V4L2_CTRL_TYPE_BITMASK: > + case V4L2_CTRL_TYPE_INTEGER_MENU: > + break; > + default: > + LOG(V4L2, Error) << "Control type '" << info->type() > + << "' not supported"; > + return -EINVAL; > + } > + > + return 0; > +} > + > } /* namespace libcamera */
Hi Laurent, On Sat, Jun 22, 2019 at 08:16:52PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Thu, Jun 20, 2019 at 05:31:40PM +0200, Jacopo Mondi wrote: > > Enumerate all the valid controls a device supports at open() time. > > A control is valid only if its type is supported. > > > > Store the control informations in a map inside the device to save > > s/informations/information/ > > https://en.wiktionary.org/wiki/informations > > "Most senses of the word “information” are uncountable. The legal sense, > referring to court filings, is one that does form a plural." > > > querying the control when setting or getting its value from the device > > and provide an operation to retrieve information by control ID. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/include/v4l2_device.h | 9 ++++ > > src/libcamera/v4l2_device.cpp | 72 +++++++++++++++++++++++++++++ > > 2 files changed, 81 insertions(+) > > > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > > index 75f23a05b903..91a72fcecbcc 100644 > > --- a/src/libcamera/include/v4l2_device.h > > +++ b/src/libcamera/include/v4l2_device.h > > @@ -7,18 +7,23 @@ > > #ifndef __LIBCAMERA_V4L2_DEVICE_H__ > > #define __LIBCAMERA_V4L2_DEVICE_H__ > > > > +#include <map> > > #include <string> > > > > #include "log.h" > > > > namespace libcamera { > > > > +class V4L2ControlInfo; > > + > > class V4L2Device : protected Loggable > > { > > public: > > void close(); > > bool isOpen() const { return fd_ != -1; } > > > > + const V4L2ControlInfo *getControlInfo(unsigned int id) const; > > + > > const std::string &deviceNode() const { return deviceNode_; } > > > > protected: > > @@ -32,6 +37,10 @@ protected: > > int fd() { return fd_; } > > > > private: > > + void listControls(); > > + int validateControlType(const V4L2ControlInfo *info); > > + > > + std::map<unsigned int, V4L2ControlInfo> controls_; > > std::string deviceNode_; > > int fd_; > > }; > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 99621a724b96..b113ff77e3cf 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -13,6 +13,7 @@ > > #include <unistd.h> > > > > #include "log.h" > > +#include "v4l2_controls.h" > > > > /** > > * \file v4l2_device.h > > @@ -82,6 +83,8 @@ int V4L2Device::open(unsigned int flags) > > > > fd_ = ret; > > > > + listControls(); > > + > > return 0; > > } > > > > @@ -107,6 +110,21 @@ void V4L2Device::close() > > * \return True if the V4L2 device node is open, false otherwise > > */ > > > > +/** > > + * \brief Retrieve information about a control > > + * \param[in] id The control ID > > + * \return A pointer to the V4L2ControlInfo for control \a id, or nullptr > > + * if the device doesn't have such a control > > + */ > > +const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const > > +{ > > + auto it = controls_.find(id); > > + if (it == controls_.end()) > > + return nullptr; > > + > > + return &it->second; > > +} > > + > > /** > > * \brief Perform an IOCTL system call on the device node > > * \param[in] request The IOCTL request code > > @@ -137,4 +155,58 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > > * \return The V4L2 device file descriptor, -1 if the device node is not open > > */ > > > > +/* > > + * \brief List and store information about all controls supported by the > > + * V4L2 device > > + */ > > +void V4L2Device::listControls() > > +{ > > + struct v4l2_query_ext_ctrl ctrl = {}; > > + > > + /* \todo Add support for menu and compound controls. */ > > + ctrl.id = V4L2_CTRL_FLAG_NEXT_CTRL; > > + while (ioctl(VIDIOC_QUERY_EXT_CTRL, &ctrl) == 0) { > > + if (ctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS || > > + ctrl.flags & V4L2_CTRL_FLAG_DISABLED) { > > + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; > > + continue; > > + } > > + > > + V4L2ControlInfo info(ctrl); > > + if (validateControlType(&info)) > > + continue; > > + > > + controls_.insert(std::pair<unsigned int, V4L2ControlInfo> > > + (ctrl.id, info)); > > Anything wrong with > > controls_[ctrl.id] = info; > > ? No, I could change it. I used std::vector::insert when I was constructing the V4L2ControlInfo directly in the operation call, it's a leftover. > > > + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; > > + } > > +} > > + > > +/* > > + * \brief Make sure the control type is supported > > + * \param[in] info The V4L2ControlInfo to inspect type of > > + * \return 0 on success or a negative error code otherwise > > + * \retval -EINVAL The control type is not supported > > + */ > > +int V4L2Device::validateControlType(const V4L2ControlInfo *info) > > As this method has a single user, how about inlining it ? > V4L2Device::listControls() isn't big. > I would rather keep the type validation centralized in one place, and in the caller it's easy enough to parse if (validateControlType(type)) for a reader than going through the switch cases. The cost of a function call is negligible considering this happens only at device open time. Thanks j > > +{ > > + /* \todo Support compound controls. */ > > + switch (info->type()) { > > + case V4L2_CTRL_TYPE_INTEGER: > > + case V4L2_CTRL_TYPE_BOOLEAN: > > + case V4L2_CTRL_TYPE_MENU: > > + case V4L2_CTRL_TYPE_BUTTON: > > + case V4L2_CTRL_TYPE_INTEGER64: > > + case V4L2_CTRL_TYPE_BITMASK: > > + case V4L2_CTRL_TYPE_INTEGER_MENU: > > + break; > > + default: > > + LOG(V4L2, Error) << "Control type '" << info->type() > > + << "' not supported"; > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > } /* namespace libcamera */ > > -- > Regards, > > Laurent Pinchart
On Mon, Jun 24, 2019 at 10:19:17AM +0200, Jacopo Mondi wrote: > Hi Laurent, > > On Sat, Jun 22, 2019 at 08:16:52PM +0300, Laurent Pinchart wrote: > > Hi Jacopo, > > > > Thank you for the patch. > > > > On Thu, Jun 20, 2019 at 05:31:40PM +0200, Jacopo Mondi wrote: > > > Enumerate all the valid controls a device supports at open() time. > > > A control is valid only if its type is supported. > > > > > > Store the control informations in a map inside the device to save > > > > s/informations/information/ > > > > https://en.wiktionary.org/wiki/informations > > > > "Most senses of the word “information” are uncountable. The legal sense, > > referring to court filings, is one that does form a plural." > > > > > querying the control when setting or getting its value from the device > > > and provide an operation to retrieve information by control ID. > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > src/libcamera/include/v4l2_device.h | 9 ++++ > > > src/libcamera/v4l2_device.cpp | 72 +++++++++++++++++++++++++++++ > > > 2 files changed, 81 insertions(+) > > > > > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > > > index 75f23a05b903..91a72fcecbcc 100644 > > > --- a/src/libcamera/include/v4l2_device.h > > > +++ b/src/libcamera/include/v4l2_device.h > > > @@ -7,18 +7,23 @@ > > > #ifndef __LIBCAMERA_V4L2_DEVICE_H__ > > > #define __LIBCAMERA_V4L2_DEVICE_H__ > > > > > > +#include <map> > > > #include <string> > > > > > > #include "log.h" > > > > > > namespace libcamera { > > > > > > +class V4L2ControlInfo; > > > + > > > class V4L2Device : protected Loggable > > > { > > > public: > > > void close(); > > > bool isOpen() const { return fd_ != -1; } > > > > > > + const V4L2ControlInfo *getControlInfo(unsigned int id) const; > > > + > > > const std::string &deviceNode() const { return deviceNode_; } > > > > > > protected: > > > @@ -32,6 +37,10 @@ protected: > > > int fd() { return fd_; } > > > > > > private: > > > + void listControls(); > > > + int validateControlType(const V4L2ControlInfo *info); > > > + > > > + std::map<unsigned int, V4L2ControlInfo> controls_; > > > std::string deviceNode_; > > > int fd_; > > > }; > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > > index 99621a724b96..b113ff77e3cf 100644 > > > --- a/src/libcamera/v4l2_device.cpp > > > +++ b/src/libcamera/v4l2_device.cpp > > > @@ -13,6 +13,7 @@ > > > #include <unistd.h> > > > > > > #include "log.h" > > > +#include "v4l2_controls.h" > > > > > > /** > > > * \file v4l2_device.h > > > @@ -82,6 +83,8 @@ int V4L2Device::open(unsigned int flags) > > > > > > fd_ = ret; > > > > > > + listControls(); > > > + > > > return 0; > > > } > > > > > > @@ -107,6 +110,21 @@ void V4L2Device::close() > > > * \return True if the V4L2 device node is open, false otherwise > > > */ > > > > > > +/** > > > + * \brief Retrieve information about a control > > > + * \param[in] id The control ID > > > + * \return A pointer to the V4L2ControlInfo for control \a id, or nullptr > > > + * if the device doesn't have such a control > > > + */ > > > +const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const > > > +{ > > > + auto it = controls_.find(id); > > > + if (it == controls_.end()) > > > + return nullptr; > > > + > > > + return &it->second; > > > +} > > > + > > > /** > > > * \brief Perform an IOCTL system call on the device node > > > * \param[in] request The IOCTL request code > > > @@ -137,4 +155,58 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > > > * \return The V4L2 device file descriptor, -1 if the device node is not open > > > */ > > > > > > +/* > > > + * \brief List and store information about all controls supported by the > > > + * V4L2 device > > > + */ > > > +void V4L2Device::listControls() > > > +{ > > > + struct v4l2_query_ext_ctrl ctrl = {}; > > > + > > > + /* \todo Add support for menu and compound controls. */ > > > + ctrl.id = V4L2_CTRL_FLAG_NEXT_CTRL; > > > + while (ioctl(VIDIOC_QUERY_EXT_CTRL, &ctrl) == 0) { > > > + if (ctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS || > > > + ctrl.flags & V4L2_CTRL_FLAG_DISABLED) { > > > + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; > > > + continue; > > > + } > > > + > > > + V4L2ControlInfo info(ctrl); > > > + if (validateControlType(&info)) > > > + continue; > > > + > > > + controls_.insert(std::pair<unsigned int, V4L2ControlInfo> > > > + (ctrl.id, info)); > > > > Anything wrong with > > > > controls_[ctrl.id] = info; > > > > ? > > No, I could change it. I used std::vector::insert when I was > constructing the V4L2ControlInfo directly in the operation call, it's > a leftover. Adtually, I'm a bit puzzled here. My understanding is that controls_[ctrl.id] = info; should insert a new element by calling the copy constructor of V4L2ControlInfo (which is defaulted and accessible) Howevere, I get this back from the compiler /usr/include/c++/8.3.0/tuple:1668:70: error: no matching function for call to ‘libcamera::V4L2ControlInfo::V4L2ControlInfo()’ second(std::forward<_Args2>(std::get<_Indexes2>(__tuple2))...) ^ In file included from ../src/libcamera/v4l2_device.cpp:16: ../src/libcamera/include/v4l2_controls.h:25:2: note: candidate: ‘libcamera::V4L2ControlInfo::V4L2ControlInfo(const libcamera::V4L2ControlInfo&)’ V4L2ControlInfo(const V4L2ControlInfo &) = default; ^~~~~~~~~~~~~~~ ../src/libcamera/include/v4l2_controls.h:25:2: note: candidate expects 1 argument, 0 provided ../src/libcamera/include/v4l2_controls.h:24:2: note: candidate: ‘libcamera::V4L2ControlInfo::V4L2ControlInfo(const v4l2_query_ext_ctrl&)’ V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl); ^~~~~~~~~~~~~~~ ../src/libcamera/include/v4l2_controls.h:24:2: note: candidate expects 1 argument, 0 provided I'm now keeping the explicit ::insert() call for now. Thanks j > > > > > > + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; > > > + } > > > +} > > > + > > > +/* > > > + * \brief Make sure the control type is supported > > > + * \param[in] info The V4L2ControlInfo to inspect type of > > > + * \return 0 on success or a negative error code otherwise > > > + * \retval -EINVAL The control type is not supported > > > + */ > > > +int V4L2Device::validateControlType(const V4L2ControlInfo *info) > > > > As this method has a single user, how about inlining it ? > > V4L2Device::listControls() isn't big. > > > > I would rather keep the type validation centralized in one place, and in > the caller it's easy enough to parse if (validateControlType(type)) > for a reader than going through the switch cases. > > The cost of a function call is negligible considering this happens > only at device open time. > > Thanks > j > > > > +{ > > > + /* \todo Support compound controls. */ > > > + switch (info->type()) { > > > + case V4L2_CTRL_TYPE_INTEGER: > > > + case V4L2_CTRL_TYPE_BOOLEAN: > > > + case V4L2_CTRL_TYPE_MENU: > > > + case V4L2_CTRL_TYPE_BUTTON: > > > + case V4L2_CTRL_TYPE_INTEGER64: > > > + case V4L2_CTRL_TYPE_BITMASK: > > > + case V4L2_CTRL_TYPE_INTEGER_MENU: > > > + break; > > > + default: > > > + LOG(V4L2, Error) << "Control type '" << info->type() > > > + << "' not supported"; > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > } /* namespace libcamera */ > > > > -- > > Regards, > > > > Laurent Pinchart > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Mon, Jun 24, 2019 at 11:26:12AM +0200, Jacopo Mondi wrote: > On Mon, Jun 24, 2019 at 10:19:17AM +0200, Jacopo Mondi wrote: > > On Sat, Jun 22, 2019 at 08:16:52PM +0300, Laurent Pinchart wrote: > >> On Thu, Jun 20, 2019 at 05:31:40PM +0200, Jacopo Mondi wrote: > >>> Enumerate all the valid controls a device supports at open() time. > >>> A control is valid only if its type is supported. > >>> > >>> Store the control informations in a map inside the device to save > >> > >> s/informations/information/ > >> > >> https://en.wiktionary.org/wiki/informations > >> > >> "Most senses of the word “information” are uncountable. The legal sense, > >> referring to court filings, is one that does form a plural." > >> > >>> querying the control when setting or getting its value from the device > >>> and provide an operation to retrieve information by control ID. > >>> > >>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > >>> --- > >>> src/libcamera/include/v4l2_device.h | 9 ++++ > >>> src/libcamera/v4l2_device.cpp | 72 +++++++++++++++++++++++++++++ > >>> 2 files changed, 81 insertions(+) > >>> > >>> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > >>> index 75f23a05b903..91a72fcecbcc 100644 > >>> --- a/src/libcamera/include/v4l2_device.h > >>> +++ b/src/libcamera/include/v4l2_device.h > >>> @@ -7,18 +7,23 @@ > >>> #ifndef __LIBCAMERA_V4L2_DEVICE_H__ > >>> #define __LIBCAMERA_V4L2_DEVICE_H__ > >>> > >>> +#include <map> > >>> #include <string> > >>> > >>> #include "log.h" > >>> > >>> namespace libcamera { > >>> > >>> +class V4L2ControlInfo; > >>> + > >>> class V4L2Device : protected Loggable > >>> { > >>> public: > >>> void close(); > >>> bool isOpen() const { return fd_ != -1; } > >>> > >>> + const V4L2ControlInfo *getControlInfo(unsigned int id) const; > >>> + > >>> const std::string &deviceNode() const { return deviceNode_; } > >>> > >>> protected: > >>> @@ -32,6 +37,10 @@ protected: > >>> int fd() { return fd_; } > >>> > >>> private: > >>> + void listControls(); > >>> + int validateControlType(const V4L2ControlInfo *info); > >>> + > >>> + std::map<unsigned int, V4L2ControlInfo> controls_; > >>> std::string deviceNode_; > >>> int fd_; > >>> }; > >>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > >>> index 99621a724b96..b113ff77e3cf 100644 > >>> --- a/src/libcamera/v4l2_device.cpp > >>> +++ b/src/libcamera/v4l2_device.cpp > >>> @@ -13,6 +13,7 @@ > >>> #include <unistd.h> > >>> > >>> #include "log.h" > >>> +#include "v4l2_controls.h" > >>> > >>> /** > >>> * \file v4l2_device.h > >>> @@ -82,6 +83,8 @@ int V4L2Device::open(unsigned int flags) > >>> > >>> fd_ = ret; > >>> > >>> + listControls(); > >>> + > >>> return 0; > >>> } > >>> > >>> @@ -107,6 +110,21 @@ void V4L2Device::close() > >>> * \return True if the V4L2 device node is open, false otherwise > >>> */ > >>> > >>> +/** > >>> + * \brief Retrieve information about a control > >>> + * \param[in] id The control ID > >>> + * \return A pointer to the V4L2ControlInfo for control \a id, or nullptr > >>> + * if the device doesn't have such a control > >>> + */ > >>> +const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const > >>> +{ > >>> + auto it = controls_.find(id); > >>> + if (it == controls_.end()) > >>> + return nullptr; > >>> + > >>> + return &it->second; > >>> +} > >>> + > >>> /** > >>> * \brief Perform an IOCTL system call on the device node > >>> * \param[in] request The IOCTL request code > >>> @@ -137,4 +155,58 @@ int V4L2Device::ioctl(unsigned long request, void *argp) > >>> * \return The V4L2 device file descriptor, -1 if the device node is not open > >>> */ > >>> > >>> +/* > >>> + * \brief List and store information about all controls supported by the > >>> + * V4L2 device > >>> + */ > >>> +void V4L2Device::listControls() > >>> +{ > >>> + struct v4l2_query_ext_ctrl ctrl = {}; > >>> + > >>> + /* \todo Add support for menu and compound controls. */ > >>> + ctrl.id = V4L2_CTRL_FLAG_NEXT_CTRL; > >>> + while (ioctl(VIDIOC_QUERY_EXT_CTRL, &ctrl) == 0) { > >>> + if (ctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS || > >>> + ctrl.flags & V4L2_CTRL_FLAG_DISABLED) { > >>> + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; > >>> + continue; > >>> + } > >>> + > >>> + V4L2ControlInfo info(ctrl); > >>> + if (validateControlType(&info)) > >>> + continue; > >>> + > >>> + controls_.insert(std::pair<unsigned int, V4L2ControlInfo> > >>> + (ctrl.id, info)); > >> > >> Anything wrong with > >> > >> controls_[ctrl.id] = info; > >> > >> ? > > > > No, I could change it. I used std::vector::insert when I was > > constructing the V4L2ControlInfo directly in the operation call, it's > > a leftover. > > Adtually, I'm a bit puzzled here. My understanding is that > controls_[ctrl.id] = info; > > should insert a new element by calling the copy constructor of > V4L2ControlInfo (which is defaulted and accessible) > > Howevere, I get this back from the compiler > > /usr/include/c++/8.3.0/tuple:1668:70: error: no matching function for call to ‘libcamera::V4L2ControlInfo::V4L2ControlInfo()’ > second(std::forward<_Args2>(std::get<_Indexes2>(__tuple2))...) > ^ > In file included from ../src/libcamera/v4l2_device.cpp:16: > ../src/libcamera/include/v4l2_controls.h:25:2: note: candidate: ‘libcamera::V4L2ControlInfo::V4L2ControlInfo(const libcamera::V4L2ControlInfo&)’ > V4L2ControlInfo(const V4L2ControlInfo &) = default; > ^~~~~~~~~~~~~~~ > ../src/libcamera/include/v4l2_controls.h:25:2: note: candidate expects 1 argument, 0 provided > ../src/libcamera/include/v4l2_controls.h:24:2: note: candidate: ‘libcamera::V4L2ControlInfo::V4L2ControlInfo(const v4l2_query_ext_ctrl&)’ > V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl); > ^~~~~~~~~~~~~~~ > ../src/libcamera/include/v4l2_controls.h:24:2: note: candidate expects 1 argument, 0 provided controls_[ctrl.id] will create an empty element, and thus needs a default constructor that V4L2ControlInfo doesn't provide. The operator returns a reference to the newly created element, and the copy operator= is then invoked. The copy constructor isn't involved at any time in that line. > I'm now keeping the explicit ::insert() call for now. If you want to make it efficient, use map::emplace() controls_.emplace(ctrl.id, info); > >>> + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; > >>> + } > >>> +} > >>> + > >>> +/* > >>> + * \brief Make sure the control type is supported > >>> + * \param[in] info The V4L2ControlInfo to inspect type of > >>> + * \return 0 on success or a negative error code otherwise > >>> + * \retval -EINVAL The control type is not supported > >>> + */ > >>> +int V4L2Device::validateControlType(const V4L2ControlInfo *info) > >> > >> As this method has a single user, how about inlining it ? > >> V4L2Device::listControls() isn't big. > > > > I would rather keep the type validation centralized in one place, and in > > the caller it's easy enough to parse if (validateControlType(type)) > > for a reader than going through the switch cases. > > > > The cost of a function call is negligible considering this happens > > only at device open time. It's not about the cost of a function call, I would find it more readable :-) > >>> +{ > >>> + /* \todo Support compound controls. */ > >>> + switch (info->type()) { > >>> + case V4L2_CTRL_TYPE_INTEGER: > >>> + case V4L2_CTRL_TYPE_BOOLEAN: > >>> + case V4L2_CTRL_TYPE_MENU: > >>> + case V4L2_CTRL_TYPE_BUTTON: > >>> + case V4L2_CTRL_TYPE_INTEGER64: > >>> + case V4L2_CTRL_TYPE_BITMASK: > >>> + case V4L2_CTRL_TYPE_INTEGER_MENU: > >>> + break; > >>> + default: > >>> + LOG(V4L2, Error) << "Control type '" << info->type() > >>> + << "' not supported"; > >>> + return -EINVAL; > >>> + } > >>> + > >>> + return 0; > >>> +} > >>> + > >>> } /* namespace libcamera */
diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h index 75f23a05b903..91a72fcecbcc 100644 --- a/src/libcamera/include/v4l2_device.h +++ b/src/libcamera/include/v4l2_device.h @@ -7,18 +7,23 @@ #ifndef __LIBCAMERA_V4L2_DEVICE_H__ #define __LIBCAMERA_V4L2_DEVICE_H__ +#include <map> #include <string> #include "log.h" namespace libcamera { +class V4L2ControlInfo; + class V4L2Device : protected Loggable { public: void close(); bool isOpen() const { return fd_ != -1; } + const V4L2ControlInfo *getControlInfo(unsigned int id) const; + const std::string &deviceNode() const { return deviceNode_; } protected: @@ -32,6 +37,10 @@ protected: int fd() { return fd_; } private: + void listControls(); + int validateControlType(const V4L2ControlInfo *info); + + std::map<unsigned int, V4L2ControlInfo> controls_; std::string deviceNode_; int fd_; }; diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 99621a724b96..b113ff77e3cf 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -13,6 +13,7 @@ #include <unistd.h> #include "log.h" +#include "v4l2_controls.h" /** * \file v4l2_device.h @@ -82,6 +83,8 @@ int V4L2Device::open(unsigned int flags) fd_ = ret; + listControls(); + return 0; } @@ -107,6 +110,21 @@ void V4L2Device::close() * \return True if the V4L2 device node is open, false otherwise */ +/** + * \brief Retrieve information about a control + * \param[in] id The control ID + * \return A pointer to the V4L2ControlInfo for control \a id, or nullptr + * if the device doesn't have such a control + */ +const V4L2ControlInfo *V4L2Device::getControlInfo(unsigned int id) const +{ + auto it = controls_.find(id); + if (it == controls_.end()) + return nullptr; + + return &it->second; +} + /** * \brief Perform an IOCTL system call on the device node * \param[in] request The IOCTL request code @@ -137,4 +155,58 @@ int V4L2Device::ioctl(unsigned long request, void *argp) * \return The V4L2 device file descriptor, -1 if the device node is not open */ +/* + * \brief List and store information about all controls supported by the + * V4L2 device + */ +void V4L2Device::listControls() +{ + struct v4l2_query_ext_ctrl ctrl = {}; + + /* \todo Add support for menu and compound controls. */ + ctrl.id = V4L2_CTRL_FLAG_NEXT_CTRL; + while (ioctl(VIDIOC_QUERY_EXT_CTRL, &ctrl) == 0) { + if (ctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS || + ctrl.flags & V4L2_CTRL_FLAG_DISABLED) { + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; + continue; + } + + V4L2ControlInfo info(ctrl); + if (validateControlType(&info)) + continue; + + controls_.insert(std::pair<unsigned int, V4L2ControlInfo> + (ctrl.id, info)); + ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL; + } +} + +/* + * \brief Make sure the control type is supported + * \param[in] info The V4L2ControlInfo to inspect type of + * \return 0 on success or a negative error code otherwise + * \retval -EINVAL The control type is not supported + */ +int V4L2Device::validateControlType(const V4L2ControlInfo *info) +{ + /* \todo Support compound controls. */ + switch (info->type()) { + case V4L2_CTRL_TYPE_INTEGER: + case V4L2_CTRL_TYPE_BOOLEAN: + case V4L2_CTRL_TYPE_MENU: + case V4L2_CTRL_TYPE_BUTTON: + case V4L2_CTRL_TYPE_INTEGER64: + case V4L2_CTRL_TYPE_BITMASK: + case V4L2_CTRL_TYPE_INTEGER_MENU: + break; + default: + LOG(V4L2, Error) << "Control type '" << info->type() + << "' not supported"; + return -EINVAL; + } + + return 0; +} + } /* namespace libcamera */
Enumerate all the valid controls a device supports at open() time. A control is valid only if its type is supported. Store the control informations in a map inside the device to save querying the control when setting or getting its value from the device and provide an operation to retrieve information by control ID. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/include/v4l2_device.h | 9 ++++ src/libcamera/v4l2_device.cpp | 72 +++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+)