[libcamera-devel] [PATCH 2/6] libcamera: v4l2_device: Add setControl function

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Jan 21, 2019 at 06:27:01PM +0100, Jacopo Mondi wrote:
> Add function to set controls on a V4L2Device object.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/include/v4l2_device.h |  2 ++
>  src/libcamera/v4l2_device.cpp       | 38 +++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index c6f3d9a..2787847 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -51,6 +51,8 @@ public:
>  	const char *deviceName() const { return caps_.card(); }
>  	const char *busName() const { return caps_.bus_info(); }
>  
> +	int setControl(unsigned int control, int value);
> +

I think this is needed, but requires a little more thoughts to define a
good API. To start with, not all controls have integer values, we also
have string controls. Furthermore, we will also likely need to use the
extended controls API to set multiple controls at a time, requiring a
vector to be passed to the function.

Here's a very preliminary proposal.

class V4L2ControlValue
{
	enum Type {
		/* The control types. */
	};

	/*
	 * This should store the type and value, with appropriate
	 * constructors and accessors.
	 /
};

	int setControl(unsigned int id, const V4L2ControlValue &control);
	int setControls(const std::vector<std::pair<unsigned int, V4L2ControlValue>> &controls);

setControl() could be implemented by calling setControls().
setControls() should use the extended controls API, and possibly fall
back to the old controls API if the extended API isn't available,
although I think we can depend on the extended API in libcamera.

We could also create a class that groups together the id and value,
possibly named V4L2Control, but I have a feeling we will later need to
enumerate controls and store the data queried from the kernel, and
V4L2Control would be a good class name for that.

>  private:
>  	std::string devnode_;
>  	int fd_;
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 59a1ad9..aef0996 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -195,4 +195,42 @@ void V4L2Device::close()
>   * \return The string containing the device location
>   */
>  
> +/**
> + * \brief Set a control value
> + * \param control The control identifier
> + * \param value The new control value
> + *
> + * Set a user control to the requested value. The function returns the
> + * new value actually assigned to the control, which might be different
> + * from the requested \a value one.
> + *
> + * \return The new control's value for success, or a negative error code otherwise
> + */
> +int V4L2Device::setControl(unsigned int control, int value)
> +{
> +	struct v4l2_control v4l2_ctrl = {
> +		.id = control,
> +		.value = value,
> +	};
> +
> +	int ret = ioctl(fd_, VIDIOC_S_CTRL, &v4l2_ctrl);
> +	if (ret) {
> +		ret = -errno;
> +		LOG(Error) << "Failed to set control: " << strerror(-ret);
> +		return ret;
> +	}
> +
> +	v4l2_ctrl = { };
> +	v4l2_ctrl.id = control;
> +
> +	ret = ioctl(fd_, VIDIOC_G_CTRL, &v4l2_ctrl);
> +	if (ret) {
> +		ret = -errno;
> +		LOG(Error) << "Failed to get control: " << strerror(-ret);
> +		return ret;
> +	}
> +
> +	return v4l2_ctrl.value;
> +}
> +
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart