From patchwork Mon Sep 14 14:02:13 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 28244 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 F28A7C3200 for ; Mon, 14 Sep 2026 14:03:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 27B3968670; Mon, 14 Sep 2026 16:03:29 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="lUip1IeY"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 88E1C68670 for ; Mon, 14 Sep 2026 16:03:27 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:a279:75fa:1f6c:7f40]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5819D512; Mon, 14 Sep 2026 16:01:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1789394507; bh=W2AQ2PwkKhG/pwdZQ4wMR054lFSZYSur423O2fxr6Bg=; h=From:To:Cc:Subject:Date:From; b=lUip1IeYaQVEjkdLLlEs/cRHwc034fpz9nu8z7YEdv3pPREmh5e1VasxVVuq2XR3D YBGLrlghFURoQrb5na+lQWuhGsqaqnopAugwT4CUeFmXWM9EXVotquDT7v5lTZYs4f dr4br235RRUzimIDjfVTJ0zMn36ykrDiVegKTGY0= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 00/41] rkisp1: pipeline rework for PFC Date: Mon, 14 Sep 2026 16:02:13 +0200 Message-ID: <20260914140309.3354666-1-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.53.0 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" Hi all, This is v3 of the pipeline rework. v2 is available here: https://patchwork.libcamera.org/project/libcamera/list/?series=5849 After the feedback on v1 it got clear that the name is not really well chosen as PFC is not the content of the series but rather the preparatory work needed to later implement PFC on rkisp1/imx8mp. I'll start with the changes to v2 first, the slightly adapted cover letter from v2 follows. In v3 a lot of cleanup was added and the series now passes CI. As this code is also in daily use in several places, quite some small bugfixes and fixups accumulated. The biggest changes (see local logs for details) in v3 are as follows: - The FCQueue no longer exits with a fatal log on a queue overrun, and these corner cases are handled gracefully. - Rebased to current master which includes the ipa rework. Due to that, the influence of this series on other platforms became larger and needs testing. - I dropped the WIP patch that replaced the FCQueue by logic in the IPA base classes as it should be added on top and this series is already big enough. The two step approch to first split buffer queues and in a followup patch use the BufferQueue class is still kept as I didn't get much feedback on the BufferQueue and it shows how the code evolved. I still feel like that concept makes things a lot easier. Now to the big picture (mostly the v1 cover-letter): From a very high level view, this series implements the mechanics needed to cope with unstable hardware and to to be able to implement per frame control on the rkisp1. It does not yet touch the possible PFC mechanics like fast-tracked controls. It was only tested on the imx8mp together with the dewarper so there might still be issues on other rkisp1 platforms. To do correct per-frame-control we need to synchronize multiple things: - The sensor needs to be fed with parameters according to the specific delays - The ISP parameters need to be in sync also. This is especially important if for example digital gain in the ISP is used to mitigate quantization in the sensor gain and therefore need to perfectly match the sensor timing. The current IPA model has basically two loops: The sensor controls loop: - Pipeline::statsBufferReady() - IPA::processStats() - IPA::setSensorControls.emit() - Pipeline::DelayedControls.push() - IPA::metadataReady.emit() - Pipeline::tryCompleteRequest() The ISP params loop: - Pipeline::queueRequestDevice() - IPA::queueRequest() - IPA::computeParams() - Pipeline::paramsComputed() - Pipeline::queueParams() This means the sensor controls and params are only indirectly coupled. To add to the complexity, in the sensor controls loop, the frame context for the frame that produced the stats is used as basis for the call to setSensorControls(). That is the point where I often fail to follow the chain of events. The reworked model now changes the mechanics: In multiple discussions it got clear that even though theoretically the ISP params have a 0-1 frame delay and are therefore 1 frame faster than typical sensor delays it is more important to ensure that sensor data and ISP params are in sync than to optimiza for that one frame. We can still have a look at that later. Stating that sensor and params should be in sync also means that we can produce them at the same time. So the pipeline was reworked with the following premises in mind: - Pipeline and IPA now handle sensor sequence numbers instead of request numbers. The stats loop only updates active state and fills metadata: - ISP calculates stats - IPA::processStats() - IPA::metadataReady.emit() - Pipeline::tryCompleteRequest() The sensor loop triggers calculation of params + sensor controls: - Pipeline::startFrame() - Pipeline::DelayedControls.applyControls(n+delay) - IPA::computeParams(n+delay+1) - IPA::setSensorControls.emit() - Pipeline::DelayedControls.push() - Pipeline::paramsComputed() - Pipeline::queueParams() The request loop: - Pipeline::queueRequestDevice() - IPA::queueRequest() The main change is that sensor controls and ISP params are computed in response to computeParams(). This has the nice effect, that we just need to ensure that we call computeParams() early enough to send it out to the sensor as that is usually the longest delay. A request underrun is easy to handle as the params/stats machinery just continues to run with scratch buffers in the kernel. The only change to do there is to repurpose the IPA::queueRequest to a IPA::initializeFrameContext and call that on demand when params for a given frame are needed. Additionally to this conceptual change, the stats, image and params buffers were decoupled. Due to the asyncronous nature of V4L2 and the usage of scratch buffers in the kernel it is impossible to ensure that a grouping of (stats, image and params) stays in sync. Especially under high load or with bad connectors, this breaks. Decoupling them makes a few things easier but also brings a bit of complexity. This rework and most of the loop structure change is unfortunately in one big commit (patch 25) as I didn't see a way to split that into easy to digest parts. For the (re)synchronization to work properly, I needed to add synchronization helpers. In the current code we have a minimal resync facility that adds 1 if it detects a sequence that is bigger than the expected one. The problem here is the queue in the kernel. If a offset is detected when dequing a buffer, we can compensate for that offset when queuing new buffers. But in case there are n buffers queued when we observe the error, we will most likely observe that error another n times until the compensation applies. By then we have overcompensated by n and started a beautiful oscillation. To handle that we need to track the observed error and the applied compensation over time. Another reason for this series was to get started faster. Therefor controls passed to Camera::start() are now handled properly. In that context a initial params buffer is also filled. This ensures that the stats on frame 0 are calculated based on the correct settings and prevents large oscillations at the beginning of the stream. Open Todos: - The semantics of delayed controls was modified to not delay, but more to a record and query. Maybe we should rename it? - Testing on more platforms (ipu3/simple/mali) Best regards, Stefan Jacopo Mondi (1): libcamera: v4l2_videodevice: Do not hide frame drops Stefan Klug (40): libcamera: delayed_controls: Add push() function that accepts a sequence number libcamera: delayed_controls: Handle missed pushes libcamera: delayed_controls: Increase log level for dummy pushes libcamera: delayed_controls: Queue noop when needed, not before libcamera: delayed_controls: Add maxDelay() function pipeline: rkisp1: Add a frameStart function to handle DelayedControls::applyControls pipeline: rkisp1: Include frame number when pushing to delayed controls libcamera: delayed_controls: Change semantics of sequence numbers libcamera: delayed_controls: Ignore double pushes for the same frame number libipa: fc_queue: Rename template argument to FC libipa: fc_queue: Add trailing underscore to private members of FrameContext ipa: rkisp1: Refactor setControls() ipa: rkisp1: Move setSensorControls signal to computeParams ipa: rkisp1: Add initializeFrameContext() function pipeline: rkisp1: Apply initial controls libipa: agc: Pass agc::Session to prepare() libipa: agc: Process frame duration at the right time ipa: rkisp1: Allow processStats() to be called without stats buffer libipa: awb: Populate metadata when stats are invalid ipa: rksip1: Call libipa::Awb::process() when stats are null libipa: agc: Populate frameContext in queueRequest() in auto mode libipa: awb: Populate frameContext in queueRequest() in auto mode pipeline: rkisp1: Pass bufferId to metadataReady() ipa: rkisp1: Lazy initialise frame context libcamera: internal: Add SequenceSyncHelper class libcamera: internal: Add a BufferQueue class to handle buffer queues pipeline: rkisp1: Decouple image, stats and param buffers pipeline: rkisp1: Reinstantiate maxQueuedRequestsDevice limit pipeline: rkisp1: Correctly handle params buffer for frame 0 pipeline: rkisp1: Fix buffer metadata when using the dewarper pipeline: rkisp1: rkisp1_path: Modify interface to be compatible with BufferQueue pipeline: rkisp1: Use BufferQueue for buffer handling libipa: algorithm: Update documentation libipa: fc_queue: Early out on first context libipa: fc_queue: Return nullptr instead of crashing libipa: algorithm: Add a initialize parameter to prepare ipa: rkisp1: Gracefully handle FCQueue overruns ipa: rkisp1: Handle frame jumps in calls to computeParams() libipa: agc: Make startup frames and regulations speed configurable ipa: rkisp1: Increase regulation speed include/libcamera/internal/buffer_queue.h | 136 +++ include/libcamera/internal/delayed_controls.h | 3 + include/libcamera/internal/meson.build | 2 + .../libcamera/internal/sequence_sync_helper.h | 31 + include/libcamera/internal/v4l2_videodevice.h | 1 - include/libcamera/ipa/rkisp1.mojom | 14 +- src/ipa/ipu3/algorithms/af.cpp | 3 +- src/ipa/ipu3/algorithms/af.h | 3 +- src/ipa/ipu3/algorithms/agc.cpp | 5 +- src/ipa/ipu3/algorithms/agc.h | 3 +- src/ipa/ipu3/algorithms/awb.cpp | 3 +- src/ipa/ipu3/algorithms/awb.h | 3 +- src/ipa/ipu3/algorithms/blc.cpp | 4 +- src/ipa/ipu3/algorithms/blc.h | 3 +- src/ipa/ipu3/algorithms/tone_mapping.cpp | 4 +- src/ipa/ipu3/algorithms/tone_mapping.h | 3 +- src/ipa/ipu3/ipu3.cpp | 31 +- src/ipa/libipa/agc.cpp | 53 +- src/ipa/libipa/agc.h | 8 +- src/ipa/libipa/agc_mean_luminance.cpp | 25 +- src/ipa/libipa/agc_mean_luminance.h | 3 + src/ipa/libipa/algorithm.cpp | 52 +- src/ipa/libipa/algorithm.h | 3 +- src/ipa/libipa/awb.cpp | 14 +- src/ipa/libipa/fc_queue.cpp | 47 +- src/ipa/libipa/fc_queue.h | 143 ++- src/ipa/mali-c55/algorithms/agc.cpp | 9 +- src/ipa/mali-c55/algorithms/agc.h | 3 +- src/ipa/mali-c55/algorithms/awb.cpp | 7 +- src/ipa/mali-c55/algorithms/awb.h | 3 +- src/ipa/mali-c55/algorithms/blc.cpp | 6 +- src/ipa/mali-c55/algorithms/blc.h | 3 +- src/ipa/mali-c55/algorithms/ccm.cpp | 5 +- src/ipa/mali-c55/algorithms/ccm.h | 3 +- src/ipa/mali-c55/algorithms/lsc.cpp | 4 +- src/ipa/mali-c55/algorithms/lsc.h | 3 +- src/ipa/mali-c55/mali-c55.cpp | 33 +- src/ipa/rkisp1/algorithms/agc.cpp | 21 +- src/ipa/rkisp1/algorithms/agc.h | 3 +- src/ipa/rkisp1/algorithms/awb.cpp | 12 +- src/ipa/rkisp1/algorithms/awb.h | 3 +- src/ipa/rkisp1/algorithms/blc.cpp | 7 +- src/ipa/rkisp1/algorithms/blc.h | 3 +- src/ipa/rkisp1/algorithms/ccm.cpp | 3 +- src/ipa/rkisp1/algorithms/ccm.h | 3 +- src/ipa/rkisp1/algorithms/compress.cpp | 3 +- src/ipa/rkisp1/algorithms/compress.h | 3 +- src/ipa/rkisp1/algorithms/cproc.cpp | 5 +- src/ipa/rkisp1/algorithms/cproc.h | 3 +- src/ipa/rkisp1/algorithms/dpcc.cpp | 7 +- src/ipa/rkisp1/algorithms/dpcc.h | 3 +- src/ipa/rkisp1/algorithms/dpf.cpp | 7 +- src/ipa/rkisp1/algorithms/dpf.h | 3 +- src/ipa/rkisp1/algorithms/filter.cpp | 5 +- src/ipa/rkisp1/algorithms/filter.h | 3 +- src/ipa/rkisp1/algorithms/goc.cpp | 5 +- src/ipa/rkisp1/algorithms/goc.h | 3 +- src/ipa/rkisp1/algorithms/gsl.cpp | 7 +- src/ipa/rkisp1/algorithms/gsl.h | 3 +- src/ipa/rkisp1/algorithms/lsc.cpp | 5 +- src/ipa/rkisp1/algorithms/lsc.h | 3 +- src/ipa/rkisp1/algorithms/lux.cpp | 3 +- src/ipa/rkisp1/algorithms/lux.h | 3 +- src/ipa/rkisp1/algorithms/wdr.cpp | 3 +- src/ipa/rkisp1/algorithms/wdr.h | 3 +- src/ipa/rkisp1/rkisp1.cpp | 151 ++- src/ipa/softisp/algorithms/adjust.cpp | 3 +- src/ipa/softisp/algorithms/adjust.h | 3 +- src/ipa/softisp/algorithms/agc.cpp | 6 +- src/ipa/softisp/algorithms/agc.h | 3 +- src/ipa/softisp/algorithms/awb.cpp | 3 +- src/ipa/softisp/algorithms/awb.h | 3 +- src/ipa/softisp/algorithms/blc.cpp | 3 +- src/ipa/softisp/algorithms/blc.h | 3 +- src/ipa/softisp/algorithms/ccm.cpp | 4 +- src/ipa/softisp/algorithms/ccm.h | 3 +- src/ipa/softisp/softisp.cpp | 30 +- src/libcamera/buffer_queue.cpp | 541 +++++++++++ src/libcamera/delayed_controls.cpp | 89 +- src/libcamera/meson.build | 2 + src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 6 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 891 +++++++++++------- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 42 +- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 23 +- src/libcamera/pipeline/simple/simple.cpp | 15 +- src/libcamera/sequence_sync_helper.cpp | 113 +++ src/libcamera/v4l2_videodevice.cpp | 15 - test/delayed_controls.cpp | 57 +- 89 files changed, 2081 insertions(+), 744 deletions(-) create mode 100644 include/libcamera/internal/buffer_queue.h create mode 100644 include/libcamera/internal/sequence_sync_helper.h create mode 100644 src/libcamera/buffer_queue.cpp create mode 100644 src/libcamera/sequence_sync_helper.cpp