[libcamera-devel] [PATCH v3 2/3] libcamera: ipa: Pass a set of controls and return results from ipa::start()
Naushir Patuck
naush at raspberrypi.com
Fri Dec 4 16:31:20 CET 2020
This change allows controls passed into PipelineHandler::start to be
forwarded onto IPAInterface::start(). We also add a return channel if the
pipeline handler must action any of these controls, e.g. setting the
analogue gain or shutter speed in the sensor device.
todo: The IPA interface wrapper needs a translation for passing
IPAOperationData into start() and configure()
Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
Tested-by: David Plowman <david.plowman at raspberrypi.com>
---
include/libcamera/internal/ipa_context_wrapper.h | 3 ++-
include/libcamera/ipa/ipa_interface.h | 3 ++-
src/ipa/libipa/ipa_interface_wrapper.cpp | 4 +++-
src/ipa/raspberrypi/raspberrypi.cpp | 5 +++--
src/ipa/rkisp1/rkisp1.cpp | 3 ++-
src/ipa/vimc/vimc.cpp | 6 ++++--
src/libcamera/ipa_context_wrapper.cpp | 5 +++--
src/libcamera/ipa_interface.cpp | 7 +++++++
src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 +++++--
src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++-
src/libcamera/pipeline/vimc/vimc.cpp | 3 ++-
src/libcamera/proxy/ipa_proxy_linux.cpp | 3 ++-
src/libcamera/proxy/ipa_proxy_thread.cpp | 13 ++++++++-----
test/ipa/ipa_interface_test.cpp | 3 ++-
test/ipa/ipa_wrappers_test.cpp | 5 +++--
15 files changed, 50 insertions(+), 23 deletions(-)
diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h
index 8f767e84..ec27e48e 100644
--- a/include/libcamera/internal/ipa_context_wrapper.h
+++ b/include/libcamera/internal/ipa_context_wrapper.h
@@ -20,7 +20,8 @@ public:
~IPAContextWrapper();
int init(const IPASettings &settings) override;
- int start() override;
+ int start(const IPAOperationData &data,
+ IPAOperationData *result) override;
void stop() override;
void configure(const CameraSensorInfo &sensorInfo,
const std::map<unsigned int, IPAStream> &streamConfig,
diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
index 322b7079..df12f9ce 100644
--- a/include/libcamera/ipa/ipa_interface.h
+++ b/include/libcamera/ipa/ipa_interface.h
@@ -153,7 +153,8 @@ public:
virtual ~IPAInterface() = default;
virtual int init(const IPASettings &settings) = 0;
- virtual int start() = 0;
+ virtual int start(const IPAOperationData &data,
+ IPAOperationData *result) = 0;
virtual void stop() = 0;
virtual void configure(const CameraSensorInfo &sensorInfo,
diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
index cee532e3..40628489 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.cpp
+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
@@ -95,7 +95,9 @@ int IPAInterfaceWrapper::start(struct ipa_context *_ctx)
{
IPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);
- return ctx->ipa_->start();
+ /* \todo Translate the data and result. */
+ IPAOperationData data = {};
+ return ctx->ipa_->start(data, nullptr);
}
void IPAInterfaceWrapper::stop(struct ipa_context *_ctx)
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 29d48b1b..b8c0f955 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -78,13 +78,14 @@ public:
}
int init(const IPASettings &settings) override;
- int start() override { return 0; }
+ int start([[maybe_unused]] const IPAOperationData &data,
+ [[maybe_unused]] IPAOperationData *result) override { return 0; }
void stop() override {}
void configure(const CameraSensorInfo &sensorInfo,
const std::map<unsigned int, IPAStream> &streamConfig,
const std::map<unsigned int, const ControlInfoMap &> &entityControls,
- const IPAOperationData &data,
+ const IPAOperationData &ipaConfig,
IPAOperationData *response) override;
void mapBuffers(const std::vector<IPABuffer> &buffers) override;
void unmapBuffers(const std::vector<unsigned int> &ids) override;
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 07d7f1b2..39783abd 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -37,7 +37,8 @@ public:
{
return 0;
}
- int start() override { return 0; }
+ int start([[maybe_unused]] const IPAOperationData &data,
+ [[maybe_unused]] IPAOperationData *result) override { return 0; }
void stop() override {}
void configure(const CameraSensorInfo &info,
diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
index cf841135..074902ee 100644
--- a/src/ipa/vimc/vimc.cpp
+++ b/src/ipa/vimc/vimc.cpp
@@ -34,7 +34,8 @@ public:
int init(const IPASettings &settings) override;
- int start() override;
+ int start(const IPAOperationData &data,
+ IPAOperationData *result) override;
void stop() override;
void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
@@ -82,7 +83,8 @@ int IPAVimc::init(const IPASettings &settings)
return 0;
}
-int IPAVimc::start()
+int IPAVimc::start([[maybe_unused]] const IPAOperationData &data,
+ [[maybe_unused]] IPAOperationData *result)
{
trace(IPAOperationStart);
diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
index 231300ce..19c44ad8 100644
--- a/src/libcamera/ipa_context_wrapper.cpp
+++ b/src/libcamera/ipa_context_wrapper.cpp
@@ -86,10 +86,11 @@ int IPAContextWrapper::init(const IPASettings &settings)
return 0;
}
-int IPAContextWrapper::start()
+int IPAContextWrapper::start(const IPAOperationData &data,
+ IPAOperationData *result)
{
if (intf_)
- return intf_->start();
+ return intf_->start(data, result);
if (!ctx_)
return 0;
diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
index 23fc56d7..5be6f787 100644
--- a/src/libcamera/ipa_interface.cpp
+++ b/src/libcamera/ipa_interface.cpp
@@ -536,10 +536,17 @@ namespace libcamera {
/**
* \fn IPAInterface::start()
* \brief Start the IPA
+ * \param[in] data Protocol-specific data for the start operation
+ * \param[out] result Result of the start operation
*
* This method informs the IPA module that the camera is about to be started.
* The IPA module shall prepare any resources it needs to operate.
*
+ * The \a data and \a result parameters carry custom data passed by the
+ * pipeline handler to the IPA and back. The pipeline handler may set the \a
+ * result parameter to null if the IPA protocol doesn't need to pass a result
+ * back through the start() function.
+ *
* \return 0 on success or a negative error code otherwise
*/
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 9937db73..bafd0b2d 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -752,7 +752,10 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont
}
/* Start the IPA. */
- ret = data->ipa_->start();
+ IPAOperationData ipaData = {}, result = {};
+ if (controls)
+ ipaData.controls.emplace_back(*controls);
+ ret = data->ipa_->start(ipaData, &result);
if (ret) {
LOG(RPI, Error)
<< "Failed to start IPA for " << camera->id();
@@ -1189,7 +1192,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
}
/* Ready the IPA - it must know about the sensor resolution. */
- IPAOperationData result;
+ IPAOperationData result = {};
ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,
&result);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 4e959fde..eaa10f9f 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -842,7 +842,8 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c
if (ret)
return ret;
- ret = data->ipa_->start();
+ IPAOperationData ipaData = {};
+ ret = data->ipa_->start(ipaData, nullptr);
if (ret) {
freeBuffers(camera);
LOG(RkISP1, Error)
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index d81b8598..2a5054a8 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -322,7 +322,8 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con
if (ret < 0)
return ret;
- ret = data->ipa_->start();
+ IPAOperationData ipaData = {};
+ ret = data->ipa_->start(ipaData, nullptr);
if (ret) {
data->video_->releaseBuffers();
return ret;
diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
index b78a0e45..ea6f3e5e 100644
--- a/src/libcamera/proxy/ipa_proxy_linux.cpp
+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
@@ -30,7 +30,8 @@ public:
{
return 0;
}
- int start() override { return 0; }
+ int start([[maybe_unused]] const IPAOperationData &data,
+ [[maybe_unused]] IPAOperationData *result) override { return 0; }
void stop() override {}
void configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,
[[maybe_unused]] 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 eead2883..a5fda2c8 100644
--- a/src/libcamera/proxy/ipa_proxy_thread.cpp
+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
@@ -26,7 +26,8 @@ public:
IPAProxyThread(IPAModule *ipam);
int init(const IPASettings &settings) override;
- int start() override;
+ int start(const IPAOperationData &data,
+ IPAOperationData *result) override;
void stop() override;
void configure(const CameraSensorInfo &sensorInfo,
@@ -50,9 +51,9 @@ private:
ipa_ = ipa;
}
- int start()
+ int start(const IPAOperationData &data, IPAOperationData *result)
{
- return ipa_->start();
+ return ipa_->start(data, result);
}
void stop()
@@ -111,12 +112,14 @@ int IPAProxyThread::init(const IPASettings &settings)
return 0;
}
-int IPAProxyThread::start()
+int IPAProxyThread::start(const IPAOperationData &data,
+ IPAOperationData *result)
{
running_ = true;
thread_.start();
- return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking);
+ return proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking,
+ data, result);
}
void IPAProxyThread::stop()
diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp
index 9f575f93..dbb6727f 100644
--- a/test/ipa/ipa_interface_test.cpp
+++ b/test/ipa/ipa_interface_test.cpp
@@ -120,7 +120,8 @@ protected:
}
/* Test start of IPA module. */
- ipa_->start();
+ IPAOperationData data = {};
+ ipa_->start(data, nullptr);
timer.start(1000);
while (timer.isRunning() && trace_ != IPAOperationStart)
dispatcher->processEvents();
diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp
index 59d991cb..47533d10 100644
--- a/test/ipa/ipa_wrappers_test.cpp
+++ b/test/ipa/ipa_wrappers_test.cpp
@@ -56,7 +56,8 @@ public:
return 0;
}
- int start() override
+ int start([[maybe_unused]] const IPAOperationData &data,
+ [[maybe_unused]] IPAOperationData *result) override
{
report(Op_start, TestPass);
return 0;
@@ -376,7 +377,7 @@ protected:
return TestFail;
}
- ret = INVOKE(start);
+ ret = INVOKE(start, ipaConfig, nullptr);
if (ret == TestFail) {
cerr << "Failed to run start()";
return TestFail;
--
2.25.1
More information about the libcamera-devel
mailing list