[PATCH v1] ipa: rpi: Apply default ControlInfo values for sensor controls

[David Plowman]

Hi Naush

Thanks for the patch.

On Tue, 4 Feb 2025 at 15:09, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> The existing IPA initialisation code did not set default values for
> some sensor related controls. This caused a crash using libcamerify
> when the it was trying to access the default value for
> controls::FrameDurationLimits as part of a recent change.
>
> Ensure controls::FrameDurationLimits, controls::AnalogueGain and
> controls::ExposureTime advertise default values along with the existing
> min/max values. The default is set to the min value.
>
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=253
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

I suppose there is a bit of a question as to what these defaults mean,
if anything. And if they don't mean much, then why do we need them.
But this is obviously better than crashing, so:

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

Though it also feels like we need some better automated testing!! :)

David

> ---
>  src/ipa/rpi/common/ipa_base.cpp | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index bd3c22000df5..5924df4c1125 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -259,15 +259,18 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa
>         ControlInfoMap::Map ctrlMap = ipaControls;
>         ctrlMap[&controls::FrameDurationLimits] =
>                 ControlInfo(static_cast<int64_t>(mode_.minFrameDuration.get<std::micro>()),
> -                           static_cast<int64_t>(mode_.maxFrameDuration.get<std::micro>()));
> +                           static_cast<int64_t>(mode_.maxFrameDuration.get<std::micro>()),
> +                           static_cast<int64_t>(mode_.minFrameDuration.get<std::micro>()));
>
>         ctrlMap[&controls::AnalogueGain] =
>                 ControlInfo(static_cast<float>(mode_.minAnalogueGain),
> -                           static_cast<float>(mode_.maxAnalogueGain));
> +                           static_cast<float>(mode_.maxAnalogueGain),
> +                           static_cast<float>(mode_.minAnalogueGain));
>
>         ctrlMap[&controls::ExposureTime] =
>                 ControlInfo(static_cast<int32_t>(mode_.minExposureTime.get<std::micro>()),
> -                           static_cast<int32_t>(mode_.maxExposureTime.get<std::micro>()));
> +                           static_cast<int32_t>(mode_.maxExposureTime.get<std::micro>()),
> +                           static_cast<int32_t>(mode_.minExposureTime.get<std::micro>()));
>
>         /* Declare colour processing related controls for non-mono sensors. */
>         if (!monoSensor_)
> --
> 2.43.0
>