[libcamera-devel] [PATCH v1 8/9] libcamera: pipeline: raspberrypi: Start IPA when starting camera

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2020-06-29 02:19:33 +0300, Laurent Pinchart wrote:
> The IPA is meant to be started when starting the camera, and stopped
> when stopping it. It was so far start early in order to handle the
> IPAInterface::processEvent() call related to lens shading table
> allocation before IPAInterface::configure() to pass the table to the
> IPA. Now that the lens shading table is passed through configure(),
> starting the IPA early isn't needed anymore.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 31 ++++++++++---------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 60b01484b329..137e07379cf5 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -302,10 +302,6 @@ public:
>  			vcsm_.free(lsTable_);
>  			lsTable_ = nullptr;
>  		}
> -
> -		/* Stop the IPA proxy thread. */
> -		if (ipa_)
> -			ipa_->stop();
>  	}
>  
>  	void frameStarted(uint32_t sequence);
> @@ -787,6 +783,16 @@ int PipelineHandlerRPi::start(Camera *camera)
>  	ret = queueAllBuffers(camera);
>  	if (ret) {
>  		LOG(RPI, Error) << "Failed to queue buffers";
> +		stop(camera);
> +		return ret;
> +	}
> +
> +	/* Start the IPA. */
> +	ret = data->ipa_->start();
> +	if (ret) {
> +		LOG(RPI, Error)
> +			<< "Failed to start IPA for " << camera->name();
> +		stop(camera);
>  		return ret;
>  	}
>  
> @@ -797,8 +803,10 @@ int PipelineHandlerRPi::start(Camera *camera)
>  	V4L2DeviceFormat sensorFormat;
>  	data->unicam_[Unicam::Image].dev()->getFormat(&sensorFormat);
>  	ret = data->isp_[Isp::Input].dev()->setFormat(&sensorFormat);
> -	if (ret)
> +	if (ret) {
> +		stop(camera);
>  		return ret;
> +	}
>  
>  	/* Enable SOF event generation. */
>  	data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true);
> @@ -842,6 +850,9 @@ void PipelineHandlerRPi::stop(Camera *camera)
>  	data->bayerQueue_ = std::queue<FrameBuffer *>{};
>  	data->embeddedQueue_ = std::queue<FrameBuffer *>{};
>  
> +	/* Stop the IPA. */
> +	data->ipa_->stop();
> +
>  	freeBuffers(camera);
>  }
>  
> @@ -1094,15 +1105,7 @@ int RPiCameraData::loadIPA()
>  		.configurationFile = ipa_->configurationFile(sensor_->model() + ".json")
>  	};
>  
> -	ipa_->init(settings);
> -
> -	/*
> -	 * Startup the IPA thread now. Without this call, none of the IPA API
> -	 * functions will run.
> -	 *
> -	 * It only gets stopped in the class destructor.
> -	 */
> -	return ipa_->start();
> +	return ipa_->init(settings);
>  }
>  
>  int RPiCameraData::configureIPA()
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund