[{"id":16158,"web_url":"https://patchwork.libcamera.org/comment/16158/","msgid":"<20210408160012.4vvvqgfic7wa5hsg@uno.localdomain>","date":"2021-04-08T16:00:12","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: raspberrypi: Update\n\tsensor's RED/BLUE balance controls when present","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Thu, Apr 08, 2021 at 02:36:34PM +0100, David Plowman wrote:\n> If the sensor exposes V4L2_CID_RED_BALANCE and V4L2_CID_BLUE_BALANCE\n> controls, assume it wants to be told the colour gains.\n>\n> We want these to be applied as soon as possible so we add a new\n\nCan you be patient with me and explain why these controls are special\nand qualify for a fast-tracked method ?\n\n> setSensorControls signal to the IPA which passes these back to the\n> pipeline handler without using the DelayedControls mechanism.\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  include/libcamera/ipa/raspberrypi.mojom            |  1 +\n>  src/ipa/raspberrypi/raspberrypi.cpp                | 13 +++++++++++++\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++++++\n>  3 files changed, 24 insertions(+)\n>\n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index f38c2261..ebaf0a04 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -117,6 +117,7 @@ interface IPARPiEventInterface {\n>  \tstatsMetadataComplete(uint32 bufferId, ControlList controls);\n>  \trunIsp(uint32 bufferId);\n>  \tembeddedComplete(uint32 bufferId);\n> +\tsetSensorControls(ControlList controls);\n>  \tsetIspControls(ControlList controls);\n>  \tsetDelayedControls(ControlList controls);\n>  };\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index dad6395f..f95896d2 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -1036,6 +1036,19 @@ void IPARPi::processStats(unsigned int bufferId)\n>\n>  \t\tsetDelayedControls.emit(ctrls);\n>  \t}\n> +\n> +\tstruct AwbStatus awbStatus;\n> +\tif (rpiMetadata_.Get(\"awb.status\", awbStatus) == 0 &&\n> +\t    sensorCtrls_.find(V4L2_CID_RED_BALANCE) != sensorCtrls_.end() &&\n> +\t    sensorCtrls_.find(V4L2_CID_BLUE_BALANCE) != sensorCtrls_.end()) {\n> +\t\tControlList ctrls(sensorCtrls_);\n> +\t\tctrls.set(V4L2_CID_RED_BALANCE,\n> +\t\t\t  static_cast<int32_t>(helper_->ColourGainCode(awbStatus.gain_r)));\n> +\t\tctrls.set(V4L2_CID_BLUE_BALANCE,\n> +\t\t\t  static_cast<int32_t>(helper_->ColourGainCode(awbStatus.gain_b)));\n\nWon't this trigger the assert(0) in the previous patch ?\n\nThanks\n  j\n\n> +\n> +\t\tsetSensorControls.emit(ctrls);\n> +\t}\n>  }\n>\n>  void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index f22e286e..8dae4912 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -152,6 +152,7 @@ public:\n>  \tvoid statsMetadataComplete(uint32_t bufferId, const ControlList &controls);\n>  \tvoid runIsp(uint32_t bufferId);\n>  \tvoid embeddedComplete(uint32_t bufferId);\n> +\tvoid setSensorControls(const ControlList &controls);\n>  \tvoid setIspControls(const ControlList &controls);\n>  \tvoid setDelayedControls(const ControlList &controls);\n>\n> @@ -1219,6 +1220,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n>  \tipa_->statsMetadataComplete.connect(this, &RPiCameraData::statsMetadataComplete);\n>  \tipa_->runIsp.connect(this, &RPiCameraData::runIsp);\n>  \tipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);\n> +\tipa_->setSensorControls.connect(this, &RPiCameraData::setSensorControls);\n>  \tipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n>  \tipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n>\n> @@ -1361,6 +1363,14 @@ void RPiCameraData::embeddedComplete(uint32_t bufferId)\n>  \thandleState();\n>  }\n>\n> +void RPiCameraData::setSensorControls(const ControlList &controls)\n> +{\n> +\tControlList ctrls = controls;\n> +\n> +\tunicam_[Unicam::Image].dev()->setControls(&ctrls);\n> +\thandleState();\n> +}\n> +\n>  void RPiCameraData::setIspControls(const ControlList &controls)\n>  {\n>  \tControlList ctrls = controls;\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 E8C37BD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  8 Apr 2021 15:59:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42AB26879E;\n\tThu,  8 Apr 2021 17:59:37 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F2F8D602CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  8 Apr 2021 17:59:35 +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 relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 8594F1BF216;\n\tThu,  8 Apr 2021 15:59:35 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 8 Apr 2021 18:00:12 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20210408160012.4vvvqgfic7wa5hsg@uno.localdomain>","References":"<20210408133634.16815-1-david.plowman@raspberrypi.com>\n\t<20210408133634.16815-4-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210408133634.16815-4-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: raspberrypi: Update\n\tsensor's RED/BLUE balance controls when present","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":16159,"web_url":"https://patchwork.libcamera.org/comment/16159/","msgid":"<CAHW6GYLgDv4bymGZM0HWmq=Wx7ehqUjQYnY3=dTZ-WTdb176Ew@mail.gmail.com>","date":"2021-04-08T17:12:45","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: raspberrypi: Update\n\tsensor's RED/BLUE balance controls when present","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks very much for the comments.\n\nOn Thu, 8 Apr 2021 at 16:59, Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n> Hi David,\n>\n> On Thu, Apr 08, 2021 at 02:36:34PM +0100, David Plowman wrote:\n> > If the sensor exposes V4L2_CID_RED_BALANCE and V4L2_CID_BLUE_BALANCE\n> > controls, assume it wants to be told the colour gains.\n> >\n> > We want these to be applied as soon as possible so we add a new\n>\n> Can you be patient with me and explain why these controls are special\n> and qualify for a fast-tracked method ?\n\nYes of course! So I'm looking at some sensors that do some \"clever\"\nprocessing on the sensor itself, for which they need to know what the\nred and blue gains are. Some examples:\n\n* A \"quad Bayer\" or \"tetracell\" sensor may have the ability to output\nan HDR Bayer pattern. In order to combine all the different exposures\nit needs to have the colour gains.\n* The same sensor may have the ability to turn the \"quad\" pattern of\npixels into a standard full resolution Bayer pattern. This needs the\ncolour gains too.\n* Sensors with non-standard patterns often support conversion to\nregular Bayer - which can work better with the colour gains.\n\nIn all these cases it's good to get the latest colour gains to the\nsensor as fast as we can, to limit the artefacts produced if the white\nbalance is changing. Note that we never need to try and read them\nback, as we do with exposure and analogue gain.\n\nThe first version of this patch used the delayed control mechanism. I\nthink that was OK too, but it felt like a slight abuse of that\nmechanism - we don't need the complexity of trying to align it with\nother sensor updates. And it was confusing - I gave it the same delay\nas the vblank control so that it would get written immediately, yet it\nactually happens on the very next frame which would normally be a\ndelay of 1, so it all seemed a bit strange...\n\n>\n> > setSensorControls signal to the IPA which passes these back to the\n> > pipeline handler without using the DelayedControls mechanism.\n> >\n> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  include/libcamera/ipa/raspberrypi.mojom            |  1 +\n> >  src/ipa/raspberrypi/raspberrypi.cpp                | 13 +++++++++++++\n> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++++++\n> >  3 files changed, 24 insertions(+)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> > index f38c2261..ebaf0a04 100644\n> > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > @@ -117,6 +117,7 @@ interface IPARPiEventInterface {\n> >       statsMetadataComplete(uint32 bufferId, ControlList controls);\n> >       runIsp(uint32 bufferId);\n> >       embeddedComplete(uint32 bufferId);\n> > +     setSensorControls(ControlList controls);\n> >       setIspControls(ControlList controls);\n> >       setDelayedControls(ControlList controls);\n> >  };\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index dad6395f..f95896d2 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -1036,6 +1036,19 @@ void IPARPi::processStats(unsigned int bufferId)\n> >\n> >               setDelayedControls.emit(ctrls);\n> >       }\n> > +\n> > +     struct AwbStatus awbStatus;\n> > +     if (rpiMetadata_.Get(\"awb.status\", awbStatus) == 0 &&\n> > +         sensorCtrls_.find(V4L2_CID_RED_BALANCE) != sensorCtrls_.end() &&\n> > +         sensorCtrls_.find(V4L2_CID_BLUE_BALANCE) != sensorCtrls_.end()) {\n> > +             ControlList ctrls(sensorCtrls_);\n> > +             ctrls.set(V4L2_CID_RED_BALANCE,\n> > +                       static_cast<int32_t>(helper_->ColourGainCode(awbStatus.gain_r)));\n> > +             ctrls.set(V4L2_CID_BLUE_BALANCE,\n> > +                       static_cast<int32_t>(helper_->ColourGainCode(awbStatus.gain_b)));\n>\n> Won't this trigger the assert(0) in the previous patch ?\n\nHopefully not - the idea is that you must override the default method\nif your sensor wants the RED/BLUE_BALANCE numbers. For sensors that\ndon't need them, everything is fine as it is.\n\nI hope that all makes sense?\n\nThanks!\nDavid\n\n>\n> Thanks\n>   j\n>\n> > +\n> > +             setSensorControls.emit(ctrls);\n> > +     }\n> >  }\n> >\n> >  void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index f22e286e..8dae4912 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -152,6 +152,7 @@ public:\n> >       void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);\n> >       void runIsp(uint32_t bufferId);\n> >       void embeddedComplete(uint32_t bufferId);\n> > +     void setSensorControls(const ControlList &controls);\n> >       void setIspControls(const ControlList &controls);\n> >       void setDelayedControls(const ControlList &controls);\n> >\n> > @@ -1219,6 +1220,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n> >       ipa_->statsMetadataComplete.connect(this, &RPiCameraData::statsMetadataComplete);\n> >       ipa_->runIsp.connect(this, &RPiCameraData::runIsp);\n> >       ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);\n> > +     ipa_->setSensorControls.connect(this, &RPiCameraData::setSensorControls);\n> >       ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n> >       ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n> >\n> > @@ -1361,6 +1363,14 @@ void RPiCameraData::embeddedComplete(uint32_t bufferId)\n> >       handleState();\n> >  }\n> >\n> > +void RPiCameraData::setSensorControls(const ControlList &controls)\n> > +{\n> > +     ControlList ctrls = controls;\n> > +\n> > +     unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> > +     handleState();\n> > +}\n> > +\n> >  void RPiCameraData::setIspControls(const ControlList &controls)\n> >  {\n> >       ControlList ctrls = controls;\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 DAA3EBD1F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  8 Apr 2021 17:13:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1CD4B6879E;\n\tThu,  8 Apr 2021 19:13:00 +0200 (CEST)","from mail-oo1-xc34.google.com (mail-oo1-xc34.google.com\n\t[IPv6:2607:f8b0:4864:20::c34])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D964F602CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  8 Apr 2021 19:12:58 +0200 (CEST)","by mail-oo1-xc34.google.com with SMTP id\n\tc12-20020a4ae24c0000b02901bad05f40e4so676451oot.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 08 Apr 2021 10:12:58 -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=\"AYsu9Q0c\"; 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=Kksu4MY6PyfvAdjG6QBcNr0EgHntffgr6kvNKynBz98=;\n\tb=AYsu9Q0cQ+bzs7CNW7LmGpWC11tFefW5d8cDgZ/Bulc8oZNH6Jg9Fhq5C6vZ+e8Nqa\n\t141uqwveesQsRS+yL5SysWgx32p+A3upHme2yK/+k8K6KiqK8LU37QUiUxZD2UQlhB6u\n\tv3oQjCfeXQteBqyDrhT5Hi7HWSTwNlbqGrpmBcATVklVJTQ/hhmu/NK9AqFM19edsui+\n\tyMcy1mSBkkF6g5kOjJNnuTKt9khzcQOJbxoMKZ2LfDUEEHQDn+8qCkMTayPOuCfwbqy9\n\ty+XJPBWz3YgIpU2CpfaQuWkmUGoZ0CDD9urKZNO28bOEZ69L0q5gvwdoaawt9QqfJcXG\n\tkHmg==","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=Kksu4MY6PyfvAdjG6QBcNr0EgHntffgr6kvNKynBz98=;\n\tb=iP6BrPXcMkBStKcCJdBd//J1QepKl7PAeJFeGeDQ29pStyGnn32W4RDQwMAohFIu4S\n\tpER9QNFmrNuVh2gkzFx70EHSmKJd0CIGgT5MKzW2mvvYQb1IkgaBvZRuMyR83Njswv+E\n\tkQahv8R7J1V0SjQwmFRPGfgWuaxk7wLKxrixoZL17AxBamNW/mDWik3gLVnAzGHuvdA7\n\tzAO4I355QZneJ4oZVjBTaFTF5fuPEcctyZdmWWayVO9akfoG1piUe2+ErZaYIxsWln/3\n\tqg5svJ83EzO205GGJgSr/XvWEWa4r2U/s2uvqohhtZKUVnk0ZV7jQbIwYk3fjx7UABwL\n\tvkCA==","X-Gm-Message-State":"AOAM531z3OUA9y9BfGn0TWT6uxmm47pYeIe+O3eNiWxgaPs6BCLA3LHs\n\tOCmR/3rjNSfCUyNkbHPcukUd/LtciyxxxamzgxN7WXJfptQ=","X-Google-Smtp-Source":"ABdhPJyAKnNZYvhnbAdg573CKqm/P+V0c9D3lqIUu41tqARmP/xIYumgDwI517ue2xpCg54NiJw510xZTwuxh77sv8U=","X-Received":"by 2002:a4a:7615:: with SMTP id t21mr8319264ooc.72.1617901977151;\n\tThu, 08 Apr 2021 10:12:57 -0700 (PDT)","MIME-Version":"1.0","References":"<20210408133634.16815-1-david.plowman@raspberrypi.com>\n\t<20210408133634.16815-4-david.plowman@raspberrypi.com>\n\t<20210408160012.4vvvqgfic7wa5hsg@uno.localdomain>","In-Reply-To":"<20210408160012.4vvvqgfic7wa5hsg@uno.localdomain>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Thu, 8 Apr 2021 18:12:45 +0100","Message-ID":"<CAHW6GYLgDv4bymGZM0HWmq=Wx7ehqUjQYnY3=dTZ-WTdb176Ew@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: raspberrypi: Update\n\tsensor's RED/BLUE balance controls when present","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 <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":16267,"web_url":"https://patchwork.libcamera.org/comment/16267/","msgid":"<7668ba45-8c4b-99f5-933d-abb216020b30@ideasonboard.com>","date":"2021-04-14T08:39:32","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: raspberrypi: Update\n\tsensor's RED/BLUE balance controls when present","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi David,\n\nOn 08/04/2021 18:12, David Plowman wrote:\n> Hi Jacopo\n> \n> Thanks very much for the comments.\n> \n> On Thu, 8 Apr 2021 at 16:59, Jacopo Mondi <jacopo@jmondi.org> wrote:\n>>\n>> Hi David,\n>>\n>> On Thu, Apr 08, 2021 at 02:36:34PM +0100, David Plowman wrote:\n>>> If the sensor exposes V4L2_CID_RED_BALANCE and V4L2_CID_BLUE_BALANCE\n>>> controls, assume it wants to be told the colour gains.\n>>>\n>>> We want these to be applied as soon as possible so we add a new\n>>\n>> Can you be patient with me and explain why these controls are special\n>> and qualify for a fast-tracked method ?\n> \n> Yes of course! So I'm looking at some sensors that do some \"clever\"\n> processing on the sensor itself, for which they need to know what the\n> red and blue gains are. Some examples:\n> \n> * A \"quad Bayer\" or \"tetracell\" sensor may have the ability to output\n> an HDR Bayer pattern. In order to combine all the different exposures\n> it needs to have the colour gains.\n> * The same sensor may have the ability to turn the \"quad\" pattern of\n> pixels into a standard full resolution Bayer pattern. This needs the\n> colour gains too.\n> * Sensors with non-standard patterns often support conversion to\n> regular Bayer - which can work better with the colour gains.\n> \n> In all these cases it's good to get the latest colour gains to the\n> sensor as fast as we can, to limit the artefacts produced if the white\n> balance is changing. Note that we never need to try and read them\n> back, as we do with exposure and analogue gain.\n> \n> The first version of this patch used the delayed control mechanism. I\n> think that was OK too, but it felt like a slight abuse of that\n> mechanism - we don't need the complexity of trying to align it with\n> other sensor updates. And it was confusing - I gave it the same delay\n> as the vblank control so that it would get written immediately, yet it\n> actually happens on the very next frame which would normally be a\n> delay of 1, so it all seemed a bit strange...\n> \n>>\n>>> setSensorControls signal to the IPA which passes these back to the\n>>> pipeline handler without using the DelayedControls mechanism.\n>>>\n>>> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n>>> ---\n>>>  include/libcamera/ipa/raspberrypi.mojom            |  1 +\n>>>  src/ipa/raspberrypi/raspberrypi.cpp                | 13 +++++++++++++\n>>>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++++++\n>>>  3 files changed, 24 insertions(+)\n>>>\n>>> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n>>> index f38c2261..ebaf0a04 100644\n>>> --- a/include/libcamera/ipa/raspberrypi.mojom\n>>> +++ b/include/libcamera/ipa/raspberrypi.mojom\n>>> @@ -117,6 +117,7 @@ interface IPARPiEventInterface {\n>>>       statsMetadataComplete(uint32 bufferId, ControlList controls);\n>>>       runIsp(uint32 bufferId);\n>>>       embeddedComplete(uint32 bufferId);\n>>> +     setSensorControls(ControlList controls);\n>>>       setIspControls(ControlList controls);\n>>>       setDelayedControls(ControlList controls);\n>>>  };\n>>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n>>> index dad6395f..f95896d2 100644\n>>> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>>> @@ -1036,6 +1036,19 @@ void IPARPi::processStats(unsigned int bufferId)\n>>>\n>>>               setDelayedControls.emit(ctrls);\n>>>       }\n>>> +\n>>> +     struct AwbStatus awbStatus;\n>>> +     if (rpiMetadata_.Get(\"awb.status\", awbStatus) == 0 &&\n>>> +         sensorCtrls_.find(V4L2_CID_RED_BALANCE) != sensorCtrls_.end() &&\n>>> +         sensorCtrls_.find(V4L2_CID_BLUE_BALANCE) != sensorCtrls_.end()) {\n>>> +             ControlList ctrls(sensorCtrls_);\n>>> +             ctrls.set(V4L2_CID_RED_BALANCE,\n>>> +                       static_cast<int32_t>(helper_->ColourGainCode(awbStatus.gain_r)));\n>>> +             ctrls.set(V4L2_CID_BLUE_BALANCE,\n>>> +                       static_cast<int32_t>(helper_->ColourGainCode(awbStatus.gain_b)));\n>>\n>> Won't this trigger the assert(0) in the previous patch ?\n> \n> Hopefully not - the idea is that you must override the default method\n> if your sensor wants the RED/BLUE_BALANCE numbers. For sensors that\n> don't need them, everything is fine as it is.\n\nYes, I can see that the ColourGainCode is only called if the sensor has\na control for boht RED and BLUE balance.\n\nAnd if that control is available on the sensor, then the CamHelper\n*must* override the ColourGainCode().\n\nI think that makes me more convinced that the assertion in the previous\npatch should be a more verbose message explaining what must be done if\nit is hit, as it could be hit by a user just changing to a new (not yet\nsupported) sensor... but otherwise it seems fine.\n\n\nI'm a bit weary of having two routes to set controls on the sensor\nthrough setSensorControls() and through setDelayedControls(), but given\nthe namings of both show that one is 'delayed', I think the reasoning is\nclear ...\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> \n> I hope that all makes sense?\n> \n> Thanks!\n> David\n> \n>>\n>> Thanks\n>>   j\n>>\n>>> +\n>>> +             setSensorControls.emit(ctrls);\n>>> +     }\n>>>  }\n>>>\n>>>  void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n>>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> index f22e286e..8dae4912 100644\n>>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> @@ -152,6 +152,7 @@ public:\n>>>       void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);\n>>>       void runIsp(uint32_t bufferId);\n>>>       void embeddedComplete(uint32_t bufferId);\n>>> +     void setSensorControls(const ControlList &controls);\n>>>       void setIspControls(const ControlList &controls);\n>>>       void setDelayedControls(const ControlList &controls);\n>>>\n>>> @@ -1219,6 +1220,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n>>>       ipa_->statsMetadataComplete.connect(this, &RPiCameraData::statsMetadataComplete);\n>>>       ipa_->runIsp.connect(this, &RPiCameraData::runIsp);\n>>>       ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);\n>>> +     ipa_->setSensorControls.connect(this, &RPiCameraData::setSensorControls);\n>>>       ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n>>>       ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n>>>\n>>> @@ -1361,6 +1363,14 @@ void RPiCameraData::embeddedComplete(uint32_t bufferId)\n>>>       handleState();\n>>>  }\n>>>\n>>> +void RPiCameraData::setSensorControls(const ControlList &controls)\n>>> +{\n>>> +     ControlList ctrls = controls;\n>>> +\n>>> +     unicam_[Unicam::Image].dev()->setControls(&ctrls);\n>>> +     handleState();\n>>> +}\n>>> +\n>>>  void RPiCameraData::setIspControls(const ControlList &controls)\n>>>  {\n>>>       ControlList ctrls = controls;\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\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 5F20CBD224\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 14 Apr 2021 08:39:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D384B68809;\n\tWed, 14 Apr 2021 10:39:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E0B86687EF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 14 Apr 2021 10:39:35 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B46456F2;\n\tWed, 14 Apr 2021 10:39:34 +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=\"RtL3oi7E\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1618389575;\n\tbh=3+YpZdtEp47H3UTVcguBCdXZLAABV2BqXyWdEdgv4AE=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=RtL3oi7EcpBwqoGmPPXCHKglIZ5/2up0j4jxmfuGIYrh8OjUgZJbD4OSrtx4P+0Gb\n\ttPOeNyXcz8FBfQmEIbtltYMMPcbpBtmEYpfUBwjk7MVqYnxYYI8eRjIxEBI4UdsO9J\n\tYRhR0sPYXBxAn8uDXDdC3s7jV/Q4D4pBqs+Ab1jY=","To":"David Plowman <david.plowman@raspberrypi.com>,\n\tJacopo Mondi <jacopo@jmondi.org>","References":"<20210408133634.16815-1-david.plowman@raspberrypi.com>\n\t<20210408133634.16815-4-david.plowman@raspberrypi.com>\n\t<20210408160012.4vvvqgfic7wa5hsg@uno.localdomain>\n\t<CAHW6GYLgDv4bymGZM0HWmq=Wx7ehqUjQYnY3=dTZ-WTdb176Ew@mail.gmail.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<7668ba45-8c4b-99f5-933d-abb216020b30@ideasonboard.com>","Date":"Wed, 14 Apr 2021 09:39:32 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<CAHW6GYLgDv4bymGZM0HWmq=Wx7ehqUjQYnY3=dTZ-WTdb176Ew@mail.gmail.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: raspberrypi: Update\n\tsensor's RED/BLUE balance controls when present","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>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel <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":16268,"web_url":"https://patchwork.libcamera.org/comment/16268/","msgid":"<CAHW6GYLXduDJLA6_GAobuxjFzgOWDXcyrcSwiVjnv0Goj3sDhw@mail.gmail.com>","date":"2021-04-14T08:44:37","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: raspberrypi: Update\n\tsensor's RED/BLUE balance controls when present","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"HI Kieran\n\nThanks for the review!\n\nOn Wed, 14 Apr 2021 at 09:39, Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Hi David,\n>\n> On 08/04/2021 18:12, David Plowman wrote:\n> > Hi Jacopo\n> >\n> > Thanks very much for the comments.\n> >\n> > On Thu, 8 Apr 2021 at 16:59, Jacopo Mondi <jacopo@jmondi.org> wrote:\n> >>\n> >> Hi David,\n> >>\n> >> On Thu, Apr 08, 2021 at 02:36:34PM +0100, David Plowman wrote:\n> >>> If the sensor exposes V4L2_CID_RED_BALANCE and V4L2_CID_BLUE_BALANCE\n> >>> controls, assume it wants to be told the colour gains.\n> >>>\n> >>> We want these to be applied as soon as possible so we add a new\n> >>\n> >> Can you be patient with me and explain why these controls are special\n> >> and qualify for a fast-tracked method ?\n> >\n> > Yes of course! So I'm looking at some sensors that do some \"clever\"\n> > processing on the sensor itself, for which they need to know what the\n> > red and blue gains are. Some examples:\n> >\n> > * A \"quad Bayer\" or \"tetracell\" sensor may have the ability to output\n> > an HDR Bayer pattern. In order to combine all the different exposures\n> > it needs to have the colour gains.\n> > * The same sensor may have the ability to turn the \"quad\" pattern of\n> > pixels into a standard full resolution Bayer pattern. This needs the\n> > colour gains too.\n> > * Sensors with non-standard patterns often support conversion to\n> > regular Bayer - which can work better with the colour gains.\n> >\n> > In all these cases it's good to get the latest colour gains to the\n> > sensor as fast as we can, to limit the artefacts produced if the white\n> > balance is changing. Note that we never need to try and read them\n> > back, as we do with exposure and analogue gain.\n> >\n> > The first version of this patch used the delayed control mechanism. I\n> > think that was OK too, but it felt like a slight abuse of that\n> > mechanism - we don't need the complexity of trying to align it with\n> > other sensor updates. And it was confusing - I gave it the same delay\n> > as the vblank control so that it would get written immediately, yet it\n> > actually happens on the very next frame which would normally be a\n> > delay of 1, so it all seemed a bit strange...\n> >\n> >>\n> >>> setSensorControls signal to the IPA which passes these back to the\n> >>> pipeline handler without using the DelayedControls mechanism.\n> >>>\n> >>> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> >>> ---\n> >>>  include/libcamera/ipa/raspberrypi.mojom            |  1 +\n> >>>  src/ipa/raspberrypi/raspberrypi.cpp                | 13 +++++++++++++\n> >>>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 ++++++++++\n> >>>  3 files changed, 24 insertions(+)\n> >>>\n> >>> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> >>> index f38c2261..ebaf0a04 100644\n> >>> --- a/include/libcamera/ipa/raspberrypi.mojom\n> >>> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> >>> @@ -117,6 +117,7 @@ interface IPARPiEventInterface {\n> >>>       statsMetadataComplete(uint32 bufferId, ControlList controls);\n> >>>       runIsp(uint32 bufferId);\n> >>>       embeddedComplete(uint32 bufferId);\n> >>> +     setSensorControls(ControlList controls);\n> >>>       setIspControls(ControlList controls);\n> >>>       setDelayedControls(ControlList controls);\n> >>>  };\n> >>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> >>> index dad6395f..f95896d2 100644\n> >>> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> >>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> >>> @@ -1036,6 +1036,19 @@ void IPARPi::processStats(unsigned int bufferId)\n> >>>\n> >>>               setDelayedControls.emit(ctrls);\n> >>>       }\n> >>> +\n> >>> +     struct AwbStatus awbStatus;\n> >>> +     if (rpiMetadata_.Get(\"awb.status\", awbStatus) == 0 &&\n> >>> +         sensorCtrls_.find(V4L2_CID_RED_BALANCE) != sensorCtrls_.end() &&\n> >>> +         sensorCtrls_.find(V4L2_CID_BLUE_BALANCE) != sensorCtrls_.end()) {\n> >>> +             ControlList ctrls(sensorCtrls_);\n> >>> +             ctrls.set(V4L2_CID_RED_BALANCE,\n> >>> +                       static_cast<int32_t>(helper_->ColourGainCode(awbStatus.gain_r)));\n> >>> +             ctrls.set(V4L2_CID_BLUE_BALANCE,\n> >>> +                       static_cast<int32_t>(helper_->ColourGainCode(awbStatus.gain_b)));\n> >>\n> >> Won't this trigger the assert(0) in the previous patch ?\n> >\n> > Hopefully not - the idea is that you must override the default method\n> > if your sensor wants the RED/BLUE_BALANCE numbers. For sensors that\n> > don't need them, everything is fine as it is.\n>\n> Yes, I can see that the ColourGainCode is only called if the sensor has\n> a control for boht RED and BLUE balance.\n>\n> And if that control is available on the sensor, then the CamHelper\n> *must* override the ColourGainCode().\n>\n> I think that makes me more convinced that the assertion in the previous\n> patch should be a more verbose message explaining what must be done if\n> it is hit, as it could be hit by a user just changing to a new (not yet\n> supported) sensor... but otherwise it seems fine.\n\nYes, I think that seems reasonable to me too. I'll update the patch shortly.\n\nBest regards\nDavid\n\n>\n>\n> I'm a bit weary of having two routes to set controls on the sensor\n> through setSensorControls() and through setDelayedControls(), but given\n> the namings of both show that one is 'delayed', I think the reasoning is\n> clear ...\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>\n>\n> >\n> > I hope that all makes sense?\n> >\n> > Thanks!\n> > David\n> >\n> >>\n> >> Thanks\n> >>   j\n> >>\n> >>> +\n> >>> +             setSensorControls.emit(ctrls);\n> >>> +     }\n> >>>  }\n> >>>\n> >>>  void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> index f22e286e..8dae4912 100644\n> >>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> @@ -152,6 +152,7 @@ public:\n> >>>       void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);\n> >>>       void runIsp(uint32_t bufferId);\n> >>>       void embeddedComplete(uint32_t bufferId);\n> >>> +     void setSensorControls(const ControlList &controls);\n> >>>       void setIspControls(const ControlList &controls);\n> >>>       void setDelayedControls(const ControlList &controls);\n> >>>\n> >>> @@ -1219,6 +1220,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n> >>>       ipa_->statsMetadataComplete.connect(this, &RPiCameraData::statsMetadataComplete);\n> >>>       ipa_->runIsp.connect(this, &RPiCameraData::runIsp);\n> >>>       ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);\n> >>> +     ipa_->setSensorControls.connect(this, &RPiCameraData::setSensorControls);\n> >>>       ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n> >>>       ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n> >>>\n> >>> @@ -1361,6 +1363,14 @@ void RPiCameraData::embeddedComplete(uint32_t bufferId)\n> >>>       handleState();\n> >>>  }\n> >>>\n> >>> +void RPiCameraData::setSensorControls(const ControlList &controls)\n> >>> +{\n> >>> +     ControlList ctrls = controls;\n> >>> +\n> >>> +     unicam_[Unicam::Image].dev()->setControls(&ctrls);\n> >>> +     handleState();\n> >>> +}\n> >>> +\n> >>>  void RPiCameraData::setIspControls(const ControlList &controls)\n> >>>  {\n> >>>       ControlList ctrls = controls;\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\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 ADDEFBD1F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 14 Apr 2021 08:44:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3038768806;\n\tWed, 14 Apr 2021 10:44:51 +0200 (CEST)","from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com\n\t[IPv6:2607:f8b0:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A30ED687EF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 14 Apr 2021 10:44:49 +0200 (CEST)","by mail-oi1-x22f.google.com with SMTP id u16so2539376oiu.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 14 Apr 2021 01:44:49 -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=\"oFWr8MJB\"; 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=o44+8+xUxWkzpgrcw0qEv0Wyy3yCRWLADq4DKZCrHdo=;\n\tb=oFWr8MJB8Ebr+MbWpwxSZbhMP88fKHxDHGfqMj4sI/AiIulXtmtJbQZre09VnfJVqH\n\t8coerl+D1z6fIR5vOEpYXrYJKHbC5+NsAaIVvSyU1f00wP/aMR8auDwUnimeusfKoEso\n\t8Y0ae82F66chhqqXydYyQP+uuRYls5P0zxetJR/KTZNBD0EqKI/K39RbmebfwjIgRJUK\n\t/seL/cA4NOwbX+66vFhDjNKbaGmDKydsw8OhH+D4F8cboL2OHe30Ynvh/vyN63TZCmqd\n\tIGH6HsQkIEBl7hqhAqKOmtwV53u92yRwoxlvX6YdlvnyP/qEGSaWaI679g1oN2WgxASU\n\tf+4w==","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=o44+8+xUxWkzpgrcw0qEv0Wyy3yCRWLADq4DKZCrHdo=;\n\tb=tLzdhWp+4LV7t1RU0fN5htRj5/mNrCypMNp/CueDLuuAkjNZlWeM/v41kKKQd00CG+\n\t8MCgmLeU4MfGcOwwbotTRvpE0EZ9HO4xSYG6c9BHfCpD+YolpVnDCYxYWLw5ppbAqWys\n\tiU6WZHb48TdTm/E9Pa5JlII9XUtIDitMGls/J/fHNESfkSiThj17YC2EZogUHSaNbqzj\n\tSOGasSeHM9Sw3zAtFescjEs0OcHw7mwFDJkGBrHs2qG0PVMgFCc5bXHnTllMefAbFEW6\n\tKYGpgvLOfJIInkOpzOubMSKpBAEAh78Vci19lI3+jtzptJ29pJKX4G6wvhrZhoONCwaz\n\tDr6w==","X-Gm-Message-State":"AOAM531IfR/8+HTGXTB96jNk7YiLQ+EY8Bpp3zPExUjn/WmKTb76JUg3\n\tAnoJCrV3IjYmmaZSk/PR1u6JkZPoaVK5/1srCuGmxQ==","X-Google-Smtp-Source":"ABdhPJxaajXBA0aepKGOoVFaaYW+02f8qFk9N18Vvyc3lWeMrxtGxoVFu9+zUo4hWhBMzOwoqpYd6PXX1V6LF+E/tA4=","X-Received":"by 2002:aca:4d0e:: with SMTP id\n\ta14mr1546817oib.107.1618389888394; \n\tWed, 14 Apr 2021 01:44:48 -0700 (PDT)","MIME-Version":"1.0","References":"<20210408133634.16815-1-david.plowman@raspberrypi.com>\n\t<20210408133634.16815-4-david.plowman@raspberrypi.com>\n\t<20210408160012.4vvvqgfic7wa5hsg@uno.localdomain>\n\t<CAHW6GYLgDv4bymGZM0HWmq=Wx7ehqUjQYnY3=dTZ-WTdb176Ew@mail.gmail.com>\n\t<7668ba45-8c4b-99f5-933d-abb216020b30@ideasonboard.com>","In-Reply-To":"<7668ba45-8c4b-99f5-933d-abb216020b30@ideasonboard.com>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Wed, 14 Apr 2021 09:44:37 +0100","Message-ID":"<CAHW6GYLXduDJLA6_GAobuxjFzgOWDXcyrcSwiVjnv0Goj3sDhw@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: raspberrypi: Update\n\tsensor's RED/BLUE balance controls when present","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 <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>"}}]