[PATCH 08/12] ipa: rkisp1: agc: Simplify predivider calculation

[Kieran Bingham]

Quoting Laurent Pinchart (2024-06-16 17:39:06)
> The condition
> 
>         if (std::pow(std::floor(root), 2) < factor)
>                 predivider = static_cast<uint8_t>(std::ceil(root));
>         else
>                 predivider = static_cast<uint8_t>(std::floor(root));
> 
> can only be false when the factor's root is an integer. In that case,
> std::ceil(root) and std::floor(root) will be equal. The computation can
> thus be simplified by always rounding up.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

This sounds fine to me. I hesitated as I wasn't sure if this was
papering over some floating point issue ... but without conjouring up a
full test suite to prove this ... I'm just going to say:


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


> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 9f3b59b45f95..a61201bb05c9 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -115,12 +115,7 @@ uint8_t Agc::computeHistogramPredivider(const Size &size,
>         int count = mode == RKISP1_CIF_ISP_HISTOGRAM_MODE_RGB_COMBINED ? 3 : 1;
>         double factor = size.width * size.height * count / 65536.0;
>         double root = std::sqrt(factor);
> -       uint8_t predivider;
> -
> -       if (std::pow(std::floor(root), 2) < factor)
> -               predivider = static_cast<uint8_t>(std::ceil(root));
> -       else
> -               predivider = static_cast<uint8_t>(std::floor(root));
> +       uint8_t predivider = static_cast<uint8_t>(std::ceil(root));
>  
>         return std::clamp<uint8_t>(predivider, 3, 127);
>  }
> -- 
> Regards,
> 
> Laurent Pinchart
>