[PATCH v4 2/2] pipeline: simple: Use proper device for frame start events

[Stanislaw Gruszka]

Currently we use frame start event from video capture device to
apply controls. But the capture device might not generate the events.
Usually CSI-2 receiver is proper device to subscribe for start
frame events.

To fix, loop over devices in the pipeline (starting from the last one),
find one that emits start frame events and connect applyControls()
to it. Additionally remove direct call to sensor_->setControls() if
the emitter device was found.

This also fixes the assertion like below:

../src/libcamera/delayed_controls.cpp:227:
libcamera::ControlList libcamera::DelayedControls::get(uint32_t):
Assertion `info.type() != ControlTypeNone' failed

which can happen rarely at the beginning of streaming when
ControlRingBuffer is not yet filled and there are errors on CSI-2.
In such rare scenario we can have call to DelayedControls::get()
with frame number that exceed number of saved entries. Handing
frame start signal assures we do DelayedConntrols::applyControls()
with (possibly out of sequence) frame number and later call
to DelayedControls::get() will get proper not-empty control entry.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=241
Co-developed-by: Hans de Goede <hdegoede at redhat.com>
Signed-off-by: Hans de Goede <hdegoede at redhat.com>
Reviewed-by: Hans de Goede <hdegoede at redhat.com> # v2
Tested-by: Hans de Goede <hdegoede at redhat.com> # Lenovo X1 Yoga IPU6 + ov2740 # v2
Reviewed-by: Milan Zamazal <mzamazal at redhat.com> # v2
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com> # v3
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 40 +++++++++++++++++++++---
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 6e039bf3..088b9156 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -277,6 +277,7 @@ public:
 	std::list<Entity> entities_;
 	std::unique_ptr<CameraSensor> sensor_;
 	V4L2VideoDevice *video_;
+	V4L2Subdevice *frameStartEmitter_;
 
 	std::vector<Configuration> configs_;
 	std::map<PixelFormat, std::vector<const Configuration *>> formats_;
@@ -579,6 +580,19 @@ int SimpleCameraData::init()
 
 	properties_ = sensor_->properties();
 
+	frameStartEmitter_ = nullptr;
+	for (auto it = entities_.rbegin(); it != entities_.rend(); ++it) {
+		V4L2Subdevice *sd = pipe->subdev(it->entity);
+		if (!sd || !sd->supportsFrameStartEvent())
+			continue;
+
+		LOG(SimplePipeline, Debug)
+			<< "Using " << it->entity->name() << " frameStart signal";
+
+		frameStartEmitter_ = sd;
+		break;
+	}
+
 	return 0;
 }
 
@@ -897,8 +911,11 @@ void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)
 void SimpleCameraData::setSensorControls(const ControlList &sensorControls)
 {
 	delayedCtrls_->push(sensorControls);
-	ControlList ctrls(sensorControls);
-	sensor_->setControls(&ctrls);
+	/* Directly apply controls now if there is no frameStart signal */
+	if (!frameStartEmitter_ || !frameStartEmitter_->frameStartEnabled()) {
+		ControlList ctrls(sensorControls);
+		sensor_->setControls(&ctrls);
+	}
 }
 
 /* Retrieve all source pads connected to a sink pad through active routes. */
@@ -1285,8 +1302,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
 	data->delayedCtrls_ =
 		std::make_unique<DelayedControls>(data->sensor_->device(),
 						  params);
-	data->video_->frameStart.connect(data->delayedCtrls_.get(),
-					 &DelayedControls::applyControls);
 
 	StreamConfiguration inputCfg;
 	inputCfg.pixelFormat = pipeConfig->captureFormat;
@@ -1325,6 +1340,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL
 {
 	SimpleCameraData *data = cameraData(camera);
 	V4L2VideoDevice *video = data->video_;
+	V4L2Subdevice *frameStartEmitter = data->frameStartEmitter_;
 	int ret;
 
 	const MediaPad *pad = acquirePipeline(data);
@@ -1354,6 +1370,15 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL
 
 	video->bufferReady.connect(data, &SimpleCameraData::imageBufferReady);
 
+	if (frameStartEmitter) {
+		ret = frameStartEmitter->setFrameStartEnabled(true);
+		if (ret == 0)
+			frameStartEmitter->frameStart.connect(data->delayedCtrls_.get(),
+							      &DelayedControls::applyControls);
+	}
+
+	data->delayedCtrls_->reset();
+
 	ret = video->streamOn();
 	if (ret < 0) {
 		stop(camera);
@@ -1385,6 +1410,13 @@ void SimplePipelineHandler::stopDevice(Camera *camera)
 {
 	SimpleCameraData *data = cameraData(camera);
 	V4L2VideoDevice *video = data->video_;
+	V4L2Subdevice *frameStartEmitter = data->frameStartEmitter_;
+
+	if (frameStartEmitter) {
+		frameStartEmitter->setFrameStartEnabled(false);
+		frameStartEmitter->frameStart.disconnect(data->delayedCtrls_.get(),
+							 &DelayedControls::applyControls);
+	}
 
 	if (data->useConversion_) {
 		if (data->converter_)
-- 
2.43.0