[libcamera-devel] [RFC PATCH 03/17] libcamera: IPA: raspberrypi: Use generated IPARPiInterface
Paul Elder
paul.elder at ideasonboard.com
Wed Aug 26 13:09:12 CEST 2020
This patch shows how the IPA would use the generated IPA interface.
Like the patch for the pipeline, this should be straightforward. It
mainly switches to using the new data structures.
A couple things that are noteworthy:
ipaCreate returns an IPAInterface, instead of a struct ipa_context. This
means we don't need the C interface anymore!
File descriptors *cannot* be sent via ControlList and be expected to
still work on the other side. This is because ControlList is expected to
be sent during the stream. Every time an fd is sent across the process
boundary, it is practically duped, which means we'll eventually run out
of fds. Keeping a map of fds cannot be maintained practically either,
since the map on the other side of the process boundary will not be
notified of changes, among other nasty issues. Another big nasty issue
is that we'll probably have a memory leak due to having so many fds open
pointing to the same resource.
The solution that I've come up with is to send one fd as a
FileDescriptor from the pipeline handler that the IPA can use, and
another fd as an int32, that the IPA can send back to the pipeline
handler, and expect the pipeline handler to be able to use it. This is
something to keep in mind during design of the IPA interface.
Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
---
src/ipa/raspberrypi/raspberrypi.cpp | 107 ++++++++++++++++------------
1 file changed, 62 insertions(+), 45 deletions(-)
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 4557016c..2c088742 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -19,6 +19,7 @@
#include <libcamera/ipa/ipa_interface.h>
#include <libcamera/ipa/ipa_module_info.h>
#include <libcamera/ipa/raspberrypi.h>
+#include <libcamera/ipa/raspberrypi_wrapper.h>
#include <libcamera/request.h>
#include <libcamera/span.h>
@@ -60,7 +61,7 @@ namespace libcamera {
LOG_DEFINE_CATEGORY(IPARPI)
-class IPARPi : public IPAInterface
+class IPARPi : public IPARPiInterface
{
public:
IPARPi()
@@ -82,12 +83,12 @@ public:
void configure(const CameraSensorInfo &sensorInfo,
const std::map<unsigned int, IPAStream> &streamConfig,
- const std::map<unsigned int, const ControlInfoMap &> &entityControls,
- const IPAOperationData &data,
- IPAOperationData *response) override;
+ const std::map<unsigned int, const ControlInfoMap> &entityControls,
+ const RPiConfigureParams &data,
+ RPiConfigureParams *response) override;
void mapBuffers(const std::vector<IPABuffer> &buffers) override;
void unmapBuffers(const std::vector<unsigned int> &ids) override;
- void processEvent(const IPAOperationData &event) override;
+ void processEvent(const RPiEventParams &event) override;
private:
void setMode(const CameraSensorInfo &sensorInfo);
@@ -143,6 +144,11 @@ private:
unsigned int mistrust_count_;
/* LS table allocation passed in from the pipeline handler. */
FileDescriptor lsTableHandle_;
+ /*
+ * LS table allocation passed in from the pipeline handler,
+ * in the context of the pipeline handler.
+ */
+ int32_t lsTableHandlePH_;
void *lsTable_;
};
@@ -192,15 +198,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
void IPARPi::configure(const CameraSensorInfo &sensorInfo,
[[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,
- const std::map<unsigned int, const ControlInfoMap &> &entityControls,
- const IPAOperationData &ipaConfig,
- IPAOperationData *result)
+ const std::map<unsigned int, const ControlInfoMap> &entityControls,
+ const RPiConfigureParams &ipaConfig,
+ RPiConfigureParams *result)
{
if (entityControls.empty())
return;
- result->operation = 0;
-
unicam_ctrls_ = entityControls.at(0);
isp_ctrls_ = entityControls.at(1);
/* Setup a metadata ControlList to output metadata. */
@@ -222,11 +226,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
helper_->GetDelays(exposureDelay, gainDelay);
sensorMetadata = helper_->SensorEmbeddedDataPresent();
- result->data.push_back(gainDelay);
- result->data.push_back(exposureDelay);
- result->data.push_back(sensorMetadata);
+ RPiConfigurePayload payload = {};
+ payload.op_ = RPI_IPA_CONFIG_STAGGERED_WRITE;
+ payload.staggeredWriteResult_.gainDelay_ = gainDelay;
+ payload.staggeredWriteResult_.exposureDelay_ = exposureDelay;
+ payload.staggeredWriteResult_.sensorMetadata_ = sensorMetadata;
- result->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE;
+ result->payload_.push_back(payload);
}
/* Re-assemble camera mode using the sensor info. */
@@ -274,15 +280,23 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {
ControlList ctrls(unicam_ctrls_);
applyAGC(&agcStatus, ctrls);
- result->controls.push_back(ctrls);
- result->operation |= RPI_IPA_CONFIG_SENSOR;
+ RPiConfigurePayload payload = {};
+ payload.op_ = RPI_IPA_CONFIG_SENSOR;
+ payload.controls_ = ctrls;
+
+ result->payload_.push_back(payload);
}
lastMode_ = mode_;
/* Store the lens shading table pointer and handle if available. */
- if (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) {
+ auto lens = std::find_if(ipaConfig.payload_.begin(),
+ ipaConfig.payload_.end(),
+ [] (const RPiConfigurePayload &p) {
+ return p.op_ == RPI_IPA_CONFIG_LS_TABLE;
+ });
+ if (lens != ipaConfig.payload_.end()) {
/* Remove any previous table, if there was one. */
if (lsTable_) {
munmap(lsTable_, MAX_LS_GRID_SIZE);
@@ -290,7 +304,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
}
/* Map the LS table buffer into user space. */
- lsTableHandle_ = FileDescriptor(ipaConfig.data[0]);
+ lsTableHandle_ = FileDescriptor(lens->lsTableHandle_);
+ lsTableHandlePH_ = lens->lsTableHandleStatic_;
if (lsTableHandle_.isValid()) {
lsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE,
MAP_SHARED, lsTableHandle_.fd(), 0);
@@ -335,11 +350,11 @@ void IPARPi::unmapBuffers(const std::vector<unsigned int> &ids)
}
}
-void IPARPi::processEvent(const IPAOperationData &event)
+void IPARPi::processEvent(const RPiEventParams &event)
{
- switch (event.operation) {
+ switch (event.ev_) {
case RPI_IPA_EVENT_SIGNAL_STAT_READY: {
- unsigned int bufferId = event.data[0];
+ unsigned int bufferId = event.bufferId_;
if (++check_count_ != frame_count_) /* assert here? */
LOG(IPARPI, Error) << "WARNING: Prepare/Process mismatch!!!";
@@ -348,17 +363,17 @@ void IPARPi::processEvent(const IPAOperationData &event)
reportMetadata();
- IPAOperationData op;
- op.operation = RPI_IPA_ACTION_STATS_METADATA_COMPLETE;
- op.data = { bufferId & RPiIpaMask::ID };
- op.controls = { libcameraMetadata_ };
+ RPiActionParams op;
+ op.op_ = RPI_IPA_ACTION_STATS_METADATA_COMPLETE;
+ op.statsComplete_.bufferId_ = { bufferId & RPiIpaMask::ID };
+ op.statsComplete_.controls_ = { libcameraMetadata_ };
queueFrameAction.emit(0, op);
break;
}
case RPI_IPA_EVENT_SIGNAL_ISP_PREPARE: {
- unsigned int embeddedbufferId = event.data[0];
- unsigned int bayerbufferId = event.data[1];
+ unsigned int embeddedbufferId = event.ispPrepare_.embeddedbufferId_;
+ unsigned int bayerbufferId = event.ispPrepare_.bayerbufferId_;
/*
* At start-up, or after a mode-switch, we may want to
@@ -368,23 +383,23 @@ void IPARPi::processEvent(const IPAOperationData &event)
prepareISP(embeddedbufferId);
/* Ready to push the input buffer into the ISP. */
- IPAOperationData op;
+ RPiActionParams op;
if (++frame_count_ > hide_count_)
- op.operation = RPI_IPA_ACTION_RUN_ISP;
+ op.op_ = RPI_IPA_ACTION_RUN_ISP;
else
- op.operation = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME;
- op.data = { bayerbufferId & RPiIpaMask::ID };
+ op.op_ = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME;
+ op.bufferId_ = { bayerbufferId & RPiIpaMask::ID };
queueFrameAction.emit(0, op);
break;
}
case RPI_IPA_EVENT_QUEUE_REQUEST: {
- queueRequest(event.controls[0]);
+ queueRequest(event.controls_);
break;
}
default:
- LOG(IPARPI, Error) << "Unknown event " << event.operation;
+ LOG(IPARPI, Error) << "Unknown event " << event.ev_;
break;
}
}
@@ -489,6 +504,8 @@ void IPARPi::queueRequest(const ControlList &controls)
/* Clear the return metadata buffer. */
libcameraMetadata_.clear();
+ LOG(IPARPI, Info) << "Request ctrl length: " << controls.size();
+
for (auto const &ctrl : controls) {
LOG(IPARPI, Info) << "Request ctrl: "
<< controls::controls.at(ctrl.first)->name()
@@ -698,9 +715,9 @@ void IPARPi::queueRequest(const ControlList &controls)
void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)
{
- IPAOperationData op;
- op.operation = RPI_IPA_ACTION_EMBEDDED_COMPLETE;
- op.data = { bufferId & RPiIpaMask::ID };
+ RPiActionParams op;
+ op.op_ = RPI_IPA_ACTION_EMBEDDED_COMPLETE;
+ op.bufferId_ = { bufferId & RPiIpaMask::ID };
queueFrameAction.emit(0, op);
}
@@ -763,9 +780,9 @@ void IPARPi::prepareISP(unsigned int bufferId)
applyDPC(dpcStatus, ctrls);
if (!ctrls.empty()) {
- IPAOperationData op;
- op.operation = RPI_IPA_ACTION_V4L2_SET_ISP;
- op.controls.push_back(ctrls);
+ RPiActionParams op;
+ op.op_ = RPI_IPA_ACTION_V4L2_SET_ISP;
+ op.controls_ = ctrls;
queueFrameAction.emit(0, op);
}
}
@@ -823,9 +840,9 @@ void IPARPi::processStats(unsigned int bufferId)
ControlList ctrls(unicam_ctrls_);
applyAGC(&agcStatus, ctrls);
- IPAOperationData op;
- op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;
- op.controls.push_back(ctrls);
+ RPiActionParams op;
+ op.op_ = RPI_IPA_ACTION_V4L2_SET_STAGGERED;
+ op.controls_ = ctrls;
queueFrameAction.emit(0, op);
}
}
@@ -1056,7 +1073,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls)
.grid_width = w,
.grid_stride = w,
.grid_height = h,
- .dmabuf = lsTableHandle_.fd(),
+ .dmabuf = lsTableHandlePH_,
.ref_transform = 0,
.corner_sampled = 1,
.gain_format = GAIN_FORMAT_U4P10
@@ -1136,9 +1153,9 @@ const struct IPAModuleInfo ipaModuleInfo = {
"raspberrypi",
};
-struct ipa_context *ipaCreate()
+IPAInterface *ipaCreate()
{
- return new IPAInterfaceWrapper(std::make_unique<IPARPi>());
+ return new IPARPi();
}
}; /* extern "C" */
--
2.27.0
More information about the libcamera-devel
mailing list