[libcamera-devel] [PATCH] pipeline: ipa: raspberrypi: Various fixups after IPAInterface changes

Naushir Patuck naush at raspberrypi.com
Wed Feb 17 09:02:41 CET 2021


This commit addresses a few fixes and tidy-ups after the IPAInterface
rework:
- Rename ConfigStaggeredWrite -> ConfigSensorParams
- Rename setIsp -> setIspControls
- Signal handlers statsMetadataComplete(), runISP(), embeddedComplete()
should only run when state != Stopped. This matches the behavior of
the original code. Without this, start/stop cycles may cause errors.
- In setIspControls(), only update the LS config (with the fd) when a LS
control is present.

Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
Fixes: e201cb4f5450 ("libcamera: IPAInterface: Replace C API with the new C++-only API")
---
 include/libcamera/ipa/raspberrypi.mojom       |  4 +-
 src/ipa/raspberrypi/raspberrypi.cpp           |  4 +-
 .../pipeline/raspberrypi/raspberrypi.cpp      | 91 ++++++++++---------
 3 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
index bab19a946e18..8a256d022270 100644
--- a/include/libcamera/ipa/raspberrypi.mojom
+++ b/include/libcamera/ipa/raspberrypi.mojom
@@ -16,7 +16,7 @@ enum BufferMask {
 const uint32 MaxLsGridSize = 0x8000;
 
 enum ConfigOutputParameters {
-	ConfigStaggeredWrite = 0x01,
+	ConfigSensorParams = 0x01,
 };
 
 struct SensorConfig {
@@ -126,6 +126,6 @@ interface IPARPiEventInterface {
 	statsMetadataComplete(uint32 bufferId, ControlList controls);
 	runIsp(uint32 bufferId);
 	embeddedComplete(uint32 bufferId);
-	setIsp(ControlList controls);
+	setIspControls(ControlList controls);
 	setDelayedControls(ControlList controls);
 };
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 81a3195c82e9..0bf88bcc5f72 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -344,7 +344,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		helper_->GetDelays(exposureDelay, gainDelay);
 		sensorMetadata = helper_->SensorEmbeddedDataPresent();
 
-		result->params |= ipa::rpi::ConfigStaggeredWrite;
+		result->params |= ipa::rpi::ConfigSensorParams;
 		result->sensorConfig.gainDelay = gainDelay;
 		result->sensorConfig.exposureDelay = exposureDelay;
 		result->sensorConfig.vblank = exposureDelay;
@@ -968,7 +968,7 @@ void IPARPi::prepareISP(unsigned int bufferId)
 			applyDPC(dpcStatus, ctrls);
 
 		if (!ctrls.empty())
-			setIsp.emit(ctrls);
+			setIspControls.emit(ctrls);
 	}
 }
 
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 15aa600ed581..d0ca015a01dc 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -152,7 +152,7 @@ public:
 	void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);
 	void runIsp(uint32_t bufferId);
 	void embeddedComplete(uint32_t bufferId);
-	void setIsp(const ControlList &controls);
+	void setIspControls(const ControlList &controls);
 	void setDelayedControls(const ControlList &controls);
 
 	/* bufferComplete signal handlers. */
@@ -1172,7 +1172,7 @@ int RPiCameraData::loadIPA()
 	ipa_->statsMetadataComplete.connect(this, &RPiCameraData::statsMetadataComplete);
 	ipa_->runIsp.connect(this, &RPiCameraData::runIsp);
 	ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);
-	ipa_->setIsp.connect(this, &RPiCameraData::setIsp);
+	ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);
 	ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);
 
 	IPASettings settings(ipa_->configurationFile(sensor_->model() + ".json"));
@@ -1241,7 +1241,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
 		return -EPIPE;
 	}
 
