PawletOS/external_libcamera
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

test: ipa: ipa_interface: Replace FIFO with pipe

Browse Source 
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 <laurent.pinchart@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
This commit is contained in:
Laurent Pinchart
2025-10-20 19:03:17 +03:00
parent b97c6f2883
commit bcaed97380
4 changed files with 28 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
include/libcamera/ipa/vimc.mojom
View File

@@ -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);

33
src/ipa/vimc/vimc.cpp
View File

@@ -33,6 +33,7 @@ public:
~IPAVimc();
int init(const IPASettings &settings,
const SharedFD &traceFd,
const ipa::vimc::IPAOperationCode code,
const Flags<ipa::vimc::TestFlag> inFlags,
Flags<ipa::vimc::TestFlag> *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<unsigned int, MappedFrameBuffer> 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<ipa::vimc::TestFlag> inFlags,
Flags<ipa::vimc::TestFlag> *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: "

2
src/libcamera/pipeline/vimc/vimc.cpp
View File

@@ -498,7 +498,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
std::string conf = data->ipa_->configurationFile("vimc.conf");
Flags<ipa::vimc::TestFlag> inFlags = ipa::vimc::TestFlag::Flag2;
Flags<ipa::vimc::TestFlag> 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)

49
test/ipa/ipa_interface_test.cpp
View File

@@ -7,9 +7,8 @@
#include <fcntl.h>
#include <iostream>
#include <memory>
#include <string.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#include <libcamera/ipa/vimc_ipa_proxy.h>
@@ -17,8 +16,10 @@
#include <libcamera/base/event_dispatcher.h>
#include <libcamera/base/event_notifier.h>
#include <libcamera/base/object.h>
#include <libcamera/base/shared_fd.h>
#include <libcamera/base/thread.h>
#include <libcamera/base/timer.h>
#include <libcamera/base/unique_fd.h>
#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<EventNotifier>(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<ipa::vimc::TestFlag> inFlags;
Flags<ipa::vimc::TestFlag> 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<GlobalConfiguration> config_;
std::unique_ptr<IPAManager> ipaManager_;
enum ipa::vimc::IPAOperationCode trace_;
EventNotifier *notifier_;
int fd_;
std::unique_ptr<EventNotifier> notifier_;
UniqueFD pipeReadFd_;
SharedFD pipeWriteFd_;
};
TEST_REGISTER(IPAInterfaceTest)