[libcamera-devel] [RFC] libcamera: v4l2_device: Update control info

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Jun 05, 2020 at 04:20:37PM +0200, Jacopo Mondi wrote:
> Setting controls on a V4L2 device might update the limits of other
> related controls (in example, setting a new vertical/horizontal blanking
> value might modify the available exposure time duration).
> 
> Add a funtion to update the ControlInfo that represents a control
> minimum, maximum and default values and call it after a new control has
> been set and format is changed on a V4L2 subdevice.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> 
> We'll soon need something like that.
> However, seems like an expensive operation to perform after every control
> and format update.
> 
> Opinions ?

I'd rather avoid this if we can. For the example you mention above, I'd
rather expose a fixed value to express the minimum margin between the
exposure time and the vertical total size (even possibly hardcoding it
in userspace like we do today). I hope we'll be able to find similar
solutions for the other cases that could benefit from control updates.
Otherwise, if we *really* have to do this, V4L2 should give us control
info change events.

> ---
>  include/libcamera/controls.h             |  4 ++
>  include/libcamera/internal/v4l2_device.h |  2 +
>  src/libcamera/controls.cpp               | 18 +++++++++
>  src/libcamera/v4l2_device.cpp            | 48 ++++++++++++++++++++++++
>  src/libcamera/v4l2_subdevice.cpp         |  3 ++
>  5 files changed, 75 insertions(+)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 80944efc133a..74b0c6b26517 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -273,6 +273,10 @@ public:
>  	const ControlValue &max() const { return max_; }
>  	const ControlValue &def() const { return def_; }
> 
> +	void setMin(ControlValue val) { min_ = val; }
> +	void setMax(ControlValue val) { max_ = val; }
> +	void setDef(ControlValue val) { def_ = val; }
> +
>  	std::string toString() const;
> 
>  	bool operator==(const ControlInfo &other) const
> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> index d491eafd262e..1563ec7272c7 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -42,6 +42,8 @@ protected:
> 
>  	int fd() { return fd_; }
> 
> +	void updateControlsInfo();
> +
>  private:
>  	void listControls();
>  	void updateControls(ControlList *ctrls,
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index dca782667d88..f92128dd1878 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -519,6 +519,24 @@ ControlInfo::ControlInfo(const ControlValue &min,
>   * \return A ControlValue with the default value for the control
>   */
> 
> +/**
> + * \fn ControlInfo::setMin()
> + * \brief Set the minimum value for the control
> + * \param[in] val The control minimum value
> + */
> +
> +/**
> + * \fn ControlInfo::setMax()
> + * \brief Set the maximum value for the control
> + * \param[in] val The control maximum value
> + */
> +
> +/**
> + * \fn ControlInfo::setDef()
> + * \brief Set the default value for the control
> + * \param[in] val The control default value
> + */
> +
>  /**
>   * \brief Provide a string representation of the ControlInfo
>   */
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 56ea1ddda2c1..7e1286d7ddd0 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -346,6 +346,7 @@ int V4L2Device::setControls(ControlList *ctrls)
>  	}
> 
>  	updateControls(ctrls, v4l2Ctrls, count);
> +	updateControlsInfo();
> 
>  	return ret;
>  }
> @@ -380,6 +381,53 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
>   * \return The V4L2 device file descriptor, -1 if the device node is not open
>   */
> 
> +/**
> + * \brief Update the control info
> + *
> + * Update all control limits (min and max) and default value.
> + */
> +void V4L2Device::updateControlsInfo()
> +{
> +	for (auto &it : controls_) {
> +		const ControlId *id = it.first;
> +		ControlInfo &info = it.second;
> +		struct v4l2_query_ext_ctrl ctrl = {};
> +		ctrl.id = id->id();
> +		int ret = ioctl(VIDIOC_QUERY_EXT_CTRL, &ctrl);
> +		if (ret) {
> +			LOG(V4L2, Error) << "Unable to query control "
> +					 << ctrl.id << ": " << strerror(-ret);
> +			return;
> +		}
> +
> +		switch (ctrl.type) {
> +		case V4L2_CTRL_TYPE_U8:
> +			info.setMin({ static_cast<uint8_t>(ctrl.minimum) });
> +			info.setMax({ static_cast<uint8_t>(ctrl.maximum) });
> +			info.setDef({ static_cast<uint8_t>(ctrl.default_value) });
> +			break;
> +
> +		case V4L2_CTRL_TYPE_BOOLEAN:
> +			info.setMin({ static_cast<bool>(ctrl.minimum) });
> +			info.setMax({ static_cast<bool>(ctrl.maximum) });
> +			info.setDef({ static_cast<bool>(ctrl.default_value) });
> +			break;
> +
> +		case V4L2_CTRL_TYPE_INTEGER64:
> +			info.setMin({ static_cast<bool>(ctrl.minimum) });
> +			info.setMax({ static_cast<bool>(ctrl.maximum) });
> +			info.setDef({ static_cast<bool>(ctrl.default_value) });
> +			break;
> +
> +		default:
> +			info.setMin({ static_cast<bool>(ctrl.minimum) });
> +			info.setMax({ static_cast<bool>(ctrl.maximum) });
> +			info.setDef({ static_cast<bool>(ctrl.default_value) });
> +			break;
> +		}
> +	}
> +}
> +
>  /*
>   * \brief List and store information about all controls supported by the
>   * V4L2 device
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 7aefc1be032d..c0dc036a09cc 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -411,6 +411,9 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
>  	format->size.height = subdevFmt.format.height;
>  	format->mbus_code = subdevFmt.format.code;
> 
> +	/* Changing the format could update the contol limits. */
> +	updateControlsInfo();
> +
>  	return 0;
>  }
> 

-- 
Regards,

Laurent Pinchart