[libcamera-devel,v3] libcamera: v4l2_videodevice: Add crop/selection control.

2768 diff mbox series

Add control for cropping/selection on a V4L2 video device through
the VIDIOC_S_SELECTION ioctl. This is similar to the existing cropping
control available on V4L2 sub-devices.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/include/v4l2_videodevice.h |  5 +++
 src/libcamera/v4l2_videodevice.cpp       | 48 ++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

Hi Naush,

On 04/02/2020 11:24, Naushir Patuck wrote:
> Add control for cropping/selection on a V4L2 video device through
> the VIDIOC_S_SELECTION ioctl. This is similar to the existing cropping
> control available on V4L2 sub-devices.

I would have said that ideally this should come with an implementation
to test it, but I'm not sure how easy it is to test crop/selection
control, perhaps vimc/vivid would give us a means to do so.

But that said, I don't think the crop/compose for the subdevs have tests
either, so I don't think that's a blocker at the moment, except for the
fact that this has no users in the code base, though I suspect there
will be one soon enough.

> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/include/v4l2_videodevice.h |  5 +++
>  src/libcamera/v4l2_videodevice.cpp       | 48 ++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index e4d35ab..fcf0726 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -182,6 +182,9 @@ public:
>  	int setFormat(V4L2DeviceFormat *format);
>  	ImageFormats formats();
>  
> +	int setCrop(Rectangle *rect);
> +	int setCompose(Rectangle *rect);
> +
>  	int exportBuffers(unsigned int count,
>  			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
>  	int importBuffers(unsigned int count);
> @@ -216,6 +219,8 @@ private:
>  	std::vector<unsigned int> enumPixelformats();
>  	std::vector<SizeRange> enumSizes(unsigned int pixelFormat);
>  
> +	int setSelection(unsigned int target, Rectangle *rect);
> +
>  	int requestBuffers(unsigned int count);
>  	std::unique_ptr<FrameBuffer> createBuffer(const struct v4l2_buffer &buf);
>  	FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane);
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 8226773..f3c66ed 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -940,6 +940,54 @@ std::vector<SizeRange> V4L2VideoDevice::enumSizes(unsigned int pixelFormat)
>  	return sizes;
>  }
>  
> +/**
> + * \brief Set a crop rectangle on the V4L2 video device node
> + * \param[inout] rect The rectangle describing the crop target area
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2VideoDevice::setCrop(Rectangle *rect)
> +{
> +	return setSelection(V4L2_SEL_TGT_CROP, rect);
> +}
> +
> +/**
> + * \brief Set a compose rectangle on the V4L2 video device node
> + * \param[inout] rect The rectangle describing the compose target area
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2VideoDevice::setCompose(Rectangle *rect)
> +{
> +	return setSelection(V4L2_SEL_TGT_COMPOSE, rect);
> +}
> +
> +int V4L2VideoDevice::setSelection(unsigned int target, Rectangle *rect)
> +{
> +	struct v4l2_selection sel = {};
> +
> +	sel.type = bufferType_;
> +	sel.target = target;
> +	sel.flags = 0;
> +
> +	sel.r.left = rect->x;
> +	sel.r.top = rect->y;
> +	sel.r.width = rect->w;
> +	sel.r.height = rect->h;
> +
> +	int ret = ioctl(VIDIOC_S_SELECTION, &sel);
> +	if (ret < 0) {
> +		LOG(V4L2, Error) << "Unable to set rectangle " << target
> +				 << ": " << strerror(-ret);
> +		return ret;
> +	}
> +
> +	rect->x = sel.r.left;
> +	rect->y = sel.r.top;
> +	rect->w = sel.r.width;
> +	rect->h = sel.r.height;
> +
> +	return 0;
> +}
> +
>  int V4L2VideoDevice::requestBuffers(unsigned int count)
>  {
>  	struct v4l2_requestbuffers rb = {};
>

Hi Naush,

Thank you for the patch.

On Tue, Feb 04, 2020 at 11:24:44AM +0000, Naushir Patuck wrote:
> Add control for cropping/selection on a V4L2 video device through
> the VIDIOC_S_SELECTION ioctl. This is similar to the existing cropping
> control available on V4L2 sub-devices.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

With the period at the end of the commit subject removed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

and pushed to the master branch.

> ---
>  src/libcamera/include/v4l2_videodevice.h |  5 +++
>  src/libcamera/v4l2_videodevice.cpp       | 48 ++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index e4d35ab..fcf0726 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -182,6 +182,9 @@ public:
>  	int setFormat(V4L2DeviceFormat *format);
>  	ImageFormats formats();
>  
> +	int setCrop(Rectangle *rect);
> +	int setCompose(Rectangle *rect);
> +
>  	int exportBuffers(unsigned int count,
>  			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
>  	int importBuffers(unsigned int count);
> @@ -216,6 +219,8 @@ private:
>  	std::vector<unsigned int> enumPixelformats();
>  	std::vector<SizeRange> enumSizes(unsigned int pixelFormat);
>  
> +	int setSelection(unsigned int target, Rectangle *rect);
> +
>  	int requestBuffers(unsigned int count);
>  	std::unique_ptr<FrameBuffer> createBuffer(const struct v4l2_buffer &buf);
>  	FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane);
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 8226773..f3c66ed 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -940,6 +940,54 @@ std::vector<SizeRange> V4L2VideoDevice::enumSizes(unsigned int pixelFormat)
>  	return sizes;
>  }
>  
> +/**
> + * \brief Set a crop rectangle on the V4L2 video device node
> + * \param[inout] rect The rectangle describing the crop target area
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2VideoDevice::setCrop(Rectangle *rect)
> +{
> +	return setSelection(V4L2_SEL_TGT_CROP, rect);
> +}
> +
> +/**
> + * \brief Set a compose rectangle on the V4L2 video device node
> + * \param[inout] rect The rectangle describing the compose target area
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2VideoDevice::setCompose(Rectangle *rect)
> +{
> +	return setSelection(V4L2_SEL_TGT_COMPOSE, rect);
> +}
> +
> +int V4L2VideoDevice::setSelection(unsigned int target, Rectangle *rect)
> +{
> +	struct v4l2_selection sel = {};
> +
> +	sel.type = bufferType_;
> +	sel.target = target;
> +	sel.flags = 0;
> +
> +	sel.r.left = rect->x;
> +	sel.r.top = rect->y;
> +	sel.r.width = rect->w;
> +	sel.r.height = rect->h;
> +
> +	int ret = ioctl(VIDIOC_S_SELECTION, &sel);
> +	if (ret < 0) {
> +		LOG(V4L2, Error) << "Unable to set rectangle " << target
> +				 << ": " << strerror(-ret);
> +		return ret;
> +	}
> +
> +	rect->x = sel.r.left;
> +	rect->y = sel.r.top;
> +	rect->w = sel.r.width;
> +	rect->h = sel.r.height;
> +
> +	return 0;
> +}
> +
>  int V4L2VideoDevice::requestBuffers(unsigned int count)
>  {
>  	struct v4l2_requestbuffers rb = {};