diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index 97fcf06cd..4159b8605 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -76,7 +76,7 @@ struct IPAActiveState { } toneMapping; }; -struct IPAFrameContext : public FrameContext { +struct IPAFrameContext { struct { uint32_t exposure; double gain; diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp index 0365e9197..ddcaca26f 100644 --- a/src/ipa/libipa/fc_queue.cpp +++ b/src/ipa/libipa/fc_queue.cpp @@ -20,28 +20,10 @@ namespace ipa { * \brief Queue of per-frame contexts */ -/** - * \struct FrameContext - * \brief Context for a frame - * - * The frame context stores data specific to a single frame processed by the - * IPA module. Each frame processed by the IPA module has a context associated - * with it, accessible through the Frame Context Queue. - * - * Fields in the frame context should reflect values and controls associated - * with the specific frame as requested by the application, and as configured by - * the hardware. Fields can be read by algorithms to determine if they should - * update any specific action for this frame, and finally to update the metadata - * control lists when the frame is fully completed. - * - * \var FrameContext::frame - * \brief The frame number - */ - /** * \class FCQueue * \brief A support class for managing FrameContext instances in IPA modules - * \tparam FrameContext The IPA module-specific FrameContext derived class type + * \tparam FrameContext The IPA module-specific frame context type * * Along with the Module and Algorithm classes, the frame context queue is a * core component of the libipa infrastructure. It stores per-frame contexts @@ -83,16 +65,12 @@ namespace ipa { * allowed to overflow, which must be ensured by pipeline handlers never * queuing more in-flight requests to the IPA module than the queue size. If an * overflow condition is detected, the queue will log a fatal error. - * - * IPA module-specific frame context implementations shall inherit from the - * FrameContext base class to support the minimum required features for a - * FrameContext. */ /** - * \fn FCQueue::FCQueue(unsigned int size) + * \fn FCQueue::FCQueue(std::size_t capacity) * \brief Construct a frame contexts queue of a specified size - * \param[in] size The number of contexts in the queue + * \param[in] capacity The number of contexts in the queue */ /** @@ -116,7 +94,8 @@ namespace ipa { * initialised already, and returned to the caller. * * If the FrameContext was already initialized for this \a frame, a warning will - * be reported and the previously initialized FrameContext is returned. + * be reported and the previously initialized FrameContext is returned. Otherwise, + * \a frame must be greater than the last allocated frame number. * * Frame contexts are expected to be initialised when a Request is first passed * to the IPA module in IPAModule::queueRequest(). @@ -129,8 +108,8 @@ namespace ipa { * \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. + * If the FrameContext is not correctly initialised for the \a frame, initialisation + * will be tried as if by calling \a alloc(frame). Note, that this may fail, and abort. * * \return A reference to the FrameContext for sequence \a frame */ diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h index a1d136521..86ae6ea1d 100644 --- a/src/ipa/libipa/fc_queue.h +++ b/src/ipa/libipa/fc_queue.h @@ -7,8 +7,11 @@ #pragma once +#include +#include +#include #include -#include +#include #include @@ -18,118 +21,129 @@ LOG_DECLARE_CATEGORY(FCQueue) namespace ipa { -template -class FCQueue; - -struct FrameContext { -private: - template friend class FCQueue; - uint32_t frame; - bool initialised = false; -}; - -template +template class FCQueue { + static_assert(std::is_default_constructible_v); + public: - FCQueue(unsigned int size) - : contexts_(size) + FCQueue(std::size_t capacity) + : entries_(std::make_unique[]>(capacity)), + capacity_(capacity) { + ASSERT(capacity > 0); } void clear() { - for (FrameContext &ctx : contexts_) { - ctx.initialised = false; - ctx.frame = 0; - } + next_ = 0; + lastFrame_.reset(); + + for (size_t i = 0; i < capacity_; i++) + entries_[i].reset(); } - FrameContext &alloc(const uint32_t frame) + T &get(uint32_t frame) { - FrameContext &frameContext = contexts_[frame % contexts_.size()]; + LOG(FCQueue, Debug) << "get(" << frame << ")"; - /* - * 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(frameContext, frame); + if (auto *d = find(frame)) + return *d; + + LOG(FCQueue, Warning) + << "Frame " << frame << " not found, trying to allocate"; - return frameContext; + return allocNext(frame); } - FrameContext &get(uint32_t frame) + T &alloc(uint32_t frame) { - FrameContext &frameContext = contexts_[frame % contexts_.size()]; + LOG(FCQueue, Debug) << "alloc(" << frame << ")"; + + if (frame <= lastFrame_) { + if (auto *d = find(frame)) { + LOG(FCQueue, Warning) + << "Frame " << frame << " already initialised"; + return *d; + } + } - /* - * If the IPA algorithms try to access a frame context slot which - * has been already overwritten by a newer context, it means the - * frame context queue has overflowed and the desired context - * has been forever lost. The pipeline handler shall avoid - * queueing more requests to the IPA than the frame context - * queue size. - */ - if (frame < frameContext.frame) - LOG(FCQueue, Fatal) << "Frame context for " << frame - << " has been overwritten by " - << frameContext.frame; + return allocNext(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(frameContext, frame); +private: + LIBCAMERA_DISABLE_COPY_AND_MOVE(FCQueue) - return frameContext; + struct Entry { + uint32_t frame; + T data; + + Entry(uint32_t f) + : frame(f), data() + { } + }; + + T *find(uint32_t frame) + { + const auto findInRange = [&](auto first, auto last) -> T * { + auto it = std::partition_point(first, last, [&](const auto &e) { + ASSERT(e); + return e->frame < frame; + }); + if (it != last && (*it)->frame == frame) + return &(*it)->data; + + return nullptr; + }; - if (frame == frameContext.frame) - return frameContext; + const auto first = entries_.get(); + const auto mid = first + next_; + const auto last = first + capacity_; /* - * 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. + * Search the more recent half: [0; next), + * the optionals in this range must always be non-empty. */ - LOG(FCQueue, Warning) - << "Obtained an uninitialised FrameContext for " << frame; + if (auto *d = findInRange(first, mid)) + return d; - init(frameContext, frame); + /* Search the less recent half: [next_; capacity_) */ + if (mid != last && mid->has_value()) { + /* + * If `next_` has wrapped around at least once, then all the optionals + * in [next_; capacity_) are non-empty. So if `*mid` is not empty, + * then all of them should be non-empty. + */ + if (auto *d = findInRange(mid, last)) + return d; + } - return frameContext; + return nullptr; } -private: - void init(FrameContext &frameContext, const uint32_t frame) + T &allocNext(uint32_t frame) { - frameContext = {}; - frameContext.frame = frame; - frameContext.initialised = true; + if (!(lastFrame_ < frame)) { + LOG(FCQueue, Fatal) + << "Tried to allocate frame context for frame " << frame + << " after having already allocated one for a later frame " + << *lastFrame_; + } + + auto &e = entries_[next_].emplace(frame); + LOG(FCQueue, Debug) << "frame " << frame << " slot " << next_; + + next_ = (next_ + 1) % capacity_; + lastFrame_ = frame; + + return e.data; } - std::vector contexts_; + std::unique_ptr[]> entries_; + std::size_t capacity_; + std::size_t next_ = 0; + std::optional lastFrame_; }; } /* namespace ipa */ diff --git a/src/ipa/mali-c55/ipa_context.h b/src/ipa/mali-c55/ipa_context.h index ac4b83773..156889dfe 100644 --- a/src/ipa/mali-c55/ipa_context.h +++ b/src/ipa/mali-c55/ipa_context.h @@ -60,7 +60,7 @@ struct IPAActiveState { } awb; }; -struct IPAFrameContext : public FrameContext { +struct IPAFrameContext { struct { uint32_t exposure; double sensorGain; diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index e61391bb1..e1aeac7fc 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -153,7 +153,7 @@ struct IPAActiveState { } lsc; }; -struct IPAFrameContext : public FrameContext { +struct IPAFrameContext { struct { uint32_t exposure; double gain; diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h index 34f7403a4..7e5e94904 100644 --- a/src/ipa/simple/ipa_context.h +++ b/src/ipa/simple/ipa_context.h @@ -63,7 +63,7 @@ struct IPAActiveState { } knobs; }; -struct IPAFrameContext : public FrameContext { +struct IPAFrameContext { Matrix ccm; struct {