{"id":26342,"url":"https://patchwork.libcamera.org/api/1.1/covers/26342/?format=json","web_url":"https://patchwork.libcamera.org/cover/26342/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20260325151416.2114564-1-stefan.klug@ideasonboard.com>","date":"2026-03-25T15:13:32","name":"[v2,00/32] rkisp1: pipeline rework for PFC","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/1.1/people/184/?format=json","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"mbox":"https://patchwork.libcamera.org/cover/26342/mbox/","series":[{"id":5849,"url":"https://patchwork.libcamera.org/api/1.1/series/5849/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=5849","date":"2026-03-25T15:13:32","name":"rkisp1: pipeline rework for PFC","version":2,"mbox":"https://patchwork.libcamera.org/series/5849/mbox/"}],"comments":"https://patchwork.libcamera.org/api/covers/26342/comments/","headers":{"Return-Path":"","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 857D2BE086\n\tfor ;\n\tWed, 25 Mar 2026 15:14:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 96E2962839;\n\tWed, 25 Mar 2026 16:14:43 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 903566274D\n\tfor ;\n\tWed, 25 Mar 2026 16:14:42 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 7EE81121A; \n\tWed, 25 Mar 2026 16:13:24 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tqtkRWES\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1774451604;\n\tbh=giZh+NT5B4UXFnAE1C1RLMA/rL2JI5nEGiz6ts4SAig=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=tqtkRWES7T/HwmOEcnEWpT+wS4n8y+mMkDRpsqKFwWm8z/xOCtUzaN8yeBkm/Y6/B\n\tFIHUqzyhPq5d4vvhB2GofArZnYK7g9i9Cd8hfvwgdIb15YdzVuh1ssFiMUKTZM8lo3\n\tJblarDeklOVOz5MUK9Tdxv3JIRCbHfDnOxxN1n8s=","From":"Stefan Klug ","To":"libcamera-devel@lists.libcamera.org","Cc":"Stefan Klug ","Subject":"[PATCH v2 00/32] rkisp1: pipeline rework for PFC","Date":"Wed, 25 Mar 2026 16:13:32 +0100","Message-ID":"<20260325151416.2114564-1-stefan.klug@ideasonboard.com>","X-Mailer":"git-send-email 2.51.0","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Hi all,\n\nThis is v2 of the pipeline rework. v1 is available here:\nhttps://patchwork.libcamera.org/project/libcamera/list/?series=5524\n\nAfter the feedback on the last series it got clear that the name is not\nreally well chosen as PFC is not the content of the series but rather\nthe preparatory work needed to later implement PFC on rkisp1/imx8mp.\n\nI'll start with the changes to v1 first, the slightly adapted cover\nletter from v1 follows.\n\nThe biggest topics in v1 review were the additional accumulation of\nlogic in the IPA and the quite complicated bookkeeping in the pipeline\nhandler.\n\nThe first Was improved by implementing a callback mechanism in FcQueue\nin patch 22. On the long run I envision moving even more logic into a\nbase class and possibly dropping the FcQueue altogether. A preview of\nthat is in patch 32.\n\nThe latter was improved by adding a BufferQueue class in patch 24. I\nleft the big rework in patch 25 as complicated as it was. The\nBufferQueue is applied in the following patches, to be able to see the\nbenefit and have a exit strategy if the BufferQueue idea doesn't find\nenough support.\n\nAside from that, a lot of smaller fixes from review were applied and a\nfew patches from the end of the series were dropped as they were not\nabsolutely necessary for this (but are still in my pipeline).\n\n\nNow to the big picture (mostly the old cover-letter):\n\nFrom a very high level view, this series implements the mechanics needed\nto cope with unstable hardware and to later implement per frame control\non the rkisp1. It does not yet touch the possible PFC mechanics like\nfast-tracked controls. It was only tested on the imx8mp together with\nthe dewarper so there might still be issues on other rkisp1 platforms.\n\nTo do correct per-frame-control we need to synchronize multiple things:\n- The sensor needs to be fed with parameters according to the specific\n delays\n- The ISP parameters need to be in sync also. This is especially\n important if for example digital gain in the ISP is used to mitigate\nquantization in the sensor gain and therefore need to perfectly match\nthe sensor timing.\n\nThe current IPA model has basically two loops:\n\nThe sensor controls loop:\n- Pipeline::statsBufferReady()\n- IPA::processStats()\n- IPA::setSensorControls.emit()\n - Pipeline::DelayedControls.push()\n- IPA::metadataReady.emit()\n - Pipeline::tryCompleteRequest()\n\nThe ISP params loop:\n- Pipeline::queueRequestDevice()\n- IPA::queueRequest()\n- IPA::computeParams()\n- Pipeline::paramsComputed()\n - Pipeline::queueParams()\n\nThis means the sensor controls and params are only indirectly coupled.\nTo add to the complexity, in the sensor controls loop, the frame context\nfor the frame that produced the stats is used as basis for the call to\nsetSensorControls(). That is the point where I often fail to follow the\nchain of events.\n\nThe reworked model now changes the mechanics: In multiple discussions it\ngot clear that even though theoretically the ISP params have a 0-1 frame\ndelay and are therefore 1 frame faster than typical sensor delays it is\nmore important to ensure that sensor data and ISP params are in sync\nthan to optimiza for that one frame. We can still have a look at that\nlater. Stating that sensor and params should be in sync also means that\nwe can produce them at the same time. So the pipeline was reworked with\nthe following premises in mind:\n\n- Pipeline and IPA now handle sensor sequence numbers instead\n of request numbers.\n\nThe stats loop only updates active state and fills metadata:\n- ISP calculates stats\n- IPA::processStats()\n- IPA::metadataReady.emit()\n- Pipeline::tryCompleteRequest()\n\nThe sensor loop triggers calculation of params + sensor controls:\n- Pipeline::startFrame()\n- Pipeline::DelayedControls.applyControls(n+delay)\n- IPA::computeParams(n+delay+1)\n - IPA::setSensorControls.emit()\n - Pipeline::DelayedControls.push()\n- Pipeline::paramsComputed()\n - Pipeline::queueParams()\n\nThe request loop:\n- Pipeline::queueRequestDevice()\n- IPA::queueRequest()\n\nThe main change is that sensor controls and ISP params are computed in\nresponse to computeParams(). This has the nice effect, that we just need\nto ensure that we call computeParams() early enough to send it out to\nthe sensor as that is usually the longest delay.\n\nA request underrun is easy to handle as the params/stats machinery just\ncontinues to run with scratch buffers in the kernel. The only change to\ndo there is to repurpose the IPA::queueRequest to a\nIPA::initializeFrameContext and call that on demand when params for a\ngiven frame are needed.\n\nAdditionally to this conceptual change, the stats, image and params\nbuffers were decoupled. Due to the asyncronous nature of V4L2 and the\nusage of scratch buffers in the kernel it is impossible to ensure that a\ngrouping of (stats, image and params) stays in sync. Especially under\nhigh load or with bad connectors, this breaks. Decoupling them makes a\nfew things easier but also brings a bit of complexity. This rework and\nmost of the loop structure change is unfortunately in one big commit\n(patch 25) as I didn't see a way to split that into easy to digest\nparts.\n\nFor the (re)synchronization to work properly, I needed to add\nsynchronization helpers.\n\nIn the current code we have a minimal resync\nfacility that adds 1 if it detects a sequence that is bigger than the\nexpected one. The problem here is the queue in the kernel. If a offset\nis detected when dequing a buffer, we can compensate for that offset\nwhen queuing new buffers. But in case there are n buffers queued when we observe the error, we will\nmost likely observe that error another n times until the compensation\napplies. By then we have overcompensated by n and started a beautiful\noscillation. To handle that we need to track the observed error and the\napplied compensation over time.\n\nAnother reason for this series was to get started faster. Therefor\ncontrols passed to Camera::start() are now handled properly. In that\ncontext a initial params buffer is also filled. This ensures that the\nstats on frame 0 are calculated based on the correct settings and\nprevents large oscillations at the beginning of the stream.\n\nOpen Todos:\n- The semantics of delayed controls was modified to not delay, but more\n to a record and query. Maybe we should rename it?\n- The changed delayed controls breaks the unit tests and possibly\n other pipelines\n- Missing documentation on SequenceSyncHelper\n- As always, a bit more cleanup on the patches and commit messages...\n\nBest regards,\nStefan\n\n---\n\nChanges in v2:\n- Reworked lazy initialization of the FrameContext\n- Added BufferQueue to simplify queue handling\n- Smaller fixes (see local changelogs)\n\nJacopo Mondi (1):\n libcamera: v4l2_videodevice: Do not hide frame drops\n\nStefan Klug (31):\n libcamera: delayed_controls: Add push() function that accepts a\n sequence number\n libcamera: delayed_controls: Handle missed pushes\n libcamera: delayed_controls: Increase log level for dummy pushes\n libcamera: delayed_controls: Queue noop when needed, not before\n libcamera: delayed_controls: Add maxDelay() function\n pipeline: rkisp1: Include frame number when pushing to delayed\n controls\n libipa: fc_queue: Rename template argument to FC\n libipa: fc_queue: Add trailing underscore to private members of\n FrameContext\n ipa: rkisp1: Refactor setControls()\n pipeline: rkisp1: Add a frameStart function to handle\n DelayedControls::applyControls\n ipa: rkisp1: Move setSensorControls signal to computeParams\n pipeline: rkisp1: Fix controls in raw mode\n ipa: rkisp1: Add initializeFrameContext() function\n pipeline: rkisp1: Apply initial controls\n ipa: rkisp1: Set frameContext.agc in queueRequest for auto mode also\n ipa: rkisp1: agc: Process frame duration at the right time\n libcamera: delayed_controls: Change semantics of sequence numbers\n libipa: algorithm: Update docs\n libcamera: delayed_controls: Ignore double pushes for the same frame\n number\n ipa: rkisp1: Allow processStats() to be called without stats buffer\n ipa: rkisp1: Lazy initialise frame context\n libcamera: internal: Add SequenceSyncHelper class\n libcamera: internal: Add a BufferQueue class to handle buffer queues\n pipeline: rkisp1: Decouple image, stats and param buffers\n pipline: rkisp1: Reinstantiate maxQueuedRequestsDevice limit\n pipeline: rkisp1: Correctly handle params buffer for frame 0\n pipeline: rkisp1: Fix buffer metadata when using the dewarper\n pipeline: rkisp1: Pass bufferId to paramsComputed()\n pipeline: rkisp1: rkisp1_path: Modify interface to be compatible with\n BufferQueue\n pipeline: rkisp1: Use BufferQueue for buffer handling\n DNI: Move all queue/algo logic into FcLogic class\n\n include/libcamera/internal/buffer_queue.h | 131 +++\n include/libcamera/internal/delayed_controls.h | 4 +-\n include/libcamera/internal/meson.build | 2 +\n .../libcamera/internal/sequence_sync_helper.h | 76 ++\n include/libcamera/internal/v4l2_videodevice.h | 1 -\n include/libcamera/ipa/rkisp1.mojom | 12 +-\n src/ipa/ipu3/ipu3.cpp | 18 +-\n src/ipa/libipa/algorithm.cpp | 41 +-\n src/ipa/libipa/algorithm.h | 5 +\n src/ipa/libipa/fc_logic.cpp | 20 +\n src/ipa/libipa/fc_logic.h | 151 +++\n src/ipa/libipa/fc_queue.cpp | 8 +-\n src/ipa/libipa/fc_queue.h | 126 ++-\n src/ipa/libipa/meson.build | 2 +\n src/ipa/mali-c55/mali-c55.cpp | 20 +-\n src/ipa/rkisp1/algorithms/agc.cpp | 31 +-\n src/ipa/rkisp1/algorithms/agc.h | 3 +-\n src/ipa/rkisp1/algorithms/algorithm.h | 5 +\n src/ipa/rkisp1/algorithms/awb.cpp | 2 +\n src/ipa/rkisp1/ipa_context.h | 5 +-\n src/ipa/rkisp1/rkisp1.cpp | 106 ++-\n src/ipa/simple/soft_simple.cpp | 18 +-\n src/libcamera/buffer_queue.cpp | 449 +++++++++\n src/libcamera/delayed_controls.cpp | 85 +-\n src/libcamera/meson.build | 2 +\n src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-\n src/libcamera/pipeline/mali-c55/mali-c55.cpp | 6 +-\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 870 +++++++++++-------\n src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 42 +-\n src/libcamera/pipeline/rkisp1/rkisp1_path.h | 23 +-\n src/libcamera/sequence_sync_helper.cpp | 21 +\n src/libcamera/v4l2_videodevice.cpp | 15 -\n test/meson.build | 2 +-\n 33 files changed, 1727 insertions(+), 577 deletions(-)\n create mode 100644 include/libcamera/internal/buffer_queue.h\n create mode 100644 include/libcamera/internal/sequence_sync_helper.h\n create mode 100644 src/ipa/libipa/fc_logic.cpp\n create mode 100644 src/ipa/libipa/fc_logic.h\n create mode 100644 src/libcamera/buffer_queue.cpp\n create mode 100644 src/libcamera/sequence_sync_helper.cpp"}