[libcamera-devel,17/17] libcamera: pipeline: uvcvideo: Add format information and validation

1310 diff mbox series

Extend the uvcvideo pipeline with format information and validation. The
format information is gathered by enumerating the v4l2 device. This
enumeration approach is valid for UVC as it has a static and simple
media graph.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/pipeline/uvcvideo.cpp | 46 +++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 9 deletions(-)

Hi Niklas,

Thank you for the patch.

On Mon, May 27, 2019 at 02:15:43AM +0200, Niklas Söderlund wrote:
> Extend the uvcvideo pipeline with format information and validation. The
> format information is gathered by enumerating the v4l2 device. This
> enumeration approach is valid for UVC as it has a static and simple
> media graph.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/pipeline/uvcvideo.cpp | 46 +++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 45260f34c8f5daba..ce9f281891a8bc5e 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -95,18 +95,42 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()
>  	}
>  
>  	StreamConfiguration &cfg = config_[0];
> +	const StreamFormats &formats = cfg.formats();
>  

You can remove this blank line.

> -	/* \todo: Validate the configuration against the device capabilities. */
>  	const unsigned int pixelFormat = cfg.pixelFormat;
>  	const Size size = cfg.size;
>  
> -	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> -	cfg.size = { 640, 480 };
> +	bool found = false;
> +	for (unsigned int pixfmt : formats.pixelformats()) {
> +		if (pixfmt == pixelFormat) {
> +			found = true;
> +			break;
> +		}
> +	}
> +	if (!found)
> +		cfg.pixelFormat = formats.pixelformats().front();

	auto iter = std::find(formats.begin(), formats.end(), pixelFormat);
	if (iter == formats.end()) {
		cfg.pixelFormat = formats.pixelformats().front();
		LOG(UVC, Debug)
			<< "Adjusting pixel format from " << pixelFormat
			<< " to " << cfg.pixelFormat;
		status = Adjusted;
	}

>  
> -	if (cfg.pixelFormat != pixelFormat || cfg.size != size) {
> +	if (cfg.pixelFormat != pixelFormat) {
>  		LOG(UVC, Debug)
> -			<< "Adjusting configuration from " << cfg.toString()
> -			<< " to " << cfg.size.toString() << "-YUYV";
> +			<< "Adjusting pixel format from " << pixelFormat
> +			<< " to " << cfg.pixelFormat;
> +		status = Adjusted;
> +	}
> +
> +	const std::vector<Size> &formatSizes = formats.sizes(cfg.pixelFormat);
> +	cfg.size = formatSizes.front();
> +	for (const Size &formatsSize : formatSizes) {
> +		if (formatsSize <= size)
> +			cfg.size = formatsSize;
> +
> +		if (formatsSize == size)

As the sizes are sorted, should this be >= ?

I really like this patch. Once the series stabilises, I think we should
do the same for all the other pipeline handlers, and always provide
stream formats when constructing the configuration. The stream
configuration constructor that takes no format should then be removed.

> +			break;
> +	}
> +
> +	if (cfg.size != size) {
> +		LOG(UVC, Debug)
> +			<< "Adjusting size from " << size.toString()
> +			<< " to " << cfg.size.toString();
>  		status = Adjusted;
>  	}
>  
> @@ -123,14 +147,18 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
>  CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
> +	UVCCameraData *data = cameraData(camera);
>  	CameraConfiguration *config = new UVCCameraConfiguration();
>  
>  	if (roles.empty())
>  		return config;
>  
> -	StreamConfiguration cfg{};
> -	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> -	cfg.size = { 640, 480 };
> +	V4L2DeviceFormats v4l2formats = data->video_->formats();
> +	StreamFormats formats(v4l2formats.data());
> +	StreamConfiguration cfg(formats);
> +
> +	cfg.pixelFormat = formats.pixelformats().front();
> +	cfg.size = formats.sizes(cfg.pixelFormat).back();
>  	cfg.bufferCount = 4;
>  
>  	config->addConfiguration(cfg);