[libcamera-devel,v3,20/22] libcamera: uvcvideo: Fill stride and frameSize at config validation

8641 diff mbox series

Fill the stride and frameSize fields of the StreamConfiguration at
configuration validation time instead of at camera configuration time.

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

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

Hi Paul,

Thank you for the patch.

On Sat, Jul 04, 2020 at 10:31:38PM +0900, Paul Elder wrote:
> Fill the stride and frameSize fields of the StreamConfiguration at
> configuration validation time instead of at camera configuration time.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> New in v3
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 31 ++++++++++++++++----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index a5feab6..f8d258e 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(V4L2VideoDevice *video);

I'd rather pass a UVCCameraData instead, to make it more generic.

>  
>  	Status validate() override;
> +
> +private:
> +	V4L2VideoDevice *video_;
>  };
>  
>  class PipelineHandlerUVC : public PipelineHandler
> @@ -89,8 +92,8 @@ private:
>  	}
>  };
>  
> -UVCCameraConfiguration::UVCCameraConfiguration()
> -	: CameraConfiguration()
> +UVCCameraConfiguration::UVCCameraConfiguration(V4L2VideoDevice *video)
> +	: CameraConfiguration(), video_(video)
>  {
>  }
>  
> @@ -141,6 +144,25 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
>  
>  	cfg.bufferCount = 4;
>  
> +	V4L2DeviceFormat format = {};
> +	format.fourcc = video_->toV4L2PixelFormat(cfg.pixelFormat);
> +	format.size = cfg.size;
> +
> +	int ret = video_->tryFormat(&format);
> +	if (ret) {
> +		LOG(UVC, Warning)
> +			<< "try_fmt failed, "
> +			<< "getting stride and frameSize from PixelFormatInfo";
> +
> +		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> +		cfg.stride = info.stride(cfg.size.width, 0);
> +		cfg.frameSize = info.frameSize(cfg.size);
> +		return status;

Same as for the previous patch, I think you can just return an error. I
would either drop the warning message (tryFormat() prints a message on
failure, doesn't it ?), or also add it in the previous patch.

> +	}
> +
> +	cfg.stride = format.planes[0].bpl;
> +	cfg.frameSize = format.planes[0].size;
> +
>  	return status;
>  }
>  
> @@ -153,7 +175,7 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
>  	UVCCameraData *data = cameraData(camera);
> -	CameraConfiguration *config = new UVCCameraConfiguration();
> +	CameraConfiguration *config = new UVCCameraConfiguration(data->video_);
>  
>  	if (roles.empty())
>  		return config;
> @@ -200,7 +222,6 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
>  		return -EINVAL;
>  
>  	cfg.setStream(&data->stream_);
> -	cfg.stride = format.planes[0].bpl;
>  
>  	return 0;
>  }