[libcamera-devel,v2,3/7] libcamera: V4L2Device: Support v4l2 menu control

12020 diff mbox series

This adds a support of v4l2 menu.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
---
 include/libcamera/internal/v4l2_controls.h |  1 +
 include/libcamera/internal/v4l2_device.h   |  3 +
 src/libcamera/v4l2_controls.cpp            | 10 ++-
 src/libcamera/v4l2_device.cpp              | 90 ++++++++++++++++++++--
 4 files changed, 98 insertions(+), 6 deletions(-)

Hi Hiro,

Thank you for the patch.

On Wed, Apr 21, 2021 at 01:23:42PM +0900, Hirokazu Honda wrote:
> This adds a support of v4l2 menu.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> ---
>  include/libcamera/internal/v4l2_controls.h |  1 +
>  include/libcamera/internal/v4l2_device.h   |  3 +
>  src/libcamera/v4l2_controls.cpp            | 10 ++-
>  src/libcamera/v4l2_device.cpp              | 90 ++++++++++++++++++++--
>  4 files changed, 98 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libcamera/internal/v4l2_controls.h b/include/libcamera/internal/v4l2_controls.h
> index 0851b8dd..c42f2529 100644
> --- a/include/libcamera/internal/v4l2_controls.h
> +++ b/include/libcamera/internal/v4l2_controls.h
> @@ -24,6 +24,7 @@ class V4L2ControlInfo : public ControlInfo
>  {
>  public:
>  	V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
> +	V4L2ControlInfo(const ControlInfo &ctrl);

The purpose of the V4L2ControlInfo class is to provide an easy way to
construct a ControlInfo from v4l2_query_ext_ctrl. It has no data member,
so it's really only a convenience constructor. If we need to make
copies, let's use the base ControlInfo class. That's what we store in
the ControlInfoMap.

>  };
>  
>  } /* namespace libcamera */
> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> index 4cce3840..2436a83c 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -53,6 +53,9 @@ protected:
>  	int fd() const { return fd_; }
>  
>  private:
> +	bool createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
> +					  V4L2ControlInfo &v4l2CtrlInfo);
> +
>  	void listControls();
>  	void updateControls(ControlList *ctrls,
>  			    const std::vector<v4l2_ext_control> &v4l2Ctrls);
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index 3f8ec6ca..2c965cfa 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -73,9 +73,12 @@ ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)
>  		return ControlTypeInteger64;
>  
>  	case V4L2_CTRL_TYPE_MENU:
> +	case V4L2_CTRL_TYPE_INTEGER_MENU:
> +		return ControlTypeMenu;
> +
>  	case V4L2_CTRL_TYPE_BUTTON:
>  	case V4L2_CTRL_TYPE_BITMASK:
> -	case V4L2_CTRL_TYPE_INTEGER_MENU:
> +
>  		/*
>  		 * More precise types may be needed, for now use a 32-bit
>  		 * integer type.
> @@ -148,4 +151,9 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
>  	}
>  }
>  
> +V4L2ControlInfo::V4L2ControlInfo(const ControlInfo &ctrl)
> +	: ControlInfo(ctrl)
> +{
> +}
> +
>  } /* namespace libcamera */
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 8f29cd7f..585e527b 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -459,6 +459,47 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
>   * \return The device node path
>   */
>  
> +bool V4L2Device::createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
> +					      V4L2ControlInfo &v4l2CtrlInfo)
> +{
> +	ASSERT(ctrl.type == V4L2_CTRL_TYPE_MENU ||
> +	       ctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU);
> +	const bool isName = ctrl.type == V4L2_CTRL_TYPE_MENU;
> +
> +	std::vector<ControlValue> values;
> +	v4l2_querymenu menu;
> +	memset(&menu, 0, sizeof(menu));
> +	menu.id = ctrl.id;
> +	for (menu.index = ctrl.minimum;
> +	     static_cast<int>(menu.index) <= ctrl.maximum; menu.index++) {
> +		if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)
> +			continue;
> +		ControlMenu ctrlMenu;
> +		ctrlMenu.index = static_cast<int32_t>(menu.index);
> +		ctrlMenu.isName = isName;
> +		if (isName) {
> +			strcpy(ctrlMenu.name,
> +			       reinterpret_cast<const char *>(menu.name));
> +		} else {
> +			ctrlMenu.value = menu.value;
> +		}
> +
> +		values.push_back(ControlValue(ctrlMenu));
> +	}
> +
> +	if (values.empty()) {
> +		LOG(V4L2, Error)
> +			<< "No applicable value: " << utils::hex(ctrl.id);
> +		return false;
> +	}
> +
> +	v4l2CtrlInfo = V4L2ControlInfo(
> +		ControlInfo(libcamera::Span<const ControlValue>(
> +			values.data(), values.size())));

