[v3,3/3] pipeline: rpi: Make control lists in requests properly atomic
diff mbox series

Message ID 20260428133952.6582-4-david.plowman@raspberrypi.com
State Superseded
Headers show
Series
  • Atomic control lists on Raspberry Pi
Related show

Commit Message

David Plowman April 28, 2026, 1:26 p.m. UTC 
When a request is about to be processed, this commit separates
"delayed controls" (camera-related ones that take "a few frames" to
apply) from "immediate controls" (ISP-related controls) that will
happen instantly.

The immediate controls are held back until the delayed controls that
they were submitted with, have happened. This means all the controls
submitted in a request happen atomically.

We therefore already have the sequence number of the request whose
controls have just been applied, so we additionally attach this to the
current request as "ControlId" metadata.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/rpi/common/ipa_base.cpp               | 24 ++++++----
 .../pipeline/rpi/common/pipeline_base.cpp     | 46 +++++++++++++++++++
 .../pipeline/rpi/common/pipeline_base.h       |  8 ++++
 src/libcamera/pipeline/rpi/pisp/pisp.cpp      |  3 ++
 src/libcamera/pipeline/rpi/vc4/vc4.cpp        |  3 ++
 5 files changed, 76 insertions(+), 8 deletions(-)

Comments

Jacopo Mondi May 7, 2026, 7:42 a.m. UTC | #1 
Hi David

