| Message ID | 20240616163910.5506-9-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | ea43e056a83fcfcf627669b4e5ee922d97c95d4b |
| Headers | show |
| Series |
|
| Related | show |
On Sun, Jun 16, 2024 at 07:39:06PM +0300, Laurent Pinchart wrote: > 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@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@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); > }
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@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@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 >
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); }
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@ideasonboard.com> --- src/ipa/rkisp1/algorithms/agc.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)