pipeline: simple: Create DelayedControls instance once only

22839 diff mbox series

The DelayedControls instance for the camera sensor is created in
SimplePipelineHandler::configure(). Constant deletion and reconstruction
of a new object is unnecessary, as the control delays are an intrinsic
property of the sensor and are known at initialization time. Move the
DelayedControls creation to the SimpleCameraData class constructor.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)


base-commit: d476f8358be1536d4edd680c6024f784ff022f5d
--
Regards,

Laurent Pinchart

On Mon, Feb 24, 2025 at 03:23:25AM +0200, Laurent Pinchart wrote:
> The DelayedControls instance for the camera sensor is created in
> SimplePipelineHandler::configure(). Constant deletion and reconstruction
> of a new object is unnecessary, as the control delays are an intrinsic
> property of the sensor and are known at initialization time. Move the
> DelayedControls creation to the SimpleCameraData class constructor.

This should be merged on top of Stanislaw's "[PATCH v4 0/2] libcamera:
start frame events changes" series (or rather on top of v5), or it would
introduce a signal duplicated connection bug.

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 6e039bf35fc1..bd3a8a6ec6b7 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -488,6 +488,13 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>  	if (!sensor_)
>  		return;
> 
> +	const CameraSensorProperties::SensorDelays &delays = sensor_->sensorDelays();
> +	std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> +		{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> +		{ V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> +	};
> +	delayedCtrls_ = std::make_unique<DelayedControls>(sensor_->device(), params);
> +
>  	LOG(SimplePipeline, Debug)
>  		<< "Found pipeline: "
>  		<< utils::join(entities_, " -> ",
> @@ -1277,14 +1284,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  	if (outputCfgs.empty())
>  		return 0;
> 
> -	const CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays();
> -	std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> -		{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> -		{ V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> -	};
> -	data->delayedCtrls_ =
> -		std::make_unique<DelayedControls>(data->sensor_->device(),
> -						  params);
>  	data->video_->frameStart.connect(data->delayedCtrls_.get(),
>  					 &DelayedControls::applyControls);
> 
> 
> base-commit: d476f8358be1536d4edd680c6024f784ff022f5d

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> The DelayedControls instance for the camera sensor is created in
> SimplePipelineHandler::configure(). Constant deletion and reconstruction
> of a new object is unnecessary, as the control delays are an intrinsic
> property of the sensor and are known at initialization time. Move the
> DelayedControls creation to the SimpleCameraData class constructor.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 6e039bf35fc1..bd3a8a6ec6b7 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -488,6 +488,13 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
>  	if (!sensor_)
>  		return;
>
> +	const CameraSensorProperties::SensorDelays &delays = sensor_->sensorDelays();
> +	std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> +		{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> +		{ V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> +	};
> +	delayedCtrls_ = std::make_unique<DelayedControls>(sensor_->device(), params);
> +
>  	LOG(SimplePipeline, Debug)
>  		<< "Found pipeline: "
>  		<< utils::join(entities_, " -> ",
> @@ -1277,14 +1284,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  	if (outputCfgs.empty())
>  		return 0;
>
> -	const CameraSensorProperties::SensorDelays &delays = data->sensor_->sensorDelays();
> -	std::unordered_map<uint32_t, DelayedControls::ControlParams> params = {
> -		{ V4L2_CID_ANALOGUE_GAIN, { delays.gainDelay, false } },
> -		{ V4L2_CID_EXPOSURE, { delays.exposureDelay, false } },
> -	};
> -	data->delayedCtrls_ =
> -		std::make_unique<DelayedControls>(data->sensor_->device(),
> -						  params);
>  	data->video_->frameStart.connect(data->delayedCtrls_.get(),
>  					 &DelayedControls::applyControls);
>
>
> base-commit: d476f8358be1536d4edd680c6024f784ff022f5d
> --
> Regards,
>
> Laurent Pinchart