[libcamera-devel,v4,2/7] libcamera: v4l2_videodevice: Expose setSelection()
diff mbox series

Message ID 20200427213236.333777-3-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: Add CameraSensorInfo
Related show

Commit Message

Jacopo Mondi April 27, 2020, 9:32 p.m. UTC 
Expose V4L2Videodevice::setSelection() method and drop
V4L2Videodevice::setCrop() and V4L2Videodevice::setComopse() as wrapping
each target with a single function does not provide any benefit.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/include/v4l2_videodevice.h |  5 +----
 src/libcamera/v4l2_videodevice.cpp       | 20 +++-----------------
 2 files changed, 4 insertions(+), 21 deletions(-)

Comments

Niklas Söderlund April 27, 2020, 11:37 p.m. UTC | #1 
Hi Jacopo,

Thanks for your work.

On 2020-04-27 23:32:31 +0200, Jacopo Mondi wrote:
> Expose V4L2Videodevice::setSelection() method and drop
> V4L2Videodevice::setCrop() and V4L2Videodevice::setComopse() as wrapping
> each target with a single function does not provide any benefit.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/include/v4l2_videodevice.h |  5 +----
>  src/libcamera/v4l2_videodevice.cpp       | 20 +++-----------------
>  2 files changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index a0409e59c08f..976ef9b6dc50 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -211,8 +211,7 @@ public:
>  	int setFormat(V4L2DeviceFormat *format);
>  	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats();
>  
> -	int setCrop(Rectangle *rect);
> -	int setCompose(Rectangle *rect);
> +	int setSelection(unsigned int target, Rectangle *rect);
>  
>  	int allocateBuffers(unsigned int count,
>  			    std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> @@ -254,8 +253,6 @@ private:
>  	std::vector<V4L2PixelFormat> enumPixelformats();
>  	std::vector<SizeRange> enumSizes(V4L2PixelFormat pixelFormat);
>  
> -	int setSelection(unsigned int target, Rectangle *rect);
> -
>  	int requestBuffers(unsigned int count, enum v4l2_memory memoryType);
>  	int createBuffers(unsigned int count,
>  			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 8d642be0d05c..b16ab82c2807 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1108,25 +1108,11 @@ std::vector<SizeRange> V4L2VideoDevice::enumSizes(V4L2PixelFormat pixelFormat)
>  }
>  
>  /**
> - * \brief Set a crop rectangle on the V4L2 video device node
> - * \param[inout] rect The rectangle describing the crop target area
> + * \brief Set a selection rectangle \a rect for \a target
> + * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
> + * \param[inout] rect The selection rectangle to be applied
>   * \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 = {};
> -- 
> 2.26.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart April 28, 2020, 1:37 a.m. UTC | #2 
Hi Jacopo,

Thank you for the patch.

On Mon, Apr 27, 2020 at 11:32:31PM +0200, Jacopo Mondi wrote:
> Expose V4L2Videodevice::setSelection() method and drop
> V4L2Videodevice::setCrop() and V4L2Videodevice::setComopse() as wrapping
> each target with a single function does not provide any benefit.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  src/libcamera/include/v4l2_videodevice.h |  5 +----
>  src/libcamera/v4l2_videodevice.cpp       | 20 +++-----------------
>  2 files changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index a0409e59c08f..976ef9b6dc50 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -211,8 +211,7 @@ public:
>  	int setFormat(V4L2DeviceFormat *format);
>  	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats();
>  
> -	int setCrop(Rectangle *rect);
> -	int setCompose(Rectangle *rect);
> +	int setSelection(unsigned int target, Rectangle *rect);
>  
>  	int allocateBuffers(unsigned int count,
>  			    std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> @@ -254,8 +253,6 @@ private:
>  	std::vector<V4L2PixelFormat> enumPixelformats();
>  	std::vector<SizeRange> enumSizes(V4L2PixelFormat pixelFormat);
>  
> -	int setSelection(unsigned int target, Rectangle *rect);
> -
>  	int requestBuffers(unsigned int count, enum v4l2_memory memoryType);
>  	int createBuffers(unsigned int count,
>  			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 8d642be0d05c..b16ab82c2807 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1108,25 +1108,11 @@ std::vector<SizeRange> V4L2VideoDevice::enumSizes(V4L2PixelFormat pixelFormat)
>  }
>  
>  /**
> - * \brief Set a crop rectangle on the V4L2 video device node
> - * \param[inout] rect The rectangle describing the crop target area
> + * \brief Set a selection rectangle \a rect for \a target
> + * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
> + * \param[inout] rect The selection rectangle to be applied
>   * \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 = {};

Patch
diff mbox series 

diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
index a0409e59c08f..976ef9b6dc50 100644
--- a/src/libcamera/include/v4l2_videodevice.h
+++ b/src/libcamera/include/v4l2_videodevice.h
@@ -211,8 +211,7 @@  public:
 	int setFormat(V4L2DeviceFormat *format);
 	std::map<V4L2PixelFormat, std::vector<SizeRange>> formats();
 
-	int setCrop(Rectangle *rect);
-	int setCompose(Rectangle *rect);
+	int setSelection(unsigned int target, Rectangle *rect);
 
 	int allocateBuffers(unsigned int count,
 			    std::vector<std::unique_ptr<FrameBuffer>> *buffers);
@@ -254,8 +253,6 @@  private:
 	std::vector<V4L2PixelFormat> enumPixelformats();
 	std::vector<SizeRange> enumSizes(V4L2PixelFormat pixelFormat);
 
-	int setSelection(unsigned int target, Rectangle *rect);
-
 	int requestBuffers(unsigned int count, enum v4l2_memory memoryType);
 	int createBuffers(unsigned int count,
 			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index 8d642be0d05c..b16ab82c2807 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -1108,25 +1108,11 @@  std::vector<SizeRange> V4L2VideoDevice::enumSizes(V4L2PixelFormat pixelFormat)
 }
 
 /**
- * \brief Set a crop rectangle on the V4L2 video device node
- * \param[inout] rect The rectangle describing the crop target area
+ * \brief Set a selection rectangle \a rect for \a target
+ * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
+ * \param[inout] rect The selection rectangle to be applied
  * \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 = {};