| Message ID | 20220713084317.24268-10-dse@thaumatec.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Daniel, +cc rpi which might find the series interesting On Wed, Jul 13, 2022 at 10:43:15AM +0200, Daniel Semkowicz via libcamera-devel wrote: > When AFTrigger is received, AF algorithm internal state is reset to > initial values. If reset happens between frames, then first contrast > value that arrives after this, is interpreted as contrast for initial > position. This is wrong, because it was measured for lens position I still don't get why the focus position should be reset... But the control description is under-specified in this case, so we should probably decide what to do and also specify that. I sounds natural to me to assume that when the mode moves from Auto to Manual the last computed value is kept until the application doesn't provide a LensPosition. When switching from manual to auto the last applied value is kept until an AF scan is request with AfTriggerStart. Does this sound reasonable ? > before the reset. Skip this first frame to allow correct contrast > measure for the initial lens position. > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> > --- > .../libipa/algorithms/af_hill_climbing.cpp | 8 +++++++ > src/ipa/libipa/algorithms/af_hill_climbing.h | 23 ++++++++++++++++++- > 2 files changed, 30 insertions(+), 1 deletion(-) > > diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.cpp b/src/ipa/libipa/algorithms/af_hill_climbing.cpp > index f666c6c2..28d09176 100644 > --- a/src/ipa/libipa/algorithms/af_hill_climbing.cpp > +++ b/src/ipa/libipa/algorithms/af_hill_climbing.cpp > @@ -84,6 +84,14 @@ namespace ipa::common::algorithms { > * \return New lens position calculated by AF algorithm > */ > > +/** > + * \fn AfHillClimbing::setFramesToSkip() > + * \brief Request AF to skip n frames > + * \param[in] n Number of frames to be skipped > + * > + * Requested number of frames will not be used for AF calculation. > + */ > + > } /* namespace ipa::common::algorithms */ > > } /* namespace libcamera */ > diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.h b/src/ipa/libipa/algorithms/af_hill_climbing.h > index db9fc058..e251f3eb 100644 > --- a/src/ipa/libipa/algorithms/af_hill_climbing.h > +++ b/src/ipa/libipa/algorithms/af_hill_climbing.h > @@ -29,7 +29,7 @@ public: > lensPosition_(0), bestPosition_(0), currentContrast_(0.0), > previousContrast_(0.0), maxContrast_(0.0), maxStep_(0), > coarseCompleted_(false), fineCompleted_(false), > - lowStep_(0), highStep_(kMaxFocusSteps) > + lowStep_(0), highStep_(kMaxFocusSteps), framesToSkip_(0) > { > } > > @@ -92,6 +92,9 @@ protected: > { > currentContrast_ = currentContrast; > > + if (shouldSkipFrame()) > + return lensPosition_; > + > /* If we are in a paused state, we won't process the stats */ > if (pauseState_ == libcamera::controls::AfPauseStatePaused) > return lensPosition_; > @@ -113,6 +116,12 @@ protected: > return lensPosition_; > } > > + void setFramesToSkip(uint32_t n) > + { > + if (n > framesToSkip_) > + framesToSkip_ = n; > + } > + > private: > void afCoarseScan() > { > @@ -191,6 +200,7 @@ private: > coarseCompleted_ = false; > fineCompleted_ = false; > maxContrast_ = 0.0; > + setFramesToSkip(1); > } > > bool afIsOutOfFocus() > @@ -206,6 +216,16 @@ private: > return false; > } > > + bool shouldSkipFrame() > + { > + if (framesToSkip_ > 0) { > + framesToSkip_--; > + return true; > + } > + > + return false; > + } > + > controls::AfModeEnum mode_; > controls::AfStateEnum state_; > controls::AfPauseStateEnum pauseState_; > @@ -229,6 +249,7 @@ private: > > uint32_t lowStep_; > uint32_t highStep_; > + uint32_t framesToSkip_; > > /* > * Maximum focus steps of the VCM control > -- > 2.34.1 >
diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.cpp b/src/ipa/libipa/algorithms/af_hill_climbing.cpp index f666c6c2..28d09176 100644 --- a/src/ipa/libipa/algorithms/af_hill_climbing.cpp +++ b/src/ipa/libipa/algorithms/af_hill_climbing.cpp @@ -84,6 +84,14 @@ namespace ipa::common::algorithms { * \return New lens position calculated by AF algorithm */ +/** + * \fn AfHillClimbing::setFramesToSkip() + * \brief Request AF to skip n frames + * \param[in] n Number of frames to be skipped + * + * Requested number of frames will not be used for AF calculation. + */ + } /* namespace ipa::common::algorithms */ } /* namespace libcamera */ diff --git a/src/ipa/libipa/algorithms/af_hill_climbing.h b/src/ipa/libipa/algorithms/af_hill_climbing.h index db9fc058..e251f3eb 100644 --- a/src/ipa/libipa/algorithms/af_hill_climbing.h +++ b/src/ipa/libipa/algorithms/af_hill_climbing.h @@ -29,7 +29,7 @@ public: lensPosition_(0), bestPosition_(0), currentContrast_(0.0), previousContrast_(0.0), maxContrast_(0.0), maxStep_(0), coarseCompleted_(false), fineCompleted_(false), - lowStep_(0), highStep_(kMaxFocusSteps) + lowStep_(0), highStep_(kMaxFocusSteps), framesToSkip_(0) { } @@ -92,6 +92,9 @@ protected: { currentContrast_ = currentContrast; + if (shouldSkipFrame()) + return lensPosition_; + /* If we are in a paused state, we won't process the stats */ if (pauseState_ == libcamera::controls::AfPauseStatePaused) return lensPosition_; @@ -113,6 +116,12 @@ protected: return lensPosition_; } + void setFramesToSkip(uint32_t n) + { + if (n > framesToSkip_) + framesToSkip_ = n; + } + private: void afCoarseScan() { @@ -191,6 +200,7 @@ private: coarseCompleted_ = false; fineCompleted_ = false; maxContrast_ = 0.0; + setFramesToSkip(1); } bool afIsOutOfFocus() @@ -206,6 +216,16 @@ private: return false; } + bool shouldSkipFrame() + { + if (framesToSkip_ > 0) { + framesToSkip_--; + return true; + } + + return false; + } + controls::AfModeEnum mode_; controls::AfStateEnum state_; controls::AfPauseStateEnum pauseState_; @@ -229,6 +249,7 @@ private: uint32_t lowStep_; uint32_t highStep_; + uint32_t framesToSkip_; /* * Maximum focus steps of the VCM control
When AFTrigger is received, AF algorithm internal state is reset to initial values. If reset happens between frames, then first contrast value that arrives after this, is interpreted as contrast for initial position. This is wrong, because it was measured for lens position before the reset. Skip this first frame to allow correct contrast measure for the initial lens position. Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> --- .../libipa/algorithms/af_hill_climbing.cpp | 8 +++++++ src/ipa/libipa/algorithms/af_hill_climbing.h | 23 ++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-)