[libcamera-devel] [PATCH v4 2/6] libcamera: v4l2_device: List controls at device open

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Jun 19, 2019 at 01:08:54PM +0200, Jacopo Mondi wrote:
> Enumerate all the controls a device supports at open() time.
> Store the control informations in a map inside the device to save
> querying the control when setting or getting its value from the device.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/include/v4l2_device.h |  8 +++++++-
>  src/libcamera/v4l2_device.cpp       | 30 +++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index 2c26f3eae2b4..563be151c257 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -7,12 +7,15 @@
>  #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:
> @@ -22,7 +25,7 @@ public:
>  
>  protected:
>  	V4L2Device(const std::string &deviceNode, const std::string &logTag);
> -	~V4L2Device() {}
> +	~V4L2Device();
>  
>  	int fd() { return fd_; }
>  
> @@ -33,6 +36,9 @@ protected:
>  	std::string logPrefix() const { return "'" + logTag_ + "'"; }
>  
>  private:
> +	void listControls();
> +
> +	std::map<unsigned int, V4L2ControlInfo *> controls_;
>  	std::string deviceNode_;
>  	std::string logTag_;
>  	int fd_;
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index eeed0a70fb0e..58dd94836686 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,12 @@ V4L2Device::V4L2Device(const std::string &deviceNode, const std::string &logTag)
>  {
>  }
>  
> +V4L2Device::~V4L2Device()
> +{
> +	for (auto &pair : controls_)
> +		delete pair.second;
> +}
> +
>  /**
>   * \fn V4L2Device::fd()
>   * \brief Retrieve the V4L2 device file descriptor
> @@ -114,6 +121,8 @@ int V4L2Device::open(unsigned int flags)
>  
>  	fd_ = ret;
>  
> +	listControls();
> +
>  	return 0;
>  }
>  
> @@ -141,4 +150,25 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
>   * \return The tag to prefix log messages with
>   */
>  
> +/*
> + * \brief List and store all controls supported by the V4L2 device

"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. */

No need for a colon after any doxygen tag. \todo Add support ...

> +	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;
> +		}
> +
> +		controls_[ctrl.id] = new V4L2ControlInfo(ctrl);

Would it be feasible to store a V4L2ControlInfo in the map instead of a
pointer ? That would simplify the destructor.

> +		ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL;
> +	}
> +}
> +
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart