[libcamera-devel] [PATCH 07/11] ipa: Pass IPA initialization settings to IPAInterface::init()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Apr 27 05:17:09 CEST 2020
Add a new IPASettings class to pass IPA initialization settings through
the IPAInterface::init() method. The settings currently only contain the
name of a configuration file, and are expected to be extended later.
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
include/ipa/ipa_interface.h | 13 +++++++--
src/ipa/libipa/ipa_interface_wrapper.cpp | 8 ++++--
src/ipa/libipa/ipa_interface_wrapper.h | 3 +-
src/ipa/rkisp1/rkisp1.cpp | 2 +-
src/ipa/vimc/vimc.cpp | 4 +--
src/libcamera/include/ipa_context_wrapper.h | 2 +-
src/libcamera/ipa_context_wrapper.cpp | 9 ++++--
src/libcamera/ipa_interface.cpp | 32 +++++++++++++++++++++
src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
src/libcamera/pipeline/vimc/vimc.cpp | 2 +-
src/libcamera/proxy/ipa_proxy_linux.cpp | 2 +-
src/libcamera/proxy/ipa_proxy_thread.cpp | 6 ++--
test/ipa/ipa_interface_test.cpp | 3 +-
test/ipa/ipa_wrappers_test.cpp | 13 +++++++--
14 files changed, 80 insertions(+), 21 deletions(-)
diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
index e65844bc7b34..ef3d6507b692 100644
--- a/include/ipa/ipa_interface.h
+++ b/include/ipa/ipa_interface.h
@@ -18,6 +18,10 @@ struct ipa_context {
const struct ipa_context_ops *ops;
};
+struct ipa_settings {
+ const char *configuration_file;
+};
+
struct ipa_stream {
unsigned int id;
unsigned int pixel_format;
@@ -63,7 +67,8 @@ struct ipa_callback_ops {
struct ipa_context_ops {
void (*destroy)(struct ipa_context *ctx);
void *(*get_interface)(struct ipa_context *ctx);
- void (*init)(struct ipa_context *ctx);
+ void (*init)(struct ipa_context *ctx,
+ const struct ipa_settings *settings);
int (*start)(struct ipa_context *ctx);
void (*stop)(struct ipa_context *ctx);
void (*register_callbacks)(struct ipa_context *ctx,
@@ -100,6 +105,10 @@ struct ipa_context *ipaCreate();
namespace libcamera {
+struct IPASettings {
+ std::string configurationFile;
+};
+
struct IPAStream {
unsigned int pixelFormat;
Size size;
@@ -121,7 +130,7 @@ class IPAInterface
public:
virtual ~IPAInterface() {}
- virtual int init() = 0;
+ virtual int init(const IPASettings &settings) = 0;
virtual int start() = 0;
virtual void stop() = 0;
diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
index f50f93a0185a..7776d9881575 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.cpp
+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
@@ -79,11 +79,15 @@ void *IPAInterfaceWrapper::get_interface(struct ipa_context *_ctx)
return ctx->ipa_.get();
}
-void IPAInterfaceWrapper::init(struct ipa_context *_ctx)
+void IPAInterfaceWrapper::init(struct ipa_context *_ctx,
+ const struct ipa_settings *settings)
{
IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
- ctx->ipa_->init();
+ IPASettings ipaSettings{
+ .configurationFile = std::string{ settings->configuration_file }
+ };
+ ctx->ipa_->init(ipaSettings);
}
int IPAInterfaceWrapper::start(struct ipa_context *_ctx)
diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h
index e4bc6dd4535d..78ccf0f59d42 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.h
+++ b/src/ipa/libipa/ipa_interface_wrapper.h
@@ -23,7 +23,8 @@ public:
private:
static void destroy(struct ipa_context *ctx);
static void *get_interface(struct ipa_context *ctx);
- static void init(struct ipa_context *ctx);
+ static void init(struct ipa_context *ctx,
+ const struct ipa_settings *settings);
static int start(struct ipa_context *ctx);
static void stop(struct ipa_context *ctx);
static void register_callbacks(struct ipa_context *ctx,
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 7f0ffb0a791f..9a347e527cd2 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -32,7 +32,7 @@ LOG_DEFINE_CATEGORY(IPARkISP1)
class IPARkISP1 : public IPAInterface
{
public:
- int init() override { return 0; }
+ int init(const IPASettings &settings) override { return 0; }
int start() override { return 0; }
void stop() override {}
diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
index cfdbd6f99155..e6bda8ec58b0 100644
--- a/src/ipa/vimc/vimc.cpp
+++ b/src/ipa/vimc/vimc.cpp
@@ -31,7 +31,7 @@ public:
IPAVimc();
~IPAVimc();
- int init() override;
+ int init(const IPASettings &settings) override;
int start() override;
void stop() override;
@@ -61,7 +61,7 @@ IPAVimc::~IPAVimc()
::close(fd_);
}
-int IPAVimc::init()
+int IPAVimc::init(const IPASettings &settings)
{
trace(IPAOperationInit);
diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h
index 0a48bfe732c8..64395b4a450b 100644
--- a/src/libcamera/include/ipa_context_wrapper.h
+++ b/src/libcamera/include/ipa_context_wrapper.h
@@ -19,7 +19,7 @@ public:
IPAContextWrapper(struct ipa_context *context);
~IPAContextWrapper();
- int init() override;
+ int init(const IPASettings &settings) override;
int start() override;
void stop() override;
void configure(const std::map<unsigned int, IPAStream> &streamConfig,
diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
index ab6ce396b88a..5bd5d916ccc0 100644
--- a/src/libcamera/ipa_context_wrapper.cpp
+++ b/src/libcamera/ipa_context_wrapper.cpp
@@ -69,15 +69,18 @@ IPAContextWrapper::~IPAContextWrapper()
ctx_->ops->destroy(ctx_);
}
-int IPAContextWrapper::init()
+int IPAContextWrapper::init(const IPASettings &settings)
{
if (intf_)
- return intf_->init();
+ return intf_->init(settings);
if (!ctx_)
return 0;
- ctx_->ops->init(ctx_);
+ struct ipa_settings c_settings;
+ c_settings.configuration_file = settings.configurationFile.c_str();
+
+ ctx_->ops->init(ctx_, &c_settings);
return 0;
}
diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
index 890d4340e4f3..3949b7d6ea5d 100644
--- a/src/libcamera/ipa_interface.cpp
+++ b/src/libcamera/ipa_interface.cpp
@@ -92,6 +92,16 @@
* \brief The IPA context operations
*/
+/**
+ * \struct ipa_settings
+ * \brief IPA initialization settings for the IPA context operations
+ * \sa IPASettings
+ *
+ * \var ipa_settings::configuration_file
+ * \brief The name of the IPA configuration file (may be null or point to an
+ * empty string)
+ */
+
/**
* \struct ipa_stream
* \brief Stream information for the IPA context operations
@@ -231,6 +241,7 @@
* \var ipa_context_ops::init
* \brief Initialise the IPA context
* \param[in] ctx The IPA context
+ * \param[in] settings The IPA initialization settings
*
* \sa libcamera::IPAInterface::init()
*/
@@ -310,6 +321,22 @@
namespace libcamera {
+/**
+ * \struct IPASettings
+ * \brief IPA interface initialization settings
+ *
+ * The IPASettings structure stores data passed to the IPAInterface::init()
+ * function.
+ */
+
+/**
+ * \var IPASettings::configurationFile
+ * \brief The name of the IPA configuration file
+ *
+ * This field may be an empty string if the IPA doesn't require a configuration
+ * file.
+ */
+
/**
* \struct IPAStream
* \brief Stream configuration for the IPA interface
@@ -424,6 +451,11 @@ namespace libcamera {
/**
* \fn IPAInterface::init()
* \brief Initialise the IPAInterface
+ * \param[in] settings The IPA initialization settings
+ *
+ * This function initializes the IPA interface. It shall be called before any
+ * other function of the IPAInterface. The \a settings carry initialization
+ * parameters that will remain valid for the whole life time of the interface.
*/
/**
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index f42264361785..fde445b99a46 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -401,7 +401,7 @@ int RkISP1CameraData::loadIPA()
ipa_->queueFrameAction.connect(this,
&RkISP1CameraData::queueFrameAction);
- ipa_->init();
+ ipa_->init(IPASettings{});
return 0;
}
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index c5eea3a01b0e..699f788aa1b8 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -374,7 +374,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
if (data->ipa_ == nullptr)
LOG(VIMC, Warning) << "no matching IPA found";
else
- data->ipa_->init();
+ data->ipa_->init(IPASettings{});
/* Locate and open the capture video node. */
if (data->init(media))
diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
index 89f5cb54687b..cd8ac824679d 100644
--- a/src/libcamera/proxy/ipa_proxy_linux.cpp
+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
@@ -26,7 +26,7 @@ public:
IPAProxyLinux(IPAModule *ipam);
~IPAProxyLinux();
- int init() override { return 0; }
+ int init(const IPASettings &settings) override { return 0; }
int start() override { return 0; }
void stop() override {}
void configure(const std::map<unsigned int, IPAStream> &streamConfig,
diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
index 1ebb9b6a6c9d..be82fde34bd9 100644
--- a/src/libcamera/proxy/ipa_proxy_thread.cpp
+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
@@ -25,7 +25,7 @@ class IPAProxyThread : public IPAProxy, public Object
public:
IPAProxyThread(IPAModule *ipam);
- int init() override;
+ int init(const IPASettings &settings) override;
int start() override;
void stop() override;
@@ -97,9 +97,9 @@ IPAProxyThread::IPAProxyThread(IPAModule *ipam)
valid_ = true;
}
-int IPAProxyThread::init()
+int IPAProxyThread::init(const IPASettings &settings)
{
- int ret = ipa_->init();
+ int ret = ipa_->init(settings);
if (ret)
return ret;
diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
index 22f9ca41ef37..2e2dfb8d1ebd 100644
--- a/test/ipa/ipa_interface_test.cpp
+++ b/test/ipa/ipa_interface_test.cpp
@@ -98,7 +98,8 @@ protected:
}
/* Test initialization of IPA module. */
- ipa_->init();
+ IPASettings settings;
+ ipa_->init(settings);
timer.start(1000);
while (timer.isRunning() && trace_ != IPAOperationInit)
dispatcher->processEvents();
diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp
index fae1d56b67c7..21bf51a2e046 100644
--- a/test/ipa/ipa_wrappers_test.cpp
+++ b/test/ipa/ipa_wrappers_test.cpp
@@ -43,8 +43,14 @@ public:
{
}
- int init() override
+ int init(const IPASettings &settings) override
{
+ if (settings.configurationFile != "/ipa/configuration/file") {
+ cerr << "init(): Invalid configuration file" << endl;
+ report(Op_init, TestFail);
+ return 0;
+ }
+
report(Op_init, TestPass);
return 0;
}
@@ -339,7 +345,10 @@ protected:
* Test init(), start() and stop() last to ensure nothing in the
* wrappers or serializer depends on them being called first.
*/
- ret = INVOKE(init);
+ IPASettings settings{
+ .configurationFile = "/ipa/configuration/file"
+ };
+ ret = INVOKE(init, settings);
if (ret == TestFail) {
cerr << "Failed to run init()";
return TestFail;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list