<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 28 Oct 2022 at 11:27, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Nicholas Roth via libcamera-devel (2022-10-28 04:17:22)<br>
> From: Nicholas Roth <<a href="mailto:nicholas@rothemail.net" target="_blank">nicholas@rothemail.net</a>><br>
> <br>
> pwl.cpp uses abs() instead of std::abs(), which causes unexpected<br>
> behavior in the Clang compiler used for Android. Replace with<br>
> C++-standard absolute value function std::abs(), which supports<br>
> double-precision absolute values in a standard way.<br>
> <br>
> Signed-off-by: Nicholas Roth <<a href="mailto:nicholas@rothemail.net" target="_blank">nicholas@rothemail.net</a>><br>
<br>
>From Jacopo on v1:<br>
<br>
Reviewed-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br>
<br>
And from me:<br>
<br>
Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>><br>
<br>
Though we really might have to do a bit of extra validation here:<br>
<br>
 <a href="https://stackoverflow.com/questions/21392627/abs-vs-stdabs-what-does-the-reference-say" rel="noreferrer" target="_blank">https://stackoverflow.com/questions/21392627/abs-vs-stdabs-what-does-the-reference-say</a><br>
<br>
Seems a bit worrying.<br>
<br>
Naush - any idea here?</blockquote><div><br></div><div><div>I think this code change ought to be safe - we are including cmath header, and dy is a double,</div><div>so the compiler uses the double abs(double arg) signature.</div><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div>  </div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br>
<br>
<br>
> ---<br>
>  src/ipa/raspberrypi/controller/pwl.cpp | 5 +++--<br>
>  1 file changed, 3 insertions(+), 2 deletions(-)<br>
> <br>
> diff --git a/src/ipa/raspberrypi/controller/pwl.cpp b/src/ipa/raspberrypi/controller/pwl.cpp<br>
> index c59f5fa1..70c2e24b 100644<br>
> --- a/src/ipa/raspberrypi/controller/pwl.cpp<br>
> +++ b/src/ipa/raspberrypi/controller/pwl.cpp<br>
> @@ -6,6 +6,7 @@<br>
>   */<br>
>  <br>
>  #include <cassert><br>
> +#include <cmath><br>
>  #include <stdexcept><br>
>  <br>
>  #include "pwl.h"<br>
> @@ -168,7 +169,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const<br>
>         while (thisSpan != (int)points_.size() - 1) {<br>
>                 double dx = points_[thisSpan + 1].x - points_[thisSpan].x,<br>
>                        dy = points_[thisSpan + 1].y - points_[thisSpan].y;<br>
> -               if (abs(dy) > eps &&<br>
> +               if (std::abs(dy) > eps &&<br>
>                     otherSpan + 1 < (int)other.points_.size() &&<br>
>                     points_[thisSpan + 1].y >=<br>
>                             other.points_[otherSpan + 1].x + eps) {<br>
> @@ -181,7 +182,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const<br>
>                                  points_[thisSpan].y) *<br>
>                                         dx / dy;<br>
>                         thisY = other.points_[++otherSpan].x;<br>
> -               } else if (abs(dy) > eps && otherSpan > 0 &&<br>
> +               } else if (std::abs(dy) > eps && otherSpan > 0 &&<br>
>                            points_[thisSpan + 1].y <=<br>
>                                    other.points_[otherSpan - 1].x - eps) {<br>
>                         /*<br>
> -- <br>
> 2.34.1<br>
><br>
</blockquote></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 28 Oct 2022 at 11:27, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Nicholas Roth via libcamera-devel (2022-10-28 04:17:22)<br>
> From: Nicholas Roth <<a href="mailto:nicholas@rothemail.net" target="_blank">nicholas@rothemail.net</a>><br>
> <br>
> pwl.cpp uses abs() instead of std::abs(), which causes unexpected<br>
> behavior in the Clang compiler used for Android. Replace with<br>
> C++-standard absolute value function std::abs(), which supports<br>
> double-precision absolute values in a standard way.<br>
> <br>
> Signed-off-by: Nicholas Roth <<a href="mailto:nicholas@rothemail.net" target="_blank">nicholas@rothemail.net</a>><br>
<br>
>From Jacopo on v1:<br>
<br>
Reviewed-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br>
<br>
And from me:<br>
<br>
Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>><br>
<br>
Though we really might have to do a bit of extra validation here:<br>
<br>
 <a href="https://stackoverflow.com/questions/21392627/abs-vs-stdabs-what-does-the-reference-say" rel="noreferrer" target="_blank">https://stackoverflow.com/questions/21392627/abs-vs-stdabs-what-does-the-reference-say</a><br>
<br>
Seems a bit worrying.<br>
<br>
Naush - any idea here? <br>
<br>
<br>
> ---<br>
>  src/ipa/raspberrypi/controller/pwl.cpp | 5 +++--<br>
>  1 file changed, 3 insertions(+), 2 deletions(-)<br>
> <br>
> diff --git a/src/ipa/raspberrypi/controller/pwl.cpp b/src/ipa/raspberrypi/controller/pwl.cpp<br>
> index c59f5fa1..70c2e24b 100644<br>
> --- a/src/ipa/raspberrypi/controller/pwl.cpp<br>
> +++ b/src/ipa/raspberrypi/controller/pwl.cpp<br>
> @@ -6,6 +6,7 @@<br>
>   */<br>
>  <br>
>  #include <cassert><br>
> +#include <cmath><br>
>  #include <stdexcept><br>
>  <br>
>  #include "pwl.h"<br>
> @@ -168,7 +169,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const<br>
>         while (thisSpan != (int)points_.size() - 1) {<br>
>                 double dx = points_[thisSpan + 1].x - points_[thisSpan].x,<br>
>                        dy = points_[thisSpan + 1].y - points_[thisSpan].y;<br>
> -               if (abs(dy) > eps &&<br>
> +               if (std::abs(dy) > eps &&<br>
>                     otherSpan + 1 < (int)other.points_.size() &&<br>
>                     points_[thisSpan + 1].y >=<br>
>                             other.points_[otherSpan + 1].x + eps) {<br>
> @@ -181,7 +182,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const<br>
>                                  points_[thisSpan].y) *<br>
>                                         dx / dy;<br>
>                         thisY = other.points_[++otherSpan].x;<br>
> -               } else if (abs(dy) > eps && otherSpan > 0 &&<br>
> +               } else if (std::abs(dy) > eps && otherSpan > 0 &&<br>
>                            points_[thisSpan + 1].y <=<br>
>                                    other.points_[otherSpan - 1].x - eps) {<br>
>                         /*<br>
> -- <br>
> 2.34.1<br>
><br>
</blockquote></div>