[{"id":16144,"web_url":"https://patchwork.libcamera.org/comment/16144/","msgid":"","date":"2021-04-07T10:32:31","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: raspberrypi: Update sensor's\n\tRED/BLUE balance controls when present","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 Wed, 24 Mar 2021 at 11:44, David Plowman \nwrote:\n\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. We will need to\n> add these to the list of delayed controls we can apply.\n>\n> Signed-off-by: David Plowman \n> ---\n> src/ipa/raspberrypi/raspberrypi.cpp | 17 ++++++++++++++---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 9 +++++++++\n> 2 files changed, 23 insertions(+), 3 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 1c928b72..7437a77e 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -1030,13 +1030,24 @@ void IPARPi::processStats(unsigned int bufferId)\n> RPiController::StatisticsPtr statistics =\n> std::make_shared(*stats);\n> controller_.Process(statistics, &rpiMetadata_);\n>\n> + ControlList ctrls(sensorCtrls_);\n> +\n> struct AgcStatus agcStatus;\n> - if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0) {\n> - ControlList ctrls(sensorCtrls_);\n> + if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0)\n> applyAGC(&agcStatus, ctrls);\n>\n> - setDelayedControls.emit(ctrls);\n> + struct AwbStatus awbStatus;\n> + if (rpiMetadata_.Get(\"awb.status\", awbStatus) == 0 &&\n> + sensorCtrls_.find(V4L2_CID_RED_BALANCE) != sensorCtrls_.end()\n> &&\n> + sensorCtrls_.find(V4L2_CID_BLUE_BALANCE) !=\n> sensorCtrls_.end()) {\n> + ctrls.set(V4L2_CID_RED_BALANCE,\n> + static_cast(awbStatus.gain_r * 256));\n> + ctrls.set(V4L2_CID_BLUE_BALANCE,\n> + static_cast(awbStatus.gain_b * 256));\n>\n\nThe 256 const bothers me slightly. Is this FP multiplier going to be true\nfor all sensors?\nAny way we can deduce this (perhaps from the min value)?\n\n\n> }\n> +\n> + if (!ctrls.empty())\n> + setDelayedControls.emit(ctrls);\n> }\n>\n> void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList\n> &ctrls)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 2cac802c..7bac1503 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1027,6 +1027,15 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> { V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false }\n> },\n> { V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }\n> };\n> +\n> + /* If the sensor exposes red/blue balance controls, we will update\n> them. */\n> + const ControlInfoMap &sensorControls =\n> data->unicam_[Unicam::Image].dev()->controls();\n> + if (sensorControls.find(V4L2_CID_RED_BALANCE) !=\n> sensorControls.end() &&\n> + sensorControls.find(V4L2_CID_BLUE_BALANCE) !=\n> sensorControls.end()) {\n> + params[V4L2_CID_RED_BALANCE] = { sensorConfig.vblankDelay,\n> false };\n> + params[V4L2_CID_BLUE_BALANCE] = {\n> sensorConfig.vblankDelay, false };\n>\n\nShould the delay value be 1 here, as the change likely happens on the very\nnext frame?\n\nRegards,\nNaush\n\n\n\n> + }\n> +\n> data->delayedCtrls_ =\n> std::make_unique(data->unicam_[Unicam::Image].dev(),\n> params);\n> data->sensorMetadata_ = sensorConfig.sensorMetadata;\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\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 13E4ABD224\n\tfor ;\n\tWed, 7 Apr 2021 10:32:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8473D60518;\n\tWed, 7 Apr 2021 12:32:49 +0200 (CEST)","from mail-lf1-x133.google.com (mail-lf1-x133.google.com\n\t[IPv6:2a00:1450:4864:20::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 170F7602CC\n\tfor ;\n\tWed, 7 Apr 2021 12:32:48 +0200 (CEST)","by mail-lf1-x133.google.com with SMTP id b4so27669910lfi.6\n\tfor ;\n\tWed, 07 Apr 2021 03:32:48 -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=\"iRo5XOK6\"; 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=amFvy2f0331i2He8FYQu4bIZO/VtpW5KSh5HkSduhvs=;\n\tb=iRo5XOK6y4IC2mEdC+RHXs3COKrEEIfKVGjSU8BQIyK2HzJuPwmCFQx7gYKTbcnyq1\n\tLk4oogfJMv5OW24uVWRvkY87Ath4rJzXgA8IhSG0HcaOyyeRx//8l0FtBb/Nnokf6MUI\n\tkB/kP1pTy39b9Q6cSmem8I2NrdkuGzUUzykOuINuDUx1bC8A7VRj7JWFdMui1YgIfymg\n\tPCSLrygXxietWh8pnbcOTtq9LuKd9qcmgYqhndV9JxZ2Xt4SEl1qvBu1pQZmElOPNDbR\n\twt7U07gD/a4Kdcs9q75fW79anZP9TVHX1mcLwND3JDDdMMqCBkStMkefSgan3TdBfmZQ\n\tEahQ==","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=amFvy2f0331i2He8FYQu4bIZO/VtpW5KSh5HkSduhvs=;\n\tb=UUIKSSPSQG1hi7VVbEqJqX9ZKeQZg9evFOSIWM4vUJ0BspEaXwZUyOuBu2USTtrAJX\n\tv1hYjsUVGV6S9u/qkWMnksyE4OpMd7hQlWY5T8LF4zcaQOB6G/Ad/ngQIWquZQdIK8Ug\n\t34kPlEbRCY0frHzuZEJtR8mI5rI6YAFQ3N8BDaqPYn+my0QKLykqcxVmgbUIDIOvBqZp\n\ti0BFet+z5AjD+SGjqfjVMAM7NkMbQ85rIVUbX67O6rzxwwOh2zVvig7CexX98wYsO4Vx\n\t1SqbQaAEHe/AuonZ6am9gXzS1o1/o4dASYrVcbqPA1VQf8jETyeDDF2Mxr9w9xm9ojOG\n\t0nAA==","X-Gm-Message-State":"AOAM530MfFFN2rr90My0wpz01DU2mKAV84ZSvNSCfC4WJlME9J9bdx/C\n\trWE13GRg3addxA3+jJ7BoRQEp6AXAZGd+UUrszx4Zg==","X-Google-Smtp-Source":"ABdhPJxO471ixLIc+zblHe7xHfAPuXT8WJyJytKD6kc9bxZ8/IDBDebnNcGHpUV9swDi9XjX2jNIqbpjpoFBrZKER80=","X-Received":"by 2002:ac2:43d4:: with SMTP id\n\tu20mr2001797lfl.210.1617791567341; \n\tWed, 07 Apr 2021 03:32:47 -0700 (PDT)","MIME-Version":"1.0","References":"<20210324114415.19866-1-david.plowman@raspberrypi.com>\n\t<20210324114415.19866-3-david.plowman@raspberrypi.com>","In-Reply-To":"<20210324114415.19866-3-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Wed, 7 Apr 2021 11:32:31 +0100","Message-ID":"","To":"David Plowman ","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: raspberrypi: Update sensor's\n\tRED/BLUE balance controls when present","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============4439175474641069587==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16155,"web_url":"https://patchwork.libcamera.org/comment/16155/","msgid":"","date":"2021-04-08T09:19:49","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: raspberrypi: Update sensor's\n\tRED/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 Naush\n\nThanks for the feedback!\n\nOn Wed, 7 Apr 2021 at 11:32, Naushir Patuck wrote:\n>\n> Hi David,\n>\n> Thank you for your patch.\n>\n> On Wed, 24 Mar 2021 at 11:44, David Plowman wrote:\n>>\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. We will need to\n>> add these to the list of delayed controls we can apply.\n>>\n>> Signed-off-by: David Plowman \n>> ---\n>> src/ipa/raspberrypi/raspberrypi.cpp | 17 ++++++++++++++---\n>> .../pipeline/raspberrypi/raspberrypi.cpp | 9 +++++++++\n>> 2 files changed, 23 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n>> index 1c928b72..7437a77e 100644\n>> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>> @@ -1030,13 +1030,24 @@ void IPARPi::processStats(unsigned int bufferId)\n>> RPiController::StatisticsPtr statistics = std::make_shared(*stats);\n>> controller_.Process(statistics, &rpiMetadata_);\n>>\n>> + ControlList ctrls(sensorCtrls_);\n>> +\n>> struct AgcStatus agcStatus;\n>> - if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0) {\n>> - ControlList ctrls(sensorCtrls_);\n>> + if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0)\n>> applyAGC(&agcStatus, ctrls);\n>>\n>> - setDelayedControls.emit(ctrls);\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>> + ctrls.set(V4L2_CID_RED_BALANCE,\n>> + static_cast(awbStatus.gain_r * 256));\n>> + ctrls.set(V4L2_CID_BLUE_BALANCE,\n>> + static_cast(awbStatus.gain_b * 256));\n>\n>\n> The 256 const bothers me slightly. Is this FP multiplier going to be true for all sensors?\n> Any way we can deduce this (perhaps from the min value)?\n\nFor the sensor I have, there doesn't seem to be a documented min\nvalue, so the driver will basically accept anything. Values less than\n256 (1.0x) might seem unlikely, but it's perhaps not reasonable to\nrule them out completely.\n\nMy other suggestion was to add a CamHelper::ColourGainCode method.\nWhat do you make of that? It would also cover the theoretical\npossibility that the value we give to the driver is not a simple\nlinear gain. (Note also that there's no need for the reverse mapping -\ncode back to gain - as we don't need it.)\n\n>\n>>\n>> }\n>> +\n>> + if (!ctrls.empty())\n>> + setDelayedControls.emit(ctrls);\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 2cac802c..7bac1503 100644\n>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> @@ -1027,6 +1027,15 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n>> { V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false } },\n>> { V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }\n>> };\n>> +\n>> + /* If the sensor exposes red/blue balance controls, we will update them. */\n>> + const ControlInfoMap &sensorControls = data->unicam_[Unicam::Image].dev()->controls();\n>> + if (sensorControls.find(V4L2_CID_RED_BALANCE) != sensorControls.end() &&\n>> + sensorControls.find(V4L2_CID_BLUE_BALANCE) != sensorControls.end()) {\n>> + params[V4L2_CID_RED_BALANCE] = { sensorConfig.vblankDelay, false };\n>> + params[V4L2_CID_BLUE_BALANCE] = { sensorConfig.vblankDelay, false };\n>\n>\n> Should the delay value be 1 here, as the change likely happens on the very next frame?\n\nAha, interesting point! So my thinking was:\n\n1. We never read these values back, so I don't care about when they're\nreported as having taken effect, and\n2. I want the value to be applied immediately, not after several\nframes of delay. As such, the same delay as \"the worst\" of the other\ncontrols should be optimal.\n\nTo some extent, perhaps this is revealing a minor abuse of the delayed\ncontrol system for something we didn't really intend. But I don't\nthink we provide any other way of getting these values back to the PH.\n\nAs a minimum though, I guess a comment would be helpful. Do you think\nwe need to do something more/different?\n\nThanks!\nDavid\n\n>\n> Regards,\n> Naush\n>\n>\n>>\n>> + }\n>> +\n>> data->delayedCtrls_ = std::make_unique(data->unicam_[Unicam::Image].dev(), params);\n>> data->sensorMetadata_ = sensorConfig.sensorMetadata;\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":"","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 E1ACEBD1F6\n\tfor ;\n\tThu, 8 Apr 2021 09:20:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36BF6687F5;\n\tThu, 8 Apr 2021 11:20:04 +0200 (CEST)","from mail-oi1-x230.google.com (mail-oi1-x230.google.com\n\t[IPv6:2607:f8b0:4864:20::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A7E7687F2\n\tfor ;\n\tThu, 8 Apr 2021 11:20:02 +0200 (CEST)","by mail-oi1-x230.google.com with SMTP id x2so1501098oiv.2\n\tfor ;\n\tThu, 08 Apr 2021 02:20:02 -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=\"tkZgyUI3\"; 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=s2J7vr9ChqbvGTiOvBikQUz2ohz6X5GXa3pymoJewJk=;\n\tb=tkZgyUI3Y5GkYBerOoZvmByn0eAasG8JogKj7NdlblGDV1ngMZBM4cLZWkkdHxonzj\n\tBxybGSEgkvWmsESdn51qUmEGJGE2X4e7VtJKW8sh/Y+5NgjOL17sd73L5ApF1qzqO26D\n\tW6OEWkdW5RTzsW+rxeAaoFlYfVXevf3t5cH324oIigOWXeI8fzRsZ6PtDrBgq+Qld98R\n\tdbIukJ0yo+YJ2Ek3hMFFKjScR8pUJr6BAxiJ9usznOYvkMJfPmXKX/33i9QEgsAkILjP\n\td5kyE2wT5EnmXVfS47OFDsvixWgoM+JCf3rVz9aPYZZqSEufvLQOY8oqmiLAyUPotgjY\n\t3hiQ==","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=s2J7vr9ChqbvGTiOvBikQUz2ohz6X5GXa3pymoJewJk=;\n\tb=syoNrIyzGK8MJkBdRCxw3RR+yzSeNYjZyxswNE8d86wcGRnhbehX7e0ouyeJdXaAI5\n\tkolODoz35jqtkTPmFZJ0fp8Gyornmq36kRL0EY2qjiThBA2WMWNigBTR2Do2H+qR3xJd\n\tO0BFQ4nuF1qhjZ4X6yWakPsA2QNF7okXB0OO5CRSd7pYuVNG9dcsKiplhy6i8AaZ8Xwx\n\tuL0a4LxjcmvEBmMCmr8J3Yp2L6xNTDRzNgvLsnhjvSTVxKeGZvdsyvakfc9pB0k4gc7N\n\tWZyPAAir4tNOX3f6aOY2MtDMaWa0jAmKuyz6K7WkhIE+ADZoxW3EetS0Ev4nfbZXkUnG\n\tASqw==","X-Gm-Message-State":"AOAM5300aJVehxYI6QcBUt19UcjROmcvTHxpyFK3BxRHCZcpwYT0ULhz\n\tst8IaHR6E6FOiTSlWkBlF0vTFqH6jkOT50hdO231zA==","X-Google-Smtp-Source":"ABdhPJxhlhlJluRynjubDJPBqLytxUl+LLMJpgerVWF1s0NII/YnitaPpm08cEOXyDLq1F7th2TEWct8226G0VRREOo=","X-Received":"by 2002:aca:314a:: with SMTP id x71mr5547296oix.55.1617873601030;\n\tThu, 08 Apr 2021 02:20:01 -0700 (PDT)","MIME-Version":"1.0","References":"<20210324114415.19866-1-david.plowman@raspberrypi.com>\n\t<20210324114415.19866-3-david.plowman@raspberrypi.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Thu, 8 Apr 2021 10:19:49 +0100","Message-ID":"","To":"Naushir Patuck ","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: raspberrypi: Update sensor's\n\tRED/BLUE balance controls when present","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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16156,"web_url":"https://patchwork.libcamera.org/comment/16156/","msgid":"","date":"2021-04-08T09:44:26","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: raspberrypi: Update sensor's\n\tRED/BLUE balance controls when present","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\n\nOn Thu, 8 Apr 2021 at 10:20, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thanks for the feedback!\n>\n> On Wed, 7 Apr 2021 at 11:32, Naushir Patuck wrote:\n> >\n> > Hi David,\n> >\n> > Thank you for your patch.\n> >\n> > On Wed, 24 Mar 2021 at 11:44, David Plowman <\n> david.plowman@raspberrypi.com> wrote:\n> >>\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. We will need to\n> >> add these to the list of delayed controls we can apply.\n> >>\n> >> Signed-off-by: David Plowman \n> >> ---\n> >> src/ipa/raspberrypi/raspberrypi.cpp | 17 ++++++++++++++---\n> >> .../pipeline/raspberrypi/raspberrypi.cpp | 9 +++++++++\n> >> 2 files changed, 23 insertions(+), 3 deletions(-)\n> >>\n> >> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> >> index 1c928b72..7437a77e 100644\n> >> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> >> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> >> @@ -1030,13 +1030,24 @@ void IPARPi::processStats(unsigned int bufferId)\n> >> RPiController::StatisticsPtr statistics =\n> std::make_shared(*stats);\n> >> controller_.Process(statistics, &rpiMetadata_);\n> >>\n> >> + ControlList ctrls(sensorCtrls_);\n> >> +\n> >> struct AgcStatus agcStatus;\n> >> - if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0) {\n> >> - ControlList ctrls(sensorCtrls_);\n> >> + if (rpiMetadata_.Get(\"agc.status\", agcStatus) == 0)\n> >> applyAGC(&agcStatus, ctrls);\n> >>\n> >> - setDelayedControls.emit(ctrls);\n> >> + struct AwbStatus awbStatus;\n> >> + if (rpiMetadata_.Get(\"awb.status\", awbStatus) == 0 &&\n> >> + sensorCtrls_.find(V4L2_CID_RED_BALANCE) !=\n> sensorCtrls_.end() &&\n> >> + sensorCtrls_.find(V4L2_CID_BLUE_BALANCE) !=\n> sensorCtrls_.end()) {\n> >> + ctrls.set(V4L2_CID_RED_BALANCE,\n> >> + static_cast(awbStatus.gain_r * 256));\n> >> + ctrls.set(V4L2_CID_BLUE_BALANCE,\n> >> + static_cast(awbStatus.gain_b * 256));\n> >\n> >\n> > The 256 const bothers me slightly. Is this FP multiplier going to be\n> true for all sensors?\n> > Any way we can deduce this (perhaps from the min value)?\n>\n> For the sensor I have, there doesn't seem to be a documented min\n> value, so the driver will basically accept anything. Values less than\n> 256 (1.0x) might seem unlikely, but it's perhaps not reasonable to\n> rule them out completely.\n>\n> My other suggestion was to add a CamHelper::ColourGainCode method.\n> What do you make of that? It would also cover the theoretical\n> possibility that the value we give to the driver is not a simple\n> linear gain. (Note also that there's no need for the reverse mapping -\n> code back to gain - as we don't need it.)\n>\n\nThis sounds like a good idea to me.\n\n\n>\n> >\n> >>\n> >> }\n> >> +\n> >> + if (!ctrls.empty())\n> >> + setDelayedControls.emit(ctrls);\n> >> }\n> >>\n> >> void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList\n> &ctrls)\n> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> index 2cac802c..7bac1503 100644\n> >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> @@ -1027,6 +1027,15 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> >> { V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay,\n> false } },\n> >> { V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }\n> >> };\n> >> +\n> >> + /* If the sensor exposes red/blue balance controls, we will\n> update them. */\n> >> + const ControlInfoMap &sensorControls =\n> data->unicam_[Unicam::Image].dev()->controls();\n> >> + if (sensorControls.find(V4L2_CID_RED_BALANCE) !=\n> sensorControls.end() &&\n> >> + sensorControls.find(V4L2_CID_BLUE_BALANCE) !=\n> sensorControls.end()) {\n> >> + params[V4L2_CID_RED_BALANCE] = {\n> sensorConfig.vblankDelay, false };\n> >> + params[V4L2_CID_BLUE_BALANCE] = {\n> sensorConfig.vblankDelay, false };\n> >\n> >\n> > Should the delay value be 1 here, as the change likely happens on the\n> very next frame?\n>\n> Aha, interesting point! So my thinking was:\n>\n> 1. We never read these values back, so I don't care about when they're\n> reported as having taken effect, and\n> 2. I want the value to be applied immediately, not after several\n> frames of delay. As such, the same delay as \"the worst\" of the other\n> controls should be optimal.\n>\n> To some extent, perhaps this is revealing a minor abuse of the delayed\n> control system for something we didn't really intend. But I don't\n> think we provide any other way of getting these values back to the PH.\n>\n> As a minimum though, I guess a comment would be helpful. Do you think\n> we need to do something more/different?\n>\n\nI wonder if we should bypass DelayedControls for this entirely?\n\nIf we create a new Signal called SetSensorControls in the IPA interface\nthat pushes the\nctrl values directly to the sensor, that should be enough. Hopefully not a\nbig change, but\nmay make this bit a bit neater.\n\nRegards,\nNaush\n\n\n\n>\n> Thanks!\n> David\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> >>\n> >> + }\n> >> +\n> >> data->delayedCtrls_ =\n> std::make_unique(data->unicam_[Unicam::Image].dev(),\n> params);\n> >> data->sensorMetadata_ = sensorConfig.sensorMetadata;\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\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 04BECBD16B\n\tfor ;\n\tThu, 8 Apr 2021 09:44:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 715DC687F4;\n\tThu, 8 Apr 2021 11:44:45 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AA8E5687F3\n\tfor ;\n\tThu, 8 Apr 2021 11:44:43 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id n138so2928407lfa.3\n\tfor ;\n\tThu, 08 Apr 2021 02:44:43 -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=\"UttmtfCR\"; 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=kRmcywFolKG9bT7XkBtuQrlZLUltc3gwmabmJ3mldLI=;\n\tb=UttmtfCRdIlLjphmapR0dTvQcs/MzN5f3sy6y5X3+LqmpjU4YUwkMAhJJl6nPKqhkW\n\tgjbZiDBuOKYDEOc6N1T0ngwy+svTLAuv8YPuz5OZHtyNPPTz76L9ZzjbDontsND5eAv1\n\tt3GgReyJVEzgKUW4o58HKKZogj6RgewE6gl/Q2MRwgvSNcpvibLHwpO2sVdS7vuPxwJV\n\t4oAeVWssWY6od0Xg6ODm2U4UoH/UcOWlbfs1yWoVJ+gNIMKesAdWkbWobFofmCIy5N+2\n\tjuy7lrJM7N4ZjccI+18qRfCaWdMbZz+vsw9qlO5wd0z8Sf9xUTiDwzCm1LNxhcZbMWBx\n\teghQ==","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=kRmcywFolKG9bT7XkBtuQrlZLUltc3gwmabmJ3mldLI=;\n\tb=qR9EPg6SnNkdO3YQuElmPiTbFvi78XWdqojtVJObgfSCkMs9HejKtSxYiXTT18UDJr\n\tLtGEahJjVWNAxPqQT4dHLOX0Uld3TsFKXYmyHIeF3dAeEs9MpCJcWf3PQIS6GR6E4EI/\n\tuy6uKVU6IyyVNNotDH93+qE+puKrRWmeyGBW/U30jwcO/2JD5L2AFQDMzmS2p9fS+wNC\n\tn5Hxas8L2/qAstRcwgKbytBYJL74I+np2Z00iNfwvHyd95bqrJkIL2r9NZIQ0GzOSqVg\n\tunb4Clui+CghETlgn6dzpOPWFg6wTbOH4Y1HNmO29TD9heZkNcDlSOIcicrTWO62jtYR\n\t10Tw==","X-Gm-Message-State":"AOAM531CgnwXdkH1l8rzrAr8bl7WY2npOg8QwH9UuVldJfFeqkgtoa+7\n\t6e2X4/nsgXtujSBsAbCke/itPukZskX2bZvYj1NAPQ==","X-Google-Smtp-Source":"ABdhPJytdWIu0HoEkjxv6i7NKCQGEcG6hsNU+6kD38yjA6WM/zyfwhxBFRFsi3YK8loy/08+P8ECjCH3mDys9nedWko=","X-Received":"by 2002:a05:6512:11cc:: with SMTP id\n\th12mr5465763lfr.567.1617875082939; \n\tThu, 08 Apr 2021 02:44:42 -0700 (PDT)","MIME-Version":"1.0","References":"<20210324114415.19866-1-david.plowman@raspberrypi.com>\n\t<20210324114415.19866-3-david.plowman@raspberrypi.com>\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Thu, 8 Apr 2021 10:44:26 +0100","Message-ID":"","To":"David Plowman ","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: raspberrypi: Update sensor's\n\tRED/BLUE balance controls when present","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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============0219673715236569992==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]