[libcamera-devel,2/2] ipa: raspberrypi: AGC: handle modes with different sensitivities

12444 diff mbox series

When the sensor is switched to a mode with a different sensitivity,
the target exposure values need to be adjusted proportionately to
maintain the same image brightness.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/controller/rpi/agc.cpp | 25 +++++++++++++++++-----
 src/ipa/raspberrypi/controller/rpi/agc.hpp |  1 +
 2 files changed, 21 insertions(+), 5 deletions(-)

Hi David,

Thank you for your work.

On Thu, 27 May 2021 at 09:45, David Plowman <david.plowman@raspberrypi.com>
wrote:

> When the sensor is switched to a mode with a different sensitivity,
> the target exposure values need to be adjusted proportionately to
> maintain the same image brightness.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>

These changes look sensible to me.

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


> ---
>  src/ipa/raspberrypi/controller/rpi/agc.cpp | 25 +++++++++++++++++-----
>  src/ipa/raspberrypi/controller/rpi/agc.hpp |  1 +
>  2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> index f4cd5d26..6f0e9702 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> @@ -155,7 +155,7 @@ Agc::Agc(Controller *controller)
>         : AgcAlgorithm(controller), metering_mode_(nullptr),
>           exposure_mode_(nullptr), constraint_mode_(nullptr),
>           frame_count_(0), lock_count_(0),
> -         last_target_exposure_(0.0),
> +         last_target_exposure_(0.0), last_sensitivity_(0.0),
>           ev_(1.0), flicker_period_(0.0),
>           max_shutter_(0), fixed_shutter_(0), fixed_analogue_gain_(0.0)
>  {
> @@ -261,7 +261,7 @@ void Agc::SetConstraintMode(std::string const
> &constraint_mode_name)
>         constraint_mode_name_ = constraint_mode_name;
>  }
>
> -void Agc::SwitchMode([[maybe_unused]] CameraMode const &camera_mode,
> +void Agc::SwitchMode(CameraMode const &camera_mode,
>                      Metadata *metadata)
>  {
>         housekeepConfig();
> @@ -285,9 +285,20 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode
> const &camera_mode,
>                 filtered_.shutter = fixed_shutter;
>                 filtered_.analogue_gain = fixed_analogue_gain_;
>         } else if (status_.total_exposure_value) {
> -               // On a mode switch, it's possible the exposure profile
> could change,
> -               // or a fixed exposure/gain might be set so we divide up
> the exposure/
> -               // gain again, but we don't change any target values.
> +               // On a mode switch, various things could happen:
> +               // - the exposure profile might change
> +               // - a fixed exposure or gain might be set
> +               // - the new mode's sensitivity might be different
> +               // We cope with the last of these by scaling the target
> values. After
> +               // that we just need to re-divide the exposure/gain
> according to the
> +               // current exposure profile, which takes care of
> everything else.
> +
> +               double ratio = last_sensitivity_ / camera_mode.sensitivity;
> +               target_.total_exposure_no_dg *= ratio;
> +               target_.total_exposure *= ratio;
> +               filtered_.total_exposure_no_dg *= ratio;
> +               filtered_.total_exposure *= ratio;
> +
>                 divideUpExposure();
>         } else {
>                 // We come through here on startup, when at least one of
> the shutter
> @@ -301,6 +312,10 @@ void Agc::SwitchMode([[maybe_unused]] CameraMode
> const &camera_mode,
>         }
>
>         writeAndFinish(metadata, false);
> +
> +       // We must remember the sensitivity of this mode for the next
> SwitchMode.
> +       ASSERT(camera_mode.sensitivity != 0.0);
> +       last_sensitivity_ = camera_mode.sensitivity;
>  }
>
>  void Agc::Prepare(Metadata *image_metadata)
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> index 0427fb59..025878a2 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> @@ -122,6 +122,7 @@ private:
>         int lock_count_;
>         DeviceStatus last_device_status_;
>         double last_target_exposure_;
> +       double last_sensitivity_; // sensitivity of the previous camera
> mode
>         // Below here the "settings" that applications can change.
>         std::string metering_mode_name_;
>         std::string exposure_mode_name_;
> --
> 2.20.1
>
>