[{"id":22131,"web_url":"https://patchwork.libcamera.org/comment/22131/","msgid":"","date":"2022-02-07T23:23:17","subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: awb: Better\n\thandling of how we disable AWB","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Fri, Feb 04, 2022 at 09:24:35AM +0000, David Plowman wrote:\n> We now handle disabling (\"pausing\") AWB in the same way as\n> AEC/AGC. Instead of letting the pause flag be set so that the code\n> never runs at all, we instead fix the manual settings to the current\n> values (but continue to be called).\n> \n> The algorithm does not restart any calculations in this state, but\n> continues to add AWB metadata to every frame. Therefore certain other\n> algorithms that want to know it (CCM and ALSC, for example) can still\n> find it.\n\nThat seems to be a good step forward.\n\nGiven that the only algorithms to ever be paused are AGC and AWB, it\nseems that we could remove the IsPaused() function as it will always\nreturn false (hardcoded for AGC and AWB, and because Pause() is never\ncalled for all the other algorithms). This can be done on top.\n\n> Signed-off-by: David Plowman \n> ---\n> src/ipa/raspberrypi/controller/rpi/awb.cpp | 21 +++++++++++++++++++++\n> src/ipa/raspberrypi/controller/rpi/awb.hpp | 4 ++++\n> 2 files changed, 25 insertions(+)\n> \n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> index 5cfd33a3..1ad912c7 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> @@ -172,6 +172,27 @@ void Awb::Initialise()\n> \tasync_results_ = sync_results_;\n> }\n> \n> +bool Awb::IsPaused() const\n> +{\n> +\treturn false;\n> +}\n> +\n> +void Awb::Pause()\n> +{\n> +\t// \"Pause\" by fixing everything to the most recent values.\n> +\tmanual_r_ = sync_results_.gain_r = prev_sync_results_.gain_r;\n> +\tmanual_b_ = sync_results_.gain_b = prev_sync_results_.gain_b;\n> +\tsync_results_.gain_g = prev_sync_results_.gain_g;\n> +\tsync_results_.gain_g = prev_sync_results_.gain_g;\n\nOnce should be enough :-)\n\n> +\tsync_results_.temperature_K = prev_sync_results_.temperature_K;\n> +}\n> +\n> +void Awb::Resume()\n> +{\n> +\tmanual_r_ = 0.0;\n> +\tmanual_b_ = 0.0;\n> +}\n> +\n> unsigned int Awb::GetConvergenceFrames() const\n> {\n> \t// If not in auto mode, there is no convergence\n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> index 8af1f27c..ac3dca6f 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> @@ -83,6 +83,10 @@ public:\n> \tchar const *Name() const override;\n> \tvoid Initialise() override;\n> \tvoid Read(boost::property_tree::ptree const ¶ms) override;\n> +\t// AWB handles \"pausing\" for itself.\n> +\tbool IsPaused() const override;\n> +\tvoid Pause() override;\n> +\tvoid Resume() override;\n> \tunsigned int GetConvergenceFrames() const override;\n> \tvoid SetMode(std::string const &name) override;\n> \tvoid SetManualGains(double manual_r, double manual_b) override;","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 91438BF415\n\tfor ;\n\tMon, 7 Feb 2022 23:23:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E429061081;\n\tTue, 8 Feb 2022 00:23:21 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0549609B9\n\tfor ;\n\tTue, 8 Feb 2022 00:23:19 +0100 (CET)","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 317D497;\n\tTue, 8 Feb 2022 00:23:19 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"eAmBfQQ9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1644276199;\n\tbh=r+cR15ewmz2x/wvcJtttEY/10MUMLA8aAsAtHMLmuU0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eAmBfQQ9Rbi0k7CeOkfxcJHl4ORauuAICAiShZeN7Wun406Csm7BrTI1yVk1sRypt\n\tYOYgOxPjOgDbPZwZ+hXI3W/ug6bJYSF2L1aRJXVzXInJtkXSNhTIceD+L5L9dYfP/G\n\tpzRKfhOM1aSnbsJtMXPvp/e7El4P+tx47WcxXJME=","Date":"Tue, 8 Feb 2022 01:23:17 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20220204092435.6343-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220204092435.6343-1-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: awb: Better\n\thandling of how we disable AWB","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":22137,"web_url":"https://patchwork.libcamera.org/comment/22137/","msgid":"","date":"2022-02-08T08:23:42","subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: awb: Better\n\thandling of how we disable AWB","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 patch.\n\nOn Fri, 4 Feb 2022 at 09:25, David Plowman \nwrote:\n\n> We now handle disabling (\"pausing\") AWB in the same way as\n> AEC/AGC. Instead of letting the pause flag be set so that the code\n> never runs at all, we instead fix the manual settings to the current\n> values (but continue to be called).\n>\n> The algorithm does not restart any calculations in this state, but\n> continues to add AWB metadata to every frame. Therefore certain other\n> algorithms that want to know it (CCM and ALSC, for example) can still\n> find it.\n>\n> Signed-off-by: David Plowman \n> ---\n> src/ipa/raspberrypi/controller/rpi/awb.cpp | 21 +++++++++++++++++++++\n> src/ipa/raspberrypi/controller/rpi/awb.hpp | 4 ++++\n> 2 files changed, 25 insertions(+)\n>\n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> index 5cfd33a3..1ad912c7 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> @@ -172,6 +172,27 @@ void Awb::Initialise()\n> async_results_ = sync_results_;\n> }\n>\n> +bool Awb::IsPaused() const\n> +{\n> + return false;\n> +}\n> +\n> +void Awb::Pause()\n> +{\n> + // \"Pause\" by fixing everything to the most recent values.\n> + manual_r_ = sync_results_.gain_r = prev_sync_results_.gain_r;\n> + manual_b_ = sync_results_.gain_b = prev_sync_results_.gain_b;\n> + sync_results_.gain_g = prev_sync_results_.gain_g;\n> + sync_results_.gain_g = prev_sync_results_.gain_g;\n>\n\nDuplicate line. Apart from that:\n\nReviewed-by: Naushir Patuck \n\n\n\n> + sync_results_.temperature_K = prev_sync_results_.temperature_K;\n> +}\n> +\n> +void Awb::Resume()\n> +{\n> + manual_r_ = 0.0;\n> + manual_b_ = 0.0;\n> +}\n> +\n> unsigned int Awb::GetConvergenceFrames() const\n> {\n> // If not in auto mode, there is no convergence\n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> index 8af1f27c..ac3dca6f 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> @@ -83,6 +83,10 @@ public:\n> char const *Name() const override;\n> void Initialise() override;\n> void Read(boost::property_tree::ptree const ¶ms) override;\n> + // AWB handles \"pausing\" for itself.\n> + bool IsPaused() const override;\n> + void Pause() override;\n> + void Resume() override;\n> unsigned int GetConvergenceFrames() const override;\n> void SetMode(std::string const &name) override;\n> void SetManualGains(double manual_r, double manual_b) override;\n> --\n> 2.30.2\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 F2EBCBF415\n\tfor ;\n\tTue, 8 Feb 2022 08:24:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 648D8610A9;\n\tTue, 8 Feb 2022 09:24:01 +0100 (CET)","from mail-lf1-x132.google.com (mail-lf1-x132.google.com\n\t[IPv6:2a00:1450:4864:20::132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BDFF60E55\n\tfor ;\n\tTue, 8 Feb 2022 09:23:59 +0100 (CET)","by mail-lf1-x132.google.com with SMTP id f18so2673080lfj.12\n\tfor ;\n\tTue, 08 Feb 2022 00:23:59 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"d3W0E4/m\"; 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=YRx4QJAlmr/Gkc1dLZc/eyuhvLnIYxGjD7n2qk3tuKQ=;\n\tb=d3W0E4/m3Hi91zcggeb6O4wtwLtQzLdlgupsFdPtAh8180qNy9TwYBdGtZzPjTFTv5\n\tDOSapSmf0YbQrEiMS5p1y8iq5gb9NxM+P9D6nHL9bf1gHImo/7YB9f49DaqmO5gdnfw4\n\tW9jstftC/V7ogYayqaxIfG1VDPcqTDEGvQEghLl4JhJtrp+Hhb2KnrLAgOO+rJnwapA7\n\tQoqNmxbec+lCP73G3FjDXrWECEhqt+snoEdKGXSi56/3/+H0ZBlIcVgna3f6ZRcmVvT4\n\tGB8DX5OMYPXOApeMI5xg98wmRN/1JMubFZWGCwCggsVwwJjXHRRZLybnLFcqloJYVMaw\n\tkiZA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=YRx4QJAlmr/Gkc1dLZc/eyuhvLnIYxGjD7n2qk3tuKQ=;\n\tb=aXaKdesKgsMGMD46TQPMO7ArSXybesyZ63rbdjteAhEzevhsb/67EEdCbSWPInU3tD\n\tGS/5N4vUPMUxKTkeyL/9yyRihvSxHvrT2WXNWQTbu228cNE5dXiIMBDOc5ZtOsnbNPfN\n\tSDlbM+G/9ft/VMll8sRU558GoPV9D6RulDOLgduCPSdEJO6JKjHzW2TZhc4BlLPoYvvV\n\tI9nQ+QAPhOPOIR9PvN5UUrcOnMTUg2SP73SLdHbsicWJ3rROHb7jPicUdukeLOGjnKbR\n\tADus15IqRbcR+3+i2LlehupZ/TDHkNcn8koH6W9k639P8UUPlv1bGzt0E83c2Aj1dvi1\n\tFPAA==","X-Gm-Message-State":"AOAM532wkQiBSFe90vEmGlAz7iXCilhjQI3ee+M7Vxd/R44LHdxLE1Jo\n\tpJk4RauUGEf7j9jhLHMOpgWRMOnpQccAda9GqY/9r9Rmjl8=","X-Google-Smtp-Source":"ABdhPJxSXcJ6eQcPh/TXbaBaiaoNapeu29jfp8sLjpqss8HYnYWruembWgrjBzpeGFxoesDIjJeeCgOcamZh7lTNjAc=","X-Received":"by 2002:a05:6512:31c4:: with SMTP id\n\tj4mr1711825lfe.315.1644308638720; \n\tTue, 08 Feb 2022 00:23:58 -0800 (PST)","MIME-Version":"1.0","References":"<20220204092435.6343-1-david.plowman@raspberrypi.com>","In-Reply-To":"<20220204092435.6343-1-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Tue, 8 Feb 2022 08:23:42 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"0000000000004ff3a605d77d70c1\"","Subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: awb: Better\n\thandling of how we disable AWB","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":22140,"web_url":"https://patchwork.libcamera.org/comment/22140/","msgid":"","date":"2022-02-08T09:18:42","subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: awb: Better\n\thandling of how we disable AWB","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent, Naush\n\nThanks for your reviews.\n\nOn Mon, 7 Feb 2022 at 23:23, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> Thank you for the patch.\n>\n> On Fri, Feb 04, 2022 at 09:24:35AM +0000, David Plowman wrote:\n> > We now handle disabling (\"pausing\") AWB in the same way as\n> > AEC/AGC. Instead of letting the pause flag be set so that the code\n> > never runs at all, we instead fix the manual settings to the current\n> > values (but continue to be called).\n> >\n> > The algorithm does not restart any calculations in this state, but\n> > continues to add AWB metadata to every frame. Therefore certain other\n> > algorithms that want to know it (CCM and ALSC, for example) can still\n> > find it.\n>\n> That seems to be a good step forward.\n>\n> Given that the only algorithms to ever be paused are AGC and AWB, it\n> seems that we could remove the IsPaused() function as it will always\n> return false (hardcoded for AGC and AWB, and because Pause() is never\n> called for all the other algorithms). This can be done on top.\n\nThat's an interesting thought, though I'm not quite sure about that\nyet. For example, there are lots of other control algorithms that you\ncould legitimately pause, perhaps ALSC being the most obvious. If you\nwanted absolutely to *guarantee* identical processing of all frames\nyou'd have to pause that too. Same goes for the \"contrast\" algorithm,\nwhich is liable to make adaptive changes to the gamma curve. Both\nthese algos would work fine with the \"default\" pause implementation.\n\nNow we don't have libcamera controls for doing any of that, so maybe\nthat's another one to think about!\n\n>\n> > Signed-off-by: David Plowman \n> > ---\n> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 21 +++++++++++++++++++++\n> > src/ipa/raspberrypi/controller/rpi/awb.hpp | 4 ++++\n> > 2 files changed, 25 insertions(+)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > index 5cfd33a3..1ad912c7 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > @@ -172,6 +172,27 @@ void Awb::Initialise()\n> > async_results_ = sync_results_;\n> > }\n> >\n> > +bool Awb::IsPaused() const\n> > +{\n> > + return false;\n> > +}\n> > +\n> > +void Awb::Pause()\n> > +{\n> > + // \"Pause\" by fixing everything to the most recent values.\n> > + manual_r_ = sync_results_.gain_r = prev_sync_results_.gain_r;\n> > + manual_b_ = sync_results_.gain_b = prev_sync_results_.gain_b;\n> > + sync_results_.gain_g = prev_sync_results_.gain_g;\n> > + sync_results_.gain_g = prev_sync_results_.gain_g;\n>\n> Once should be enough :-)\n\nYears of programming has made me paranoid! I'll submit a v2...\n\nThanks\nDavid\n\n>\n> > + sync_results_.temperature_K = prev_sync_results_.temperature_K;\n> > +}\n> > +\n> > +void Awb::Resume()\n> > +{\n> > + manual_r_ = 0.0;\n> > + manual_b_ = 0.0;\n> > +}\n> > +\n> > unsigned int Awb::GetConvergenceFrames() const\n> > {\n> > // If not in auto mode, there is no convergence\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> > index 8af1f27c..ac3dca6f 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> > @@ -83,6 +83,10 @@ public:\n> > char const *Name() const override;\n> > void Initialise() override;\n> > void Read(boost::property_tree::ptree const ¶ms) override;\n> > + // AWB handles \"pausing\" for itself.\n> > + bool IsPaused() const override;\n> > + void Pause() override;\n> > + void Resume() override;\n> > unsigned int GetConvergenceFrames() const override;\n> > void SetMode(std::string const &name) override;\n> > void SetManualGains(double manual_r, double manual_b) override;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 24697BF415\n\tfor ;\n\tTue, 8 Feb 2022 09:18:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 59055610BF;\n\tTue, 8 Feb 2022 10:18:55 +0100 (CET)","from mail-wm1-x331.google.com (mail-wm1-x331.google.com\n\t[IPv6:2a00:1450:4864:20::331])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E2AF610AD\n\tfor ;\n\tTue, 8 Feb 2022 10:18:53 +0100 (CET)","by mail-wm1-x331.google.com with SMTP id\n\to1-20020a1c4d01000000b0034d95625e1fso900522wmh.4\n\tfor ;\n\tTue, 08 Feb 2022 01:18:53 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"XQEJV+Oy\"; 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=4jL6KDsTtcetef+ZLIGWRBkyWryQ/wDRPJJB5KY5RvQ=;\n\tb=XQEJV+OyIdFFsa5B1j7ulfaYTNc++BrgMXUOx6rZWJEIaUx1nXhwoJqE5MWnXe+hy9\n\tNxGbKdVLQTBwZIb7pA6sfg7wdPvtDDznfTYlkAHw/gqLgsuiqaR4GqlKm7C0yAuNqrTP\n\txHTxeVhgtg8QKqbraROhbNrCplordEEj55bM5R05Mg23nt40cP9cRR5ZZLppBKERulSZ\n\tt7QjFX+AI1eZLO8ESg0XfYueyi535Iz3ntI79AvHzRLGMkUP0i29Yg1hz18QmrNnB7S6\n\toPLssLoVkKQ4U0gSzgEycLfmTy+/tPb1gWgzukgplbn1VbGQrBDMiLcYt5Wby/vKS+fF\n\tchcA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=4jL6KDsTtcetef+ZLIGWRBkyWryQ/wDRPJJB5KY5RvQ=;\n\tb=0Bn1uqFSvZuTghQpwA/IbSSv8rNxQiR42W6bmcaFT0PHfYPLblldy6BKiF/6E4OT5L\n\t/AxBeRBVGGo0U9lUWcWzfL0x/XREFWbcRn7uPNoxO5XtsYGiG+ASW4BZV8Fg7EKLuBMS\n\tTqm55X9lO2vUG/OkOc2fpxFImGiDCRAHm1sqCaG7F6DeeOdFcEUUUxuwNZV/V0zGXEwL\n\t5nmreUy8Q7Jp+tWocLx4R5Vdj0AVyZLJfu8KiQcUrIYd6K+lzsHCSOuonzFlril3QL7s\n\tV7sJrr42JRPvXnS+T0EvCli3uETsQOO8BoaMTr+N0mp/vcPNwAzYKrBch+YhHTL+tG9l\n\tEhdQ==","X-Gm-Message-State":"AOAM5338KcxoIdNT+OfNK513RIFUtRCNnWZR5v0nQrECFQo0qmQLOlw0\n\tew2dt6rkvegCRTDeVIWx/2t+jGQCCLNLvVg1D6KU+w==","X-Google-Smtp-Source":"ABdhPJxQuE+98axXfJFFq9sFHWfHw9dzViLebHCeZzNoAhHdA1Mi3C5SUTiSTZHGuusZ7V+sZOB6VTiCp602yJ1jkq0=","X-Received":"by 2002:a05:600c:1ca0:: with SMTP id\n\tk32mr244690wms.171.1644311933589; \n\tTue, 08 Feb 2022 01:18:53 -0800 (PST)","MIME-Version":"1.0","References":"<20220204092435.6343-1-david.plowman@raspberrypi.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Tue, 8 Feb 2022 09:18:42 +0000","Message-ID":"","To":"Laurent Pinchart , \n\tNaushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: awb: Better\n\thandling of how we disable AWB","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":22142,"web_url":"https://patchwork.libcamera.org/comment/22142/","msgid":"","date":"2022-02-08T10:38:35","subject":"Re: [libcamera-devel] [PATCH] ipa: raspberrypi: awb: Better\n\thandling of how we disable AWB","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Tue, Feb 08, 2022 at 09:18:42AM +0000, David Plowman wrote:\n> On Mon, 7 Feb 2022 at 23:23, Laurent Pinchart wrote:\n> > On Fri, Feb 04, 2022 at 09:24:35AM +0000, David Plowman wrote:\n> > > We now handle disabling (\"pausing\") AWB in the same way as\n> > > AEC/AGC. Instead of letting the pause flag be set so that the code\n> > > never runs at all, we instead fix the manual settings to the current\n> > > values (but continue to be called).\n> > >\n> > > The algorithm does not restart any calculations in this state, but\n> > > continues to add AWB metadata to every frame. Therefore certain other\n> > > algorithms that want to know it (CCM and ALSC, for example) can still\n> > > find it.\n> >\n> > That seems to be a good step forward.\n> >\n> > Given that the only algorithms to ever be paused are AGC and AWB, it\n> > seems that we could remove the IsPaused() function as it will always\n> > return false (hardcoded for AGC and AWB, and because Pause() is never\n> > called for all the other algorithms). This can be done on top.\n> \n> That's an interesting thought, though I'm not quite sure about that\n> yet. For example, there are lots of other control algorithms that you\n> could legitimately pause, perhaps ALSC being the most obvious. If you\n> wanted absolutely to *guarantee* identical processing of all frames\n> you'd have to pause that too. Same goes for the \"contrast\" algorithm,\n> which is liable to make adaptive changes to the gamma curve. Both\n> these algos would work fine with the \"default\" pause implementation.\n> \n> Now we don't have libcamera controls for doing any of that, so maybe\n> that's another one to think about!\n\nThere may be a need at that point to also keep those algorithms\n\"running\" to produce metadata. I agree that we can consider this later\nthough.\n\n> > > Signed-off-by: David Plowman \n> > > ---\n> > > src/ipa/raspberrypi/controller/rpi/awb.cpp | 21 +++++++++++++++++++++\n> > > src/ipa/raspberrypi/controller/rpi/awb.hpp | 4 ++++\n> > > 2 files changed, 25 insertions(+)\n> > >\n> > > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > > index 5cfd33a3..1ad912c7 100644\n> > > --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > > +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > > @@ -172,6 +172,27 @@ void Awb::Initialise()\n> > > async_results_ = sync_results_;\n> > > }\n> > >\n> > > +bool Awb::IsPaused() const\n> > > +{\n> > > + return false;\n> > > +}\n> > > +\n> > > +void Awb::Pause()\n> > > +{\n> > > + // \"Pause\" by fixing everything to the most recent values.\n> > > + manual_r_ = sync_results_.gain_r = prev_sync_results_.gain_r;\n> > > + manual_b_ = sync_results_.gain_b = prev_sync_results_.gain_b;\n> > > + sync_results_.gain_g = prev_sync_results_.gain_g;\n> > > + sync_results_.gain_g = prev_sync_results_.gain_g;\n> >\n> > Once should be enough :-)\n> \n> Years of programming has made me paranoid! I'll submit a v2...\n> \n> > > + sync_results_.temperature_K = prev_sync_results_.temperature_K;\n> > > +}\n> > > +\n> > > +void Awb::Resume()\n> > > +{\n> > > + manual_r_ = 0.0;\n> > > + manual_b_ = 0.0;\n> > > +}\n> > > +\n> > > unsigned int Awb::GetConvergenceFrames() const\n> > > {\n> > > // If not in auto mode, there is no convergence\n> > > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> > > index 8af1f27c..ac3dca6f 100644\n> > > --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> > > +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp\n> > > @@ -83,6 +83,10 @@ public:\n> > > char const *Name() const override;\n> > > void Initialise() override;\n> > > void Read(boost::property_tree::ptree const ¶ms) override;\n> > > + // AWB handles \"pausing\" for itself.\n> > > + bool IsPaused() const override;\n> > > + void Pause() override;\n> > > + void Resume() override;\n> > > unsigned int GetConvergenceFrames() const override;\n> > > void SetMode(std::string const &name) override;\n> > > void SetManualGains(double manual_r, double manual_b) override;","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 8E88ABDCBF\n\tfor ;\n\tTue, 8 Feb 2022 10:38:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E5F41610BF;\n\tTue, 8 Feb 2022 11:38:40 +0100 (CET)","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 CB3EA610AD\n\tfor ;\n\tTue, 8 Feb 2022 11:38:38 +0100 (CET)","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 41ECD480;\n\tTue, 8 Feb 2022 11:38:38 +0100 (CET)"],"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=\"G3XHxNtr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1644316718;\n\tbh=oecgOpX3uAQp3SDyffMMRg5Ey5dX+3XlJNmZ7fY9yEA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=G3XHxNtriLBp6CaDWtNxzte7SWyDQGfZrdXQFX9DrZsffCKz6phTZFmndkFVv6Hqm\n\t1u2hc85IetefrPh25MgpKxW1WtLdbD3ywftRbXkOWhtMHxm8NvJLmhxdAG1Xp/vKTY\n\tnDXWLgYxfYv/Lgn8DvZBX+gB7qEFROclbDGyaAgA=","Date":"Tue, 8 Feb 2022 12:38:35 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20220204092435.6343-1-david.plowman@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] ipa: raspberrypi: awb: Better\n\thandling of how we disable AWB","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\" "}}]