[{"id":31114,"web_url":"https://patchwork.libcamera.org/comment/31114/","msgid":"","date":"2024-09-09T07:34:46","subject":"Re: [PATCH v6 05/18] libcamera: software_isp: Make stats frame and\n\tbuffer aware","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Milan\n\nOn 06/09/2024 13:09, Milan Zamazal wrote:\n> This patch adds frame and bufferId arguments to stats related calls.\n> Although the parameters are currently unused, because frame ids are not\n> tracked and used and the stats buffer is passed around directly rather\n> than being referred by its id, they bring the internal APIs closer to\n> their counterparts in hardware pipelines.\n>\n> It serves as a preparation for followup patches that will introduce:\n>\n> - Frame number tracking in order to switch to DelayedControls\n> (software ISP TODO #11 + #12).\n> - A ring buffer for stats in order to improve passing the stats\n> (software ISP TODO #2).\n>\n> Frame and buffer ids are unrelated for the given purposes but since they\n> are passed together at the same places, the change is implemented as a\n> single patch rather than two, basically the same, patches.\n>\n> Signed-off-by: Milan Zamazal \n> Reviewed-by: Laurent Pinchart \n> ---\nReviewed-by: Daniel Scally \n> .../libcamera/internal/software_isp/software_isp.h | 8 +++++---\n> include/libcamera/ipa/soft.mojom | 4 +++-\n> src/ipa/simple/soft_simple.cpp | 7 +++++--\n> src/libcamera/pipeline/simple/simple.cpp | 8 +++++---\n> src/libcamera/software_isp/debayer_cpu.cpp | 8 +++++++-\n> src/libcamera/software_isp/software_isp.cpp | 11 +++++++----\n> src/libcamera/software_isp/swstats_cpu.cpp | 6 ++++--\n> src/libcamera/software_isp/swstats_cpu.h | 4 ++--\n> 8 files changed, 38 insertions(+), 18 deletions(-)\n>\n> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\n> index f8e00003..3602bce8 100644\n> --- a/include/libcamera/internal/software_isp/software_isp.h\n> +++ b/include/libcamera/internal/software_isp/software_isp.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -66,7 +67,8 @@ public:\n> \tint exportBuffers(const Stream *stream, unsigned int count,\n> \t\t\t std::vector> *buffers);\n> \n> -\tvoid processStats(const ControlList &sensorControls);\n> +\tvoid processStats(const uint32_t frame, const uint32_t bufferId,\n> +\t\t\t const ControlList &sensorControls);\n> \n> \tint start();\n> \tvoid stop();\n> @@ -78,13 +80,13 @@ public:\n> \n> \tSignal inputBufferReady;\n> \tSignal outputBufferReady;\n> -\tSignal<> ispStatsReady;\n> +\tSignal ispStatsReady;\n> \tSignal setSensorControls;\n> \n> private:\n> \tvoid saveIspParams();\n> \tvoid setSensorCtrls(const ControlList &sensorControls);\n> -\tvoid statsReady();\n> +\tvoid statsReady(uint32_t frame, uint32_t bufferId);\n> \tvoid inputReady(FrameBuffer *input);\n> \tvoid outputReady(FrameBuffer *output);\n> \n> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom\n> index 0fd47bb0..f65b6231 100644\n> --- a/include/libcamera/ipa/soft.mojom\n> +++ b/include/libcamera/ipa/soft.mojom\n> @@ -23,7 +23,9 @@ interface IPASoftInterface {\n> \tconfigure(libcamera.ControlInfoMap sensorCtrlInfoMap)\n> \t\t=> (int32 ret);\n> \n> -\t[async] processStats(libcamera.ControlList sensorControls);\n> +\t[async] processStats(uint32 frame,\n> +\t\t\t uint32 bufferId,\n> +\t\t\t libcamera.ControlList sensorControls);\n> };\n> \n> interface IPASoftEventInterface {\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index 72321f44..12b5245e 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -75,7 +75,8 @@ public:\n> \tint start() override;\n> \tvoid stop() override;\n> \n> -\tvoid processStats(const ControlList &sensorControls) override;\n> +\tvoid processStats(const uint32_t frame, const uint32_t bufferId,\n> +\t\t\t const ControlList &sensorControls) override;\n> \n> protected:\n> \tstd::string logPrefix() const override;\n> @@ -249,7 +250,9 @@ void IPASoftSimple::stop()\n> {\n> }\n> \n> -void IPASoftSimple::processStats(const ControlList &sensorControls)\n> +void IPASoftSimple::processStats([[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t [[maybe_unused]] const uint32_t bufferId,\n> +\t\t\t\t const ControlList &sensorControls)\n> {\n> \tSwIspStats::Histogram histogram = stats_->yHistogram;\n> \tif (ignoreUpdates_ > 0)\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 1e7ec7d9..48a568da 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -13,6 +13,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -291,7 +292,7 @@ private:\n> \tvoid conversionInputDone(FrameBuffer *buffer);\n> \tvoid conversionOutputDone(FrameBuffer *buffer);\n> \n> -\tvoid ispStatsReady();\n> +\tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n> \tvoid setSensorControls(const ControlList &sensorControls);\n> };\n> \n> @@ -891,10 +892,11 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> \t\tpipe->completeRequest(request);\n> }\n> \n> -void SimpleCameraData::ispStatsReady()\n> +void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n> {\n> \t/* \\todo Use the DelayedControls class */\n> -\tswIsp_->processStats(sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,\n> +\tswIsp_->processStats(frame, bufferId,\n> +\t\t\t sensor_->getControls({ V4L2_CID_ANALOGUE_GAIN,\n> \t\t\t\t\t\t V4L2_CID_EXPOSURE }));\n> }\n> \n> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\n> index 077f7f4b..2a2e7edb 100644\n> --- a/src/libcamera/software_isp/debayer_cpu.cpp\n> +++ b/src/libcamera/software_isp/debayer_cpu.cpp\n> @@ -777,7 +777,13 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams\n> \t\t}\n> \t}\n> \n> -\tstats_->finishFrame();\n> +\t/*\n> +\t * Frame and buffer ids are currently not used, so pass zeros as parameters.\n> +\t *\n> +\t * \\todo Pass real values once frame is passed here and stats buffer passing\n> +\t * is changed.\n> +\t */\n> +\tstats_->finishFrame(0, 0);\n> \toutputBufferReady.emit(output);\n> \tinputBufferReady.emit(input);\n> }\n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index a3ae3e22..a3855568 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -155,15 +155,18 @@ SoftwareIsp::~SoftwareIsp()\n> \n> /**\n> * \\brief Process the statistics gathered\n> + * \\param[in] frame The frame number\n> + * \\param[in] bufferId ID of the statistics buffer\n> * \\param[in] sensorControls The sensor controls\n> *\n> * Requests the IPA to calculate new parameters for ISP and new control\n> * values for the sensor.\n> */\n> -void SoftwareIsp::processStats(const ControlList &sensorControls)\n> +void SoftwareIsp::processStats(const uint32_t frame, const uint32_t bufferId,\n> +\t\t\t const ControlList &sensorControls)\n> {\n> \tASSERT(ipa_);\n> -\tipa_->processStats(sensorControls);\n> +\tipa_->processStats(frame, bufferId, sensorControls);\n> }\n> \n> /**\n> @@ -349,9 +352,9 @@ void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)\n> \tsetSensorControls.emit(sensorControls);\n> }\n> \n> -void SoftwareIsp::statsReady()\n> +void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)\n> {\n> -\tispStatsReady.emit();\n> +\tispStatsReady.emit(frame, bufferId);\n> }\n> \n> void SoftwareIsp::inputReady(FrameBuffer *input)\n> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\n> index 815c4d4f..c520c806 100644\n> --- a/src/libcamera/software_isp/swstats_cpu.cpp\n> +++ b/src/libcamera/software_isp/swstats_cpu.cpp\n> @@ -311,13 +311,15 @@ void SwStatsCpu::startFrame(void)\n> \n> /**\n> * \\brief Finish statistics calculation for the current frame\n> + * \\param[in] frame The frame number\n> + * \\param[in] bufferId ID of the statistics buffer\n> *\n> * This may only be called after a successful setWindow() call.\n> */\n> -void SwStatsCpu::finishFrame(void)\n> +void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId)\n> {\n> \t*sharedStats_ = stats_;\n> -\tstatsReady.emit();\n> +\tstatsReady.emit(frame, bufferId);\n> }\n> \n> /**\n> diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h\n> index 363e326f..26a2f462 100644\n> --- a/src/libcamera/software_isp/swstats_cpu.h\n> +++ b/src/libcamera/software_isp/swstats_cpu.h\n> @@ -41,7 +41,7 @@ public:\n> \tint configure(const StreamConfiguration &inputCfg);\n> \tvoid setWindow(const Rectangle &window);\n> \tvoid startFrame();\n> -\tvoid finishFrame();\n> +\tvoid finishFrame(uint32_t frame, uint32_t bufferId);\n> \n> \tvoid processLine0(unsigned int y, const uint8_t *src[])\n> \t{\n> @@ -61,7 +61,7 @@ public:\n> \t\t(this->*stats2_)(src);\n> \t}\n> \n> -\tSignal<> statsReady;\n> +\tSignal statsReady;\n> \n> private:\n> \tusing statsProcessFn = void (SwStatsCpu::*)(const uint8_t *src[]);","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 CAEFBC324C\n\tfor ;\n\tMon, 9 Sep 2024 07:34:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D756634F5;\n\tMon, 9 Sep 2024 09:34:52 +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 EDFF1634E4\n\tfor ;\n\tMon, 9 Sep 2024 09:34:49 +0200 (CEST)","from [192.168.0.53] (213-229-8-243.static.upcbusiness.at\n\t[213.229.8.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E91743EA;\n\tMon, 9 Sep 2024 09:33: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=\"b+iNyvf/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725867214;\n\tbh=+hEr2fTR03bUxB6AbbVCTmKuxk9uFK8kOuJj5wXxhSA=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=b+iNyvf/m1z8PnR01ABmPPitVO26EWsEXIfU/bf+7Ep6yNT4Gb6xVqxZ+W63jLKIX\n\tk+mR4suEsCXAXCCDhU8IhhbGzdmnQF95q5QzYrtDyb9xJRp3hqro5dkAi1JkiEOL8A\n\tXDylPXAIf7xgDNyJz5EbQvovf9ru2He6mOQ18eqw=","Message-ID":"","Date":"Mon, 9 Sep 2024 08:34:46 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v6 05/18] libcamera: software_isp: Make stats frame and\n\tbuffer aware","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Cc":"Umang Jain ,\n\tLaurent Pinchart ","References":"<20240906120927.4071508-1-mzamazal@redhat.com>\n\t<20240906120927.4071508-6-mzamazal@redhat.com>","Content-Language":"en-US","From":"Dan Scally ","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":"<20240906120927.4071508-6-mzamazal@redhat.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}}]