[libcamera-devel] [PATCH v2 1/4] libcamera: rkisp1: Generate config using main path

[Paul Elder]

On Wed, Feb 22, 2023 at 04:19:14PM +0100, Jacopo Mondi via libcamera-devel wrote:
> The generateConfiguration() implementation in the Rockchip RkISP1
> pipeline handler uses by default the self path (if available) for
> the Viewfinder and VideoRecording StreamRoles.
> 
> The validate() implementation, at the contrary, prefers using the main
> path, when available, for all streams.
> 
> As the self-path is limited in output resolution to 1920x1920,
> generating a configuration using the self path limits the maximum
> stream size to 1920x1920, while higher resolutions can be obtained by
> using the main path.
> 
> Align the generateConfiguration() implementation to the validate() one
> by using the main path by default if available.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=180
> Reported-by: libcamera at luigi311.com
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 8a30fe061d04..569fb8ecb629 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -630,23 +630,19 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
>  	 * first stream and use it for all streams.
>  	 */
>  	std::optional<ColorSpace> colorSpace;
> -
>  	bool mainPathAvailable = true;
> -	bool selfPathAvailable = data->selfPath_;
>  
>  	for (const StreamRole role : roles) {
> -		bool useMainPath;
> +		bool useMainPath = mainPathAvailable;
>  
>  		switch (role) {
>  		case StreamRole::StillCapture:
> -			useMainPath = mainPathAvailable;
>  			/* JPEG encoders typically expect sYCC. */
>  			if (!colorSpace)
>  				colorSpace = ColorSpace::Sycc;
>  			break;
>  
>  		case StreamRole::Viewfinder:
> -			useMainPath = !selfPathAvailable;
>  			/*
>  			 * sYCC is the YCbCr encoding of sRGB, which is commonly
>  			 * used by displays.
> @@ -656,7 +652,6 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
>  			break;
>  
>  		case StreamRole::VideoRecording:
> -			useMainPath = !selfPathAvailable;
>  			/* Rec. 709 is a good default for HD video recording. */
>  			if (!colorSpace)
>  				colorSpace = ColorSpace::Rec709;
> @@ -669,7 +664,6 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
>  				return nullptr;
>  			}
>  
> -			useMainPath = true;
>  			colorSpace = ColorSpace::Raw;
>  			break;
>  
> @@ -686,7 +680,6 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
>  			mainPathAvailable = false;
>  		} else {
>  			path = data->selfPath_;
> -			selfPathAvailable = false;
>  		}
>  
>  		StreamConfiguration cfg =
> -- 
> 2.39.0
>