[libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support v4l2 menu control

[Jacopo Mondi]

Hi Hiro,

On Mon, Jun 07, 2021 at 10:13:58AM +0900, Hirokazu Honda wrote:
> This adds a support of v4l2 menu. The control info for v4l2 menu
> contains indices without names and 64bit values of querymenu.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  include/libcamera/internal/v4l2_device.h |  2 +
>  src/libcamera/v4l2_device.cpp            | 53 ++++++++++++++++++++++--
>  2 files changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> index b82e2a14..687be9b2 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -56,6 +56,8 @@ protected:
>  	int fd() const { return fd_; }
>
>  private:
> +	ControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);
> +
>  	void listControls();
>  	void updateControls(ControlList *ctrls,
>  			    Span<const v4l2_ext_control> v4l2Ctrls);
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index b39c6266..40e2c74c 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -524,6 +524,38 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
>   * \return The V4L2 device file descriptor, -1 if the device node is not open
>   */
>
> +/**
> + * \brief Create ControlInfo for a V4L2 menu ctrl
> + * \param[in] ctrl The v4l2_query_ext_ctrl that represents a menu
> + *
> + * The created ControlInfo contains indices acquired by VIDIOC_QUERYMENU.
> + *
> + * \return ControlInfo for a V4L2 menu ctrl.

nit: no full stop at the end
Can be changed when applying!

Thanks for keep pushing this series
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j


> + */
> +ControlInfo V4L2Device::v4l2MenuControlInfo(
> +	const struct v4l2_query_ext_ctrl &ctrl)
> +{
> +	std::vector<ControlValue> indices;
> +	struct v4l2_querymenu menu = {};
> +	menu.id = ctrl.id;
> +
> +	if (ctrl.minimum < 0)
> +		return ControlInfo();
> +
> +	ControlValue def = {};
> +	for (int32_t index = ctrl.minimum; index <= ctrl.maximum; ++index) {
> +		menu.index = index;
> +		if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)
> +			continue;
> +
> +		indices.push_back(index);
> +		if (index == ctrl.default_value)
> +			def = ControlValue(index);
> +	}
> +
> +	return ControlInfo(indices, def);
> +}
> +
>  /*
>   * \brief List and store information about all controls supported by the
>   * V4L2 device
> @@ -533,7 +565,6 @@ void V4L2Device::listControls()
>  	ControlInfoMap::Map ctrls;
>  	struct v4l2_query_ext_ctrl ctrl = {};
>
> -	/* \todo Add support for menu controls. */
>  	while (1) {
>  		ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |
>  			   V4L2_CTRL_FLAG_NEXT_COMPOUND;
> @@ -544,16 +575,23 @@ void V4L2Device::listControls()
>  		    ctrl.flags & V4L2_CTRL_FLAG_DISABLED)
>  			continue;
>
> +		ControlInfo ctrlInfo;
> +
>  		switch (ctrl.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:
>  		case V4L2_CTRL_TYPE_U8:
> +			ctrlInfo = v4l2ControlInfo(ctrl);
> +			break;
> +
> +		case V4L2_CTRL_TYPE_INTEGER_MENU:
> +		case V4L2_CTRL_TYPE_MENU:
> +			ctrlInfo = v4l2MenuControlInfo(ctrl);
>  			break;
> +
>  		/* \todo Support other control types. */
>  		default:
>  			LOG(V4L2, Debug)
> @@ -562,10 +600,13 @@ void V4L2Device::listControls()
>  			continue;
>  		}
>
> +		LOG(V4L2, Debug) << "Control: " << ctrl.name
> +				 << " (" << utils::hex(ctrl.id) << ")";
> +
>  		controlIds_.emplace_back(v4l2ControlId(ctrl));
>  		controlInfo_.emplace(ctrl.id, ctrl);
>
> -		ctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl));
> +		ctrls.emplace(controlIds_.back().get(), std::move(ctrlInfo));
>  	}
>
>  	controls_ = std::move(ctrls);
> @@ -630,6 +671,10 @@ void V4L2Device::updateControls(ControlList *ctrls,
>  			value.set<int64_t>(v4l2Ctrl.value64);
>  			break;
>
> +		case ControlTypeInteger32:
> +			value.set<int32_t>(v4l2Ctrl.value);
> +			break;
> +
>  		case ControlTypeByte:
>  			/*
>  			 * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl
> --
> 2.32.0.rc1.229.g3e70b5a671-goog
>