There's no need to go through V4L2ControlInfo here, ControlInfo is all
you need.

> +
> +	return true;
> +}
> +
>  /**
>   * \fn V4L2Device::fd()
>   * \brief Retrieve the V4L2 device file descriptor
> @@ -474,7 +515,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;
> @@ -485,15 +525,20 @@ void V4L2Device::listControls()
>  		    ctrl.flags & V4L2_CTRL_FLAG_DISABLED)
>  			continue;
>  
> +		V4L2ControlInfo v4l2CtrlInfo(ControlInfo(0));

That seems like a hack :-)

>  		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:
> +			v4l2CtrlInfo = V4L2ControlInfo(ctrl);
> +			break;
> +		case V4L2_CTRL_TYPE_MENU:
> +		case V4L2_CTRL_TYPE_INTEGER_MENU:
> +			if (!createV4L2ControlInfoForMenu(ctrl, v4l2CtrlInfo))
> +				continue;
>  			break;
>  		/* \todo Support other control types. */
>  		default:
> @@ -503,10 +548,13 @@ void V4L2Device::listControls()
>  			continue;
>  		}
>  
> +		LOG(V4L2, Debug) << "Control: " << ctrl.name
> +				 << " (" << utils::hex(ctrl.id) << ")";
> +
>  		controlIds_.emplace_back(std::make_unique<V4L2ControlId>(ctrl));
>  		controlInfo_.emplace(ctrl.id, ctrl);
>  
> -		ctrls.emplace(controlIds_.back().get(), V4L2ControlInfo(ctrl));
> +		ctrls.emplace(controlIds_.back().get(), std::move(v4l2CtrlInfo));
>  	}
>  
>  	controls_ = std::move(ctrls);
> @@ -539,7 +587,39 @@ void V4L2Device::updateControls(ControlList *ctrls,
>  		case ControlTypeInteger64:
>  			newValue.set<int64_t>(v4l2Ctrl.value64);
>  			break;
> -
> +		case ControlTypeMenu:
> +			switch (v4l2Ctrl.id) {
> +			case V4L2_CID_TEST_PATTERN: {

Let's keep specific control IDs out of this code, it should be
implemented generically for all menu controls.

> +				ControlMenu menu;
> +				menu.index = v4l2Ctrl.value;
> +				menu.isName = true;
> +
> +				bool found = false;
> +				for (const ControlValue &validValue : it->second.values()) {
> +					const auto &ctrlMenu = validValue.get<ControlMenu>();
> +					if (!ctrlMenu.isName)
> +						continue;
> +					if (menu.index == ctrlMenu.index) {
> +						strcpy(menu.name, ctrlMenu.name);
> +						found = true;
> +						break;
> +					}
> +				}
> +
> +				if (!found) {
> +					LOG(V4L2, Error)
> +						<< "Matched value is not found"
> +						<< ", index=" << menu.index;
> +					continue;
> +				}
> +
> +				newValue.set<ControlMenu>(menu);
> +				break;
> +			}
> +			default:
> +				LOG(V4L2, Error) << "Unknown id: " << v4l2Ctrl.id;
> +				break;
> +			}
>  		case ControlTypeByte:
>  			/*
>  			 * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl