[libcamera-devel] [PATCH v2 1/1] pipeline: raspberrypi: Create empty control lists correctly

[Naushir Patuck]

Hi David,

Thank you for this fix.

On Tue, 5 Oct 2021 at 09:57, David Plowman <david.plowman at raspberrypi.com>
wrote:

> When the pipeline handler start() method is supplied with a NULL list
> of controls, we send an empty control list to the IPA. When the IPA is
> running in isolated mode the control list goes through the data
> serializer, for which it must be marked correctly as a list of
> "controls::controls", otherwise the IPA process will abort.
>
> The IPA has a similar problem returning a control list in its
> configure() method. We must be careful to initialise it properly even
> when empty.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>

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


> ---
>  src/ipa/raspberrypi/raspberrypi.cpp                | 13 +++++++++----
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp |  3 ++-
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index 047123ab..fed82e22 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -389,21 +389,26 @@ int IPARPi::configure(const IPACameraSensorInfo
> &sensorInfo,
>         /* Pass the camera mode to the CamHelper to setup algorithms. */
>         helper_->SetCameraMode(mode_);
>
> +       /*
> +        * Initialise this ControlList correctly, even if empty, in case
> the IPA is
> +        * running is isolation mode (passing the ControlList through the
> IPC layer).
> +        */
> +       ControlList ctrls(sensorCtrls_);
> +
>         if (firstStart_) {
>                 /* Supply initial values for frame durations. */
>                 applyFrameDurations(defaultMinFrameDuration,
> defaultMaxFrameDuration);
>
>                 /* Supply initial values for gain and exposure. */
> -               ControlList ctrls(sensorCtrls_);
>                 AgcStatus agcStatus;
>                 agcStatus.shutter_time = defaultExposureTime;
>                 agcStatus.analogue_gain = defaultAnalogueGain;
>                 applyAGC(&agcStatus, ctrls);
> -
> -               ASSERT(controls);
> -               *controls = std::move(ctrls);
>         }
>
> +       ASSERT(controls);
> +       *controls = std::move(ctrls);
> +
>         return 0;
>  }
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 0bdfa727..1634ca98 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -825,7 +825,8 @@ int PipelineHandlerRPi::start(Camera *camera, const
> ControlList *controls)
>
>         /* Start the IPA. */
>         ipa::RPi::StartConfig startConfig;
> -       data->ipa_->start(controls ? *controls : ControlList{},
> &startConfig);
> +       data->ipa_->start(controls ? *controls : ControlList{
> controls::controls },
> +                         &startConfig);
>
>         /* Apply any gain/exposure settings that the IPA may have passed
> back. */
>         if (!startConfig.controls.empty())
> --
> 2.20.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211005/433f71d0/attachment.htm>