From patchwork Mon Aug 12 11:50:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Milan Zamazal X-Patchwork-Id: 20889 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 26491C324E for ; Mon, 12 Aug 2024 11:50:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 84476633D2; Mon, 12 Aug 2024 13:50:55 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.b="JP09QSAF"; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A7D0633D3 for ; Mon, 12 Aug 2024 13:50:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723463445; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=e+8dUtgJJp1QhBqBjIxq7uQCwmxP1iH+rGvy3/Mu6ps=; b=JP09QSAFhhmtDXz5oP38/j5/GccpQqVKoKJ5+2JyrpFW43RQPPzz18r7MWRKtcvcQOyXF0 RLM2EgeeCZtCQaEO0MOVGTZrCBoMMMV3CW3I9y+WblC8kxQ41o3Mkq50T+etL95MBqKTN+ h1jInauEPindvB6Qzddq6a0oDeFIt/8= Received: from mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-203-Co5p2UpFPfOhYX5gNpuHUg-1; Mon, 12 Aug 2024 07:50:43 -0400 X-MC-Unique: Co5p2UpFPfOhYX5gNpuHUg-1 Received: from mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id D91A6192538C for ; Mon, 12 Aug 2024 11:50:42 +0000 (UTC) Received: from nuthatch.redhat.com (unknown [10.45.225.57]) by mx-prod-int-02.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id A3F3319772C4; Mon, 12 Aug 2024 11:50:41 +0000 (UTC) From: Milan Zamazal To: libcamera-devel@lists.libcamera.org Cc: Milan Zamazal Subject: [PATCH 15/16] libcamera: software_isp: Share statistics buffers with IPA 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 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The last step to complete statistics buffer sharing is to pass all the allocated statistics buffers to the IPA and refer to them using their ids. This allows to remove the buffer copying in SwStatsCpu::finishFrame. In order to track the current statistics buffer in SwStatsCpu instead, we change SwStatsCpu::stats_ to a wrapper structure. This is because we need a reference to a shared mem object but a class attribute cannot be a dynamically assigned reference. This hack works around the problem. We can also remove now the methods that served for handling the former single buffer shared between debayering and IPA. Signed-off-by: Milan Zamazal --- include/libcamera/ipa/soft.mojom | 2 +- src/ipa/simple/soft_simple.cpp | 43 +++++++++++---------- src/libcamera/software_isp/debayer_cpu.h | 7 ---- src/libcamera/software_isp/software_isp.cpp | 2 +- src/libcamera/software_isp/swstats_cpu.cpp | 37 ++++++------------ src/libcamera/software_isp/swstats_cpu.h | 12 +++--- 6 files changed, 43 insertions(+), 60 deletions(-) diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom index 7b85c454..e5af50d6 100644 --- a/include/libcamera/ipa/soft.mojom +++ b/include/libcamera/ipa/soft.mojom @@ -14,7 +14,7 @@ struct IPAConfigInfo { interface IPASoftInterface { init(libcamera.IPASettings settings, - libcamera.SharedFD fdStats, + array fdStats, array fdParams, libcamera.ControlInfoMap sensorCtrlInfoMap) => (int32 ret); diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index 3c95c4d9..2769bce2 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -47,7 +47,7 @@ public: ~IPASoftSimple(); int init(const IPASettings &settings, - const SharedFD &fdStats, + const std::vector &fdStats, const std::vector &fdParams, const ControlInfoMap &sensorInfoMap) override; int configure(const IPAConfigInfo &configInfo) override; @@ -67,7 +67,7 @@ private: void updateExposure(double exposureMSV); std::map paramsBuffers_; - SwIspStats *stats_; + std::map statsBuffers_; std::unique_ptr camHelper_; ControlInfoMap sensorInfoMap_; @@ -77,14 +77,14 @@ private: IPASoftSimple::~IPASoftSimple() { - if (stats_) - munmap(stats_, sizeof(SwIspStats)); + for (auto &item : statsBuffers_) + munmap(item.second, sizeof(SwIspStats)); for (auto &item : paramsBuffers_) munmap(item.second, sizeof(DebayerParams)); } int IPASoftSimple::init(const IPASettings &settings, - const SharedFD &fdStats, + const std::vector &fdStats, const std::vector &fdParams, const ControlInfoMap &sensorInfoMap) { @@ -122,11 +122,21 @@ int IPASoftSimple::init(const IPASettings &settings, if (ret) return ret; - stats_ = nullptr; + for (auto &sharedFd : fdStats) { + if (!sharedFd.isValid()) { + LOG(IPASoft, Error) << "Invalid Statistics handle"; + return -ENODEV; + } - if (!fdStats.isValid()) { - LOG(IPASoft, Error) << "Invalid Statistics handle"; - return -ENODEV; + void *mem = mmap(nullptr, sizeof(SwIspStats), PROT_READ, + MAP_SHARED, sharedFd.get(), 0); + if (mem == MAP_FAILED) { + LOG(IPASoft, Error) << "Unable to map Statistics"; + return -errno; + } + + ASSERT(sharedFd.get() >= 0); + statsBuffers_[sharedFd.get()] = static_cast(mem); } for (auto &sharedFd : fdParams) { @@ -146,17 +156,6 @@ int IPASoftSimple::init(const IPASettings &settings, paramsBuffers_[sharedFd.get()] = static_cast(mem); } - { - void *mem = mmap(nullptr, sizeof(SwIspStats), PROT_READ, - MAP_SHARED, fdStats.get(), 0); - if (mem == MAP_FAILED) { - LOG(IPASoft, Error) << "Unable to map Statistics"; - return -errno; - } - - stats_ = static_cast(mem); - } - /* * Check if the sensor driver supports the controls required by the * Soft IPA. @@ -271,6 +270,8 @@ void IPASoftSimple::processStats( { IPAFrameContext &frameContext = context_.frameContexts.get(frame); + const SwIspStats *stats = statsBuffers_.at(statsBufferId); + frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get(); int32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get(); @@ -284,7 +285,7 @@ void IPASoftSimple::processStats( */ ControlList metadata(controls::controls); for (auto const &algo : algorithms()) - algo->process(context_, frame, frameContext, stats_, metadata); + algo->process(context_, frame, frameContext, stats, metadata); statsProcessed.emit(statsBufferId); /* Sanity check */ diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h index 7fb399b0..36300534 100644 --- a/src/libcamera/software_isp/debayer_cpu.h +++ b/src/libcamera/software_isp/debayer_cpu.h @@ -43,13 +43,6 @@ public: FrameBuffer *input, FrameBuffer *output); SizeRange sizes(PixelFormat inputFormat, const Size &inputSize); - /** - * \brief Get the file descriptor for the statistics - * - * \return the file descriptor pointing to the statistics - */ - const SharedFD &getStatsFD() { return stats_->getStatsFD(); } - /** * \brief Get the output frame size * diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index c4ad22be..025093a9 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -113,7 +113,7 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor, ipa_->configurationFile(sensor->model() + ".yaml", "uncalibrated.yaml"); int ret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() }, - debayer_->getStatsFD(), + fdStats, fdParams, sensor->controls()); if (ret) { diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp index cb411a18..2c7acf47 100644 --- a/src/libcamera/software_isp/swstats_cpu.cpp +++ b/src/libcamera/software_isp/swstats_cpu.cpp @@ -36,20 +36,6 @@ namespace libcamera { * instead of processing the whole frame. */ -/** - * \fn bool SwStatsCpu::isValid() const - * \brief Gets whether the statistics object is valid - * - * \return True if it's valid, false otherwise - */ - -/** - * \fn const SharedFD &SwStatsCpu::getStatsFD() - * \brief Get the file descriptor for the statistics - * - * \return The file descriptor - */ - /** * \fn const Size &SwStatsCpu::patternSize() * \brief Get the pattern size @@ -161,12 +147,12 @@ static constexpr unsigned int kBlueYMul = 29; /* 0.114 * 256 */ yVal = r * kRedYMul; \ yVal += g * kGreenYMul; \ yVal += b * kBlueYMul; \ - stats_.yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++; + stats_->stats->yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++; -#define SWSTATS_FINISH_LINE_STATS() \ - stats_.sumR_ += sumR; \ - stats_.sumG_ += sumG; \ - stats_.sumB_ += sumB; +#define SWSTATS_FINISH_LINE_STATS() \ + stats_->stats->sumR_ += sumR; \ + stats_->stats->sumG_ += sumG; \ + stats_->stats->sumB_ += sumB; void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[]) { @@ -302,15 +288,17 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[]) * * This may only be called after a successful setWindow() call. */ -void SwStatsCpu::startFrame([[maybe_unused]] const uint32_t statsBufferId) +void SwStatsCpu::startFrame(const uint32_t statsBufferId) { if (window_.width == 0) LOG(SwStatsCpu, Error) << "Calling startFrame() without setWindow()"; - stats_.sumR_ = 0; - stats_.sumB_ = 0; - stats_.sumG_ = 0; - stats_.yHistogram.fill(0); + auto &s = sharedStats_->at(statsBufferId); + stats_ = std::make_unique(s); + stats_->stats->sumR_ = 0; + stats_->stats->sumB_ = 0; + stats_->stats->sumG_ = 0; + stats_->stats->yHistogram.fill(0); } /** @@ -320,7 +308,6 @@ void SwStatsCpu::startFrame([[maybe_unused]] const uint32_t statsBufferId) */ void SwStatsCpu::finishFrame(uint32_t frame, const uint32_t statsBufferId) { - *(sharedStats_->at(statsBufferId)) = stats_; statsReady.emit(frame, statsBufferId); } diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h index fbdea4a3..2b4a98c2 100644 --- a/src/libcamera/software_isp/swstats_cpu.h +++ b/src/libcamera/software_isp/swstats_cpu.h @@ -12,6 +12,7 @@ #pragma once #include +#include #include #include @@ -33,10 +34,6 @@ public: SwStatsCpu(std::unique_ptr>> sharedStats); ~SwStatsCpu() = default; - bool isValid() const { return sharedStats_->begin()->second.fd().isValid(); } - - const SharedFD &getStatsFD() { return sharedStats_->begin()->second.fd(); } - const Size &patternSize() { return patternSize_; } int configure(const StreamConfiguration &inputCfg); @@ -92,7 +89,12 @@ private: unsigned int xShift_; std::unique_ptr>> sharedStats_; - SwIspStats stats_; + struct SwIspStatsRef { + SharedMemObject &stats; + SwIspStatsRef(SharedMemObject &_stats) + : stats(_stats){}; + }; + std::unique_ptr stats_; }; } /* namespace libcamera */