[libcamera-devel] [PATCH v3 3/4] libcamera: rkisp1: Reserve main path for StillCapture

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Mar 07, 2023 at 12:48:03PM +0100, Jacopo Mondi via libcamera-devel wrote:
> The main output path can produce images in higher resolution and
> should be reserved for the StillCapture role when a configuration
> is generated.
> 
> Before this change if StillCapture was not requested first it got
> assigned to the self-path and thus down-scaled to 1920x1920.
> 
> With this change, StillCapture can be asked last and it would take
> precedence over other streams for the usage of the main path.

The order in which roles are requested matters, applications must
specify them in decreasing order of priority. Why is it better to
priority the still capture role even if specified last ?

> $ cam -c1 --stream role=viewfinder --stream role=still
> Camera camera.cpp:969 streams configuration: (0) 1920x1080-NV12 (1) 4208x3120-NV12
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 60ab7380034c..cd92e99a50b3 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -643,6 +643,10 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
>  	if (roles.empty())
>  		return config;
>  
> +	/* If still capture is requested, reserve the main path for it. */
> +	bool reserveMainPath = std::find(roles.cbegin(), roles.cend(),
> +					 StreamRole::StillCapture) != roles.cend();
> +
>  	/*
>  	 * As the ISP can't output different color spaces for the main and self
>  	 * path, pick a sensible default color space based on the role of the
> @@ -661,6 +665,9 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
>  			if (!colorSpace)
>  				colorSpace = ColorSpace::Sycc;
>  
> +			/* Unlock usage of main path which was reserved. */
> +			reserveMainPath = false;
> +
>  			size = data->sensor_->resolution();
>  			break;
>  
> @@ -702,7 +709,7 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
>  
>  		RkISP1Path *path;
>  
> -		if (useMainPath) {
> +		if (useMainPath && !reserveMainPath) {
>  			path = data->mainPath_;
>  			mainPathAvailable = false;
>  		} else {

-- 
Regards,

Laurent Pinchart