[{"id":25650,"web_url":"https://patchwork.libcamera.org/comment/25650/","msgid":"<166695285295.3974115.17824437985253142294@Monstersaurus>","date":"2022-10-28T10:27:32","subject":"Re: [libcamera-devel] [PATCH v5 06/10] ipa: raspberry: replace\n\tabs() with std::abs()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Nicholas Roth via libcamera-devel (2022-10-28 04:17:22)\n> From: Nicholas Roth \n> \n> pwl.cpp uses abs() instead of std::abs(), which causes unexpected\n> behavior in the Clang compiler used for Android. Replace with\n> C++-standard absolute value function std::abs(), which supports\n> double-precision absolute values in a standard way.\n> \n> Signed-off-by: Nicholas Roth \n\nFrom Jacopo on v1:\n\nReviewed-by: Jacopo Mondi \n\nAnd from me:\n\nReviewed-by: Kieran Bingham \n\nThough we really might have to do a bit of extra validation here:\n\n https://stackoverflow.com/questions/21392627/abs-vs-stdabs-what-does-the-reference-say\n\nSeems a bit worrying.\n\nNaush - any idea here? \n\n\n> ---\n> src/ipa/raspberrypi/controller/pwl.cpp | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/pwl.cpp b/src/ipa/raspberrypi/controller/pwl.cpp\n> index c59f5fa1..70c2e24b 100644\n> --- a/src/ipa/raspberrypi/controller/pwl.cpp\n> +++ b/src/ipa/raspberrypi/controller/pwl.cpp\n> @@ -6,6 +6,7 @@\n> */\n> \n> #include \n> +#include \n> #include \n> \n> #include \"pwl.h\"\n> @@ -168,7 +169,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const\n> while (thisSpan != (int)points_.size() - 1) {\n> double dx = points_[thisSpan + 1].x - points_[thisSpan].x,\n> dy = points_[thisSpan + 1].y - points_[thisSpan].y;\n> - if (abs(dy) > eps &&\n> + if (std::abs(dy) > eps &&\n> otherSpan + 1 < (int)other.points_.size() &&\n> points_[thisSpan + 1].y >=\n> other.points_[otherSpan + 1].x + eps) {\n> @@ -181,7 +182,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps) const\n> points_[thisSpan].y) *\n> dx / dy;\n> thisY = other.points_[++otherSpan].x;\n> - } else if (abs(dy) > eps && otherSpan > 0 &&\n> + } else if (std::abs(dy) > eps && otherSpan > 0 &&\n> points_[thisSpan + 1].y <=\n> other.points_[otherSpan - 1].x - eps) {\n> /*\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A89BCBD16B\n\tfor ;\n\tFri, 28 Oct 2022 10:27:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 208A861F4A;\n\tFri, 28 Oct 2022 12:27:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB02C61F4A\n\tfor ;\n\tFri, 28 Oct 2022 12:27:35 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6C5116BE;\n\tFri, 28 Oct 2022 12:27:35 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666952857;\n\tbh=MmhGz9KNdngb7d6jCZT6vRGrtVkwytLivgWb+6NF1NM=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=UrJhPJVVe54lzcMDSvY3HOOV6miWnbGXoGKuxXIAEwSuuuXEwXHTcfwzIWppeCODq\n\tOHWnDzjMNYHa7lem+ur7fl+Kfu8CvfVH7G0A4+lR/zUyLEhsTsHB3idAV2z0AM0yMc\n\tlOV+8lk/z8hD7IdgL6pg+UlTNSiRPRmleRUILa663Yn0Bn0uRBY4eQMLhle32tDRHr\n\tZ72tloUEdCpf+b8WiuP8di52ewZ79xELOgZvgVrb6fu91Ob8MmlEwv+lwBUADS0V3A\n\tiY6eNc/abJcHf/dak6gApwoUBn8fHR4TsvKMb9OC+o4rcetiWHtSTJkCtgloo8FtJq\n\t3wTvGIoYlFyCA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666952855;\n\tbh=MmhGz9KNdngb7d6jCZT6vRGrtVkwytLivgWb+6NF1NM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=cF6E5qKOZOREmIc5+1EYxt/ODTq8cTg/LjcMjYi/LzVT1rk1w7yByc07rOvQw8qln\n\tq+FW2PDP4Xg9gpVHdwNCsk+doWqNLh6aArWxNaEeWSdbng5nvIy0BfAnFfdxrODjRU\n\tZGubJG6RGu/ax7zlduGTYKQ8UEYPClUgLxxuGCtw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"cF6E5qKO\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20221028031726.4849-7-nicholas@rothemail.net>","References":"<20221027224135.348115-1-nicholas@rothemail.net>\n\t<20221028031726.4849-1-nicholas@rothemail.net>\n\t<20221028031726.4849-7-nicholas@rothemail.net>","To":"libcamera-devel@lists.libcamera.org,\n\tNaushir Patuck , ","Date":"Fri, 28 Oct 2022 11:27:32 +0100","Message-ID":"<166695285295.3974115.17824437985253142294@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v5 06/10] ipa: raspberry: replace\n\tabs() with std::abs()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"nicholas@rothemail.net","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25654,"web_url":"https://patchwork.libcamera.org/comment/25654/","msgid":"","date":"2022-10-28T10:40:50","subject":"Re: [libcamera-devel] [PATCH v5 06/10] ipa: raspberry: replace\n\tabs() with std::abs()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Fri, 28 Oct 2022 at 11:27, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Nicholas Roth via libcamera-devel (2022-10-28 04:17:22)\n> > From: Nicholas Roth \n> >\n> > pwl.cpp uses abs() instead of std::abs(), which causes unexpected\n> > behavior in the Clang compiler used for Android. Replace with\n> > C++-standard absolute value function std::abs(), which supports\n> > double-precision absolute values in a standard way.\n> >\n> > Signed-off-by: Nicholas Roth \n>\n> From Jacopo on v1:\n>\n> Reviewed-by: Jacopo Mondi \n>\n> And from me:\n>\n> Reviewed-by: Kieran Bingham \n>\n> Though we really might have to do a bit of extra validation here:\n>\n>\n> https://stackoverflow.com/questions/21392627/abs-vs-stdabs-what-does-the-reference-say\n>\n> Seems a bit worrying.\n>\n> Naush - any idea here?\n\n\nI think this code change ought to be safe - we are including cmath header,\nand dy is a double,\nso the compiler uses the double abs(double arg) signature.\n\nReviewed-by: Naushir Patuck \n\n\n>\n>\n>\n> > ---\n> > src/ipa/raspberrypi/controller/pwl.cpp | 5 +++--\n> > 1 file changed, 3 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/pwl.cpp\n> b/src/ipa/raspberrypi/controller/pwl.cpp\n> > index c59f5fa1..70c2e24b 100644\n> > --- a/src/ipa/raspberrypi/controller/pwl.cpp\n> > +++ b/src/ipa/raspberrypi/controller/pwl.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> >\n> > #include \n> > +#include \n> > #include \n> >\n> > #include \"pwl.h\"\n> > @@ -168,7 +169,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps)\n> const\n> > while (thisSpan != (int)points_.size() - 1) {\n> > double dx = points_[thisSpan + 1].x -\n> points_[thisSpan].x,\n> > dy = points_[thisSpan + 1].y -\n> points_[thisSpan].y;\n> > - if (abs(dy) > eps &&\n> > + if (std::abs(dy) > eps &&\n> > otherSpan + 1 < (int)other.points_.size() &&\n> > points_[thisSpan + 1].y >=\n> > other.points_[otherSpan + 1].x + eps) {\n> > @@ -181,7 +182,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps)\n> const\n> > points_[thisSpan].y) *\n> > dx / dy;\n> > thisY = other.points_[++otherSpan].x;\n> > - } else if (abs(dy) > eps && otherSpan > 0 &&\n> > + } else if (std::abs(dy) > eps && otherSpan > 0 &&\n> > points_[thisSpan + 1].y <=\n> > other.points_[otherSpan - 1].x - eps)\n> {\n> > /*\n> > --\n> > 2.34.1\n> >\n>\n\nOn Fri, 28 Oct 2022 at 11:27, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Nicholas Roth via libcamera-devel (2022-10-28 04:17:22)\n> > From: Nicholas Roth \n> >\n> > pwl.cpp uses abs() instead of std::abs(), which causes unexpected\n> > behavior in the Clang compiler used for Android. Replace with\n> > C++-standard absolute value function std::abs(), which supports\n> > double-precision absolute values in a standard way.\n> >\n> > Signed-off-by: Nicholas Roth \n>\n> From Jacopo on v1:\n>\n> Reviewed-by: Jacopo Mondi \n>\n> And from me:\n>\n> Reviewed-by: Kieran Bingham \n>\n> Though we really might have to do a bit of extra validation here:\n>\n>\n> https://stackoverflow.com/questions/21392627/abs-vs-stdabs-what-does-the-reference-say\n>\n> Seems a bit worrying.\n>\n> Naush - any idea here?\n>\n>\n> > ---\n> > src/ipa/raspberrypi/controller/pwl.cpp | 5 +++--\n> > 1 file changed, 3 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/pwl.cpp\n> b/src/ipa/raspberrypi/controller/pwl.cpp\n> > index c59f5fa1..70c2e24b 100644\n> > --- a/src/ipa/raspberrypi/controller/pwl.cpp\n> > +++ b/src/ipa/raspberrypi/controller/pwl.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> >\n> > #include \n> > +#include \n> > #include \n> >\n> > #include \"pwl.h\"\n> > @@ -168,7 +169,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps)\n> const\n> > while (thisSpan != (int)points_.size() - 1) {\n> > double dx = points_[thisSpan + 1].x -\n> points_[thisSpan].x,\n> > dy = points_[thisSpan + 1].y -\n> points_[thisSpan].y;\n> > - if (abs(dy) > eps &&\n> > + if (std::abs(dy) > eps &&\n> > otherSpan + 1 < (int)other.points_.size() &&\n> > points_[thisSpan + 1].y >=\n> > other.points_[otherSpan + 1].x + eps) {\n> > @@ -181,7 +182,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps)\n> const\n> > points_[thisSpan].y) *\n> > dx / dy;\n> > thisY = other.points_[++otherSpan].x;\n> > - } else if (abs(dy) > eps && otherSpan > 0 &&\n> > + } else if (std::abs(dy) > eps && otherSpan > 0 &&\n> > points_[thisSpan + 1].y <=\n> > other.points_[otherSpan - 1].x - eps)\n> {\n> > /*\n> > --\n> > 2.34.1\n> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4CC10BD16B\n\tfor ;\n\tFri, 28 Oct 2022 10:41:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AB7A362FD1;\n\tFri, 28 Oct 2022 12:41:09 +0200 (CEST)","from mail-qk1-x72c.google.com (mail-qk1-x72c.google.com\n\t[IPv6:2607:f8b0:4864:20::72c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DA17861F4A\n\tfor ;\n\tFri, 28 Oct 2022 12:41:07 +0200 (CEST)","by mail-qk1-x72c.google.com with SMTP id b25so3126468qkk.7\n\tfor ;\n\tFri, 28 Oct 2022 03:41:07 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666953669;\n\tbh=CBYWeqAKHiBtA6WuLzQWImWWqujoZfOXKJuu2USrD9c=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=PvKBCyHe9t2t84aLUM7f42YXJS7nqpfSoBL5xG2bnt96KquZPPuGrerZr7dtaH+fa\n\tVpTKHY3w+p47xWbkMO3Bt0FxFFo/YJaeVd17tVd5APlgB6sF+Bz60Jfh/ciu5P1Bmd\n\tEIApjGiJvp1dL5B8GTxe7vW1mNYApNSmX9HE6dJ/J9lzByGyV99up2cPlocJ9OWBab\n\tDLsz8udh1ObLv/dk19s4JZGoydVq6rfgfu9Qn5qOdOBe0MVav5WxfxOirTi1EY0RKz\n\tBE5+qzt3iGagYBHM6nmUCPEadfU090LvJHXnDV8uvJkli2CPCXiLgFHq32LQRJa3qd\n\tXk5U7FQb4zikw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=d8oZLnzutuqVVVX4NLib/I2PakSuSKEqWAvenN+JDWU=;\n\tb=oLuRHdgcsSFXqerlef9mbhwac0/4acrQzm15sN8RY7O/iqH/ECS3Di9CGVrxqXhu9+\n\tjdWyPz/NCO1eGrYNeWWCZFgLpO8kz3Qje/aP+n8SpYUVNv/IO4alRDHoFa67/yS1iAgz\n\tbPcROA0fyiy97KeUTfxXOQk4Iq7ZaBCE/R+GB68nWl769rpmt6rTytjhPlN3qTTwsnhd\n\tjc22RWFrEyVzU0KWHa3lAyToFzIwwoRXG5Ck29yoGvozVEo9hCPJcm4UJYKbmuzI5RUU\n\t8C98CXrorRogP7A5Hp5w9t7cTan40wrmH2qEPkaSNH/6o2b1KbClhMs6Axn8jF1kE60Q\n\t6HGA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"oLuRHdgc\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=d8oZLnzutuqVVVX4NLib/I2PakSuSKEqWAvenN+JDWU=;\n\tb=oTf7GZVLgl6e94TFMyV6IMnaqIkkUHveil74rc3tsOGsJmYq0uNCSoJp7V4zmFtPoY\n\t1o5iZkrbhfch88u/YREItiAm72E8jnbmR+VQzQXEdZN016ykoYGtg1VfaHMNMwC0v/Nn\n\t9O2U7nrutMHzcHUno2bvGOjmaMURTg0mbTed48uDNgkGF7dUVwsTV/YZDrJepeb18tXd\n\tVi/24QOpr9c4hH5raXrArTFvI2miSa+nXfhiGWg0SkxFLV1mMaB/iIUZJfKdlTnshZ36\n\tvNQ6sm4tQrrb4rRJXnV74IRMEwCtBbmCAx+SNOSAzeLw4TMSnvhiBS+JbVKJtUU0DpJI\n\thNCg==","X-Gm-Message-State":"ACrzQf3JhRmy3IXpwg4dyvlIXex3IK4FY5qt1sQAwf3+hyJrh3H069wE\n\t0rLheuEmd8+8JAdiF0FyCLuKWUjREzLW2wR9hhKlkA==","X-Google-Smtp-Source":"AMsMyM7jpRkILgbJc74fNlzun2J+CxaU8UIEidw49cJyKTvzR7xDoNvOJLPPnd2Z9OH/rXGLyEangxg3CLtYdobdzOU=","X-Received":"by 2002:a05:620a:215e:b0:6e7:dafb:a71a with SMTP id\n\tm30-20020a05620a215e00b006e7dafba71amr38398059qkm.61.1666953666769;\n\tFri, 28 Oct 2022 03:41:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20221027224135.348115-1-nicholas@rothemail.net>\n\t<20221028031726.4849-1-nicholas@rothemail.net>\n\t<20221028031726.4849-7-nicholas@rothemail.net>\n\t<166695285295.3974115.17824437985253142294@Monstersaurus>","In-Reply-To":"<166695285295.3974115.17824437985253142294@Monstersaurus>","Date":"Fri, 28 Oct 2022 11:40:50 +0100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"0000000000002a47c505ec15e573\"","Subject":"Re: [libcamera-devel] [PATCH v5 06/10] ipa: raspberry: replace\n\tabs() with std::abs()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"nicholas@rothemail.net, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25657,"web_url":"https://patchwork.libcamera.org/comment/25657/","msgid":"<20221028110730.da4jzwpuootn5vf5@uno.localdomain>","date":"2022-10-28T11:07:30","subject":"Re: [libcamera-devel] [PATCH v5 06/10] ipa: raspberry: replace\n\tabs() with std::abs()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n\nOn Fri, Oct 28, 2022 at 11:40:50AM +0100, Naushir Patuck via libcamera-devel wrote:\n> On Fri, 28 Oct 2022 at 11:27, Kieran Bingham <\n> kieran.bingham@ideasonboard.com> wrote:\n>\n> > Quoting Nicholas Roth via libcamera-devel (2022-10-28 04:17:22)\n> > > From: Nicholas Roth \n> > >\n> > > pwl.cpp uses abs() instead of std::abs(), which causes unexpected\n> > > behavior in the Clang compiler used for Android. Replace with\n> > > C++-standard absolute value function std::abs(), which supports\n> > > double-precision absolute values in a standard way.\n> > >\n> > > Signed-off-by: Nicholas Roth \n> >\n> > From Jacopo on v1:\n> >\n> > Reviewed-by: Jacopo Mondi \n> >\n> > And from me:\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> > Though we really might have to do a bit of extra validation here:\n> >\n> >\n> > https://stackoverflow.com/questions/21392627/abs-vs-stdabs-what-does-the-reference-say\n> >\n> > Seems a bit worrying.\n> >\n> > Naush - any idea here?\n\nThe change matches our suggested coding practices\n\nfrom Documentation/coding-style.rst:\n\nUsage of the C compatibility headers is preferred, except for the math.h header.\nWhere math.h defines separate functions for different argument types (e.g.\nabs(int), labs(long int), fabs(double) and fabsf(float)) and requires the\ndeveloper to pick the right function, cmath defines overloaded functions\n(std::abs(int), std::abs(long int), std::abs(double) and std::abs(float) to let\nthe compiler select the right function. This avoids potential errors such as\ncalling abs(int) with a float argument, performing an unwanted implicit integer\nconversion. For this reason, cmath is preferred over math.h.\n\n\n>\n>\n> I think this code change ought to be safe - we are including cmath header,\n> and dy is a double,\n> so the compiler uses the double abs(double arg) signature.\n>\n> Reviewed-by: Naushir Patuck \n>\n>\n> >\n> >\n> >\n> > > ---\n> > > src/ipa/raspberrypi/controller/pwl.cpp | 5 +++--\n> > > 1 file changed, 3 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/ipa/raspberrypi/controller/pwl.cpp\n> > b/src/ipa/raspberrypi/controller/pwl.cpp\n> > > index c59f5fa1..70c2e24b 100644\n> > > --- a/src/ipa/raspberrypi/controller/pwl.cpp\n> > > +++ b/src/ipa/raspberrypi/controller/pwl.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > >\n> > > #include \n> > > +#include \n> > > #include \n> > >\n> > > #include \"pwl.h\"\n> > > @@ -168,7 +169,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps)\n> > const\n> > > while (thisSpan != (int)points_.size() - 1) {\n> > > double dx = points_[thisSpan + 1].x -\n> > points_[thisSpan].x,\n> > > dy = points_[thisSpan + 1].y -\n> > points_[thisSpan].y;\n> > > - if (abs(dy) > eps &&\n> > > + if (std::abs(dy) > eps &&\n> > > otherSpan + 1 < (int)other.points_.size() &&\n> > > points_[thisSpan + 1].y >=\n> > > other.points_[otherSpan + 1].x + eps) {\n> > > @@ -181,7 +182,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps)\n> > const\n> > > points_[thisSpan].y) *\n> > > dx / dy;\n> > > thisY = other.points_[++otherSpan].x;\n> > > - } else if (abs(dy) > eps && otherSpan > 0 &&\n> > > + } else if (std::abs(dy) > eps && otherSpan > 0 &&\n> > > points_[thisSpan + 1].y <=\n> > > other.points_[otherSpan - 1].x - eps)\n> > {\n> > > /*\n> > > --\n> > > 2.34.1\n> > >\n> >\n>\n> On Fri, 28 Oct 2022 at 11:27, Kieran Bingham <\n> kieran.bingham@ideasonboard.com> wrote:\n>\n> > Quoting Nicholas Roth via libcamera-devel (2022-10-28 04:17:22)\n> > > From: Nicholas Roth \n> > >\n> > > pwl.cpp uses abs() instead of std::abs(), which causes unexpected\n> > > behavior in the Clang compiler used for Android. Replace with\n> > > C++-standard absolute value function std::abs(), which supports\n> > > double-precision absolute values in a standard way.\n> > >\n> > > Signed-off-by: Nicholas Roth \n> >\n> > From Jacopo on v1:\n> >\n> > Reviewed-by: Jacopo Mondi \n> >\n> > And from me:\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> > Though we really might have to do a bit of extra validation here:\n> >\n> >\n> > https://stackoverflow.com/questions/21392627/abs-vs-stdabs-what-does-the-reference-say\n> >\n> > Seems a bit worrying.\n> >\n> > Naush - any idea here?\n> >\n> >\n> > > ---\n> > > src/ipa/raspberrypi/controller/pwl.cpp | 5 +++--\n> > > 1 file changed, 3 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/ipa/raspberrypi/controller/pwl.cpp\n> > b/src/ipa/raspberrypi/controller/pwl.cpp\n> > > index c59f5fa1..70c2e24b 100644\n> > > --- a/src/ipa/raspberrypi/controller/pwl.cpp\n> > > +++ b/src/ipa/raspberrypi/controller/pwl.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > >\n> > > #include \n> > > +#include \n> > > #include \n> > >\n> > > #include \"pwl.h\"\n> > > @@ -168,7 +169,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps)\n> > const\n> > > while (thisSpan != (int)points_.size() - 1) {\n> > > double dx = points_[thisSpan + 1].x -\n> > points_[thisSpan].x,\n> > > dy = points_[thisSpan + 1].y -\n> > points_[thisSpan].y;\n> > > - if (abs(dy) > eps &&\n> > > + if (std::abs(dy) > eps &&\n> > > otherSpan + 1 < (int)other.points_.size() &&\n> > > points_[thisSpan + 1].y >=\n> > > other.points_[otherSpan + 1].x + eps) {\n> > > @@ -181,7 +182,7 @@ Pwl Pwl::compose(Pwl const &other, const double eps)\n> > const\n> > > points_[thisSpan].y) *\n> > > dx / dy;\n> > > thisY = other.points_[++otherSpan].x;\n> > > - } else if (abs(dy) > eps && otherSpan > 0 &&\n> > > + } else if (std::abs(dy) > eps && otherSpan > 0 &&\n> > > points_[thisSpan + 1].y <=\n> > > other.points_[otherSpan - 1].x - eps)\n> > {\n> > > /*\n> > > --\n> > > 2.34.1\n> > >\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BF683BDB16\n\tfor ;\n\tFri, 28 Oct 2022 11:07:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1439E62FD5;\n\tFri, 28 Oct 2022 13:07:35 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7964561F4A\n\tfor ;\n\tFri, 28 Oct 2022 13:07:33 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 8094B100005;\n\tFri, 28 Oct 2022 11:07:32 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666955255;\n\tbh=XR7VqQzfRoD5Pl6VajmGEJlDmCvq6YY1iHB3+D9M+CQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=vhHKTy0IwKgGr+OwrNoQ2UOvnQdpZgAbnypQksmtGgjO9CXpLTldU7O9CIlqR54rq\n\t+RvsHCQFn99bgPqpoQarWKI4RB9k5sF7m4uZwI2X5llTf/eo05B4bB5yD5eJTFO+7Y\n\tLXxMkDXuJCP9JSmHnUr+IhaSUxFwTizxZFji48HJhumn1O+B2e9zESCUI9d5KQYRWX\n\t9r6k/pRStJ7SraP5uO7CwD3jJq4+quoZJiGp3VOplI72ERH6hXdUiGx6HvefllACy5\n\tI5LCmYySIsmd2Q2d05IzBh4bMd/zJKWhzRQWFyO2MpUM2qOPvYiOs/gw7lJhqxU/ZD\n\tCRFelB4BZhcmg==","Date":"Fri, 28 Oct 2022 13:07:30 +0200","To":"Naushir Patuck ","Message-ID":"<20221028110730.da4jzwpuootn5vf5@uno.localdomain>","References":"<20221027224135.348115-1-nicholas@rothemail.net>\n\t<20221028031726.4849-1-nicholas@rothemail.net>\n\t<20221028031726.4849-7-nicholas@rothemail.net>\n\t<166695285295.3974115.17824437985253142294@Monstersaurus>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 06/10] ipa: raspberry: replace\n\tabs() with std::abs()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"nicholas@rothemail.net, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]