On Tue, Apr 28, 2026 at 02:26:39PM +0100, David Plowman wrote:
> When a request is about to be processed, this commit separates
> "delayed controls" (camera-related ones that take "a few frames" to
> apply) from "immediate controls" (ISP-related controls) that will
> happen instantly.
>
> The immediate controls are held back until the delayed controls that
> they were submitted with, have happened. This means all the controls
> submitted in a request happen atomically.
>
> We therefore already have the sequence number of the request whose
> controls have just been applied, so we additionally attach this to the
> current request as "ControlId" metadata.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/ipa/rpi/common/ipa_base.cpp               | 24 ++++++----
>  .../pipeline/rpi/common/pipeline_base.cpp     | 46 +++++++++++++++++++
>  .../pipeline/rpi/common/pipeline_base.h       |  8 ++++
>  src/libcamera/pipeline/rpi/pisp/pisp.cpp      |  3 ++
>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |  3 ++
>  5 files changed, 76 insertions(+), 8 deletions(-)
>
> 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 &params)
>  	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 &params)
>  	 */
>  	AgcStatus agcStatus;
>  	bool hdrChange = false;
> -	RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext];
> +	RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext % rpiMetadata_.size()];
>  	if (!delayedMetadata.get<AgcStatus>("agc.status", agcStatus)) {
>  		rpiMetadata.set("agc.delayed_status", agcStatus);
>  		hdrChange = agcStatus.hdr.mode != hdrStatus_.mode;
> @@ -464,9 +473,13 @@ void IpaBase::prepareIsp(const PrepareParams &params)
>  	 */
>  	helper_->prepare(embeddedBuffer, rpiMetadata);
>
> +	bool delayedRequestControls = false;
> +	delayedMetadata.get<bool>("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 &params)
>  		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<int32_t>() * 1.0us);
> -
> -			libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>());
>  			break;
>  		}
>
> @@ -976,9 +987,6 @@ void IpaBase::applyControls(const ControlList &controls)
>  				break;
>
>  			agc->setFixedGain(0, ctrl.second.get<float>());
> -
> -			libcameraMetadata_.set(controls::AnalogueGain,
> -					       ctrl.second.get<float>());
>  			break;
>  		}
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index f6ef8a3b..28f44753 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -1528,4 +1528,50 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request
>  	}
>  }
>
> +static bool isControlDelayed(unsigned int id)
> +{
> +	return id == controls::ExposureTime ||
> +	       id == controls::AnalogueGain ||
> +	       id == controls::FrameDurationLimits ||

Fine with these that match a V4L2 control

> +	       id == controls::AeEnable ||
> +	       id == controls::ExposureTimeMode ||
> +	       id == controls::AnalogueGainMode;

But aren't these knobs that control the IPA behaviour rather than
controls for the HW (which take a delay to apply) ?

> +}
> +
> +void CameraData::handleControlLists(uint32_t delayContext)
> +{
> +	/*
> +	 * 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.
> +	 */
> +	ControlList controls = std::move(request->controls());
> +	request->controls().clear();

I'm fine with the control list sequence number being reported through
controls::rpi::ControlListSequence.

I'm less at ease with the idea we're re-constructing the ControlList
inside a Request. Any application that holds a reference to any of
those controls will fail if we're messing up the storage where those
controls live.

I recall you said you don't really care about the controls of a
completed request as there's nothing interesting there. Does that mean
you would be fine simply reporting ControlListSequence and not
reworking the ControlList inside a Request ?



> +	immediateControls_.push({ request->sequence(), {} });
> +	for (const auto &ctrl : controls) {
> +		if (isControlDelayed(ctrl.first))
> +			request->controls().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) {
> +		request->controls().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..d3f044a4 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> @@ -180,10 +180,18 @@ public:
>
>  	ClockRecovery wallClockRecovery_;
>
> +	struct ImmediateControlsEntry {
> +		uint64_t controlListId;
> +		ControlList controls;
> +	};
> +	std::queue<ImmediateControlsEntry> immediateControls_;
> +
>  protected:
>  	void fillRequestMetadata(const ControlList &bufferControls,
>  				 Request *request);
>
> +	void handleControlLists(uint32_t delayContext);
> +
>  	virtual void tryRunPipeline() = 0;
>
>  private:
> diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
> index c7799f2b..70bbac5a 100644
> --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
> +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
> @@ -2322,6 +2322,9 @@ void PiSPCameraData::tryRunPipeline()
>
>  	fillRequestMetadata(job.sensorControls, request);
>
> +	/* This sorts out synchronisation with ControlLists in earlier requests. */
> +	handleControlLists(job.delayContext);
> +
>  	/* Set our state to say the pipeline is active. */
>  	state_ = State::Busy;
>
> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> index f99cfdbc..a1b44af1 100644
> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> @@ -939,6 +939,9 @@ void Vc4CameraData::tryRunPipeline()
>
>  	fillRequestMetadata(bayerFrame.controls, request);
>
> +	/* This sorts out synchronisation with ControlLists in earlier requests. */
> +	handleControlLists(bayerFrame.delayContext);
> +
>  	/* Set our state to say the pipeline is active. */
>  	state_ = State::Busy;
>
> --
> 2.47.3
>
David Plowman May 7, 2026, 12:44 p.m. UTC | #2 
Hi Jacopo

Thanks for looking at this again.

On Thu, 7 May 2026 at 08:42, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:
>
> Hi David
>
> On Tue, Apr 28, 2026 at 02:26:39PM +0100, David Plowman wrote:
> > When a request is about to be processed, this commit separates
> > "delayed controls" (camera-related ones that take "a few frames" to
> > apply) from "immediate controls" (ISP-related controls) that will
> > happen instantly.
> >
> > The immediate controls are held back until the delayed controls that
> > they were submitted with, have happened. This means all the controls
> > submitted in a request happen atomically.
> >
> > We therefore already have the sequence number of the request whose
> > controls have just been applied, so we additionally attach this to the
> > current request as "ControlId" metadata.
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  src/ipa/rpi/common/ipa_base.cpp               | 24 ++++++----
> >  .../pipeline/rpi/common/pipeline_base.cpp     | 46 +++++++++++++++++++
> >  .../pipeline/rpi/common/pipeline_base.h       |  8 ++++
> >  src/libcamera/pipeline/rpi/pisp/pisp.cpp      |  3 ++
> >  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |  3 ++
> >  5 files changed, 76 insertions(+), 8 deletions(-)
> >
> > 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 &params)
> >       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 &params)
> >        */
> >       AgcStatus agcStatus;
> >       bool hdrChange = false;
> > -     RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext];
> > +     RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext % rpiMetadata_.size()];
> >       if (!delayedMetadata.get<AgcStatus>("agc.status", agcStatus)) {
> >               rpiMetadata.set("agc.delayed_status", agcStatus);
> >               hdrChange = agcStatus.hdr.mode != hdrStatus_.mode;
> > @@ -464,9 +473,13 @@ void IpaBase::prepareIsp(const PrepareParams &params)
> >        */
> >       helper_->prepare(embeddedBuffer, rpiMetadata);
> >
> > +     bool delayedRequestControls = false;
> > +     delayedMetadata.get<bool>("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 &params)
> >               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<int32_t>() * 1.0us);
> > -
> > -                     libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>());
> >                       break;
> >               }
> >
> > @@ -976,9 +987,6 @@ void IpaBase::applyControls(const ControlList &controls)
> >                               break;
> >
> >                       agc->setFixedGain(0, ctrl.second.get<float>());
> > -
> > -                     libcameraMetadata_.set(controls::AnalogueGain,
> > -                                            ctrl.second.get<float>());
> >                       break;
> >               }
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index f6ef8a3b..28f44753 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -1528,4 +1528,50 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request
> >       }
> >  }
> >
> > +static bool isControlDelayed(unsigned int id)
> > +{
> > +     return id == controls::ExposureTime ||
> > +            id == controls::AnalogueGain ||
> > +            id == controls::FrameDurationLimits ||
>
> Fine with these that match a V4L2 control
>
> > +            id == controls::AeEnable ||
> > +            id == controls::ExposureTimeMode ||
> > +            id == controls::AnalogueGainMode;
>
> But aren't these knobs that control the IPA behaviour rather than
> controls for the HW (which take a delay to apply) ?

That's true, but on the Pi, changing exposure or analogue gain values,
even in manual mode, is always mediated by the AEC/AGC algorithm. So
if someone sets (for example) ExposureTimeMode to Manual, and sends a
shutter time, then we have to put the algorithm into the correct mode
at the same moment, or the update will just get ignored.

I guess you could ask what state the algorithm is really in when it's
done something, but which hasn't happened yet. But I'll leave that
question for Schrodinger and his cat!

>
> > +}
> > +
> > +void CameraData::handleControlLists(uint32_t delayContext)
> > +{
> > +     /*
> > +      * 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.
> > +      */
> > +     ControlList controls = std::move(request->controls());
> > +     request->controls().clear();
>
> I'm fine with the control list sequence number being reported through
> controls::rpi::ControlListSequence.
>
> I'm less at ease with the idea we're re-constructing the ControlList
> inside a Request. Any application that holds a reference to any of
> those controls will fail if we're messing up the storage where those
> controls live.
>
> I recall you said you don't really care about the controls of a
> completed request as there's nothing interesting there. Does that mean
> you would be fine simply reporting ControlListSequence and not
> reworking the ControlList inside a Request ?

I'll post a v4 to address this in just a moment.

Thanks!

David

>
>
>
> > +     immediateControls_.push({ request->sequence(), {} });
> > +     for (const auto &ctrl : controls) {
> > +             if (isControlDelayed(ctrl.first))
> > +                     request->controls().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) {
> > +             request->controls().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..d3f044a4 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > @@ -180,10 +180,18 @@ public:
> >
> >       ClockRecovery wallClockRecovery_;
> >
> > +     struct ImmediateControlsEntry {
> > +             uint64_t controlListId;
> > +             ControlList controls;
> > +     };
> > +     std::queue<ImmediateControlsEntry> immediateControls_;
> > +
> >  protected:
> >       void fillRequestMetadata(const ControlList &bufferControls,
> >                                Request *request);
> >
> > +     void handleControlLists(uint32_t delayContext);
> > +
> >       virtual void tryRunPipeline() = 0;
> >
> >  private:
> > diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
> > index c7799f2b..70bbac5a 100644
> > --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
> > +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
> > @@ -2322,6 +2322,9 @@ void PiSPCameraData::tryRunPipeline()
> >
> >       fillRequestMetadata(job.sensorControls, request);
> >
> > +     /* This sorts out synchronisation with ControlLists in earlier requests. */
> > +     handleControlLists(job.delayContext);
> > +
> >       /* Set our state to say the pipeline is active. */
> >       state_ = State::Busy;
> >
> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > index f99cfdbc..a1b44af1 100644
> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> > @@ -939,6 +939,9 @@ void Vc4CameraData::tryRunPipeline()
> >
> >       fillRequestMetadata(bayerFrame.controls, request);
> >
> > +     /* This sorts out synchronisation with ControlLists in earlier requests. */
> > +     handleControlLists(bayerFrame.delayContext);
> > +
> >       /* Set our state to say the pipeline is active. */
> >       state_ = State::Busy;
> >
> > --
> > 2.47.3
> >

Patch
diff mbox series 

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 &params)
 	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 &params)
 	 */
 	AgcStatus agcStatus;
 	bool hdrChange = false;
