[v5,2/5] pipeline: simple: Use proper device for frame start events

22866 diff mbox series

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 to find the one that
supports events and use it as start frame emitter.

Co-developed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

Quoting Stanislaw Gruszka (2025-02-25 16:41:13)
> 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 to find the one that
> supports events and use it as start frame emitter.
> 
> Co-developed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

This is something that I should incorporate into the MediaPipeline (or
CameraPipeline, or Pipeline?) class I have worked on to generalise
camera pipes...

But I think it's helpful here now.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 6e039bf3..063a098f 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 (const Entity &entity : entities_) {
> +               V4L2Subdevice *sd = pipe->subdev(entity.entity);
> +               if (!sd || !sd->supportsFrameStartEvent())
> +                       continue;
> +
> +               LOG(SimplePipeline, Debug)
> +                       << "Using " << entity.entity->name() << " frameStart signal";
> +
> +               frameStartEmitter_ = sd;
> +               break;
> +       }
> +
>         return 0;
>  }
>  
> @@ -1215,6 +1229,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>                 static_cast<SimpleCameraConfiguration *>(c);
>         SimpleCameraData *data = cameraData(camera);
>         V4L2VideoDevice *video = data->video_;
> +       V4L2Subdevice *frameStartEmitter = data->frameStartEmitter_;
>         int ret;
>  
>         /*
> @@ -1285,8 +1300,9 @@ 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);
> +       if (frameStartEmitter)
> +               frameStartEmitter->frameStart.connect(data->delayedCtrls_.get(),
> +                                                     &DelayedControls::applyControls);
>  
>         StreamConfiguration inputCfg;
>         inputCfg.pixelFormat = pipeConfig->captureFormat;
> -- 
> 2.43.0
>

Hi Stanislaw,

Thank you for the patch.

On Tue, Feb 25, 2025 at 05:41:13PM +0100, Stanislaw Gruszka wrote:
> 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 to find the one that

s/the one/the first one/

> supports events and use it as start frame emitter.
> 
> Co-developed-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 6e039bf3..063a098f 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();
>  

	/* Find the first subdev that can generate a frame start signal, if any. */

> +	frameStartEmitter_ = nullptr;
> +	for (const Entity &entity : entities_) {
> +		V4L2Subdevice *sd = pipe->subdev(entity.entity);
> +		if (!sd || !sd->supportsFrameStartEvent())
> +			continue;
> +
> +		LOG(SimplePipeline, Debug)
> +			<< "Using " << entity.entity->name() << " frameStart signal";

Let's quote the name as it can contain spaces.

			<< "Using frameStart signal from '"
			<< entity.entity->name() << "'";

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +		frameStartEmitter_ = sd;
> +		break;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1215,6 +1229,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  		static_cast<SimpleCameraConfiguration *>(c);
>  	SimpleCameraData *data = cameraData(camera);
>  	V4L2VideoDevice *video = data->video_;
> +	V4L2Subdevice *frameStartEmitter = data->frameStartEmitter_;
>  	int ret;
>  
>  	/*
> @@ -1285,8 +1300,9 @@ 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);
> +	if (frameStartEmitter)
> +		frameStartEmitter->frameStart.connect(data->delayedCtrls_.get(),
> +						      &DelayedControls::applyControls);
>  
>  	StreamConfiguration inputCfg;
>  	inputCfg.pixelFormat = pipeConfig->captureFormat;