[libcamera-devel] [PATCH v5 2/6] libcamera: v4l2_device: List valid controls at open
Jacopo Mondi
jacopo at jmondi.org
Mon Jun 24 11:26:12 CEST 2019
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 at 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 at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190624/7a712dd2/attachment.sig>
More information about the libcamera-devel
mailing list