Patch Detail
Show a patch.
GET /api/1.1/patches/20886/?format=api
{ "id": 20886, "url": "https://patchwork.libcamera.org/api/1.1/patches/20886/?format=api", "web_url": "https://patchwork.libcamera.org/patch/20886/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20240812115009.946036-13-mzamazal@redhat.com>", "date": "2024-08-12T11:50:01", "name": "[12/16] libcamera: software_isp: Introduce arguments for statistics buffers", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "92d6ba88a310fc3e5a9469db34690ec89fe30a69", "submitter": { "id": 177, "url": "https://patchwork.libcamera.org/api/1.1/people/177/?format=api", "name": "Milan Zamazal", "email": "mzamazal@redhat.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/20886/mbox/", "series": [ { "id": 4511, "url": "https://patchwork.libcamera.org/api/1.1/series/4511/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4511", "date": "2024-08-12T11:49:49", "name": "Software ISP: Share params and stats buffers", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4511/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/20886/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/20886/checks/", "tags": {}, "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 77184C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 Aug 2024 11:50:50 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D7DF6633C9;\n\tMon, 12 Aug 2024 13:50:49 +0200 (CEST)", "from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D718C633C9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 Aug 2024 13:50:41 +0200 (CEST)", "from mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com\n\t(ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-241-iPoelU1yMNqJKMOUGTwD9A-1;\n\tMon, 12 Aug 2024 07:50:38 -0400", "from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com\n\t(mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com\n\t[10.30.177.15])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS\n\tid 352BD195420E for <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 Aug 2024 11:50:38 +0000 (UTC)", "from nuthatch.redhat.com (unknown [10.45.225.57])\n\tby mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id 0D4AB19772C4; Mon, 12 Aug 2024 11:50:36 +0000 (UTC)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"DesCpda1\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1723463440;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc: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=Jinnvnu8XCLnfQ5GOxGmcrGvFsqD0Ia5DcXJl/WHwzA=;\n\tb=DesCpda1wOFxP32gfFL6/j13iggmpd12allpaY3TTrbejwlXSNeyU3rfpAE5touGp0OGT2\n\tMTablHBJ+unZWO3YDkR3BzotRZtJ058psbMUcfS9XtWrd39It4WywowGLjFsLMZBHv2qAV\n\txDurpISEPEwcpCIEHwJ7tAIuSpKZAn0=", "X-MC-Unique": "iPoelU1yMNqJKMOUGTwD9A-1", "From": "Milan Zamazal <mzamazal@redhat.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Milan Zamazal <mzamazal@redhat.com>", "Subject": "[PATCH 12/16] libcamera: software_isp: Introduce arguments for\n\tstatistics buffers", "Date": "Mon, 12 Aug 2024 13:50:01 +0200", "Message-ID": "<20240812115009.946036-13-mzamazal@redhat.com>", "In-Reply-To": "<20240812115009.946036-1-mzamazal@redhat.com>", "References": "<20240812115009.946036-1-mzamazal@redhat.com>", "MIME-Version": "1.0", "X-Scanned-By": "MIMEDefang 3.0 on 10.30.177.15", "X-Mimecast-Spam-Score": "0", "X-Mimecast-Originator": "redhat.com", "Content-Transfer-Encoding": "8bit", "Content-Type": "text/plain; charset=\"US-ASCII\"; x-default=true", "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>" }, "content": "Statistics in software ISP is held in two fixed buffers, copying the\nstatistics from the working one to the shared one in\nSwIspStats::finishFrame. This patch introduces support for a ring of\nstatistics buffers that are never copied, similarly to parameters\nbuffers and to how hardware pipelines operate. This is to address\nsoftware ISP TODO #2.\n\nThe patch adds a new argument statsBufferId for the future parameters\nbuffer ids passed to the calls. The buffer ids must be passed to the\nfollowing groups of methods:\n\n- SwStatsCpu::startFrame, to reset the statistics.\n\n- Debayer::process, to signal what statistics buffer is started and\n stopped being used to gather the statistics.\n\n- SoftwareIsp::statsProcessed, a new signal to indicate that the given\n statistics buffer contents is no longer needed.\n\nThe type of the buffer id statistics is set to uint32_t because:\n\n- It can be used in mojom.\n- The same type is used for parameters buffers.\n- It is consistent with the similar types in the hardware pipelines.\n- It covers file descriptor number range, which will be used as buffer\n ids.\n\nAlready provided statistics buffer arguments are renamed from bufferId\nto statsBufferId for consistency and to avoid confusion between\nstatistics and parameters buffer id arguments.\n\nThis patch doesn't do more than adding the arguments, to keep the patch\nsimple. The buffer handling will be implemented in the followup\npatches.\n\nStatistics and parameters buffers use is coupled to much extent. It can\nbe tempting to perhaps use the same buffer ids and perhaps some common\nsignals. But it's better to keep them separate to avoid contingent\nfuture refactoring in case this coupling becomes less tight in future.\n\nSigned-off-by: Milan Zamazal <mzamazal@redhat.com>\n---\n .../libcamera/internal/software_isp/software_isp.h | 3 ++-\n include/libcamera/ipa/soft.mojom | 3 ++-\n src/ipa/simple/soft_simple.cpp | 3 ++-\n src/libcamera/pipeline/simple/simple.cpp | 7 ++++---\n src/libcamera/software_isp/debayer.cpp | 3 ++-\n src/libcamera/software_isp/debayer.h | 2 +-\n src/libcamera/software_isp/debayer_cpu.cpp | 11 +++--------\n src/libcamera/software_isp/debayer_cpu.h | 2 +-\n src/libcamera/software_isp/software_isp.cpp | 13 ++++++++++---\n src/libcamera/software_isp/swstats_cpu.cpp | 6 +++---\n src/libcamera/software_isp/swstats_cpu.h | 4 ++--\n 11 files changed, 32 insertions(+), 25 deletions(-)", "diff": "diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h\nindex 8956978c..ae64b247 100644\n--- a/include/libcamera/internal/software_isp/software_isp.h\n+++ b/include/libcamera/internal/software_isp/software_isp.h\n@@ -89,7 +89,8 @@ private:\n \tvoid saveIspParams(uint32_t paramsBufferId);\n \tvoid releaseIspParams(uint32_t paramsBufferId);\n \tvoid setSensorCtrls(const ControlList &sensorControls);\n-\tvoid statsReady(uint32_t frame, uint32_t bufferId);\n+\tvoid statsReady(uint32_t frame, const uint32_t statsBufferId);\n+\tvoid statsProcessed(const uint32_t statsBufferId);\n \tvoid inputReady(FrameBuffer *input);\n \tvoid outputReady(FrameBuffer *output);\n \ndiff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom\nindex 86dd988f..7b85c454 100644\n--- a/include/libcamera/ipa/soft.mojom\n+++ b/include/libcamera/ipa/soft.mojom\n@@ -25,10 +25,11 @@ interface IPASoftInterface {\n \n \t[async] queueRequest(uint32 frame, libcamera.ControlList sensorControls);\n \tprepare(uint32 frame, uint32 paramsBufferId);\n-\t[async] processStats(uint32 frame, uint32 bufferId, libcamera.ControlList sensorControls);\n+\t[async] processStats(uint32 frame, uint32 statsBufferId, libcamera.ControlList sensorControls);\n };\n \n interface IPASoftEventInterface {\n \tsetSensorControls(libcamera.ControlList sensorControls);\n+\tstatsProcessed(uint32 statsBufferId);\n \tsetIspParams(uint32 paramsBufferId);\n };\ndiff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\nindex 50f8f194..3c95c4d9 100644\n--- a/src/ipa/simple/soft_simple.cpp\n+++ b/src/ipa/simple/soft_simple.cpp\n@@ -266,7 +266,7 @@ void IPASoftSimple::prepare(const uint32_t frame,\n \n void IPASoftSimple::processStats(\n \tconst uint32_t frame,\n-\t[[maybe_unused]] const uint32_t bufferId,\n+\tconst uint32_t statsBufferId,\n \tconst ControlList &sensorControls)\n {\n \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n@@ -285,6 +285,7 @@ void IPASoftSimple::processStats(\n \tControlList metadata(controls::controls);\n \tfor (auto const &algo : algorithms())\n \t\talgo->process(context_, frame, frameContext, stats_, metadata);\n+\tstatsProcessed.emit(statsBufferId);\n \n \t/* Sanity check */\n \tif (!sensorControls.contains(V4L2_CID_EXPOSURE) ||\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex ebead8f0..d9791ab1 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -295,7 +295,7 @@ private:\n \tvoid conversionInputDone(FrameBuffer *buffer);\n \tvoid conversionOutputDone(FrameBuffer *buffer);\n \n-\tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n+\tvoid ispStatsReady(uint32_t frame, const uint32_t statsBufferId);\n \tvoid setSensorControls(const ControlList &sensorControls);\n };\n \n@@ -901,10 +901,11 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n \t\tpipe->completeRequest(request);\n }\n \n-void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)\n+void SimpleCameraData::ispStatsReady(uint32_t frame,\n+\t\t\t\t const uint32_t statsBufferId)\n {\n \tswIsp_->processStats(\n-\t\tframe, bufferId,\n+\t\tframe, statsBufferId,\n \t\tdelayedCtrls_->get(frame));\n }\n \ndiff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp\nindex 52df8c23..af54bb18 100644\n--- a/src/libcamera/software_isp/debayer.cpp\n+++ b/src/libcamera/software_isp/debayer.cpp\n@@ -94,9 +94,10 @@ Debayer::~Debayer()\n */\n \n /**\n- * \\fn void Debayer::process(uint32_t frame, const uint32_t paramsBufferId, FrameBuffer *input, FrameBuffer *output)\n+ * \\fn void Debayer::process(uint32_t frame, const uint32_t statsBufferId, const uint32_t paramsBufferId, FrameBuffer *input, FrameBuffer *output)\n * \\brief Process the bayer data into the requested format\n * \\param[in] frame The frame number\n+ * \\param[in] statsBufferId The id of the stats buffer to use\n * \\param[in] paramsBufferId The id of the params buffer in use\n * \\param[in] input The input buffer\n * \\param[in] output The output buffer\ndiff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h\nindex 251b14fd..0be7c59a 100644\n--- a/src/libcamera/software_isp/debayer.h\n+++ b/src/libcamera/software_isp/debayer.h\n@@ -41,7 +41,7 @@ public:\n \tstrideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0;\n \n \tvirtual void process(uint32_t frame,\n-\t\t\t const uint32_t paramsBufferId,\n+\t\t\t const uint32_t statsBufferId, const uint32_t paramsBufferId,\n \t\t\t FrameBuffer *input, FrameBuffer *output) = 0;\n \n \tvirtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;\ndiff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp\nindex d3831474..14bd58b5 100644\n--- a/src/libcamera/software_isp/debayer_cpu.cpp\n+++ b/src/libcamera/software_isp/debayer_cpu.cpp\n@@ -742,7 +742,7 @@ static inline int64_t timeDiff(timespec &after, timespec &before)\n }\n \n void DebayerCpu::process(uint32_t frame,\n-\t\t\t const uint32_t paramsBufferId,\n+\t\t\t const uint32_t statsBufferId, const uint32_t paramsBufferId,\n \t\t\t FrameBuffer *input, FrameBuffer *output)\n {\n \ttimespec frameStartTime;\n@@ -772,7 +772,7 @@ void DebayerCpu::process(uint32_t frame,\n \t\treturn;\n \t}\n \n-\tstats_->startFrame();\n+\tstats_->startFrame(statsBufferId);\n \n \tif (inputConfig_.patternSize.height == 2)\n \t\tprocess2(in.planes()[0].data(), out.planes()[0].data());\n@@ -799,12 +799,7 @@ void DebayerCpu::process(uint32_t frame,\n \t\t}\n \t}\n \n-\t/*\n-\t * Buffer ids are currently not used, so pass zeros as its parameter.\n-\t *\n-\t * \\todo Pass real bufferId once stats buffer passing is changed.\n-\t */\n-\tstats_->finishFrame(frame, 0);\n+\tstats_->finishFrame(frame, statsBufferId);\n \toutputBufferReady.emit(output);\n \tinputBufferReady.emit(input);\n }\ndiff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\nindex f25520be..7fb399b0 100644\n--- a/src/libcamera/software_isp/debayer_cpu.h\n+++ b/src/libcamera/software_isp/debayer_cpu.h\n@@ -39,7 +39,7 @@ public:\n \tstd::tuple<unsigned int, unsigned int>\n \tstrideAndFrameSize(const PixelFormat &outputFormat, const Size &size);\n \tvoid process(uint32_t frame,\n-\t\t const uint32_t paramsBufferId,\n+\t\t const uint32_t statsBufferId, const uint32_t paramsBufferId,\n \t\t FrameBuffer *input, FrameBuffer *output);\n \tSizeRange sizes(PixelFormat inputFormat, const Size &inputSize);\n \ndiff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\nindex db77f6f9..5d9f5008 100644\n--- a/src/libcamera/software_isp/software_isp.cpp\n+++ b/src/libcamera/software_isp/software_isp.cpp\n@@ -120,6 +120,7 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,\n \t\treturn;\n \t}\n \n+\tipa_->statsProcessed.connect(this, &SoftwareIsp::statsProcessed);\n \tipa_->setIspParams.connect(this, &SoftwareIsp::saveIspParams);\n \tipa_->setSensorControls.connect(this, &SoftwareIsp::setSensorCtrls);\n \n@@ -386,8 +387,10 @@ void SoftwareIsp::process(uint32_t frame, FrameBuffer *input, FrameBuffer *outpu\n \tconst uint32_t paramsBufferId = availableParams_.front();\n \tavailableParams_.pop();\n \tipa_->prepare(frame, paramsBufferId);\n+\tconst uint32_t statsBufferId = 0;\n \tdebayer_->invokeMethod(&DebayerCpu::process,\n-\t\t\t ConnectionTypeQueued, frame, paramsBufferId, input, output);\n+\t\t\t ConnectionTypeQueued, frame,\n+\t\t\t statsBufferId, paramsBufferId, input, output);\n }\n \n void SoftwareIsp::saveIspParams(uint32_t paramsBufferId)\n@@ -405,9 +408,13 @@ void SoftwareIsp::setSensorCtrls(const ControlList &sensorControls)\n \tsetSensorControls.emit(sensorControls);\n }\n \n-void SoftwareIsp::statsReady(uint32_t frame, uint32_t bufferId)\n+void SoftwareIsp::statsReady(uint32_t frame, const uint32_t statsBufferId)\n+{\n+\tispStatsReady.emit(frame, statsBufferId);\n+}\n+\n+void SoftwareIsp::statsProcessed([[maybe_unused]] const uint32_t statsBufferId)\n {\n-\tispStatsReady.emit(frame, bufferId);\n }\n \n void SoftwareIsp::inputReady(FrameBuffer *input)\ndiff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\nindex a9b1f35a..b8ee06a0 100644\n--- a/src/libcamera/software_isp/swstats_cpu.cpp\n+++ b/src/libcamera/software_isp/swstats_cpu.cpp\n@@ -298,7 +298,7 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n *\n * This may only be called after a successful setWindow() call.\n */\n-void SwStatsCpu::startFrame(void)\n+void SwStatsCpu::startFrame([[maybe_unused]] const uint32_t statsBufferId)\n {\n \tif (window_.width == 0)\n \t\tLOG(SwStatsCpu, Error) << \"Calling startFrame() without setWindow()\";\n@@ -314,10 +314,10 @@ void SwStatsCpu::startFrame(void)\n *\n * This may only be called after a successful setWindow() call.\n */\n-void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId)\n+void SwStatsCpu::finishFrame(uint32_t frame, const uint32_t statsBufferId)\n {\n \t*sharedStats_ = stats_;\n-\tstatsReady.emit(frame, bufferId);\n+\tstatsReady.emit(frame, statsBufferId);\n }\n \n /**\ndiff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h\nindex 26a2f462..402eef2d 100644\n--- a/src/libcamera/software_isp/swstats_cpu.h\n+++ b/src/libcamera/software_isp/swstats_cpu.h\n@@ -40,8 +40,8 @@ public:\n \n \tint configure(const StreamConfiguration &inputCfg);\n \tvoid setWindow(const Rectangle &window);\n-\tvoid startFrame();\n-\tvoid finishFrame(uint32_t frame, uint32_t bufferId);\n+\tvoid startFrame(const uint32_t statsBufferId);\n+\tvoid finishFrame(uint32_t frame, const uint32_t statsBufferId);\n \n \tvoid processLine0(unsigned int y, const uint8_t *src[])\n \t{\n", "prefixes": [ "12/16" ] }