[2/3] libcamera: rkisp1: Rectify SensorConfiguration check

21588 diff mbox series

The 'found' flag was mistakenly understood that a compatible sensor
format has been found when a sensor configuration is passed in. However,
'found' related to the stream configuration's pixelformat, whether it is
supported by the RkISP1Path video node or not. It does not relate to the
sensor format, hence the check:

	if (sensorConfig && !found)

doesn't make sense.

Rectify the above check with:

	if (sensorConfig && !rawFormat.isValid())

to ensure a sensor format compatible with sensor configuration has been
set to rawFormat.

Fixes: 047d647452c4 ("libcamera: rkisp1: Integrate SensorConfiguration support")
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoting Umang Jain (2024-10-11 10:22:21)
> The 'found' flag was mistakenly understood that a compatible sensor
> format has been found when a sensor configuration is passed in. However,
> 'found' related to the stream configuration's pixelformat, whether it is
> supported by the RkISP1Path video node or not. It does not relate to the
> sensor format, hence the check:
> 
>         if (sensorConfig && !found)
> 
> doesn't make sense.
> 
> Rectify the above check with:
> 
>         if (sensorConfig && !rawFormat.isValid())
> 
> to ensure a sensor format compatible with sensor configuration has been
> set to rawFormat.
> 
> Fixes: 047d647452c4 ("libcamera: rkisp1: Integrate SensorConfiguration support")
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index 4a3b779c..236d05af 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -304,7 +304,7 @@ RkISP1Path::validate(const CameraSensor *sensor,
>                 }
>         }
>  
> -       if (sensorConfig && !found)
> +       if (sensorConfig && !rawFormat.isValid())
>                 return CameraConfiguration::Invalid;
>  
>         bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding ==
> -- 
> 2.45.2
>

Hi Umang,

Thank you for the patch.

On Fri, Oct 11, 2024 at 02:52:21PM +0530, Umang Jain wrote:
> The 'found' flag was mistakenly understood that a compatible sensor
> format has been found when a sensor configuration is passed in. However,
> 'found' related to the stream configuration's pixelformat, whether it is
> supported by the RkISP1Path video node or not. It does not relate to the
> sensor format, hence the check:
> 
> 	if (sensorConfig && !found)
> 
> doesn't make sense.
> 
> Rectify the above check with:
> 
> 	if (sensorConfig && !rawFormat.isValid())
> 
> to ensure a sensor format compatible with sensor configuration has been
> set to rawFormat.
> 
> Fixes: 047d647452c4 ("libcamera: rkisp1: Integrate SensorConfiguration support")
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index 4a3b779c..236d05af 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -304,7 +304,7 @@ RkISP1Path::validate(const CameraSensor *sensor,
>  		}
>  	}
>  
> -	if (sensorConfig && !found)
> +	if (sensorConfig && !rawFormat.isValid())
>  		return CameraConfiguration::Invalid;
>  
>  	bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding ==
> -- 
> 2.45.2
>