Patch Detail
Show a patch.
GET /api/1.1/patches/26446/?format=api
{ "id": 26446, "url": "https://patchwork.libcamera.org/api/1.1/patches/26446/?format=api", "web_url": "https://patchwork.libcamera.org/patch/26446/", "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": "<20260407153427.1825999-7-laurent.pinchart@ideasonboard.com>", "date": "2026-04-07T15:33:51", "name": "[v2,06/42] test: ipa: ipa_interface: Replace FIFO with pipe", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "595998e96b2b0970ef88437a785400a92a4064f9", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/26446/mbox/", "series": [ { "id": 5873, "url": "https://patchwork.libcamera.org/api/1.1/series/5873/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5873", "date": "2026-04-07T15:33:45", "name": "libcamera: Global configuration file improvements", "version": 2, "mbox": "https://patchwork.libcamera.org/series/5873/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/26446/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/26446/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 22767C32F7\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 7 Apr 2026 15:34:43 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B1F1C62DB2;\n\tTue, 7 Apr 2026 17:34:42 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A393162DB4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 7 Apr 2026 17:34:36 +0200 (CEST)", "from killaraus.ideasonboard.com\n\t(2001-14ba-703d-e500--2a1.rev.dnainternet.fi\n\t[IPv6:2001:14ba:703d:e500::2a1])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 13D46596\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 7 Apr 2026 17:33:09 +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=\"T38MO1P3\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1775575989;\n\tbh=qBteUXCX+a0df/fse16sghyXjj/ZtdSN3WkmOX0/2hE=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=T38MO1P36BL5dVOXhPB5XSYj/LEsnXkfvH4eETjnchUFFHbYZpr5zC66yQSoec3nR\n\tUkn0FeZuLdGtEe4ismYrN4psF8WAIHAN2OlbTT/7W3N67RfYHsxQrxgXkR05bhYH7z\n\tv9CcYC4wtZR0cjlJyoPC+1jCnnaWzsPjxBoceKk4=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "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", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "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 ipa_interface unit test uses a FIFO to communicate with the vimc IPA\nmodule. FIFOs are named pipes, created in the file system. The path to\nthe FIFO is hardcoded so that both the unit test and IPA module know\nwhere to locate the file.\n\nIf the ipa_interface crashes for any reason, the FIFO will not be\nremoved, and subsequent usage of the vimc IPA module will hang when\ntrying to write to the FIFO in the IPA module.\n\nFix this by replacing the FIFO with a pipe. Pipes are unidirectional\ndata channels that are represented by a pair of file descriptors,\nwithout any presence in the file system. The write end of the pipe is\npassed to the vimc IPA module init() function, and then used the same\nway as the FIFO.\n\nWhile at it, use a std::unique_ptr to manage the notifier in the unit\ntest instead of manually allocating and deleting the object.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n---\n include/libcamera/ipa/vimc.mojom | 3 +-\n src/ipa/vimc/vimc.cpp | 33 ++++---------------\n src/libcamera/pipeline/vimc/vimc.cpp | 2 +-\n test/ipa/ipa_interface_test.cpp | 49 +++++++++++-----------------\n 4 files changed, 28 insertions(+), 59 deletions(-)", "diff": "diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\nindex c5c5fe83734e..b1bee05a81a9 100644\n--- a/include/libcamera/ipa/vimc.mojom\n+++ b/include/libcamera/ipa/vimc.mojom\n@@ -8,8 +8,6 @@ module ipa.vimc;\n \n import \"include/libcamera/ipa/core.mojom\";\n \n-const string VimcIPAFIFOPath = \"/tmp/libcamera_ipa_vimc_fifo\";\n-\n enum IPAOperationCode {\n \tIPAOperationNone,\n \tIPAOperationInit,\n@@ -26,6 +24,7 @@ enum IPAOperationCode {\n \n interface IPAVimcInterface {\n \tinit(libcamera.IPASettings settings,\n+\t libcamera.SharedFD traceFd,\n \t IPAOperationCode code,\n \t [flags] TestFlag inFlags)\n \t=> (int32 ret, [flags] TestFlag outFlags);\ndiff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\nindex a1351a0f45ce..4162b848f7b1 100644\n--- a/src/ipa/vimc/vimc.cpp\n+++ b/src/ipa/vimc/vimc.cpp\n@@ -33,6 +33,7 @@ public:\n \t~IPAVimc();\n \n \tint init(const IPASettings &settings,\n+\t\t const SharedFD &traceFd,\n \t\t const ipa::vimc::IPAOperationCode code,\n \t\t const Flags<ipa::vimc::TestFlag> inFlags,\n \t\t Flags<ipa::vimc::TestFlag> *outFlags) override;\n@@ -51,30 +52,28 @@ public:\n \tvoid computeParams(uint32_t frame, uint32_t bufferId) override;\n \n private:\n-\tvoid initTrace();\n \tvoid trace(enum ipa::vimc::IPAOperationCode operation);\n \n-\tint fd_;\n+\tSharedFD fd_;\n \tstd::map<unsigned int, MappedFrameBuffer> buffers_;\n };\n \n IPAVimc::IPAVimc()\n-\t: fd_(-1)\n {\n-\tinitTrace();\n }\n \n IPAVimc::~IPAVimc()\n {\n-\tif (fd_ != -1)\n-\t\t::close(fd_);\n }\n \n int IPAVimc::init(const IPASettings &settings,\n+\t\t const SharedFD &traceFd,\n \t\t const ipa::vimc::IPAOperationCode code,\n \t\t const Flags<ipa::vimc::TestFlag> inFlags,\n \t\t Flags<ipa::vimc::TestFlag> *outFlags)\n {\n+\tfd_ = traceFd;\n+\n \ttrace(ipa::vimc::IPAOperationInit);\n \n \tLOG(IPAVimc, Debug)\n@@ -162,30 +161,12 @@ void IPAVimc::computeParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)\n \tparamsComputed.emit(bufferId, flags);\n }\n \n-void IPAVimc::initTrace()\n-{\n-\tstruct stat fifoStat;\n-\tint ret = stat(ipa::vimc::VimcIPAFIFOPath.c_str(), &fifoStat);\n-\tif (ret)\n-\t\treturn;\n-\n-\tret = ::open(ipa::vimc::VimcIPAFIFOPath.c_str(), O_WRONLY | O_CLOEXEC);\n-\tif (ret < 0) {\n-\t\tret = errno;\n-\t\tLOG(IPAVimc, Error) << \"Failed to open vimc IPA test FIFO: \"\n-\t\t\t\t << strerror(ret);\n-\t\treturn;\n-\t}\n-\n-\tfd_ = ret;\n-}\n-\n void IPAVimc::trace(enum ipa::vimc::IPAOperationCode operation)\n {\n-\tif (fd_ < 0)\n+\tif (!fd_.isValid())\n \t\treturn;\n \n-\tint ret = ::write(fd_, &operation, sizeof(operation));\n+\tint ret = ::write(fd_.get(), &operation, sizeof(operation));\n \tif (ret < 0) {\n \t\tret = errno;\n \t\tLOG(IPAVimc, Error) << \"Failed to write to vimc IPA test FIFO: \"\ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex e915d5c2df23..6681c74ee564 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -498,7 +498,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n \tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n \tFlags<ipa::vimc::TestFlag> inFlags = ipa::vimc::TestFlag::Flag2;\n \tFlags<ipa::vimc::TestFlag> outFlags;\n-\tdata->ipa_->init(IPASettings{ conf, data->sensor_->model() },\n+\tdata->ipa_->init(IPASettings{ conf, data->sensor_->model() }, SharedFD{},\n \t\t\t ipa::vimc::IPAOperationInit, inFlags, &outFlags);\n \n \tLOG(VIMC, Debug)\ndiff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\nindex 41ef81721bfa..c1fe2267cc6e 100644\n--- a/test/ipa/ipa_interface_test.cpp\n+++ b/test/ipa/ipa_interface_test.cpp\n@@ -7,9 +7,8 @@\n \n #include <fcntl.h>\n #include <iostream>\n+#include <memory>\n #include <string.h>\n-#include <sys/stat.h>\n-#include <sys/types.h>\n #include <unistd.h>\n \n #include <libcamera/ipa/vimc_ipa_proxy.h>\n@@ -17,8 +16,10 @@\n #include <libcamera/base/event_dispatcher.h>\n #include <libcamera/base/event_notifier.h>\n #include <libcamera/base/object.h>\n+#include <libcamera/base/shared_fd.h>\n #include <libcamera/base/thread.h>\n #include <libcamera/base/timer.h>\n+#include <libcamera/base/unique_fd.h>\n \n #include \"libcamera/internal/camera_manager.h\"\n #include \"libcamera/internal/device_enumerator.h\"\n@@ -37,13 +38,13 @@ class IPAInterfaceTest : public Test, public Object\n {\n public:\n \tIPAInterfaceTest()\n-\t\t: trace_(ipa::vimc::IPAOperationNone), notifier_(nullptr), fd_(-1)\n+\t\t: trace_(ipa::vimc::IPAOperationNone)\n \t{\n \t}\n \n \t~IPAInterfaceTest()\n \t{\n-\t\tdelete notifier_;\n+\t\tnotifier_.reset();\n \t\tipa_.reset();\n \t\tipaManager_.reset();\n \t\tconfig_.reset();\n@@ -70,28 +71,22 @@ protected:\n \t\t\treturn TestPass;\n \t\t}\n \n-\t\t/* Create and open the communication FIFO. */\n-\t\tint ret = mkfifo(ipa::vimc::VimcIPAFIFOPath.c_str(), S_IRUSR | S_IWUSR);\n+\t\t/* Create the communication pipe. */\n+\t\tint pipefds[2];\n+\t\tint ret = pipe2(pipefds, O_NONBLOCK);\n \t\tif (ret) {\n \t\t\tret = errno;\n-\t\t\tcerr << \"Failed to create IPA test FIFO at '\"\n-\t\t\t << ipa::vimc::VimcIPAFIFOPath << \"': \" << strerror(ret)\n+\t\t\tcerr << \"Failed to create IPA test pipe: \" << strerror(ret)\n \t\t\t << endl;\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tret = open(ipa::vimc::VimcIPAFIFOPath.c_str(), O_RDONLY | O_NONBLOCK);\n-\t\tif (ret < 0) {\n-\t\t\tret = errno;\n-\t\t\tcerr << \"Failed to open IPA test FIFO at '\"\n-\t\t\t << ipa::vimc::VimcIPAFIFOPath << \"': \" << strerror(ret)\n-\t\t\t << endl;\n-\t\t\tunlink(ipa::vimc::VimcIPAFIFOPath.c_str());\n-\t\t\treturn TestFail;\n-\t\t}\n-\t\tfd_ = ret;\n+\t\tpipeReadFd_ = UniqueFD(pipefds[0]);\n+\t\tpipeWriteFd_ = SharedFD(pipefds[1]);\n \n-\t\tnotifier_ = new EventNotifier(fd_, EventNotifier::Read, this);\n+\t\tnotifier_ = std::make_unique<EventNotifier>(pipeReadFd_.get(),\n+\t\t\t\t\t\t\t EventNotifier::Read,\n+\t\t\t\t\t\t\t this);\n \t\tnotifier_->activated.connect(this, &IPAInterfaceTest::readTrace);\n \n \t\t/* Create the IPA manager. */\n@@ -116,7 +111,7 @@ protected:\n \t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\");\n \t\tFlags<ipa::vimc::TestFlag> inFlags;\n \t\tFlags<ipa::vimc::TestFlag> outFlags;\n-\t\tint ret = ipa_->init(IPASettings{ conf, \"vimc\" },\n+\t\tint ret = ipa_->init(IPASettings{ conf, \"vimc\" }, pipeWriteFd_,\n \t\t\t\t ipa::vimc::IPAOperationInit,\n \t\t\t\t inFlags, &outFlags);\n \t\tif (ret < 0) {\n@@ -159,20 +154,13 @@ protected:\n \t\treturn TestPass;\n \t}\n \n-\tvoid cleanup() override\n-\t{\n-\t\tclose(fd_);\n-\t\tunlink(ipa::vimc::VimcIPAFIFOPath.c_str());\n-\t}\n-\n private:\n \tvoid readTrace()\n \t{\n \t\tssize_t s = read(notifier_->fd(), &trace_, sizeof(trace_));\n \t\tif (s < 0) {\n \t\t\tint ret = errno;\n-\t\t\tcerr << \"Failed to read from IPA test FIFO at '\"\n-\t\t\t << ipa::vimc::VimcIPAFIFOPath << \"': \" << strerror(ret)\n+\t\t\tcerr << \"Failed to read from IPA test pipe: \" << strerror(ret)\n \t\t\t << endl;\n \t\t\ttrace_ = ipa::vimc::IPAOperationNone;\n \t\t}\n@@ -184,8 +172,9 @@ private:\n \tstd::unique_ptr<GlobalConfiguration> config_;\n \tstd::unique_ptr<IPAManager> ipaManager_;\n \tenum ipa::vimc::IPAOperationCode trace_;\n-\tEventNotifier *notifier_;\n-\tint fd_;\n+\tstd::unique_ptr<EventNotifier> notifier_;\n+\tUniqueFD pipeReadFd_;\n+\tSharedFD pipeWriteFd_;\n };\n \n TEST_REGISTER(IPAInterfaceTest)\n", "prefixes": [ "v2", "06/42" ] }