[{"id":20452,"web_url":"https://patchwork.libcamera.org/comment/20452/","msgid":"","date":"2021-10-25T19:56:38","subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nThank you for the patch.\n\nOn Fri, Oct 22, 2021 at 05:12:00PM +0200, Jean-Michel Hautbois wrote:\n> The IPU3 IPA is maturing to a modular and extensible system capable of\n> handling specific algorithms for the processing blocks on the ImgU.\n> \n> Provide a top-level class documentation to provide an overview of the\n> IPA, detailing what events are used and what algorithms are currently\n> supported, as well as the limitations currently imposed.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 64 insertions(+)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 65d3fd2c..b5438ed1 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;\n> \n> namespace ipa::ipu3 {\n> \n> +/**\n> + * \\brief The IPU3 IPA implementation\n> + *\n> + * The IPU3 Pipeline defines an IPU3-specific interface for communication\n> + * between the PipelineHandler and the IPA module.\n> + *\n> + * We extend the IPAIPU3Interface to implement our algorithms and handle events\n> + * from the IPU3 PipelineHandler to satisfy requests from the application.\n> + *\n> + * At initialisation time, a CameraSensorHelper is instantiated to support\n> + * camera-specific calculations, while the default controls are computed, and\n> + * the algorithms are constructed and placed in an ordered list.\n> + *\n> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into\n> + * rectangular cells of pixels. When the IPA is configured, we determine the\n> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler\n> + * output size.\n> + *\n> + * Two main events are then handled to operate the IPU3 ImgU by populating its\n> + * parameter buffer, and adapting the settings of the sensor attached to the\n> + * IPU3 CIO2 through sensor-specific V4L2 controls.\n> + *\n> + * When the event \\a EventFillParams occurs we populate the ImgU parameter\n> + * buffer with settings to configure the device in preparation for handling the\n> + * frame queued in the Request.\n> + *\n> + * When the frame has completed processing, the ImgU will generate a statistics\n> + * buffer which is given to the IPA as part of the \\a EventStatReady event. At\n> + * this event we run the algorithms to parse the statistics and cache any\n> + * results for the next \\a EventFillParams event.\n> + *\n> + * The individual algorithms are split into modular components that are called\n> + * iteratively to allow them to process statistics from the ImgU in a defined\n> + * order.\n> + *\n> + * The current implementation supports three core algorithms:\n> + * - Automatic white balance (AWB)\n> + * - Automatic gain and exposure control (AGC)\n> + * - Tone mapping (Gamma)\n> + *\n> + * AWB is implemented using a Greyworld algorithm, and calculates the red and\n> + * blue gains to apply to generate a neutral grey frame overall.\n> + *\n> + * AGC is handled by calculating a histogram of the green channel to estimate an\n> + * analogue gain and shutter time which will provide a well exposed frame. A\n> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce\n> + * perceivable steps.\n> + *\n> + * The Tone mapping algorithm provides a gamma correction table to improve the\n> + * contrast of the scene.\n> + *\n> + * The IPU3 ImgU has further processing blocks to support image quality\n> + * improvements through bayer and temporal noise reductions, however those are\n> + * not supported in the current implementation, and will use default settings as\n> + * provided by the kernel driver.\n> + *\n> + * Demosaicing is operating with the default parameters and could be further\n> + * optimised to provide improved sharpening coefficients, checker artifact\n> + * removal, and false color correction.\n> + *\n> + * Additional image enhancements can be made by providing lens and\n> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens\n> + * shading correction (SHD) and Color correction (CCM).\n> + */\n> class IPAIPU3 : public IPAIPU3Interface\n> {\n> public:","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A42C8BF415\n\tfor ;\n\tMon, 25 Oct 2021 19:57:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 07DF664878;\n\tMon, 25 Oct 2021 21:57:03 +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 1D42160125\n\tfor ;\n\tMon, 25 Oct 2021 21:57:01 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 91596E0A;\n\tMon, 25 Oct 2021 21:57:00 +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=\"mPs6wCqJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635191820;\n\tbh=rVaVUzz/QHGoLet36XHHVNhKmM/qQUCNhEsm/h57Ofk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mPs6wCqJe/RjfVhHKwx+lMcWu6YZDrBatQ7T6+x+bLyg2navCi3CKeM+ODI29/kER\n\tx2wIa01feahH103VQ702oyZFoIGAdBrcAgIekFnlkiESqPfrOHRoQkeukCTxd7OOQ+\n\txhI9Vr8oLRmnypkRHuirdtSQOFI9Bd9cq7dtuRo4=","Date":"Mon, 25 Oct 2021 22:56:38 +0300","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211022151218.111966-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211022151218.111966-2-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211022151218.111966-2-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20456,"web_url":"https://patchwork.libcamera.org/comment/20456/","msgid":"<163519244189.1095920.6325459015986723471@Monstersaurus>","date":"2021-10-25T20:07:21","subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-10-22 16:12:00)\n> The IPU3 IPA is maturing to a modular and extensible system capable of\n> handling specific algorithms for the processing blocks on the ImgU.\n> \n> Provide a top-level class documentation to provide an overview of the\n> IPA, detailing what events are used and what algorithms are currently\n> supported, as well as the limitations currently imposed.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 64 insertions(+)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 65d3fd2c..b5438ed1 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;\n> \n> namespace ipa::ipu3 {\n> \n> +/**\n> + * \\brief The IPU3 IPA implementation\n> + *\n> + * The IPU3 Pipeline defines an IPU3-specific interface for communication\n> + * between the PipelineHandler and the IPA module.\n> + *\n> + * We extend the IPAIPU3Interface to implement our algorithms and handle events\n> + * from the IPU3 PipelineHandler to satisfy requests from the application.\n> + *\n> + * At initialisation time, a CameraSensorHelper is instantiated to support\n> + * camera-specific calculations, while the default controls are computed, and\n> + * the algorithms are constructed and placed in an ordered list.\n> + *\n> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into\n> + * rectangular cells of pixels. When the IPA is configured, we determine the\n> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler\n> + * output size.\n> + *\n> + * Two main events are then handled to operate the IPU3 ImgU by populating its\n> + * parameter buffer, and adapting the settings of the sensor attached to the\n> + * IPU3 CIO2 through sensor-specific V4L2 controls.\n> + *\n> + * When the event \\a EventFillParams occurs we populate the ImgU parameter\n> + * buffer with settings to configure the device in preparation for handling the\n> + * frame queued in the Request.\n> + *\n> + * When the frame has completed processing, the ImgU will generate a statistics\n> + * buffer which is given to the IPA as part of the \\a EventStatReady event. At\n> + * this event we run the algorithms to parse the statistics and cache any\n> + * results for the next \\a EventFillParams event.\n> + *\n> + * The individual algorithms are split into modular components that are called\n> + * iteratively to allow them to process statistics from the ImgU in a defined\n> + * order.\n> + *\n> + * The current implementation supports three core algorithms:\n> + * - Automatic white balance (AWB)\n> + * - Automatic gain and exposure control (AGC)\n> + * - Tone mapping (Gamma)\n\nHas this been extended with BLC now? Or is it not worth mentioning that\nexplicitly?\n\n> + *\n> + * AWB is implemented using a Greyworld algorithm, and calculates the red and\n> + * blue gains to apply to generate a neutral grey frame overall.\n> + *\n> + * AGC is handled by calculating a histogram of the green channel to estimate an\n> + * analogue gain and shutter time which will provide a well exposed frame. A\n> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce\n> + * perceivable steps.\n> + *\n> + * The Tone mapping algorithm provides a gamma correction table to improve the\n> + * contrast of the scene.\n> + *\n> + * The IPU3 ImgU has further processing blocks to support image quality\n> + * improvements through bayer and temporal noise reductions, however those are\n> + * not supported in the current implementation, and will use default settings as\n> + * provided by the kernel driver.\n> + *\n> + * Demosaicing is operating with the default parameters and could be further\n> + * optimised to provide improved sharpening coefficients, checker artifact\n> + * removal, and false color correction.\n> + *\n> + * Additional image enhancements can be made by providing lens and\n> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens\n\nAnd perhaps check if BLC needs removing from here. If BLC still needs\nmore wrok, then perhaps it should stay.\n\nReviewed-by: Kieran Bingham \n\n> + * shading correction (SHD) and Color correction (CCM).\n> + */\n> class IPAIPU3 : public IPAIPU3Interface\n> {\n> public:\n> -- \n> 2.32.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 54A3CBF415\n\tfor ;\n\tMon, 25 Oct 2021 20:07:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CE72364878;\n\tMon, 25 Oct 2021 22:07:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6653760125\n\tfor ;\n\tMon, 25 Oct 2021 22:07:24 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0EE60E0A;\n\tMon, 25 Oct 2021 22:07:24 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TzCduaCV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635192444;\n\tbh=BessKeUKmEL6128VOsWuEFQfhQDmTiyt204TsZg3U2A=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=TzCduaCVDFrVHDa3a5dZfRmxdP5D15lid/ICq8UlVTrUw/TeVwDSDGqVhaZHu8OuB\n\tYhl48YQGr0iqDSMXwoPQP4UDBPuKS4juQm33B4dGNiDPBqSueA9/jc1E3aU9PA86iT\n\tfODhpSmtA5buTBISu4gLr74O4yWFz8Ve145q+BuE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211022151218.111966-2-jeanmichel.hautbois@ideasonboard.com>","References":"<20211022151218.111966-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211022151218.111966-2-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 25 Oct 2021 21:07:21 +0100","Message-ID":"<163519244189.1095920.6325459015986723471@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20494,"web_url":"https://patchwork.libcamera.org/comment/20494/","msgid":"<6255bcd6-e961-0b9b-cb0a-a1138a8f3165@ideasonboard.com>","date":"2021-10-26T05:34:50","subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Kieran, question for Laurent below ;-)\n\nOn 25/10/2021 22:07, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois (2021-10-22 16:12:00)\n>> The IPU3 IPA is maturing to a modular and extensible system capable of\n>> handling specific algorithms for the processing blocks on the ImgU.\n>>\n>> Provide a top-level class documentation to provide an overview of the\n>> IPA, detailing what events are used and what algorithms are currently\n>> supported, as well as the limitations currently imposed.\n>>\n>> Signed-off-by: Jean-Michel Hautbois \n>> Signed-off-by: Kieran Bingham \n>> ---\n>> src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++\n>> 1 file changed, 64 insertions(+)\n>>\n>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n>> index 65d3fd2c..b5438ed1 100644\n>> --- a/src/ipa/ipu3/ipu3.cpp\n>> +++ b/src/ipa/ipu3/ipu3.cpp\n>> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;\n>> \n>> namespace ipa::ipu3 {\n>> \n>> +/**\n>> + * \\brief The IPU3 IPA implementation\n>> + *\n>> + * The IPU3 Pipeline defines an IPU3-specific interface for communication\n>> + * between the PipelineHandler and the IPA module.\n>> + *\n>> + * We extend the IPAIPU3Interface to implement our algorithms and handle events\n>> + * from the IPU3 PipelineHandler to satisfy requests from the application.\n>> + *\n>> + * At initialisation time, a CameraSensorHelper is instantiated to support\n>> + * camera-specific calculations, while the default controls are computed, and\n>> + * the algorithms are constructed and placed in an ordered list.\n>> + *\n>> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into\n>> + * rectangular cells of pixels. When the IPA is configured, we determine the\n>> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler\n>> + * output size.\n>> + *\n>> + * Two main events are then handled to operate the IPU3 ImgU by populating its\n>> + * parameter buffer, and adapting the settings of the sensor attached to the\n>> + * IPU3 CIO2 through sensor-specific V4L2 controls.\n>> + *\n>> + * When the event \\a EventFillParams occurs we populate the ImgU parameter\n>> + * buffer with settings to configure the device in preparation for handling the\n>> + * frame queued in the Request.\n>> + *\n>> + * When the frame has completed processing, the ImgU will generate a statistics\n>> + * buffer which is given to the IPA as part of the \\a EventStatReady event. At\n>> + * this event we run the algorithms to parse the statistics and cache any\n>> + * results for the next \\a EventFillParams event.\n>> + *\n>> + * The individual algorithms are split into modular components that are called\n>> + * iteratively to allow them to process statistics from the ImgU in a defined\n>> + * order.\n>> + *\n>> + * The current implementation supports three core algorithms:\n>> + * - Automatic white balance (AWB)\n>> + * - Automatic gain and exposure control (AGC)\n>> + * - Tone mapping (Gamma)\n> \n> Has this been extended with BLC now? Or is it not worth mentioning that\n> explicitly?\n\nWell, according to me, it is not mature enough to be considered \nsupported (but neither is tone mapping, ahah :-)).\n\n> \n>> + *\n>> + * AWB is implemented using a Greyworld algorithm, and calculates the red and\n>> + * blue gains to apply to generate a neutral grey frame overall.\n>> + *\n>> + * AGC is handled by calculating a histogram of the green channel to estimate an\n>> + * analogue gain and shutter time which will provide a well exposed frame. A\n>> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce\n>> + * perceivable steps.\n>> + *\n>> + * The Tone mapping algorithm provides a gamma correction table to improve the\n>> + * contrast of the scene.\n>> + *\n>> + * The IPU3 ImgU has further processing blocks to support image quality\n>> + * improvements through bayer and temporal noise reductions, however those are\n>> + * not supported in the current implementation, and will use default settings as\n>> + * provided by the kernel driver.\n>> + *\n>> + * Demosaicing is operating with the default parameters and could be further\n>> + * optimised to provide improved sharpening coefficients, checker artifact\n>> + * removal, and false color correction.\n>> + *\n>> + * Additional image enhancements can be made by providing lens and\n>> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens\n> \n> And perhaps check if BLC needs removing from here. If BLC still needs\n> more wrok, then perhaps it should stay.\n\nLaurent, what do you think, should I add it now or later on top ?\n\n> \n> Reviewed-by: Kieran Bingham \n> \n>> + * shading correction (SHD) and Color correction (CCM).\n>> + */\n>> class IPAIPU3 : public IPAIPU3Interface\n>> {\n>> public:\n>> -- \n>> 2.32.0\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3A057BF415\n\tfor ;\n\tTue, 26 Oct 2021 05:34:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8994564878;\n\tTue, 26 Oct 2021 07:34:54 +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 D52136486B\n\tfor ;\n\tTue, 26 Oct 2021 07:34:52 +0200 (CEST)","from [IPV6:2a01:e0a:169:7140:dce3:eb54:18d7:6f3d] (unknown\n\t[IPv6:2a01:e0a:169:7140:dce3:eb54:18d7:6f3d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A66E3F0;\n\tTue, 26 Oct 2021 07:34:52 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jQL6xLAj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635226492;\n\tbh=ssfzG8LGiOya4jwys0pp1fqXZ/mGH7Q9FFPXmTb99sI=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=jQL6xLAj35NvEeysAIujE5bXnNpgkvy0riGZ1bmLAiV1QxttqOiELC/GM/bb/uWla\n\tW6caaLvpkFBI1NWM8QgFU+aD3O72av8XI29alcKVoOZ3Rk7bmqlXSpk7bDXLQavWeh\n\tq1DyPSc0LnaTmpr0Yqla/Ppw9gq3ZX4hv76QU0+s=","Message-ID":"<6255bcd6-e961-0b9b-cb0a-a1138a8f3165@ideasonboard.com>","Date":"Tue, 26 Oct 2021 07:34:50 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.1.2","Content-Language":"en-US","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart ","References":"<20211022151218.111966-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211022151218.111966-2-jeanmichel.hautbois@ideasonboard.com>\n\t<163519244189.1095920.6325459015986723471@Monstersaurus>","From":"Jean-Michel Hautbois ","In-Reply-To":"<163519244189.1095920.6325459015986723471@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20509,"web_url":"https://patchwork.libcamera.org/comment/20509/","msgid":"","date":"2021-10-26T07:49:52","subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hello JM,\n\nThank you for the patch.\n\nOn 10/22/21 8:42 PM, Jean-Michel Hautbois wrote:\n> The IPU3 IPA is maturing to a modular and extensible system capable of\n> handling specific algorithms for the processing blocks on the ImgU.\n>\n> Provide a top-level class documentation to provide an overview of the\n> IPA, detailing what events are used and what algorithms are currently\n> supported, as well as the limitations currently imposed.\n>\n> Signed-off-by: Jean-Michel Hautbois \n> Signed-off-by: Kieran Bingham \n\n\nReviewed-by: Umang Jain \n> ---\n> src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 64 insertions(+)\n>\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 65d3fd2c..b5438ed1 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;\n> \n> namespace ipa::ipu3 {\n> \n> +/**\n> + * \\brief The IPU3 IPA implementation\n> + *\n> + * The IPU3 Pipeline defines an IPU3-specific interface for communication\n> + * between the PipelineHandler and the IPA module.\n> + *\n> + * We extend the IPAIPU3Interface to implement our algorithms and handle events\n> + * from the IPU3 PipelineHandler to satisfy requests from the application.\n> + *\n> + * At initialisation time, a CameraSensorHelper is instantiated to support\n> + * camera-specific calculations, while the default controls are computed, and\n> + * the algorithms are constructed and placed in an ordered list.\n> + *\n> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into\n> + * rectangular cells of pixels. When the IPA is configured, we determine the\n> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler\n> + * output size.\n> + *\n> + * Two main events are then handled to operate the IPU3 ImgU by populating its\n> + * parameter buffer, and adapting the settings of the sensor attached to the\n> + * IPU3 CIO2 through sensor-specific V4L2 controls.\n> + *\n> + * When the event \\a EventFillParams occurs we populate the ImgU parameter\n> + * buffer with settings to configure the device in preparation for handling the\n> + * frame queued in the Request.\n> + *\n> + * When the frame has completed processing, the ImgU will generate a statistics\n> + * buffer which is given to the IPA as part of the \\a EventStatReady event. At\n> + * this event we run the algorithms to parse the statistics and cache any\n> + * results for the next \\a EventFillParams event.\n> + *\n> + * The individual algorithms are split into modular components that are called\n> + * iteratively to allow them to process statistics from the ImgU in a defined\n> + * order.\n> + *\n> + * The current implementation supports three core algorithms:\n> + * - Automatic white balance (AWB)\n> + * - Automatic gain and exposure control (AGC)\n> + * - Tone mapping (Gamma)\n> + *\n> + * AWB is implemented using a Greyworld algorithm, and calculates the red and\n> + * blue gains to apply to generate a neutral grey frame overall.\n> + *\n> + * AGC is handled by calculating a histogram of the green channel to estimate an\n> + * analogue gain and shutter time which will provide a well exposed frame. A\n> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce\n> + * perceivable steps.\n> + *\n> + * The Tone mapping algorithm provides a gamma correction table to improve the\n> + * contrast of the scene.\n> + *\n> + * The IPU3 ImgU has further processing blocks to support image quality\n> + * improvements through bayer and temporal noise reductions, however those are\n> + * not supported in the current implementation, and will use default settings as\n> + * provided by the kernel driver.\n> + *\n> + * Demosaicing is operating with the default parameters and could be further\n> + * optimised to provide improved sharpening coefficients, checker artifact\n> + * removal, and false color correction.\n> + *\n> + * Additional image enhancements can be made by providing lens and\n> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens\n> + * shading correction (SHD) and Color correction (CCM).\n> + */\n> class IPAIPU3 : public IPAIPU3Interface\n> {\n> public:","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5E525BDB1C\n\tfor ;\n\tTue, 26 Oct 2021 07:49:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1139E64878;\n\tTue, 26 Oct 2021 09:49: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 BAA2260123\n\tfor ;\n\tTue, 26 Oct 2021 09:49:56 +0200 (CEST)","from [192.168.1.106] (unknown [103.251.226.211])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EEE033F0;\n\tTue, 26 Oct 2021 09:49:55 +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=\"u0uJYemd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635234596;\n\tbh=plKRMhttwHQ4pZXSfQLPRzY0jqHkVHbkP31lA8V+TNA=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=u0uJYemdPqFUi/4xl857zyte9kCZhe4t8sB+FuXfg7USbrK8Q2TKAIejLADzINnuk\n\t14BT8PMKZpd6EBT2kShx4rD3tNWpuIhE65AM7XMtWmLFcDsXycbzg1Kk1fLaQZ/sJb\n\tkhnetUpd1oa8pdoNZEIewC9AxHe5ddCWSRK7KLoE=","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211022151218.111966-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211022151218.111966-2-jeanmichel.hautbois@ideasonboard.com>","From":"Umang Jain ","Message-ID":"","Date":"Tue, 26 Oct 2021 13:19:52 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20211022151218.111966-2-jeanmichel.hautbois@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20518,"web_url":"https://patchwork.libcamera.org/comment/20518/","msgid":"","date":"2021-10-26T09:10:54","subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Oct 26, 2021 at 07:34:50AM +0200, Jean-Michel Hautbois wrote:\n> Hi Kieran, question for Laurent below ;-)\n> \n> On 25/10/2021 22:07, Kieran Bingham wrote:\n> > Quoting Jean-Michel Hautbois (2021-10-22 16:12:00)\n> >> The IPU3 IPA is maturing to a modular and extensible system capable of\n> >> handling specific algorithms for the processing blocks on the ImgU.\n> >>\n> >> Provide a top-level class documentation to provide an overview of the\n> >> IPA, detailing what events are used and what algorithms are currently\n> >> supported, as well as the limitations currently imposed.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois \n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> src/ipa/ipu3/ipu3.cpp | 64 +++++++++++++++++++++++++++++++++++++++++++\n> >> 1 file changed, 64 insertions(+)\n> >>\n> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> >> index 65d3fd2c..b5438ed1 100644\n> >> --- a/src/ipa/ipu3/ipu3.cpp\n> >> +++ b/src/ipa/ipu3/ipu3.cpp\n> >> @@ -179,6 +179,70 @@ using namespace std::literals::chrono_literals;\n> >> \n> >> namespace ipa::ipu3 {\n> >> \n> >> +/**\n> >> + * \\brief The IPU3 IPA implementation\n> >> + *\n> >> + * The IPU3 Pipeline defines an IPU3-specific interface for communication\n> >> + * between the PipelineHandler and the IPA module.\n> >> + *\n> >> + * We extend the IPAIPU3Interface to implement our algorithms and handle events\n> >> + * from the IPU3 PipelineHandler to satisfy requests from the application.\n> >> + *\n> >> + * At initialisation time, a CameraSensorHelper is instantiated to support\n> >> + * camera-specific calculations, while the default controls are computed, and\n> >> + * the algorithms are constructed and placed in an ordered list.\n> >> + *\n> >> + * The IPU3 ImgU operates with a grid layout to divide the overall frame into\n> >> + * rectangular cells of pixels. When the IPA is configured, we determine the\n> >> + * best grid for the statistics based on the pipeline handler Bayer Down Scaler\n> >> + * output size.\n> >> + *\n> >> + * Two main events are then handled to operate the IPU3 ImgU by populating its\n> >> + * parameter buffer, and adapting the settings of the sensor attached to the\n> >> + * IPU3 CIO2 through sensor-specific V4L2 controls.\n> >> + *\n> >> + * When the event \\a EventFillParams occurs we populate the ImgU parameter\n> >> + * buffer with settings to configure the device in preparation for handling the\n> >> + * frame queued in the Request.\n> >> + *\n> >> + * When the frame has completed processing, the ImgU will generate a statistics\n> >> + * buffer which is given to the IPA as part of the \\a EventStatReady event. At\n> >> + * this event we run the algorithms to parse the statistics and cache any\n> >> + * results for the next \\a EventFillParams event.\n> >> + *\n> >> + * The individual algorithms are split into modular components that are called\n> >> + * iteratively to allow them to process statistics from the ImgU in a defined\n> >> + * order.\n> >> + *\n> >> + * The current implementation supports three core algorithms:\n> >> + * - Automatic white balance (AWB)\n> >> + * - Automatic gain and exposure control (AGC)\n> >> + * - Tone mapping (Gamma)\n> > \n> > Has this been extended with BLC now? Or is it not worth mentioning that\n> > explicitly?\n> \n> Well, according to me, it is not mature enough to be considered \n> supported (but neither is tone mapping, ahah :-)).\n\nThe implementation is really a skeleton, but it's there, so I'd list it\nhere as it's not really worse than the tone mapping.\n\n> >> + *\n> >> + * AWB is implemented using a Greyworld algorithm, and calculates the red and\n> >> + * blue gains to apply to generate a neutral grey frame overall.\n> >> + *\n> >> + * AGC is handled by calculating a histogram of the green channel to estimate an\n> >> + * analogue gain and shutter time which will provide a well exposed frame. A\n> >> + * low-pass IIR filter is used to smooth the changes to the sensor to reduce\n> >> + * perceivable steps.\n> >> + *\n> >> + * The Tone mapping algorithm provides a gamma correction table to improve the\n\nAs a general rule, we need to capitalize class names to get doxygen to\ngenerate links (this is also why capitalized class names must not be in\nplural form, Framebuffers or Cameras won't generate a link to the\ncorresponding class). When the name of a class is used as a normal word,\nit should not be capitalized (in particular here, \"tone mapping\" with a\nspace isn't the name of the class). Specific terms are also usually\ncapitalized the first time they are introduced in a text, and they can\nbe capitalized when we want to emphasize that they are terms with a\nspecific meaning in the context, not just English words.\n\nBottom line, here it should be either \"tone mapping\" or \"Tone Mapping\".\n\n> >> + * contrast of the scene.\n\n * The black level compensation algorithm subtracts a hardcoded black level from\n * all pixels.\n\n(depending on how you capitalize tone mapping, this may need to be Black\nLevel Compensation)\n\n> >> + *\n> >> + * The IPU3 ImgU has further processing blocks to support image quality\n> >> + * improvements through bayer and temporal noise reductions, however those are\n> >> + * not supported in the current implementation, and will use default settings as\n> >> + * provided by the kernel driver.\n> >> + *\n> >> + * Demosaicing is operating with the default parameters and could be further\n> >> + * optimised to provide improved sharpening coefficients, checker artifact\n> >> + * removal, and false color correction.\n> >> + *\n> >> + * Additional image enhancements can be made by providing lens and\n> >> + * sensor-specific tuning to adapt for Black Level compensation (BLC), Lens\n> > \n> > And perhaps check if BLC needs removing from here. If BLC still needs\n> > more wrok, then perhaps it should stay.\n> \n> Laurent, what do you think, should I add it now or later on top ?\n\nI'd keep it in the list here, as we hardcode the value and need to move\nit to camera tuning data.\n\n> > Reviewed-by: Kieran Bingham \n> > \n> >> + * shading correction (SHD) and Color correction (CCM).\n> >> + */\n> >> class IPAIPU3 : public IPAIPU3Interface\n> >> {\n> >> public:","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 92BDABDB1C\n\tfor ;\n\tTue, 26 Oct 2021 09:11:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E25846487A;\n\tTue, 26 Oct 2021 11:11:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 01AB660123\n\tfor ;\n\tTue, 26 Oct 2021 11:11:16 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 752F63F0;\n\tTue, 26 Oct 2021 11:11:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qARF8rmb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635239476;\n\tbh=ivSVBGzowAokJA/QcB12FPb7/nTPtER2wyM3DHmgXPo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qARF8rmbn7YgDVkbPnsXaWP3iQWpMxYcdTg3VZuMR1SqNW1vJjvjp1pOz8brgvhxh\n\tUvIkkC6Fa64Dl2yBL429c/kgDGeEo1hxbFl0BRfRXP+E8GlKh55hj9XclhN3jLj6s2\n\tvPl/jdbhY6BPk+fsHBHa/Da9QWlC2U9u3PY900/A=","Date":"Tue, 26 Oct 2021 12:10:54 +0300","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211022151218.111966-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211022151218.111966-2-jeanmichel.hautbois@ideasonboard.com>\n\t<163519244189.1095920.6325459015986723471@Monstersaurus>\n\t<6255bcd6-e961-0b9b-cb0a-a1138a8f3165@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<6255bcd6-e961-0b9b-cb0a-a1138a8f3165@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 01/19] ipa: ipu3: Document IPAIPU3\n\tclass interface","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]