@@ -169,6 +169,8 @@ private:
void setControls(unsigned int frame);
void calculateBdsGrid(const Size &bdsOutputSize);
+ void initializeFrameContext(IPAFrameContext &frameContext,
+ const ControlList &controls);
std::map<unsigned int, MappedFrameBuffer> buffers_;
@@ -181,6 +183,10 @@ private:
IPAIPU3::IPAIPU3()
: context_(kMaxFrameContexts)
{
+ context_.frameContexts.setInitCallback(
+ [this](IPAFrameContext &fc, const ControlList &c) {
+ this->initializeFrameContext(fc, c);
+ });
}
std::string IPAIPU3::logPrefix() const
@@ -467,7 +473,7 @@ void IPAIPU3::computeParams(const uint32_t frame, const uint32_t bufferId)
*/
params->use = {};
- IPAFrameContext &frameContext = context_.frameContexts.get(frame);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
for (const auto &algo : algorithms())
algo->prepare(context_, frame, frameContext, params);
@@ -500,7 +506,7 @@ void IPAIPU3::processStats(const uint32_t frame,
const ipu3_uapi_stats_3a *stats =
reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
- IPAFrameContext &frameContext = context_.frameContexts.get(frame);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
std::tie(frameContext.sensor.exposure, frameContext.sensor.gain) =
agc::extractControls(sensorControls, context_.camHelper.get());
@@ -533,10 +539,14 @@ void IPAIPU3::processStats(const uint32_t frame,
*/
void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)
{
- IPAFrameContext &frameContext = context_.frameContexts.alloc(frame);
+ context_.frameContexts.getOrInitContext(frame, controls);
+}
+void IPAIPU3::initializeFrameContext(IPAFrameContext &frameContext,
+ const ControlList &controls)
+{
for (const auto &algo : algorithms())
- algo->queueRequest(context_, frame, frameContext, controls);
+ algo->queueRequest(context_, frameContext.frame(), frameContext, controls);
}
/**
@@ -548,7 +558,7 @@ void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)
*/
void IPAIPU3::setControls(unsigned int frame)
{
- IPAFrameContext &frameContext = context_.frameContexts.get(frame);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
ControlList ctrls(context_.sensorControls);
agc::prepareControls(ctrls, context_.camHelper.get(),
@@ -101,6 +101,19 @@ namespace ipa {
* \param[in] size The number of contexts in the queue
*/
+/**
+ * \typedef FCQueue::InitCallback
+ * \brief The init callback type
+ */
+
+/**
+ * \fn FCQueue::setInitCallback()
+ * \brief Set the init callback
+ *
+ * The init callback is called when a frame context is allocated and needs to be
+ * initialized.
+ */
+
/**
* \fn FCQueue::clear()
* \brief Clear the contexts queue
@@ -113,16 +126,17 @@ namespace ipa {
*/
/**
- * \fn FCQueue::alloc(uint32_t frame)
- * \brief Allocate and return a FrameContext for the \a frame
+ * \fn FCQueue::getOrInitContext(uint32_t frame, const ControlList &controls)
+ * \brief Get or allocate and return a FrameContext for the \a frame
* \param[in] frame The frame context sequence number
+ * \param[in] controls Controls to pass to the init function
*
- * The first call to obtain a FrameContext from the FCQueue should be handled
- * through this function. The FrameContext will be initialised, if not
- * initialised already, and returned to the caller.
+ * If a FrameContext for frame \a frame is not yet initialized, it will be
+ * initialized and \a controls is passed to the initialization function.
*
- * If the FrameContext was already initialized for this \a frame, a warning will
- * be reported and the previously initialized FrameContext is returned.
+ * If a FrameContext is already initialized, it is returned to the caller. The
+ * passed controls are stored and used for the next initialization of a
+ * FrameContext (the control lists will be merged in order).
*
* Frame contexts are expected to be initialised when a Request is first passed
* to the IPA module in IPAModule::queueRequest().
@@ -130,17 +144,6 @@ namespace ipa {
* \return A reference to the FrameContext for sequence \a frame
*/
-/**
- * \fn FCQueue::get(uint32_t frame)
- * \brief Obtain the FrameContext for the \a frame
- * \param[in] frame The frame context sequence number
- *
- * If the FrameContext is not correctly initialised for the \a frame, it will be
- * initialised.
- *
- * \return A reference to the FrameContext for sequence \a frame
- */
-
} /* namespace ipa */
} /* namespace libcamera */
@@ -11,6 +11,7 @@
#include <vector>
#include <libcamera/base/log.h>
+#include <libcamera/controls.h>
namespace libcamera {
@@ -27,52 +28,33 @@ struct FrameContext {
private:
template<typename T> friend class FCQueue;
uint32_t frame_;
- bool initialised_ = false;
};
template<typename FC>
class FCQueue
{
public:
+ using InitCallback = std::function<void(FC &, const ControlList &)>;
+
FCQueue(unsigned int size)
: contexts_(size)
{
}
+ void setInitCallback(const InitCallback &cb)
+ {
+ initCallback_ = cb;
+ }
+
void clear()
{
for (FC &ctx : contexts_) {
- ctx.initialised_ = false;
ctx.frame_ = 0;
}
+ initialized_ = false;
}
- FC &alloc(const uint32_t frame)
- {
- FC &fc = contexts_[frame % contexts_.size()];
- FrameContext &frameContext = fc;
-
- /*
- * Do not re-initialise if a get() call has already fetched this
- * frame context to preseve the context.
- *
- * \todo If the the sequence number of the context to initialise
- * is smaller than the sequence number of the queue slot to use,
- * it means that we had a serious request underrun and more
- * frames than the queue size has been produced since the last
- * time the application has queued a request. Does this deserve
- * an error condition ?
- */
- if (frame != 0 && frame <= frameContext.frame_)
- LOG(FCQueue, Warning)
- << "Frame " << frame << " already initialised";
- else
- init(fc, frame);
-
- return fc;
- }
-
- FC &get(uint32_t frame)
+ FC &getOrInitContext(unsigned int frame, const ControlList &controls = {})
{
FC &fc = contexts_[frame % contexts_.size()];
FrameContext &frameContext = fc;
@@ -90,51 +72,42 @@ public:
<< " has been overwritten by "
<< frameContext.frame_;
- if (frame == 0 && !frameContext.initialised_) {
- /*
- * If the IPA calls get() at start() time it will get an
- * un-intialized FrameContext as the below "frame ==
- * frameContext.frame_" check will return success
- * because FrameContexts are zeroed at creation time.
- *
- * Make sure the FrameContext gets initialised if get()
- * is called before alloc() by the IPA for frame#0.
- */
- init(fc, frame);
-
+ if (initialized_ && frame == frameContext.frame_) {
+ if (!controls.empty()) {
+ /* Too late to apply the controls. Store them for later. */
+ LOG(FCQueue, Warning)
+ << "Request underrun. Controls for frame "
+ << frame << " are delayed ";
+ controlsToApply_.merge(controls,
+ ControlList::MergePolicy::OverwriteExisting);
+ }
+ LOG(FCQueue, Debug) << "Got " << frame;
return fc;
}
- if (frame == frameContext.frame_)
- return fc;
+ const ControlList *controls2 = &controls;
+ if (!controlsToApply_.empty()) {
+ LOG(FCQueue, Debug) << "Applied late controls on frame" << frame;
+ controlsToApply_.merge(controls, ControlList::MergePolicy::OverwriteExisting);
+ controls2 = &controlsToApply_;
+ }
- /*
- * The frame context has been retrieved before it was
- * initialised through the initialise() call. This indicates an
- * algorithm attempted to access a Frame context before it was
- * queued to the IPA. Controls applied for this request may be
- * left unhandled.
- *
- * \todo Set an error flag for per-frame control errors.
- */
- LOG(FCQueue, Warning)
- << "Obtained an uninitialised FrameContext for " << frame;
+ LOG(FCQueue, Debug) << "Init " << frame;
- init(fc, frame);
+ fc = {};
+ frameContext.frame_ = frame;
+ initCallback_(fc, *controls2);
+ initialized_ = true;
+ controlsToApply_.clear();
return fc;
}
private:
- void init(FC &fc, const uint32_t frame)
- {
- fc = {};
- FrameContext &frameContext = fc;
- frameContext.frame_ = frame;
- frameContext.initialised_ = true;
- }
-
std::vector<FC> contexts_;
+ InitCallback initCallback_;
+ ControlList controlsToApply_;
+ bool initialized_;
};
} /* namespace ipa */
@@ -67,6 +67,8 @@ protected:
private:
void updateControls(ControlInfoMap *ipaControls);
void setControls(const IPAFrameContext &frameContext);
+ void initializeFrameContext(IPAFrameContext &frameContext,
+ const ControlList &controls);
std::map<unsigned int, MappedFrameBuffer> buffers_;
@@ -81,6 +83,10 @@ namespace {
IPAMaliC55::IPAMaliC55()
: context_(kMaxFrameContexts)
{
+ context_.frameContexts.setInitCallback(
+ [this](IPAFrameContext &fc, const ControlList &c) {
+ this->initializeFrameContext(fc, c);
+ });
}
std::string IPAMaliC55::logPrefix() const
@@ -219,21 +225,25 @@ void IPAMaliC55::unmapBuffers(const std::vector<IPABuffer> &buffers)
}
}
-void IPAMaliC55::queueRequest(const uint32_t request, const ControlList &controls)
+void IPAMaliC55::queueRequest(const uint32_t frame, const ControlList &controls)
{
- IPAFrameContext &frameContext = context_.frameContexts.alloc(request);
+ context_.frameContexts.getOrInitContext(frame, controls);
+}
+void IPAMaliC55::initializeFrameContext(IPAFrameContext &frameContext,
+ const ControlList &controls)
+{
for (const auto &a : algorithms()) {
Algorithm *algo = static_cast<Algorithm *>(a.get());
- algo->queueRequest(context_, request, frameContext, controls);
+ algo->queueRequest(context_, frameContext.frame(), frameContext, controls);
}
}
void IPAMaliC55::fillParams(unsigned int request,
[[maybe_unused]] uint32_t bufferId)
{
- IPAFrameContext &frameContext = context_.frameContexts.get(request);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(request);
MaliC55Params params(buffers_.at(bufferId).planes()[0]);
for (const auto &algo : algorithms())
@@ -245,7 +255,7 @@ void IPAMaliC55::fillParams(unsigned int request,
void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId,
const ControlList &sensorControls)
{
- IPAFrameContext &frameContext = context_.frameContexts.get(request);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(request);
const mali_c55_stats_buffer *stats = nullptr;
stats = reinterpret_cast<mali_c55_stats_buffer *>(
@@ -6,6 +6,7 @@
*/
#include <algorithm>
+#include <functional>
#include <stdint.h>
#include <string.h>
@@ -64,8 +65,7 @@ public:
void queueRequest(const uint32_t frame, const ControlList &controls) override;
void computeParams(const uint32_t frame, const uint32_t bufferId) override;
- void initializeFrameContext(const uint32_t frame,
- IPAFrameContext &frameContext,
+ void initializeFrameContext(IPAFrameContext &frameContext,
const ControlList &controls);
void processStats(const uint32_t frame, const uint32_t bufferId,
const ControlList &sensorControls) override;
@@ -124,6 +124,10 @@ const ControlInfoMap::Map rkisp1Controls{
IPARkISP1::IPARkISP1()
: context_(kMaxFrameContexts)
{
+ context_.frameContexts.setInitCallback(
+ [this](IPAFrameContext &fc, const ControlList &c) {
+ this->initializeFrameContext(fc, c);
+ });
}
std::string IPARkISP1::logPrefix() const
@@ -208,8 +212,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,
void IPARkISP1::start(const ControlList &controls, StartResult *result)
{
- IPAFrameContext frameContext = {};
- initializeFrameContext(0, frameContext, controls);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(0, controls);
result->controls = getSensorControls(frameContext);
result->code = 0;
}
@@ -295,27 +298,23 @@ void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)
{
- IPAFrameContext &frameContext = context_.frameContexts.alloc(frame);
context_.debugMetadata.enableByControl(controls);
-
- initializeFrameContext(frame, frameContext, controls);
+ context_.frameContexts.getOrInitContext(frame, controls);
}
-void IPARkISP1::initializeFrameContext(const uint32_t frame,
- IPAFrameContext &frameContext,
- const ControlList &controls)
+void IPARkISP1::initializeFrameContext(IPAFrameContext &fc, const ControlList &controls)
{
for (const auto &a : algorithms()) {
Algorithm *algo = static_cast<Algorithm *>(a.get());
if (algo->disabled_)
continue;
- algo->queueRequest(context_, frame, frameContext, controls);
+ algo->queueRequest(context_, fc.frame(), fc, controls);
}
}
void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)
{
- IPAFrameContext &frameContext = context_.frameContexts.get(frame);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
if (bufferId != 0) {
RkISP1Params params(context_.configuration.paramFormat,
@@ -334,7 +333,7 @@ void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId)
void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId,
const ControlList &sensorControls)
{
- IPAFrameContext &frameContext = context_.frameContexts.get(frame);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
/*
* In raw capture mode, the ISP is bypassed and no statistics buffer is
@@ -48,6 +48,10 @@ public:
IPASoftIsp()
: context_(kMaxFrameContexts)
{
+ context_.frameContexts.setInitCallback(
+ [this](IPAFrameContext &fc, const ControlList &c) {
+ this->initializeFrameContext(fc, c);
+ });
}
~IPASoftIsp();
@@ -75,6 +79,8 @@ protected:
private:
void updateExposure(double exposureMSV);
+ void initializeFrameContext(IPAFrameContext &frameContext,
+ const ControlList &controls);
DebayerParams *params_;
SwIspStats *stats_;
@@ -216,17 +222,22 @@ void IPASoftIsp::stop()
void IPASoftIsp::queueRequest(const uint32_t frame, const ControlList &controls)
{
- IPAFrameContext &frameContext = context_.frameContexts.alloc(frame);
+ context_.frameContexts.getOrInitContext(frame, controls);
+}
+
+void IPASoftIsp::initializeFrameContext(IPAFrameContext &frameContext,
+ const ControlList &controls)
+{
for (const auto &algo : algorithms())
- algo->queueRequest(context_, frame, frameContext, controls);
+ algo->queueRequest(context_, frameContext.frame(), frameContext, controls);
}
void IPASoftIsp::computeParams(const uint32_t frame)
{
context_.activeState.combinedMatrix = Matrix<float, 3, 3>::identity();
- IPAFrameContext &frameContext = context_.frameContexts.get(frame);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
for (const auto &algo : algorithms())
algo->prepare(context_, frame, frameContext, params_);
params_->combinedMatrix = context_.activeState.combinedMatrix;
@@ -238,7 +249,7 @@ void IPASoftIsp::processStats(const uint32_t frame,
[[maybe_unused]] const uint32_t bufferId,
const ControlList &sensorControls)
{
- IPAFrameContext &frameContext = context_.frameContexts.get(frame);
+ IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame);
std::tie(frameContext.sensor.exposure, frameContext.sensor.gain) =
agc::extractControls(sensorControls, context_.camHelper.get());
For per frame control we want to tick the IPA by the sensor frame sequence instead of the request sequence. This has the side effect that the IPA must be able to cope with situations where a frame context is required for a frame that was not queued before (computeParams is called without a corresponding request) or processStats is called for an unexpected sequence number (because a scratch buffer was used on kernel side). With the current FCQueue implementation this is not easy to model, as it has distinct calls for alloc() and get(). Simplify that by passing the FCQueue a callback that it can call to initialize a frame context when needed. This has the added benefit that the FCQueue can collate controls for requests that were queued in too late. This simplifies the logic on the IPA side. As fetching an uninitialized frame context is no error anymore, demote the corresponding warnings to debug messages. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- Changes in v3: - Fixed rebase conflicts Changes in v2: - Rewrote the patch to use a callback based mechanism. --- src/ipa/ipu3/ipu3.cpp | 20 ++++++-- src/ipa/libipa/fc_queue.cpp | 39 +++++++------- src/ipa/libipa/fc_queue.h | 97 +++++++++++++---------------------- src/ipa/mali-c55/mali-c55.cpp | 20 ++++++-- src/ipa/rkisp1/rkisp1.cpp | 25 +++++---- src/ipa/softisp/softisp.cpp | 19 +++++-- 6 files changed, 113 insertions(+), 107 deletions(-)