-	if (result.params & ipa::rpi::ConfigStaggeredWrite) {
+	if (result.params & ipa::rpi::ConfigSensorParams) {
 		/*
 		 * Setup our delayed control writer with the sensor default
 		 * gain and exposure delays.
@@ -1280,75 +1280,76 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
 
 void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &controls)
 {
-	if (state_ == State::Stopped)
-		handleState();
+	if (state_ != State::Stopped) {
+		FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
 
-	FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
+		handleStreamBuffer(buffer, &isp_[Isp::Stats]);
 
-	handleStreamBuffer(buffer, &isp_[Isp::Stats]);
+		/* Fill the Request metadata buffer with what the IPA has provided */
+		Request *request = requestQueue_.front();
+		request->metadata() = std::move(controls);
 
-	/* Fill the Request metadata buffer with what the IPA has provided */
-	Request *request = requestQueue_.front();
-	request->metadata() = std::move(controls);
+		/*
+		* Also update the ScalerCrop in the metadata with what we actually
+		* used. But we must first rescale that from ISP (camera mode) pixels
+		* back into sensor native pixels.
+		*
+		* Sending this information on every frame may be helpful.
+		*/
+		if (updateScalerCrop_) {
+			updateScalerCrop_ = false;
+			scalerCrop_ = ispCrop_.scaledBy(sensorInfo_.analogCrop.size(),
+							sensorInfo_.outputSize);
+			scalerCrop_.translateBy(sensorInfo_.analogCrop.topLeft());
+		}
+		request->metadata().set(controls::ScalerCrop, scalerCrop_);
 
-	/*
-	 * Also update the ScalerCrop in the metadata with what we actually
-	 * used. But we must first rescale that from ISP (camera mode) pixels
-	 * back into sensor native pixels.
-	 *
-	 * Sending this information on every frame may be helpful.
-	 */
-	if (updateScalerCrop_) {
-		updateScalerCrop_ = false;
-		scalerCrop_ = ispCrop_.scaledBy(sensorInfo_.analogCrop.size(),
-						sensorInfo_.outputSize);
-		scalerCrop_.translateBy(sensorInfo_.analogCrop.topLeft());
+		state_ = State::IpaComplete;
 	}
-	request->metadata().set(controls::ScalerCrop, scalerCrop_);
-
-	state_ = State::IpaComplete;
 
 	handleState();
 }
 
 void RPiCameraData::runIsp(uint32_t bufferId)
 {
-	if (state_ == State::Stopped)
-		handleState();
+	if (state_ != State::Stopped) {
+		FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
 
-	FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
+		LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << bufferId
+				<< ", timestamp: " << buffer->metadata().timestamp;
 
-	LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << bufferId
-			<< ", timestamp: " << buffer->metadata().timestamp;
+		isp_[Isp::Input].queueBuffer(buffer);
+		ispOutputCount_ = 0;
+	}
 
-	isp_[Isp::Input].queueBuffer(buffer);
-	ispOutputCount_ = 0;
 	handleState();
 }
 
 void RPiCameraData::embeddedComplete(uint32_t bufferId)
 {
-	if (state_ == State::Stopped)
-		handleState();
+	if (state_ != State::Stopped) {
+		FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers().at(bufferId);
+		handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
+	}
 
-	FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers().at(bufferId);
-	handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
 	handleState();
 }
 
-void RPiCameraData::setIsp(const ControlList &controls)
+void RPiCameraData::setIspControls(const ControlList &controls)
 {
 	ControlList ctrls = controls;
 
-	Span<const uint8_t> s =
-		ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();
-	bcm2835_isp_lens_shading ls =
-		*reinterpret_cast<const bcm2835_isp_lens_shading *>(s.data());
-	ls.dmabuf = lsTable_.fd();
+	if (ctrls.contains(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING)) {
+		Span<const uint8_t> s =
+			ctrls.get(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING).data();
+		bcm2835_isp_lens_shading ls =
+			*reinterpret_cast<const bcm2835_isp_lens_shading *>(s.data());
+		ls.dmabuf = lsTable_.fd();
 
-	ControlValue c(Span<const uint8_t>{ reinterpret_cast<uint8_t *>(&ls),
-					    sizeof(ls) });
-	ctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);
+		ControlValue c(Span<const uint8_t>{ reinterpret_cast<uint8_t *>(&ls),
+						    sizeof(ls) });
+		ctrls.set(V4L2_CID_USER_BCM2835_ISP_LENS_SHADING, c);
+	}
 
 	isp_[Isp::Input].dev()->setControls(&ctrls);
 	handleState();
-- 
2.25.1



More information about the libcamera-devel mailing list