[PATCH v8.1 05/12] ipa: raspberry: Port to the new AEGC controls
Naushir Patuck
naush at raspberrypi.com
Wed Jan 15 09:55:31 CET 2025
Hi Paul,
On Tue, 14 Jan 2025 at 21:36, Paul Elder <paul.elder at ideasonboard.com> wrote:
>
> From: Jacopo Mondi <jacopo at jmondi.org>
>
> The newly introduced controls to drive the AEGC algorithm allow
> to control the computation of the exposure time and analogue gain
> separately.
>
> The RPi AEGC implementation already computes the shutter and gain
> values separately but does not expose separate functions to control
> them.
>
> Augment the AgcAlgorithm interface to allow pausing/resuming the shutter
> and gain automatic computations separately and plumb them to the newly
> introduced controls.
>
> Add safety checks to ignore ExposureTime and AnalogueGain values if the
> algorithms are not paused, and report the correct AeState value by
> checking if both algorithms have been paused or if they have converged.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
>
Thanks for the update, this looks much better!
Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
Before merging, I would like an ack by @David Plowman if he's fine with this.
> ---
> Changes in v8.1:
> - fix the racy check in handling the auto/manual controls and the manual
> control value control
>
> Changes in v8:
> - add auto as default value for ExposureMode and AnalogueGainMode
> - add todo for control processing order
>
> No change in v7
>
> No change in v6
>
> No change in v5
>
> Changes in v4:
> - s/shutter/exposure/
>
> Changes in v3:
> - recovered from 2-year-old bitrot
> ---
> src/ipa/rpi/common/ipa_base.cpp | 106 +++++++++++++++++----
> src/ipa/rpi/controller/agc_algorithm.h | 8 +-
> src/ipa/rpi/controller/rpi/agc.cpp | 52 ++++++++--
> src/ipa/rpi/controller/rpi/agc.h | 8 +-
> src/ipa/rpi/controller/rpi/agc_channel.cpp | 24 ++++-
> src/ipa/rpi/controller/rpi/agc_channel.h | 8 +-
> 6 files changed, 171 insertions(+), 35 deletions(-)
>
> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
> index 6ff1e22b1..78206bb4a 100644
> --- a/src/ipa/rpi/common/ipa_base.cpp
> +++ b/src/ipa/rpi/common/ipa_base.cpp
> @@ -55,8 +55,15 @@ constexpr Duration controllerMinFrameDuration = 1.0s / 30.0;
>
> /* List of controls handled by the Raspberry Pi IPA */
> const ControlInfoMap::Map ipaControls{
> - { &controls::AeEnable, ControlInfo(false, true) },
> + { &controls::ExposureTimeMode,
> + ControlInfo(static_cast<int32_t>(controls::ExposureTimeModeAuto),
> + static_cast<int32_t>(controls::ExposureTimeModeManual),
> + static_cast<int32_t>(controls::ExposureTimeModeAuto)) },
> { &controls::ExposureTime, ControlInfo(0, 66666) },
> + { &controls::AnalogueGainMode,
> + ControlInfo(static_cast<int32_t>(controls::AnalogueGainModeAuto),
> + static_cast<int32_t>(controls::AnalogueGainModeManual),
> + static_cast<int32_t>(controls::AnalogueGainModeAuto)) },
> { &controls::AnalogueGain, ControlInfo(1.0f, 16.0f) },
> { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },
> { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
> @@ -749,6 +756,46 @@ void IpaBase::applyControls(const ControlList &controls)
> af->setMode(mode->second);
> }
>
> + /*
> + * Because some AE controls are mode-specific, handle the AE-related
> + * mode changes first.
> + */
> + if (controls.contains(controls::EXPOSURE_TIME_MODE)) {
> + RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
> + controller_.getAlgorithm("agc"));
> + if (!agc) {
> + LOG(IPARPI, Warning)
> + << "Could not set EXPOSURE_TIME_MODE - no AGC algorithm";
> + break;
> + }
> +
> + if (ctrl.second.get<int32_t>() == controls::ExposureTimeModeManual)
> + agc->disableAutoExposure();
> + else
> + agc->enableAutoExposure();
> +
> + libcameraMetadata_.set(controls::ExposureTimeMode,
> + ctrl.second.get<int32_t>());
> + }
> +
> + if (controls.contains(controls::ANALOGUE_GAIN_MODE)) {
> + RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
> + controller_.getAlgorithm("agc"));
> + if (!agc) {
> + LOG(IPARPI, Warning)
> + << "Could not set ANALOGUE_GAIN_MODE - no AGC algorithm";
> + break;
> + }
> +
> + if (ctrl.second.get<int32_t>() == controls::AnalogueGainModeManual)
> + agc->disableAutoGain();
> + else
> + agc->enableAutoGain();
> +
> + libcameraMetadata_.set(controls::AnalogueGainMode,
> + ctrl.second.get<int32_t>());
> + }
> +
> /* Iterate over controls */
> for (auto const &ctrl : controls) {
> LOG(IPARPI, Debug) << "Request ctrl: "
> @@ -756,23 +803,8 @@ void IpaBase::applyControls(const ControlList &controls)
> << " = " << ctrl.second.toString();
>
> switch (ctrl.first) {
> - case controls::AE_ENABLE: {
> - RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
> - controller_.getAlgorithm("agc"));
> - if (!agc) {
> - LOG(IPARPI, Warning)
> - << "Could not set AE_ENABLE - no AGC algorithm";
> - break;
> - }
> -
> - if (ctrl.second.get<bool>() == false)
> - agc->disableAuto();
> - else
> - agc->enableAuto();
> -
> - libcameraMetadata_.set(controls::AeEnable, ctrl.second.get<bool>());
> - break;
> - }
> + case controls::EXPOSURE_TIME_MODE:
> + break; /* We already handled this one above */
>
> case controls::EXPOSURE_TIME: {
> RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
> @@ -783,6 +815,13 @@ void IpaBase::applyControls(const ControlList &controls)
> break;
> }
>
> + /*
> + * Ignore manual exposure time when the auto exposure
> + * algorithm is running.
> + */
> + if (agc->autoExposureEnabled())
> + break;
> +
> /* The control provides units of microseconds. */
> agc->setFixedExposureTime(0, ctrl.second.get<int32_t>() * 1.0us);
>
> @@ -790,6 +829,9 @@ void IpaBase::applyControls(const ControlList &controls)
> break;
> }
>
> + case controls::ANALOGUE_GAIN_MODE:
> + break; /* We already handled this one above */
> +
> case controls::ANALOGUE_GAIN: {
> RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
> controller_.getAlgorithm("agc"));
> @@ -799,6 +841,13 @@ void IpaBase::applyControls(const ControlList &controls)
> break;
> }
>
> + /*
> + * Ignore manual analogue gain value when the auto gain
> + * algorithm is running.
> + */
> + if (agc->autoGainEnabled())
> + break;
> +
> agc->setFixedAnalogueGain(0, ctrl.second.get<float>());
>
> libcameraMetadata_.set(controls::AnalogueGain,
> @@ -855,6 +904,13 @@ void IpaBase::applyControls(const ControlList &controls)
> break;
> }
>
> + /*
> + * Ignore AE_EXPOSURE_MODE if the shutter or the gain
> + * are in auto mode.
> + */
> + if (agc->autoExposureEnabled() || agc->autoGainEnabled())
> + break;
> +
> int32_t idx = ctrl.second.get<int32_t>();
> if (ExposureModeTable.count(idx)) {
> agc->setExposureMode(ExposureModeTable.at(idx));
> @@ -1334,9 +1390,19 @@ void IpaBase::reportMetadata(unsigned int ipaContext)
> }
>
> AgcPrepareStatus *agcPrepareStatus = rpiMetadata.getLocked<AgcPrepareStatus>("agc.prepare_status");
> - if (agcPrepareStatus) {
> - libcameraMetadata_.set(controls::AeLocked, agcPrepareStatus->locked);
> + if (agcPrepareStatus)
> libcameraMetadata_.set(controls::DigitalGain, agcPrepareStatus->digitalGain);
> +
> + RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
> + controller_.getAlgorithm("agc"));
> + if (agc) {
> + if (!agc->autoExposureEnabled() && !agc->autoGainEnabled())
> + libcameraMetadata_.set(controls::AeState, controls::AeStateIdle);
> + else if (agcPrepareStatus)
> + libcameraMetadata_.set(controls::AeState,
> + agcPrepareStatus->locked
> + ? controls::AeStateConverged
> + : controls::AeStateSearching);
> }
>
> LuxStatus *luxStatus = rpiMetadata.getLocked<LuxStatus>("lux.status");
> diff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h
> index c97828577..fdaa10e6c 100644
> --- a/src/ipa/rpi/controller/agc_algorithm.h
> +++ b/src/ipa/rpi/controller/agc_algorithm.h
> @@ -30,8 +30,12 @@ public:
> virtual void setMeteringMode(std::string const &meteringModeName) = 0;
> virtual void setExposureMode(std::string const &exposureModeName) = 0;
> virtual void setConstraintMode(std::string const &contraintModeName) = 0;
> - virtual void enableAuto() = 0;
> - virtual void disableAuto() = 0;
> + virtual void enableAutoExposure() = 0;
> + virtual void disableAutoExposure() = 0;
> + virtual bool autoExposureEnabled() const = 0;
> + virtual void enableAutoGain() = 0;
> + virtual void disableAutoGain() = 0;
> + virtual bool autoGainEnabled() const = 0;
> virtual void setActiveChannels(const std::vector<unsigned int> &activeChannels) = 0;
> };
>
> diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp
> index c48fdf156..02bfdb4a5 100644
> --- a/src/ipa/rpi/controller/rpi/agc.cpp
> +++ b/src/ipa/rpi/controller/rpi/agc.cpp
> @@ -74,22 +74,62 @@ int Agc::checkChannel(unsigned int channelIndex) const
> return 0;
> }
>
> -void Agc::disableAuto()
> +void Agc::disableAutoExposure()
> {
> - LOG(RPiAgc, Debug) << "disableAuto";
> + LOG(RPiAgc, Debug) << "disableAutoExposure";
>
> /* All channels are enabled/disabled together. */
> for (auto &data : channelData_)
> - data.channel.disableAuto();
> + data.channel.disableAutoExposure();
> }
>
> -void Agc::enableAuto()
> +void Agc::enableAutoExposure()
> {
> - LOG(RPiAgc, Debug) << "enableAuto";
> + LOG(RPiAgc, Debug) << "enableAutoExposure";
>
> /* All channels are enabled/disabled together. */
> for (auto &data : channelData_)
> - data.channel.enableAuto();
> + data.channel.enableAutoExposure();
> +}
> +
> +bool Agc::autoExposureEnabled() const
> +{
> + LOG(RPiAgc, Debug) << "autoExposureEnabled";
> +
> + /*
> + * We always have at least one channel, and since all channels are
> + * enabled and disabled together we can simply check the first one.
> + */
> + return channelData_[0].channel.autoExposureEnabled();
> +}
> +
> +void Agc::disableAutoGain()
> +{
> + LOG(RPiAgc, Debug) << "disableAutoGain";
> +
> + /* All channels are enabled/disabled together. */
> + for (auto &data : channelData_)
> + data.channel.disableAutoGain();
> +}
> +
> +void Agc::enableAutoGain()
> +{
> + LOG(RPiAgc, Debug) << "enableAutoGain";
> +
> + /* All channels are enabled/disabled together. */
> + for (auto &data : channelData_)
> + data.channel.enableAutoGain();
> +}
> +
> +bool Agc::autoGainEnabled() const
> +{
> + LOG(RPiAgc, Debug) << "autoGainEnabled";
> +
> + /*
> + * We always have at least one channel, and since all channels are
> + * enabled and disabled together we can simply check the first one.
> + */
> + return channelData_[0].channel.autoGainEnabled();
> }
>
> unsigned int Agc::getConvergenceFrames() const
> diff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h
> index 3aca000bb..c3a940bf6 100644
> --- a/src/ipa/rpi/controller/rpi/agc.h
> +++ b/src/ipa/rpi/controller/rpi/agc.h
> @@ -40,8 +40,12 @@ public:
> void setMeteringMode(std::string const &meteringModeName) override;
> void setExposureMode(std::string const &exposureModeName) override;
> void setConstraintMode(std::string const &contraintModeName) override;
> - void enableAuto() override;
> - void disableAuto() override;
> + void enableAutoExposure() override;
> + void disableAutoExposure() override;
> + bool autoExposureEnabled() const override;
> + void enableAutoGain() override;
> + void disableAutoGain() override;
> + bool autoGainEnabled() const override;
> void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
> void prepare(Metadata *imageMetadata) override;
> void process(StatisticsPtr &stats, Metadata *imageMetadata) override;
> diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp
> index 79c459735..e79184b7a 100644
> --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp
> +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp
> @@ -319,18 +319,36 @@ int AgcChannel::read(const libcamera::YamlObject ¶ms,
> return 0;
> }
>
> -void AgcChannel::disableAuto()
> +void AgcChannel::disableAutoExposure()
> {
> fixedExposureTime_ = status_.exposureTime;
> - fixedAnalogueGain_ = status_.analogueGain;
> }
>
> -void AgcChannel::enableAuto()
> +void AgcChannel::enableAutoExposure()
> {
> fixedExposureTime_ = 0s;
> +}
> +
> +bool AgcChannel::autoExposureEnabled() const
> +{
> + return fixedExposureTime_ == 0s;
> +}
> +
> +void AgcChannel::disableAutoGain()
> +{
> + fixedAnalogueGain_ = status_.analogueGain;
> +}
> +
> +void AgcChannel::enableAutoGain()
> +{
> fixedAnalogueGain_ = 0;
> }
>
> +bool AgcChannel::autoGainEnabled() const
> +{
> + return fixedAnalogueGain_ == 0;
> +}
> +
> unsigned int AgcChannel::getConvergenceFrames() const
> {
> /*
> diff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h
> index 734e5efd3..fa697e6fa 100644
> --- a/src/ipa/rpi/controller/rpi/agc_channel.h
> +++ b/src/ipa/rpi/controller/rpi/agc_channel.h
> @@ -96,8 +96,12 @@ public:
> void setMeteringMode(std::string const &meteringModeName);
> void setExposureMode(std::string const &exposureModeName);
> void setConstraintMode(std::string const &contraintModeName);
> - void enableAuto();
> - void disableAuto();
> + void enableAutoExposure();
> + void disableAutoExposure();
> + bool autoExposureEnabled() const;
> + void enableAutoGain();
> + void disableAutoGain();
> + bool autoGainEnabled() const;
> void switchMode(CameraMode const &cameraMode, Metadata *metadata);
> void prepare(Metadata *imageMetadata);
> void process(StatisticsPtr &stats, DeviceStatus const &deviceStatus, Metadata *imageMetadata,
> --
> 2.39.2
>
More information about the libcamera-devel
mailing list