pipeline: rpi: Fix crash when configuring timeout

26809 diff mbox series

When loading a config containing a nonzero "camera_timeout_value_ms"
(as is required for externally-triggered cameras) it would crash as
the pipeline handler tried to access both the IPA and frontEndDevice
before either had been created. Re-order to avoid the crash.

Fixes: cc74afcdbd3e ("Make the controller min frame duration configurable")

Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
---
 .../pipeline/rpi/common/pipeline_base.cpp        | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Hi Nick,

Thanks for the fix.

On Wed, 27 May 2026 at 11:26, Nick Hollinghurst <
nick.hollinghurst@raspberrypi.com> wrote:

> When loading a config containing a nonzero "camera_timeout_value_ms"
> (as is required for externally-triggered cameras) it would crash as
> the pipeline handler tried to access both the IPA and frontEndDevice
> before either had been created. Re-order to avoid the crash.
>
> Fixes: cc74afcdbd3e ("Make the controller min frame duration configurable")
>
> Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


> ---
>  .../pipeline/rpi/common/pipeline_base.cpp        | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index ace38997..263a4838 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -878,6 +878,16 @@ int
> PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
>         data->ipa_->setLensControls.connect(data,
> &CameraData::setLensControls);
>         data->ipa_->metadataReady.connect(data,
> &CameraData::metadataReady);
>
> +       /*
> +        * Disable the IPA signal to control timeout, when there is a
> +        * user-requested value. We do this here now that the IPA and
> +        * front end device are both initialized.
> +        */
> +       if (data->config_.cameraTimeoutValue) {
> +               data->ipa_->setCameraTimeout.disconnect();
> +
>  data->frontendDevice()->setDequeueTimeout(data->config_.cameraTimeoutValue
> * 1ms);
> +       }
> +
>         return 0;
>  }
>
> @@ -1145,12 +1155,6 @@ int CameraData::loadPipelineConfiguration()
>         config_.cameraTimeoutValue =
>                 phConfig["camera_timeout_value_ms"].get<unsigned
> int>(config_.cameraTimeoutValue);
>
> -       if (config_.cameraTimeoutValue) {
> -               /* Disable the IPA signal to control timeout and set the
> user requested value. */
> -               ipa_->setCameraTimeout.disconnect();
> -
>  frontendDevice()->setDequeueTimeout(config_.cameraTimeoutValue * 1ms);
> -       }
> -
>         config_.controllerMinFrameDurationUs =
>
> phConfig["controller_min_frame_duration_us"].get<double>(config_.controllerMinFrameDurationUs);
>
> --
> 2.47.3
>
>

Hi Nick

Thanks for the patch!

On Wed, 27 May 2026 at 13:56, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> Hi Nick,
>
> Thanks for the fix.
>
> On Wed, 27 May 2026 at 11:26, Nick Hollinghurst <nick.hollinghurst@raspberrypi.com> wrote:
>>
>> When loading a config containing a nonzero "camera_timeout_value_ms"
>> (as is required for externally-triggered cameras) it would crash as
>> the pipeline handler tried to access both the IPA and frontEndDevice
>> before either had been created. Re-order to avoid the crash.
>>
>> Fixes: cc74afcdbd3e ("Make the controller min frame duration configurable")
>>
>> Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
>
>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

David

>
>>
>> ---
>>  .../pipeline/rpi/common/pipeline_base.cpp        | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> index ace38997..263a4838 100644
>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> @@ -878,6 +878,16 @@ int PipelineHandlerBase::registerCamera(std::unique_ptr<RPi::CameraData> &camera
>>         data->ipa_->setLensControls.connect(data, &CameraData::setLensControls);
>>         data->ipa_->metadataReady.connect(data, &CameraData::metadataReady);
>>
>> +       /*
>> +        * Disable the IPA signal to control timeout, when there is a
>> +        * user-requested value. We do this here now that the IPA and
>> +        * front end device are both initialized.
>> +        */
>> +       if (data->config_.cameraTimeoutValue) {
>> +               data->ipa_->setCameraTimeout.disconnect();
>> +               data->frontendDevice()->setDequeueTimeout(data->config_.cameraTimeoutValue * 1ms);
>> +       }
>> +
>>         return 0;
>>  }
>>
>> @@ -1145,12 +1155,6 @@ int CameraData::loadPipelineConfiguration()
>>         config_.cameraTimeoutValue =
>>                 phConfig["camera_timeout_value_ms"].get<unsigned int>(config_.cameraTimeoutValue);
>>
>> -       if (config_.cameraTimeoutValue) {
>> -               /* Disable the IPA signal to control timeout and set the user requested value. */
>> -               ipa_->setCameraTimeout.disconnect();
>> -               frontendDevice()->setDequeueTimeout(config_.cameraTimeoutValue * 1ms);
>> -       }
>> -
>>         config_.controllerMinFrameDurationUs =
>>                 phConfig["controller_min_frame_duration_us"].get<double>(config_.controllerMinFrameDurationUs);
>>
>> --
>> 2.47.3
>>