[libcamera-devel,v3,2/3] pipeline: ipa: raspberrypi: Correctly report available control limits

16308 diff mbox series

The ipa currently advertises a static ControlInfoMap to specify the available
controls and their limits. Fix this limitation by having the IPA populate a new
ControlInfoMap with updated limits for ExposureTime, AnalogueGain, and
FrameDurationLimits controls based on the current sensor mode. This new
ControlInfoMap is then returned back to the pipeline handler and available to
the application after a successful Camera::configure() call.

Before the first call to Camera::configure(), this ControlInfoMap provides some
reasonable default limits for ExposureTime, AnalogueGain, and
FrameDurationLimits. However, applications must not rely on these values, but
obtain the correct limits after the call to Camera::configure().

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/ipa/raspberrypi.mojom       |  1 +
 src/ipa/raspberrypi/raspberrypi.cpp           | 33 +++++++++++++++++--
 .../pipeline/raspberrypi/raspberrypi.cpp      |  3 ++
 3 files changed, 34 insertions(+), 3 deletions(-)

HI Naush

Thanks for the patch. LGTM.

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

David

On Wed, 22 Jun 2022 at 11:21, Naushir Patuck via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> The ipa currently advertises a static ControlInfoMap to specify the available
> controls and their limits. Fix this limitation by having the IPA populate a new
> ControlInfoMap with updated limits for ExposureTime, AnalogueGain, and
> FrameDurationLimits controls based on the current sensor mode. This new
> ControlInfoMap is then returned back to the pipeline handler and available to
> the application after a successful Camera::configure() call.
>
> Before the first call to Camera::configure(), this ControlInfoMap provides some
> reasonable default limits for ExposureTime, AnalogueGain, and
> FrameDurationLimits. However, applications must not rely on these values, but
> obtain the correct limits after the call to Camera::configure().
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  include/libcamera/ipa/raspberrypi.mojom       |  1 +
>  src/ipa/raspberrypi/raspberrypi.cpp           | 33 +++++++++++++++++--
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 ++
>  3 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> index 77f52c282b0f..c0de435b7b33 100644
> --- a/include/libcamera/ipa/raspberrypi.mojom
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -45,6 +45,7 @@ struct IPAConfig {
>
>  struct IPAConfigResult {
>         float modeSensitivity;
> +       libcamera.ControlInfoMap controlInfo;
>  };
>
>  struct StartConfig {
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 089528f5e126..8f57350878cf 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -74,8 +74,8 @@ constexpr Duration controllerMinFrameDuration = 1.0s / 30.0;
>  /* List of controls handled by the Raspberry Pi IPA */
>  static const ControlInfoMap::Map ipaControls{
>         { &controls::AeEnable, ControlInfo(false, true) },
> -       { &controls::ExposureTime, ControlInfo(0, 999999) },
> -       { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> +       { &controls::ExposureTime, ControlInfo(0, 66666) },
> +       { &controls::AnalogueGain, ControlInfo(1.0f, 16.0f) },
>         { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },
>         { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
>         { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },
> @@ -89,7 +89,7 @@ static const ControlInfoMap::Map ipaControls{
>         { &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(1000), INT64_C(1000000000)) },
> +       { &controls::FrameDurationLimits, ControlInfo(INT64_C(33333), INT64_C(120000)) },
>         { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) }
>  };
>
> @@ -446,6 +446,33 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
>         ASSERT(controls);
>         *controls = std::move(ctrls);
>
> +       /*
> +        * Apply the correct limits to the exposure, gain and frame duration controls
> +        * based on the current sensor mode.
> +        */
> +       ControlInfoMap::Map ctrlMap = ipaControls;
> +       const Duration minSensorFrameDuration = mode_.min_frame_length * mode_.line_length;
> +       const Duration maxSensorFrameDuration = mode_.max_frame_length * mode_.line_length;
> +       ctrlMap[&controls::FrameDurationLimits] =
> +               ControlInfo(static_cast<int64_t>(minSensorFrameDuration.get<std::micro>()),
> +                           static_cast<int64_t>(maxSensorFrameDuration.get<std::micro>()));
> +
> +       ctrlMap[&controls::AnalogueGain] =
> +               ControlInfo(1.0f, static_cast<float>(helper_->Gain(maxSensorGainCode_)));
> +
> +       /*
> +        * Calculate the max exposure limit from the frame duration limit as V4L2
> +        * will limit the maximum control value based on the current VBLANK value.
> +        */
> +       Duration maxShutter = Duration::max();
> +       helper_->GetVBlanking(maxShutter, minSensorFrameDuration, maxSensorFrameDuration);
> +       const uint32_t exposureMin = sensorCtrls_.at(V4L2_CID_EXPOSURE).min().get<int32_t>();
> +
> +       ctrlMap[&controls::ExposureTime] =
> +               ControlInfo(static_cast<int32_t>(helper_->Exposure(exposureMin).get<std::micro>()),
> +                           static_cast<int32_t>(maxShutter.get<std::micro>()));
> +
> +       result->controlInfo = ControlInfoMap(std::move(ctrlMap), controls::controls);
>         return 0;
>  }
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index d980c1a71dd8..4596f2babcea 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -940,6 +940,9 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
>         /* Store the mode sensitivity for the application. */
>         data->properties_.set(properties::SensorSensitivity, result.modeSensitivity);
>
> +       /* Update the controls that the Raspberry Pi IPA can handle. */
> +       data->controlInfo_ = result.controlInfo;
> +
>         /* Setup the Video Mux/Bridge entities. */
>         for (auto &[device, link] : data->bridgeDevices_) {
>                 /*
> --
> 2.25.1
>