[libcamera-devel,2/3] libcamera: ipu3: Accept an empty roles list

8475 diff mbox series

The IPU3 pipeline handler that does not support receiving an empty list
of roles at generateConfiguration() time. This contradicts the camera
API which allows application to generate empty CameraConfiguration to
be later manually filled.

Fix this by returning an empty CameraConfiguration if the list of
requested roles is empty. While at it, align the style with the other
pipeline handlers.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Hi Jacopo,

Thanks for your work.

On 2020-06-28 17:55:38 +0200, Jacopo Mondi wrote:
> The IPU3 pipeline handler that does not support receiving an empty list
> of roles at generateConfiguration() time. This contradicts the camera
> API which allows application to generate empty CameraConfiguration to
> be later manually filled.
> 
> Fix this by returning an empty CameraConfiguration if the list of
> requested roles is empty. While at it, align the style with the other
> pipeline handlers.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

I'm not sure this is the way we wish this API to work in the end. But a 
good start is that all pipelines do the same thing :-)

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index ef57196c32da..cbf19793c43e 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -292,14 +292,15 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	IPU3CameraConfiguration *config;
> +	IPU3CameraConfiguration *config = new IPU3CameraConfiguration(camera, data);
>  	std::set<Stream *> streams = {
>  		&data->outStream_,
>  		&data->vfStream_,
>  		&data->rawStream_,
>  	};
>  
> -	config = new IPU3CameraConfiguration(camera, data);
> +	if (roles.empty())
> +		return config;
>  
>  	for (const StreamRole role : roles) {
>  		StreamConfiguration cfg = {};
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Jacopo,

Thank you for the patch.

On Sun, Jun 28, 2020 at 05:55:38PM +0200, Jacopo Mondi wrote:
> The IPU3 pipeline handler that does not support receiving an empty list
> of roles at generateConfiguration() time. This contradicts the camera
> API which allows application to generate empty CameraConfiguration to
> be later manually filled.
> 
> Fix this by returning an empty CameraConfiguration if the list of
> requested roles is empty. While at it, align the style with the other
> pipeline handlers.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index ef57196c32da..cbf19793c43e 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -292,14 +292,15 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	IPU3CameraConfiguration *config;
> +	IPU3CameraConfiguration *config = new IPU3CameraConfiguration(camera, data);

I wouldn't have changed this, up to you.

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

>  	std::set<Stream *> streams = {
>  		&data->outStream_,
>  		&data->vfStream_,
>  		&data->rawStream_,
>  	};
>  
> -	config = new IPU3CameraConfiguration(camera, data);
> +	if (roles.empty())
> +		return config;
>  
>  	for (const StreamRole role : roles) {
>  		StreamConfiguration cfg = {};