[v3,11/17] libcamera: rkisp1: Refactor path validation
diff mbox series

Message ID 20241206101344.767170-12-stefan.klug@ideasonboard.com
State Superseded
Delegated to: Paul Elder
Headers show
Series
  • rkisp1: Fix aspect ratio and ScalerCrop
Related show

Commit Message

Stefan Klug Dec. 6, 2024, 10:13 a.m. UTC
Refactor validation code to prepare for extensions in the upcoming
patches. Code duplication is reduced by moving parts of the validation
logic into a lambda function. This patch does not include any functional
changes.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

---
Changes in v3:
- Added this patch
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 35 +++++++++++++-----------
 1 file changed, 19 insertions(+), 16 deletions(-)

Comments

Paul Elder Dec. 12, 2024, 12:03 p.m. UTC | #1
On Fri, Dec 06, 2024 at 11:13:33AM +0100, Stefan Klug wrote:
> Refactor validation code to prepare for extensions in the upcoming
> patches. Code duplication is reduced by moving parts of the validation
> logic into a lambda function. This patch does not include any functional
> changes.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

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

> 
> ---
> Changes in v3:
> - Added this patch
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 35 +++++++++++++-----------
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 098c560ca5c8..7f41092ee2d5 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -558,50 +558,53 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  	if (config_.size() == 2 && fitsAllPaths(config_[0]))
>  		std::reverse(order.begin(), order.end());
>  
> +	auto validateConfig = [&](StreamConfiguration &cfg, RkISP1Path *path,
> +				  Stream *stream, Status expectedStatus) {
> +		StreamConfiguration tryCfg = cfg;
> +		if (path->validate(sensor, sensorConfig, &tryCfg) != expectedStatus)
> +			return false;
> +
> +		cfg = tryCfg;
> +		cfg.setStream(stream);
> +		return true;
> +	};
> +
>  	bool mainPathAvailable = true;
>  	bool selfPathAvailable = data_->selfPath_;
> +	RkISP1Path *mainPath = data_->mainPath_;
> +	RkISP1Path *selfPath = data_->selfPath_;
> +	Stream *mainPathStream = const_cast<Stream *>(&data_->mainPathStream_);
> +	Stream *selfPathStream = const_cast<Stream *>(&data_->selfPathStream_);
>  	for (unsigned int index : order) {
>  		StreamConfiguration &cfg = config_[index];
>  
>  		/* Try to match stream without adjusting configuration. */
>  		if (mainPathAvailable) {
> -			StreamConfiguration tryCfg = cfg;
> -			if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) {
> +			if (validateConfig(cfg, mainPath, mainPathStream, Valid)) {
>  				mainPathAvailable = false;
> -				cfg = tryCfg;
> -				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
>  				continue;
>  			}
>  		}
>  
>  		if (selfPathAvailable) {
> -			StreamConfiguration tryCfg = cfg;
> -			if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) {
> +			if (validateConfig(cfg, selfPath, selfPathStream, Valid)) {
>  				selfPathAvailable = false;
> -				cfg = tryCfg;
> -				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
>  				continue;
>  			}
>  		}
>  
>  		/* Try to match stream allowing adjusting configuration. */
>  		if (mainPathAvailable) {
> -			StreamConfiguration tryCfg = cfg;
> -			if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) {
> +			if (validateConfig(cfg, mainPath, mainPathStream, Adjusted)) {
>  				mainPathAvailable = false;
> -				cfg = tryCfg;
> -				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
>  				status = Adjusted;
>  				continue;
>  			}
>  		}
>  
>  		if (selfPathAvailable) {
> -			StreamConfiguration tryCfg = cfg;
> -			if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) {
> +			if (validateConfig(cfg, selfPath, selfPathStream, Adjusted)) {
>  				selfPathAvailable = false;
> -				cfg = tryCfg;
> -				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
>  				status = Adjusted;
>  				continue;
>  			}
> -- 
> 2.43.0
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 098c560ca5c8..7f41092ee2d5 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -558,50 +558,53 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 	if (config_.size() == 2 && fitsAllPaths(config_[0]))
 		std::reverse(order.begin(), order.end());
 
+	auto validateConfig = [&](StreamConfiguration &cfg, RkISP1Path *path,
+				  Stream *stream, Status expectedStatus) {
+		StreamConfiguration tryCfg = cfg;
+		if (path->validate(sensor, sensorConfig, &tryCfg) != expectedStatus)
+			return false;
+
+		cfg = tryCfg;
+		cfg.setStream(stream);
+		return true;
+	};
+
 	bool mainPathAvailable = true;
 	bool selfPathAvailable = data_->selfPath_;
+	RkISP1Path *mainPath = data_->mainPath_;
+	RkISP1Path *selfPath = data_->selfPath_;
+	Stream *mainPathStream = const_cast<Stream *>(&data_->mainPathStream_);
+	Stream *selfPathStream = const_cast<Stream *>(&data_->selfPathStream_);
 	for (unsigned int index : order) {
 		StreamConfiguration &cfg = config_[index];
 
 		/* Try to match stream without adjusting configuration. */
 		if (mainPathAvailable) {
-			StreamConfiguration tryCfg = cfg;
-			if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) {
+			if (validateConfig(cfg, mainPath, mainPathStream, Valid)) {
 				mainPathAvailable = false;
-				cfg = tryCfg;
-				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
 				continue;
 			}
 		}
 
 		if (selfPathAvailable) {
-			StreamConfiguration tryCfg = cfg;
-			if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) {
+			if (validateConfig(cfg, selfPath, selfPathStream, Valid)) {
 				selfPathAvailable = false;
-				cfg = tryCfg;
-				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
 				continue;
 			}
 		}
 
 		/* Try to match stream allowing adjusting configuration. */
 		if (mainPathAvailable) {
-			StreamConfiguration tryCfg = cfg;
-			if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) {
+			if (validateConfig(cfg, mainPath, mainPathStream, Adjusted)) {
 				mainPathAvailable = false;
-				cfg = tryCfg;
-				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
 				status = Adjusted;
 				continue;
 			}
 		}
 
 		if (selfPathAvailable) {
-			StreamConfiguration tryCfg = cfg;
-			if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) {
+			if (validateConfig(cfg, selfPath, selfPathStream, Adjusted)) {
 				selfPathAvailable = false;
-				cfg = tryCfg;
-				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
 				status = Adjusted;
 				continue;
 			}