[{"id":38755,"web_url":"https://patchwork.libcamera.org/comment/38755/","msgid":"<afxAenFX3ouRG5o2@zed>","date":"2026-05-07T07:42:21","subject":"Re: [PATCH v3 3/3] pipeline: rpi: Make control lists in requests\n\tproperly atomic","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi David\n\nOn Tue, Apr 28, 2026 at 02:26:39PM +0100, David Plowman wrote:\n> When a request is about to be processed, this commit separates\n> \"delayed controls\" (camera-related ones that take \"a few frames\" to\n> apply) from \"immediate controls\" (ISP-related controls) that will\n> happen instantly.\n>\n> The immediate controls are held back until the delayed controls that\n> they were submitted with, have happened. This means all the controls\n> submitted in a request happen atomically.\n>\n> We therefore already have the sequence number of the request whose\n> controls have just been applied, so we additionally attach this to the\n> current request as \"ControlId\" metadata.\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  src/ipa/rpi/common/ipa_base.cpp               | 24 ++++++----\n>  .../pipeline/rpi/common/pipeline_base.cpp     | 46 +++++++++++++++++++\n>  .../pipeline/rpi/common/pipeline_base.h       |  8 ++++\n>  src/libcamera/pipeline/rpi/pisp/pisp.cpp      |  3 ++\n>  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |  3 ++\n>  5 files changed, 76 insertions(+), 8 deletions(-)\n>\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index faa77719..dacafa57 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -431,6 +431,15 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n>  \trpiMetadata.clear();\n>  \tfillDeviceStatus(params.sensorControls, ipaContext);\n>\n> +\t/*\n> +\t * When there are controls, it's important that we don't skip running the\n> +\t * IPAs, as that can mess with synchronisation. Crucially though, we need\n> +\t * to know whether there were controls when this comes back as the\n> +\t * _delayed_ metadata, hence why we flag this in the metadata itself.\n> +\t */\n> +\tif (!params.requestControls.empty())\n> +\t\trpiMetadata.set(\"ipa.request_controls\", true);\n> +\n>  \tif (params.buffers.embedded) {\n>  \t\t/*\n>  \t\t * Pipeline handler has supplied us with an embedded data buffer,\n> @@ -451,7 +460,7 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n>  \t */\n>  \tAgcStatus agcStatus;\n>  \tbool hdrChange = false;\n> -\tRPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext];\n> +\tRPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext % rpiMetadata_.size()];\n>  \tif (!delayedMetadata.get<AgcStatus>(\"agc.status\", agcStatus)) {\n>  \t\trpiMetadata.set(\"agc.delayed_status\", agcStatus);\n>  \t\thdrChange = agcStatus.hdr.mode != hdrStatus_.mode;\n> @@ -464,9 +473,13 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n>  \t */\n>  \thelper_->prepare(embeddedBuffer, rpiMetadata);\n>\n> +\tbool delayedRequestControls = false;\n> +\tdelayedMetadata.get<bool>(\"ipa.request_controls\", delayedRequestControls);\n> +\n>  \t/* Allow a 10% margin on the comparison below. */\n>  \tDuration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;\n> -\tif (lastRunTimestamp_ && frameCount_ > invalidCount_ &&\n> +\tif (!delayedRequestControls && params.requestControls.empty() &&\n> +\t    lastRunTimestamp_ && frameCount_ > invalidCount_ &&\n>  \t    delta < controllerMinFrameDuration_ * 0.9 && !hdrChange) {\n>  \t\t/*\n>  \t\t * Ensure we merge the previous frame's metadata with the current\n> @@ -535,7 +548,7 @@ void IpaBase::processStats(const ProcessParams &params)\n>  \t\tControlList ctrls(sensorCtrls_);\n>  \t\tapplyAGC(&agcStatus, ctrls);\n>  \t\trpiMetadata.set(\"agc.status\", agcStatus);\n> -\t\tsetDelayedControls.emit(ctrls, ipaContext);\n> +\t\tsetDelayedControls.emit(ctrls, params.ipaContext);\n>  \t\tsetCameraTimeoutValue();\n>  \t}\n>\n> @@ -951,8 +964,6 @@ void IpaBase::applyControls(const ControlList &controls)\n>\n>  \t\t\t/* The control provides units of microseconds. */\n>  \t\t\tagc->setFixedExposureTime(0, ctrl.second.get<int32_t>() * 1.0us);\n> -\n> -\t\t\tlibcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>());\n>  \t\t\tbreak;\n>  \t\t}\n>\n> @@ -976,9 +987,6 @@ void IpaBase::applyControls(const ControlList &controls)\n>  \t\t\t\tbreak;\n>\n>  \t\t\tagc->setFixedGain(0, ctrl.second.get<float>());\n> -\n> -\t\t\tlibcameraMetadata_.set(controls::AnalogueGain,\n> -\t\t\t\t\t       ctrl.second.get<float>());\n>  \t\t\tbreak;\n>  \t\t}\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index f6ef8a3b..28f44753 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -1528,4 +1528,50 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request\n>  \t}\n>  }\n>\n> +static bool isControlDelayed(unsigned int id)\n> +{\n> +\treturn id == controls::ExposureTime ||\n> +\t       id == controls::AnalogueGain ||\n> +\t       id == controls::FrameDurationLimits ||\n\nFine with these that match a V4L2 control\n\n> +\t       id == controls::AeEnable ||\n> +\t       id == controls::ExposureTimeMode ||\n> +\t       id == controls::AnalogueGainMode;\n\nBut aren't these knobs that control the IPA behaviour rather than\ncontrols for the HW (which take a delay to apply) ?\n\n> +}\n> +\n> +void CameraData::handleControlLists(uint32_t delayContext)\n> +{\n> +\t/*\n> +\t * The delayContext is the sequence number after it's gone through the various\n> +\t * pipeline delays, so that's what gets reported as the \"ControlListSequence\"\n> +\t * in the metadata, being the sequence number of the request whose ControlList\n> +\t * has just been applied.\n> +\t */\n> +\tRequest *request = requestQueue_.front();\n> +\trequest->_d()->metadata().set(controls::rpi::ControlListSequence, delayContext);\n> +\n> +\t/*\n> +\t * Controls that take effect immediately (typically ISP controls) have to be\n> +\t * delayed so as to synchronise with those controls that do get delayed. So we\n> +\t * must remove them from the current request, and push them onto a queue so\n> +\t * that they can be used later.\n> +\t */\n> +\tControlList controls = std::move(request->controls());\n> +\trequest->controls().clear();\n\nI'm fine with the control list sequence number being reported through\ncontrols::rpi::ControlListSequence.\n\nI'm less at ease with the idea we're re-constructing the ControlList\ninside a Request. Any application that holds a reference to any of\nthose controls will fail if we're messing up the storage where those\ncontrols live.\n\nI recall you said you don't really care about the controls of a\ncompleted request as there's nothing interesting there. Does that mean\nyou would be fine simply reporting ControlListSequence and not\nreworking the ControlList inside a Request ?\n\n\n\n> +\timmediateControls_.push({ request->sequence(), {} });\n> +\tfor (const auto &ctrl : controls) {\n> +\t\tif (isControlDelayed(ctrl.first))\n> +\t\t\trequest->controls().set(ctrl.first, ctrl.second);\n> +\t\telse\n> +\t\t\timmediateControls_.back().controls.set(ctrl.first, ctrl.second);\n> +\t}\n> +\n> +\t/* \"Immediate\" controls that have become due are now merged back into this request. */\n> +\twhile (!immediateControls_.empty() &&\n> +\t       immediateControls_.front().controlListId <= delayContext) {\n> +\t\trequest->controls().merge(immediateControls_.front().controls,\n> +\t\t\t\t\t  ControlList::MergePolicy::OverwriteExisting);\n> +\t\timmediateControls_.pop();\n> +\t}\n> +}\n> +\n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index 7bfac33e..d3f044a4 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -180,10 +180,18 @@ public:\n>\n>  \tClockRecovery wallClockRecovery_;\n>\n> +\tstruct ImmediateControlsEntry {\n> +\t\tuint64_t controlListId;\n> +\t\tControlList controls;\n> +\t};\n> +\tstd::queue<ImmediateControlsEntry> immediateControls_;\n> +\n>  protected:\n>  \tvoid fillRequestMetadata(const ControlList &bufferControls,\n>  \t\t\t\t Request *request);\n>\n> +\tvoid handleControlLists(uint32_t delayContext);\n> +\n>  \tvirtual void tryRunPipeline() = 0;\n>\n>  private:\n> diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> index c7799f2b..70bbac5a 100644\n> --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> @@ -2322,6 +2322,9 @@ void PiSPCameraData::tryRunPipeline()\n>\n>  \tfillRequestMetadata(job.sensorControls, request);\n>\n> +\t/* This sorts out synchronisation with ControlLists in earlier requests. */\n> +\thandleControlLists(job.delayContext);\n> +\n>  \t/* Set our state to say the pipeline is active. */\n>  \tstate_ = State::Busy;\n>\n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index f99cfdbc..a1b44af1 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -939,6 +939,9 @@ void Vc4CameraData::tryRunPipeline()\n>\n>  \tfillRequestMetadata(bayerFrame.controls, request);\n>\n> +\t/* This sorts out synchronisation with ControlLists in earlier requests. */\n> +\thandleControlLists(bayerFrame.delayContext);\n> +\n>  \t/* Set our state to say the pipeline is active. */\n>  \tstate_ = State::Busy;\n>\n> --\n> 2.47.3\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B2C51BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  7 May 2026 07:42:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 609BE63020;\n\tThu,  7 May 2026 09:42:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B387A62FB1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2026 09:42:24 +0200 (CEST)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A64B05B2;\n\tThu,  7 May 2026 09:42:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pFHVHmNT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1778139740;\n\tbh=/tc9e8oty7llGtPM/ELxdKebmAFitPNCcb0unefNfGY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pFHVHmNTqbLzdDyU0G5g42HwAJOWGIxX26YYqZo09HcljfWJy7/zgswfrGhamOgVW\n\t0JSZtvr+4iCHnZb4VDhzytn8LLyCqSOVf0K+ByuLxXvn3reOCYofSZri5LeyF57Dkr\n\tJx471Nf76jzysRuTZhiYhHfox2HHiMLxGHKsSPT4=","Date":"Thu, 7 May 2026 09:42:21 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 3/3] pipeline: rpi: Make control lists in requests\n\tproperly atomic","Message-ID":"<afxAenFX3ouRG5o2@zed>","References":"<20260428133952.6582-1-david.plowman@raspberrypi.com>\n\t<20260428133952.6582-4-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260428133952.6582-4-david.plowman@raspberrypi.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":38771,"web_url":"https://patchwork.libcamera.org/comment/38771/","msgid":"<CAHW6GYJisBZ6oDLO0ZE0m2AeWcMohNTp5ecp58P98-3mzhVfVA@mail.gmail.com>","date":"2026-05-07T12:44:15","subject":"Re: [PATCH v3 3/3] pipeline: rpi: Make control lists in requests\n\tproperly atomic","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for looking at this again.\n\nOn Thu, 7 May 2026 at 08:42, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:\n>\n> Hi David\n>\n> On Tue, Apr 28, 2026 at 02:26:39PM +0100, David Plowman wrote:\n> > When a request is about to be processed, this commit separates\n> > \"delayed controls\" (camera-related ones that take \"a few frames\" to\n> > apply) from \"immediate controls\" (ISP-related controls) that will\n> > happen instantly.\n> >\n> > The immediate controls are held back until the delayed controls that\n> > they were submitted with, have happened. This means all the controls\n> > submitted in a request happen atomically.\n> >\n> > We therefore already have the sequence number of the request whose\n> > controls have just been applied, so we additionally attach this to the\n> > current request as \"ControlId\" metadata.\n> >\n> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  src/ipa/rpi/common/ipa_base.cpp               | 24 ++++++----\n> >  .../pipeline/rpi/common/pipeline_base.cpp     | 46 +++++++++++++++++++\n> >  .../pipeline/rpi/common/pipeline_base.h       |  8 ++++\n> >  src/libcamera/pipeline/rpi/pisp/pisp.cpp      |  3 ++\n> >  src/libcamera/pipeline/rpi/vc4/vc4.cpp        |  3 ++\n> >  5 files changed, 76 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > index faa77719..dacafa57 100644\n> > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > @@ -431,6 +431,15 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n> >       rpiMetadata.clear();\n> >       fillDeviceStatus(params.sensorControls, ipaContext);\n> >\n> > +     /*\n> > +      * When there are controls, it's important that we don't skip running the\n> > +      * IPAs, as that can mess with synchronisation. Crucially though, we need\n> > +      * to know whether there were controls when this comes back as the\n> > +      * _delayed_ metadata, hence why we flag this in the metadata itself.\n> > +      */\n> > +     if (!params.requestControls.empty())\n> > +             rpiMetadata.set(\"ipa.request_controls\", true);\n> > +\n> >       if (params.buffers.embedded) {\n> >               /*\n> >                * Pipeline handler has supplied us with an embedded data buffer,\n> > @@ -451,7 +460,7 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n> >        */\n> >       AgcStatus agcStatus;\n> >       bool hdrChange = false;\n> > -     RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext];\n> > +     RPiController::Metadata &delayedMetadata = rpiMetadata_[params.delayContext % rpiMetadata_.size()];\n> >       if (!delayedMetadata.get<AgcStatus>(\"agc.status\", agcStatus)) {\n> >               rpiMetadata.set(\"agc.delayed_status\", agcStatus);\n> >               hdrChange = agcStatus.hdr.mode != hdrStatus_.mode;\n> > @@ -464,9 +473,13 @@ void IpaBase::prepareIsp(const PrepareParams &params)\n> >        */\n> >       helper_->prepare(embeddedBuffer, rpiMetadata);\n> >\n> > +     bool delayedRequestControls = false;\n> > +     delayedMetadata.get<bool>(\"ipa.request_controls\", delayedRequestControls);\n> > +\n> >       /* Allow a 10% margin on the comparison below. */\n> >       Duration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;\n> > -     if (lastRunTimestamp_ && frameCount_ > invalidCount_ &&\n> > +     if (!delayedRequestControls && params.requestControls.empty() &&\n> > +         lastRunTimestamp_ && frameCount_ > invalidCount_ &&\n> >           delta < controllerMinFrameDuration_ * 0.9 && !hdrChange) {\n> >               /*\n> >                * Ensure we merge the previous frame's metadata with the current\n> > @@ -535,7 +548,7 @@ void IpaBase::processStats(const ProcessParams &params)\n> >               ControlList ctrls(sensorCtrls_);\n> >               applyAGC(&agcStatus, ctrls);\n> >               rpiMetadata.set(\"agc.status\", agcStatus);\n> > -             setDelayedControls.emit(ctrls, ipaContext);\n> > +             setDelayedControls.emit(ctrls, params.ipaContext);\n> >               setCameraTimeoutValue();\n> >       }\n> >\n> > @@ -951,8 +964,6 @@ void IpaBase::applyControls(const ControlList &controls)\n> >\n> >                       /* The control provides units of microseconds. */\n> >                       agc->setFixedExposureTime(0, ctrl.second.get<int32_t>() * 1.0us);\n> > -\n> > -                     libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>());\n> >                       break;\n> >               }\n> >\n> > @@ -976,9 +987,6 @@ void IpaBase::applyControls(const ControlList &controls)\n> >                               break;\n> >\n> >                       agc->setFixedGain(0, ctrl.second.get<float>());\n> > -\n> > -                     libcameraMetadata_.set(controls::AnalogueGain,\n> > -                                            ctrl.second.get<float>());\n> >                       break;\n> >               }\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index f6ef8a3b..28f44753 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -1528,4 +1528,50 @@ void CameraData::fillRequestMetadata(const ControlList &bufferControls, Request\n> >       }\n> >  }\n> >\n> > +static bool isControlDelayed(unsigned int id)\n> > +{\n> > +     return id == controls::ExposureTime ||\n> > +            id == controls::AnalogueGain ||\n> > +            id == controls::FrameDurationLimits ||\n>\n> Fine with these that match a V4L2 control\n>\n> > +            id == controls::AeEnable ||\n> > +            id == controls::ExposureTimeMode ||\n> > +            id == controls::AnalogueGainMode;\n>\n> But aren't these knobs that control the IPA behaviour rather than\n> controls for the HW (which take a delay to apply) ?\n\nThat's true, but on the Pi, changing exposure or analogue gain values,\neven in manual mode, is always mediated by the AEC/AGC algorithm. So\nif someone sets (for example) ExposureTimeMode to Manual, and sends a\nshutter time, then we have to put the algorithm into the correct mode\nat the same moment, or the update will just get ignored.\n\nI guess you could ask what state the algorithm is really in when it's\ndone something, but which hasn't happened yet. But I'll leave that\nquestion for Schrodinger and his cat!\n\n>\n> > +}\n> > +\n> > +void CameraData::handleControlLists(uint32_t delayContext)\n> > +{\n> > +     /*\n> > +      * The delayContext is the sequence number after it's gone through the various\n> > +      * pipeline delays, so that's what gets reported as the \"ControlListSequence\"\n> > +      * in the metadata, being the sequence number of the request whose ControlList\n> > +      * has just been applied.\n> > +      */\n> > +     Request *request = requestQueue_.front();\n> > +     request->_d()->metadata().set(controls::rpi::ControlListSequence, delayContext);\n> > +\n> > +     /*\n> > +      * Controls that take effect immediately (typically ISP controls) have to be\n> > +      * delayed so as to synchronise with those controls that do get delayed. So we\n> > +      * must remove them from the current request, and push them onto a queue so\n> > +      * that they can be used later.\n> > +      */\n> > +     ControlList controls = std::move(request->controls());\n> > +     request->controls().clear();\n>\n> I'm fine with the control list sequence number being reported through\n> controls::rpi::ControlListSequence.\n>\n> I'm less at ease with the idea we're re-constructing the ControlList\n> inside a Request. Any application that holds a reference to any of\n> those controls will fail if we're messing up the storage where those\n> controls live.\n>\n> I recall you said you don't really care about the controls of a\n> completed request as there's nothing interesting there. Does that mean\n> you would be fine simply reporting ControlListSequence and not\n> reworking the ControlList inside a Request ?\n\nI'll post a v4 to address this in just a moment.\n\nThanks!\n\nDavid\n\n>\n>\n>\n> > +     immediateControls_.push({ request->sequence(), {} });\n> > +     for (const auto &ctrl : controls) {\n> > +             if (isControlDelayed(ctrl.first))\n> > +                     request->controls().set(ctrl.first, ctrl.second);\n> > +             else\n> > +                     immediateControls_.back().controls.set(ctrl.first, ctrl.second);\n> > +     }\n> > +\n> > +     /* \"Immediate\" controls that have become due are now merged back into this request. */\n> > +     while (!immediateControls_.empty() &&\n> > +            immediateControls_.front().controlListId <= delayContext) {\n> > +             request->controls().merge(immediateControls_.front().controls,\n> > +                                       ControlList::MergePolicy::OverwriteExisting);\n> > +             immediateControls_.pop();\n> > +     }\n> > +}\n> > +\n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > index 7bfac33e..d3f044a4 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > @@ -180,10 +180,18 @@ public:\n> >\n> >       ClockRecovery wallClockRecovery_;\n> >\n> > +     struct ImmediateControlsEntry {\n> > +             uint64_t controlListId;\n> > +             ControlList controls;\n> > +     };\n> > +     std::queue<ImmediateControlsEntry> immediateControls_;\n> > +\n> >  protected:\n> >       void fillRequestMetadata(const ControlList &bufferControls,\n> >                                Request *request);\n> >\n> > +     void handleControlLists(uint32_t delayContext);\n> > +\n> >       virtual void tryRunPipeline() = 0;\n> >\n> >  private:\n> > diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> > index c7799f2b..70bbac5a 100644\n> > --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> > +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp\n> > @@ -2322,6 +2322,9 @@ void PiSPCameraData::tryRunPipeline()\n> >\n> >       fillRequestMetadata(job.sensorControls, request);\n> >\n> > +     /* This sorts out synchronisation with ControlLists in earlier requests. */\n> > +     handleControlLists(job.delayContext);\n> > +\n> >       /* Set our state to say the pipeline is active. */\n> >       state_ = State::Busy;\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > index f99cfdbc..a1b44af1 100644\n> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > @@ -939,6 +939,9 @@ void Vc4CameraData::tryRunPipeline()\n> >\n> >       fillRequestMetadata(bayerFrame.controls, request);\n> >\n> > +     /* This sorts out synchronisation with ControlLists in earlier requests. */\n> > +     handleControlLists(bayerFrame.delayContext);\n> > +\n> >       /* Set our state to say the pipeline is active. */\n> >       state_ = State::Busy;\n> >\n> > --\n> > 2.47.3\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7211ABE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  7 May 2026 12:44:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1BF7863026;\n\tThu,  7 May 2026 14:44:30 +0200 (CEST)","from mail-ej1-x633.google.com (mail-ej1-x633.google.com\n\t[IPv6:2a00:1450:4864:20::633])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C09C762FE1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 May 2026 14:44:27 +0200 (CEST)","by mail-ej1-x633.google.com with SMTP id\n\ta640c23a62f3a-b9d9971d059so120593366b.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 07 May 2026 05:44:27 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"aewrVdF5\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1778157867; cv=none;\n\td=google.com; s=arc-20240605;\n\tb=RRVQcQQl5Mqn5KwglDgUHHg+LRFw7o7wtwQhkp1UtpwRyMvHCBdL7P9Jx0dAeQ2s22\n\txZiq75K3u1SObkmRM21gdZf8uq94WPPszzbyPA2m1+OUu4xYU0GyLJ1X554vhEifUEQh\n\tHIBjDS+u+/Xs0z0IIYeF8i2jIIZKTHZBmwdniTFCt8nOdGc2D2f1sPZidp0veLTaJVT3\n\tT4HeNxZngqc5Ymw5yNHB7NjAveT3nVtntuFBFGE0p6l0judbltuE5NZHmfzbgq09fJJc\n\tVsNXBs0ai5GaEqhJBhQAzwsviqE+9TkGVjNxnOY0mBLcF4OACK8rXRqBxs/Kf1Nwr0yJ\n\tDtlQ==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=arc-20240605; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:dkim-signature;\n\tbh=XJbddFArSrVhlc9YQjmuCw9iNR8BDVx+QuZNI2g+3zw=;\n\tfh=Gjw9R6WC/I7P2CTMaaQH6a2n8qWptBCQ2iHEZUVPgB8=;\n\tb=Fkxw8aAG8Dyzhl3DCYkTxoC4hfpR8ZbFcdF3UcgVz6xD9+mG7kIpacsRfN8xwAC9hO\n\tqWyRkx1Zbpt9gC8QOKg4qGdH5PYmSD0Ap+PKh8as29H9S/hxY7K/PTwtWplfikkoojNQ\n\ttMWe5EbEWgAgpwz/bi2w+wOEgK8qTS2IuEqxLxQ9C7KfUf4shyru+CTziboyGWirxNfQ\n\tpjLUKBDE1LUkXq15JQzBsoqLdMH8An3ad/QI/GcuODy/arAlWM/zzaF22mS5iEvrPNJk\n\t6NIDrgJaDbr6MOALeaBCzq2n+ngC6q4AjUiapYdNF9ZzL4mZwdKQr/91aTUB5G/epZt1\n\tWFRg==; darn=lists.libcamera.org","ARC-Authentication-Results":"i=1; mx.google.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1778157867; x=1778762667;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=XJbddFArSrVhlc9YQjmuCw9iNR8BDVx+QuZNI2g+3zw=;\n\tb=aewrVdF5h1nHE2xap17ZdBe8BccfQEaxVTKArIXEalvz3oS/ASHql7DeEhwFtSw0WX\n\tl/PZdPbyQEk5yeSeBAb4OBChsiUHQgLd35BRVtR9Bf96U6MQgiHYKDUV7mcSYBQaKen0\n\tii2KGNgdaozocYiFQKEunhhAmOxYj1lONWNb5E2j1Qvt1c3SNlEXv/tqfprF3HBEbNDd\n\tcAwSFnDWDumppkTtatm+qfIe8QjrC0JL1ZT7mckey9V03A6Z7PRUs6NHL+5b7aHu9t5K\n\tFnQdgk5qGELvIa25eFGYC5OO8ApVebibu+8sH77ykVMMCbWZ3SxrlEDDW/nx1vT1xK05\n\t0nag==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1778157867; x=1778762667;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=XJbddFArSrVhlc9YQjmuCw9iNR8BDVx+QuZNI2g+3zw=;\n\tb=bYCfDnMPTAFkGP+VWwiUZJq5A/GdIS/dyO3yNB5cv7Ayeo9KWk+YuMkVNZNzng0gqN\n\tUUwt43stXhNKsX21Tupgxl8OrSRGMAOWaNxT+jAUPBzaL0ZijBDSAFuSguw+M+U7tqG/\n\txrwXeNBrjfh3A7DnRcGsvL6ppOhMJPrf1T45rhlhKhTHh1DJmDV7MPvWC8VjVYgcY7Nx\n\t3ykllQLRuiMPm5L6ceHcYeBFdPzea5Kx0DdNlQaS8HRGQL+3yp1poFpdAQ80I9YBhpeY\n\tJ+DR4pM+FK1R1YPSLxZ3+ezZsgpj6K+ruVk1N/HGdLe41/8zbiswLh5+ny5t70+7jpab\n\tj8vQ==","X-Gm-Message-State":"AOJu0YxRTkd78W2r6p6Osyat+GI54mAaHCm8NMfQ5ygfLK5dxniLDbNu\n\thmxUSUUPVzIJPCfYwCOzB+ylOYFxyLAI+ZXGIloIL5oC5kIsHGuPyPT9PrWcIftPGnRioNJklY9\n\txy1MMsQozDH2US3/25a+7xpQbhy6mNStYe2B7UEyGetmiOlYCtjGc","X-Gm-Gg":"AeBDiesGZg/Uwmd55mH5qU39Aamp5Pv15xjdwpOGZ4pn1LNAxGN1vAuIf+Bhj13/oYw\n\temVHSgr4wMciynL1Vfp0v1twiE2fbjrPG5tiKbfqILOrck7c2IWwLSQTQXphONGyem35u5D2Lmk\n\tdQ9pisd9AdOY7mSUvNtpU5xGdv7di6AR4padIBjcQM7yUUS7ZJV45EIAMeMXb8OviC+kmJ+wOV2\n\tMa6y0RQs+1aYaAKI99f4perwYof7scxzyQX2iWHKpr3dJa26p9EYg1BPRpZQTJvaCoyxBagPnuP\n\tkxp+gx5vg4ecEb+iTQP6R6sI3SW+aep3v1aDet/+ARi5T04V5MPm2fvwgXJgaGZCFMl4xTJC5Y7\n\tR2r2PkVBiVAlViWRWus+5oGAvX9BBP/hvDqVnKt/Yhp9vTquo1Ia1HY4=","X-Received":"by 2002:a17:907:9721:b0:baa:2d37:cbf9 with SMTP id\n\ta640c23a62f3a-bc56ae270b4mr437929466b.1.1778157866792;\n\tThu, 07 May 2026 05:44:26 -0700 (PDT)","MIME-Version":"1.0","References":"<20260428133952.6582-1-david.plowman@raspberrypi.com>\n\t<20260428133952.6582-4-david.plowman@raspberrypi.com>\n\t<afxAenFX3ouRG5o2@zed>","In-Reply-To":"<afxAenFX3ouRG5o2@zed>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Thu, 7 May 2026 13:44:15 +0100","X-Gm-Features":"AVHnY4KvXxTnKsB3UFbkHsjI8O2Ipo6fZq2M9eCiv65mTn3oJyqtXReduGhdEa0","Message-ID":"<CAHW6GYJisBZ6oDLO0ZE0m2AeWcMohNTp5ecp58P98-3mzhVfVA@mail.gmail.com>","Subject":"Re: [PATCH v3 3/3] pipeline: rpi: Make control lists in requests\n\tproperly atomic","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]