[libcamera-devel] [PATCH] pipeline: rpi: ipa_base: Parse config files after platformRegister()

[Jacopo Mondi]

Hi Naush

On Tue, May 23, 2023 at 01:02:08PM +0100, Naushir Patuck via libcamera-devel wrote:
> If a user uses a pipeline handler config file where a custom timeout
> value is specified, it would lead to a segmentation fault when
> loadPipelineConfiguration() tried to access the as yet uninitialised

s/the as yet/the yet/

> V4L2VideoDevice object.
>
> To fix this, parse the pipeline handler config file only after fully
> initialising and registering the pipeline handler and V4L2VideoDevice
> objects.

Indeed data->frontendDevice() will return null in that case.

Alternatively, if you think loadind configuration file before
platformRegister() might be useful, you can set the dequeue timeout
after having created the V4L2Device in platformRegister().

Either ways
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j
>
> Fixes: 6c71ee1f15305 ("pipeline: raspberrypi: Introduce PipelineHandlerBase class")
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index ba1797bcfef0..3bb5ec531e4f 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -888,16 +888,16 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
>  	}
>  	data->nativeBayerOrder_ = bayerFormat.order;
>
> +	ret = platformRegister(cameraData, frontend, backend);
> +	if (ret)
> +		return ret;
> +
>  	ret = data->loadPipelineConfiguration();
>  	if (ret) {
>  		LOG(RPI, Error) << "Unable to load pipeline configuration";
>  		return ret;
>  	}
>
> -	ret = platformRegister(cameraData, frontend, backend);
> -	if (ret)
> -		return ret;
> -
>  	/* Setup the general IPA signal handlers. */
>  	data->frontendDevice()->dequeueTimeout.connect(data, &RPi::CameraData::cameraTimeout);
>  	data->frontendDevice()->frameStart.connect(data, &RPi::CameraData::frameStarted);
> --
> 2.34.1
>