[libcamera-devel] [PATCH 3/3] libcamera: pipelines: Fail if more than one role is requested

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Sun, Jun 28, 2020 at 06:17:23PM +0200, Jacopo Mondi wrote:
> The rkisp1, vimc, uvc and simple pipeline handlers only support a single
> stream.
> 
> Their generateConfiguration() implementations does not check how many
> roles have been requested, silently ignoring requests for more than one
> role that cannot be satisfied.
> 
> Fix this and comply to the documented Camera API by failing if more than one
> role is requested by applications.

I don't think this is needed, Camera::generateConfiguration() already
has

	if (roles.size() > streams().size())
		return nullptr;

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 5 +++++
>  src/libcamera/pipeline/simple/simple.cpp     | 5 +++++
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 5 +++++
>  src/libcamera/pipeline/vimc/vimc.cpp         | 5 +++++
>  4 files changed, 20 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 3c01821135f8..6d5c7c39edf1 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -566,6 +566,11 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera
>  	if (roles.empty())
>  		return config;
>  
> +	if (roles.size() > 1) {
> +		delete config;
> +		return nullptr;
> +	}
> +
>  	StreamConfiguration cfg{};
>  	cfg.pixelFormat = formats::NV12;
>  	cfg.size = data->sensor_->resolution();
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 1ec8d0f7de03..20675de568a8 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -484,6 +484,11 @@ CameraConfiguration *SimplePipelineHandler::generateConfiguration(Camera *camera
>  	if (roles.empty())
>  		return config;
>  
> +	if (roles.size() > 1) {
> +		delete config;
> +		return nullptr;
> +	}
> +
>  	/* Create the formats map. */
>  	std::map<PixelFormat, std::vector<SizeRange>> formats;
>  	std::transform(data->formats_.begin(), data->formats_.end(),
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 80a0e77ba3fc..773480e0a821 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -158,6 +158,11 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
>  	if (roles.empty())
>  		return config;
>  
> +	if (roles.size() > 1) {
> +		delete config;
> +		return nullptr;
> +	}
> +
>  	std::map<V4L2PixelFormat, std::vector<SizeRange>> v4l2Formats =
>  		data->video_->formats();
>  	std::map<PixelFormat, std::vector<SizeRange>> deviceFormats;
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index b6530662a9ba..e0980fc4a64c 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -177,6 +177,11 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
>  	if (roles.empty())
>  		return config;
>  
> +	if (roles.size() > 1) {
> +		delete config;
> +		return nullptr;
> +	}
> +
>  	std::map<PixelFormat, std::vector<SizeRange>> formats;
>  
>  	for (const auto &pixelformat : pixelformats) {

-- 
Regards,

Laurent Pinchart