[{"id":36428,"web_url":"https://patchwork.libcamera.org/comment/36428/","msgid":"","date":"2025-10-24T11:00:30","subject":"Re: [PATCH v1 00/35] rkisp1: pipeline rework for PFC","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Stefan\n\nYes indeed, always excited when folks want to talk about PFC!\n\nOn Fri, 24 Oct 2025 at 09:51, Stefan Klug wrote:\n>\n> Hi all,\n>\n> This series accumulated for a while but I think it is time to get it\n> into the public.\n>\n> From a very highlevel view, this series implements per frame control on\n> the rkisp1. It does not yet touch the possible PFC mechanics like\n> fast-tracked controls. It was only tested on the imx8mp together with\n> the dewarper so there might still be issues on other rkisp1 platforms.\n\nIn many ways there are probably too many implementation specifics\nabout non-Raspberry Pi code for me really to appreciate what's going\non, nonetheless some degree of high level understanding might still be\nhelpful.\n\nAlso, by \"fast-tracked controls\" I assume you're referring to\nBarnabas's proposal? As I remarked at the time, I liked that so long\nas it involves a queue, and I don't really have too many concerns\nabout their interaction.\n\n>\n> The series depends on [PATCH v2 00/35] Full dewarper support on imx8mp\n> and won't apply otherwise. A complete branch is available here:\n> https://git.uk.ideasonboard.com/sklug/libcamera/src/branch/sklug/imx8mp-dewarp-and-regulation-v1\n>\n> There are a lot of smaller cleanup patches in there, but to get the\n> overall picture I'll try to summarize my thoughts behind that:\n>\n> To do correct per-frame-control we need to synchronize multiple things:\n\nI guess this is the obvious question - what do we mean by \"correct\nper-frame-control\"? Can you perhaps give a brief description of what\nthat means here, and perhaps how it compares with Raspberry Pi's\nversion (as shown in Nice)?\n\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\n> quantization in the sensor gain and therefore need to perfectly match\n> the sensor timing.\n>\n> The current IPA model has basically two loops:\n>\n> The 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>\n> The ISP params loop:\n> - Pipeline::queueRequestDevice()\n> - IPA::queueRequest()\n> - IPA::computeParams()\n> - Pipeline::paramsComputed()\n> - Pipeline::queueParams()\n>\n> This means the sensor controls and params are only indirectly coupled.\n> To add to the complexity, in the sensor controls loop, the frame context\n> for the frame that produced the stats is used as basis for the call to\n> setSensorControls(). That is the point where I often fail to follow the\n> chain of events.\n>\n> The reworked model now changes the mechanics: In multiple discussions it\n> got clear that even though theoretically the ISP params have a 0-1 frame\n> delay and are therefore 1 frame faster than typical sensor delays it is\n> more important to ensure that sensor data and ISP params are in sync\n> than to optimiza for that one frame. We can still have a look at that\n> later. Stating that sensor and params should be in sync also means that\n> we can produce them at the same time. So the pipeline was reworked with\n> the following premises in mind:\n\nI certainly agree with keeping sensor and ISP parameters in sync. Not\ndoing so would make them very difficult for applications to use, and I\nwouldn't want to inflict that on them!\n\n>\n> - Pipeline and IPA now internally handle sensor sequence numbers instead\n> of request numbers.\n>\n> The stats loop only updates active state and fills metadata:\n> - ISP calculates stats\n> - IPA::processStats()\n> - IPA::metadataReady.emit()\n> - Pipeline::tryCompleteRequest()\n>\n> The 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>\n> The request loop:\n> - Pipeline::queueRequestDevice()\n> - IPA::queueRequest()\n>\n> The main change is that sensor controls and ISP params are computed in\n> response to computeParams(). This has the nice effect, that we just need\n> to ensure that we call computeParams() early enough to send it out to\n> the sensor as that is usually the longest delay.\n>\n> A request underrun is easy to handle as the params/stats machinery just\n> continues to run with scratch buffers in the kernel. The only change to\n> do there is to repurpose the IPA::queueRequest to a\n> IPA::initializeFrameContext and call that on demand when params for a\n> given frame are needed.\n>\n> Additionally to this conceptual change, the stats, image and params\n> buffers were decoupled. Due to the asyncronous nature of V4L2 and the\n> usage of scratch buffers in the kernel it is impossible to ensure that a\n> grouping of (stats, image and params) stays in sync. Especially under\n> high load or with bad connectors, this breaks. Decoupling them makes a\n> few things easier but also brings a bit of complexity. This rework and\n> most 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\n> parts.\n>\n> For the (re)synchronization to work properly, I needed to add\n> synchronization helpers. In the current code we have a minimal resync\n> facility that adds 1 if it detects a sequence that is bigger than the\n> expected one. The problem here is the queue in the kernel. If a offset\n> is detected when dequing a buffer, we can compensate for that offset\n> when queuing new buffers. But in case there are n buffers queued we will\n> most likely observe that error another n times until the compensation\n> applies. By then we have overcompensated by n and started a beautiful\n> oscillation. To handle that we need to track the observed error and the\n> applied compensation over time.\n>\n> Another reason for this series was to get started faster. Therefor\n> controls passed to Camera::start() are now handled properly. In that\n> context a initial params buffer is also filled. This ensures that the\n> stats on frame 0 are calculated based on the correct settings and\n> prevents large oscillations at the beginning of the stream.\n>\n> Open 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\n> like 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>\n> I'm excited to hear your opinions.\n\nMe too!\n\nBest regards\nDavid\n\n>\n> Best regards,\n> Stefan\n>\n> Jacopo Mondi (1):\n> libcamera: v4l2_videodevice: Do not hide frame drops\n>\n> Stefan 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\n>\n> --\n> 2.48.1\n>\n>","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 85DF7BE080\n\tfor ;\n\tFri, 24 Oct 2025 11:00:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 419A4608BD;\n\tFri, 24 Oct 2025 13:00:46 +0200 (CEST)","from mail-qk1-x72a.google.com (mail-qk1-x72a.google.com\n\t[IPv6:2607:f8b0:4864:20::72a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6A121608DC\n\tfor ;\n\tFri, 24 Oct 2025 13:00:43 +0200 (CEST)","by mail-qk1-x72a.google.com with SMTP id\n\taf79cd13be357-88f239686f2so177054785a.0\n\tfor ;\n\tFri, 24 Oct 2025 04:00:43 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"cmuQixKm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1761303642; x=1761908442;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=p2HroeLbPP4NFqyy/dDKOtBqsxpiO/mKnZ6BRfh0ZAc=;\n\tb=cmuQixKmyXIlSDWb/ApB/8Obl5FNFOXmUFYeOyLh1VqmtZ8rAfKr/YHBx1N9Cy4uub\n\tJRlPEP+Bn1+PFXjgnkC6xzE/2Kp7cSGSt+DsiwWQ+OMoKS+vEOr9yYmvUeYNmg0191mK\n\tn9BtsDbdw+qPFUZO5qjEwh2ga2PUFRvIFdZgkwaQQ2t/DcHGLU2ju3y5KIDtDbf/jst9\n\tvLLGugZvhsximoRC6MIgxsLT6vE/oeT5pKAidaMTJEzGjjOos6BGeobIqqZiAVajBhwk\n\tfov+1B2mbgNW2OE8yqni1Y3WZ5IKw4tnCITtf8K1ziE0SNOZtkcKomf7cfolfRPto2dp\n\tyfnA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1761303642; x=1761908442;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=p2HroeLbPP4NFqyy/dDKOtBqsxpiO/mKnZ6BRfh0ZAc=;\n\tb=eEfuLxiMZPn8UWaYdcuz3GwNgUd0JZrp6ulDX5yT+bhSxwLRNZg31tBKlkugA6Z2q5\n\tr3c0C2DGjzyxsz6JnlsJaPKi1U4Ejez0XX3zQ307pD1ugv+HPZbxRJzHmZzpayyi3DJi\n\twmVKAf0B16E6Y6frFTOJKKl/2xz5Q9/fp+ZXPKFG2zTcyx2WMkbai13RDdJgtYK6ia3j\n\tBaBcCA4XX0Kl2sYn63s5Ma/FP8C92oDWUVtzfPp+1sVGfg3ucsZ/O9wfvliJF/ZbTuEg\n\tpscxGlgcMrVubUMnJURlsYGVA9yp0Q+LseDtIfBxP1p9kflS1HqGfDoBT11fQNOvurtm\n\tQEIQ==","X-Gm-Message-State":"AOJu0YzUqDIbkDKDK8cIfybvJfo8Hy0zCExUnZZQTd35j+unjQuCOEyp\n\tdhquWkZhtavoHY+xjVYM63h6mySmfsC+xv6izMaBAWPVLJ/3/k5aWajNxk/Z+3KKr6/U7BQ9nJk\n\tigIPC5B6sP06QbUE4WG1rpZfA5P7p282lu2Vl9yZemnTU82BAnHqoqXY=","X-Gm-Gg":"ASbGncsRhwKH0Q5Omje/jT7LJ79flxChH4NNt8+edAn+qqSqNA8BXxq5CkTMUFifPid\n\t8wCwREuGM45FJhgyER6FsYzfC1Y5/TzjQjkKfOHVSvkziCTuwIMEYwEqqhf7924Fzy1q4p5Fk3H\n\tRyWmJYR8Yy8/rs0fOj+AEqQ/p+v+yrW+zUsB8xKRWEe6bDu2LlR087tcy5NHyWQmV7Bi9upEqRg\n\tpwWlLmjWjT8yTi4h2pSogXz/2WK6nBvm3Qm0+1OhG3mfJt2wTFaV4IdjWMM5/x/LE4T9h7hcR+7\n\twjWTFTA1tND+mYbqNrSfVD1qwU3UofyCFkf69AkS0f4nsAOp/j1vAPe/vRxaeh074PciN6csUPi\n\tW","X-Google-Smtp-Source":"AGHT+IG+H9NL6bluH9LjFUTj06M6gyHIFpVs4us2bkyUQvnvLBE2ZQwodFQdlk2QSikYs1v++ovCQSFNmwOXWzl7hyw=","X-Received":"by 2002:a05:620a:258b:b0:81d:26e6:ef8c with SMTP id\n\taf79cd13be357-89070dd7439mr3439064985a.79.1761303641743;\n\tFri, 24 Oct 2025 04:00:41 -0700 (PDT)","MIME-Version":"1.0","References":"<20251024085130.995967-1-stefan.klug@ideasonboard.com>","In-Reply-To":"<20251024085130.995967-1-stefan.klug@ideasonboard.com>","From":"David Plowman ","Date":"Fri, 24 Oct 2025 12:00:30 +0100","X-Gm-Features":"AS18NWDWZboLNdpTvRRUQYKLyru2RBmJ5FrE5GUnrWI5OW_XJZ-8PvtCUCHfF3k","Message-ID":"","Subject":"Re: [PATCH v1 00/35] rkisp1: pipeline rework for PFC","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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\" "}},{"id":36434,"web_url":"https://patchwork.libcamera.org/comment/36434/","msgid":"<176130681660.997414.9930952176426921077@localhost>","date":"2025-10-24T11:53:36","subject":"Re: [PATCH v1 00/35] rkisp1: pipeline rework for PFC","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi David,\n\nThanks for your message.\n\nQuoting David Plowman (2025-10-24 13:00:30)\n> Hi Stefan\n> \n> Yes indeed, always excited when folks want to talk about PFC!\n> \n> On Fri, 24 Oct 2025 at 09:51, Stefan Klug wrote:\n> >\n> > Hi all,\n> >\n> > This series accumulated for a while but I think it is time to get it\n> > into the public.\n> >\n> > From a very highlevel view, this series implements per frame control on\n> > the rkisp1. It does not yet touch the possible PFC mechanics like\n> > fast-tracked controls. It was only tested on the imx8mp together with\n> > the dewarper so there might still be issues on other rkisp1 platforms.\n> \n> In many ways there are probably too many implementation specifics\n> about non-Raspberry Pi code for me really to appreciate what's going\n> on, nonetheless some degree of high level understanding might still be\n> helpful.\n> \n> Also, by \"fast-tracked controls\" I assume you're referring to\n> Barnabas's proposal? As I remarked at the time, I liked that so long\n> as it involves a queue, and I don't really have too many concerns\n> about their interaction.\n> \n> >\n> > The series depends on [PATCH v2 00/35] Full dewarper support on imx8mp\n> > and won't apply otherwise. A complete branch is available here:\n> > https://git.uk.ideasonboard.com/sklug/libcamera/src/branch/sklug/imx8mp-dewarp-and-regulation-v1\n> >\n> > There are a lot of smaller cleanup patches in there, but to get the\n> > overall picture I'll try to summarize my thoughts behind that:\n> >\n> > To do correct per-frame-control we need to synchronize multiple things:\n> \n> I guess this is the obvious question - what do we mean by \"correct\n> per-frame-control\"? Can you perhaps give a brief description of what\n> that means here, and perhaps how it compares with Raspberry Pi's\n> version (as shown in Nice)?\n\nMaybe the word PFC is already a bit burned. The internal working title\nwas merely \"regulation rework\". My main motivation was to have a\ndeterministic machinery that (assuming everything is in flow and there\nare enough requests queued in) finishes a request with exactly the\nsettings that were requested. This was not the case on rkisp1 and made\nany discussion about what we could do (like for example the RPi model\nwith queuing less buffers but still having deterministic control) a\nnightmare to even think about. This series only modifies the inner\nworkings. On top of that we can implement things like the above\nmentioned proposal or sequenceId that you proposed. But I didn't have a\nbasis where I could guarantee that the ISP params really are in sync\nwith the sensor params and stats which lead to oscillations all over the\nplace and especially at start.\n\nSo with this series hopefully merged sometime, I expect there to be\nmental space to look at the open points in the PFC space. One big piece\nof that puzzle is in my opinion the long wanted separation of buffers\nfrom requests. But that is for another day.\n\nSo tldr, I don't want to push a new concept for PFC that goes against\nthe RPi version. My plan is to build the ground so we can actually work\non PFC. So PFC in this series is more \"The pipeline handler knows for\nevery frame what's in there :-)\"\n\nI hope that answers your question.\n\nBest regards,\nStefan\n\n> \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\n> > quantization in the sensor gain and therefore need to perfectly match\n> > the sensor timing.\n> >\n> > The current IPA model has basically two loops:\n> >\n> > The 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> >\n> > The ISP params loop:\n> > - Pipeline::queueRequestDevice()\n> > - IPA::queueRequest()\n> > - IPA::computeParams()\n> > - Pipeline::paramsComputed()\n> > - Pipeline::queueParams()\n> >\n> > This means the sensor controls and params are only indirectly coupled.\n> > To add to the complexity, in the sensor controls loop, the frame context\n> > for the frame that produced the stats is used as basis for the call to\n> > setSensorControls(). That is the point where I often fail to follow the\n> > chain of events.\n> >\n> > The reworked model now changes the mechanics: In multiple discussions it\n> > got clear that even though theoretically the ISP params have a 0-1 frame\n> > delay and are therefore 1 frame faster than typical sensor delays it is\n> > more important to ensure that sensor data and ISP params are in sync\n> > than to optimiza for that one frame. We can still have a look at that\n> > later. Stating that sensor and params should be in sync also means that\n> > we can produce them at the same time. So the pipeline was reworked with\n> > the following premises in mind:\n> \n> I certainly agree with keeping sensor and ISP parameters in sync. Not\n> doing so would make them very difficult for applications to use, and I\n> wouldn't want to inflict that on them!\n> \n> >\n> > - Pipeline and IPA now internally handle sensor sequence numbers instead\n> > of request numbers.\n> >\n> > The stats loop only updates active state and fills metadata:\n> > - ISP calculates stats\n> > - IPA::processStats()\n> > - IPA::metadataReady.emit()\n> > - Pipeline::tryCompleteRequest()\n> >\n> > The 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> >\n> > The request loop:\n> > - Pipeline::queueRequestDevice()\n> > - IPA::queueRequest()\n> >\n> > The main change is that sensor controls and ISP params are computed in\n> > response to computeParams(). This has the nice effect, that we just need\n> > to ensure that we call computeParams() early enough to send it out to\n> > the sensor as that is usually the longest delay.\n> >\n> > A request underrun is easy to handle as the params/stats machinery just\n> > continues to run with scratch buffers in the kernel. The only change to\n> > do there is to repurpose the IPA::queueRequest to a\n> > IPA::initializeFrameContext and call that on demand when params for a\n> > given frame are needed.\n> >\n> > Additionally to this conceptual change, the stats, image and params\n> > buffers were decoupled. Due to the asyncronous nature of V4L2 and the\n> > usage of scratch buffers in the kernel it is impossible to ensure that a\n> > grouping of (stats, image and params) stays in sync. Especially under\n> > high load or with bad connectors, this breaks. Decoupling them makes a\n> > few things easier but also brings a bit of complexity. This rework and\n> > most 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\n> > parts.\n> >\n> > For the (re)synchronization to work properly, I needed to add\n> > synchronization helpers. In the current code we have a minimal resync\n> > facility that adds 1 if it detects a sequence that is bigger than the\n> > expected one. The problem here is the queue in the kernel. If a offset\n> > is detected when dequing a buffer, we can compensate for that offset\n> > when queuing new buffers. But in case there are n buffers queued we will\n> > most likely observe that error another n times until the compensation\n> > applies. By then we have overcompensated by n and started a beautiful\n> > oscillation. To handle that we need to track the observed error and the\n> > applied compensation over time.\n> >\n> > Another reason for this series was to get started faster. Therefor\n> > controls passed to Camera::start() are now handled properly. In that\n> > context a initial params buffer is also filled. This ensures that the\n> > stats on frame 0 are calculated based on the correct settings and\n> > prevents large oscillations at the beginning of the stream.\n> >\n> > Open 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\n> > like 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> >\n> > I'm excited to hear your opinions.\n> \n> Me too!\n> \n> Best regards\n> David\n> \n> >\n> > Best regards,\n> > Stefan\n> >\n> > Jacopo Mondi (1):\n> > libcamera: v4l2_videodevice: Do not hide frame drops\n> >\n> > Stefan 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\n> >\n> > --\n> > 2.48.1\n> >\n> >","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 57CF0BE080\n\tfor ;\n\tFri, 24 Oct 2025 11:53:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1960260966;\n\tFri, 24 Oct 2025 13:53:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E8E28608EF\n\tfor ;\n\tFri, 24 Oct 2025 13:53:38 +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 5EBB5591;\n\tFri, 24 Oct 2025 13:51:53 +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=\"MCCZf4Yt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761306713;\n\tbh=NCDl2gaB5xdn5ELtfgHih1yWxDp3XSzyJzLY3X+THac=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=MCCZf4Yt1W395p8QNoguOjHyer6wPHQKOmOfU1dv4ZqOvZgu5ZIYzLmUNjK+pBFtC\n\t4N8BZGXhWs1uVvndOMjnA4a0NwWERXzmvoZe1crSkONBltT0yQLqy9lK4e59Z8Y82o\n\tqTqKb8SDVAD+J3e+bfJ3zkRBwVq7MeMWKQSevetk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20251024085130.995967-1-stefan.klug@ideasonboard.com>\n\t","Subject":"Re: [PATCH v1 00/35] rkisp1: pipeline rework for PFC","From":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","To":"David Plowman ","Date":"Fri, 24 Oct 2025 13:53:36 +0200","Message-ID":"<176130681660.997414.9930952176426921077@localhost>","User-Agent":"alot/0.12.dev8+g2c003385c862.d20250602","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\" "}}]