| Message ID | 20220315141318.329309-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Kieran Bingham (2022-03-15 14:13:18) > These changes are expected to be squashed into the AF implementation. > > The changes here include: > - Correct alignment of documentation *'s > - Normalise return value documentation > - Fix spelling of /afEstemateVariance/afEstimateVariance/ > - Expand documentation of Af::afEstimateVariance > > With these, for Kate's patch: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > I have a few more potential changes, but they can be handled as patches > on top and reviewed independently. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 76 +++++++++++++++++++++++----------- > 1 file changed, 52 insertions(+), 24 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index 17055c043200..541b200bcc61 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -33,25 +33,25 @@ > * \var kAfMinGridWidth > * \brief the minimum width of AF grid. > * The minimum grid horizontal dimensions. > -*/ > + */ > > /** > * \var kAfMinGridHeight > * \brief the minimum height of AF grid. > * The minimum grid vertical dimensions. > -*/ > + */ > > /** > * \var kAfMaxGridWidth > * \brief the maximum width of AF grid. > * The maximum grid horizontal dimensions. > -*/ > + */ > > /** > * \var kAfMaxGridHeight > * \brief The maximum height of AF grid. > * The maximum grid vertical dimensions. > -*/ > + */ > > /** > * \var kAfMinGridBlockWidth > @@ -127,6 +127,7 @@ static struct ipu3_uapi_af_filter_config afFilterConfigDefault = { > /** > * \class Af > * \brief An auto-focus algorithm based on IPU3 statistics > + * > * This algorithm is used to determine the position of the lens to make a > * focused image. The IPU3 AF processing block computes the statistics that > * are composed by two types of filtered value and stores in a AF buffer. > @@ -158,7 +159,7 @@ void Af::prepare(IPAContext &context, ipu3_uapi_params *params) > * \brief Configure the Af given a configInfo > * \param[in] context The shared IPA context > * \param[in] configInfo The IPA configuration data > - * \return 0 > + * \return 0 on success, a negative error code otherwise > */ > int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > { > @@ -195,7 +196,9 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > /** > * \brief AF coarse scan > + * > * Find a near focused image using a coarse step. The step is determined by coarseSearchStep. > + * > * \param[in] context The shared IPA context > */ > void Af::afCoarseScan(IPAContext &context) > @@ -220,7 +223,9 @@ void Af::afCoarseScan(IPAContext &context) > > /** > * \brief AF fine scan > + * > * Find an optimum lens position with moving 1 step for each search. > + * > * \param[in] context The shared IPA context > */ > void Af::afFineScan(IPAContext &context) > @@ -239,7 +244,9 @@ void Af::afFineScan(IPAContext &context) > > /** > * \brief AF reset > + * > * Reset all the parameters to start over the AF process. > + * > * \param[in] context The shared IPA context > */ > void Af::afReset(IPAContext &context) > @@ -260,11 +267,13 @@ void Af::afReset(IPAContext &context) > > /** > * \brief AF variance comparison. > - * It always picks the largest variance to replace the previous one. The image > - * with a larger variance also indicates it is a clearer image than previous > - * one. If it finds the negative sign of derivative, it returns immediately. > * \param[in] context The IPA context > * \param min_step The VCM movement step. > + * > + * We always pick the largest variance to replace the previous one. The image > + * with a larger variance also indicates it is a clearer image than previous > + * one. If we find a negative derivative, we return immediately. > + * > * \return True, if it finds a AF value. > */ > bool Af::afScan(IPAContext &context, int min_step) > @@ -278,7 +287,7 @@ bool Af::afScan(IPAContext &context, int min_step) > * Find the maximum of the variance by estimating its > * derivative. If the direction changes, it means we have > * passed a maximum one step before. > - */ > + */ > if ((currentVariance_ - context.frameContext.af.maxVariance) >= > -(context.frameContext.af.maxVariance * 0.1)) { > /* > @@ -313,8 +322,7 @@ bool Af::afScan(IPAContext &context, int min_step) > > /** > * \brief Determine the frame to be ignored. > - * \return Return true the frame is ignored. > - * \return Return false the frame should be processed. > + * \return Return True if the frame should be ignored, false otherwise > */ > bool Af::afNeedIgnoreFrame() > { > @@ -334,9 +342,22 @@ void Af::afIgnoreFrameReset() > } > > /** > - * \brief Estemate variance > + * \brief Estimate variance > + * \param y_item The AF filter data set from the IPU3 statistics buffer > + * \param len The quantity of table item entries which are valid to process > + * \param isY1 Selects between filter Y1 or Y2 to calculate the variance > + * > + * Calculate the mean of the data set provided by \a y_item, and then calculate > + * the variance of that data set from the mean. > + * > + * The operation can work on one of two sets of values contained within the > + * y_item data set supplied by the IPU3. The two data sets are the results of > + * both the Y1 and Y2 filters which are used to support coarse (Y1) and fine > + * (Y2) calculations of the contrast. > + * > + * \return The variance of the values in the data set \a y_item selected by \a isY1 > */ > -double Af::afEstemateVariance(y_table_item_t *y_item, uint32_t len, > +double Af::afEstimateVariance(y_table_item_t *y_item, uint32_t len, Of course, there is also the corresponding update to the class definition in the header for this change. -- Kieran > bool isY1) > { > uint32_t z = 0; > @@ -363,21 +384,25 @@ double Af::afEstemateVariance(y_table_item_t *y_item, uint32_t len, > > /** > * \brief Determine out-of-focus situation. > + * \param context The IPA context. > + * > * Out-of-focus means that the variance change rate for a focused and a new > * variance is greater than a threshold. > - * \param context The IPA context. > - * \return If it is out-of-focus, return true. > - * \return If is is focused, return false. > + * > + * \return True if the variance threshold is crossed indicating lost focus, > + * false otherwise. > */ > bool Af::afIsOutOfFocus(IPAContext context) > { > const uint32_t diff_var = std::abs(currentVariance_ - > context.frameContext.af.maxVariance); > const double var_ratio = diff_var / context.frameContext.af.maxVariance; > + > LOG(IPU3Af, Debug) << "Variance change rate: " > << var_ratio > << " Current VCM step: " > << context.frameContext.af.focus; > + > if (var_ratio > kMaxChange) > return true; > else > @@ -386,16 +411,19 @@ bool Af::afIsOutOfFocus(IPAContext context) > > /** > * \brief Determine the max contrast image and lens position. > - * Ideally, a clear image also has a raletively higher contrast. So, every > - * images for each focus step should be tested to find a optimal focus step. > + * \param[in] context The IPA context. > + * \param[in] stats The statistic buffer of IPU3. > + * > + * Ideally, a clear image also has a relatively higher contrast. So, every > + * image for each focus step should be tested to find an optimal focus step. > + * > * The Hill Climbing Algorithm[1] is used to find the maximum variance of the > - * AF statistic which is the AF output of IPU3. The focus step is increased > + * AF statistics which is the AF output of IPU3. The focus step is increased > * then the variance of the AF statistic is estimated. If it finds the negative > - * derivative which means we just passed the peak, the best focus is found. > + * derivative we have just passed the peak, and we infer that the best focus is > + * found. > * > * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing > - * \param[in] context The IPA context. > - * \param[in] stats The statistic buffer of IPU3. > */ > void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > { > @@ -415,9 +443,9 @@ void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > * For fine: y2 results are used. > */ > if (coarseCompleted_) > - currentVariance_ = afEstemateVariance(y_item, afRawBufferLen, false); > + currentVariance_ = afEstimateVariance(y_item, afRawBufferLen, false); > else > - currentVariance_ = afEstemateVariance(y_item, afRawBufferLen, true); > + currentVariance_ = afEstimateVariance(y_item, afRawBufferLen, true); > > if (!context.frameContext.af.stable) { > afCoarseScan(context); > -- > 2.32.0 >
Hi Kieran, On 15/03/2022 15:13, Kieran Bingham wrote: > These changes are expected to be squashed into the AF implementation. > > The changes here include: > - Correct alignment of documentation *'s > - Normalise return value documentation > - Fix spelling of /afEstemateVariance/afEstimateVariance/ > - Expand documentation of Af::afEstimateVariance Thanks for the improvements ! > > With these, for Kate's patch: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > I have a few more potential changes, but they can be handled as patches > on top and reviewed independently. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/af.cpp | 76 +++++++++++++++++++++++----------- > 1 file changed, 52 insertions(+), 24 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp > index 17055c043200..541b200bcc61 100644 > --- a/src/ipa/ipu3/algorithms/af.cpp > +++ b/src/ipa/ipu3/algorithms/af.cpp > @@ -33,25 +33,25 @@ > * \var kAfMinGridWidth > * \brief the minimum width of AF grid. > * The minimum grid horizontal dimensions. > -*/ > + */ > > /** > * \var kAfMinGridHeight > * \brief the minimum height of AF grid. > * The minimum grid vertical dimensions. > -*/ > + */ > > /** > * \var kAfMaxGridWidth > * \brief the maximum width of AF grid. > * The maximum grid horizontal dimensions. > -*/ > + */ > > /** > * \var kAfMaxGridHeight > * \brief The maximum height of AF grid. > * The maximum grid vertical dimensions. > -*/ > + */ > > /** > * \var kAfMinGridBlockWidth > @@ -127,6 +127,7 @@ static struct ipu3_uapi_af_filter_config afFilterConfigDefault = { > /** > * \class Af > * \brief An auto-focus algorithm based on IPU3 statistics > + * > * This algorithm is used to determine the position of the lens to make a > * focused image. The IPU3 AF processing block computes the statistics that > * are composed by two types of filtered value and stores in a AF buffer. > @@ -158,7 +159,7 @@ void Af::prepare(IPAContext &context, ipu3_uapi_params *params) > * \brief Configure the Af given a configInfo > * \param[in] context The shared IPA context > * \param[in] configInfo The IPA configuration data > - * \return 0 > + * \return 0 on success, a negative error code otherwise > */ > int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > { > @@ -195,7 +196,9 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > /** > * \brief AF coarse scan > + * > * Find a near focused image using a coarse step. The step is determined by coarseSearchStep. > + * > * \param[in] context The shared IPA context > */ > void Af::afCoarseScan(IPAContext &context) > @@ -220,7 +223,9 @@ void Af::afCoarseScan(IPAContext &context) > > /** > * \brief AF fine scan > + * > * Find an optimum lens position with moving 1 step for each search. > + * > * \param[in] context The shared IPA context > */ > void Af::afFineScan(IPAContext &context) > @@ -239,7 +244,9 @@ void Af::afFineScan(IPAContext &context) > > /** > * \brief AF reset > + * > * Reset all the parameters to start over the AF process. > + * > * \param[in] context The shared IPA context > */ > void Af::afReset(IPAContext &context) > @@ -260,11 +267,13 @@ void Af::afReset(IPAContext &context) > > /** > * \brief AF variance comparison. > - * It always picks the largest variance to replace the previous one. The image > - * with a larger variance also indicates it is a clearer image than previous > - * one. If it finds the negative sign of derivative, it returns immediately. > * \param[in] context The IPA context > * \param min_step The VCM movement step. > + * > + * We always pick the largest variance to replace the previous one. The image > + * with a larger variance also indicates it is a clearer image than previous > + * one. If we find a negative derivative, we return immediately. > + * > * \return True, if it finds a AF value. > */ > bool Af::afScan(IPAContext &context, int min_step) > @@ -278,7 +287,7 @@ bool Af::afScan(IPAContext &context, int min_step) > * Find the maximum of the variance by estimating its > * derivative. If the direction changes, it means we have > * passed a maximum one step before. > - */ > + */ > if ((currentVariance_ - context.frameContext.af.maxVariance) >= > -(context.frameContext.af.maxVariance * 0.1)) { > /* > @@ -313,8 +322,7 @@ bool Af::afScan(IPAContext &context, int min_step) > > /** > * \brief Determine the frame to be ignored. > - * \return Return true the frame is ignored. > - * \return Return false the frame should be processed. > + * \return Return True if the frame should be ignored, false otherwise > */ > bool Af::afNeedIgnoreFrame() > { > @@ -334,9 +342,22 @@ void Af::afIgnoreFrameReset() > } > > /** > - * \brief Estemate variance > + * \brief Estimate variance Nice typo catch ! > + * \param y_item The AF filter data set from the IPU3 statistics buffer > + * \param len The quantity of table item entries which are valid to process > + * \param isY1 Selects between filter Y1 or Y2 to calculate the variance > + * > + * Calculate the mean of the data set provided by \a y_item, and then calculate > + * the variance of that data set from the mean. > + * > + * The operation can work on one of two sets of values contained within the > + * y_item data set supplied by the IPU3. The two data sets are the results of > + * both the Y1 and Y2 filters which are used to support coarse (Y1) and fine s/fine/finer maybe ? > + * (Y2) calculations of the contrast. > + * > + * \return The variance of the values in the data set \a y_item selected by \a isY1 > */ > -double Af::afEstemateVariance(y_table_item_t *y_item, uint32_t len, > +double Af::afEstimateVariance(y_table_item_t *y_item, uint32_t len, > bool isY1) > { > uint32_t z = 0; > @@ -363,21 +384,25 @@ double Af::afEstemateVariance(y_table_item_t *y_item, uint32_t len, > > /** > * \brief Determine out-of-focus situation. > + * \param context The IPA context. > + * > * Out-of-focus means that the variance change rate for a focused and a new > * variance is greater than a threshold. > - * \param context The IPA context. > - * \return If it is out-of-focus, return true. > - * \return If is is focused, return false. > + * > + * \return True if the variance threshold is crossed indicating lost focus, > + * false otherwise. > */ > bool Af::afIsOutOfFocus(IPAContext context) > { > const uint32_t diff_var = std::abs(currentVariance_ - > context.frameContext.af.maxVariance); > const double var_ratio = diff_var / context.frameContext.af.maxVariance; > + > LOG(IPU3Af, Debug) << "Variance change rate: " > << var_ratio > << " Current VCM step: " > << context.frameContext.af.focus; > + > if (var_ratio > kMaxChange) > return true; > else > @@ -386,16 +411,19 @@ bool Af::afIsOutOfFocus(IPAContext context) > > /** > * \brief Determine the max contrast image and lens position. > - * Ideally, a clear image also has a raletively higher contrast. So, every > - * images for each focus step should be tested to find a optimal focus step. > + * \param[in] context The IPA context. > + * \param[in] stats The statistic buffer of IPU3. s/statistic/statistics > + * > + * Ideally, a clear image also has a relatively higher contrast. So, every > + * image for each focus step should be tested to find an optimal focus step. > + * > * The Hill Climbing Algorithm[1] is used to find the maximum variance of the > - * AF statistic which is the AF output of IPU3. The focus step is increased > + * AF statistics which is the AF output of IPU3. The focus step is increased > * then the variance of the AF statistic is estimated. If it finds the negative > - * derivative which means we just passed the peak, the best focus is found. > + * derivative we have just passed the peak, and we infer that the best focus is > + * found. > * > * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing > - * \param[in] context The IPA context. > - * \param[in] stats The statistic buffer of IPU3. > */ > void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > { > @@ -415,9 +443,9 @@ void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) > * For fine: y2 results are used. > */ > if (coarseCompleted_) > - currentVariance_ = afEstemateVariance(y_item, afRawBufferLen, false); > + currentVariance_ = afEstimateVariance(y_item, afRawBufferLen, false); > else > - currentVariance_ = afEstemateVariance(y_item, afRawBufferLen, true); > + currentVariance_ = afEstimateVariance(y_item, afRawBufferLen, true); > > if (!context.frameContext.af.stable) { > afCoarseScan(context); Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp index 17055c043200..541b200bcc61 100644 --- a/src/ipa/ipu3/algorithms/af.cpp +++ b/src/ipa/ipu3/algorithms/af.cpp @@ -33,25 +33,25 @@ * \var kAfMinGridWidth * \brief the minimum width of AF grid. * The minimum grid horizontal dimensions. -*/ + */ /** * \var kAfMinGridHeight * \brief the minimum height of AF grid. * The minimum grid vertical dimensions. -*/ + */ /** * \var kAfMaxGridWidth * \brief the maximum width of AF grid. * The maximum grid horizontal dimensions. -*/ + */ /** * \var kAfMaxGridHeight * \brief The maximum height of AF grid. * The maximum grid vertical dimensions. -*/ + */ /** * \var kAfMinGridBlockWidth @@ -127,6 +127,7 @@ static struct ipu3_uapi_af_filter_config afFilterConfigDefault = { /** * \class Af * \brief An auto-focus algorithm based on IPU3 statistics + * * This algorithm is used to determine the position of the lens to make a * focused image. The IPU3 AF processing block computes the statistics that * are composed by two types of filtered value and stores in a AF buffer. @@ -158,7 +159,7 @@ void Af::prepare(IPAContext &context, ipu3_uapi_params *params) * \brief Configure the Af given a configInfo * \param[in] context The shared IPA context * \param[in] configInfo The IPA configuration data - * \return 0 + * \return 0 on success, a negative error code otherwise */ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) { @@ -195,7 +196,9 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo) /** * \brief AF coarse scan + * * Find a near focused image using a coarse step. The step is determined by coarseSearchStep. + * * \param[in] context The shared IPA context */ void Af::afCoarseScan(IPAContext &context) @@ -220,7 +223,9 @@ void Af::afCoarseScan(IPAContext &context) /** * \brief AF fine scan + * * Find an optimum lens position with moving 1 step for each search. + * * \param[in] context The shared IPA context */ void Af::afFineScan(IPAContext &context) @@ -239,7 +244,9 @@ void Af::afFineScan(IPAContext &context) /** * \brief AF reset + * * Reset all the parameters to start over the AF process. + * * \param[in] context The shared IPA context */ void Af::afReset(IPAContext &context) @@ -260,11 +267,13 @@ void Af::afReset(IPAContext &context) /** * \brief AF variance comparison. - * It always picks the largest variance to replace the previous one. The image - * with a larger variance also indicates it is a clearer image than previous - * one. If it finds the negative sign of derivative, it returns immediately. * \param[in] context The IPA context * \param min_step The VCM movement step. + * + * We always pick the largest variance to replace the previous one. The image + * with a larger variance also indicates it is a clearer image than previous + * one. If we find a negative derivative, we return immediately. + * * \return True, if it finds a AF value. */ bool Af::afScan(IPAContext &context, int min_step) @@ -278,7 +287,7 @@ bool Af::afScan(IPAContext &context, int min_step) * Find the maximum of the variance by estimating its * derivative. If the direction changes, it means we have * passed a maximum one step before. - */ + */ if ((currentVariance_ - context.frameContext.af.maxVariance) >= -(context.frameContext.af.maxVariance * 0.1)) { /* @@ -313,8 +322,7 @@ bool Af::afScan(IPAContext &context, int min_step) /** * \brief Determine the frame to be ignored. - * \return Return true the frame is ignored. - * \return Return false the frame should be processed. + * \return Return True if the frame should be ignored, false otherwise */ bool Af::afNeedIgnoreFrame() { @@ -334,9 +342,22 @@ void Af::afIgnoreFrameReset() } /** - * \brief Estemate variance + * \brief Estimate variance + * \param y_item The AF filter data set from the IPU3 statistics buffer + * \param len The quantity of table item entries which are valid to process + * \param isY1 Selects between filter Y1 or Y2 to calculate the variance + * + * Calculate the mean of the data set provided by \a y_item, and then calculate + * the variance of that data set from the mean. + * + * The operation can work on one of two sets of values contained within the + * y_item data set supplied by the IPU3. The two data sets are the results of + * both the Y1 and Y2 filters which are used to support coarse (Y1) and fine + * (Y2) calculations of the contrast. + * + * \return The variance of the values in the data set \a y_item selected by \a isY1 */ -double Af::afEstemateVariance(y_table_item_t *y_item, uint32_t len, +double Af::afEstimateVariance(y_table_item_t *y_item, uint32_t len, bool isY1) { uint32_t z = 0; @@ -363,21 +384,25 @@ double Af::afEstemateVariance(y_table_item_t *y_item, uint32_t len, /** * \brief Determine out-of-focus situation. + * \param context The IPA context. + * * Out-of-focus means that the variance change rate for a focused and a new * variance is greater than a threshold. - * \param context The IPA context. - * \return If it is out-of-focus, return true. - * \return If is is focused, return false. + * + * \return True if the variance threshold is crossed indicating lost focus, + * false otherwise. */ bool Af::afIsOutOfFocus(IPAContext context) { const uint32_t diff_var = std::abs(currentVariance_ - context.frameContext.af.maxVariance); const double var_ratio = diff_var / context.frameContext.af.maxVariance; + LOG(IPU3Af, Debug) << "Variance change rate: " << var_ratio << " Current VCM step: " << context.frameContext.af.focus; + if (var_ratio > kMaxChange) return true; else @@ -386,16 +411,19 @@ bool Af::afIsOutOfFocus(IPAContext context) /** * \brief Determine the max contrast image and lens position. - * Ideally, a clear image also has a raletively higher contrast. So, every - * images for each focus step should be tested to find a optimal focus step. + * \param[in] context The IPA context. + * \param[in] stats The statistic buffer of IPU3. + * + * Ideally, a clear image also has a relatively higher contrast. So, every + * image for each focus step should be tested to find an optimal focus step. + * * The Hill Climbing Algorithm[1] is used to find the maximum variance of the - * AF statistic which is the AF output of IPU3. The focus step is increased + * AF statistics which is the AF output of IPU3. The focus step is increased * then the variance of the AF statistic is estimated. If it finds the negative - * derivative which means we just passed the peak, the best focus is found. + * derivative we have just passed the peak, and we infer that the best focus is + * found. * * [1] Hill Climbing Algorithm, https://en.wikipedia.org/wiki/Hill_climbing - * \param[in] context The IPA context. - * \param[in] stats The statistic buffer of IPU3. */ void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) { @@ -415,9 +443,9 @@ void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats) * For fine: y2 results are used. */ if (coarseCompleted_) - currentVariance_ = afEstemateVariance(y_item, afRawBufferLen, false); + currentVariance_ = afEstimateVariance(y_item, afRawBufferLen, false); else - currentVariance_ = afEstemateVariance(y_item, afRawBufferLen, true); + currentVariance_ = afEstimateVariance(y_item, afRawBufferLen, true); if (!context.frameContext.af.stable) { afCoarseScan(context);