[{"id":11045,"web_url":"https://patchwork.libcamera.org/comment/11045/","msgid":"<20200701161302.h5hcrz5atgovrpai@uno.localdomain>","date":"2020-07-01T16:13:02","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa: raspberrypi:\n\tEnable focus measure without recompile","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Wed, Jul 01, 2020 at 01:50:21PM +0100, David Plowman wrote:\n> Previously, output of the focus measure could not be enabled without\n> recompiling (because of the RPI_LOGGING_ENABLE macro). This uses the\n> libcamera logging mechanism instead, so can be enabled/disabled at\n> runtime.\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  src/ipa/raspberrypi/controller/rpi/focus.cpp | 16 ++++++----------\n>  src/ipa/raspberrypi/controller/rpi/focus.hpp |  3 ---\n>  src/ipa/raspberrypi/data/imx477.json         |  2 +-\n>  3 files changed, 7 insertions(+), 14 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> index 1e2b649..573831b 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> @@ -6,11 +6,15 @@\n>   */\n>  #include <stdint.h>\n>\n> +#include \"libcamera/internal/log.h\"\n> +\n>  #include \"../focus_status.h\"\n> -#include \"../logging.hpp\"\n>  #include \"focus.hpp\"\n>\n>  using namespace RPi;\n> +using namespace libcamera;\n> +\n\nOCD says 'l' comes before 'R'\n\n> +LOG_DEFINE_CATEGORY(RPI_FOCUS)\n>\n>  #define NAME \"rpi.focus\"\n>\n> @@ -24,11 +28,6 @@ char const *Focus::Name() const\n>  \treturn NAME;\n>  }\n>\n> -void Focus::Read(boost::property_tree::ptree const &params)\n> -{\n> -\tprint_ = params.get<int>(\"print\", 0);\n> -}\n> -\n>  void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)\n>  {\n>  \tFocusStatus status;\n> @@ -37,10 +36,7 @@ void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)\n>  \t\tstatus.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1] / 1000;\n>  \tstatus.num = i;\n>  \timage_metadata->Set(\"focus.status\", status);\n> -\tif (print_) {\n> -\t\tuint32_t value = (status.focus_measures[5] + status.focus_measures[6]) / 10;\n> -\t\tRPI_LOG(\"Focus contrast measure: \" << value);\n> -\t}\n> +\tLOG(RPI_FOCUS, Debug) << \"Focus contrast measure: \" << (status.focus_measures[5] + status.focus_measures[6]) / 10;\n\nmaybe break this line\n\n>  }\n>\n>  /* Register algorithm with the system. */\n> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.hpp b/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> index d53401f..a9756ea 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> @@ -22,10 +22,7 @@ class Focus : public Algorithm\n>  public:\n>  \tFocus(Controller *controller);\n>  \tchar const *Name() const override;\n> -\tvoid Read(boost::property_tree::ptree const &params) override;\n>  \tvoid Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> -private:\n> -\tbool print_;\n>  };\n>\n>  } /* namespace RPi */\n> diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json\n> index 389e8ce..747cd8d 100644\n> --- a/src/ipa/raspberrypi/data/imx477.json\n> +++ b/src/ipa/raspberrypi/data/imx477.json\n> @@ -415,6 +415,6 @@\n>      },\n>      \"rpi.focus\":\n>      {\n> -\t\"print\": 1\n> +\n\nIs the empty line intentional ?\n\nMinors apart, which can be fixed while applying:\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n>      }\n>  }\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 96E03BFFE2\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  1 Jul 2020 16:09:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7097260C57;\n\tWed,  1 Jul 2020 18:09:35 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B56E8609C6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  1 Jul 2020 18:09:34 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 4B360FF80B;\n\tWed,  1 Jul 2020 16:09:33 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 1 Jul 2020 18:13:02 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200701161302.h5hcrz5atgovrpai@uno.localdomain>","References":"<20200701125021.1388-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200701125021.1388-1-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa: raspberrypi:\n\tEnable focus measure without recompile","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11050,"web_url":"https://patchwork.libcamera.org/comment/11050/","msgid":"<CAHW6GY+fk7cjtX7a5tcRXcjA=HGQqpqZ-KG+C+xtxuDBRV9rpQ@mail.gmail.com>","date":"2020-07-01T16:25:00","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa: raspberrypi:\n\tEnable focus measure without recompile","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for the review!\n\nOn Wed, 1 Jul 2020 at 17:09, Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> Hi David,\n>\n> On Wed, Jul 01, 2020 at 01:50:21PM +0100, David Plowman wrote:\n> > Previously, output of the focus measure could not be enabled without\n> > recompiling (because of the RPI_LOGGING_ENABLE macro). This uses the\n> > libcamera logging mechanism instead, so can be enabled/disabled at\n> > runtime.\n> >\n> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  src/ipa/raspberrypi/controller/rpi/focus.cpp | 16 ++++++----------\n> >  src/ipa/raspberrypi/controller/rpi/focus.hpp |  3 ---\n> >  src/ipa/raspberrypi/data/imx477.json         |  2 +-\n> >  3 files changed, 7 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > index 1e2b649..573831b 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > @@ -6,11 +6,15 @@\n> >   */\n> >  #include <stdint.h>\n> >\n> > +#include \"libcamera/internal/log.h\"\n> > +\n> >  #include \"../focus_status.h\"\n> > -#include \"../logging.hpp\"\n> >  #include \"focus.hpp\"\n> >\n> >  using namespace RPi;\n> > +using namespace libcamera;\n> > +\n>\n> OCD says 'l' comes before 'R'\n\nNo problem with that!\n\n>\n> > +LOG_DEFINE_CATEGORY(RPI_FOCUS)\n> >\n> >  #define NAME \"rpi.focus\"\n> >\n> > @@ -24,11 +28,6 @@ char const *Focus::Name() const\n> >       return NAME;\n> >  }\n> >\n> > -void Focus::Read(boost::property_tree::ptree const &params)\n> > -{\n> > -     print_ = params.get<int>(\"print\", 0);\n> > -}\n> > -\n> >  void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> >  {\n> >       FocusStatus status;\n> > @@ -37,10 +36,7 @@ void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> >               status.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1] / 1000;\n> >       status.num = i;\n> >       image_metadata->Set(\"focus.status\", status);\n> > -     if (print_) {\n> > -             uint32_t value = (status.focus_measures[5] + status.focus_measures[6]) / 10;\n> > -             RPI_LOG(\"Focus contrast measure: \" << value);\n> > -     }\n> > +     LOG(RPI_FOCUS, Debug) << \"Focus contrast measure: \" << (status.focus_measures[5] + status.focus_measures[6]) / 10;\n>\n> maybe break this line\n\nPretty sure I had it like that first but checkstyle complained, and\nthen it was OK with this. But happy if we prefer to change it...\n\n>\n> >  }\n> >\n> >  /* Register algorithm with the system. */\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.hpp b/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> > index d53401f..a9756ea 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> > @@ -22,10 +22,7 @@ class Focus : public Algorithm\n> >  public:\n> >       Focus(Controller *controller);\n> >       char const *Name() const override;\n> > -     void Read(boost::property_tree::ptree const &params) override;\n> >       void Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> > -private:\n> > -     bool print_;\n> >  };\n> >\n> >  } /* namespace RPi */\n> > diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json\n> > index 389e8ce..747cd8d 100644\n> > --- a/src/ipa/raspberrypi/data/imx477.json\n> > +++ b/src/ipa/raspberrypi/data/imx477.json\n> > @@ -415,6 +415,6 @@\n> >      },\n> >      \"rpi.focus\":\n> >      {\n> > -     \"print\": 1\n> > +\n>\n> Is the empty line intentional ?\n\nWell, for reasons I'm not totally clear about, our Python tuning tool\ntends to produce a blank line when there's nothing between the curlies\n- so I just tend to copy it like that. Mostly. But not always. So I'm\nfine to delete it too...\n\nBest regards\nDavid\n\n>\n> Minors apart, which can be fixed while applying:\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> Thanks\n>    j\n>\n> >      }\n> >  }\n> > --\n> > 2.20.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 88394BE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  1 Jul 2020 16:25:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5684660C53;\n\tWed,  1 Jul 2020 18:25:13 +0200 (CEST)","from mail-ot1-x341.google.com (mail-ot1-x341.google.com\n\t[IPv6:2607:f8b0:4864:20::341])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C4EFF609A9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  1 Jul 2020 18:25:11 +0200 (CEST)","by mail-ot1-x341.google.com with SMTP id 5so19856449oty.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 01 Jul 2020 09:25:11 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"bSLieA8P\"; 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=fYAjfioCAJDxtzihT1/+mKyXCrcy8gDeNtlVpGGtJts=;\n\tb=bSLieA8Px/xSO1QZF4lNbAgMIojLGPt4knv02YEnAarRraqfAf3mdAKXnkl926GtnS\n\t0HcpHA5HWxIyyaOJ0RXSC2QBQeTWOohiDIYRAOJFDxwUsC55Y0kBDgb49+7tDpYYUZfD\n\tudLCHPCjy8G/B5HTmzUZgxms3BQCoGpkFHTYZouHcbJerDA6iZ+WmTSY4N3MPgPKoH5y\n\tC4cNQeJQfRRHCt3wuSr84+cXlRaw4+px4YHfM8Y9+eaotiYYGf91jBo+SyUQE5xSVU4w\n\tmL4PgnWzkX0Fi+gr1MqKTO3M1ZbZNMR0T81cWsL4o1fvBHdRF4AaWXJM+eI4FC4jpDB4\n\tgEKQ==","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=fYAjfioCAJDxtzihT1/+mKyXCrcy8gDeNtlVpGGtJts=;\n\tb=hGE4RUZbT+XD7eT1yz0q9pj89kynFOoGYlOEVXFDv6Na3lzOojr/OTAwmv8soXb3s2\n\tAJsgnrd9t0iypFlDs5pMfBmuyF+0YeWF3LdjMfKffF4OCTYwQxWB8UvHUe/EzHm687cs\n\tKrwgXhnhUpqinBcsIHdguk9w2fVyvjlOmMnQs9OQdJL81aqBc7FBIpfuD8y0MmdvJNRP\n\tzWyVkR5CGmrXwkifhuxXB38NdfXj0lNshYrRnycgtyivw0dVYT/FxowSYUQlILOYRkVv\n\tnIe4T/R0uMLxHOs/GrYe4lnq73txF7n+pVazCuGASbychCnTHl7Lonvtx12UJzLRRKeI\n\tPoNg==","X-Gm-Message-State":"AOAM533LzRwUiHeyeUXLRLt2Rb5nE4wWJGMadm0Y//eAYRsUN3BuAiae\n\tGgFCQs2INwGDI+AsEQqq81664LimmgESrz82ByazxQ==","X-Google-Smtp-Source":"ABdhPJyxbkN48N7BN0sEJveGGGjvM5eGK9H4VR9VkNwu3li4IHTBKcnPezIJYwplwssCz5fOt8xaZJcyylvPUP4nVoU=","X-Received":"by 2002:a9d:6a92:: with SMTP id\n\tl18mr2460457otq.317.1593620710486; \n\tWed, 01 Jul 2020 09:25:10 -0700 (PDT)","MIME-Version":"1.0","References":"<20200701125021.1388-1-david.plowman@raspberrypi.com>\n\t<20200701161302.h5hcrz5atgovrpai@uno.localdomain>","In-Reply-To":"<20200701161302.h5hcrz5atgovrpai@uno.localdomain>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Wed, 1 Jul 2020 17:25:00 +0100","Message-ID":"<CAHW6GY+fk7cjtX7a5tcRXcjA=HGQqpqZ-KG+C+xtxuDBRV9rpQ@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa: raspberrypi:\n\tEnable focus measure without recompile","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11100,"web_url":"https://patchwork.libcamera.org/comment/11100/","msgid":"<20200703001736.GA12562@pendragon.ideasonboard.com>","date":"2020-07-03T00:17:36","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa: raspberrypi:\n\tEnable focus measure without recompile","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David and Jacopo,\n\nOn Wed, Jul 01, 2020 at 05:25:00PM +0100, David Plowman wrote:\n> On Wed, 1 Jul 2020 at 17:09, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> > On Wed, Jul 01, 2020 at 01:50:21PM +0100, David Plowman wrote:\n> > > Previously, output of the focus measure could not be enabled without\n> > > recompiling (because of the RPI_LOGGING_ENABLE macro). This uses the\n> > > libcamera logging mechanism instead, so can be enabled/disabled at\n> > > runtime.\n> > >\n> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > > ---\n> > >  src/ipa/raspberrypi/controller/rpi/focus.cpp | 16 ++++++----------\n> > >  src/ipa/raspberrypi/controller/rpi/focus.hpp |  3 ---\n> > >  src/ipa/raspberrypi/data/imx477.json         |  2 +-\n> > >  3 files changed, 7 insertions(+), 14 deletions(-)\n> > >\n> > > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > > index 1e2b649..573831b 100644\n> > > --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > > +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > > @@ -6,11 +6,15 @@\n> > >   */\n> > >  #include <stdint.h>\n> > >\n> > > +#include \"libcamera/internal/log.h\"\n> > > +\n> > >  #include \"../focus_status.h\"\n> > > -#include \"../logging.hpp\"\n> > >  #include \"focus.hpp\"\n> > >\n> > >  using namespace RPi;\n> > > +using namespace libcamera;\n> > > +\n> >\n> > OCD says 'l' comes before 'R'\n\n>>> using = ['RPi', 'libcamera']\n>>> using.sort()\n>>> using\n['RPi', 'libcamera']\n\nPython disagrees with your OCD ;-)\n\n> No problem with that!\n> \n> > > +LOG_DEFINE_CATEGORY(RPI_FOCUS)\n\nWe tend to use CamelCase for log categories. Would RPiFocus be OK for\nyou ?\n\nI may work in supporting subcategories at some point, in which case this\nwould become RPi.Focus (the name used with the LOG macro would will\nstill be RPiFocus though, that one needs to be a valid C++ symbol name).\n\n> > >\n> > >  #define NAME \"rpi.focus\"\n> > >\n> > > @@ -24,11 +28,6 @@ char const *Focus::Name() const\n> > >       return NAME;\n> > >  }\n> > >\n> > > -void Focus::Read(boost::property_tree::ptree const &params)\n> > > -{\n> > > -     print_ = params.get<int>(\"print\", 0);\n> > > -}\n> > > -\n> > >  void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> > >  {\n> > >       FocusStatus status;\n> > > @@ -37,10 +36,7 @@ void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> > >               status.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1] / 1000;\n> > >       status.num = i;\n> > >       image_metadata->Set(\"focus.status\", status);\n> > > -     if (print_) {\n> > > -             uint32_t value = (status.focus_measures[5] + status.focus_measures[6]) / 10;\n> > > -             RPI_LOG(\"Focus contrast measure: \" << value);\n> > > -     }\n> > > +     LOG(RPI_FOCUS, Debug) << \"Focus contrast measure: \" << (status.focus_measures[5] + status.focus_measures[6]) / 10;\n> >\n> > maybe break this line\n> \n> Pretty sure I had it like that first but checkstyle complained, and\n> then it was OK with this. But happy if we prefer to change it...\n\nlibcamera has a hard 120 columns limit, and a soft 80 columns limit.\nThis means we prefer keeping lines under 80 columns, but going over up\nto 120 is fine when it improves readability. checkstyle.py relies on\nclang-format, which doesn't support soft and hard limits, so it will\nsometimes propose removing line breaks.\n\nAnd now that I've written this, I see that our .clang-format file has\nColumnLimit set to 0 (no limit). I have thus no idea what's going on :-)\n\n> > >  }\n> > >\n> > >  /* Register algorithm with the system. */\n> > > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.hpp b/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> > > index d53401f..a9756ea 100644\n> > > --- a/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> > > +++ b/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> > > @@ -22,10 +22,7 @@ class Focus : public Algorithm\n> > >  public:\n> > >       Focus(Controller *controller);\n> > >       char const *Name() const override;\n> > > -     void Read(boost::property_tree::ptree const &params) override;\n> > >       void Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> > > -private:\n> > > -     bool print_;\n> > >  };\n> > >\n> > >  } /* namespace RPi */\n> > > diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json\n> > > index 389e8ce..747cd8d 100644\n> > > --- a/src/ipa/raspberrypi/data/imx477.json\n> > > +++ b/src/ipa/raspberrypi/data/imx477.json\n> > > @@ -415,6 +415,6 @@\n> > >      },\n> > >      \"rpi.focus\":\n> > >      {\n> > > -     \"print\": 1\n> > > +\n> >\n> > Is the empty line intentional ?\n> \n> Well, for reasons I'm not totally clear about, our Python tuning tool\n> tends to produce a blank line when there's nothing between the curlies\n> - so I just tend to copy it like that. Mostly. But not always. So I'm\n> fine to delete it too...\n\n\"[PATCH 00/10] utils: raspberrypi: ctt: Improve JSON pretty printer\"\n\nI'll delete the empty line :-)\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> > Minors apart, which can be fixed while applying:\n> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > >      }\n> > >  }","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A89A6BE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  3 Jul 2020 00:17:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 34A2C60C55;\n\tFri,  3 Jul 2020 02:17:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A229609C7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  3 Jul 2020 02:17:40 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E6BCF9CB;\n\tFri,  3 Jul 2020 02:17:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"QZ4qBWGx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593735460;\n\tbh=wifeUgqaaEAwlGkjnwD/g+sUzGdPVmzEwmxYLKSNCgg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QZ4qBWGxxIpRYAACx9cSaUV+pMJiujIRuaPyYs6nmVPonXQrpZFpr7fd8tiHeATt+\n\t7LE1uX15G/OjBTz3yAO3wXmBu9KKKsC7xJ3W0O/jD9U0i1NNnZvGLk/cjHrLIM3cou\n\twRZZdAUxUQ9Wc/eVfJhpNTCOH5e1vorqUmdhVFJY=","Date":"Fri, 3 Jul 2020 03:17:36 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200703001736.GA12562@pendragon.ideasonboard.com>","References":"<20200701125021.1388-1-david.plowman@raspberrypi.com>\n\t<20200701161302.h5hcrz5atgovrpai@uno.localdomain>\n\t<CAHW6GY+fk7cjtX7a5tcRXcjA=HGQqpqZ-KG+C+xtxuDBRV9rpQ@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAHW6GY+fk7cjtX7a5tcRXcjA=HGQqpqZ-KG+C+xtxuDBRV9rpQ@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa: raspberrypi:\n\tEnable focus measure without recompile","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11116,"web_url":"https://patchwork.libcamera.org/comment/11116/","msgid":"<CAHW6GYJFYC8VTEYQN4wx+wVgs-+=fPVvBXAfoRY=qhtMZkN2ZQ@mail.gmail.com>","date":"2020-07-03T08:22:15","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa: raspberrypi:\n\tEnable focus measure without recompile","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nThanks for the various changes, they're all fine with me!\n\nAnd I'll review the ctt patches today. Thanks for all that work, I\nnever thought anyone was going to touch that ever again!\n\nBest regards\nDavid\n\nOn Fri, 3 Jul 2020 at 01:17, Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi David and Jacopo,\n>\n> On Wed, Jul 01, 2020 at 05:25:00PM +0100, David Plowman wrote:\n> > On Wed, 1 Jul 2020 at 17:09, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> > > On Wed, Jul 01, 2020 at 01:50:21PM +0100, David Plowman wrote:\n> > > > Previously, output of the focus measure could not be enabled without\n> > > > recompiling (because of the RPI_LOGGING_ENABLE macro). This uses the\n> > > > libcamera logging mechanism instead, so can be enabled/disabled at\n> > > > runtime.\n> > > >\n> > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > ---\n> > > >  src/ipa/raspberrypi/controller/rpi/focus.cpp | 16 ++++++----------\n> > > >  src/ipa/raspberrypi/controller/rpi/focus.hpp |  3 ---\n> > > >  src/ipa/raspberrypi/data/imx477.json         |  2 +-\n> > > >  3 files changed, 7 insertions(+), 14 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > > > index 1e2b649..573831b 100644\n> > > > --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > > > +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > > > @@ -6,11 +6,15 @@\n> > > >   */\n> > > >  #include <stdint.h>\n> > > >\n> > > > +#include \"libcamera/internal/log.h\"\n> > > > +\n> > > >  #include \"../focus_status.h\"\n> > > > -#include \"../logging.hpp\"\n> > > >  #include \"focus.hpp\"\n> > > >\n> > > >  using namespace RPi;\n> > > > +using namespace libcamera;\n> > > > +\n> > >\n> > > OCD says 'l' comes before 'R'\n>\n> >>> using = ['RPi', 'libcamera']\n> >>> using.sort()\n> >>> using\n> ['RPi', 'libcamera']\n>\n> Python disagrees with your OCD ;-)\n>\n> > No problem with that!\n> >\n> > > > +LOG_DEFINE_CATEGORY(RPI_FOCUS)\n>\n> We tend to use CamelCase for log categories. Would RPiFocus be OK for\n> you ?\n>\n> I may work in supporting subcategories at some point, in which case this\n> would become RPi.Focus (the name used with the LOG macro would will\n> still be RPiFocus though, that one needs to be a valid C++ symbol name).\n>\n> > > >\n> > > >  #define NAME \"rpi.focus\"\n> > > >\n> > > > @@ -24,11 +28,6 @@ char const *Focus::Name() const\n> > > >       return NAME;\n> > > >  }\n> > > >\n> > > > -void Focus::Read(boost::property_tree::ptree const &params)\n> > > > -{\n> > > > -     print_ = params.get<int>(\"print\", 0);\n> > > > -}\n> > > > -\n> > > >  void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> > > >  {\n> > > >       FocusStatus status;\n> > > > @@ -37,10 +36,7 @@ void Focus::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> > > >               status.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1] / 1000;\n> > > >       status.num = i;\n> > > >       image_metadata->Set(\"focus.status\", status);\n> > > > -     if (print_) {\n> > > > -             uint32_t value = (status.focus_measures[5] + status.focus_measures[6]) / 10;\n> > > > -             RPI_LOG(\"Focus contrast measure: \" << value);\n> > > > -     }\n> > > > +     LOG(RPI_FOCUS, Debug) << \"Focus contrast measure: \" << (status.focus_measures[5] + status.focus_measures[6]) / 10;\n> > >\n> > > maybe break this line\n> >\n> > Pretty sure I had it like that first but checkstyle complained, and\n> > then it was OK with this. But happy if we prefer to change it...\n>\n> libcamera has a hard 120 columns limit, and a soft 80 columns limit.\n> This means we prefer keeping lines under 80 columns, but going over up\n> to 120 is fine when it improves readability. checkstyle.py relies on\n> clang-format, which doesn't support soft and hard limits, so it will\n> sometimes propose removing line breaks.\n>\n> And now that I've written this, I see that our .clang-format file has\n> ColumnLimit set to 0 (no limit). I have thus no idea what's going on :-)\n>\n> > > >  }\n> > > >\n> > > >  /* Register algorithm with the system. */\n> > > > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.hpp b/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> > > > index d53401f..a9756ea 100644\n> > > > --- a/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> > > > +++ b/src/ipa/raspberrypi/controller/rpi/focus.hpp\n> > > > @@ -22,10 +22,7 @@ class Focus : public Algorithm\n> > > >  public:\n> > > >       Focus(Controller *controller);\n> > > >       char const *Name() const override;\n> > > > -     void Read(boost::property_tree::ptree const &params) override;\n> > > >       void Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> > > > -private:\n> > > > -     bool print_;\n> > > >  };\n> > > >\n> > > >  } /* namespace RPi */\n> > > > diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json\n> > > > index 389e8ce..747cd8d 100644\n> > > > --- a/src/ipa/raspberrypi/data/imx477.json\n> > > > +++ b/src/ipa/raspberrypi/data/imx477.json\n> > > > @@ -415,6 +415,6 @@\n> > > >      },\n> > > >      \"rpi.focus\":\n> > > >      {\n> > > > -     \"print\": 1\n> > > > +\n> > >\n> > > Is the empty line intentional ?\n> >\n> > Well, for reasons I'm not totally clear about, our Python tuning tool\n> > tends to produce a blank line when there's nothing between the curlies\n> > - so I just tend to copy it like that. Mostly. But not always. So I'm\n> > fine to delete it too...\n>\n> \"[PATCH 00/10] utils: raspberrypi: ctt: Improve JSON pretty printer\"\n>\n> I'll delete the empty line :-)\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n> > > Minors apart, which can be fixed while applying:\n> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> > >\n> > > >      }\n> > > >  }\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 DC6AFBE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  3 Jul 2020 08:22:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 79BD660C50;\n\tFri,  3 Jul 2020 10:22:29 +0200 (CEST)","from mail-ot1-x344.google.com (mail-ot1-x344.google.com\n\t[IPv6:2607:f8b0:4864:20::344])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C233B603AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  3 Jul 2020 10:22:27 +0200 (CEST)","by mail-ot1-x344.google.com with SMTP id d4so26161577otk.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 03 Jul 2020 01:22:27 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"JWoMCCI7\"; 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=Q1pD1T4q6Q7x6cn8B4ojBED9i5NanwCWvW0X4PSsAug=;\n\tb=JWoMCCI7DmlrchxZ6ze2+hA6woKbWeBDo/l5JuU38X/0v/B2Q7zw/VrXFEGbKViVLq\n\tSxFoiYO82cJR3BRvmOGhfzN2rmNo+bQXCt6ltxE/ihqeXe0PerhF/AYzDGK1p83PGnzI\n\tOFWNRBbbTuvkrLlrpbvW2WK5kRcrKqTqli9t6q94BbToXoQXLpm8coxLqjnToGje4olz\n\tP2Sv4S/R3AyQ4n0jEe0HW/4DqSUnMoHFJakqx2jqxPaGw6CPU/7A3AaKw15fdoK+67mM\n\tmpM54fkB6o6nLwiYYejw7F6U96irf7H1tahEmVW+ArjM0/AXYf5N0Sq0mhpfXuaQi+NR\n\tYGeQ==","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=Q1pD1T4q6Q7x6cn8B4ojBED9i5NanwCWvW0X4PSsAug=;\n\tb=TWFkRdYRcPa8xvoyf3fwvOcdq8593NB2DbVE9d1GQjGtauXpRmWrUNXwHU8ZRbsY0S\n\takaLpBzF3PiGgcugG+qDWqj1E117SXEZAxxrYoZ0bpwtG+S4oXrtZBQ/eHf4sGkWahhQ\n\t0g+vNiA7L0PccZmUEUdz8qY4V6Vvwhbx5ApE8qsuXbhqhlndBywnMi9CDpzQR5gIerwM\n\tXZO5bMhW26ZVmZ33MV9KTQA4HfyDEX0Dfm+5twRA0lxo1fzXEq1u7V82wKSUIpRlsYHW\n\tPve7wL4UnUXCeYsJk6ZBLZ6G3KFXyWUddOu9jK/tqGMv78WflwVkH7u293hhDlRmCfTg\n\tdxZw==","X-Gm-Message-State":"AOAM532tcDKeqOJG2E1GIHiDcP8343uEQ7U4u2FJN8rHyph2PJ8Vv3Aj\n\tFzB5ri4kXE8gC33+5G1j/MbLyKjmop7KXD+qj31q0A==","X-Google-Smtp-Source":"ABdhPJyKgf32lgnX+OacVVWFv2yHjG900XPMYT13RBVdAzOY6nZ1xKuLD337KGU4lCHpA5fW9xF4praLihdBHDFq8a0=","X-Received":"by 2002:a9d:6a92:: with SMTP id\n\tl18mr9578291otq.317.1593764545518; \n\tFri, 03 Jul 2020 01:22:25 -0700 (PDT)","MIME-Version":"1.0","References":"<20200701125021.1388-1-david.plowman@raspberrypi.com>\n\t<20200701161302.h5hcrz5atgovrpai@uno.localdomain>\n\t<CAHW6GY+fk7cjtX7a5tcRXcjA=HGQqpqZ-KG+C+xtxuDBRV9rpQ@mail.gmail.com>\n\t<20200703001736.GA12562@pendragon.ideasonboard.com>","In-Reply-To":"<20200703001736.GA12562@pendragon.ideasonboard.com>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Fri, 3 Jul 2020 09:22:15 +0100","Message-ID":"<CAHW6GYJFYC8VTEYQN4wx+wVgs-+=fPVvBXAfoRY=qhtMZkN2ZQ@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: ipa: raspberrypi:\n\tEnable focus measure without recompile","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]