From patchwork Tue Mar 24 14:32:09 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 26326 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id E83C7C32F6 for ; Tue, 24 Mar 2026 15:17:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 795466278D; Tue, 24 Mar 2026 16:17:24 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="WJ3j3b12"; dkim-atps=neutral Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8141762784 for ; Tue, 24 Mar 2026 16:17:20 +0100 (CET) Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-48374014a77so54137695e9.3 for ; Tue, 24 Mar 2026 08:17:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1774365439; x=1774970239; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=Felfuv/vx6wuAkWZ2K+rXbJMlM17aSgPLpgoMJipYjY=; b=WJ3j3b12kYbxenCgaf4BcYpuE2CKJTeGf7DyOD7C8lo32g0ULX2ZEepdlbPD6ruLRs YM8owPNlVAEmR9nAoZCKdYMoO+PWZMSWcDlPUqwvmoHd1+rRcAKS8TYRNBTcFw36uu3I Pn1ZHRybDKMvLg7c2e0PCww8Y8krSjnSFxnkQ+lUbWxF7sGV1nos0uUkvIVdv9ejNaV4 k1NTMi3i8AhBVOOnSaSaZvwLrBngqPzwIyOawEOgQp+9ecCuKAfunRwmjuOSjflQ7iWz s+DKVGwzzhBL6/sCJp8RoeVmyIbyQi4FlkbK39q3OD5/U85HBkb0MKfOSJWVJPxxufAT Pceg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774365439; x=1774970239; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=Felfuv/vx6wuAkWZ2K+rXbJMlM17aSgPLpgoMJipYjY=; b=rhHVLtngdF9x0isGrtd0K8FOiqWwgHtjYneVOjRahymFQpND+UMBPCv8PkP0ncy8K9 1gjrvuC3sfUTNMKUkSfs/Wf6PNggT4I3pJNWtYhP/lbOBsTq8XCnHGwlT9oli3fThZzg teyhEdj1G0VwWB6h3bc13YyQZaHDdME147+DIq7aD9pboZJWAh40S48EWNWnYP3XFPlG /F5GcU+QEAXoM8QbxGPAH9IkPgElhcfv62O1VzMOPOzWmF+kwkukFhheEdD/PRUrS514 7Ab5zpcypK8lacjsrGV8tRnEapageVCvb3Tl5xSPHqfDIIDBtHqye6S0MEc9Xy5/yBXC e4qA== X-Gm-Message-State: AOJu0Yxk7hpAok1yplAUU0TuhNOB3VwIsFk1k1IbRvKa2rlHcRiRUylw ubPW5O+HErvq9zJnxzjyjFmyPwpTl9WkVc6nNiUNF8xuBssW5L2WqTN9n++Usetgf0kHf44Mgsy uj1aD X-Gm-Gg: ATEYQzzyuybYypMrFBRamHF/2/ZlYfqrGiKM2cL72G/6JIMeZh4955JLdOF9D3eTTd6 M4JiOW0AAw96EX+6s1zEzyQwy7seA2P44FQ6QVgymhnVpY1CB9+HgoHuYGBIZDQye9MwIEUPLpD 5yLWhpUfB5G/tu8YGlBAz4L35l4rhRYzr/SdWG6wVMrynaxmjR+1bJHqPJfb56NTtYbC6Snud+d fplVkwzaJB/ZtB62tTV5pGl2ppRU0O+k/RY2552vRi6HTDKjk0osVhOj6REZbba52UD0K5Pvji+ QLngkhyV/HqwMW7n7/VIrp0ECP5Ve6OPpFtwc7UrCbozpktTHge15yEh2iQF1etM2drlOHYgx/5 035JKBXz5Q2R5wuwilfqiqbGOYHUztWok8VRP3UouUvzpqbqTpkkX1cJ25uhFjXIs8qzCwF3TK8 /LDxpBi5P9BhqpECMgVqh5CS886l8TCm6+3EUlipWICWuYFSKI10sVRKZ1rMPyf+XSYlIiw3KFj NO2basp6JntY1sJAu/cSzGvBd4GBJgRNI9tPtXWaA== X-Received: by 2002:a05:600c:3ba1:b0:485:3ff1:d5c3 with SMTP id 5b1f17b1804b1-48715fc322bmr2570915e9.5.1774365439283; Tue, 24 Mar 2026 08:17:19 -0700 (PDT) Received: from davidp-pi5.pitowers.org ([2a00:1098:3142:1f:88ea:c658:5b20:5e46]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-487116c44bfsm63826065e9.9.2026.03.24.08.17.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Mar 2026 08:17:18 -0700 (PDT) From: David Plowman To: libcamera-devel@lists.libcamera.org Cc: David Plowman Subject: [PATCH v1 3/3] pipeline: rpi: Make control lists in requests properly atomic Date: Tue, 24 Mar 2026 14:32:09 +0000 Message-ID: <20260324151714.3345-4-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260324151714.3345-1-david.plowman@raspberrypi.com> References: <20260324151714.3345-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- 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 ¶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 867ecf1b..9e162870 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 "ControlId" 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::ControlId, 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 597eb587..65d8efdc 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 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 dff73a79..cc8aa4d4 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 b734889d..f743c8a7 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;