[libcamera-devel] [PATCH v4 01/32] ipa: ipu3: Fix style of Doxygen comment blocks

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:29)
> Fix various issues in Doxygen comment blocks:
> 
> - \param requires an [in] or [out] tag
> - \param must come before the body of the documetation
> - Rename coarseSearchStep to kCoarseSearchStep
> - White space and line wrap
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/af.cpp           | 23 ++++++++++-------------
>  src/ipa/ipu3/algorithms/blc.cpp          |  4 ++--
>  src/ipa/ipu3/algorithms/tone_mapping.cpp | 10 +++++-----
>  3 files changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index 4835a0345931..106a7614e2c7 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -196,10 +196,10 @@ 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
> + *
> + * Find a near focused image using a coarse step. The step is determined by
> + * kCoarseSearchStep.
>   */
>  void Af::afCoarseScan(IPAContext &context)
>  {
> @@ -223,10 +223,9 @@ void Af::afCoarseScan(IPAContext &context)
>  
>  /**
>   * \brief AF fine scan
> + * \param[in] context The shared IPA context
>   *
>   * Find an optimum lens position with moving 1 step for each search.
> - *
> - * \param[in] context The shared IPA context
>   */
>  void Af::afFineScan(IPAContext &context)
>  {
> @@ -244,10 +243,9 @@ void Af::afFineScan(IPAContext &context)
>  
>  /**
>   * \brief AF reset
> + * \param[in] context The shared IPA context
>   *
>   * Reset all the parameters to start over the AF process.
> - *
> - * \param[in] context The shared IPA context
>   */
>  void Af::afReset(IPAContext &context)
>  {
> @@ -268,7 +266,7 @@ void Af::afReset(IPAContext &context)
>  /**
>   * \brief AF variance comparison.
>   * \param[in] context The IPA context
> - * \param min_step The VCM movement step.
> + * \param[in] 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
> @@ -343,9 +341,8 @@ void Af::afIgnoreFrameReset()
>  
>  /**
>   * \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
> + * \param[in] y_items The AF filter data set from the IPU3 statistics buffer
> + * \param[in] 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.
> @@ -378,13 +375,13 @@ double Af::afEstimateVariance(Span<const y_table_item_t> y_items, bool isY1)
>  
>  /**
>   * \brief Determine out-of-focus situation.
> - * \param context The IPA context.
> + * \param[in] 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.
>   *
>   * \return True if the variance threshold is crossed indicating lost focus,
> - *         false otherwise.
> + * false otherwise

:-( I prefer the indentation here so it's clearer in the code but I don't
think it matters either way. And would require fixing throughout I
expect.

But otherwise, this all looks reasonable.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

>   */
>  bool Af::afIsOutOfFocus(IPAContext context)
>  {
> diff --git a/src/ipa/ipu3/algorithms/blc.cpp b/src/ipa/ipu3/algorithms/blc.cpp
> index c561aa858b96..c32427c99549 100644
> --- a/src/ipa/ipu3/algorithms/blc.cpp
> +++ b/src/ipa/ipu3/algorithms/blc.cpp
> @@ -38,8 +38,8 @@ BlackLevelCorrection::BlackLevelCorrection()
>  
>  /**
>   * \brief Fill in the parameter structure, and enable black level correction
> - * \param context The shared IPA context
> - * \param params The IPU3 parameters
> + * \param[in] context The shared IPA context
> + * \param[out] params The IPU3 parameters
>   *
>   * Populate the IPU3 parameter structure with the correction values for each
>   * channel and enable the corresponding ImgU block processing.
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> index 49a5558b6faa..c21647e8c51b 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> @@ -49,8 +49,8 @@ int ToneMapping::configure(IPAContext &context,
>  
>  /**
>   * \brief Fill in the parameter structure, and enable gamma control
> - * \param context The shared IPA context
> - * \param params The IPU3 parameters
> + * \param[in] context The shared IPA context
> + * \param[out] params The IPU3 parameters
>   *
>   * Populate the IPU3 parameter structure with our tone mapping look up table and
>   * enable the gamma control module in the processing blocks.
> @@ -71,9 +71,9 @@ void ToneMapping::prepare([[maybe_unused]] IPAContext &context,
>  
>  /**
>   * \brief Calculate the tone mapping look up table
> - * \param context The shared IPA context
> - * \param frameContext The current frame context
> - * \param stats The IPU3 statistics and ISP results
> + * \param[in] context The shared IPA context
> + * \param[in] frameContext The current frame context
> + * \param[in] stats The IPU3 statistics and ISP results
>   *
>   * The tone mapping look up table is generated as an inverse power curve from
>   * our gamma setting.
> -- 
> Regards,
> 
> Laurent Pinchart
>