[{"id":27787,"web_url":"https://patchwork.libcamera.org/comment/27787/","msgid":"<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>","date":"2023-09-15T14:29:35","subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman via libcamera-devel (2023-09-15 13:29:51)\n> This commit does the basic reorganisation of the code in order to\n> implement multi-channel AGC. The main changes are:\n> \n> * The previous Agc class (in agc.cpp) has become the AgcChannel class\n>   in (agc_channel.cpp).\n> \n> * A new Agc class is introduced which is a wrapper round a number of\n>   AgcChannels.\n> \n> * The basic plumbing from ipa_base.cpp to Agc is updated to include a\n>   channel number. All the existing controls are hardwired to talk\n>   directly to channel 0.\n> \n> There are a couple of limitations which we expect to apply to\n> multi-channel AGC. We're not allowing different frame durations to be\n> applied to the channels, nor are we allowing separate metering\n> modes. To be fair, supporting these things is not impossible, but\n> there are reasons why it may be tricky so they remain \"TBD\" for now.\n> \n> This patch only includes the basic reorganisation and plumbing. It\n> does not yet update the important methods (switchMode, prepare and\n> process) to implement multi-channel AGC properly. This will appear in\n> a subsequent commit. For now, these functions are hard-coded just to\n> use channel 0, thereby preserving the existing behaviour.\n> \n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n\n<snippity>\n\n> +static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n> +                             std::vector<double> &weights, double gain)\n> +{\n> +       constexpr uint64_t maxVal = 1 << Statistics::NormalisationFactorPow2;\n> +\n> +       /*\n> +        * If we have no AGC region stats, but do have a a Y histogram, use that\n> +        * directly to caluclate the mean Y value of the image.\n> +        */\n> +       if (!stats->agcRegions.numRegions() && stats->yHist.bins()) {\n> +               /*\n> +                * When the gain is applied to the histogram, anything below minBin\n> +                * will scale up directly with the gain, but anything above that\n> +                * will saturate into the top bin.\n> +                */\n> +               auto &hist = stats->yHist;\n> +               double minBin = std::min(1.0, 1.0 / gain) * hist.bins();\n> +               double binMean = hist.interBinMean(0.0, minBin);\n> +               double numUnsaturated = hist.cumulativeFreq(minBin);\n> +               /* This term is from all the pixels that won't saturate. */\n> +               double ySum = binMean * gain * numUnsaturated;\n> +               /* And add the ones that will saturate. */\n> +               ySum += (hist.total() - numUnsaturated) * hist.bins();\n> +               return ySum / hist.total() / hist.bins();\n> +       }\n> +\n> +       ASSERT(weights.size() == stats->agcRegions.numRegions());\n> +\n> +       /*\n> +        * Note that the weights are applied by the IPA to the statistics directly,\n> +        * before they are given to us here.\n> +        */\n> +       double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n\nFAILED: src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o \nclang++-13 -Isrc/ipa/rpi/controller/librpi_ipa_controller.a.p -Isrc/ipa/rpi/controller -I../../../src/libcamera/src/ipa/rpi/controller -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-13/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -MF src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o.d -o src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -c ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp\n../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp:675:29: error: variable 'bSum' set but not used [-Werror,-Wunused-but-set-variable]\n        double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n                                   ^\n1 error generated.\n\nI think we can just remove this bSum = 0, local var.\n\nLikely no need for a resend.\n\n\n> +       for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {\n> +               auto &region = stats->agcRegions.get(i);\n> +               rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);\n> +               gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);\n> +               bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);\n> +               pixelSum += region.counted;\n> +       }\n> +       if (pixelSum == 0.0) {\n> +               LOG(RPiAgc, Warning) << \"computeInitialY: pixelSum is zero\";\n> +               return 0;\n> +       }\n> +\n> +       double ySum;\n> +       /* Factor in the AWB correction if needed. */\n> +       if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) {\n> +               ySum = rSum * awb.gainR * .299 +\n> +                      gSum * awb.gainG * .587 +\n> +                      gSum * awb.gainB * .114;\n> +       } else\n> +               ySum = rSum * .299 + gSum * .587 + gSum * .114;\n> +\n> +       return ySum / pixelSum / (1 << 16);\n> +}\n> +","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DDB73BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Sep 2023 14:29:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4141F62918;\n\tFri, 15 Sep 2023 16:29:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BA124628EC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 16:29:38 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 344299A8;\n\tFri, 15 Sep 2023 16:28:05 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694788180;\n\tbh=W/1exok1S5jihYqsC2ehs3evUPziBkViITDm8kS26mE=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=DmJseLUcLCdPZgN7ee/VDYXLKen0AdvnHj6eOkf4GAWT5IcCNQ1gNo2CX8xdcivsl\n\tTzfIsyF/e3P0uq9cIMjnueoy4FL3yWOg4PnCZMDgfjuFwpv+ISC9Qhu/3r31ez74LO\n\tmlCZthfyYNgiUzp+k6cwJQpqTct4GFOUfWNlQkCPP9b1YSw9wo7LEWUS0aDsO+1hJa\n\tzNLdCm4EtoF6lxVmbR5yEq4nPrshoh4FLUE8vDrhL84sT5bYTa6gp1zy16oU8o3nwc\n\tRK578Fg8lr40/36ENGEF6iSvlDoQz1j27Nb9vqRTJSdbHtdTymg1QkgN27U9I7dCWo\n\tpTsVA5//rnxsQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694788085;\n\tbh=W/1exok1S5jihYqsC2ehs3evUPziBkViITDm8kS26mE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=OAa/WwzcGOrVxVYHNoRbjl/5FxXpPkUvNG234Jli1xRJMAOlE3uTPyE9jA5N1XQyE\n\touDbp4j2biI9K9crMJF0huxztNGWhL2j2z7N2B5YxHuhw6r8JQXpyB5BtQiqHmxMsX\n\tY/IcJ8RVFZ+BDbCXW9vs1pZwzapCejYhqbig6RpI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"OAa/Wwzc\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230915122954.5231-3-david.plowman@raspberrypi.com>","References":"<20230915122954.5231-1-david.plowman@raspberrypi.com>\n\t<20230915122954.5231-3-david.plowman@raspberrypi.com>","To":"David Plowman <david.plowman@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 15 Sep 2023 15:29:35 +0100","Message-ID":"<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27788,"web_url":"https://patchwork.libcamera.org/comment/27788/","msgid":"<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>","date":"2023-09-15T14:36:53","subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2023-09-15 15:29:35)\n> Quoting David Plowman via libcamera-devel (2023-09-15 13:29:51)\n> > This commit does the basic reorganisation of the code in order to\n> > implement multi-channel AGC. The main changes are:\n> > \n> > * The previous Agc class (in agc.cpp) has become the AgcChannel class\n> >   in (agc_channel.cpp).\n> > \n> > * A new Agc class is introduced which is a wrapper round a number of\n> >   AgcChannels.\n> > \n> > * The basic plumbing from ipa_base.cpp to Agc is updated to include a\n> >   channel number. All the existing controls are hardwired to talk\n> >   directly to channel 0.\n> > \n> > There are a couple of limitations which we expect to apply to\n> > multi-channel AGC. We're not allowing different frame durations to be\n> > applied to the channels, nor are we allowing separate metering\n> > modes. To be fair, supporting these things is not impossible, but\n> > there are reasons why it may be tricky so they remain \"TBD\" for now.\n> > \n> > This patch only includes the basic reorganisation and plumbing. It\n> > does not yet update the important methods (switchMode, prepare and\n> > process) to implement multi-channel AGC properly. This will appear in\n> > a subsequent commit. For now, these functions are hard-coded just to\n> > use channel 0, thereby preserving the existing behaviour.\n> > \n> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> \n> <snippity>\n> \n> > +static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n> > +                             std::vector<double> &weights, double gain)\n> > +{\n> > +       constexpr uint64_t maxVal = 1 << Statistics::NormalisationFactorPow2;\n> > +\n> > +       /*\n> > +        * If we have no AGC region stats, but do have a a Y histogram, use that\n> > +        * directly to caluclate the mean Y value of the image.\n> > +        */\n> > +       if (!stats->agcRegions.numRegions() && stats->yHist.bins()) {\n> > +               /*\n> > +                * When the gain is applied to the histogram, anything below minBin\n> > +                * will scale up directly with the gain, but anything above that\n> > +                * will saturate into the top bin.\n> > +                */\n> > +               auto &hist = stats->yHist;\n> > +               double minBin = std::min(1.0, 1.0 / gain) * hist.bins();\n> > +               double binMean = hist.interBinMean(0.0, minBin);\n> > +               double numUnsaturated = hist.cumulativeFreq(minBin);\n> > +               /* This term is from all the pixels that won't saturate. */\n> > +               double ySum = binMean * gain * numUnsaturated;\n> > +               /* And add the ones that will saturate. */\n> > +               ySum += (hist.total() - numUnsaturated) * hist.bins();\n> > +               return ySum / hist.total() / hist.bins();\n> > +       }\n> > +\n> > +       ASSERT(weights.size() == stats->agcRegions.numRegions());\n> > +\n> > +       /*\n> > +        * Note that the weights are applied by the IPA to the statistics directly,\n> > +        * before they are given to us here.\n> > +        */\n> > +       double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> \n> FAILED: src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o \n> clang++-13 -Isrc/ipa/rpi/controller/librpi_ipa_controller.a.p -Isrc/ipa/rpi/controller -I../../../src/libcamera/src/ipa/rpi/controller -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-13/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -MF src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o.d -o src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -c ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp\n> ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp:675:29: error: variable 'bSum' set but not used [-Werror,-Wunused-but-set-variable]\n>         double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n>                                    ^\n> 1 error generated.\n> \n> I think we can just remove this bSum = 0, local var.\n> \n> Likely no need for a resend.\n> \n> \n> > +       for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {\n> > +               auto &region = stats->agcRegions.get(i);\n> > +               rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);\n> > +               gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);\n> > +               bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);\n\nDavid,\n\nWe accumulate the bSum here - but don't then use it. Do you foresee\nneeding this bSum anywhere else? or is this safe to drop?\n\n\n> > +               pixelSum += region.counted;\n> > +       }\n> > +       if (pixelSum == 0.0) {\n> > +               LOG(RPiAgc, Warning) << \"computeInitialY: pixelSum is zero\";\n> > +               return 0;\n> > +       }\n> > +\n> > +       double ySum;\n> > +       /* Factor in the AWB correction if needed. */\n> > +       if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) {\n> > +               ySum = rSum * awb.gainR * .299 +\n> > +                      gSum * awb.gainG * .587 +\n> > +                      gSum * awb.gainB * .114;\n> > +       } else\n> > +               ySum = rSum * .299 + gSum * .587 + gSum * .114;\n\nOr ... is the fault here, that it should be bSum * .114 ? (Looks quite\nlikely given the block above it).\n\n\n--\nKieran\n\n\n> > +\n> > +       return ySum / pixelSum / (1 << 16);\n> > +}\n> > +","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F1FBBBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Sep 2023 14:36:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 387CF62918;\n\tFri, 15 Sep 2023 16:36:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 10DF3628EC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 16:36:57 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 857949A8;\n\tFri, 15 Sep 2023 16:35:23 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694788618;\n\tbh=00pAh0rZ/2MwAP5A8sMUxUjuShsGebe/5rIkFiMR2sk=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=wma7M4Ze9/bO/OlKprHvzTUtNoW0vYBm3LP08d5H+wRXcaICJllm7wFNGbKJm3i8/\n\tlmjR7FmXUQfqe72yv5mGz918hb8o/DcdgBczsaNFAkrCQfbtF2EbzQaZNePDk66CBr\n\t8M9WvLCwbeErsChdNg2fGSAcl6/+ylp2ssoDXZkBb/6zZ4cZ5ZbTDqA8Of5e+d424L\n\t+AJRStaRxiC2YepCtGj+l/whghS/f9IpOsVhtJEw8Ym6OUM3KeoCiLQ5HnlRlcwEAA\n\tUtW54AgCY37mkYF1a5Mhtlq4hFYYWgtLmpuqdP1MBfgAoq9xFD316kBoZtH2SxGyVR\n\tdfhWwXUUnSEgQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694788523;\n\tbh=00pAh0rZ/2MwAP5A8sMUxUjuShsGebe/5rIkFiMR2sk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=MTdQdS7f3FE0utJjCFNEeaY/DxBAyxDXnT4V6hb/6yoFqykVPw2fzJ8Rp9i/M7Q6T\n\t/giIT/6TJ/wrHBHLaq4an5EBeOfwSJ7s4S3tSgFriobstqCjQgwPV+u5cxJH0L9naY\n\tHdIn4xtNHsfVP1qdCuccoRu4hjonjRhfXAFE9BvM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MTdQdS7f\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>","References":"<20230915122954.5231-1-david.plowman@raspberrypi.com>\n\t<20230915122954.5231-3-david.plowman@raspberrypi.com>\n\t<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>","To":"David Plowman <david.plowman@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 15 Sep 2023 15:36:53 +0100","Message-ID":"<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27789,"web_url":"https://patchwork.libcamera.org/comment/27789/","msgid":"<169478871382.2427060.4183094997043561165@ping.linuxembedded.co.uk>","date":"2023-09-15T14:38:33","subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2023-09-15 15:36:53)\n> Quoting Kieran Bingham (2023-09-15 15:29:35)\n> > Quoting David Plowman via libcamera-devel (2023-09-15 13:29:51)\n> > > This commit does the basic reorganisation of the code in order to\n> > > implement multi-channel AGC. The main changes are:\n> > > \n> > > * The previous Agc class (in agc.cpp) has become the AgcChannel class\n> > >   in (agc_channel.cpp).\n> > > \n> > > * A new Agc class is introduced which is a wrapper round a number of\n> > >   AgcChannels.\n> > > \n> > > * The basic plumbing from ipa_base.cpp to Agc is updated to include a\n> > >   channel number. All the existing controls are hardwired to talk\n> > >   directly to channel 0.\n> > > \n> > > There are a couple of limitations which we expect to apply to\n> > > multi-channel AGC. We're not allowing different frame durations to be\n> > > applied to the channels, nor are we allowing separate metering\n> > > modes. To be fair, supporting these things is not impossible, but\n> > > there are reasons why it may be tricky so they remain \"TBD\" for now.\n> > > \n> > > This patch only includes the basic reorganisation and plumbing. It\n> > > does not yet update the important methods (switchMode, prepare and\n> > > process) to implement multi-channel AGC properly. This will appear in\n> > > a subsequent commit. For now, these functions are hard-coded just to\n> > > use channel 0, thereby preserving the existing behaviour.\n> > > \n> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > ---\n> > \n> > <snippity>\n> > \n> > > +static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n> > > +                             std::vector<double> &weights, double gain)\n> > > +{\n> > > +       constexpr uint64_t maxVal = 1 << Statistics::NormalisationFactorPow2;\n> > > +\n> > > +       /*\n> > > +        * If we have no AGC region stats, but do have a a Y histogram, use that\n> > > +        * directly to caluclate the mean Y value of the image.\n> > > +        */\n> > > +       if (!stats->agcRegions.numRegions() && stats->yHist.bins()) {\n> > > +               /*\n> > > +                * When the gain is applied to the histogram, anything below minBin\n> > > +                * will scale up directly with the gain, but anything above that\n> > > +                * will saturate into the top bin.\n> > > +                */\n> > > +               auto &hist = stats->yHist;\n> > > +               double minBin = std::min(1.0, 1.0 / gain) * hist.bins();\n> > > +               double binMean = hist.interBinMean(0.0, minBin);\n> > > +               double numUnsaturated = hist.cumulativeFreq(minBin);\n> > > +               /* This term is from all the pixels that won't saturate. */\n> > > +               double ySum = binMean * gain * numUnsaturated;\n> > > +               /* And add the ones that will saturate. */\n> > > +               ySum += (hist.total() - numUnsaturated) * hist.bins();\n> > > +               return ySum / hist.total() / hist.bins();\n> > > +       }\n> > > +\n> > > +       ASSERT(weights.size() == stats->agcRegions.numRegions());\n> > > +\n> > > +       /*\n> > > +        * Note that the weights are applied by the IPA to the statistics directly,\n> > > +        * before they are given to us here.\n> > > +        */\n> > > +       double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> > \n> > FAILED: src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o \n> > clang++-13 -Isrc/ipa/rpi/controller/librpi_ipa_controller.a.p -Isrc/ipa/rpi/controller -I../../../src/libcamera/src/ipa/rpi/controller -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-13/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -MF src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o.d -o src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -c ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp\n> > ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp:675:29: error: variable 'bSum' set but not used [-Werror,-Wunused-but-set-variable]\n> >         double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> >                                    ^\n> > 1 error generated.\n> > \n> > I think we can just remove this bSum = 0, local var.\n> > \n> > Likely no need for a resend.\n> > \n> > \n> > > +       for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {\n> > > +               auto &region = stats->agcRegions.get(i);\n> > > +               rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);\n> > > +               gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);\n> > > +               bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);\n> \n> David,\n> \n> We accumulate the bSum here - but don't then use it. Do you foresee\n> needing this bSum anywhere else? or is this safe to drop?\n> \n> \n> > > +               pixelSum += region.counted;\n> > > +       }\n> > > +       if (pixelSum == 0.0) {\n> > > +               LOG(RPiAgc, Warning) << \"computeInitialY: pixelSum is zero\";\n> > > +               return 0;\n> > > +       }\n> > > +\n> > > +       double ySum;\n> > > +       /* Factor in the AWB correction if needed. */\n> > > +       if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) {\n> > > +               ySum = rSum * awb.gainR * .299 +\n> > > +                      gSum * awb.gainG * .587 +\n> > > +                      gSum * awb.gainB * .114;\n\nEeek - and should this be:\n\tbSum * awb.gainB * .114; ?\n\n> > > +       } else\n> > > +               ySum = rSum * .299 + gSum * .587 + gSum * .114;\n> \n> Or ... is the fault here, that it should be bSum * .114 ? (Looks quite\n> likely given the block above it).\n> \n> \n> --\n> Kieran\n> \n> \n> > > +\n> > > +       return ySum / pixelSum / (1 << 16);\n> > > +}\n> > > +","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5BA1CBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Sep 2023 14:38:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0704F62919;\n\tFri, 15 Sep 2023 16:38:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B483062911\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 16:38:36 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 62AA31536;\n\tFri, 15 Sep 2023 16:37:03 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694788718;\n\tbh=UdM6I2jDCNJ3UFuadhMRWVJTf9UxgIcCGUC6PWHkHww=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=hxBsHrBnRB6ZAnJ/WLc4LOPgk94Uo9YB1Qm0sV8F3+K5U8pEudmDmyKoGALTKFmQ3\n\teD4PNq08xFQixxJ2UTZz4LrcQL4bLQtZKDVXGgmMyOfpE+0inLzBP77H67wvfG1WQu\n\t7qXqrvgtpxHTA3QWp4HHTY+jvtdJGXaFFyfck38Gfmb01/g1mMteq5tJL3c7biS7CI\n\t5m07Z446LHfjbyu8CYhhasndrCwVcF0OKEDf5fOBmNzXMjcAGBf6tHoBuQ2vlLiJUd\n\ty+3+Z4DHwfd7oLYzspOQyFKBtePYXv+yiTxvU4WmOsUgt77QtwLsWDEcIIXDlbVY1d\n\t672TZ32yT4J9Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694788623;\n\tbh=UdM6I2jDCNJ3UFuadhMRWVJTf9UxgIcCGUC6PWHkHww=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=C+/pfz2gaX0t1YstecHETj6KgFNSmEKN/+w3PCQK4yDgGzm14U/c86f0wk34v7xpf\n\tFfC6kHL72WP5Nc35FYdIn0j4srifeCQb5MjJPVQwnhXNoIpWjtljPxwuIGuCbVnmG3\n\tJMwT7Zwjrg5oVxIz3Lpjt1TzSX/cTLvgfGj7xRNg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"C+/pfz2g\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>","References":"<20230915122954.5231-1-david.plowman@raspberrypi.com>\n\t<20230915122954.5231-3-david.plowman@raspberrypi.com>\n\t<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>\n\t<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>","To":"David Plowman <david.plowman@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 15 Sep 2023 15:38:33 +0100","Message-ID":"<169478871382.2427060.4183094997043561165@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27790,"web_url":"https://patchwork.libcamera.org/comment/27790/","msgid":"<CAHW6GYJ60r3twDS9BLt_ehfkO1we+c=73376MXqhQeOOf4U2Dw@mail.gmail.com>","date":"2023-09-15T14:39:07","subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Kieran\n\nYes, I was just looking at that when your 2nd email arrived. It's a\nbug, exactly as you describe it!\n\nI'll make the fix and give it some testing, though I can't believe it\nwill do anything bad!\n\nDavid\n\nOn Fri, 15 Sept 2023 at 15:36, Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Quoting Kieran Bingham (2023-09-15 15:29:35)\n> > Quoting David Plowman via libcamera-devel (2023-09-15 13:29:51)\n> > > This commit does the basic reorganisation of the code in order to\n> > > implement multi-channel AGC. The main changes are:\n> > >\n> > > * The previous Agc class (in agc.cpp) has become the AgcChannel class\n> > >   in (agc_channel.cpp).\n> > >\n> > > * A new Agc class is introduced which is a wrapper round a number of\n> > >   AgcChannels.\n> > >\n> > > * The basic plumbing from ipa_base.cpp to Agc is updated to include a\n> > >   channel number. All the existing controls are hardwired to talk\n> > >   directly to channel 0.\n> > >\n> > > There are a couple of limitations which we expect to apply to\n> > > multi-channel AGC. We're not allowing different frame durations to be\n> > > applied to the channels, nor are we allowing separate metering\n> > > modes. To be fair, supporting these things is not impossible, but\n> > > there are reasons why it may be tricky so they remain \"TBD\" for now.\n> > >\n> > > This patch only includes the basic reorganisation and plumbing. It\n> > > does not yet update the important methods (switchMode, prepare and\n> > > process) to implement multi-channel AGC properly. This will appear in\n> > > a subsequent commit. For now, these functions are hard-coded just to\n> > > use channel 0, thereby preserving the existing behaviour.\n> > >\n> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > ---\n> >\n> > <snippity>\n> >\n> > > +static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n> > > +                             std::vector<double> &weights, double gain)\n> > > +{\n> > > +       constexpr uint64_t maxVal = 1 << Statistics::NormalisationFactorPow2;\n> > > +\n> > > +       /*\n> > > +        * If we have no AGC region stats, but do have a a Y histogram, use that\n> > > +        * directly to caluclate the mean Y value of the image.\n> > > +        */\n> > > +       if (!stats->agcRegions.numRegions() && stats->yHist.bins()) {\n> > > +               /*\n> > > +                * When the gain is applied to the histogram, anything below minBin\n> > > +                * will scale up directly with the gain, but anything above that\n> > > +                * will saturate into the top bin.\n> > > +                */\n> > > +               auto &hist = stats->yHist;\n> > > +               double minBin = std::min(1.0, 1.0 / gain) * hist.bins();\n> > > +               double binMean = hist.interBinMean(0.0, minBin);\n> > > +               double numUnsaturated = hist.cumulativeFreq(minBin);\n> > > +               /* This term is from all the pixels that won't saturate. */\n> > > +               double ySum = binMean * gain * numUnsaturated;\n> > > +               /* And add the ones that will saturate. */\n> > > +               ySum += (hist.total() - numUnsaturated) * hist.bins();\n> > > +               return ySum / hist.total() / hist.bins();\n> > > +       }\n> > > +\n> > > +       ASSERT(weights.size() == stats->agcRegions.numRegions());\n> > > +\n> > > +       /*\n> > > +        * Note that the weights are applied by the IPA to the statistics directly,\n> > > +        * before they are given to us here.\n> > > +        */\n> > > +       double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> >\n> > FAILED: src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o\n> > clang++-13 -Isrc/ipa/rpi/controller/librpi_ipa_controller.a.p -Isrc/ipa/rpi/controller -I../../../src/libcamera/src/ipa/rpi/controller -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-13/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -MF src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o.d -o src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -c ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp\n> > ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp:675:29: error: variable 'bSum' set but not used [-Werror,-Wunused-but-set-variable]\n> >         double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> >                                    ^\n> > 1 error generated.\n> >\n> > I think we can just remove this bSum = 0, local var.\n> >\n> > Likely no need for a resend.\n> >\n> >\n> > > +       for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {\n> > > +               auto &region = stats->agcRegions.get(i);\n> > > +               rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);\n> > > +               gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);\n> > > +               bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);\n>\n> David,\n>\n> We accumulate the bSum here - but don't then use it. Do you foresee\n> needing this bSum anywhere else? or is this safe to drop?\n>\n>\n> > > +               pixelSum += region.counted;\n> > > +       }\n> > > +       if (pixelSum == 0.0) {\n> > > +               LOG(RPiAgc, Warning) << \"computeInitialY: pixelSum is zero\";\n> > > +               return 0;\n> > > +       }\n> > > +\n> > > +       double ySum;\n> > > +       /* Factor in the AWB correction if needed. */\n> > > +       if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) {\n> > > +               ySum = rSum * awb.gainR * .299 +\n> > > +                      gSum * awb.gainG * .587 +\n> > > +                      gSum * awb.gainB * .114;\n> > > +       } else\n> > > +               ySum = rSum * .299 + gSum * .587 + gSum * .114;\n>\n> Or ... is the fault here, that it should be bSum * .114 ? (Looks quite\n> likely given the block above it).\n>\n>\n> --\n> Kieran\n>\n>\n> > > +\n> > > +       return ySum / pixelSum / (1 << 16);\n> > > +}\n> > > +","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C39DEBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Sep 2023 14:39:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 81C5C62916;\n\tFri, 15 Sep 2023 16:39:22 +0200 (CEST)","from mail-vs1-xe31.google.com (mail-vs1-xe31.google.com\n\t[IPv6:2607:f8b0:4864:20::e31])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 255F062911\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 16:39:20 +0200 (CEST)","by mail-vs1-xe31.google.com with SMTP id\n\tada2fe7eead31-450f68feee2so822971137.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 07:39:20 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694788762;\n\tbh=kdYv1gAjGr8ivZnPJ9wIU/GhX7uQqvp67rQqpIcdDmI=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=H0E5nqJm2Oq+HU+3vlT3l+0ur1o0gMqyypzrFoypmX9D9HfAUO039TaVkxwCzXUz8\n\tVolTiNP9ahV8DaHy/BPI/lsha2wsN3S03TNW11jg/8E5/k9Hk+Bk/D4L2LKyfYCLUq\n\tJo71w0m2uoeK4+OGvOLP2rhFXN07i1dt2cDxqOm8AaMUFvfYUe7iE+4vnIVw5gYXF4\n\tlSQDcllky494VPWvqpq2wM1W9iKn8Z/RRE6FLTc6shbHT7tRw117GCGJ8b+3+D3ZeW\n\trCuWvEL3zXBYfuiluZD/xuYrL0mCNqaA8xSIxjKibaSEbs7qfYIiA+HcGOOiwP3014\n\tSBpZZ1yTAXl5A==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1694788759; x=1695393559;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=hTTUTUTYi5xvyQNw7LcoMcKZ0Nb/h1r21IsTudgXzRM=;\n\tb=kndqYkV+zO9juK9kBgtPysW9CPMQek5YhmUedI6SxTY071Jdy4aEwr7e//RCJjmJw6\n\tzN7bZy4BfoYf9dHfEAXeZJL7NjBZTYVHoMq5tc4qkkFQGjT9ARA5GqPdt/QxglWAoY81\n\tQNzBMSo/luukMyV52bNHp19tlFLJBNqAB3lH0E0sv4FMYDiX9KHSyIlzyRlQ+lATw8ht\n\tI9o0KBcLHpnvsgqA5zOgvtaT25ORoB1MIXvvUL2hjGMOVYkmF0E0VF6w+mCsGmQWqcj3\n\twwx0EnM+43V4pMZ2u0YAzFU/EY1VXDHPp1PraB8pLRybaccyS4Qw+Zt5PCLGqLSrrcSl\n\tBSvA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"kndqYkV+\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1694788759; x=1695393559;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=hTTUTUTYi5xvyQNw7LcoMcKZ0Nb/h1r21IsTudgXzRM=;\n\tb=IWYGyc69rterpZnSReBCRjBjyG85DdX+E9kVsQgohTYvcW0oGNxwVT6o7+QHOO8WHO\n\tDPS/67qk2Op3KhHkiOyO9EJ6SMsBLgsFtwDmGXlT2lnszBXytHuEt1HSfZZq5aMQlwzm\n\tm/PW9mPDVxaxa454s9CMQDs96hcNrguU2uKxDKvuXkFp1bJhHfRdva6VlQZ+nszP/LPz\n\tTMnHZ7p45CadPHMHl8hqkX0Yg1h/+RwurGqthO1IGSyuU2ZtBb4IgsvhgDBbg4I9ws8U\n\tMAC7YWjgBzFufbK2bmawRM4b+uYa3xMIe8/DcE3R8izPtcY4vYKNKVtVK2xSyDDg5yNu\n\tJqSg==","X-Gm-Message-State":"AOJu0Yxgi0luH2xgODlB3mW57aPEHzky9di6/WHdgMidi12msMHZL6rd\n\tc5jP5490uCtMuK7BJM7k42DRMh+8t3+EHfDxQxTMFd2Q47OOrnBc","X-Google-Smtp-Source":"AGHT+IFc5ESAdDxMDbvwihiKSeKzwkLQgFfiGpywPRTyRgZF24/qmttrDozyChXXS9rVEgjiGescXxZxne7AFwlyRF0=","X-Received":"by 2002:a67:ee41:0:b0:450:bd08:e168 with SMTP id\n\tg1-20020a67ee41000000b00450bd08e168mr2049223vsp.13.1694788758940;\n\tFri, 15 Sep 2023 07:39:18 -0700 (PDT)","MIME-Version":"1.0","References":"<20230915122954.5231-1-david.plowman@raspberrypi.com>\n\t<20230915122954.5231-3-david.plowman@raspberrypi.com>\n\t<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>\n\t<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>","In-Reply-To":"<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>","Date":"Fri, 15 Sep 2023 15:39:07 +0100","Message-ID":"<CAHW6GYJ60r3twDS9BLt_ehfkO1we+c=73376MXqhQeOOf4U2Dw@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","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>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27791,"web_url":"https://patchwork.libcamera.org/comment/27791/","msgid":"<g46iv66adpbhkqbftaarboq4erbwngcdpmevrlzdhp5qltepti@jawxoqvaghq7>","date":"2023-09-15T14:40:24","subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi\n\nOn Fri, Sep 15, 2023 at 03:38:33PM +0100, Kieran Bingham via libcamera-devel wrote:\n> Quoting Kieran Bingham (2023-09-15 15:36:53)\n> > Quoting Kieran Bingham (2023-09-15 15:29:35)\n> > > Quoting David Plowman via libcamera-devel (2023-09-15 13:29:51)\n> > > > This commit does the basic reorganisation of the code in order to\n> > > > implement multi-channel AGC. The main changes are:\n> > > >\n> > > > * The previous Agc class (in agc.cpp) has become the AgcChannel class\n> > > >   in (agc_channel.cpp).\n> > > >\n> > > > * A new Agc class is introduced which is a wrapper round a number of\n> > > >   AgcChannels.\n> > > >\n> > > > * The basic plumbing from ipa_base.cpp to Agc is updated to include a\n> > > >   channel number. All the existing controls are hardwired to talk\n> > > >   directly to channel 0.\n> > > >\n> > > > There are a couple of limitations which we expect to apply to\n> > > > multi-channel AGC. We're not allowing different frame durations to be\n> > > > applied to the channels, nor are we allowing separate metering\n> > > > modes. To be fair, supporting these things is not impossible, but\n> > > > there are reasons why it may be tricky so they remain \"TBD\" for now.\n> > > >\n> > > > This patch only includes the basic reorganisation and plumbing. It\n> > > > does not yet update the important methods (switchMode, prepare and\n> > > > process) to implement multi-channel AGC properly. This will appear in\n> > > > a subsequent commit. For now, these functions are hard-coded just to\n> > > > use channel 0, thereby preserving the existing behaviour.\n> > > >\n> > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n> > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > > ---\n> > >\n> > > <snippity>\n> > >\n> > > > +static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n> > > > +                             std::vector<double> &weights, double gain)\n> > > > +{\n> > > > +       constexpr uint64_t maxVal = 1 << Statistics::NormalisationFactorPow2;\n> > > > +\n> > > > +       /*\n> > > > +        * If we have no AGC region stats, but do have a a Y histogram, use that\n> > > > +        * directly to caluclate the mean Y value of the image.\n> > > > +        */\n> > > > +       if (!stats->agcRegions.numRegions() && stats->yHist.bins()) {\n> > > > +               /*\n> > > > +                * When the gain is applied to the histogram, anything below minBin\n> > > > +                * will scale up directly with the gain, but anything above that\n> > > > +                * will saturate into the top bin.\n> > > > +                */\n> > > > +               auto &hist = stats->yHist;\n> > > > +               double minBin = std::min(1.0, 1.0 / gain) * hist.bins();\n> > > > +               double binMean = hist.interBinMean(0.0, minBin);\n> > > > +               double numUnsaturated = hist.cumulativeFreq(minBin);\n> > > > +               /* This term is from all the pixels that won't saturate. */\n> > > > +               double ySum = binMean * gain * numUnsaturated;\n> > > > +               /* And add the ones that will saturate. */\n> > > > +               ySum += (hist.total() - numUnsaturated) * hist.bins();\n> > > > +               return ySum / hist.total() / hist.bins();\n> > > > +       }\n> > > > +\n> > > > +       ASSERT(weights.size() == stats->agcRegions.numRegions());\n> > > > +\n> > > > +       /*\n> > > > +        * Note that the weights are applied by the IPA to the statistics directly,\n> > > > +        * before they are given to us here.\n> > > > +        */\n> > > > +       double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> > >\n> > > FAILED: src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o\n> > > clang++-13 -Isrc/ipa/rpi/controller/librpi_ipa_controller.a.p -Isrc/ipa/rpi/controller -I../../../src/libcamera/src/ipa/rpi/controller -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-13/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -MF src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o.d -o src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -c ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp\n> > > ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp:675:29: error: variable 'bSum' set but not used [-Werror,-Wunused-but-set-variable]\n> > >         double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> > >                                    ^\n> > > 1 error generated.\n> > >\n> > > I think we can just remove this bSum = 0, local var.\n> > >\n> > > Likely no need for a resend.\n> > >\n> > >\n> > > > +       for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {\n> > > > +               auto &region = stats->agcRegions.get(i);\n> > > > +               rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);\n> > > > +               gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);\n> > > > +               bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);\n> >\n> > David,\n> >\n> > We accumulate the bSum here - but don't then use it. Do you foresee\n> > needing this bSum anywhere else? or is this safe to drop?\n> >\n> >\n> > > > +               pixelSum += region.counted;\n> > > > +       }\n> > > > +       if (pixelSum == 0.0) {\n> > > > +               LOG(RPiAgc, Warning) << \"computeInitialY: pixelSum is zero\";\n> > > > +               return 0;\n> > > > +       }\n> > > > +\n> > > > +       double ySum;\n> > > > +       /* Factor in the AWB correction if needed. */\n> > > > +       if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) {\n> > > > +               ySum = rSum * awb.gainR * .299 +\n> > > > +                      gSum * awb.gainG * .587 +\n> > > > +                      gSum * awb.gainB * .114;\n>\n> Eeek - and should this be:\n> \tbSum * awb.gainB * .114; ?\n>\n\nThe original code was\n\n-       double ySum = rSum * awb.gainR * .299 +\n-                     gSum * awb.gainG * .587 +\n-                     bSum * awb.gainB * .114;\n\n> > > > +       } else\n> > > > +               ySum = rSum * .299 + gSum * .587 + gSum * .114;\n> >\n> > Or ... is the fault here, that it should be bSum * .114 ? (Looks quite\n> > likely given the block above it).\n> >\n> >\n> > --\n> > Kieran\n> >\n> >\n> > > > +\n> > > > +       return ySum / pixelSum / (1 << 16);\n> > > > +}\n> > > > +","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 43FD0BD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Sep 2023 14:40:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0442462919;\n\tFri, 15 Sep 2023 16:40:29 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0D5FD62911\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 16:40:28 +0200 (CEST)","from ideasonboard.com (93-46-82-201.ip106.fastwebnet.it\n\t[93.46.82.201])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8AC7F9A8;\n\tFri, 15 Sep 2023 16:38:54 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694788829;\n\tbh=7C6Kw/w21mpLbajbCF0n8ytzDvXSCR/PsIRTXaXrYL4=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=DT48R25hSBpBAF21PoTUtk0kYB8kWqRpBPrDxraQ3I1D+VtLG+7Gd1pvBru0LoCj9\n\t+ur1zj6m/qPdSbu3jXg7AcQZjzr3U+mh3LUd3lrP35/K/71u1IgfNUORIoeyiA4z/L\n\tJ9ZoePvBhpDrEIt+cCiUR1YIcT/2WCIII3w4HkIQT1O6CnS27m0LLEQhiNUu4XTPfd\n\tvRTwdr/o5UZyVNXhJKPdo0SYB4pYn4FPVez063sKv5dUPXQAIAiUSBL0Ez2pEjuF7a\n\tfD46T8Yr7426+7eq3WhV15Ic+5Lsi4JpYdG4T5hgFLIzeEvPXAgSVTq3+XyLxX921I\n\tEYR/Fq0aAvQpQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694788734;\n\tbh=7C6Kw/w21mpLbajbCF0n8ytzDvXSCR/PsIRTXaXrYL4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DvXfdwt1veArklIrROjhDonFCc66BOAQIfnF02Pbl5jnbXxl39Z87BYEdDIZnyoaQ\n\t/ihjlO1q/FCqFUlPwCgx/MIp73eCu1iu2m2SyUDRHrI/itWK/1bwMD+HPFp9Eedw16\n\t6ERbt/Pcyj7cOSV0AOw777rA6J9eyAEWMZVK837k="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"DvXfdwt1\"; dkim-atps=neutral","Date":"Fri, 15 Sep 2023 16:40:24 +0200","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<g46iv66adpbhkqbftaarboq4erbwngcdpmevrlzdhp5qltepti@jawxoqvaghq7>","References":"<20230915122954.5231-1-david.plowman@raspberrypi.com>\n\t<20230915122954.5231-3-david.plowman@raspberrypi.com>\n\t<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>\n\t<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>\n\t<169478871382.2427060.4183094997043561165@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<169478871382.2427060.4183094997043561165@ping.linuxembedded.co.uk>","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27792,"web_url":"https://patchwork.libcamera.org/comment/27792/","msgid":"<CAHW6GYJod_CfHAkCTJ9sUYRkG-u9X1CofsEJxZf_Hj9CEC6PfQ@mail.gmail.com>","date":"2023-09-15T14:43:15","subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Indeed, I think I just moved the code so the problem has presumably\nbeen there a little while. So probably best to submit a new patch to\nfix it? Good thing Kieran's compiler was suitably picky...\n\nDavid\n\nOn Fri, 15 Sept 2023 at 15:40, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n> Hi\n>\n> On Fri, Sep 15, 2023 at 03:38:33PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > Quoting Kieran Bingham (2023-09-15 15:36:53)\n> > > Quoting Kieran Bingham (2023-09-15 15:29:35)\n> > > > Quoting David Plowman via libcamera-devel (2023-09-15 13:29:51)\n> > > > > This commit does the basic reorganisation of the code in order to\n> > > > > implement multi-channel AGC. The main changes are:\n> > > > >\n> > > > > * The previous Agc class (in agc.cpp) has become the AgcChannel class\n> > > > >   in (agc_channel.cpp).\n> > > > >\n> > > > > * A new Agc class is introduced which is a wrapper round a number of\n> > > > >   AgcChannels.\n> > > > >\n> > > > > * The basic plumbing from ipa_base.cpp to Agc is updated to include a\n> > > > >   channel number. All the existing controls are hardwired to talk\n> > > > >   directly to channel 0.\n> > > > >\n> > > > > There are a couple of limitations which we expect to apply to\n> > > > > multi-channel AGC. We're not allowing different frame durations to be\n> > > > > applied to the channels, nor are we allowing separate metering\n> > > > > modes. To be fair, supporting these things is not impossible, but\n> > > > > there are reasons why it may be tricky so they remain \"TBD\" for now.\n> > > > >\n> > > > > This patch only includes the basic reorganisation and plumbing. It\n> > > > > does not yet update the important methods (switchMode, prepare and\n> > > > > process) to implement multi-channel AGC properly. This will appear in\n> > > > > a subsequent commit. For now, these functions are hard-coded just to\n> > > > > use channel 0, thereby preserving the existing behaviour.\n> > > > >\n> > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n> > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > > > ---\n> > > >\n> > > > <snippity>\n> > > >\n> > > > > +static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb,\n> > > > > +                             std::vector<double> &weights, double gain)\n> > > > > +{\n> > > > > +       constexpr uint64_t maxVal = 1 << Statistics::NormalisationFactorPow2;\n> > > > > +\n> > > > > +       /*\n> > > > > +        * If we have no AGC region stats, but do have a a Y histogram, use that\n> > > > > +        * directly to caluclate the mean Y value of the image.\n> > > > > +        */\n> > > > > +       if (!stats->agcRegions.numRegions() && stats->yHist.bins()) {\n> > > > > +               /*\n> > > > > +                * When the gain is applied to the histogram, anything below minBin\n> > > > > +                * will scale up directly with the gain, but anything above that\n> > > > > +                * will saturate into the top bin.\n> > > > > +                */\n> > > > > +               auto &hist = stats->yHist;\n> > > > > +               double minBin = std::min(1.0, 1.0 / gain) * hist.bins();\n> > > > > +               double binMean = hist.interBinMean(0.0, minBin);\n> > > > > +               double numUnsaturated = hist.cumulativeFreq(minBin);\n> > > > > +               /* This term is from all the pixels that won't saturate. */\n> > > > > +               double ySum = binMean * gain * numUnsaturated;\n> > > > > +               /* And add the ones that will saturate. */\n> > > > > +               ySum += (hist.total() - numUnsaturated) * hist.bins();\n> > > > > +               return ySum / hist.total() / hist.bins();\n> > > > > +       }\n> > > > > +\n> > > > > +       ASSERT(weights.size() == stats->agcRegions.numRegions());\n> > > > > +\n> > > > > +       /*\n> > > > > +        * Note that the weights are applied by the IPA to the statistics directly,\n> > > > > +        * before they are given to us here.\n> > > > > +        */\n> > > > > +       double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> > > >\n> > > > FAILED: src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o\n> > > > clang++-13 -Isrc/ipa/rpi/controller/librpi_ipa_controller.a.p -Isrc/ipa/rpi/controller -I../../../src/libcamera/src/ipa/rpi/controller -Iinclude -I../../../src/libcamera/include -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O0 -g -Wextra-semi -Wthread-safety -Wshadow -include /home/kbingham/iob/libcamera/ci/integrator/builds/build-matrix/clang-13/config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -MF src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o.d -o src/ipa/rpi/controller/librpi_ipa_controller.a.p/rpi_agc_channel.cpp.o -c ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp\n> > > > ../../../src/libcamera/src/ipa/rpi/controller/rpi/agc_channel.cpp:675:29: error: variable 'bSum' set but not used [-Werror,-Wunused-but-set-variable]\n> > > >         double rSum = 0, gSum = 0, bSum = 0, pixelSum = 0;\n> > > >                                    ^\n> > > > 1 error generated.\n> > > >\n> > > > I think we can just remove this bSum = 0, local var.\n> > > >\n> > > > Likely no need for a resend.\n> > > >\n> > > >\n> > > > > +       for (unsigned int i = 0; i < stats->agcRegions.numRegions(); i++) {\n> > > > > +               auto &region = stats->agcRegions.get(i);\n> > > > > +               rSum += std::min<double>(region.val.rSum * gain, (maxVal - 1) * region.counted);\n> > > > > +               gSum += std::min<double>(region.val.gSum * gain, (maxVal - 1) * region.counted);\n> > > > > +               bSum += std::min<double>(region.val.bSum * gain, (maxVal - 1) * region.counted);\n> > >\n> > > David,\n> > >\n> > > We accumulate the bSum here - but don't then use it. Do you foresee\n> > > needing this bSum anywhere else? or is this safe to drop?\n> > >\n> > >\n> > > > > +               pixelSum += region.counted;\n> > > > > +       }\n> > > > > +       if (pixelSum == 0.0) {\n> > > > > +               LOG(RPiAgc, Warning) << \"computeInitialY: pixelSum is zero\";\n> > > > > +               return 0;\n> > > > > +       }\n> > > > > +\n> > > > > +       double ySum;\n> > > > > +       /* Factor in the AWB correction if needed. */\n> > > > > +       if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) {\n> > > > > +               ySum = rSum * awb.gainR * .299 +\n> > > > > +                      gSum * awb.gainG * .587 +\n> > > > > +                      gSum * awb.gainB * .114;\n> >\n> > Eeek - and should this be:\n> >       bSum * awb.gainB * .114; ?\n> >\n>\n> The original code was\n>\n> -       double ySum = rSum * awb.gainR * .299 +\n> -                     gSum * awb.gainG * .587 +\n> -                     bSum * awb.gainB * .114;\n>\n> > > > > +       } else\n> > > > > +               ySum = rSum * .299 + gSum * .587 + gSum * .114;\n> > >\n> > > Or ... is the fault here, that it should be bSum * .114 ? (Looks quite\n> > > likely given the block above it).\n> > >\n> > >\n> > > --\n> > > Kieran\n> > >\n> > >\n> > > > > +\n> > > > > +       return ySum / pixelSum / (1 << 16);\n> > > > > +}\n> > > > > +","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5566EBE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Sep 2023 14:43:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD56062919;\n\tFri, 15 Sep 2023 16:43:29 +0200 (CEST)","from mail-oo1-xc29.google.com (mail-oo1-xc29.google.com\n\t[IPv6:2607:f8b0:4864:20::c29])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1373162911\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 16:43:28 +0200 (CEST)","by mail-oo1-xc29.google.com with SMTP id\n\t006d021491bc7-576918d0a42so1200925eaf.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 07:43:27 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694789009;\n\tbh=LQdoVbDxt/J1QqAJgJ7lY7uNU+jZOTvGTIG1V/Wpt8c=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=MOkOiVSxwq5MQ4ii0RC1i3y2n0Q2Mg/J1DrBOtzf+WKfCHvKRzJ3EYW/AMMfUj7EH\n\tBn84JGJoDxZP8hj4LD+1idYhJ5d3zQR/x3qqrB/Wfw4pCdQxdYNdKeBwtb5NTXlboe\n\tEIqsPHNtyTockTXfY7VxLaFvIeDCEa/kt0I/098ohMrSH+AGdx4EGoRjUAiA09xTyZ\n\tGnuKoo+mRiQDoMTdklInozvaHZcE6UzKpzwd0z+KS0OCGnuVPcxuqinXZsQJKEaK9C\n\tiwbqdxT2hR/qYxjwZcciqIqPAQcnltfMxaNoNCmpcHW9tw+iaN1Wjybv0Fkf7L/jWG\n\tpbkskmmk8B+cw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1694789007; x=1695393807;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=0TfhAKD6oAy62iA629YzPApAbLfM7au6G7pD/OGL8Js=;\n\tb=FK2syvkGG9Vdpo1r7VXfPjUnIxGqtiPRfpCye2wPLfEswydEAhwEhrlcYThg3R9G+Z\n\tviN/GBTpnWMSUkwpG8XDf6Rji17ZnZ+cAbxsseKuVgkJnWJ8gu3MIae45JPFU4KFUWCg\n\tGx4ogdo+f0dmztMXqFqFho63TAF2u8Uhw2554N0JWgWD0oeeFKx8kTcyvBcNV0rc0a1A\n\tRO6+BeETmhW/Lne8KU+gonD2nllDt7ZGfENbYrpKDPLDY7FgDZP7tWaBg9Ur2cclISPp\n\tfvjHZyOAL8lBs8vtBqIBf8cfsgn6pxEIqyFtp4bHh+hUbUFNJ0TXXKLrJbr1cJhwStuA\n\tVhog=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"FK2syvkG\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1694789007; x=1695393807;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=0TfhAKD6oAy62iA629YzPApAbLfM7au6G7pD/OGL8Js=;\n\tb=Bl2PhKr5EpirCuUQaLvDIcgk95oTyR/q7yM77BuL1b7rGwMCuA9TdDa/FWLYhAw9qo\n\tahfvrFWf/+UG706ggxUIxB9zQgzBVE/ErWruqOqKnPMZsJGIb/eSP/q8tzRusLyecQ/Y\n\tFyrB/k6Ue+Q9SLrOL398M/gLDUalmFsHQYJHjKO0HWrHsNeXYuFdo167txVx5guERJXF\n\towE5lfJ0HLlIQRqk/JINheqVynX2jbvgo/qQZA73OE2v/SQBOKIRi6HaYru6xzc/YpAe\n\t6NE+wYalIIPdYLBt/CO3BQ2ZkpE1orw2QPo1fKDaSXcM/4pB78oZE7v3sYOPTlFZyz+J\n\tWtWw==","X-Gm-Message-State":"AOJu0YwamBsOZq3r0RApqjLX9H9JcHQrqc28dm+H5S4dRPjlmZP+zhCy\n\t+LRjhZ9cH8dIa8sjUJJTDLswbJoCghV7pVYNMcBmtg==","X-Google-Smtp-Source":"AGHT+IF5QyLsDTnG/ws+2nwxs0JnUEgZU5hdJQoZixN5XAdbmqIfTybjQ5fwmCOACW/Jo/lm7InpB5X2i9LHItSifDw=","X-Received":"by 2002:a05:6358:2624:b0:129:c9c0:ca64 with SMTP id\n\tl36-20020a056358262400b00129c9c0ca64mr2156006rwc.15.1694789006671;\n\tFri, 15 Sep 2023 07:43:26 -0700 (PDT)","MIME-Version":"1.0","References":"<20230915122954.5231-1-david.plowman@raspberrypi.com>\n\t<20230915122954.5231-3-david.plowman@raspberrypi.com>\n\t<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>\n\t<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>\n\t<169478871382.2427060.4183094997043561165@ping.linuxembedded.co.uk>\n\t<g46iv66adpbhkqbftaarboq4erbwngcdpmevrlzdhp5qltepti@jawxoqvaghq7>","In-Reply-To":"<g46iv66adpbhkqbftaarboq4erbwngcdpmevrlzdhp5qltepti@jawxoqvaghq7>","Date":"Fri, 15 Sep 2023 15:43:15 +0100","Message-ID":"<CAHW6GYJod_CfHAkCTJ9sUYRkG-u9X1CofsEJxZf_Hj9CEC6PfQ@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","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>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27793,"web_url":"https://patchwork.libcamera.org/comment/27793/","msgid":"<169478960202.2427060.15187485675779863768@ping.linuxembedded.co.uk>","date":"2023-09-15T14:53:22","subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2023-09-15 15:43:15)\n> Indeed, I think I just moved the code so the problem has presumably\n> been there a little while. So probably best to submit a new patch to\n> fix it? Good thing Kieran's compiler was suitably picky...\n> \n\nThis is why I'm a fan of -Wall,-Werror - even if lots of people\nhate it ;-)\n\nI really should try to fix the coverity checker sometime too. It stopped\nworking and I never did repair it....\n\n\n--\nKieran","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 E52A9BD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Sep 2023 14:53:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 39645628EC;\n\tFri, 15 Sep 2023 16:53:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B1605628EC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 16:53:24 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3410D9A8;\n\tFri, 15 Sep 2023 16:51:51 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694789606;\n\tbh=R6ACVPd5vbBzr+16HVxc369gtTF+MmQaQplNYwH7rgs=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=wT7vB1TckzuMcDpsxidjqlFSvgQRFIBQ72/qMrxScMxAx1T3IlAm8z2Q81kVh0o7C\n\tV9etUOEx4oI+AvJdtQTxt2VYOYVIrpjOF1CzqZ/RRnlLCmjHTo7JJ26/6iVFdxgegm\n\tk9MLXRUflnx50wqFBuoqP0Ucmn3IbD00L6s3sDbMh6s0NsWS0xNB3r59Rirx4myBuM\n\tilg7e1l7VUsJOc62Dq7BhqvAI85RrOFMg1/Ilmlut1i3Xx+QcKD6oa3emWFJJ66Jwn\n\t2TJKewckS/JFoWq6gDasYfz8s9AQ+Fz695aP49fFXASfmZwNRdwz2yzP0hWAjQbQ5i\n\t9f1Vi6I5gukYw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694789511;\n\tbh=R6ACVPd5vbBzr+16HVxc369gtTF+MmQaQplNYwH7rgs=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=b+TSkIcoucuqpHeS9uEKb5lBEOoksvfN/LnlvTX/I6nc8NFUF6L4YrqjfFiaxRqyQ\n\tSg8iSQeQtEPX3ftBoTBPB5PbEx6Tzq18jKZRXpxssZIlHP5/5OVdrnmcKtED4KV6Hz\n\tpwNldyRiI1KXlhWH6Sc9N/1mBnP92GBVkXlZh4Cw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"b+TSkIco\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<CAHW6GYJod_CfHAkCTJ9sUYRkG-u9X1CofsEJxZf_Hj9CEC6PfQ@mail.gmail.com>","References":"<20230915122954.5231-1-david.plowman@raspberrypi.com>\n\t<20230915122954.5231-3-david.plowman@raspberrypi.com>\n\t<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>\n\t<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>\n\t<169478871382.2427060.4183094997043561165@ping.linuxembedded.co.uk>\n\t<g46iv66adpbhkqbftaarboq4erbwngcdpmevrlzdhp5qltepti@jawxoqvaghq7>\n\t<CAHW6GYJod_CfHAkCTJ9sUYRkG-u9X1CofsEJxZf_Hj9CEC6PfQ@mail.gmail.com>","To":"David Plowman <david.plowman@raspberrypi.com>,\n\tJacopo Mondi <jacopo.mondi@ideasonboard.com>","Date":"Fri, 15 Sep 2023 15:53:22 +0100","Message-ID":"<169478960202.2427060.15187485675779863768@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27794,"web_url":"https://patchwork.libcamera.org/comment/27794/","msgid":"<CAHW6GY+Z9TdSyeYTqo1epcxaw0aK+5Vq=StvxWAAGT0xhpLP3A@mail.gmail.com>","date":"2023-09-15T14:58:40","subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"I thought we did have -Wall, -Werror set. If not, how would I set that?\n\nAnyway, would you rather have an extra patch in this set, or a lone\npatch that can be applied after?\n\nDavid\n\nOn Fri, 15 Sept 2023 at 15:53, Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Quoting David Plowman (2023-09-15 15:43:15)\n> > Indeed, I think I just moved the code so the problem has presumably\n> > been there a little while. So probably best to submit a new patch to\n> > fix it? Good thing Kieran's compiler was suitably picky...\n> >\n>\n> This is why I'm a fan of -Wall,-Werror - even if lots of people\n> hate it ;-)\n>\n> I really should try to fix the coverity checker sometime too. It stopped\n> working and I never did repair it....\n>\n>\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 AF624BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Sep 2023 14:58:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 28CB262916;\n\tFri, 15 Sep 2023 16:58:55 +0200 (CEST)","from mail-qv1-xf2b.google.com (mail-qv1-xf2b.google.com\n\t[IPv6:2607:f8b0:4864:20::f2b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 291C062911\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 16:58:53 +0200 (CEST)","by mail-qv1-xf2b.google.com with SMTP id\n\t6a1803df08f44-656307a52e8so6308186d6.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 07:58:53 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694789935;\n\tbh=pKtbYzWXAlwemJGwJ/Dt24QVwvpv8Nxzl0baT1C39Xw=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=MI38iE0EOCacLKiu/GXXN6XdUSoLSy3DuGy2yQBHuJ/sEqcxCgVkTs2gSHqHhhxa7\n\tJGqSWol1nDpfFpLxj4l2CgJoag+JmtK0BH9uBxRQduyATNMJnpNUDnrikiPeC9LCdT\n\tjc1+tvo2bkRF6BQeaEllB42rV2OcOi7aH1tLtGJmbbZdBrEQM3aaWg0Z84aEvX0lBn\n\tC5rF0hLzP0ASsuOib6RuOO+cVK+5KclQb7TidBzzBPdUITuRVmwpTqcpaR+F4wlWXM\n\tLpYlEJU///vSJe+qk2DFqpslHpbAmbLpPPsViMw1Jn3FGrtXZPkogELpVFRcPfzA6N\n\tmpEu0yfVOCwtA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1694789932; x=1695394732;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=pKtbYzWXAlwemJGwJ/Dt24QVwvpv8Nxzl0baT1C39Xw=;\n\tb=L1jQ1yj/cF2SvvNzskafGkgIsK3rn+gfFYV6bTXK0Z+WeKJJruWMrF0GUQCFuFQLaU\n\tf7VcvOxyFmL1x74dNbbfxrHJKqPHeM3+dKfcyuOD1QaggcwtJpkAeKUtByPRDZPXNLTX\n\t5a+1YAyF73O6Y94zhjoSXmRDmlURNztr6M03LbQ/fIW3QQa+Llmg8SAUOTbeP9pJdQvM\n\t75jOtwXYyYmMnWn+1ihEpy14I+UcMekP1x68pXuocTlorYmCzdofoyXQpg6m+TLVH2Lv\n\tyQli9PZqeY3kaEfHL+1ne7plr1cmvMMpk1Y/5Zv9fks9jt8b4/aEJCXTqffVAyaGhmD+\n\tq5cQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"L1jQ1yj/\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1694789932; x=1695394732;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=pKtbYzWXAlwemJGwJ/Dt24QVwvpv8Nxzl0baT1C39Xw=;\n\tb=rVRN87yIIzauR1abAXviNNPWNWLJwQ2zqaz6PAwRVDUGa6WCR0VdP1z3TafppE+O/b\n\tUlqVhzP3vuC2IOVbn/SoWXSm2KjrleeXOcSe+n8XVgfiVULOvbBvTvY3fDGu8gI40Kyi\n\t9NZUuaRVmOUhi4NFhvyXMq1yP955HoIfzAcEc7vr7hlRjFOYJu5ZdvcTuD2HI03154db\n\tZ0gDv7FVGwqi75kzQ8NXwkgY8y+PWm1LZJr5SlCA4vuYnCcNzyhXkhCHW1eWkHP/8MdX\n\tjabe40591nMJfn2bAS/cyhccAuwbSkO8u5VJd5qBU3O6uGUUB0bDgkneN72e55SYJOGK\n\trUrA==","X-Gm-Message-State":"AOJu0YwjNvyKWTXmSrXrQx2nogPw1xQHWFjxMl6f8mQ00xJ4FE0nr/vs\n\tyNIC1Q57I/uf3wylEsWchvy1JAonsQ2mFcOaSpLA8KVRU5GsJDas","X-Google-Smtp-Source":"AGHT+IFc/hfitG+3cNscu0XA8BsvFBlvQyo1lD6XL7jReGV7dclVDfdSjYdkLlXYKQmKm4dbD8tAsXspgcTQjo1qlJQ=","X-Received":"by 2002:a0c:f5d2:0:b0:63d:6138:1027 with SMTP id\n\tq18-20020a0cf5d2000000b0063d61381027mr1851361qvm.42.1694789931901;\n\tFri, 15 Sep 2023 07:58:51 -0700 (PDT)","MIME-Version":"1.0","References":"<20230915122954.5231-1-david.plowman@raspberrypi.com>\n\t<20230915122954.5231-3-david.plowman@raspberrypi.com>\n\t<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>\n\t<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>\n\t<169478871382.2427060.4183094997043561165@ping.linuxembedded.co.uk>\n\t<g46iv66adpbhkqbftaarboq4erbwngcdpmevrlzdhp5qltepti@jawxoqvaghq7>\n\t<CAHW6GYJod_CfHAkCTJ9sUYRkG-u9X1CofsEJxZf_Hj9CEC6PfQ@mail.gmail.com>\n\t<169478960202.2427060.15187485675779863768@ping.linuxembedded.co.uk>","In-Reply-To":"<169478960202.2427060.15187485675779863768@ping.linuxembedded.co.uk>","Date":"Fri, 15 Sep 2023 15:58:40 +0100","Message-ID":"<CAHW6GY+Z9TdSyeYTqo1epcxaw0aK+5Vq=StvxWAAGT0xhpLP3A@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","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>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27795,"web_url":"https://patchwork.libcamera.org/comment/27795/","msgid":"<169479168855.2427060.16008096234821243962@ping.linuxembedded.co.uk>","date":"2023-09-15T15:28:08","subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2023-09-15 15:58:40)\n> I thought we did have -Wall, -Werror set. If not, how would I set that?\n> \n\nSorry for the confusion - I mean in general. libcamera sets werror in\nthe top of the meson.build project file.\n\nBut many people don't like failing to build because of warnings...\n\n> Anyway, would you rather have an extra patch in this set, or a lone\n> patch that can be applied after?\n\nI'm not sure - that depends on the patch?\n\nIs it just a fixup to this patch? or a fix before hand, and then a fix\nto this patch ?\n\nIt's better not to have a patch that introduces 'known bad code' - so\nsomething that fixes this patch would be best IMO.\n\nIs there more than just fixing the two locations in this patch ?\n\n--\nKieran\n\n> \n> David\n> \n> On Fri, 15 Sept 2023 at 15:53, Kieran Bingham\n> <kieran.bingham@ideasonboard.com> wrote:\n> >\n> > Quoting David Plowman (2023-09-15 15:43:15)\n> > > Indeed, I think I just moved the code so the problem has presumably\n> > > been there a little while. So probably best to submit a new patch to\n> > > fix it? Good thing Kieran's compiler was suitably picky...\n> > >\n> >\n> > This is why I'm a fan of -Wall,-Werror - even if lots of people\n> > hate it ;-)\n> >\n> > I really should try to fix the coverity checker sometime too. It stopped\n> > working and I never did repair it....\n> >\n> >\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 3D05CBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 15 Sep 2023 15:28:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A734B62918;\n\tFri, 15 Sep 2023 17:28:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 06EB7628EC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 15 Sep 2023 17:28:11 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 71E17B1;\n\tFri, 15 Sep 2023 17:26:37 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1694791691;\n\tbh=lHwlkzlCrX3HcI4iwpfnXJUHMCkbeMPNJYvo1MVSppk=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=nPOtwznLzu377UN7epTQJy/D4RJFfjr0bGO+8TsUovACpiG2dYHdwXH2/RGnw2Co9\n\tMTLfLUbcTXsiS2lsY5+5hvD3fsKHbgjFlQaO8qncYW+8RMrV9ZxGn96IqfR2Uts3Z5\n\tbtnKCYbCQx0c1B3mSpSoMW1bIUtmtblBN5BI5SmH+YwYlhHViXwh1c4wzmwrZAUSGC\n\t2tsd718erstYft5IVJfXpdiaYjUe+z+zdgTOyhmjc8YZL+tcX4eWWdc9DeHAi5iK7z\n\tEdoU6IbdyTyRO8g6mSE+ZCgYyq263/rWD/9msQtW1WN5AVHrk+eZVr/CV0690TCO1A\n\t+E2ydG9d7R2zA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1694791597;\n\tbh=lHwlkzlCrX3HcI4iwpfnXJUHMCkbeMPNJYvo1MVSppk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=p+CP5r75QvY4o3r5Cb750mLezk/YRzkjxj19Sy0KUnBbN81RNNyL5MD/yY+JskcEW\n\tZp949MEx8xnXZ3F5h6Bu7L37bilB6mb+rNtH3VBknkn2rjDpbzrlposufwLrrn5/wy\n\tGfSfnFSppEcZpwlDnzziTUb/ZxnKyKh7x9fTFZ9E="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"p+CP5r75\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<CAHW6GY+Z9TdSyeYTqo1epcxaw0aK+5Vq=StvxWAAGT0xhpLP3A@mail.gmail.com>","References":"<20230915122954.5231-1-david.plowman@raspberrypi.com>\n\t<20230915122954.5231-3-david.plowman@raspberrypi.com>\n\t<169478817579.2427060.11781180406604196200@ping.linuxembedded.co.uk>\n\t<169478861397.2427060.14765116356008519941@ping.linuxembedded.co.uk>\n\t<169478871382.2427060.4183094997043561165@ping.linuxembedded.co.uk>\n\t<g46iv66adpbhkqbftaarboq4erbwngcdpmevrlzdhp5qltepti@jawxoqvaghq7>\n\t<CAHW6GYJod_CfHAkCTJ9sUYRkG-u9X1CofsEJxZf_Hj9CEC6PfQ@mail.gmail.com>\n\t<169478960202.2427060.15187485675779863768@ping.linuxembedded.co.uk>\n\t<CAHW6GY+Z9TdSyeYTqo1epcxaw0aK+5Vq=StvxWAAGT0xhpLP3A@mail.gmail.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Date":"Fri, 15 Sep 2023 16:28:08 +0100","Message-ID":"<169479168855.2427060.16008096234821243962@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] ipa: rpi: agc: Reorganise code\n\tfor multi-channel AGC","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]