[libcamera-devel,v4,18/21] libcamera: uvcvideo: Fill stride and frameSize at config validation
diff mbox series

Message ID 20200708134417.67747-19-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • Clean up formats in v4l2-compat and pipeline handlers
Related show

Commit Message

Paul Elder July 8, 2020, 1:44 p.m. UTC 
Fill the stride and frameSize fields of the StreamConfiguration at
configuration validation time instead of at camera configuration time.
This allows applications to get the stride when trying a configuration
without modifying the active configuration of the camera.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v4:
- return error if tryFormat fails
- pass UVCCameraData to UVCCameraConfiguration, instead of
  V4l2VideoDevice
- mention motivation in commit message

New in v3
---
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 23 +++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

Comments

Laurent Pinchart July 8, 2020, 3:29 p.m. UTC | #1 
Hi Paul,

Thank you for the patch.

On Wed, Jul 08, 2020 at 10:44:14PM +0900, Paul Elder wrote:
> Fill the stride and frameSize fields of the StreamConfiguration at
> configuration validation time instead of at camera configuration time.
> This allows applications to get the stride when trying a configuration
> without modifying the active configuration of the camera.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

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

> ---
> Changes in v4:
> - return error if tryFormat fails
> - pass UVCCameraData to UVCCameraConfiguration, instead of
>   V4l2VideoDevice
> - mention motivation in commit message
> 
> New in v3
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 23 +++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 9db69dd..47f383d 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -53,9 +53,12 @@ public:
>  class UVCCameraConfiguration : public CameraConfiguration
>  {
>  public:
> -	UVCCameraConfiguration();
> +	UVCCameraConfiguration(UVCCameraData *data);
>  
>  	Status validate() override;
> +
> +private:
> +	UVCCameraData *data_;
>  };
>  
>  class PipelineHandlerUVC : public PipelineHandler
> @@ -89,8 +92,8 @@ private:
>  	}
>  };
>  
> -UVCCameraConfiguration::UVCCameraConfiguration()
> -	: CameraConfiguration()
> +UVCCameraConfiguration::UVCCameraConfiguration(UVCCameraData *data)
> +	: CameraConfiguration(), data_(data)
>  {
>  }
>  
> @@ -141,6 +144,17 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
>  
>  	cfg.bufferCount = 4;
>  
> +	V4L2DeviceFormat format = {};
> +	format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
> +	format.size = cfg.size;
> +
> +	int ret = data_->video_->tryFormat(&format);
> +	if (ret)
> +		return Invalid;
> +
> +	cfg.stride = format.planes[0].bpl;
> +	cfg.frameSize = format.planes[0].size;
> +
>  	return status;
>  }
>  
> @@ -153,7 +167,7 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
>  	UVCCameraData *data = cameraData(camera);
> -	CameraConfiguration *config = new UVCCameraConfiguration();
> +	CameraConfiguration *config = new UVCCameraConfiguration(data);
>  
>  	if (roles.empty())
>  		return config;
> @@ -200,7 +214,6 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
>  		return -EINVAL;
>  
>  	cfg.setStream(&data->stream_);
> -	cfg.stride = format.planes[0].bpl;
>  
>  	return 0;
>  }
Jacopo Mondi July 8, 2020, 9:36 p.m. UTC | #2 
Hi Paul,

On Wed, Jul 08, 2020 at 10:44:14PM +0900, Paul Elder wrote:
> Fill the stride and frameSize fields of the StreamConfiguration at
> configuration validation time instead of at camera configuration time.
> This allows applications to get the stride when trying a configuration
> without modifying the active configuration of the camera.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
>

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

Thanks
  j

> ---
> Changes in v4:
> - return error if tryFormat fails
> - pass UVCCameraData to UVCCameraConfiguration, instead of
>   V4l2VideoDevice
> - mention motivation in commit message
>
> New in v3
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 23 +++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 9db69dd..47f383d 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -53,9 +53,12 @@ public:
>  class UVCCameraConfiguration : public CameraConfiguration
>  {
>  public:
> -	UVCCameraConfiguration();
> +	UVCCameraConfiguration(UVCCameraData *data);
>
>  	Status validate() override;
> +
> +private:
> +	UVCCameraData *data_;
>  };
>
>  class PipelineHandlerUVC : public PipelineHandler
> @@ -89,8 +92,8 @@ private:
>  	}
>  };
>
> -UVCCameraConfiguration::UVCCameraConfiguration()
> -	: CameraConfiguration()
> +UVCCameraConfiguration::UVCCameraConfiguration(UVCCameraData *data)
> +	: CameraConfiguration(), data_(data)
>  {
>  }
>
> @@ -141,6 +144,17 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
>
>  	cfg.bufferCount = 4;
>
> +	V4L2DeviceFormat format = {};
> +	format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
> +	format.size = cfg.size;
> +
> +	int ret = data_->video_->tryFormat(&format);
> +	if (ret)
> +		return Invalid;
> +
> +	cfg.stride = format.planes[0].bpl;
> +	cfg.frameSize = format.planes[0].size;
> +
>  	return status;
>  }
>
> @@ -153,7 +167,7 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
>  	UVCCameraData *data = cameraData(camera);
> -	CameraConfiguration *config = new UVCCameraConfiguration();
> +	CameraConfiguration *config = new UVCCameraConfiguration(data);
>
>  	if (roles.empty())
>  		return config;
> @@ -200,7 +214,6 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
>  		return -EINVAL;
>
>  	cfg.setStream(&data->stream_);
> -	cfg.stride = format.planes[0].bpl;
>
>  	return 0;
>  }
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index 9db69dd..47f383d 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -53,9 +53,12 @@  public:
 class UVCCameraConfiguration : public CameraConfiguration
 {
 public:
-	UVCCameraConfiguration();
+	UVCCameraConfiguration(UVCCameraData *data);
 
 	Status validate() override;
+
+private:
+	UVCCameraData *data_;
 };
 
 class PipelineHandlerUVC : public PipelineHandler
@@ -89,8 +92,8 @@  private:
 	}
 };
 
-UVCCameraConfiguration::UVCCameraConfiguration()
-	: CameraConfiguration()
+UVCCameraConfiguration::UVCCameraConfiguration(UVCCameraData *data)
+	: CameraConfiguration(), data_(data)
 {
 }
 
@@ -141,6 +144,17 @@  CameraConfiguration::Status UVCCameraConfiguration::validate()
 
 	cfg.bufferCount = 4;
 
+	V4L2DeviceFormat format = {};
+	format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
+	format.size = cfg.size;
+
+	int ret = data_->video_->tryFormat(&format);
+	if (ret)
+		return Invalid;
+
+	cfg.stride = format.planes[0].bpl;
+	cfg.frameSize = format.planes[0].size;
+
 	return status;
 }
 
@@ -153,7 +167,7 @@  CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
 	const StreamRoles &roles)
 {
 	UVCCameraData *data = cameraData(camera);
-	CameraConfiguration *config = new UVCCameraConfiguration();
+	CameraConfiguration *config = new UVCCameraConfiguration(data);
 
 	if (roles.empty())
 		return config;
@@ -200,7 +214,6 @@  int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
 		return -EINVAL;
 
 	cfg.setStream(&data->stream_);
-	cfg.stride = format.planes[0].bpl;
 
 	return 0;
 }