Cover Letter Detail
Show a cover letter.
GET /api/covers/24766/?format=api
{ "id": 24766, "url": "https://patchwork.libcamera.org/api/covers/24766/?format=api", "web_url": "https://patchwork.libcamera.org/cover/24766/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20251024085130.995967-1-stefan.klug@ideasonboard.com>", "date": "2025-10-24T08:50:24", "name": "[v1,00/35] rkisp1: pipeline rework for PFC", "submitter": { "id": 184, "url": "https://patchwork.libcamera.org/api/people/184/?format=api", "name": "Stefan Klug", "email": "stefan.klug@ideasonboard.com" }, "mbox": "https://patchwork.libcamera.org/cover/24766/mbox/", "series": [ { "id": 5524, "url": "https://patchwork.libcamera.org/api/series/5524/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5524", "date": "2025-10-24T08:50:24", "name": "rkisp1: pipeline rework for PFC", "version": 1, "mbox": "https://patchwork.libcamera.org/series/5524/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/covers/24766/comments/", "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 CF1CFC3259\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Oct 2025 08:51:37 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 12F8060871;\n\tFri, 24 Oct 2025 10:51:37 +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 6CF926085B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Oct 2025 10:51:36 +0200 (CEST)", "from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7edc:62f4:c118:1549])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id C7C63177F; \n\tFri, 24 Oct 2025 10:49:50 +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=\"Gxlg8SWt\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761295790;\n\tbh=9bSsNmJXT4vbRQRGjjVfLpFBxxuY5AOYlOv5s1MRZ8o=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=Gxlg8SWtEzAeobWAiuZSv4e32EIk5d9mPwSF7O8qpQ9t6i86ZCB2kv41h8oHbOkTW\n\tNynXSg56kxNBn9xnNA0jdkB5Jfley2syz+zj2Xxm2pmgjEP+oXQs6BjT/HaHlG18zv\n\tsx08MaNnk/RwoT425i3X2oFB4EsLFAhH0fcIPqCo=", "From": "Stefan Klug <stefan.klug@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Stefan Klug <stefan.klug@ideasonboard.com>", "Subject": "[PATCH v1 00/35] rkisp1: pipeline rework for PFC", "Date": "Fri, 24 Oct 2025 10:50:24 +0200", "Message-ID": "<20251024085130.995967-1-stefan.klug@ideasonboard.com>", "X-Mailer": "git-send-email 2.48.1", "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": "<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>" }, "content": "Hi all,\n\nThis series accumulated for a while but I think it is time to get it\ninto the public.\n\nFrom a very highlevel view, this series implements per frame control on\nthe 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\nThe series depends on [PATCH v2 00/35] Full dewarper support on imx8mp\nand won't apply otherwise. A complete branch is available here:\nhttps://git.uk.ideasonboard.com/sklug/libcamera/src/branch/sklug/imx8mp-dewarp-and-regulation-v1\n\nThere are a lot of smaller cleanup patches in there, but to get the\noverall picture I'll try to summarize my thoughts behind that:\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 internally 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 27/35) 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. In 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 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 most likely\n other pipelines\n- Lot's of debug logs are generated internally. That might be a bit of\n an overhead but is super useful to debug the pipeline. So I'd somehow\nlike to keep them in, but be able to compile time disable them.\n- As always, a bit more cleanup on the patches and commit messages...\n\nI'm excited to hear your opinions.\n\nBest regards,\nStefan\n\nJacopo Mondi (1):\n libcamera: v4l2_videodevice: Do not hide frame drops\n\nStefan Klug (34):\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 ipa: rkisp1: agc: Fix vblank, when computeParams prepare is not called\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 ipa: rkisp1: Ensure controls don't get lost\n pipeline: rkisp1: Add SequenceSyncHelper class\n ipa: rkisp1: awb: Ignore empty AWB statistics\n pipeline: rkisp1: Decouple image, stats and param buffers\n pipline: rkisp1: Reinstantiate maxQueuedRequestsDevice limit\n ipa: libipa: Reduce log level when obtaining an uninitialized frame\n context\n pipeline: rkisp1: Correctly handle params buffer for frame 0\n WIP ipa: rkisp1: Post quantization gain as digital gain in metadata\n WIP: rkisp1: agc: Add digital gain\n libipa: agc_mean_luminance: Make startup frames and regulations speed\n configurable\n ipa: rkisp1: Increase regulation speed\n [WIP] Add imx335 delays\n\n include/libcamera/internal/delayed_controls.h | 3 +\n include/libcamera/internal/v4l2_videodevice.h | 1 -\n include/libcamera/ipa/rkisp1.mojom | 10 +-\n src/ipa/libipa/agc_mean_luminance.cpp | 25 +-\n src/ipa/libipa/agc_mean_luminance.h | 4 +\n src/ipa/libipa/algorithm.cpp | 28 +-\n src/ipa/libipa/fc_queue.cpp | 8 +-\n src/ipa/libipa/fc_queue.h | 75 +-\n src/ipa/rkisp1/algorithms/agc.cpp | 56 +-\n src/ipa/rkisp1/algorithms/agc.h | 3 +-\n src/ipa/rkisp1/algorithms/awb.cpp | 7 +\n src/ipa/rkisp1/ipa_context.h | 4 +\n src/ipa/rkisp1/rkisp1.cpp | 105 ++-\n src/libcamera/delayed_controls.cpp | 83 +-\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 788 ++++++++++++------\n .../pipeline/rkisp1/sequence_sync_helper.h | 69 ++\n .../sensor/camera_sensor_properties.cpp | 7 +-\n src/libcamera/v4l2_videodevice.cpp | 15 -\n test/meson.build | 2 +-\n 19 files changed, 903 insertions(+), 390 deletions(-)\n create mode 100644 src/libcamera/pipeline/rkisp1/sequence_sync_helper.h" }