[{"id":29264,"web_url":"https://patchwork.libcamera.org/comment/29264/","msgid":"<84315077-bca9-43ff-bb78-ab3b0f935448@yoseli.org>","date":"2024-04-18T12:54:31","subject":"Re: [PATCH 3/3] ipa: ipu3: Use a Y histogram instead of green","submitter":{"id":127,"url":"https://patchwork.libcamera.org/api/people/127/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@yoseli.org"},"content":"Hi Daniel,\n\nOn 18/04/2024 14:46, Daniel Scally wrote:\n> The IPU3 IPA module uses a green histogram for the AGC algorithm's\n> constraint mode calculations. Switch instead to a luminance histogram\n> calculated using the Rec.601 coefficients.\n> \n> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> ---\n>   src/ipa/ipu3/algorithms/agc.cpp | 15 ++++++++-------\n>   1 file changed, 8 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> index a59b73fb..76d2bb60 100644\n> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> @@ -152,18 +152,19 @@ Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n>   \t\t\t\treinterpret_cast<const ipu3_uapi_awb_set_item *>(\n>   \t\t\t\t\t&stats->awb_raw_buffer.meta_data[cellPosition]);\n>   \n> -\t\t\trgbTriples_.push_back({\n> -\t\t\t\tcell->R_avg,\n> -\t\t\t\t(cell->Gr_avg + cell->Gb_avg) / 2,\n> -\t\t\t\tcell->B_avg\n> -\t\t\t});\n> +\t\t\tuint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n> +\n> +\t\t\trgbTriples_.push_back({cell->R_avg, G_avg, cell->B_avg});\n>   \n>   \t\t\t/*\n> -\t\t\t * Store the average green value to estimate the\n> +\t\t\t * Store the average luminance value to estimate the\n>   \t\t\t * brightness. Even the overexposed pixels are\n>   \t\t\t * taken into account.\n>   \t\t\t */\n> -\t\t\thist[(cell->Gr_avg + cell->Gb_avg) / 2]++;\n> +\t\t\tuint8_t y = (cell->R_avg * .299) +\n> +\t\t\t\t    (G_avg * .587) +\n> +\t\t\t\t    (cell->B_avg * .114);\n> +\t\t\thist[y]++;\n\nIs there a benefit to have the \"real\" Y value and not only the green \nparts (which reflect this Y value quite nicely as it is ~60% of the \nvalue) ? Could you measure it ? No increase in CPU time for big stats \ngrids ?\n\nJM","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 64D07C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 18 Apr 2024 12:54:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69D9D633F9;\n\tThu, 18 Apr 2024 14:54:34 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::224])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C8B96334D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 18 Apr 2024 14:54:32 +0200 (CEST)","by mail.gandi.net (Postfix) with ESMTPSA id 14988E000A;\n\tThu, 18 Apr 2024 12:54:31 +0000 (UTC)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=yoseli.org header.i=@yoseli.org\n\theader.b=\"a1YBFZBE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=yoseli.org; s=gm1;\n\tt=1713444872;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=FS+R8TqNcCyD8mGQOLcyNUaWCHEw0bjgLHd4Kt2j6Ik=;\n\tb=a1YBFZBEk3Rd9eo74fTtkRJ2CpRZiQVLGMZUNgYFMh3PdSdqJ4LmF4sC5I7Ksr6y4KrzRv\n\tZ/naaXhaKnubT/vYKQma+c5GYFzOEiNLv+4+XCQRle99JYYS+ux13VJm7LM03jtqA+rH6K\n\t5GzMjL1D12ZCenfkWWXIZAgyUXpuMgKSqtrMlpveo1uRtamoKqKR4Fb48/IdfgwvDb71Q5\n\tUmQf9dJISbi2D4R/rAhSlyQkomS+XfB+/11JKAvNvbtYtCXhHCmnJuK8vpgs5Rxd1XiXWZ\n\twurenO2QlRuH9mEDfVILQJaAM6t/WTffdVIiQ/r8NH7zEjl3OCS81lgohS8eiQ==","Message-ID":"<84315077-bca9-43ff-bb78-ab3b0f935448@yoseli.org>","Date":"Thu, 18 Apr 2024 14:54:31 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 3/3] ipa: ipu3: Use a Y histogram instead of green","To":"Daniel Scally <dan.scally@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240418124632.652163-1-dan.scally@ideasonboard.com>\n\t<20240418124632.652163-4-dan.scally@ideasonboard.com>","Content-Language":"en-US","From":"Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>","In-Reply-To":"<20240418124632.652163-4-dan.scally@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-GND-Sasl":"jeanmichel.hautbois@yoseli.org","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29423,"web_url":"https://patchwork.libcamera.org/comment/29423/","msgid":"<6a53b92f-0179-41c2-95fc-7c7fecb188df@ideasonboard.com>","date":"2024-05-04T19:58:57","subject":"Re: [PATCH 3/3] ipa: ipu3: Use a Y histogram instead of green","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jean-Michel - sorry for the delay replying to you\n\nOn 18/04/2024 13:54, Jean-Michel Hautbois wrote:\n> Hi Daniel,\n>\n> On 18/04/2024 14:46, Daniel Scally wrote:\n>> The IPU3 IPA module uses a green histogram for the AGC algorithm's\n>> constraint mode calculations. Switch instead to a luminance histogram\n>> calculated using the Rec.601 coefficients.\n>>\n>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n>> ---\n>>   src/ipa/ipu3/algorithms/agc.cpp | 15 ++++++++-------\n>>   1 file changed, 8 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n>> index a59b73fb..76d2bb60 100644\n>> --- a/src/ipa/ipu3/algorithms/agc.cpp\n>> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n>> @@ -152,18 +152,19 @@ Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n>>                   reinterpret_cast<const ipu3_uapi_awb_set_item *>(\n>> &stats->awb_raw_buffer.meta_data[cellPosition]);\n>>   -            rgbTriples_.push_back({\n>> -                cell->R_avg,\n>> -                (cell->Gr_avg + cell->Gb_avg) / 2,\n>> -                cell->B_avg\n>> -            });\n>> +            uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n>> +\n>> +            rgbTriples_.push_back({cell->R_avg, G_avg, cell->B_avg});\n>>                 /*\n>> -             * Store the average green value to estimate the\n>> +             * Store the average luminance value to estimate the\n>>                * brightness. Even the overexposed pixels are\n>>                * taken into account.\n>>                */\n>> -            hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;\n>> +            uint8_t y = (cell->R_avg * .299) +\n>> +                    (G_avg * .587) +\n>> +                    (cell->B_avg * .114);\n>> +            hist[y]++;\n>\n> Is there a benefit to have the \"real\" Y value and not only the green parts (which reflect this Y \n> value quite nicely as it is ~60% of the value) ?\n\n\nOnly that it more accurately matches the Y-value that's used in the rest of the algorithm; \nestimateLuminance() uses the same Rec.601 coefficients and the gain that's derived from that \ncalculation is compared against one derived from this Histogram.\n\n\n> Could you measure it ? No increase in CPU time for big stats grids ?\n\n\nI didn't check specifically but I can do - I didn't lose any framerate on my Surface Go2...but I \nimagine that the Imgu can run much faster than that is doing currently.\n\n\nCheers\n\nDan\n\n\n\n>\n> JM","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 54F1CC3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat,  4 May 2024 19:59:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 592AB604D4;\n\tSat,  4 May 2024 21:59:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 833A2604C3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  4 May 2024 21:59:01 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 50E6633D;\n\tSat,  4 May 2024 21:58:02 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"SKwyrYoA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1714852682;\n\tbh=JkMueTY7C9Jr3bQ5gPcCurCNl67Au9FG8qxHqM9M0M0=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=SKwyrYoAXZEWqQ/Bt2spuFA8wD6ftPleRV5fWdq6f7d3U5oWZ98KXXfrIfyTIzhxX\n\tDNyV0cXDXw3kKBBbWGYVmN8VS9j3VxqRmwfgzaYpLfBdQjiHS9R1pdbNC2nF/BFjYV\n\tGV/bQLy/FarvhMY/TrTY4I7BiaSyUm2GdvRss9fc=","Message-ID":"<6a53b92f-0179-41c2-95fc-7c7fecb188df@ideasonboard.com>","Date":"Sat, 4 May 2024 20:58:57 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 3/3] ipa: ipu3: Use a Y histogram instead of green","To":"Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240418124632.652163-1-dan.scally@ideasonboard.com>\n\t<20240418124632.652163-4-dan.scally@ideasonboard.com>\n\t<84315077-bca9-43ff-bb78-ab3b0f935448@yoseli.org>","Content-Language":"en-US","From":"Dan Scally <dan.scally@ideasonboard.com>","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<84315077-bca9-43ff-bb78-ab3b0f935448@yoseli.org>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29445,"web_url":"https://patchwork.libcamera.org/comment/29445/","msgid":"<171517275296.1857112.6716225758404197356@ping.linuxembedded.co.uk>","date":"2024-05-08T12:52:32","subject":"Re: [PATCH 3/3] ipa: ipu3: Use a Y histogram instead of green","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Dan Scally (2024-05-04 20:58:57)\n> Hi Jean-Michel - sorry for the delay replying to you\n> \n> On 18/04/2024 13:54, Jean-Michel Hautbois wrote:\n> > Hi Daniel,\n> >\n> > On 18/04/2024 14:46, Daniel Scally wrote:\n> >> The IPU3 IPA module uses a green histogram for the AGC algorithm's\n> >> constraint mode calculations. Switch instead to a luminance histogram\n> >> calculated using the Rec.601 coefficients.\n> >>\n> >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> >> ---\n> >>   src/ipa/ipu3/algorithms/agc.cpp | 15 ++++++++-------\n> >>   1 file changed, 8 insertions(+), 7 deletions(-)\n> >>\n> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> >> index a59b73fb..76d2bb60 100644\n> >> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> >> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> >> @@ -152,18 +152,19 @@ Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n> >>                   reinterpret_cast<const ipu3_uapi_awb_set_item *>(\n> >> &stats->awb_raw_buffer.meta_data[cellPosition]);\n> >>   -            rgbTriples_.push_back({\n> >> -                cell->R_avg,\n> >> -                (cell->Gr_avg + cell->Gb_avg) / 2,\n> >> -                cell->B_avg\n> >> -            });\n> >> +            uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n> >> +\n> >> +            rgbTriples_.push_back({cell->R_avg, G_avg, cell->B_avg});\n> >>                 /*\n> >> -             * Store the average green value to estimate the\n> >> +             * Store the average luminance value to estimate the\n> >>                * brightness. Even the overexposed pixels are\n> >>                * taken into account.\n> >>                */\n> >> -            hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;\n> >> +            uint8_t y = (cell->R_avg * .299) +\n> >> +                    (G_avg * .587) +\n> >> +                    (cell->B_avg * .114);\n\nSomewhere in libipa we should really have helpers or classes for the\ncolour space coefficients to make this more expressive.\n\n> >> +            hist[y]++;\n> >\n> > Is there a benefit to have the \"real\" Y value and not only the green\n> > parts (which reflect this Y value quite nicely as it is ~60% of the\n> > value) ?\n> \n> \n> Only that it more accurately matches the Y-value that's used in the\n> rest of the algorithm; estimateLuminance() uses the same Rec.601\n> coefficients and the gain that's derived from that calculation is\n> compared against one derived from this Histogram.\n\nPresumably I would get different results if I were in a Red or Green\nt-shirt ... so I certainly think it's more appropriate to use the\nluminance correctly.\n\n> > Could you measure it ? No increase in CPU time for big stats grids ?\n> \n> \n> I didn't check specifically but I can do - I didn't lose any framerate\n> on my Surface Go2...but I imagine that the Imgu can run much faster\n> than that is doing currently.\n\nI don't think I'm worried about doing the correct thing here. It's once\nper frame, on the stats - not per pixel in the image.\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> \n> \n> Cheers\n> \n> Dan\n> \n> \n> \n> >\n> > JM","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 4294CC3226\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  8 May 2024 12:52:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5239163437;\n\tWed,  8 May 2024 14:52: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 598EB61A73\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  8 May 2024 14:52:36 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4FB2A19FA;\n\tWed,  8 May 2024 14:52:33 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ollSVMJn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715172753;\n\tbh=qLEGaWHYtacWJx+Tq6NO8wREfG9rDuQi7ruU5Dh1sCk=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=ollSVMJngaAu1nLiscIDKLK40BQRN13txoz9aQjWh7CxR7VRTpsArI80l/GxGDEo8\n\t+czOLn82Kln6eznBiCRh11dhUcKFwWU2qKmLK00p1ms762WvASnQtPF0uWehs2KG9E\n\tB0pNr2X5DtEXMblErrPHn424bZWoms44misC5Mf4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<6a53b92f-0179-41c2-95fc-7c7fecb188df@ideasonboard.com>","References":"<20240418124632.652163-1-dan.scally@ideasonboard.com>\n\t<20240418124632.652163-4-dan.scally@ideasonboard.com>\n\t<84315077-bca9-43ff-bb78-ab3b0f935448@yoseli.org>\n\t<6a53b92f-0179-41c2-95fc-7c7fecb188df@ideasonboard.com>","Subject":"Re: [PATCH 3/3] ipa: ipu3: Use a Y histogram instead of green","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Dan Scally <dan.scally@ideasonboard.com>,\n\tJean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 08 May 2024 13:52:32 +0100","Message-ID":"<171517275296.1857112.6716225758404197356@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29451,"web_url":"https://patchwork.libcamera.org/comment/29451/","msgid":"<20240508140729.GM2012@pendragon.ideasonboard.com>","date":"2024-05-08T14:07:29","subject":"Re: [PATCH 3/3] ipa: ipu3: Use a Y histogram instead of green","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, May 08, 2024 at 01:52:32PM +0100, Kieran Bingham wrote:\n> Quoting Dan Scally (2024-05-04 20:58:57)\n> > On 18/04/2024 13:54, Jean-Michel Hautbois wrote:\n> > > On 18/04/2024 14:46, Daniel Scally wrote:\n> > >> The IPU3 IPA module uses a green histogram for the AGC algorithm's\n> > >> constraint mode calculations. Switch instead to a luminance histogram\n> > >> calculated using the Rec.601 coefficients.\n> > >>\n> > >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n> > >> ---\n> > >>   src/ipa/ipu3/algorithms/agc.cpp | 15 ++++++++-------\n> > >>   1 file changed, 8 insertions(+), 7 deletions(-)\n> > >>\n> > >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp\n> > >> index a59b73fb..76d2bb60 100644\n> > >> --- a/src/ipa/ipu3/algorithms/agc.cpp\n> > >> +++ b/src/ipa/ipu3/algorithms/agc.cpp\n> > >> @@ -152,18 +152,19 @@ Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,\n> > >>                   reinterpret_cast<const ipu3_uapi_awb_set_item *>(\n> > >> &stats->awb_raw_buffer.meta_data[cellPosition]);\n> > >> -            rgbTriples_.push_back({\n> > >> -                cell->R_avg,\n> > >> -                (cell->Gr_avg + cell->Gb_avg) / 2,\n> > >> -                cell->B_avg\n> > >> -            });\n> > >> +            uint8_t G_avg = (cell->Gr_avg + cell->Gb_avg) / 2;\n> > >> +\n> > >> +            rgbTriples_.push_back({cell->R_avg, G_avg, cell->B_avg});\n> > >>                 /*\n> > >> -             * Store the average green value to estimate the\n> > >> +             * Store the average luminance value to estimate the\n> > >>                * brightness. Even the overexposed pixels are\n> > >>                * taken into account.\n> > >>                */\n> > >> -            hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;\n> > >> +            uint8_t y = (cell->R_avg * .299) +\n> > >> +                    (G_avg * .587) +\n> > >> +                    (cell->B_avg * .114);\n> \n> Somewhere in libipa we should really have helpers or classes for the\n> colour space coefficients to make this more expressive.\n>\n> > >> +            hist[y]++;\n> > >\n> > > Is there a benefit to have the \"real\" Y value and not only the green\n> > > parts (which reflect this Y value quite nicely as it is ~60% of the\n> > > value) ?\n> > \n> > Only that it more accurately matches the Y-value that's used in the\n> > rest of the algorithm; estimateLuminance() uses the same Rec.601\n> > coefficients and the gain that's derived from that calculation is\n> > compared against one derived from this Histogram.\n> \n> Presumably I would get different results if I were in a Red or Green\n> t-shirt ... so I certainly think it's more appropriate to use the\n> luminance correctly.\n\nExcept it would be too easy if it could be simply done this way :-) It\nall depends on how you define luminance. Not only do we have different\ndefinitions in different colour spaces, but the above calculation would\nneed to be done after applying the CCM in order to account for\ndifferences in spectral response between sensors.\n\nThis being said, I'm not opposed to taking the three colour components\ninto account, but I also wonder as Jean-Michel if it really improves the\nAGC operation compared to using the green channel only. Dan, have you\nobserved as qualitative or quantitative improvements with this patch ?\n\n> > > Could you measure it ? No increase in CPU time for big stats grids ?\n> > \n> > I didn't check specifically but I can do - I didn't lose any framerate\n> > on my Surface Go2...but I imagine that the Imgu can run much faster\n> > than that is doing currently.\n> \n> I don't think I'm worried about doing the correct thing here. It's once\n> per frame, on the stats - not per pixel in the image.\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>","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 66F64BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  8 May 2024 14:07:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C7C563439;\n\tWed,  8 May 2024 16:07:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D771661A73\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  8 May 2024 16:07:37 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8B893B0B;\n\tWed,  8 May 2024 16:07:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OT7T8Q4N\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715177254;\n\tbh=VobbL5tA5uMAaHcKJgGj14+Z6aNKqemKdITGDFKXuzg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OT7T8Q4NWq7Udkv6Zhveen2YiNLmL8F+6JiRFrGuoBe1+mwqiRxj0SXOfLPMYCFKR\n\td7k8bpFyyIojYYLGB6mUDqOj9QoQFQwstdfBr/f279e1Ri+MTG3N/5mAAB3bidrcvw\n\tco6bukTq2xt59HDubCQAwVNAFshyTtEK+gCqw5O8=","Date":"Wed, 8 May 2024 17:07:29 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Dan Scally <dan.scally@ideasonboard.com>,\n\tJean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 3/3] ipa: ipu3: Use a Y histogram instead of green","Message-ID":"<20240508140729.GM2012@pendragon.ideasonboard.com>","References":"<20240418124632.652163-1-dan.scally@ideasonboard.com>\n\t<20240418124632.652163-4-dan.scally@ideasonboard.com>\n\t<84315077-bca9-43ff-bb78-ab3b0f935448@yoseli.org>\n\t<6a53b92f-0179-41c2-95fc-7c7fecb188df@ideasonboard.com>\n\t<171517275296.1857112.6716225758404197356@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<171517275296.1857112.6716225758404197356@ping.linuxembedded.co.uk>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]