Patch Detail
Show a patch.
GET /api/1.1/patches/20889/?format=api
{ "id": 20889, "url": "https://patchwork.libcamera.org/api/1.1/patches/20889/?format=api", "web_url": "https://patchwork.libcamera.org/patch/20889/", "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-16-mzamazal@redhat.com>", "date": "2024-08-12T11:50:04", "name": "[15/16] libcamera: software_isp: Share statistics buffers with IPA", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "0efd5eb6a6e61c4e74f7012b9eb00144401d4208", "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/20889/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/20889/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/20889/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 26491C324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 Aug 2024 11:50:56 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84476633D2;\n\tMon, 12 Aug 2024 13:50:55 +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 3A7D0633D3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 Aug 2024 13:50:46 +0200 (CEST)", "from mx-prod-mc-04.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-203-Co5p2UpFPfOhYX5gNpuHUg-1;\n\tMon, 12 Aug 2024 07:50:43 -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-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS\n\tid D91A6192538C for <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 Aug 2024 11:50:42 +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 A3F3319772C4; Mon, 12 Aug 2024 11:50:41 +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=\"JP09QSAF\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1723463445;\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=e+8dUtgJJp1QhBqBjIxq7uQCwmxP1iH+rGvy3/Mu6ps=;\n\tb=JP09QSAFhhmtDXz5oP38/j5/GccpQqVKoKJ5+2JyrpFW43RQPPzz18r7MWRKtcvcQOyXF0\n\tRLM2EgeeCZtCQaEO0MOVGTZrCBoMMMV3CW3I9y+WblC8kxQ41o3Mkq50T+etL95MBqKTN+\n\th1jInauEPindvB6Qzddq6a0oDeFIt/8=", "X-MC-Unique": "Co5p2UpFPfOhYX5gNpuHUg-1", "From": "Milan Zamazal <mzamazal@redhat.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Milan Zamazal <mzamazal@redhat.com>", "Subject": "[PATCH 15/16] libcamera: software_isp: Share statistics buffers with\n\tIPA", "Date": "Mon, 12 Aug 2024 13:50:04 +0200", "Message-ID": "<20240812115009.946036-16-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": "The last step to complete statistics buffer sharing is to pass all the\nallocated statistics buffers to the IPA and refer to them using their ids.\n\nThis allows to remove the buffer copying in SwStatsCpu::finishFrame.\nIn order to track the current statistics buffer in SwStatsCpu instead,\nwe change SwStatsCpu::stats_ to a wrapper structure. This is because we\nneed a reference to a shared mem object but a class attribute cannot be\na dynamically assigned reference. This hack works around the problem.\n\nWe can also remove now the methods that served for handling the former\nsingle buffer shared between debayering and IPA.\n\nSigned-off-by: Milan Zamazal <mzamazal@redhat.com>\n---\n include/libcamera/ipa/soft.mojom | 2 +-\n src/ipa/simple/soft_simple.cpp | 43 +++++++++++----------\n src/libcamera/software_isp/debayer_cpu.h | 7 ----\n src/libcamera/software_isp/software_isp.cpp | 2 +-\n src/libcamera/software_isp/swstats_cpu.cpp | 37 ++++++------------\n src/libcamera/software_isp/swstats_cpu.h | 12 +++---\n 6 files changed, 43 insertions(+), 60 deletions(-)", "diff": "diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom\nindex 7b85c454..e5af50d6 100644\n--- a/include/libcamera/ipa/soft.mojom\n+++ b/include/libcamera/ipa/soft.mojom\n@@ -14,7 +14,7 @@ struct IPAConfigInfo {\n \n interface IPASoftInterface {\n \tinit(libcamera.IPASettings settings,\n-\t libcamera.SharedFD fdStats,\n+\t array<libcamera.SharedFD> fdStats,\n \t array<libcamera.SharedFD> fdParams,\n \t libcamera.ControlInfoMap sensorCtrlInfoMap)\n \t\t=> (int32 ret);\ndiff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\nindex 3c95c4d9..2769bce2 100644\n--- a/src/ipa/simple/soft_simple.cpp\n+++ b/src/ipa/simple/soft_simple.cpp\n@@ -47,7 +47,7 @@ public:\n \t~IPASoftSimple();\n \n \tint init(const IPASettings &settings,\n-\t\t const SharedFD &fdStats,\n+\t\t const std::vector<SharedFD> &fdStats,\n \t\t const std::vector<SharedFD> &fdParams,\n \t\t const ControlInfoMap &sensorInfoMap) override;\n \tint configure(const IPAConfigInfo &configInfo) override;\n@@ -67,7 +67,7 @@ private:\n \tvoid updateExposure(double exposureMSV);\n \n \tstd::map<unsigned int, DebayerParams *> paramsBuffers_;\n-\tSwIspStats *stats_;\n+\tstd::map<unsigned int, SwIspStats *> statsBuffers_;\n \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n \tControlInfoMap sensorInfoMap_;\n \n@@ -77,14 +77,14 @@ private:\n \n IPASoftSimple::~IPASoftSimple()\n {\n-\tif (stats_)\n-\t\tmunmap(stats_, sizeof(SwIspStats));\n+\tfor (auto &item : statsBuffers_)\n+\t\tmunmap(item.second, sizeof(SwIspStats));\n \tfor (auto &item : paramsBuffers_)\n \t\tmunmap(item.second, sizeof(DebayerParams));\n }\n \n int IPASoftSimple::init(const IPASettings &settings,\n-\t\t\tconst SharedFD &fdStats,\n+\t\t\tconst std::vector<SharedFD> &fdStats,\n \t\t\tconst std::vector<SharedFD> &fdParams,\n \t\t\tconst ControlInfoMap &sensorInfoMap)\n {\n@@ -122,11 +122,21 @@ int IPASoftSimple::init(const IPASettings &settings,\n \tif (ret)\n \t\treturn ret;\n \n-\tstats_ = nullptr;\n+\tfor (auto &sharedFd : fdStats) {\n+\t\tif (!sharedFd.isValid()) {\n+\t\t\tLOG(IPASoft, Error) << \"Invalid Statistics handle\";\n+\t\t\treturn -ENODEV;\n+\t\t}\n \n-\tif (!fdStats.isValid()) {\n-\t\tLOG(IPASoft, Error) << \"Invalid Statistics handle\";\n-\t\treturn -ENODEV;\n+\t\tvoid *mem = mmap(nullptr, sizeof(SwIspStats), PROT_READ,\n+\t\t\t\t MAP_SHARED, sharedFd.get(), 0);\n+\t\tif (mem == MAP_FAILED) {\n+\t\t\tLOG(IPASoft, Error) << \"Unable to map Statistics\";\n+\t\t\treturn -errno;\n+\t\t}\n+\n+\t\tASSERT(sharedFd.get() >= 0);\n+\t\tstatsBuffers_[sharedFd.get()] = static_cast<SwIspStats *>(mem);\n \t}\n \n \tfor (auto &sharedFd : fdParams) {\n@@ -146,17 +156,6 @@ int IPASoftSimple::init(const IPASettings &settings,\n \t\tparamsBuffers_[sharedFd.get()] = static_cast<DebayerParams *>(mem);\n \t}\n \n-\t{\n-\t\tvoid *mem = mmap(nullptr, sizeof(SwIspStats), PROT_READ,\n-\t\t\t\t MAP_SHARED, fdStats.get(), 0);\n-\t\tif (mem == MAP_FAILED) {\n-\t\t\tLOG(IPASoft, Error) << \"Unable to map Statistics\";\n-\t\t\treturn -errno;\n-\t\t}\n-\n-\t\tstats_ = static_cast<SwIspStats *>(mem);\n-\t}\n-\n \t/*\n \t * Check if the sensor driver supports the controls required by the\n \t * Soft IPA.\n@@ -271,6 +270,8 @@ void IPASoftSimple::processStats(\n {\n \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n \n+\tconst SwIspStats *stats = statsBuffers_.at(statsBufferId);\n+\n \tframeContext.sensor.exposure =\n \t\tsensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();\n \tint32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();\n@@ -284,7 +285,7 @@ void IPASoftSimple::processStats(\n \t */\n \tControlList metadata(controls::controls);\n \tfor (auto const &algo : algorithms())\n-\t\talgo->process(context_, frame, frameContext, stats_, metadata);\n+\t\talgo->process(context_, frame, frameContext, stats, metadata);\n \tstatsProcessed.emit(statsBufferId);\n \n \t/* Sanity check */\ndiff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h\nindex 7fb399b0..36300534 100644\n--- a/src/libcamera/software_isp/debayer_cpu.h\n+++ b/src/libcamera/software_isp/debayer_cpu.h\n@@ -43,13 +43,6 @@ public:\n \t\t FrameBuffer *input, FrameBuffer *output);\n \tSizeRange sizes(PixelFormat inputFormat, const Size &inputSize);\n \n-\t/**\n-\t * \\brief Get the file descriptor for the statistics\n-\t *\n-\t * \\return the file descriptor pointing to the statistics\n-\t */\n-\tconst SharedFD &getStatsFD() { return stats_->getStatsFD(); }\n-\n \t/**\n \t * \\brief Get the output frame size\n \t *\ndiff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\nindex c4ad22be..025093a9 100644\n--- a/src/libcamera/software_isp/software_isp.cpp\n+++ b/src/libcamera/software_isp/software_isp.cpp\n@@ -113,7 +113,7 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,\n \t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n \n \tint ret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },\n-\t\t\t debayer_->getStatsFD(),\n+\t\t\t fdStats,\n \t\t\t fdParams,\n \t\t\t sensor->controls());\n \tif (ret) {\ndiff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp\nindex cb411a18..2c7acf47 100644\n--- a/src/libcamera/software_isp/swstats_cpu.cpp\n+++ b/src/libcamera/software_isp/swstats_cpu.cpp\n@@ -36,20 +36,6 @@ namespace libcamera {\n * instead of processing the whole frame.\n */\n \n-/**\n- * \\fn bool SwStatsCpu::isValid() const\n- * \\brief Gets whether the statistics object is valid\n- *\n- * \\return True if it's valid, false otherwise\n- */\n-\n-/**\n- * \\fn const SharedFD &SwStatsCpu::getStatsFD()\n- * \\brief Get the file descriptor for the statistics\n- *\n- * \\return The file descriptor\n- */\n-\n /**\n * \\fn const Size &SwStatsCpu::patternSize()\n * \\brief Get the pattern size\n@@ -161,12 +147,12 @@ static constexpr unsigned int kBlueYMul = 29; /* 0.114 * 256 */\n \tyVal = r * kRedYMul; \\\n \tyVal += g * kGreenYMul; \\\n \tyVal += b * kBlueYMul; \\\n-\tstats_.yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;\n+\tstats_->stats->yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;\n \n-#define SWSTATS_FINISH_LINE_STATS() \\\n-\tstats_.sumR_ += sumR; \\\n-\tstats_.sumG_ += sumG; \\\n-\tstats_.sumB_ += sumB;\n+#define SWSTATS_FINISH_LINE_STATS() \\\n+\tstats_->stats->sumR_ += sumR; \\\n+\tstats_->stats->sumG_ += sumG; \\\n+\tstats_->stats->sumB_ += sumB;\n \n void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])\n {\n@@ -302,15 +288,17 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])\n *\n * This may only be called after a successful setWindow() call.\n */\n-void SwStatsCpu::startFrame([[maybe_unused]] const uint32_t statsBufferId)\n+void SwStatsCpu::startFrame(const uint32_t statsBufferId)\n {\n \tif (window_.width == 0)\n \t\tLOG(SwStatsCpu, Error) << \"Calling startFrame() without setWindow()\";\n \n-\tstats_.sumR_ = 0;\n-\tstats_.sumB_ = 0;\n-\tstats_.sumG_ = 0;\n-\tstats_.yHistogram.fill(0);\n+\tauto &s = sharedStats_->at(statsBufferId);\n+\tstats_ = std::make_unique<SwIspStatsRef>(s);\n+\tstats_->stats->sumR_ = 0;\n+\tstats_->stats->sumB_ = 0;\n+\tstats_->stats->sumG_ = 0;\n+\tstats_->stats->yHistogram.fill(0);\n }\n \n /**\n@@ -320,7 +308,6 @@ void SwStatsCpu::startFrame([[maybe_unused]] const uint32_t statsBufferId)\n */\n void SwStatsCpu::finishFrame(uint32_t frame, const uint32_t statsBufferId)\n {\n-\t*(sharedStats_->at(statsBufferId)) = stats_;\n \tstatsReady.emit(frame, statsBufferId);\n }\n \ndiff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h\nindex fbdea4a3..2b4a98c2 100644\n--- a/src/libcamera/software_isp/swstats_cpu.h\n+++ b/src/libcamera/software_isp/swstats_cpu.h\n@@ -12,6 +12,7 @@\n #pragma once\n \n #include <map>\n+#include <memory>\n #include <stdint.h>\n \n #include <libcamera/base/signal.h>\n@@ -33,10 +34,6 @@ public:\n \tSwStatsCpu(std::unique_ptr<std::map<uint32_t, SharedMemObject<SwIspStats>>> sharedStats);\n \t~SwStatsCpu() = default;\n \n-\tbool isValid() const { return sharedStats_->begin()->second.fd().isValid(); }\n-\n-\tconst SharedFD &getStatsFD() { return sharedStats_->begin()->second.fd(); }\n-\n \tconst Size &patternSize() { return patternSize_; }\n \n \tint configure(const StreamConfiguration &inputCfg);\n@@ -92,7 +89,12 @@ private:\n \tunsigned int xShift_;\n \n \tstd::unique_ptr<std::map<uint32_t, SharedMemObject<SwIspStats>>> sharedStats_;\n-\tSwIspStats stats_;\n+\tstruct SwIspStatsRef {\n+\t\tSharedMemObject<SwIspStats> &stats;\n+\t\tSwIspStatsRef(SharedMemObject<SwIspStats> &_stats)\n+\t\t\t: stats(_stats){};\n+\t};\n+\tstd::unique_ptr<SwIspStatsRef> stats_;\n };\n \n } /* namespace libcamera */\n", "prefixes": [ "15/16" ] }