[libcamera-devel] [PATCH 1/3] libcamera: raspberrypi: Refuse invalid roles configuration

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Sun, Jun 28, 2020 at 06:17:21PM +0200, Jacopo Mondi wrote:
> The generateConfiguration() implementation does not check if the
> requested list of roles can actually be satisfied. The camera API
> documentation prescribes the function shall fail in that case, instead
> of silently adjust the returned confiuguration.

s/confiuguration/configuration/

> Fix this by implementing the same logic as the validate() function
> implements, as the pipeline handler supports one raw stream and up to
> two output streams.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index dcd737a1d1a0..d1338b640e3c 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -526,6 +526,8 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  	if (roles.empty())
>  		return config;
>  
> +	unsigned int rawCount = 0;
> +	unsigned int outCount = 0;
>  	for (const StreamRole role : roles) {
>  		switch (role) {
>  		case StreamRole::StillCaptureRaw:
> @@ -535,6 +537,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  			pixelFormat = sensorFormat.fourcc.toPixelFormat();
>  			ASSERT(pixelFormat.isValid());
>  			bufferCount = 1;
> +			rawCount++;
>  			break;
>  
>  		case StreamRole::StillCapture:
> @@ -543,6 +546,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  			/* Return the largest sensor resolution. */
>  			size = data->sensor_->resolution();
>  			bufferCount = 1;
> +			outCount++;
>  			break;
>  
>  		case StreamRole::VideoRecording:
> @@ -550,6 +554,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  			pixelFormat = formats::NV12;
>  			size = { 1920, 1080 };
>  			bufferCount = 4;
> +			outCount++;
>  			break;
>  
>  		case StreamRole::Viewfinder:
> @@ -557,6 +562,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  			pixelFormat = formats::ARGB8888;
>  			size = { 800, 600 };
>  			bufferCount = 4;
> +			outCount++;
>  			break;
>  
>  		default:
> @@ -565,6 +571,11 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>  			break;
>  		}
>  
> +		if (rawCount > 1 || outCount > 2) {

Would an error message be a good idea ?

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

> +			delete config;
> +			return nullptr;
> +		}
> +
>  		/* Translate the V4L2PixelFormat to PixelFormat. */
>  		std::map<PixelFormat, std::vector<SizeRange>> deviceFormats;
>  		std::transform(fmts.begin(), fmts.end(), std::inserter(deviceFormats, deviceFormats.end()),

-- 
Regards,

Laurent Pinchart