diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index faa77719..dacafa57 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -431,6 +431,15 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms) rpiMetadata.clear(); fillDeviceStatus(params.sensorControls, 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, @@ -451,7 +460,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; @@ -464,9 +473,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 @@ -535,7 +548,7 @@ void IpaBase::processStats(const ProcessParams ¶ms) ControlList ctrls(sensorCtrls_); applyAGC(&agcStatus, ctrls); rpiMetadata.set("agc.status", agcStatus); - setDelayedControls.emit(ctrls, ipaContext); + setDelayedControls.emit(ctrls, params.ipaContext); setCameraTimeoutValue(); } @@ -951,8 +964,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; } @@ -976,9 +987,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/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp index f6ef8a3b..0c82f549 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 &requestControls) +{ + /* + * 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 passed a copy of the request's controls, so that we can + * avoid disturbing what was in the original request. + */ + ControlList controls = std::move(requestControls); + requestControls.clear(); + immediateControls_.push({ request->sequence(), {} }); + for (const auto &ctrl : controls) { + if (isControlDelayed(ctrl.first)) + requestControls.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) { + requestControls.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 7bfac33e..134262b6 100644 --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h @@ -180,10 +180,19 @@ public: 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 &requestControls); + virtual void tryRunPipeline() = 0; private: diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp index cc7e32c3..690d2511 100644 --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp @@ -2322,9 +2322,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); @@ -2343,6 +2340,12 @@ void PiSPCameraData::tryRunPipeline() params.sensorControls = std::move(job.sensorControls); params.requestControls = request->controls(); + /* 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 = cfe_[Cfe::Embedded].getBufferId(job.buffers[&cfe_[Cfe::Embedded]]); diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp index f99cfdbc..28e7ad46 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -939,9 +939,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:" @@ -955,6 +952,12 @@ void Vc4CameraData::tryRunPipeline() params.delayContext = bayerFrame.delayContext; params.buffers.embedded = 0; + /* 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);