[libcamera-devel] [PATCH v2.1] ipa: raspberrypi: Use direct return value for configure()

Paul Elder paul.elder at ideasonboard.com
Tue Mar 9 03:39:07 CET 2021


Now that we support returning int directly in addition to other output
parameters, improve the configure() function in the raspberrypi IPA
interface.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

---
Changes in v2.1:
- remove init() changes, keep configure changes
---
 include/libcamera/ipa/raspberrypi.mojom       |  2 +-
 src/ipa/raspberrypi/raspberrypi.cpp           | 34 ++++++++-----------
 .../pipeline/raspberrypi/raspberrypi.cpp      |  5 ++-
 3 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
index c3d614b8..eb427697 100644
--- a/include/libcamera/ipa/raspberrypi.mojom
+++ b/include/libcamera/ipa/raspberrypi.mojom
@@ -77,7 +77,7 @@ interface IPARPiInterface {
 		  map<uint32, IPAStream> streamConfig,
 		  map<uint32, ControlInfoMap> entityControls,
 		  ConfigInput ipaConfig)
-		=> (ConfigOutput results, int32 ret);
+		=> (int32 ret, ConfigOutput results);
 
 	/**
 	 * \fn mapBuffers()
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 85a2b846..7904225a 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -84,11 +84,11 @@ public:
 		   ipa::RPi::StartControls *result) override;
 	void stop() override {}
 
-	void configure(const CameraSensorInfo &sensorInfo,
-		       const std::map<unsigned int, IPAStream> &streamConfig,
-		       const std::map<unsigned int, ControlInfoMap> &entityControls,
-		       const ipa::RPi::ConfigInput &data,
-		       ipa::RPi::ConfigOutput *response, int32_t *ret) override;
+	int configure(const CameraSensorInfo &sensorInfo,
+		      const std::map<unsigned int, IPAStream> &streamConfig,
+		      const std::map<unsigned int, ControlInfoMap> &entityControls,
+		      const ipa::RPi::ConfigInput &data,
+		      ipa::RPi::ConfigOutput *response) override;
 	void mapBuffers(const std::vector<IPABuffer> &buffers) override;
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
 	void signalStatReady(const uint32_t bufferId) override;
@@ -290,16 +290,15 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
 	mode_.max_frame_length = sensorInfo.maxFrameLength;
 }
 
-void IPARPi::configure(const CameraSensorInfo &sensorInfo,
-		       [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
-		       const std::map<unsigned int, ControlInfoMap> &entityControls,
-		       const ipa::RPi::ConfigInput &ipaConfig,
-		       ipa::RPi::ConfigOutput *result, int32_t *ret)
+int IPARPi::configure(const CameraSensorInfo &sensorInfo,
+		      [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
+		      const std::map<unsigned int, ControlInfoMap> &entityControls,
+		      const ipa::RPi::ConfigInput &ipaConfig,
+		      ipa::RPi::ConfigOutput *result)
 {
 	if (entityControls.size() != 2) {
 		LOG(IPARPI, Error) << "No ISP or sensor controls found.";
-		*ret = -1;
-		return;
+		return -1;
 	}
 
 	result->params = 0;
@@ -309,14 +308,12 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 
 	if (!validateSensorControls()) {
 		LOG(IPARPI, Error) << "Sensor control validation failed.";
-		*ret = -1;
-		return;
+		return -1;
 	}
 
 	if (!validateIspControls()) {
 		LOG(IPARPI, Error) << "ISP control validation failed.";
-		*ret = -1;
-		return;
+		return -1;
 	}
 
 	/* Setup a metadata ControlList to output metadata. */
@@ -334,8 +331,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		if (!helper_) {
 			LOG(IPARPI, Error) << "Could not create camera helper for "
 					   << cameraName;
-			*ret = -1;
-			return;
+			return -1;
 		}
 
 		/*
@@ -403,7 +399,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 
 	lastMode_ = mode_;
 
-	*ret = 0;
+	return 0;
 }
 
 void IPARPi::mapBuffers(const std::vector<IPABuffer> &buffers)
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 6387fae5..0f01ce8f 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -1250,9 +1250,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
 	/* Ready the IPA - it must know about the sensor resolution. */
 	ipa::RPi::ConfigOutput result;
 
-	ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
-			&result, &ret);
-
+	ret = ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
+			      &result);
 	if (ret < 0) {
 		LOG(RPI, Error) << "IPA configuration failed!";
 		return -EPIPE;
-- 
2.27.0



More information about the libcamera-devel mailing list