diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index 49d49658c..f982e0bfd 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -440,6 +440,15 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms) fillDeviceStatus(params.sensorControls, ipaContext); fillSyncParams(params, ipaContext); + /* + * When there are controls, it's important that we don't skip running the + * IPAs, as that can mess with synchronisation. Crucially though, we need + * to know whether there were controls when this comes back as the + * _delayed_ metadata, hence why we flag this in the metadata itself. + */ + if (!params.requestControls.empty()) + rpiMetadata.set("ipa.request_controls", true); + if (params.buffers.embedded) { /* * Pipeline handler has supplied us with an embedded data buffer, @@ -460,7 +469,7 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms) */ AgcStatus agcStatus; bool hdrChange = false; - RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext]; + RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext % rpiMetadata_.size()]; if (!delayedMetadata.get("agc.status", agcStatus)) { rpiMetadata.set("agc.delayed_status", agcStatus); hdrChange = agcStatus.hdr.mode != hdrStatus_.mode; @@ -473,9 +482,13 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms) */ helper_->prepare(embeddedBuffer, rpiMetadata); + bool delayedRequestControls = false; + delayedMetadata.get("ipa.request_controls", delayedRequestControls); + /* Allow a 10% margin on the comparison below. */ Duration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns; - if (lastRunTimestamp_ && frameCount_ > invalidCount_ && + if (!delayedRequestControls && params.requestControls.empty() && + lastRunTimestamp_ && frameCount_ > invalidCount_ && delta < controllerMinFrameDuration_ * 0.9 && !hdrChange) { /* * Ensure we merge the previous frame's metadata with the current @@ -558,7 +571,7 @@ void IpaBase::processStats(const ProcessParams ¶ms) ControlList ctrls(sensorCtrls_); applyAGC(&agcStatus, ctrls, offset); rpiMetadata.set("agc.status", agcStatus); - setDelayedControls.emit(ctrls, ipaContext); + setDelayedControls.emit(ctrls, params.ipaContext); setCameraTimeoutValue(); } @@ -975,8 +988,6 @@ void IpaBase::applyControls(const ControlList &controls) /* The control provides units of microseconds. */ agc->setFixedExposureTime(0, ctrl.second.get() * 1.0us); - - libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get()); break; } @@ -1000,9 +1011,6 @@ void IpaBase::applyControls(const ControlList &controls) break; agc->setFixedGain(0, ctrl.second.get()); - - libcameraMetadata_.set(controls::AnalogueGain, - ctrl.second.get()); break; } diff --git a/src/libcamera/control_ids_rpi.yaml b/src/libcamera/control_ids_rpi.yaml index 76f4b5a1b..40a78f80a 100644 --- a/src/libcamera/control_ids_rpi.yaml +++ b/src/libcamera/control_ids_rpi.yaml @@ -299,4 +299,13 @@ controls: This control returns performance metrics for the CNN processing stage. Two values are returned in this span, the runtime of the CNN/DNN stage and the DSP stage in milliseconds. + + - ControlListSequence: + type: int64_t + direction: out + description: | + This is the sequence number of the request whose control list has + just been applied. Controls normally take several frames to apply, + so the number here will refer to a request submitted a number of + frames earlier. ... diff --git a/src/libcamera/pipeline/rpi/common/delayed_controls.cpp b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp index 19c719460..b647174f5 100644 --- a/src/libcamera/pipeline/rpi/common/delayed_controls.cpp +++ b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp @@ -32,14 +32,15 @@ namespace RPi { * * Some sensor controls take effect with a delay as the sensor needs time to * adjust, for example exposure and analog gain. This is a helper class to deal - * with such controls and the intended users are pipeline handlers. + * with such controls. * - * The idea is to extend the concept of the buffer depth of a pipeline the - * application needs to maintain to also cover controls. Just as with buffer - * depth if the application keeps the number of requests queued above the - * control depth the controls are guaranteed to take effect for the correct - * request. The control depth is determined by the control with the greatest - * delay. + * The idea is to maintain a queue of controls that have been submitted. + * Whenever a new frame starts, we can "peak back" into this queue to send + * controls at the correct number of frames in advance to account for each + * control's delay. + * + * The overall delay for controls becomes the maximum delay of any of the + * controls. */ /** @@ -119,7 +120,6 @@ DelayedControls::DelayedControls(V4L2Device *device, */ void DelayedControls::reset(unsigned int cookie) { - queueCount_ = 1; writeCount_ = 0; cookies_[0] = cookie; @@ -146,18 +146,25 @@ void DelayedControls::reset(unsigned int cookie) * \brief Push a set of controls on the queue * \param[in] controls List of controls to add to the device queue * - * Push a set of controls to the control queue. This increases the control queue - * depth by one. + * Push a set of controls to the control queue. The next call to applyControls + * will advance the slot in the queue where the next call to push will write + * controls. * * \returns true if \a controls are accepted, or false otherwise */ bool DelayedControls::push(const ControlList &controls, const unsigned int cookie) { /* Copy state from previous frame. */ - for (auto &ctrl : values_) { - Info &info = ctrl.second[queueCount_]; - info = values_[ctrl.first][queueCount_ - 1]; - info.updated = false; + if (writeCount_ > 0) { + for (auto &ctrl : values_) { + Info &info = ctrl.second[writeCount_]; + info = values_[ctrl.first][writeCount_ - 1]; + info.updated = false; + } + } else { + /* Although it works, we don't expect this. */ + LOG(RPiDelayedControls, Warning) + << "push called before applyControls"; } /* Update with new controls. */ @@ -175,18 +182,17 @@ bool DelayedControls::push(const ControlList &controls, const unsigned int cooki if (controlParams_.find(id) == controlParams_.end()) return false; - Info &info = values_[id][queueCount_]; + Info &info = values_[id][writeCount_]; info = Info(control.second); LOG(RPiDelayedControls, Debug) << "Queuing " << id->name() << " to " << info.toString() - << " at index " << queueCount_; + << " at index " << writeCount_; } - cookies_[queueCount_] = cookie; - queueCount_++; + cookies_[writeCount_] = cookie; return true; } @@ -200,9 +206,9 @@ bool DelayedControls::push(const ControlList &controls, const unsigned int cooki * the callers responsibility to not read too old sequence numbers that have been * pushed out of the history. * - * Historic values are evicted by pushing new values onto the queue using - * push(). The max history from the current sequence number that yields valid - * values are thus 16 minus number of controls pushed. + * Historic values are evicted by new frames arriving and applyControls + * advancing the head of the queue in the ring buffer. The valid history in + * the queue consists of 16 entries from this head of the queue. * * \return The controls at \a sequence number */ @@ -234,6 +240,10 @@ std::pair DelayedControls::get(uint32_t sequence) * number. Any user of these helpers is responsible to inform the helper about * the start of any frame. This can be connected with ease to the start of a * exposure (SOE) V4L2 event. + * + * Controls will be sent to the device, each staggered by the appropriate + * number of frames for that control, so that they are applied at the same + * time. */ void DelayedControls::applyControls(uint32_t sequence) { @@ -277,12 +287,12 @@ void DelayedControls::applyControls(uint32_t sequence) } } - writeCount_ = sequence + 1; - - while (writeCount_ > queueCount_) { + while (writeCount_ < sequence + 1) { + writeCount_++; LOG(RPiDelayedControls, Debug) - << "Queue is empty, auto queue no-op."; - push({}, cookies_[queueCount_ - 1]); + << "Pushing noop with for index " << writeCount_ + << " with cookie " << cookies_[writeCount_ - 1]; + push({}, cookies_[writeCount_ - 1]); } device_->setControls(&out); diff --git a/src/libcamera/pipeline/rpi/common/delayed_controls.h b/src/libcamera/pipeline/rpi/common/delayed_controls.h index 487b0057b..48ad5a0f4 100644 --- a/src/libcamera/pipeline/rpi/common/delayed_controls.h +++ b/src/libcamera/pipeline/rpi/common/delayed_controls.h @@ -76,7 +76,6 @@ class DelayedControls std::unordered_map controlParams_; unsigned int maxDelay_; - uint32_t queueCount_; uint32_t writeCount_; std::unordered_map> values_; RingBuffer cookies_; diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index f6ef8a3be..ace38997b 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp @@ -1528,4 +1528,53 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request } } +static bool isControlDelayed(unsigned int id) +{ + return id == controls::ExposureTime || + id == controls::AnalogueGain || + id == controls::FrameDurationLimits || + id == controls::AeEnable || + id == controls::ExposureTimeMode || + id == controls::AnalogueGainMode; +} + +void CameraData::handleControlLists(uint32_t delayContext, ControlList ¶mControls) +{ + /* + * The delayContext is the sequence number after it's gone through the various + * pipeline delays, so that's what gets reported as the "ControlListSequence" + * in the metadata, being the sequence number of the request whose ControlList + * has just been applied. + */ + Request *request = requestQueue_.front(); + request->_d()->metadata().set(controls::rpi::ControlListSequence, delayContext); + + /* + * Controls that take effect immediately (typically ISP controls) have to be + * delayed so as to synchronise with those controls that do get delayed. So we + * must remove them from the current request, and push them onto a queue so + * that they can be used later. + * + * Note that we are given a separate control list (paramControls) so that + * we can pass back the controls that really need to happen now, without + * disturbing the controls that were submitted with the request. + */ + ASSERT(paramControls.empty()); + immediateControls_.push({ request->sequence(), {} }); + for (const auto &ctrl : request->controls()) { + if (isControlDelayed(ctrl.first)) + paramControls.set(ctrl.first, ctrl.second); + else + immediateControls_.back().controls.set(ctrl.first, ctrl.second); + } + + /* "Immediate" controls that have become due are now merged back into this request. */ + while (!immediateControls_.empty() && + immediateControls_.front().controlListId <= delayContext) { + paramControls.merge(immediateControls_.front().controls, + ControlList::MergePolicy::OverwriteExisting); + immediateControls_.pop(); + } +} + } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h index 7bfac33eb..bc927b558 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h @@ -180,10 +180,19 @@ class CameraData : public Camera::Private ClockRecovery wallClockRecovery_; + struct ImmediateControlsEntry { + uint64_t controlListId; + ControlList controls; + }; + std::queue immediateControls_; + protected: void fillRequestMetadata(const ControlList &bufferControls, Request *request); + void handleControlLists(uint32_t delayContext, + ControlList ¶msControls); + virtual void tryRunPipeline() = 0; private: diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp index 12194d977..c9d89d58b 100644 --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp @@ -2311,9 +2311,6 @@ void PiSPCameraData::tryRunPipeline() fillRequestMetadata(job.sensorControls, request); - /* Set our state to say the pipeline is active. */ - state_ = State::Busy; - unsigned int bayerId = cfe_[Cfe::Output0].getBufferId(job.buffers[&cfe_[Cfe::Output0]]); unsigned int statsId = cfe_[Cfe::Stats].getBufferId(job.buffers[&cfe_[Cfe::Stats]]); ASSERT(bayerId && statsId); @@ -2330,7 +2327,13 @@ void PiSPCameraData::tryRunPipeline() params.ipaContext = requestQueue_.front()->sequence(); params.delayContext = job.delayContext; params.sensorControls = std::move(job.sensorControls); - params.requestControls = request->controls(); + /* params.requestControls is set by handleControlLists. */ + + /* This sorts out synchronisation with ControlLists in earlier requests. */ + handleControlLists(job.delayContext, params.requestControls); + + /* Set our state to say the pipeline is active. */ + state_ = State::Busy; if (sensorMetadata_) { unsigned int embeddedId = diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp index 805e3c78c..ac86a43ad 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -984,9 +984,6 @@ void Vc4CameraData::tryRunPipeline() fillRequestMetadata(bayerFrame.controls, request); - /* Set our state to say the pipeline is active. */ - state_ = State::Busy; - unsigned int bayer = unicam_[Unicam::Image].getBufferId(bayerFrame.buffer); LOG(RPI, Debug) << "Signalling prepareIsp:" @@ -995,10 +992,16 @@ void Vc4CameraData::tryRunPipeline() ipa::RPi::PrepareParams params; params.buffers.bayer = RPi::MaskBayerData | bayer; params.sensorControls = std::move(bayerFrame.controls); - params.requestControls = request->controls(); params.ipaContext = request->sequence(); params.delayContext = bayerFrame.delayContext; params.buffers.embedded = 0; + /* params.requestControls is set by handleControlLists. */ + + /* This sorts out synchronisation with ControlLists in earlier requests. */ + handleControlLists(bayerFrame.delayContext, params.requestControls); + + /* Set our state to say the pipeline is active. */ + state_ = State::Busy; if (embeddedBuffer) { unsigned int embeddedId = unicam_[Unicam::Embedded].getBufferId(embeddedBuffer);