[v3,36/41] libipa: fc_queue: Return nullptr instead of crashing
diff mbox series

Message ID 20260914140309.3354666-37-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • rkisp1: pipeline rework for PFC
Related show

Commit Message

Stefan Klug Sept. 14, 2026, 2:02 p.m. UTC 
If a requested frame context in the FCQueue was overwritten, libcamera
crashes with a fatal log message. This condition occurs in rare
cases with very high system load or in multi camera setups where the main
camera thread is blocked by e.g. starting a seond camera for a
considerable amount of time.

Still we sould not crash in these cases. To be able to handle the
situation in upcoming patches, modify getOrInitContext() to return a
pointer instead of a reference and return a nullptr instead of the fatal
log.

Add an ASSERT() to all callers so that we still crash in these cases.
More fine grained handling needs to be implemented later.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/ipu3/ipu3.cpp         | 17 ++++++++++-------
 src/ipa/libipa/fc_queue.h     | 32 ++++++++++++++------------------
 src/ipa/mali-c55/mali-c55.cpp | 16 ++++++++++------
 src/ipa/rkisp1/rkisp1.cpp     | 21 ++++++++++++---------
 src/ipa/softisp/softisp.cpp   | 15 +++++++++------
 5 files changed, 55 insertions(+), 46 deletions(-)

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index 24bbfa86c255..ed873784c730 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -473,10 +473,11 @@  void IPAIPU3::computeParams(const uint32_t frame, const uint32_t bufferId)
 	 */
 	params->use = {};
 
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(frame);
+	ASSERT(frameContext);
 
 	for (const auto &algo : algorithms())
-		algo->prepare(context_, frame, frameContext, params);
+		algo->prepare(context_, frame, *frameContext, params);
 
 	paramsComputed.emit(frame);
 }
