[v2,25/35] pipeline: rkisp1: Drop rawFormat variable
diff mbox series

Message ID 20251023144841.403689-26-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Full dewarper support on imx8mp
Related show

Commit Message

Stefan Klug Oct. 23, 2025, 2:48 p.m. UTC
In raw mode we know there is only a single configuration so there
is no need to iterate over all configurations to find the format.
Drop that.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Comments

Umang Jain Oct. 25, 2025, 12:59 p.m. UTC | #1
On Thu, Oct 23, 2025 at 04:48:26PM +0200, Stefan Klug wrote:
> In raw mode we know there is only a single configuration so there
> is no need to iterate over all configurations to find the format.
> Drop that.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

Reviewed-by: Umang Jain <uajain@igalia.com>

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 35cb54c812bc..d7bb30f20668 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -677,21 +677,16 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  	}
>  
>  	/* Select the sensor format. */
> -	PixelFormat rawFormat;
>  	Size maxSize;
>  
>  	for (const StreamConfiguration &cfg : config_) {
> -		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> -		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> -			rawFormat = cfg.pixelFormat;
> -
>  		maxSize = std::max(maxSize, cfg.size);
>  	}
>  
>  	std::vector<unsigned int> mbusCodes;
>  
> -	if (rawFormat.isValid()) {
> -		mbusCodes = { rawFormats.at(rawFormat) };
> +	if (isRaw) {
> +		mbusCodes = { rawFormats.at(config_[0].pixelFormat) };
>  	} else {
>  		std::transform(rawFormats.begin(), rawFormats.end(),
>  			       std::back_inserter(mbusCodes),
> -- 
> 2.48.1
>
Pőcze Barnabás Oct. 27, 2025, 3:06 p.m. UTC | #2
2025. 10. 23. 16:48 keltezéssel, Stefan Klug írta:
> In raw mode we know there is only a single configuration so there
> is no need to iterate over all configurations to find the format.
> Drop that.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Although I'm wondering if you might want to keep `rawFormat`
and remove `isRaw` instead.


>   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++-------
>   1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 35cb54c812bc..d7bb30f20668 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -677,21 +677,16 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>   	}
> 
>   	/* Select the sensor format. */
> -	PixelFormat rawFormat;
>   	Size maxSize;
> 
>   	for (const StreamConfiguration &cfg : config_) {
> -		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> -		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> -			rawFormat = cfg.pixelFormat;
> -
>   		maxSize = std::max(maxSize, cfg.size);
>   	}
> 
>   	std::vector<unsigned int> mbusCodes;
> 
> -	if (rawFormat.isValid()) {
> -		mbusCodes = { rawFormats.at(rawFormat) };
> +	if (isRaw) {
> +		mbusCodes = { rawFormats.at(config_[0].pixelFormat) };
>   	} else {
>   		std::transform(rawFormats.begin(), rawFormats.end(),
>   			       std::back_inserter(mbusCodes),
> --
> 2.48.1
>
Paul Elder Nov. 6, 2025, 11:45 a.m. UTC | #3
Quoting Stefan Klug (2025-10-23 23:48:26)
> In raw mode we know there is only a single configuration so there
> is no need to iterate over all configurations to find the format.
> Drop that.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

Looks good to me.

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

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 35cb54c812bc..d7bb30f20668 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -677,21 +677,16 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>         }
>  
>         /* Select the sensor format. */
> -       PixelFormat rawFormat;
>         Size maxSize;
>  
>         for (const StreamConfiguration &cfg : config_) {
> -               const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> -               if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
> -                       rawFormat = cfg.pixelFormat;
> -
>                 maxSize = std::max(maxSize, cfg.size);
>         }
>  
>         std::vector<unsigned int> mbusCodes;
>  
> -       if (rawFormat.isValid()) {
> -               mbusCodes = { rawFormats.at(rawFormat) };
> +       if (isRaw) {
> +               mbusCodes = { rawFormats.at(config_[0].pixelFormat) };
>         } else {
>                 std::transform(rawFormats.begin(), rawFormats.end(),
>                                std::back_inserter(mbusCodes),
> -- 
> 2.48.1
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 35cb54c812bc..d7bb30f20668 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -677,21 +677,16 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 	}
 
 	/* Select the sensor format. */
-	PixelFormat rawFormat;
 	Size maxSize;
 
 	for (const StreamConfiguration &cfg : config_) {
-		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
-		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)
-			rawFormat = cfg.pixelFormat;
-
 		maxSize = std::max(maxSize, cfg.size);
 	}
 
 	std::vector<unsigned int> mbusCodes;
 
-	if (rawFormat.isValid()) {
-		mbusCodes = { rawFormats.at(rawFormat) };
+	if (isRaw) {
+		mbusCodes = { rawFormats.at(config_[0].pixelFormat) };
 	} else {
 		std::transform(rawFormats.begin(), rawFormats.end(),
 			       std::back_inserter(mbusCodes),