From patchwork Tue Apr 7 15:33:51 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 26446 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 22767C32F7 for ; Tue, 7 Apr 2026 15:34:43 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B1F1C62DB2; Tue, 7 Apr 2026 17:34:42 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="T38MO1P3"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A393162DB4 for ; Tue, 7 Apr 2026 17:34:36 +0200 (CEST) Received: from killaraus.ideasonboard.com (2001-14ba-703d-e500--2a1.rev.dnainternet.fi [IPv6:2001:14ba:703d:e500::2a1]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 13D46596 for ; Tue, 7 Apr 2026 17:33:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1775575989; bh=qBteUXCX+a0df/fse16sghyXjj/ZtdSN3WkmOX0/2hE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=T38MO1P36BL5dVOXhPB5XSYj/LEsnXkfvH4eETjnchUFFHbYZpr5zC66yQSoec3nR Ukn0FeZuLdGtEe4ismYrN4psF8WAIHAN2OlbTT/7W3N67RfYHsxQrxgXkR05bhYH7z v9CcYC4wtZR0cjlJyoPC+1jCnnaWzsPjxBoceKk4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 06/42] test: ipa: ipa_interface: Replace FIFO with pipe Date: Tue, 7 Apr 2026 18:33:51 +0300 Message-ID: <20260407153427.1825999-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.52.0 In-Reply-To: <20260407153427.1825999-1-laurent.pinchart@ideasonboard.com> References: <20260407153427.1825999-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 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 ipa_interface unit test uses a FIFO to communicate with the vimc IPA module. FIFOs are named pipes, created in the file system. The path to the FIFO is hardcoded so that both the unit test and IPA module know where to locate the file. If the ipa_interface crashes for any reason, the FIFO will not be removed, and subsequent usage of the vimc IPA module will hang when trying to write to the FIFO in the IPA module. Fix this by replacing the FIFO with a pipe. Pipes are unidirectional data channels that are represented by a pair of file descriptors, without any presence in the file system. The write end of the pipe is passed to the vimc IPA module init() function, and then used the same way as the FIFO. While at it, use a std::unique_ptr to manage the notifier in the unit test instead of manually allocating and deleting the object. Signed-off-by: Laurent Pinchart Reviewed-by: Barnabás Pőcze --- include/libcamera/ipa/vimc.mojom | 3 +- src/ipa/vimc/vimc.cpp | 33 ++++--------------- src/libcamera/pipeline/vimc/vimc.cpp | 2 +- test/ipa/ipa_interface_test.cpp | 49 +++++++++++----------------- 4 files changed, 28 insertions(+), 59 deletions(-) diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom index c5c5fe83734e..b1bee05a81a9 100644 --- a/include/libcamera/ipa/vimc.mojom +++ b/include/libcamera/ipa/vimc.mojom @@ -8,8 +8,6 @@ module ipa.vimc; import "include/libcamera/ipa/core.mojom"; -const string VimcIPAFIFOPath = "/tmp/libcamera_ipa_vimc_fifo"; - enum IPAOperationCode { IPAOperationNone, IPAOperationInit, @@ -26,6 +24,7 @@ enum IPAOperationCode { interface IPAVimcInterface { init(libcamera.IPASettings settings, + libcamera.SharedFD traceFd, IPAOperationCode code, [flags] TestFlag inFlags) => (int32 ret, [flags] TestFlag outFlags); diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp index a1351a0f45ce..4162b848f7b1 100644 --- a/src/ipa/vimc/vimc.cpp +++ b/src/ipa/vimc/vimc.cpp @@ -33,6 +33,7 @@ public: ~IPAVimc(); int init(const IPASettings &settings, + const SharedFD &traceFd, const ipa::vimc::IPAOperationCode code, const Flags inFlags, Flags *outFlags) override; @@ -51,30 +52,28 @@ public: void computeParams(uint32_t frame, uint32_t bufferId) override; private: - void initTrace(); void trace(enum ipa::vimc::IPAOperationCode operation); - int fd_; + SharedFD fd_; std::map buffers_; }; IPAVimc::IPAVimc() - : fd_(-1) { - initTrace(); } IPAVimc::~IPAVimc() { - if (fd_ != -1) - ::close(fd_); } int IPAVimc::init(const IPASettings &settings, + const SharedFD &traceFd, const ipa::vimc::IPAOperationCode code, const Flags inFlags, Flags *outFlags) { + fd_ = traceFd; + trace(ipa::vimc::IPAOperationInit); LOG(IPAVimc, Debug) @@ -162,30 +161,12 @@ void IPAVimc::computeParams([[maybe_unused]] uint32_t frame, uint32_t bufferId) paramsComputed.emit(bufferId, flags); } -void IPAVimc::initTrace() -{ - struct stat fifoStat; - int ret = stat(ipa::vimc::VimcIPAFIFOPath.c_str(), &fifoStat); - if (ret) - return; - - ret = ::open(ipa::vimc::VimcIPAFIFOPath.c_str(), O_WRONLY | O_CLOEXEC); - if (ret < 0) { - ret = errno; - LOG(IPAVimc, Error) << "Failed to open vimc IPA test FIFO: " - << strerror(ret); - return; - } - - fd_ = ret; -} - void IPAVimc::trace(enum ipa::vimc::IPAOperationCode operation) { - if (fd_ < 0) + if (!fd_.isValid()) return; - int ret = ::write(fd_, &operation, sizeof(operation)); + int ret = ::write(fd_.get(), &operation, sizeof(operation)); if (ret < 0) { ret = errno; LOG(IPAVimc, Error) << "Failed to write to vimc IPA test FIFO: " diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp index e915d5c2df23..6681c74ee564 100644 --- a/src/libcamera/pipeline/vimc/vimc.cpp +++ b/src/libcamera/pipeline/vimc/vimc.cpp @@ -498,7 +498,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) std::string conf = data->ipa_->configurationFile("vimc.conf"); Flags inFlags = ipa::vimc::TestFlag::Flag2; Flags outFlags; - data->ipa_->init(IPASettings{ conf, data->sensor_->model() }, + data->ipa_->init(IPASettings{ conf, data->sensor_->model() }, SharedFD{}, ipa::vimc::IPAOperationInit, inFlags, &outFlags); LOG(VIMC, Debug) diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp index 41ef81721bfa..c1fe2267cc6e 100644 --- a/test/ipa/ipa_interface_test.cpp +++ b/test/ipa/ipa_interface_test.cpp @@ -7,9 +7,8 @@ #include #include +#include #include -#include -#include #include #include @@ -17,8 +16,10 @@ #include #include #include +#include #include #include +#include #include "libcamera/internal/camera_manager.h" #include "libcamera/internal/device_enumerator.h" @@ -37,13 +38,13 @@ class IPAInterfaceTest : public Test, public Object { public: IPAInterfaceTest() - : trace_(ipa::vimc::IPAOperationNone), notifier_(nullptr), fd_(-1) + : trace_(ipa::vimc::IPAOperationNone) { } ~IPAInterfaceTest() { - delete notifier_; + notifier_.reset(); ipa_.reset(); ipaManager_.reset(); config_.reset(); @@ -70,28 +71,22 @@ protected: return TestPass; } - /* Create and open the communication FIFO. */ - int ret = mkfifo(ipa::vimc::VimcIPAFIFOPath.c_str(), S_IRUSR | S_IWUSR); + /* Create the communication pipe. */ + int pipefds[2]; + int ret = pipe2(pipefds, O_NONBLOCK); if (ret) { ret = errno; - cerr << "Failed to create IPA test FIFO at '" - << ipa::vimc::VimcIPAFIFOPath << "': " << strerror(ret) + cerr << "Failed to create IPA test pipe: " << strerror(ret) << endl; return TestFail; } - ret = open(ipa::vimc::VimcIPAFIFOPath.c_str(), O_RDONLY | O_NONBLOCK); - if (ret < 0) { - ret = errno; - cerr << "Failed to open IPA test FIFO at '" - << ipa::vimc::VimcIPAFIFOPath << "': " << strerror(ret) - << endl; - unlink(ipa::vimc::VimcIPAFIFOPath.c_str()); - return TestFail; - } - fd_ = ret; + pipeReadFd_ = UniqueFD(pipefds[0]); + pipeWriteFd_ = SharedFD(pipefds[1]); - notifier_ = new EventNotifier(fd_, EventNotifier::Read, this); + notifier_ = std::make_unique(pipeReadFd_.get(), + EventNotifier::Read, + this); notifier_->activated.connect(this, &IPAInterfaceTest::readTrace); /* Create the IPA manager. */ @@ -116,7 +111,7 @@ protected: std::string conf = ipa_->configurationFile("vimc.conf"); Flags inFlags; Flags outFlags; - int ret = ipa_->init(IPASettings{ conf, "vimc" }, + int ret = ipa_->init(IPASettings{ conf, "vimc" }, pipeWriteFd_, ipa::vimc::IPAOperationInit, inFlags, &outFlags); if (ret < 0) { @@ -159,20 +154,13 @@ protected: return TestPass; } - void cleanup() override - { - close(fd_); - unlink(ipa::vimc::VimcIPAFIFOPath.c_str()); - } - private: void readTrace() { ssize_t s = read(notifier_->fd(), &trace_, sizeof(trace_)); if (s < 0) { int ret = errno; - cerr << "Failed to read from IPA test FIFO at '" - << ipa::vimc::VimcIPAFIFOPath << "': " << strerror(ret) + cerr << "Failed to read from IPA test pipe: " << strerror(ret) << endl; trace_ = ipa::vimc::IPAOperationNone; } @@ -184,8 +172,9 @@ private: std::unique_ptr config_; std::unique_ptr ipaManager_; enum ipa::vimc::IPAOperationCode trace_; - EventNotifier *notifier_; - int fd_; + std::unique_ptr notifier_; + UniqueFD pipeReadFd_; + SharedFD pipeWriteFd_; }; TEST_REGISTER(IPAInterfaceTest)