[{"id":17015,"web_url":"https://patchwork.libcamera.org/comment/17015/","msgid":"","date":"2021-05-19T10:38:25","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThank you for this patch.\n\nOn Tue, 18 May 2021 at 11:09, Naushir Patuck wrote:\n>\n> A new RPiController::Duration typedef is defined to represent a\n> std::chrono::duration type with double precision nanosecond timebase\n> that will be used throughout. This minimises the loss of precision when\n> converting timebases. A function for returning the duration count in any\n> timebase is also provided.\n>\n> An operator << overload is define to help with displaying\n> RPiController::Duration value in stream objects.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/controller/duration.hpp | 33 +++++++++++++++++++++\n> 1 file changed, 33 insertions(+)\n> create mode 100644 src/ipa/raspberrypi/controller/duration.hpp\n>\n> diff --git a/src/ipa/raspberrypi/controller/duration.hpp b/src/ipa/raspberrypi/controller/duration.hpp\n> new file mode 100644\n> index 000000000000..98aa3d78f52f\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/duration.hpp\n> @@ -0,0 +1,33 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> + *\n> + * duration.hpp - Helper macros for std::chrono::duration handling.\n> + */\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +namespace RPiController {\n> +\n> +using Duration = std::chrono::duration;\n> +\n> +// Helper to convert and return the count of any std::chrono::duration type to\n> +// another timebase. Use a double rep type to try and preserve precision.\n> +template \n> +static constexpr double DurationValue(T const &d)\n> +{\n> + return std::chrono::duration_cast>(d).count();\n> +};\n> +\n> +static inline std::ostream &operator<<(std::ostream &os, const Duration &duration)\n> +{\n> + std::streamsize ss = os.precision();\n> + os << std::fixed << std::setprecision(2) << DurationValue(duration) << \" us\";\n\nI suppose I wonder slightly why we change the precision, I assume the\noutput is just a bit tidier like that? And a space before the \"us\"? I\nguess, why not!\n\nReviewed-by: David Plowman \n\nThanks\nDavid\n\n> + os << std::setprecision(ss) << std::defaultfloat;\n> + return os;\n> +}\n> +\n> +} // namespace RPiController\n> --\n> 2.25.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 C378EC31FB\n\tfor ;\n\tWed, 19 May 2021 10:38:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1B8836891F;\n\tWed, 19 May 2021 12:38:39 +0200 (CEST)","from mail-wm1-x334.google.com (mail-wm1-x334.google.com\n\t[IPv6:2a00:1450:4864:20::334])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A4439602B2\n\tfor ;\n\tWed, 19 May 2021 12:38:37 +0200 (CEST)","by mail-wm1-x334.google.com with SMTP id\n\tu5-20020a7bc0450000b02901480e40338bso3040044wmc.1\n\tfor ;\n\tWed, 19 May 2021 03:38:37 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"FNOyKysv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=vX/yviQ1MsLj0v5s/wOT9u6s7kq5ZMBUj0vfbRduUb4=;\n\tb=FNOyKysvg1/2W3yRjZUrFzrr7aZp9Y9I0x3R+Xs7y4Ry31WvB4crIINVY6Htsiktcg\n\tlLQsnWHZR+BE0ZepGrtrqN11TvlTBBgjh+ntuMX3fP/gYbKDo03mTCayzqzhJ6tpYW43\n\t2SHcUT71ga9WZEwFn6qEFR38IJ/jPexyuxlJ3GBHPNfCT9RWokDTGGMZVr8g4O6g56oB\n\tErNOeXiXL5TxbMhCKxtvhIMcYzcr+NZ1QDATlSn106+fPlfxIUD2Xt1gM5hdqUNWgNOM\n\tuIvY40DPycEaBj/L956ljhbU7g5dS/tjLCAbwEjPOzQQn41YCZbQT5YPYrhzhcpyH4Bt\n\tnTCw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=vX/yviQ1MsLj0v5s/wOT9u6s7kq5ZMBUj0vfbRduUb4=;\n\tb=AVtzDOA0dLQl+gFPSIqGjsQPRddtfsJp2c9dA0NqVy/qZ1zqg51eKu63CB3dUR0Hbq\n\tqzzPHHYAZAzm5wR91HjoYqpMic/PIxpojqS/xOd2NqxUZqLsmnuUwA7BIYNZIWMNrqyN\n\thipnZT2e2uubh0/vA4NtjTuGzYf2XlO1F20z/MpKNglFGVUllkyF38omcD1pC3RD3TFS\n\th25WtMh4G/mHUR0Cu59EIMN0vc3mqOCryMGtMgmgcN2d5bL56ERRXT5jycqt2bUyoa91\n\tSCtAevIcky8i1gaZisnQhbHiCgfYN/sgaKvnA9QfexcHo06GSmVHDqVC4UJxGf0rUIaf\n\tkjEQ==","X-Gm-Message-State":"AOAM533vivD6GrtpqosvJzBeFnXEWHv6543rDFZ3gwGl6G0AH1dMRAdh\n\tF2//r5SLcB/qN2N2kvYGR7HicTPjhgYL/GD/n+tmuw==","X-Google-Smtp-Source":"ABdhPJzMKulZnhdWS+R/lmHJ0gPdcB+Fivmvs3u3DzbborfaXj2HvBjwT7xeQmM/25WtpqszYfqO/npE49eUgq/mZbA=","X-Received":"by 2002:a1c:1dcf:: with SMTP id\n\td198mr8814762wmd.40.1621420717278; \n\tWed, 19 May 2021 03:38:37 -0700 (PDT)","MIME-Version":"1.0","References":"<20210518100706.578526-1-naush@raspberrypi.com>\n\t<20210518100706.578526-2-naush@raspberrypi.com>","In-Reply-To":"<20210518100706.578526-2-naush@raspberrypi.com>","From":"David Plowman ","Date":"Wed, 19 May 2021 11:38:25 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17016,"web_url":"https://patchwork.libcamera.org/comment/17016/","msgid":"","date":"2021-05-19T10:50:22","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for your review feedback.\n\nOn Wed, 19 May 2021 at 11:38, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thank you for this patch.\n>\n> On Tue, 18 May 2021 at 11:09, Naushir Patuck \n> wrote:\n> >\n> > A new RPiController::Duration typedef is defined to represent a\n> > std::chrono::duration type with double precision nanosecond timebase\n> > that will be used throughout. This minimises the loss of precision when\n> > converting timebases. A function for returning the duration count in any\n> > timebase is also provided.\n> >\n> > An operator << overload is define to help with displaying\n> > RPiController::Duration value in stream objects.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/ipa/raspberrypi/controller/duration.hpp | 33 +++++++++++++++++++++\n> > 1 file changed, 33 insertions(+)\n> > create mode 100644 src/ipa/raspberrypi/controller/duration.hpp\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/duration.hpp\n> b/src/ipa/raspberrypi/controller/duration.hpp\n> > new file mode 100644\n> > index 000000000000..98aa3d78f52f\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/duration.hpp\n> > @@ -0,0 +1,33 @@\n> > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > +/*\n> > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> > + *\n> > + * duration.hpp - Helper macros for std::chrono::duration handling.\n> > + */\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +namespace RPiController {\n> > +\n> > +using Duration = std::chrono::duration;\n> > +\n> > +// Helper to convert and return the count of any std::chrono::duration\n> type to\n> > +// another timebase. Use a double rep type to try and preserve\n> precision.\n> > +template \n> > +static constexpr double DurationValue(T const &d)\n> > +{\n> > + return std::chrono::duration_cast P>>(d).count();\n> > +};\n> > +\n> > +static inline std::ostream &operator<<(std::ostream &os, const Duration\n> &duration)\n> > +{\n> > + std::streamsize ss = os.precision();\n> > + os << std::fixed << std::setprecision(2) <<\n> DurationValue(duration) << \" us\";\n>\n> I suppose I wonder slightly why we change the precision, I assume the\n> output is just a bit tidier like that? And a space before the \"us\"? I\n> guess, why not!\n>\n\nYes, I did not want more than 2 decimal places of precision in the output.\nI can remove the space\nbefore the \"us\" in the next revision.\n\nRegards,\nNaush\n\n\n>\n> Reviewed-by: David Plowman \n>\n> Thanks\n> David\n>\n> > + os << std::setprecision(ss) << std::defaultfloat;\n> > + return os;\n> > +}\n> > +\n> > +} // namespace RPiController\n> > --\n> > 2.25.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 3F7EFC31FF\n\tfor ;\n\tWed, 19 May 2021 10:50:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AEF726891D;\n\tWed, 19 May 2021 12:50:40 +0200 (CEST)","from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com\n\t[IPv6:2a00:1450:4864:20::12c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A7151602B2\n\tfor ;\n\tWed, 19 May 2021 12:50:39 +0200 (CEST)","by mail-lf1-x12c.google.com with SMTP id i22so18264987lfl.10\n\tfor ;\n\tWed, 19 May 2021 03:50:39 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"OBPSwlN2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=uTAQKLoDt/HNer22qyERiAhaEq4Sl70MVLc1mnXi2zw=;\n\tb=OBPSwlN29t8flo2rLwV4XvycwWzsjs8gS61CEeV7SK4UVYj7aZxDuvsYLOwKnsyDCW\n\tT8ImG1qFyPOY74a/PYIAE89w4K73i/s/S1qyqoOTHA/9doqC1XdqXNyNy/xscvdvkzjj\n\tbj2VhQSH2XMJdr8o7pNVLPxTSs10lmhUZMb2tbjB8uzho3VPT/JkUn66G7bW6FDWqfqV\n\trwSyIEL+f8it+KRQxLgy4/aRvO8EbnwB01YhocHWQivAp062lcBgy3IYhzFpmWLA4HpR\n\tQGGBRY4iuKn/UU7hDqJMSXCd+tJcfnDow5rT/5Gs4cHl50d3QwR6P63yYetvEA6AkCfz\n\tMiyg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=uTAQKLoDt/HNer22qyERiAhaEq4Sl70MVLc1mnXi2zw=;\n\tb=sK/Bnxl0H/3Tc4g4klHUh0skr01dwAFZsdt+/1kn/BXusnCblgH9WNicSY45RH2fdb\n\tju4UGQ9NcFaEJvReWx9OXAONVit27WWfnY61pmkxA3fLSvjG9JMNiXvm4wSnPuuF8TTD\n\tN3QNHB0gypTBM30grOn5NscuYof2ctTurhfy/mEOXdi6HW12h50x5RG7Ykq8zj0+U5eJ\n\tp8pcpsnyp1C1fxkZT+OwUuk0WWdy20Mez+LUv/+RbAWas8jK95gOXcxdq7vr1luXnkZj\n\tdVxmMP2wTq9KIp4p7ieUGGijfxezhdc73FliVHKrQjtT3DWipb0wC77hh9INhnEVAP8W\n\tz6hQ==","X-Gm-Message-State":"AOAM5328WO43FzmNvuDosTxiJmxw5nRTj2iHlfbKHzimPkDtSHQWrc9Y\n\tABT1OiPK/wfr5cuTEW2SsWmUCbFpBPvPJnnx416TQg==","X-Google-Smtp-Source":"ABdhPJx8mnBhrXcictfcYFMjLcICCftIu85kZj67t9ZkJPyywfigqYd41ZsEOJj17yn3hYFhJ2CaCmyYKZYWN0egAdw=","X-Received":"by 2002:a05:6512:3f87:: with SMTP id\n\tx7mr8077437lfa.617.1621421438766; \n\tWed, 19 May 2021 03:50:38 -0700 (PDT)","MIME-Version":"1.0","References":"<20210518100706.578526-1-naush@raspberrypi.com>\n\t<20210518100706.578526-2-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 19 May 2021 11:50:22 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000e3a4c205c2ac984f\"","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17017,"web_url":"https://patchwork.libcamera.org/comment/17017/","msgid":"","date":"2021-05-19T10:52:28","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nOn Wed, 19 May 2021 at 11:50, Naushir Patuck wrote:\n>\n> Hi David,\n>\n> Thank you for your review feedback.\n>\n> On Wed, 19 May 2021 at 11:38, David Plowman wrote:\n>>\n>> Hi Naush\n>>\n>> Thank you for this patch.\n>>\n>> On Tue, 18 May 2021 at 11:09, Naushir Patuck wrote:\n>> >\n>> > A new RPiController::Duration typedef is defined to represent a\n>> > std::chrono::duration type with double precision nanosecond timebase\n>> > that will be used throughout. This minimises the loss of precision when\n>> > converting timebases. A function for returning the duration count in any\n>> > timebase is also provided.\n>> >\n>> > An operator << overload is define to help with displaying\n>> > RPiController::Duration value in stream objects.\n>> >\n>> > Signed-off-by: Naushir Patuck \n>> > ---\n>> > src/ipa/raspberrypi/controller/duration.hpp | 33 +++++++++++++++++++++\n>> > 1 file changed, 33 insertions(+)\n>> > create mode 100644 src/ipa/raspberrypi/controller/duration.hpp\n>> >\n>> > diff --git a/src/ipa/raspberrypi/controller/duration.hpp b/src/ipa/raspberrypi/controller/duration.hpp\n>> > new file mode 100644\n>> > index 000000000000..98aa3d78f52f\n>> > --- /dev/null\n>> > +++ b/src/ipa/raspberrypi/controller/duration.hpp\n>> > @@ -0,0 +1,33 @@\n>> > +/* SPDX-License-Identifier: BSD-2-Clause */\n>> > +/*\n>> > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n>> > + *\n>> > + * duration.hpp - Helper macros for std::chrono::duration handling.\n>> > + */\n>> > +#pragma once\n>> > +\n>> > +#include \n>> > +#include \n>> > +#include \n>> > +\n>> > +namespace RPiController {\n>> > +\n>> > +using Duration = std::chrono::duration;\n>> > +\n>> > +// Helper to convert and return the count of any std::chrono::duration type to\n>> > +// another timebase. Use a double rep type to try and preserve precision.\n>> > +template \n>> > +static constexpr double DurationValue(T const &d)\n>> > +{\n>> > + return std::chrono::duration_cast>(d).count();\n>> > +};\n>> > +\n>> > +static inline std::ostream &operator<<(std::ostream &os, const Duration &duration)\n>> > +{\n>> > + std::streamsize ss = os.precision();\n>> > + os << std::fixed << std::setprecision(2) << DurationValue(duration) << \" us\";\n>>\n>> I suppose I wonder slightly why we change the precision, I assume the\n>> output is just a bit tidier like that? And a space before the \"us\"? I\n>> guess, why not!\n>\n>\n> Yes, I did not want more than 2 decimal places of precision in the output. I can remove the space\n> before the \"us\" in the next revision.\n\nIt's OK... I don't mind the space!\n\nDavid\n\n>\n> Regards,\n> Naush\n>\n>>\n>>\n>> Reviewed-by: David Plowman \n>>\n>> Thanks\n>> David\n>>\n>> > + os << std::setprecision(ss) << std::defaultfloat;\n>> > + return os;\n>> > +}\n>> > +\n>> > +} // namespace RPiController\n>> > --\n>> > 2.25.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 CD3E7C31FB\n\tfor ;\n\tWed, 19 May 2021 10:52:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 392116891D;\n\tWed, 19 May 2021 12:52:42 +0200 (CEST)","from mail-wr1-x435.google.com (mail-wr1-x435.google.com\n\t[IPv6:2a00:1450:4864:20::435])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 82C77602B2\n\tfor ;\n\tWed, 19 May 2021 12:52:40 +0200 (CEST)","by mail-wr1-x435.google.com with SMTP id j14so11775703wrq.5\n\tfor ;\n\tWed, 19 May 2021 03:52:40 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"KmeR8/jL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=bZPhc6oISi3SAOm8oY7dNdvMlfN66rVBL5Pa6amYbCQ=;\n\tb=KmeR8/jLUnO/plJSxcpxsecnLbt0faD2oG/q0tWiqRidmpFLXLsKIO4soltfiKyz5Y\n\t5gB7v8CBry9sjEWargOKDqy65M+vjYgsUH2EsItrUEst34jwf5aobPxJLrMkhZGzAYja\n\t1GZXXsQC5ZdaSzUgAXisvX0ttbts+/RQw/KnIpUqG7s29e9+H3+GPtKt9ej1rYk4seGK\n\t5iHd+PHJBpRznlZg3eixO4l2ReTL9vdFoJhI8qbUkIygFCEKizQ3oTTTJTZezKmUxib6\n\tSdEmMYheN63ZpoumJRLaFnVjm6RA/9k6ElUT2baZo5K6mgulKQePVrh2WIodEg/bCrd7\n\tej+w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=bZPhc6oISi3SAOm8oY7dNdvMlfN66rVBL5Pa6amYbCQ=;\n\tb=blkR4UrKDk+SgwbTN3Et4u9ccqLO6MIEyrmpbu0HXIWgqvir67IIYn/OKQxW1io2c0\n\tzXrlVCyOoCHsPN1Eclp/Kgc/0derpznULmvNLhz3lSCW6ud+Vqv09uMoD18xjBh3oozd\n\th93NLEOt4KPdO16Bu3t7knJwgSj/vDWhLPqUr5EyUc+Zqy9nC4UvnvyUhsBZL1m/QP2P\n\tIlUGbX9/RU5wcs+VseuaKZnyyvAOmuWdToWEBoLh98er/5mxPfNQupkbl/NZXeRwZI7S\n\tbAJ6p2/KghwpK8gIiRJn2yqDo0//HAVoUAanBpEoGh0TGVPXsIoDOr4ZDKWokxtijS87\n\tmF6w==","X-Gm-Message-State":"AOAM5320UUi3fDj2dCXVkFTUPX3PaaNs2NbM+K9+RMvzmI1rhZ1PSWF7\n\tWCkH//GmpgIq0vgxrFzePZ/6kQC8LPpaZTLTKVr24Q==","X-Google-Smtp-Source":"ABdhPJySEIBApxe54tzt+b26j2sdiPi0v5aUYy+PNjXh6iyUEYXQm80HvcCpgg2zVsEOC//6UWnCGfPjIorvjzBy8aE=","X-Received":"by 2002:a5d:6485:: with SMTP id o5mr8142090wri.163.1621421560143;\n\tWed, 19 May 2021 03:52:40 -0700 (PDT)","MIME-Version":"1.0","References":"<20210518100706.578526-1-naush@raspberrypi.com>\n\t<20210518100706.578526-2-naush@raspberrypi.com>\n\t\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Wed, 19 May 2021 11:52:28 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17021,"web_url":"https://patchwork.libcamera.org/comment/17021/","msgid":"","date":"2021-05-19T14:24:24","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Tue, May 18, 2021 at 11:07:03AM +0100, Naushir Patuck wrote:\n> A new RPiController::Duration typedef is defined to represent a\n> std::chrono::duration type with double precision nanosecond timebase\n> that will be used throughout. This minimises the loss of precision when\n> converting timebases. A function for returning the duration count in any\n> timebase is also provided.\n> \n> An operator << overload is define to help with displaying\n> RPiController::Duration value in stream objects.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/controller/duration.hpp | 33 +++++++++++++++++++++\n> 1 file changed, 33 insertions(+)\n> create mode 100644 src/ipa/raspberrypi/controller/duration.hpp\n> \n> diff --git a/src/ipa/raspberrypi/controller/duration.hpp b/src/ipa/raspberrypi/controller/duration.hpp\n> new file mode 100644\n> index 000000000000..98aa3d78f52f\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/duration.hpp\n> @@ -0,0 +1,33 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> + *\n> + * duration.hpp - Helper macros for std::chrono::duration handling.\n> + */\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +namespace RPiController {\n> +\n> +using Duration = std::chrono::duration;\n> +\n> +// Helper to convert and return the count of any std::chrono::duration type to\n> +// another timebase. Use a double rep type to try and preserve precision.\n> +template \n> +static constexpr double DurationValue(T const &d)\n> +{\n> +\treturn std::chrono::duration_cast>(d).count();\n> +};\n> +\n> +static inline std::ostream &operator<<(std::ostream &os, const Duration &duration)\n> +{\n> + std::streamsize ss = os.precision();\n> + os << std::fixed << std::setprecision(2) << DurationValue(duration) << \" us\";\n> + os << std::setprecision(ss) << std::defaultfloat;\n\nThis will reset to std::defaultfloat, while the stream may already be in\nstd::fixed.\n\n> + return os;\n> +}\n\nWould it be too much yak-shaving to ask for the implementation to be\ncloser to\nhttps://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt, and\nmoved to utils.h ? It could be useful more widely in libcamera than just\nin the RPi IPA.\n\n> +\n> +} // namespace RPiController","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 DFB2BC31FF\n\tfor ;\n\tWed, 19 May 2021 14:24:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40EC66891F;\n\tWed, 19 May 2021 16:24:27 +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 48AC268915\n\tfor ;\n\tWed, 19 May 2021 16:24:26 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B4D84BA7;\n\tWed, 19 May 2021 16:24:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"acMS+gFz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621434265;\n\tbh=9MC06TWH5NqewPrnC7wDWzk7XEANa3d4wIxXPgu0RQU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=acMS+gFzD49Z3jvCy/4z6LVJMaoELTDIP5x07ksgdPZqCIJtYa9hwWFJPkiXx1qRu\n\t4tk+mHfddBnuV+SNFQpSDKb3jNZ5T4e+CxYjFQDPcPkWuB6qw7i8KuNRJFyNDABZt+\n\te2DrJ/NBEsFBDBKxE+eRhl2GImjjciApTzY06B8Y=","Date":"Wed, 19 May 2021 17:24:24 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210518100706.578526-1-naush@raspberrypi.com>\n\t<20210518100706.578526-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210518100706.578526-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17024,"web_url":"https://patchwork.libcamera.org/comment/17024/","msgid":"","date":"2021-05-19T14:34:05","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Wed, 19 May 2021 at 15:24, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Tue, May 18, 2021 at 11:07:03AM +0100, Naushir Patuck wrote:\n> > A new RPiController::Duration typedef is defined to represent a\n> > std::chrono::duration type with double precision nanosecond timebase\n> > that will be used throughout. This minimises the loss of precision when\n> > converting timebases. A function for returning the duration count in any\n> > timebase is also provided.\n> >\n> > An operator << overload is define to help with displaying\n> > RPiController::Duration value in stream objects.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/ipa/raspberrypi/controller/duration.hpp | 33 +++++++++++++++++++++\n> > 1 file changed, 33 insertions(+)\n> > create mode 100644 src/ipa/raspberrypi/controller/duration.hpp\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/duration.hpp\n> b/src/ipa/raspberrypi/controller/duration.hpp\n> > new file mode 100644\n> > index 000000000000..98aa3d78f52f\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/duration.hpp\n> > @@ -0,0 +1,33 @@\n> > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > +/*\n> > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> > + *\n> > + * duration.hpp - Helper macros for std::chrono::duration handling.\n> > + */\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +namespace RPiController {\n> > +\n> > +using Duration = std::chrono::duration;\n> > +\n> > +// Helper to convert and return the count of any std::chrono::duration\n> type to\n> > +// another timebase. Use a double rep type to try and preserve\n> precision.\n> > +template \n> > +static constexpr double DurationValue(T const &d)\n> > +{\n> > + return std::chrono::duration_cast P>>(d).count();\n> > +};\n> > +\n> > +static inline std::ostream &operator<<(std::ostream &os, const Duration\n> &duration)\n> > +{\n> > + std::streamsize ss = os.precision();\n> > + os << std::fixed << std::setprecision(2) <<\n> DurationValue(duration) << \" us\";\n> > + os << std::setprecision(ss) << std::defaultfloat;\n>\n> This will reset to std::defaultfloat, while the stream may already be in\n> std::fixed.\n>\n\nGood point, I should save and reapply like I do with precision.\n\n\n>\n> > + return os;\n> > +}\n>\n> Would it be too much yak-shaving to ask for the implementation to be\n> closer to\n> https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt, and\n> moved to utils.h ? It could be useful more widely in libcamera than just\n> in the RPi IPA.\n>\n\nYes, I could look into that. Did you mean move the whole\nimplementation into utils.h, or\nonly the operator <<() bit? Actually, I would like to make an impactful\nchange in the next\nrevision of the series, and replace Duration with a class derived from a\nstd::chrono::duration\nobject. This is a bit neater in terms of encapsulation, and allows me to\nimplement an\noperator bool() method that cleans up the code a bit more. Functionally,\neverything else\nremains identical. Did you want me to put that into utils.h?\n\nSorry David, your review for patch 1/4 may need redoing with :-(\n\nRegards,\nNaush\n\n\n> > +\n> > +} // namespace RPiController\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 A8C39C31FF\n\tfor ;\n\tWed, 19 May 2021 14:34:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 120C36891F;\n\tWed, 19 May 2021 16:34:24 +0200 (CEST)","from mail-qv1-xf2a.google.com (mail-qv1-xf2a.google.com\n\t[IPv6:2607:f8b0:4864:20::f2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C8B3168915\n\tfor ;\n\tWed, 19 May 2021 16:34:22 +0200 (CEST)","by mail-qv1-xf2a.google.com with SMTP id ee9so6857904qvb.8\n\tfor ;\n\tWed, 19 May 2021 07:34:22 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"MB0rQKdj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=Pe4Esq5WpS10smsPQ3BtdD9nXuIhztWNwvWBmljvml8=;\n\tb=MB0rQKdjxLRHbJ+KnFb8p9IRQqEBsZ9r2icn4w1pBz5dtbXqStrwy//MAId5/Tw7rN\n\tZZ2hFEGcG/Fbu2kH1I12AS8RwfaRvNItTcOQQ7gheUkGPUGfJp7shDFFa9xVZpQVH+cn\n\tdFcvS2oG/4f2sdUDiiS/zBAX8Z9SUOAjqYuYZRzL4kT1aHo04/x+/mNAMMHB2TE+aJDj\n\tFnGlL6d/cAf6/PAJj2MaEBn0db/pvRibCICe9uUtFj8FHBMBREBvM8i+Fo70tSYflYiM\n\tDdt2dU1ym/1EO+uOUIKBPVCmckCQzRvgGnMs5G+t21U86sTRgNk7f1hE91XKb5D8A5L6\n\tlCgg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Pe4Esq5WpS10smsPQ3BtdD9nXuIhztWNwvWBmljvml8=;\n\tb=CdsFIktpbTosebO5BTysSRUMYnraEKXaRj712NhYjxVHArCnleBPaqABFKyIkwNGSq\n\tPP0TKTIJ7SMM1SrL5FbDss6JVbtjpxEpHfou16AlIWLSaB8myJHMl3IORw34beTyIOmE\n\trfS58ZFqawZLW33kXireisV3rzy3ujhN6L3Y/K+NuHBYeGfzBe//Xa1Ub2mOD5kay57S\n\tgqwbMl13vZuZnL1J1k+SA6OR3uxqJKT1wDGOC+66ZKRBtkmHnXFzCf1S6WZDn2jy3+cl\n\trFBCdVcOxDci6uwbdrfUZlz2QLZc4XJS9ugJAteo6lgyhBca5qxZZWhbZu0IRBv5XkOz\n\tpucQ==","X-Gm-Message-State":"AOAM530odlWwdTlfTbKRuJKwoNC5LQDuZwV4POVVUerUXz30fegrL66z\n\tnxxSBk/LVqoHaKzaPwhVb0jzePLON6hCyivSN+x8NEHYdbqdCw==","X-Google-Smtp-Source":"ABdhPJy0c5+9qmIgrT1nQwj3m/e5+781QKmSckD3yMAdTxP7uSGBgMXOE60GBt8S2BO/Ta1AhbZBVIsXR6NT4wk+1sI=","X-Received":"by 2002:a0c:fe48:: with SMTP id u8mr13301573qvs.44.1621434861643;\n\tWed, 19 May 2021 07:34:21 -0700 (PDT)","MIME-Version":"1.0","References":"<20210518100706.578526-1-naush@raspberrypi.com>\n\t<20210518100706.578526-2-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 19 May 2021 15:34:05 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000f482eb05c2afb893\"","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17025,"web_url":"https://patchwork.libcamera.org/comment/17025/","msgid":"","date":"2021-05-19T14:48:05","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Wed, May 19, 2021 at 03:34:05PM +0100, Naushir Patuck wrote:\n> On Wed, 19 May 2021 at 15:24, Laurent Pinchart wrote:\n> > On Tue, May 18, 2021 at 11:07:03AM +0100, Naushir Patuck wrote:\n> > > A new RPiController::Duration typedef is defined to represent a\n> > > std::chrono::duration type with double precision nanosecond timebase\n> > > that will be used throughout. This minimises the loss of precision when\n> > > converting timebases. A function for returning the duration count in any\n> > > timebase is also provided.\n> > >\n> > > An operator << overload is define to help with displaying\n> > > RPiController::Duration value in stream objects.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > src/ipa/raspberrypi/controller/duration.hpp | 33 +++++++++++++++++++++\n> > > 1 file changed, 33 insertions(+)\n> > > create mode 100644 src/ipa/raspberrypi/controller/duration.hpp\n> > >\n> > > diff --git a/src/ipa/raspberrypi/controller/duration.hpp b/src/ipa/raspberrypi/controller/duration.hpp\n> > > new file mode 100644\n> > > index 000000000000..98aa3d78f52f\n> > > --- /dev/null\n> > > +++ b/src/ipa/raspberrypi/controller/duration.hpp\n> > > @@ -0,0 +1,33 @@\n> > > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > > +/*\n> > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> > > + *\n> > > + * duration.hpp - Helper macros for std::chrono::duration handling.\n> > > + */\n> > > +#pragma once\n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +namespace RPiController {\n> > > +\n> > > +using Duration = std::chrono::duration;\n> > > +\n> > > +// Helper to convert and return the count of any std::chrono::duration type to\n> > > +// another timebase. Use a double rep type to try and preserve precision.\n> > > +template \n> > > +static constexpr double DurationValue(T const &d)\n> > > +{\n> > > + return std::chrono::duration_cast>(d).count();\n> > > +};\n> > > +\n> > > +static inline std::ostream &operator<<(std::ostream &os, const Duration &duration)\n> > > +{\n> > > + std::streamsize ss = os.precision();\n> > > + os << std::fixed << std::setprecision(2) << DurationValue(duration) << \" us\";\n> > > + os << std::setprecision(ss) << std::defaultfloat;\n> >\n> > This will reset to std::defaultfloat, while the stream may already be in\n> > std::fixed.\n> \n> Good point, I should save and reapply like I do with precision.\n> \n> > > + return os;\n> > > +}\n> >\n> > Would it be too much yak-shaving to ask for the implementation to be\n> > closer to\n> > https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt, and\n> > moved to utils.h ? It could be useful more widely in libcamera than just\n> > in the RPi IPA.\n> \n> Yes, I could look into that. Did you mean move the whole implementation into utils.h, or\n> only the operator <<() bit?\n\nI meant operator<<(). DurationValue would likely need to go there too if\nit's used internally, but the implementation of operator<<() could also\nhardcode it as\n\n\tos << std::fixed << std::setprecision(2) << duration.count() / 1000.0 << \" us\";\n\nif needed.\n\n> Actually, I would like to make an impactful change in the next\n> revision of the series, and replace Duration with a class derived from a std::chrono::duration\n> object. This is a bit neater in terms of encapsulation, and allows me to implement an\n> operator bool() method that cleans up the code a bit more. Functionally, everything else\n> remains identical. Did you want me to put that into utils.h?\n\nI think it should actually go to a public header if we want to\nstandardize all timestamps and durations to std::chrono, but that's too\nmuch yak-shaving :-) We can handle it later.\n\nWe already have the following types in utils.h:\n\nusing clock = std::chrono::steady_clock;\nusing duration = std::chrono::steady_clock::duration;\nusing time_point = std::chrono::steady_clock::time_point;\n\nThere are also a few helpers to perform string conversion and to convert\nto a timespec. Ideally all of this should be consolidated, but again\nit's propably a bit too much work. If we can make operator<<() conform\nto the C++20 definition to be able to later switch to it, I'll be happy\nalready.\n\n> Sorry David, your review for patch 1/4 may need redoing with :-(\n> \n> > > +\n> > > +} // namespace RPiController","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 91A72C31FB\n\tfor ;\n\tWed, 19 May 2021 14:48:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 044656891A;\n\tWed, 19 May 2021 16:48:08 +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 5AF5A68915\n\tfor ;\n\tWed, 19 May 2021 16:48:07 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C82E9BA7;\n\tWed, 19 May 2021 16:48:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UiAKx+CH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621435687;\n\tbh=TZ4SdVZTs9ZWJmJpekg91ISpkquP+nAJ7/PH9dHY5Yo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UiAKx+CHKy6UsUCO1w1yNqWclRLY3mlINVSdqAIHO8wrrswyT0CYFNthHbXh/cuvB\n\tR6rv2rPp9zDxUitDLM7aYArThq2NWo0wxagoGWio1qs4jfQzzPSdAd0VxNvXsPMqbk\n\tVmeHWsMw+DQ3NYcS00GnOPgEQIiij+LdnqX+UGsw=","Date":"Wed, 19 May 2021 17:48:05 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210518100706.578526-1-naush@raspberrypi.com>\n\t<20210518100706.578526-2-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17026,"web_url":"https://patchwork.libcamera.org/comment/17026/","msgid":"","date":"2021-05-19T15:21:04","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\n\nOn Wed, 19 May 2021 at 15:48, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Wed, May 19, 2021 at 03:34:05PM +0100, Naushir Patuck wrote:\n> > On Wed, 19 May 2021 at 15:24, Laurent Pinchart wrote:\n> > > On Tue, May 18, 2021 at 11:07:03AM +0100, Naushir Patuck wrote:\n> > > > A new RPiController::Duration typedef is defined to represent a\n> > > > std::chrono::duration type with double precision nanosecond timebase\n> > > > that will be used throughout. This minimises the loss of precision\n> when\n> > > > converting timebases. A function for returning the duration count in\n> any\n> > > > timebase is also provided.\n> > > >\n> > > > An operator << overload is define to help with displaying\n> > > > RPiController::Duration value in stream objects.\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > src/ipa/raspberrypi/controller/duration.hpp | 33\n> +++++++++++++++++++++\n> > > > 1 file changed, 33 insertions(+)\n> > > > create mode 100644 src/ipa/raspberrypi/controller/duration.hpp\n> > > >\n> > > > diff --git a/src/ipa/raspberrypi/controller/duration.hpp\n> b/src/ipa/raspberrypi/controller/duration.hpp\n> > > > new file mode 100644\n> > > > index 000000000000..98aa3d78f52f\n> > > > --- /dev/null\n> > > > +++ b/src/ipa/raspberrypi/controller/duration.hpp\n> > > > @@ -0,0 +1,33 @@\n> > > > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > > > +/*\n> > > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> > > > + *\n> > > > + * duration.hpp - Helper macros for std::chrono::duration handling.\n> > > > + */\n> > > > +#pragma once\n> > > > +\n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +\n> > > > +namespace RPiController {\n> > > > +\n> > > > +using Duration = std::chrono::duration;\n> > > > +\n> > > > +// Helper to convert and return the count of any\n> std::chrono::duration type to\n> > > > +// another timebase. Use a double rep type to try and preserve\n> precision.\n> > > > +template \n> > > > +static constexpr double DurationValue(T const &d)\n> > > > +{\n> > > > + return\n> std::chrono::duration_cast>(d).count();\n> > > > +};\n> > > > +\n> > > > +static inline std::ostream &operator<<(std::ostream &os, const\n> Duration &duration)\n> > > > +{\n> > > > + std::streamsize ss = os.precision();\n> > > > + os << std::fixed << std::setprecision(2) <<\n> DurationValue(duration) << \" us\";\n> > > > + os << std::setprecision(ss) << std::defaultfloat;\n> > >\n> > > This will reset to std::defaultfloat, while the stream may already be\n> in\n> > > std::fixed.\n> >\n> > Good point, I should save and reapply like I do with precision.\n> >\n> > > > + return os;\n> > > > +}\n> > >\n> > > Would it be too much yak-shaving to ask for the implementation to be\n> > > closer to\n> > > https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt, and\n> > > moved to utils.h ? It could be useful more widely in libcamera than\n> just\n> > > in the RPi IPA.\n> >\n> > Yes, I could look into that. Did you mean move the whole implementation\n> into utils.h, or\n> > only the operator <<() bit?\n>\n> I meant operator<<(). DurationValue would likely need to go there too if\n> it's used internally, but the implementation of operator<<() could also\n> hardcode it as\n>\n> os << std::fixed << std::setprecision(2) << duration.count() /\n> 1000.0 << \" us\";\n>\n> if needed.\n>\n> > Actually, I would like to make an impactful change in the next\n> > revision of the series, and replace Duration with a class derived from a\n> std::chrono::duration\n> > object. This is a bit neater in terms of encapsulation, and allows me\n> to implement an\n> > operator bool() method that cleans up the code a bit more.\n> Functionally, everything else\n> > remains identical. Did you want me to put that into utils.h?\n>\n> I think it should actually go to a public header if we want to\n> standardize all timestamps and durations to std::chrono, but that's too\n> much yak-shaving :-) We can handle it later.\n>\n> We already have the following types in utils.h:\n>\n> using clock = std::chrono::steady_clock;\n> using duration = std::chrono::steady_clock::duration;\n> using time_point = std::chrono::steady_clock::time_point;\n>\n> There are also a few helpers to perform string conversion and to convert\n> to a timespec. Ideally all of this should be consolidated, but again\n> it's propably a bit too much work. If we can make operator<<() conform\n> to the C++20 definition to be able to later switch to it, I'll be happy\n> already.\n>\n\nHere's what I think I will do:\n\nI will update patch 1/4 with a new Duration class, now defined in utils.h.\nThis will include\noperator <<() based on the C++20 signature from:\nhttps://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt\n\nThe only thing I would change from the C++20 functionally is to hardcode it\nto output in\nmicroseconds rather than taking the base unit of the Duration class (which\nis nanoseconds).\nFor my current usage, us makes more sense than ns, but I do accept that for\ntimestamps, etc.\nns may be more appropriate. Perhaps some further specialization is needed?\n\nAll the existing std::chrono definitions will remain as-is and we can\nconvert to Duration in due\ncourse.\n\nLet me know if you agree/disagree with the above.\n\nRegards,\nNaush\n\n\n>\n> > Sorry David, your review for patch 1/4 may need redoing with :-(\n> >\n> > > > +\n> > > > +} // namespace RPiController\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 7A94EC31FF\n\tfor ;\n\tWed, 19 May 2021 15:21:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DC8D368915;\n\tWed, 19 May 2021 17:21:22 +0200 (CEST)","from mail-qv1-xf2f.google.com (mail-qv1-xf2f.google.com\n\t[IPv6:2607:f8b0:4864:20::f2f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 78E4468915\n\tfor ;\n\tWed, 19 May 2021 17:21:21 +0200 (CEST)","by mail-qv1-xf2f.google.com with SMTP id h7so6309618qvs.12\n\tfor ;\n\tWed, 19 May 2021 08:21:21 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"nQhx/PH9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=RaWjrHSEyDgcYhWtT6cKsst0qL9Pg/yByBQTz/oie6k=;\n\tb=nQhx/PH9oWd93Bw1c05u9KnJaJw6di1Ycp8Ly/LsdtrGy90ADcM4YJHoa6BIfVpWkC\n\t7e0sfr4SgZKhvz9ggWactqai3TJnI5KhgvZGUQsGURJIuq/0zYuTA3qXAaKrtZWzxpgQ\n\tpYa8yhYfZfzA6wwms977xy3MM2NvkRn6oDO33OjzZkJv9BMAoSVYlvdGe3pgyI8S4Zyg\n\tr733YhzPhTLDVGMvw8RuzskeyGNQJRZBTf3WsSlueW369orEfUHTOBT5PabaAA2Pnx8I\n\tdxrRXCQEZ2nDiBr3LNyxXuqI+ChbT5aixwZBq6SWXIyN7DnuaEIO5KzzzY10kQtPWEDX\n\tHqsA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=RaWjrHSEyDgcYhWtT6cKsst0qL9Pg/yByBQTz/oie6k=;\n\tb=bU0R5fFdLiYmQypmu7Vq9+H8Yp/luvlU57gHng0KbbsXxjtx2x+EJpomLf0x07vgd/\n\t+Q3j/6kfwTsrskX3jV5jzSTBnTMGgnPjOhqLAJbHnBV2o79e5qoZNh57CaiRQA9yLHvQ\n\t8jztx8MEmgLbZ8ZGCZ/4Du7uaFQkXb0/2wpC5vza3O6bPO9l73//yhRjq7+ZWKnmNOLf\n\thPXnQzr+ZZAP9izb1zD7EZOmPmZuPBI/0NBfZNNBvALJ3b16TjINmp+jxYKk7hqaEgSu\n\t+08LQVNHOrBuEjTK0TcPYRuZsEz1RbUW/0IanqwhRhZ2n9pXkTRP6vpCqch/DfXAhYEq\n\tdpUQ==","X-Gm-Message-State":"AOAM533GcwWo4aMYqZ+PHCtaNeic+jCkR40tHgBNTvBlTGJbjfK2+k1Y\n\t3GCS2Jr3Kc+NAn4raBYhErQnavAh2OjniBkj/pdbYnVBn8dpzg==","X-Google-Smtp-Source":"ABdhPJxlp4K6YijjGt0xxx8RQkgii0Egxl+8waMXG0xB9IrjIRvBoof4txapUs3tasRyqUsVhiBfDgw5BM5pLvg0W9I=","X-Received":"by 2002:a05:6214:a11:: with SMTP id\n\tdw17mr13888702qvb.8.1621437680441; \n\tWed, 19 May 2021 08:21:20 -0700 (PDT)","MIME-Version":"1.0","References":"<20210518100706.578526-1-naush@raspberrypi.com>\n\t<20210518100706.578526-2-naush@raspberrypi.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 19 May 2021 16:21:04 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000f7eea605c2b060c2\"","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17029,"web_url":"https://patchwork.libcamera.org/comment/17029/","msgid":"","date":"2021-05-19T15:38:14","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Wed, May 19, 2021 at 04:21:04PM +0100, Naushir Patuck wrote:\n> On Wed, 19 May 2021 at 15:48, Laurent Pinchart wrote:\n> > On Wed, May 19, 2021 at 03:34:05PM +0100, Naushir Patuck wrote:\n> > > On Wed, 19 May 2021 at 15:24, Laurent Pinchart wrote:\n> > > > On Tue, May 18, 2021 at 11:07:03AM +0100, Naushir Patuck wrote:\n> > > > > A new RPiController::Duration typedef is defined to represent a\n> > > > > std::chrono::duration type with double precision nanosecond timebase\n> > > > > that will be used throughout. This minimises the loss of precision when\n> > > > > converting timebases. A function for returning the duration count in any\n> > > > > timebase is also provided.\n> > > > >\n> > > > > An operator << overload is define to help with displaying\n> > > > > RPiController::Duration value in stream objects.\n> > > > >\n> > > > > Signed-off-by: Naushir Patuck \n> > > > > ---\n> > > > > src/ipa/raspberrypi/controller/duration.hpp | 33 +++++++++++++++++++++\n> > > > > 1 file changed, 33 insertions(+)\n> > > > > create mode 100644 src/ipa/raspberrypi/controller/duration.hpp\n> > > > >\n> > > > > diff --git a/src/ipa/raspberrypi/controller/duration.hpp b/src/ipa/raspberrypi/controller/duration.hpp\n> > > > > new file mode 100644\n> > > > > index 000000000000..98aa3d78f52f\n> > > > > --- /dev/null\n> > > > > +++ b/src/ipa/raspberrypi/controller/duration.hpp\n> > > > > @@ -0,0 +1,33 @@\n> > > > > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > > > > +/*\n> > > > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> > > > > + *\n> > > > > + * duration.hpp - Helper macros for std::chrono::duration handling.\n> > > > > + */\n> > > > > +#pragma once\n> > > > > +\n> > > > > +#include \n> > > > > +#include \n> > > > > +#include \n> > > > > +\n> > > > > +namespace RPiController {\n> > > > > +\n> > > > > +using Duration = std::chrono::duration;\n> > > > > +\n> > > > > +// Helper to convert and return the count of any std::chrono::duration type to\n> > > > > +// another timebase. Use a double rep type to try and preserve precision.\n> > > > > +template \n> > > > > +static constexpr double DurationValue(T const &d)\n> > > > > +{\n> > > > > + return std::chrono::duration_cast>(d).count();\n> > > > > +};\n> > > > > +\n> > > > > +static inline std::ostream &operator<<(std::ostream &os, const Duration &duration)\n> > > > > +{\n> > > > > + std::streamsize ss = os.precision();\n> > > > > + os << std::fixed << std::setprecision(2) << DurationValue(duration) << \" us\";\n> > > > > + os << std::setprecision(ss) << std::defaultfloat;\n> > > >\n> > > > This will reset to std::defaultfloat, while the stream may already be in\n> > > > std::fixed.\n> > >\n> > > Good point, I should save and reapply like I do with precision.\n> > >\n> > > > > + return os;\n> > > > > +}\n> > > >\n> > > > Would it be too much yak-shaving to ask for the implementation to be\n> > > > closer to\n> > > > https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt, and\n> > > > moved to utils.h ? It could be useful more widely in libcamera than just\n> > > > in the RPi IPA.\n> > >\n> > > Yes, I could look into that. Did you mean move the whole implementation into utils.h, or\n> > > only the operator <<() bit?\n> >\n> > I meant operator<<(). DurationValue would likely need to go there too if\n> > it's used internally, but the implementation of operator<<() could also\n> > hardcode it as\n> >\n> > os << std::fixed << std::setprecision(2) << duration.count() / 1000.0 << \" us\";\n> >\n> > if needed.\n> >\n> > > Actually, I would like to make an impactful change in the next\n> > > revision of the series, and replace Duration with a class derived from a std::chrono::duration\n> > > object. This is a bit neater in terms of encapsulation, and allows me to implement an\n> > > operator bool() method that cleans up the code a bit more. Functionally, everything else\n> > > remains identical. Did you want me to put that into utils.h?\n> >\n> > I think it should actually go to a public header if we want to\n> > standardize all timestamps and durations to std::chrono, but that's too\n> > much yak-shaving :-) We can handle it later.\n> >\n> > We already have the following types in utils.h:\n> >\n> > using clock = std::chrono::steady_clock;\n> > using duration = std::chrono::steady_clock::duration;\n> > using time_point = std::chrono::steady_clock::time_point;\n> >\n> > There are also a few helpers to perform string conversion and to convert\n> > to a timespec. Ideally all of this should be consolidated, but again\n> > it's propably a bit too much work. If we can make operator<<() conform\n> > to the C++20 definition to be able to later switch to it, I'll be happy\n> > already.\n> \n> Here's what I think I will do:\n> \n> I will update patch 1/4 with a new Duration class, now defined in utils.h.\n> This will include\n> operator <<() based on the C++20 signature from:\n> https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt\n\nSounds good.\n\n> The only thing I would change from the C++20 functionally is to hardcode it to output in\n> microseconds rather than taking the base unit of the Duration class (which is nanoseconds).\n> For my current usage, us makes more sense than ns, but I do accept that for timestamps, etc.\n> ns may be more appropriate. Perhaps some further specialization is needed?\n\nGiven that you print the value as a floating point number, would it make\nsense to print it in seconds ?\n\n> All the existing std::chrono definitions will remain as-is and we can\n> convert to Duration in due\n> course.\n> \n> Let me know if you agree/disagree with the above.\n> \n> > > Sorry David, your review for patch 1/4 may need redoing with :-(\n> > >\n> > > > > +\n> > > > > +} // namespace RPiController","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 5C879C31FB\n\tfor ;\n\tWed, 19 May 2021 15:38:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CBCCF6891A;\n\tWed, 19 May 2021 17:38:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C1B0E68915\n\tfor ;\n\tWed, 19 May 2021 17:38:16 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 39393BA7;\n\tWed, 19 May 2021 17:38:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ViUnoscK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621438696;\n\tbh=Cud1rxhHmudXQXCCkGCBSayKuqf3z22IODNAEFS1/Hc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ViUnoscKqcS8ZHQUubUju5805D0KNWzQ9u6lE62GipmH5Tups3143sFDsP6ctsZbZ\n\tHE/y6G66Zgd0iunxjOd4azOUsAiCo+/8uq5G7cCzvo6Em14S+JOneiprUV1j1I0Hx2\n\tMMcT+ol8bWC0q1tZrcgCP5Y82IQFbDjFWzZTxXmk=","Date":"Wed, 19 May 2021 18:38:14 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210518100706.578526-1-naush@raspberrypi.com>\n\t<20210518100706.578526-2-naush@raspberrypi.com>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17040,"web_url":"https://patchwork.libcamera.org/comment/17040/","msgid":"","date":"2021-05-20T07:43:23","subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Wed, 19 May 2021 at 16:38, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Wed, May 19, 2021 at 04:21:04PM +0100, Naushir Patuck wrote:\n> > On Wed, 19 May 2021 at 15:48, Laurent Pinchart wrote:\n> > > On Wed, May 19, 2021 at 03:34:05PM +0100, Naushir Patuck wrote:\n> > > > On Wed, 19 May 2021 at 15:24, Laurent Pinchart wrote:\n> > > > > On Tue, May 18, 2021 at 11:07:03AM +0100, Naushir Patuck wrote:\n> > > > > > A new RPiController::Duration typedef is defined to represent a\n> > > > > > std::chrono::duration type with double precision nanosecond\n> timebase\n> > > > > > that will be used throughout. This minimises the loss of\n> precision when\n> > > > > > converting timebases. A function for returning the duration\n> count in any\n> > > > > > timebase is also provided.\n> > > > > >\n> > > > > > An operator << overload is define to help with displaying\n> > > > > > RPiController::Duration value in stream objects.\n> > > > > >\n> > > > > > Signed-off-by: Naushir Patuck \n> > > > > > ---\n> > > > > > src/ipa/raspberrypi/controller/duration.hpp | 33\n> +++++++++++++++++++++\n> > > > > > 1 file changed, 33 insertions(+)\n> > > > > > create mode 100644 src/ipa/raspberrypi/controller/duration.hpp\n> > > > > >\n> > > > > > diff --git a/src/ipa/raspberrypi/controller/duration.hpp\n> b/src/ipa/raspberrypi/controller/duration.hpp\n> > > > > > new file mode 100644\n> > > > > > index 000000000000..98aa3d78f52f\n> > > > > > --- /dev/null\n> > > > > > +++ b/src/ipa/raspberrypi/controller/duration.hpp\n> > > > > > @@ -0,0 +1,33 @@\n> > > > > > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > > > > > +/*\n> > > > > > + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> > > > > > + *\n> > > > > > + * duration.hpp - Helper macros for std::chrono::duration\n> handling.\n> > > > > > + */\n> > > > > > +#pragma once\n> > > > > > +\n> > > > > > +#include \n> > > > > > +#include \n> > > > > > +#include \n> > > > > > +\n> > > > > > +namespace RPiController {\n> > > > > > +\n> > > > > > +using Duration = std::chrono::duration;\n> > > > > > +\n> > > > > > +// Helper to convert and return the count of any\n> std::chrono::duration type to\n> > > > > > +// another timebase. Use a double rep type to try and preserve\n> precision.\n> > > > > > +template \n> > > > > > +static constexpr double DurationValue(T const &d)\n> > > > > > +{\n> > > > > > + return\n> std::chrono::duration_cast>(d).count();\n> > > > > > +};\n> > > > > > +\n> > > > > > +static inline std::ostream &operator<<(std::ostream &os, const\n> Duration &duration)\n> > > > > > +{\n> > > > > > + std::streamsize ss = os.precision();\n> > > > > > + os << std::fixed << std::setprecision(2) <<\n> DurationValue(duration) << \" us\";\n> > > > > > + os << std::setprecision(ss) << std::defaultfloat;\n> > > > >\n> > > > > This will reset to std::defaultfloat, while the stream may already\n> be in\n> > > > > std::fixed.\n> > > >\n> > > > Good point, I should save and reapply like I do with precision.\n> > > >\n> > > > > > + return os;\n> > > > > > +}\n> > > > >\n> > > > > Would it be too much yak-shaving to ask for the implementation to\n> be\n> > > > > closer to\n> > > > > https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt,\n> and\n> > > > > moved to utils.h ? It could be useful more widely in libcamera\n> than just\n> > > > > in the RPi IPA.\n> > > >\n> > > > Yes, I could look into that. Did you mean move the whole\n> implementation into utils.h, or\n> > > > only the operator <<() bit?\n> > >\n> > > I meant operator<<(). DurationValue would likely need to go there too\n> if\n> > > it's used internally, but the implementation of operator<<() could also\n> > > hardcode it as\n> > >\n> > > os << std::fixed << std::setprecision(2) << duration.count() /\n> 1000.0 << \" us\";\n> > >\n> > > if needed.\n> > >\n> > > > Actually, I would like to make an impactful change in the next\n> > > > revision of the series, and replace Duration with a class derived\n> from a std::chrono::duration\n> > > > object. This is a bit neater in terms of encapsulation, and allows\n> me to implement an\n> > > > operator bool() method that cleans up the code a bit more.\n> Functionally, everything else\n> > > > remains identical. Did you want me to put that into utils.h?\n> > >\n> > > I think it should actually go to a public header if we want to\n> > > standardize all timestamps and durations to std::chrono, but that's too\n> > > much yak-shaving :-) We can handle it later.\n> > >\n> > > We already have the following types in utils.h:\n> > >\n> > > using clock = std::chrono::steady_clock;\n> > > using duration = std::chrono::steady_clock::duration;\n> > > using time_point = std::chrono::steady_clock::time_point;\n> > >\n> > > There are also a few helpers to perform string conversion and to\n> convert\n> > > to a timespec. Ideally all of this should be consolidated, but again\n> > > it's propably a bit too much work. If we can make operator<<() conform\n> > > to the C++20 definition to be able to later switch to it, I'll be happy\n> > > already.\n> >\n> > Here's what I think I will do:\n> >\n> > I will update patch 1/4 with a new Duration class, now defined in\n> utils.h.\n> > This will include\n> > operator <<() based on the C++20 signature from:\n> > https://en.cppreference.com/w/cpp/chrono/duration/operator_ltlt\n>\n> Sounds good.\n>\n> > The only thing I would change from the C++20 functionally is to hardcode\n> it to output in\n> > microseconds rather than taking the base unit of the Duration class\n> (which is nanoseconds).\n> > For my current usage, us makes more sense than ns, but I do accept that\n> for timestamps, etc.\n> > ns may be more appropriate. Perhaps some further specialization is\n> needed?\n>\n> Given that you print the value as a floating point number, would it make\n> sense to print it in seconds ?\n>\n\nMy personal preference would be to keep it in microseconds, as I find it\neasier to read the\nvalues that relate to exposure, frame durations, etc. However, I am not\nstrongly opposed to\nchanging it if the consensus goes the other way.\n\n\n>\n> > All the existing std::chrono definitions will remain as-is and we can\n> > convert to Duration in due\n> > course.\n> >\n> > Let me know if you agree/disagree with the above.\n> >\n> > > > Sorry David, your review for patch 1/4 may need redoing with :-(\n> > > >\n> > > > > > +\n> > > > > > +} // namespace RPiController\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 3CCC3C31FF\n\tfor ;\n\tThu, 20 May 2021 07:43:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 876B16891D;\n\tThu, 20 May 2021 09:43:41 +0200 (CEST)","from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com\n\t[IPv6:2a00:1450:4864:20::22b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02FD768919\n\tfor ;\n\tThu, 20 May 2021 09:43:39 +0200 (CEST)","by mail-lj1-x22b.google.com with SMTP id b12so11328461ljp.1\n\tfor ;\n\tThu, 20 May 2021 00:43:39 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"LNTiPDjr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=uPH7kvHgrC6uCxOhNVutu7wHF2y4d3qOG9VaI0YPqvU=;\n\tb=LNTiPDjrDXVp0wFijfN4yicnsuGDiFTC/Obbie/k70PabUUKuDfNeyKLlexV7OGvgE\n\teN9w1AfA+utAW/0D2vLDo22Jsa81ZVuFCcDMFQ3G4RWm0d6BS7d2WzKrK/VYpjIuHlnA\n\tZUR6CxZDYamZat/73afE80jhNxyfR8XxWC7WeTutPB6bsMVEVtur9tFSnPuBwowv7vmb\n\thqtxHOGn3l60jSHl5M/7f8q0hl7v6DoiSsJ7Wb4p20SSpmdo0arhhuzUUkQ3A2wxxMfu\n\tWwRoPRM8YyDRZAuuhUuo6Crxuk1xaxLyZ8GmpSHje7lmLFzPsdVJikKPYO4hH9tTSEa9\n\t/wFw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=uPH7kvHgrC6uCxOhNVutu7wHF2y4d3qOG9VaI0YPqvU=;\n\tb=Smh8bAbl2z2+nou1DBTVoHC31UFA9GkgoJQmQB9wyWj5fe2LNwwG4UH0es49JuxYaf\n\t372bCjtC46qlAbCjPPQzojQj4dtJ3wa0ZR22wsvZbZPAcbxU4jdt/XakJZ97b+p+lsSg\n\t+V2DiOk3xhCU2JNd+MIi6dEKkawcVlsSk3gdKq/XmLk2gOEcJBzVRlsIQd4V+FijrZWX\n\tL9hRvXh71TeWSqzCwG0MCO0RSo7vx0Tn0vhCDrN8NSx4PMeY5kz4S0YmjEmAqTYySesx\n\thug4V8ZakdTROXT+3v4YuYcEeFaRVh0x+aiSgRTCOLxnrgXmJLy+A71qg5DtftmSMgw2\n\tOpyA==","X-Gm-Message-State":"AOAM533N4jHOEW51aUK+dbyTUvY8H0Q6ts9slaAJect5mhoL8qh/EJtW\n\taua0yXQrfnOHactTTEcAkDLTOJgeALdJlDOdRj8GjA==","X-Google-Smtp-Source":"ABdhPJyAaPp4V1NhH8pLQbL+2Ny+essGMnvRdzmR14pDdg3/k0x6xeOlYhxyxAZjfC3L4nxE+xYSNgplrt6t6vxQlLM=","X-Received":"by 2002:a2e:9896:: with SMTP id\n\tb22mr2147203ljj.329.1621496619169; \n\tThu, 20 May 2021 00:43:39 -0700 (PDT)","MIME-Version":"1.0","References":"<20210518100706.578526-1-naush@raspberrypi.com>\n\t<20210518100706.578526-2-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Thu, 20 May 2021 08:43:23 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000fd910b05c2be1900\"","Subject":"Re: [libcamera-devel] [PATCH 1/4] ipa: raspberrypi: Add helper\n\tmacros for std::chrono::duration","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","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]