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

[Laurent Pinchart]

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 at 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;
>  }

-- 
Regards,

Laurent Pinchart