From patchwork Wed Nov 13 13:12:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 21888 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 12362BE173 for ; Wed, 13 Nov 2024 13:13:35 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B2B1865828; Wed, 13 Nov 2024 14:13:34 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="rC9d1K2l"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D979C65829 for ; Wed, 13 Nov 2024 14:13:31 +0100 (CET) Received: from neptunite.flets-east.jp (unknown [IPv6:2404:7a81:160:2100:eb25:7aa0:7da7:eb24]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8A15119AE; Wed, 13 Nov 2024 14:13:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1731503598; bh=KvbLXLk7pFvdhO4NLtCTjG8JWMuBAf3sjj9X69UWPcM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rC9d1K2lIqflen9juH0COC4oETXcA9QhGBYs8OQdPJbr8aWA2SB1sXbnlj/CgNi+n I2J1vUDfEj1N0AheU1xizrZe7AcOnjzRaD/JOa18d/d+Hm1RHBmC6K7MyiWNI3SPMK XrEG6NAghkvEq44PIFbxFGYGejR5T6v4fyJlcfcA= From: Paul Elder To: libcamera-devel@lists.libcamera.org Cc: laurent.pinchart@ideasonboard.com, jacopo.mondi@ideasonboard.com, naush@raspberrypi.com, david.plowman@raspberrypi.com, Jacopo Mondi , Paul Elder Subject: [PATCH v3 5/8] ipa: raspberry: Port to the new AEGC controls Date: Wed, 13 Nov 2024 22:12:53 +0900 Message-Id: <20241113131256.3170817-6-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20241113131256.3170817-1-paul.elder@ideasonboard.com> References: <20241113131256.3170817-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Jacopo Mondi 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 Signed-off-by: Paul Elder Reviewed-by: Paul Elder --- Changes in v3: - recovered from 2-year-old bitrot --- src/ipa/rpi/common/ipa_base.cpp | 74 +++++++++++++++++++--- 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, 150 insertions(+), 24 deletions(-) diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index 468f36a82ab2..2d26d497be85 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -55,8 +55,13 @@ 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(controls::ExposureTimeModeAuto), + static_cast(controls::ExposureTimeModeManual)) }, { &controls::ExposureTime, ControlInfo(0, 66666) }, + { &controls::AnalogueGainMode, + ControlInfo(static_cast(controls::AnalogueGainModeAuto), + static_cast(controls::AnalogueGainModeManual)) }, { &controls::AnalogueGain, ControlInfo(1.0f, 16.0f) }, { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) }, { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) }, @@ -760,21 +765,22 @@ void IpaBase::applyControls(const ControlList &controls) << " = " << ctrl.second.toString(); switch (ctrl.first) { - case controls::AE_ENABLE: { + case controls::EXPOSURE_TIME_MODE: { RPiController::AgcAlgorithm *agc = dynamic_cast( controller_.getAlgorithm("agc")); if (!agc) { LOG(IPARPI, Warning) - << "Could not set AE_ENABLE - no AGC algorithm"; + << "Could not set EXPOSURE_TIME_MODE - no AGC algorithm"; break; } - if (ctrl.second.get() == false) - agc->disableAuto(); + if (ctrl.second.get() == controls::ExposureTimeModeManual) + agc->disableAutoShutter(); else - agc->enableAuto(); + agc->enableAutoShutter(); - libcameraMetadata_.set(controls::AeEnable, ctrl.second.get()); + libcameraMetadata_.set(controls::ExposureTimeMode, + ctrl.second.get()); break; } @@ -787,6 +793,13 @@ void IpaBase::applyControls(const ControlList &controls) break; } + /* + * Ignore manual exposure time when the auto exposure + * algorithm is running. + */ + if (agc->autoShutterEnabled()) + break; + /* The control provides units of microseconds. */ agc->setFixedShutter(0, ctrl.second.get() * 1.0us); @@ -794,6 +807,25 @@ void IpaBase::applyControls(const ControlList &controls) break; } + case controls::ANALOGUE_GAIN_MODE: { + RPiController::AgcAlgorithm *agc = dynamic_cast( + controller_.getAlgorithm("agc")); + if (!agc) { + LOG(IPARPI, Warning) + << "Could not set ANALOGUE_GAIN_MODE - no AGC algorithm"; + break; + } + + if (ctrl.second.get() == controls::AnalogueGainModeManual) + agc->disableAutoGain(); + else + agc->enableAutoGain(); + + libcameraMetadata_.set(controls::AnalogueGainMode, + ctrl.second.get()); + break; + } + case controls::ANALOGUE_GAIN: { RPiController::AgcAlgorithm *agc = dynamic_cast( controller_.getAlgorithm("agc")); @@ -803,6 +835,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()); libcameraMetadata_.set(controls::AnalogueGain, @@ -859,6 +898,13 @@ void IpaBase::applyControls(const ControlList &controls) break; } + /* + * Ignore AE_EXPOSURE_MODE if the shutter or the gain + * are in auto mode. + */ + if (agc->autoShutterEnabled() || agc->autoGainEnabled()) + break; + int32_t idx = ctrl.second.get(); if (ExposureModeTable.count(idx)) { agc->setExposureMode(ExposureModeTable.at(idx)); @@ -1319,9 +1365,19 @@ void IpaBase::reportMetadata(unsigned int ipaContext) } AgcPrepareStatus *agcPrepareStatus = rpiMetadata.getLocked("agc.prepare_status"); - if (agcPrepareStatus) { - libcameraMetadata_.set(controls::AeLocked, agcPrepareStatus->locked); + if (agcPrepareStatus) libcameraMetadata_.set(controls::DigitalGain, agcPrepareStatus->digitalGain); + + RPiController::AgcAlgorithm *agc = dynamic_cast( + controller_.getAlgorithm("agc")); + if (agc) { + if (!agc->autoShutterEnabled() && !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("lux.status"); diff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h index 1132de7e050e..64ab024beecd 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 enableAutoShutter() = 0; + virtual void disableAutoShutter() = 0; + virtual bool autoShutterEnabled() const = 0; + virtual void enableAutoGain() = 0; + virtual void disableAutoGain() = 0; + virtual bool autoGainEnabled() const = 0; virtual void setActiveChannels(const std::vector &activeChannels) = 0; }; diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp index fcf7aec99375..6b038a0197be 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::disableAutoShutter() { - LOG(RPiAgc, Debug) << "disableAuto"; + LOG(RPiAgc, Debug) << "disableAutoShutter"; /* All channels are enabled/disabled together. */ for (auto &data : channelData_) - data.channel.disableAuto(); + data.channel.disableAutoShutter(); } -void Agc::enableAuto() +void Agc::enableAutoShutter() { - LOG(RPiAgc, Debug) << "enableAuto"; + LOG(RPiAgc, Debug) << "enableAutoShutter"; /* All channels are enabled/disabled together. */ for (auto &data : channelData_) - data.channel.enableAuto(); + data.channel.enableAutoShutter(); +} + +bool Agc::autoShutterEnabled() const +{ + LOG(RPiAgc, Debug) << "autoShutterEnabled"; + + /* + * 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.autoShutterEnabled(); +} + +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 5d056f02e2ac..f7519074b022 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 enableAutoShutter() override; + void disableAutoShutter() override; + bool autoShutterEnabled() 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 c9df9b5b6e1d..bea08252029f 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp @@ -316,18 +316,36 @@ int AgcChannel::read(const libcamera::YamlObject ¶ms, return 0; } -void AgcChannel::disableAuto() +void AgcChannel::disableAutoShutter() { fixedShutter_ = status_.shutterTime; - fixedAnalogueGain_ = status_.analogueGain; } -void AgcChannel::enableAuto() +void AgcChannel::enableAutoShutter() { fixedShutter_ = 0s; +} + +bool AgcChannel::autoShutterEnabled() const +{ + return fixedShutter_ == 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 58368889e6a2..5548d772725d 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 enableAutoShutter(); + void disableAutoShutter(); + bool autoShutterEnabled() 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,