@@ -506,15 +507,16 @@  void IPAIPU3::processStats(const uint32_t frame,
 	const ipu3_uapi_stats_3a *stats =
 		reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
 
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(frame);
+	ASSERT(frameContext);
 
-	std::tie(frameContext.sensor.exposure, frameContext.sensor.gain) =
+	std::tie(frameContext->sensor.exposure, frameContext->sensor.gain) =
 		agc::extractControls(sensorControls, context_.camHelper.get());
 
 	ControlList metadata(controls::controls);
 
 	for (const auto &algo : algorithms())
-		algo->process(context_, frame, frameContext, stats, metadata);
+		algo->process(context_, frame, *frameContext, stats, metadata);
 
 	setControls(frame);
 
@@ -558,11 +560,12 @@  void IPAIPU3::initializeFrameContext(IPAFrameContext &frameContext,
  */
 void IPAIPU3::setControls(unsigned int frame)
 {
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(frame);
+	ASSERT(frameContext);
 
 	ControlList ctrls(context_.sensorControls);
 	agc::prepareControls(ctrls, context_.camHelper.get(),
-			     frameContext.agc.exposure, frameContext.agc.gain);
+			     frameContext->agc.exposure, frameContext->agc.gain);
 
 	ControlList lensCtrls(lensCtrls_);
 	lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE,
diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h
index 796ab656cf40..1dd04ad1ae2c 100644
--- a/src/ipa/libipa/fc_queue.h
+++ b/src/ipa/libipa/fc_queue.h
@@ -54,7 +54,7 @@  public:
 		initialized_ = false;
 	}
 
-	FC &getOrInitContext(unsigned int frame, const ControlList &controls = {})
+	FC *getOrInitContext(unsigned int frame, const ControlList &controls = {})
 	{
 		FC &fc = contexts_[frame % contexts_.size()];
 		FrameContext &frameContext = fc;
@@ -64,23 +64,10 @@  public:
 			frameContext.frame_ = frame;
 			initCallback_(fc, controls);
 			initialized_ = true;
-			return fc;
+			return &fc;
 		}
 
-		/*
-		 * If the IPA algorithms try to access a frame context slot which
-		 * has been already overwritten by a newer context, it means the
-		 * frame context queue has overflowed and the desired context
-		 * has been forever lost. The pipeline handler shall avoid
-		 * queueing more requests to the IPA than the frame context
-		 * queue size.
-		 */
-		if (frame < frameContext.frame_)
-			LOG(FCQueue, Fatal) << "Frame context for " << frame
-					    << " has been overwritten by "
-					    << frameContext.frame_;
-
-		if (frame == frameContext.frame_) {
+		if (frame <= frameContext.frame_) {
 			if (!controls.empty()) {
 				/* Too late to apply the controls. Store them for later. */
 				LOG(FCQueue, Warning)
@@ -89,8 +76,17 @@  public:
 				controlsToApply_.merge(controls,
 						       ControlList::MergePolicy::OverwriteExisting);
 			}
+
+			if (frame < frameContext.frame_) {
+				LOG(FCQueue, Warning)
+					<< "Frame context for " << frame
+					<< " is already overwritten by "
+					<< frameContext.frame_;
+				return nullptr;
+			}
+
 			LOG(FCQueue, Debug) << "Got " << frame;
-			return fc;
+			return &fc;
 		}
 
 		const ControlList *controls2 = &controls;
@@ -107,7 +103,7 @@  public:
 		initCallback_(fc, *controls2);
 		controlsToApply_.clear();
 
-		return fc;
+		return &fc;
 	}
 
 private:
diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp
index d3d17bb5ae18..7ee4a9349af1 100644
--- a/src/ipa/mali-c55/mali-c55.cpp
+++ b/src/ipa/mali-c55/mali-c55.cpp
@@ -243,11 +243,13 @@  void IPAMaliC55::initializeFrameContext(IPAFrameContext &frameContext,
 void IPAMaliC55::fillParams(unsigned int request,
 			    [[maybe_unused]] uint32_t bufferId)
 {
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(request);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(request);
 	MaliC55Params params(buffers_.at(bufferId).planes()[0]);
 
+	ASSERT(frameContext);
+
 	for (const auto &algo : algorithms())
-		algo->prepare(context_, request, frameContext, &params);
+		algo->prepare(context_, request, *frameContext, &params);
 
 	paramsComputed.emit(request, params.bytesused());
 }
@@ -255,13 +257,15 @@  void IPAMaliC55::fillParams(unsigned int request,
 void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
 			      const ControlList &sensorControls)
 {
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(request);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(request);
 	const mali_c55_stats_buffer *stats = nullptr;
 
+	ASSERT(frameContext);
+
 	stats = reinterpret_cast<mali_c55_stats_buffer *>(
 		buffers_.at(bufferId).planes()[0].data());
 
-	std::tie(frameContext.sensor.exposure, frameContext.sensor.gain) =
+	std::tie(frameContext->sensor.exposure, frameContext->sensor.gain) =
 		agc::extractControls(sensorControls, context_.camHelper.get());
 
 	ControlList metadata(controls::controls);
@@ -269,10 +273,10 @@  void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
 	for (const auto &a : algorithms()) {
 		Algorithm *algo = static_cast<Algorithm *>(a.get());
 
-		algo->process(context_, request, frameContext, stats, metadata);
+		algo->process(context_, request, *frameContext, stats, metadata);
 	}
 
-	setControls(frameContext);
+	setControls(*frameContext);
 
 	statsProcessed.emit(request, metadata);
 }
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index b8b4a79830c8..750c0fa72733 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -216,15 +216,16 @@  int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
 void IPARkISP1::start(const ControlList &controls, const uint32_t paramBufferId,
 		      StartResult *result)
 {
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(0, controls);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(0, controls);
+	ASSERT(frameContext);
 
 	if (paramBufferId != 0)
-		result->paramBufferBytesUsed = computeParamsInternal(frameContext,
+		result->paramBufferBytesUsed = computeParamsInternal(*frameContext,
 								     paramBufferId);
 	else
 		result->paramBufferBytesUsed = 0;
 
-	result->controls = getSensorControls(frameContext);
+	result->controls = getSensorControls(*frameContext);
 	result->code = 0;
 }
 
@@ -336,21 +337,23 @@  uint32_t IPARkISP1::computeParamsInternal(IPAFrameContext &frameContext, const u
 
 void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)
 {
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(frame);
+	ASSERT(frameContext);
 
 	if (bufferId != 0) {
-		uint32_t size = computeParamsInternal(frameContext, bufferId);
+		uint32_t size = computeParamsInternal(*frameContext, bufferId);
 		paramsComputed.emit(frame, bufferId, size);
 	}
 
-	ControlList ctrls = getSensorControls(frameContext);
+	ControlList ctrls = getSensorControls(*frameContext);
 	setSensorControls.emit(frame, ctrls);
 }
 
 void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,
 			     const ControlList &sensorControls)
 {
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(frame);
+	ASSERT(frameContext);
 
 	/*
 	 * In raw capture mode, the ISP is bypassed and no statistics buffer is
@@ -361,7 +364,7 @@  void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,
 		stats = reinterpret_cast<rkisp1_stat_buffer *>(
 			mappedBuffers_.at(bufferId).planes()[0].data());
 
-	std::tie(frameContext.sensor.exposure, frameContext.sensor.gain) =
+	std::tie(frameContext->sensor.exposure, frameContext->sensor.gain) =
 		agc::extractControls(sensorControls, context_.camHelper.get());
 
 	ControlList metadata(controls::controls);
@@ -370,7 +373,7 @@  void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,
 		Algorithm *algo = static_cast<Algorithm *>(a.get());
 		if (algo->disabled_)
 			continue;
-		algo->process(context_, frame, frameContext, stats, metadata);
+		algo->process(context_, frame, *frameContext, stats, metadata);
 	}
 
 	context_.debugMetadata.moveEntries(metadata);
diff --git a/src/ipa/softisp/softisp.cpp b/src/ipa/softisp/softisp.cpp
index c00d36bdff1a..2a65cd7cebfe 100644
--- a/src/ipa/softisp/softisp.cpp
+++ b/src/ipa/softisp/softisp.cpp
@@ -237,9 +237,11 @@  void IPASoftIsp::computeParams(const uint32_t frame)
 {
 	context_.activeState.combinedMatrix = Matrix<float, 3, 3>::identity();
 
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(frame);
+	ASSERT(frameContext);
+
 	for (const auto &algo : algorithms())
-		algo->prepare(context_, frame, frameContext, params_);
+		algo->prepare(context_, frame, *frameContext, params_);
 	params_->combinedMatrix = context_.activeState.combinedMatrix;
 
 	paramsComputed.emit(frame);
@@ -249,19 +251,20 @@  void IPASoftIsp::processStats(const uint32_t frame,
 			      [[maybe_unused]] const uint32_t bufferId,
 			      const ControlList &sensorControls)
 {
-	IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
+	IPAFrameContext *frameContext = context_.frameContexts.getOrInitContext(frame);
+	ASSERT(frameContext);
 
-	std::tie(frameContext.sensor.exposure, frameContext.sensor.gain) =
+	std::tie(frameContext->sensor.exposure, frameContext->sensor.gain) =
 		agc::extractControls(sensorControls, context_.camHelper.get());
 
 	ControlList metadata(controls::controls);
 	for (const auto &algo : algorithms())
-		algo->process(context_, frame, frameContext, stats_, metadata);
+		algo->process(context_, frame, *frameContext, stats_, metadata);
 	metadataReady.emit(frame, metadata);
 
 	ControlList ctrls(context_.sensorControls);
 	agc::prepareControls(ctrls, context_.camHelper.get(),
-			     frameContext.agc.exposure, frameContext.agc.gain);
+			     frameContext->agc.exposure, frameContext->agc.gain);
 	setSensorControls.emit(ctrls);
 }