[libcamera-devel] [PATCH v1 2/2] ipa: rpi: Handle controls for mono variant sensors

[David Plowman]

Hi Naush

Thanks for the patch!

On Wed, 31 May 2023 at 15:39, Naushir Patuck via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Do not advertise colour related controls (i.e. [A]WB, colour saturation)
> in the ControlInfoMap of available controls returned out to the
> application.
>
> Silently fail these controls in the control handler in case applications
> don't use the advertised ControlInfoMap to validate controls.
>
> As a drive-by, don't advertise controls::ColourCorrectionMatrix in the
> ControlInfoMap as it is not handled by the IPA.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

I think silently ignoring the colour controls in this situation seems
reasonable.

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

Thanks
David

> ---
>  src/ipa/rpi/common/ipa_base.cpp | 34 ++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index 150fe433d0df..8de454637cfe 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -60,19 +60,22 @@ const ControlInfoMap::Map ipaControls{
>         { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
>         { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },
>         { &controls::ExposureValue, ControlInfo(-8.0f, 8.0f, 0.0f) },
> -       { &controls::AwbEnable, ControlInfo(false, true) },
> -       { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> -       { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
>         { &controls::Brightness, ControlInfo(-1.0f, 1.0f, 0.0f) },
>         { &controls::Contrast, ControlInfo(0.0f, 32.0f, 1.0f) },
> -       { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },
>         { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> -       { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
>         { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
>         { &controls::FrameDurationLimits, ControlInfo(INT64_C(33333), INT64_C(120000)) },
>         { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) }
>  };
>
> +/* IPA controls handled conditionally, if the sensor is not mono */
> +const ControlInfoMap::Map ipaColourControls{
> +       { &controls::AwbEnable, ControlInfo(false, true) },
> +       { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
> +       { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> +       { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },
> +};
> +
>  /* IPA controls handled conditionally, if the lens has a focus control */
>  const ControlInfoMap::Map ipaAfControls{
>         { &controls::AfMode, ControlInfo(controls::AfModeValues) },
> @@ -145,6 +148,8 @@ int32_t IpaBase::init(const IPASettings &settings, const InitParams &params, Ini
>
>         /* Return the controls handled by the IPA */
>         ControlInfoMap::Map ctrlMap = ipaControls;
> +       if (!monoSensor_)
> +               ctrlMap.merge(ControlInfoMap::Map(ipaColourControls));
>         if (lensPresent_)
>                 ctrlMap.merge(ControlInfoMap::Map(ipaAfControls));
>         result->controlInfo = ControlInfoMap(std::move(ctrlMap), controls::controls);
> @@ -221,6 +226,9 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa
>                 ControlInfo(static_cast<int32_t>(mode_.minShutter.get<std::micro>()),
>                             static_cast<int32_t>(mode_.maxShutter.get<std::micro>()));
>
> +       if (!monoSensor_)
> +               ctrlMap.merge(ControlInfoMap::Map(ipaColourControls));
> +
>         /* Declare Autofocus controls, only if we have a controllable lens */
>         if (lensPresent_)
>                 ctrlMap.merge(ControlInfoMap::Map(ipaAfControls));
> @@ -781,6 +789,10 @@ void IpaBase::applyControls(const ControlList &controls)
>                 }
>
>                 case controls::AWB_ENABLE: {
> +                       /* Silently ignore this control for a mono sensor. */
> +                       if (monoSensor_)
> +                               break;
> +
>                         RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(
>                                 controller_.getAlgorithm("awb"));
>                         if (!awb) {
> @@ -800,6 +812,10 @@ void IpaBase::applyControls(const ControlList &controls)
>                 }
>
>                 case controls::AWB_MODE: {
> +                       /* Silently ignore this control for a mono sensor. */
> +                       if (monoSensor_)
> +                               break;
> +
>                         RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(
>                                 controller_.getAlgorithm("awb"));
>                         if (!awb) {
> @@ -820,6 +836,10 @@ void IpaBase::applyControls(const ControlList &controls)
>                 }
>
>                 case controls::COLOUR_GAINS: {
> +                       /* Silently ignore this control for a mono sensor. */
> +                       if (monoSensor_)
> +                               break;
> +
>                         auto gains = ctrl.second.get<Span<const float>>();
>                         RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(
>                                 controller_.getAlgorithm("awb"));
> @@ -868,6 +888,10 @@ void IpaBase::applyControls(const ControlList &controls)
>                 }
>
>                 case controls::SATURATION: {
> +                       /* Silently ignore this control for a mono sensor. */
> +                       if (monoSensor_)
> +                               break;
> +
>                         RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>(
>                                 controller_.getAlgorithm("ccm"));
>                         if (!ccm) {
> --
> 2.34.1
>