[{"id":37589,"web_url":"https://patchwork.libcamera.org/comment/37589/","msgid":"","date":"2026-01-13T09:05:31","subject":"Re: [PATCH 06/36] test: ipa: ipa_interface: Replace FIFO with pipe","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n> The ipa_interface unit test uses a FIFO to communicate with the vimc IPA\n> module. FIFOs are named pipes, created in the file system. The path to\n> the FIFO is hardcoded so that both the unit test and IPA module know\n> where to locate the file.\n> \n> If the ipa_interface crashes for any reason, the FIFO will not be\n> removed, and subsequent usage of the vimc IPA module will hang when\n> trying to write to the FIFO in the IPA module.\n> \n> Fix this by replacing the FIFO with a pipe. Pipes are unidirectional\n> data channels that are represented by a pair of file descriptors,\n> without any presence in the file system. The write end of the pipe is\n> passed to the vimc IPA module init() function, and then used the same\n> way as the FIFO.\n> \n> While at it, use a std::unique_ptr to manage the notifier in the unit\n> test instead of manually allocating and deleting the object.\n> \n> Signed-off-by: Laurent Pinchart \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(-)\n> \n> [...]\n> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> index 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 \n> #include \n> +#include \n> #include \n> -#include \n> -#include \n> #include \n> \n> #include \n> @@ -17,8 +16,10 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> +#include \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(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 inFlags;\n> \t\tFlags 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\nIs there any need to keep the write end of the pipe open after this point?\nIf not, I think it should just be a local in this function.\n\n\n> \t\tif (ret < 0) {\n> [...]","headers":{"Return-Path":"","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 127DDBDCBF\n\tfor ;\n\tTue, 13 Jan 2026 09:05:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A4DCE61FBC;\n\tTue, 13 Jan 2026 10:05:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 92ACF61F84\n\tfor ;\n\tTue, 13 Jan 2026 10:05:34 +0100 (CET)","from [192.168.33.30] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6B9792E0;\n\tTue, 13 Jan 2026 10:05:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"NWrUOhSj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768295108;\n\tbh=mW8oGaasM+bG91d3qt/RfQgCSsSJgflagQwyU1WmLy4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=NWrUOhSjqiE7LE9PS4D8Ax/CtxRlI8K4ZbanhPNgFIIHOP8JHJlBEH4wPjEOSjFxK\n\tvttiixqs16k7Jos//XX6FQxxjgwjJj7ddsHmp23Z0R9BoCXErsebWFOS6ZZG1CwxDC\n\t3mQMuOLRlrPkc/+Oz/GEjrJbUvd8gZU3R/M+2Ohc=","Message-ID":"","Date":"Tue, 13 Jan 2026 10:05:31 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 06/36] test: ipa: ipa_interface: Replace FIFO with pipe","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<-6YO0Eiz2NGeR1PshWDUaRx3rMyLb5aft178pD13T92BiwbyAvJ5ulqIHQ8qCbfaE1FJja2jAS5pgBtEeLa-5A==@protonmail.internalid>\n\t<20260113000808.15395-7-laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260113000808.15395-7-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37596,"web_url":"https://patchwork.libcamera.org/comment/37596/","msgid":"<20260113100511.GC18320@pendragon.ideasonboard.com>","date":"2026-01-13T10:05:11","subject":"Re: [PATCH 06/36] test: ipa: ipa_interface: Replace FIFO with pipe","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jan 13, 2026 at 10:05:31AM +0100, Barnabás Pőcze wrote:\n> 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n> > The ipa_interface unit test uses a FIFO to communicate with the vimc IPA\n> > module. FIFOs are named pipes, created in the file system. The path to\n> > the FIFO is hardcoded so that both the unit test and IPA module know\n> > where to locate the file.\n> > \n> > If the ipa_interface crashes for any reason, the FIFO will not be\n> > removed, and subsequent usage of the vimc IPA module will hang when\n> > trying to write to the FIFO in the IPA module.\n> > \n> > Fix this by replacing the FIFO with a pipe. Pipes are unidirectional\n> > data channels that are represented by a pair of file descriptors,\n> > without any presence in the file system. The write end of the pipe is\n> > passed to the vimc IPA module init() function, and then used the same\n> > way as the FIFO.\n> > \n> > While at it, use a std::unique_ptr to manage the notifier in the unit\n> > test instead of manually allocating and deleting the object.\n> > \n> > Signed-off-by: Laurent Pinchart \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(-)\n> > \n> > [...]\n> > diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> > index 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 \n> > #include \n> > +#include \n> > #include \n> > -#include \n> > -#include \n> > #include \n> > \n> > #include \n> > @@ -17,8 +16,10 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > #include \n> > #include \n> > +#include \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(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 inFlags;\n> > \t\tFlags 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> \n> Is there any need to keep the write end of the pipe open after this point?\n> If not, I think it should just be a local in this function.\n\nThere isn't. The whole init() function could be moved to run(). There is\nno very clear rule regarding what needs to go in init() and run(), my\ntake on it is that init() should contain all the initialization that is\nnot considered part of the unit test itself, where an init() failure\nwould be a platform issue or bug in the test, while a run() failure\nindicates an issue in the libcamera feature being tested.\n\n> > \t\tif (ret < 0) {\n> > [...]","headers":{"Return-Path":"","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 CD1C9BE08B\n\tfor ;\n\tTue, 13 Jan 2026 10:05:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D11261FC2;\n\tTue, 13 Jan 2026 11:05:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8958D61F84\n\tfor ;\n\tTue, 13 Jan 2026 11:05:32 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-152.bb.dnainternet.fi\n\t[81.175.209.152])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 693D6227;\n\tTue, 13 Jan 2026 11:05:06 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jEFWxAwd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768298706;\n\tbh=qOLYLC4z02aRhH/+yomqxcLDkRGfT7DKMdQtNTfK0Eo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jEFWxAwdOUEBtDk/DLYDfg0TaiSfURr4PNe2VF/8lzvf49Wqc5hI68BsEVvCxlnv0\n\tv+nkR570XwOnguWCG0iUTm9ODP9cyjuOsJPbq7pPMZyqC2m1llnlDhZsrQovA5mbM6\n\tm8ltWuq7yG6PUPcp5suL+Osd4Z4ob5o1pMpx+qs8=","Date":"Tue, 13 Jan 2026 12:05:11 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 06/36] test: ipa: ipa_interface: Replace FIFO with pipe","Message-ID":"<20260113100511.GC18320@pendragon.ideasonboard.com>","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<-6YO0Eiz2NGeR1PshWDUaRx3rMyLb5aft178pD13T92BiwbyAvJ5ulqIHQ8qCbfaE1FJja2jAS5pgBtEeLa-5A==@protonmail.internalid>\n\t<20260113000808.15395-7-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37597,"web_url":"https://patchwork.libcamera.org/comment/37597/","msgid":"<45a82bda-3c99-4b4b-908d-34f745677d8e@ideasonboard.com>","date":"2026-01-13T12:23:09","subject":"Re: [PATCH 06/36] test: ipa: ipa_interface: Replace FIFO with pipe","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 01. 13. 11:05 keltezéssel, Laurent Pinchart írta:\n> On Tue, Jan 13, 2026 at 10:05:31AM +0100, Barnabás Pőcze wrote:\n>> 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n>>> The ipa_interface unit test uses a FIFO to communicate with the vimc IPA\n>>> module. FIFOs are named pipes, created in the file system. The path to\n>>> the FIFO is hardcoded so that both the unit test and IPA module know\n>>> where to locate the file.\n>>>\n>>> If the ipa_interface crashes for any reason, the FIFO will not be\n>>> removed, and subsequent usage of the vimc IPA module will hang when\n>>> trying to write to the FIFO in the IPA module.\n>>>\n>>> Fix this by replacing the FIFO with a pipe. Pipes are unidirectional\n>>> data channels that are represented by a pair of file descriptors,\n>>> without any presence in the file system. The write end of the pipe is\n>>> passed to the vimc IPA module init() function, and then used the same\n>>> way as the FIFO.\n>>>\n>>> While at it, use a std::unique_ptr to manage the notifier in the unit\n>>> test instead of manually allocating and deleting the object.\n>>>\n>>> Signed-off-by: Laurent Pinchart \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(-)\n>>>\n>>> [...]\n>>> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n>>> index 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 \n>>> #include \n>>> +#include \n>>> #include \n>>> -#include \n>>> -#include \n>>> #include \n>>>\n>>> #include \n>>> @@ -17,8 +16,10 @@\n>>> #include \n>>> #include \n>>> #include \n>>> +#include \n>>> #include \n>>> #include \n>>> +#include \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(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 inFlags;\n>>> \t\tFlags 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>>\n>> Is there any need to keep the write end of the pipe open after this point?\n>> If not, I think it should just be a local in this function.\n> \n> There isn't. The whole init() function could be moved to run(). There is\n> no very clear rule regarding what needs to go in init() and run(), my\n> take on it is that init() should contain all the initialization that is\n> not considered part of the unit test itself, where an init() failure\n> would be a platform issue or bug in the test, while a run() failure\n> indicates an issue in the libcamera feature being tested.\n\nOops, sorry, I didn't notice that they are different in functions.\nThen it looks ok.\n\nReviewed-by: Barnabás Pőcze \n\n\n> \n>>> \t\tif (ret < 0) {\n>>> [...]\n>","headers":{"Return-Path":"","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 D6A48BDCBF\n\tfor ;\n\tTue, 13 Jan 2026 12:23:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A6BBF61FBC;\n\tTue, 13 Jan 2026 13:23:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C06C61FA0\n\tfor ;\n\tTue, 13 Jan 2026 13:23:13 +0100 (CET)","from [192.168.33.30] (185.221.143.114.nat.pool.zt.hu\n\t[185.221.143.114])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E910250A;\n\tTue, 13 Jan 2026 13:22:46 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Ou3fLG3H\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768306967;\n\tbh=OILIn7nnOcQe67nGvCL5RP2YCQ78UiHTYxPZ94e2PzM=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Ou3fLG3Hj94yzv6eH8VMnw5l8KMsZujnZ/E5V/o6kEPjAbL02yT/rBeQ3QJlFyxDu\n\tsdpO2n+lGls9QESL7oastgciMzU1Y+dRbv2YCPjVrt5+goMY+Whl4+Xiilh94FZxas\n\tG7rtzzkzFs3oFkHg9XIDe6pHZDDGV+UHe5sMAbOw=","Message-ID":"<45a82bda-3c99-4b4b-908d-34f745677d8e@ideasonboard.com>","Date":"Tue, 13 Jan 2026 13:23:09 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 06/36] test: ipa: ipa_interface: Replace FIFO with pipe","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<-6YO0Eiz2NGeR1PshWDUaRx3rMyLb5aft178pD13T92BiwbyAvJ5ulqIHQ8qCbfaE1FJja2jAS5pgBtEeLa-5A==@protonmail.internalid>\n\t<20260113000808.15395-7-laurent.pinchart@ideasonboard.com>\n\t\n\t<20260113100511.GC18320@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260113100511.GC18320@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]