From 30114cadd8cb651231efcd5fe32f2c8a0a72b7b4 Mon Sep 17 00:00:00 2001 From: Nick Hollinghurst Date: Fri, 20 Jun 2025 13:42:22 +0100 Subject: [PATCH] ipa: rpi: Defer initialising AF LensPosition ControlInfo and value This fixes two small bugs: We previously populated LensPosition's ControlInfo with hard-coded values, ignoring the tuning file. Now we query the AfAlgorithm to get limits (over all AF ranges) and default (for AfRangeNormal). We previously sent a default position to the lens driver, even when a user-specified starting position would follow. Defer doing this, to reduce unnecessary lens movement at startup (for some drivers). Bug: https://bugs.libcamera.org/show_bug.cgi?id=258 Signed-off-by: Nick Hollinghurst Signed-off-by: Naushir Patuck Reviewed-by: Naushir Patuck Signed-off-by: Kieran Bingham --- src/ipa/rpi/common/ipa_base.cpp | 53 +++++++++++++++++---------- src/ipa/rpi/controller/af_algorithm.h | 8 +++- src/ipa/rpi/controller/rpi/af.cpp | 16 +++++++- src/ipa/rpi/controller/rpi/af.h | 4 +- 4 files changed, 57 insertions(+), 24 deletions(-) diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index 6565f5366..a5bdcbb58 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -233,25 +233,6 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa agcStatus.analogueGain = defaultAnalogueGain; applyAGC(&agcStatus, ctrls); - /* - * Set the lens to the default (typically hyperfocal) position - * on first start. - */ - if (lensPresent_) { - RPiController::AfAlgorithm *af = - dynamic_cast(controller_.getAlgorithm("af")); - - if (af) { - float defaultPos = - ipaAfControls.at(&controls::LensPosition).def().get(); - ControlList lensCtrl(lensCtrls_); - int32_t hwpos; - - af->setLensPosition(defaultPos, &hwpos); - lensCtrl.set(V4L2_CID_FOCUS_ABSOLUTE, hwpos); - result->lensControls = std::move(lensCtrl); - } - } } result->sensorControls = std::move(ctrls); @@ -281,8 +262,20 @@ int32_t IpaBase::configure(const IPACameraSensorInfo &sensorInfo, const ConfigPa ctrlMap.merge(ControlInfoMap::Map(ipaColourControls)); /* Declare Autofocus controls, only if we have a controllable lens */ - if (lensPresent_) + if (lensPresent_) { ctrlMap.merge(ControlInfoMap::Map(ipaAfControls)); + RPiController::AfAlgorithm *af = + dynamic_cast(controller_.getAlgorithm("af")); + if (af) { + double min, max, dflt; + af->getLensLimits(min, max); + dflt = af->getDefaultLensPosition(); + ctrlMap[&controls::LensPosition] = + ControlInfo(static_cast(min), + static_cast(max), + static_cast(dflt)); + } + } result->controlInfo = ControlInfoMap(std::move(ctrlMap), controls::controls); @@ -320,6 +313,26 @@ void IpaBase::start(const ControlList &controls, StartResult *result) /* Make a note of this as it tells us the HDR status of the first few frames. */ hdrStatus_ = agcStatus.hdr; + /* + * AF: If no lens position was specified, drive lens to a default position. + * This had to be deferred (not initialised by a constructor) until here + * to ensure that exactly ONE starting position is sent to the lens driver. + * It should be the static API default, not dependent on AF range or mode. + */ + if (firstStart_ && lensPresent_) { + RPiController::AfAlgorithm *af = dynamic_cast( + controller_.getAlgorithm("af")); + if (af && !af->getLensPosition()) { + int32_t hwpos; + double pos = af->getDefaultLensPosition(); + if (af->setLensPosition(pos, &hwpos, true)) { + ControlList lensCtrls(lensCtrls_); + lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, hwpos); + setLensControls.emit(lensCtrls); + } + } + } + /* * Initialise frame counts, and decide how many frames must be hidden or * "mistrusted", which depends on whether this is a startup from cold, diff --git a/src/ipa/rpi/controller/af_algorithm.h b/src/ipa/rpi/controller/af_algorithm.h index ad9b57545..382609f9b 100644 --- a/src/ipa/rpi/controller/af_algorithm.h +++ b/src/ipa/rpi/controller/af_algorithm.h @@ -33,6 +33,10 @@ public: * * getMode() is provided mainly for validating controls. * getLensPosition() is provided for populating DeviceStatus. + * + * getDefaultlensPosition() and getLensLimits() were added for + * populating ControlInfoMap. They return the static API limits + * which should be independent of the current range or mode. */ enum AfRange { AfRangeNormal = 0, @@ -66,7 +70,9 @@ public: } virtual void setMode(AfMode mode) = 0; virtual AfMode getMode() const = 0; - virtual bool setLensPosition(double dioptres, int32_t *hwpos) = 0; + virtual double getDefaultLensPosition() const = 0; + virtual void getLensLimits(double &min, double &max) const = 0; + virtual bool setLensPosition(double dioptres, int32_t *hwpos, bool force = false) = 0; virtual std::optional getLensPosition() const = 0; virtual void triggerScan() = 0; virtual void cancelScan() = 0; diff --git a/src/ipa/rpi/controller/rpi/af.cpp b/src/ipa/rpi/controller/rpi/af.cpp index 2157eb94f..041cb51db 100644 --- a/src/ipa/rpi/controller/rpi/af.cpp +++ b/src/ipa/rpi/controller/rpi/af.cpp @@ -715,11 +715,23 @@ void Af::setWindows(libcamera::Span const &wins) invalidateWeights(); } -bool Af::setLensPosition(double dioptres, int *hwpos) +double Af::getDefaultLensPosition() const +{ + return cfg_.ranges[AfRangeNormal].focusDefault; +} + +void Af::getLensLimits(double &min, double &max) const +{ + /* Limits for manual focus are set by map, not by ranges */ + min = cfg_.map.domain().start; + max = cfg_.map.domain().end; +} + +bool Af::setLensPosition(double dioptres, int *hwpos, bool force) { bool changed = false; - if (mode_ == AfModeManual) { + if (mode_ == AfModeManual || force) { LOG(RPiAf, Debug) << "setLensPosition: " << dioptres; ftarget_ = cfg_.map.domain().clamp(dioptres); changed = !(initted_ && fsmooth_ == ftarget_); diff --git a/src/ipa/rpi/controller/rpi/af.h b/src/ipa/rpi/controller/rpi/af.h index 317a51f3e..d8ece1ab1 100644 --- a/src/ipa/rpi/controller/rpi/af.h +++ b/src/ipa/rpi/controller/rpi/af.h @@ -54,7 +54,9 @@ public: void setWindows(libcamera::Span const &wins) override; void setMode(AfMode mode) override; AfMode getMode() const override; - bool setLensPosition(double dioptres, int32_t *hwpos) override; + double getDefaultLensPosition() const override; + void getLensLimits(double &min, double &max) const override; + bool setLensPosition(double dioptres, int32_t *hwpos, bool force) override; std::optional getLensPosition() const override; void triggerScan() override; void cancelScan() override;