[v4,2/2] pipeline: simple: Use proper device for frame start events
diff mbox series

Message ID 20250221112610.42402-3-stanislaw.gruszka@linux.intel.com
State Changes Requested
Headers show
Series
  • libcamera: start frame events changes
Related show

Commit Message

Stanislaw Gruszka Feb. 21, 2025, 11:26 a.m. UTC 
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@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com> # v2
Tested-by: Hans de Goede <hdegoede@redhat.com> # Lenovo X1 Yoga IPU6 + ov2740 # v2
Reviewed-by: Milan Zamazal <mzamazal@redhat.com> # v2
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> # v3
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 40 +++++++++++++++++++++---
 1 file changed, 36 insertions(+), 4 deletions(-)

Patch
diff mbox series 

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_)