[libcamera-devel,v2,1/3] libcamera: v4l2_device: Add tryFormat support

1270 diff mbox series

Extend the internal functionality of setFormat{Single,Multi}plane to
allow 'trying' a format. This provides the facility of checking a format
with the driver before committing to it.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/include/v4l2_device.h |  7 +++++--
 src/libcamera/v4l2_device.cpp       | 29 +++++++++++++++++++++++------
 2 files changed, 28 insertions(+), 8 deletions(-)

Hi Kieran,

On Thu, May 23, 2019 at 02:58:58PM +0100, Kieran Bingham wrote:
> Extend the internal functionality of setFormat{Single,Multi}plane to
> allow 'trying' a format. This provides the facility of checking a format
> with the driver before committing to it.
>

Regardless of the format enumeration that Niklas' working on, I think
having a try format API is still a worthwile change.

Patch looks fine
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/include/v4l2_device.h |  7 +++++--
>  src/libcamera/v4l2_device.cpp       | 29 +++++++++++++++++++++++------
>  2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index 2e7bd1e7f4cc..a08615c3bd9a 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -126,6 +126,7 @@ public:
>
>  	int getFormat(V4L2DeviceFormat *format);
>  	int setFormat(V4L2DeviceFormat *format);
> +	int tryFormat(V4L2DeviceFormat *format);
>
>  	int exportBuffers(BufferPool *pool);
>  	int importBuffers(BufferPool *pool);
> @@ -145,10 +146,12 @@ protected:
>
>  private:
>  	int getFormatSingleplane(V4L2DeviceFormat *format);
> -	int setFormatSingleplane(V4L2DeviceFormat *format);
> +	int trySetFormatSingleplane(V4L2DeviceFormat *format,
> +				    unsigned long request);
>
>  	int getFormatMultiplane(V4L2DeviceFormat *format);
> -	int setFormatMultiplane(V4L2DeviceFormat *format);
> +	int trySetFormatMultiplane(V4L2DeviceFormat *format,
> +				   unsigned long request);
>
>  	int requestBuffers(unsigned int count);
>  	int createPlane(Buffer *buffer, unsigned int plane,
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 8366ffc4db55..2cdbc2696e55 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -434,8 +434,23 @@ int V4L2Device::getFormat(V4L2DeviceFormat *format)
>   */
>  int V4L2Device::setFormat(V4L2DeviceFormat *format)
>  {
> -	return caps_.isMultiplanar() ? setFormatMultiplane(format) :
> -				       setFormatSingleplane(format);
> +	return caps_.isMultiplanar() ? trySetFormatMultiplane(format, VIDIOC_S_FMT) :
> +				       trySetFormatSingleplane(format, VIDIOC_S_FMT);
> +}
> +
> +/**
> + * \brief Validate an image format on the V4L2 device
> + * \param[inout] format The image format to apply to the device
> + *
> + * Try the supplied \a format on the device, and return the format parameters
> + * that would have been applied, as \ref V4L2Device::setFormat would do.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2Device::tryFormat(V4L2DeviceFormat *format)
> +{
> +	return caps_.isMultiplanar() ? trySetFormatMultiplane(format, VIDIOC_TRY_FMT) :
> +				       trySetFormatSingleplane(format, VIDIOC_TRY_FMT);
>  }
>
>  int V4L2Device::getFormatSingleplane(V4L2DeviceFormat *format)
> @@ -462,7 +477,8 @@ int V4L2Device::getFormatSingleplane(V4L2DeviceFormat *format)
>  	return 0;
>  }
>
> -int V4L2Device::setFormatSingleplane(V4L2DeviceFormat *format)
> +int V4L2Device::trySetFormatSingleplane(V4L2DeviceFormat *format,
> +					unsigned long request)
>  {
>  	struct v4l2_format v4l2Format = {};
>  	struct v4l2_pix_format *pix = &v4l2Format.fmt.pix;
> @@ -475,7 +491,7 @@ int V4L2Device::setFormatSingleplane(V4L2DeviceFormat *format)
>  	pix->bytesperline = format->planes[0].bpl;
>  	pix->field = V4L2_FIELD_NONE;
>
> -	ret = ioctl(fd_, VIDIOC_S_FMT, &v4l2Format);
> +	ret = ioctl(fd_, request, &v4l2Format);
>  	if (ret) {
>  		ret = -errno;
>  		LOG(V4L2, Error) << "Unable to set format: " << strerror(-ret);
> @@ -523,7 +539,8 @@ int V4L2Device::getFormatMultiplane(V4L2DeviceFormat *format)
>  	return 0;
>  }
>
> -int V4L2Device::setFormatMultiplane(V4L2DeviceFormat *format)
> +int V4L2Device::trySetFormatMultiplane(V4L2DeviceFormat *format,
> +				       unsigned long request)
>  {
>  	struct v4l2_format v4l2Format = {};
>  	struct v4l2_pix_format_mplane *pix = &v4l2Format.fmt.pix_mp;
> @@ -541,7 +558,7 @@ int V4L2Device::setFormatMultiplane(V4L2DeviceFormat *format)
>  		pix->plane_fmt[i].sizeimage = format->planes[i].size;
>  	}
>
> -	ret = ioctl(fd_, VIDIOC_S_FMT, &v4l2Format);
> +	ret = ioctl(fd_, request, &v4l2Format);
>  	if (ret) {
>  		ret = -errno;
>  		LOG(V4L2, Error) << "Unable to set format: " << strerror(-ret);
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel