From patchwork Tue Apr 28 13:26:39 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 26587 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 1F432C32F7 for ; Tue, 28 Apr 2026 13:40:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CE6AE62FE2; Tue, 28 Apr 2026 15:40:03 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="WYxaUeRp"; dkim-atps=neutral Received: from mail-wm1-x335.google.com (mail-wm1-x335.google.com [IPv6:2a00:1450:4864:20::335]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F4FF62FD2 for ; Tue, 28 Apr 2026 15:39:59 +0200 (CEST) Received: by mail-wm1-x335.google.com with SMTP id 5b1f17b1804b1-4891c00e7aeso87679225e9.2 for ; Tue, 28 Apr 2026 06:39:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1777383599; x=1777988399; 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=aX68pVIfwS/iqGU8xne8xIfWg+LTMMw5YrnovQVx2yY=; b=WYxaUeRpP3QnFdHivheoEPFknhNeocSGoPS+H/BG0Em71D9v1GDzd1vk6t2F2OdrAu JGEnKZpHIHquO/LZcJ9yhGzPW99NKlUAvuxXrw1ubyxWjS+Lm7lG3+m4ktR4kNiZzlte 7kAnY8nnX6SoYtD8F5qD4AE1bTfe3mBbDtQW7aW3pglyHWo6MjwFPGHy/YBO8epqlGkp ph4LAHSf6Vjg/HTvKTlX5PtwgAJTAxtIjjN6AgdAo2/r3b8hv8FpIXQThNSh583XheXU fDaX9anyFzrTKtdCxbtIPZYmkn8U3mH+4OBMMxsJ7LRiqP0K8M28rW6E+9D6Dr9mUEJ+ bT/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777383599; x=1777988399; 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=aX68pVIfwS/iqGU8xne8xIfWg+LTMMw5YrnovQVx2yY=; b=rqnHvhlq7d6bIhbkBht6Xxdg96YbiYvmJER0Ds1ApaIEv/Gr7sna7wGeNOjHmP/9DQ bL9zqtjkJyLWiAVCSyPe4SLRnRNdlDv1YzGizjqhd+CoeBQooYUe+hUw9SeyS6V4h6l6 KRkBawVzqOBUPDY1AuoSw7ieFtyJLBWqI6CRid8T/Saevqqj1gGO9wE5RtBU7MGV0pWG jdHmtk3itFFoPksEoZIHJ+Ais/EEjqa/pIz6DR3gLorpNi0fEYVzN/MIb6sw4bHanqhS AAbPuJhteMnZC30rH/i9JRCyE7W8DkjXR+ePFkRX1PdtoAgFhFK5Poi56ikbIf+lZUJl sUyA== X-Gm-Message-State: AOJu0YzEP50Nt7S6eEQ4KAcTznEI00k1QKi5dgJUOEfuy/jnQvui6GGs Ym0X5EVa7MFGJz4G9TB+ypWd6pRCz4jpaBtw+xl8O/PST1lEqwXxyKAaIMNmIASni7TysnwPBBH bOihw X-Gm-Gg: AeBDieul1LLvF63S2ODhh5PfbanT9/4eXKNpGbXvz1LGgoMOVKrcjo73X4HNb9BbeE/ xvqEUKMPtmvWI+ujjxvaziIsrilWV+ktv5ca7YX6nxJzKBbgOK9IJnHDVTWOPwKLmj2mOcLIhVV BpEbUgDxoobRDER8K1vVQ2vYAwfbf3dzu85ebeKqkrIvGMQgv7rtAQh8cOuLq8l2wVMWIsfALma f6EX0oDRh6x0ui1qdUQMPfyKfzf6aQuekcUU+7o8x4QFImZ1mSsXqsG2UNut85Vod6wxHVkrY0R WxeOuKcpRSHKlH/WiFLNXaASLP+66CxMSh/97nxSUl+cRUFG65LyypWW3pb19lPoO4YylOdGlNR HDXUqqSGVRcFJ19JMl92Dm6FOeA9m2WB7yFqNQyJNiY9No75qgwbaaMQaF+3hJew7e1JbngsdAC I8IxqQA7K+YvSk9s3P35JFZv8UTIb0oERMb4bTACMeDneaYSwr8v7WUHy/mCmOqVp/r/NvVAbOn QwfqXeW3FFr2jfSv94LkRmbMEUBjCwCThHUMzn3fdnlKmIbGCuE X-Received: by 2002:a05:600c:3b0f:b0:487:20ee:bef6 with SMTP id 5b1f17b1804b1-48a77afec2amr51180015e9.11.1777383598474; Tue, 28 Apr 2026 06:39:58 -0700 (PDT) Received: from davidp-pi5.pitowers.org ([2a00:1098:3142:1f:88ea:c658:5b20:5e46]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48a77c24ca0sm51374525e9.14.2026.04.28.06.39.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Apr 2026 06:39:58 -0700 (PDT) From: David Plowman To: libcamera-devel@lists.libcamera.org Cc: David Plowman Subject: [PATCH v3 3/3] pipeline: rpi: Make control lists in requests properly atomic Date: Tue, 28 Apr 2026 14:26:39 +0100 Message-ID: <20260428133952.6582-4-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260428133952.6582-1-david.plowman@raspberrypi.com> References: <20260428133952.6582-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 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 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;