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)