-	RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext];
+	RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext % rpiMetadata_.size()];
 	if (!delayedMetadata.get<AgcStatus>("agc.status", agcStatus)) {
 		rpiMetadata.set("agc.delayed_status", agcStatus);
 		hdrChange = agcStatus.hdr.mode != hdrStatus_.mode;
@@ -464,9 +473,13 @@  void IpaBase::prepareIsp(const PrepareParams &params)
 	 */
 	helper_->prepare(embeddedBuffer, rpiMetadata);
 
+	bool delayedRequestControls = false;
+	delayedMetadata.get<bool>("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 &params)
 		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<int32_t>() * 1.0us);
-
-			libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>());
 			break;
 		}
 
@@ -976,9 +987,6 @@  void IpaBase::applyControls(const ControlList &controls)
 				break;
 
 			agc->setFixedGain(0, ctrl.second.get<float>());
-
-			libcameraMetadata_.set(controls::AnalogueGain,
-					       ctrl.second.get<float>());
 			break;
 		}
 
diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index f6ef8a3b..28f44753 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -1528,4 +1528,50 @@  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)
+{
+	/*
+	 * 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.
+	 */
+	ControlList controls = std::move(request->controls());
+	request->controls().clear();
+	immediateControls_.push({ request->sequence(), {} });
+	for (const auto &ctrl : controls) {
+		if (isControlDelayed(ctrl.first))
+			request->controls().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) {
+		request->controls().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..d3f044a4 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
@@ -180,10 +180,18 @@  public:
 
 	ClockRecovery wallClockRecovery_;
 
+	struct ImmediateControlsEntry {
+		uint64_t controlListId;
+		ControlList controls;
+	};
+	std::queue<ImmediateControlsEntry> immediateControls_;
+
 protected:
 	void fillRequestMetadata(const ControlList &bufferControls,
 				 Request *request);
 
+	void handleControlLists(uint32_t delayContext);
+
 	virtual void tryRunPipeline() = 0;
 
 private:
diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
index c7799f2b..70bbac5a 100644
--- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp
+++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp
@@ -2322,6 +2322,9 @@  void PiSPCameraData::tryRunPipeline()
 
 	fillRequestMetadata(job.sensorControls, request);
 
+	/* This sorts out synchronisation with ControlLists in earlier requests. */
+	handleControlLists(job.delayContext);
+
 	/* Set our state to say the pipeline is active. */
 	state_ = State::Busy;
 
diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
index f99cfdbc..a1b44af1 100644
--- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
+++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
@@ -939,6 +939,9 @@  void Vc4CameraData::tryRunPipeline()
 
 	fillRequestMetadata(bayerFrame.controls, request);
 
+	/* This sorts out synchronisation with ControlLists in earlier requests. */
+	handleControlLists(bayerFrame.delayContext);
+
 	/* Set our state to say the pipeline is active. */
 	state_ = State::Busy;