[{"id":28938,"web_url":"https://patchwork.libcamera.org/comment/28938/","msgid":"<171033070503.252503.7634953262791642046@ping.linuxembedded.co.uk>","date":"2024-03-13T11:51:45","subject":"Re: [PATCH 00/12] Preparation for per-frame-controls and initial\n\ttests","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Stefan,\n\nI tried to send this for testing @ gitlab, but the patches didn't apply\nto master.\n\nWhat is your current base?\n\n./send-for-testing.sh 4220\nFrom https://git.libcamera.org/libcamera/libcamera\n 2e2ba223f3a2..d54abd32affd master -> libcamera.org/master\nFrom gitlab.freedesktop.org:camera/libcamera\n 2e2ba223f3a2..d54abd32affd master -> gl.fdo/master\nHEAD is now at 2e2ba223f3a2 libcamera: framebuffer_allocator: Remove entry if allocation fails\nPrevious HEAD position was 2e2ba223f3a2 libcamera: framebuffer_allocator: Remove entry if allocation fails\nBranch 'testing/4220' set up to track remote branch 'master' from 'libcamera.org'.\nSwitched to a new branch 'testing/4220'\nApplying: libcamera: delayed_controls: add ctrls list to reset function\nerror: sha1 information is lacking or useless (include/libcamera/internal/delayed_controls.h).\nerror: could not build fake ancestor\nhint: Use 'git am --show-current-patch=diff' to see the failed patch\nPatch failed at 0001 libcamera: delayed_controls: add ctrls list to reset function\nWhen you have resolved this problem, run \"git am --continue\".\nIf you prefer to skip this patch, run \"git am --skip\" instead.\nTo restore the original branch and stop patching, run \"git am --abort\".\n\n--\nKieran\n\n\n\nQuoting Stefan Klug (2024-03-13 10:56:32)\n> Hi everyone,\n> \n> unknowingly I stumbled into a topic that is way larger than I thought in the \n> beginning. Delayed-controls and a closely related topic per-frame-controls.\n> \n> I (now) know that a lot of work already went into this, noteably by \n> Jacopo Mondi, David Plowman and Naushir Patuck:\n> https://patchwork.libcamera.org/cover/16458/\n> https://github.com/raspberrypi/libcamera/tree/pfc\n> Thanks for that!\n> \n> I started off without per-frame-controls in mind (only delayed-controls) \n> and soon realized, that these two are closely related. (It's difficult to\n> test delayed-controls in a real pipeline without working per-frame-controls)\n> \n> To be able to tackle the whole topic in the long run I'd like to split it into parts:\n> \n> 1. Get a initial set of tests on mainline\n> I'd like to have them as a discussion basis and a reference to test against. \n> These shouldn't be treated as mandatory for now (ideas on how to express that in \n> code are very welcome)\n> - Initial lc-compliance tests for delayed-controls (and because these are system tests\n> also for per-frame-controls)\n> - A reworked delayed controls class that hopefully makes things easier\n> - Tested on rkisp1 (details see below)\n> \n> 2. Get an agreement on \"RaspberryPi\" mode\n> In the previous work by David and Naushir, the (in my understanding) most prominent\n> requiremnet was the ability to set a control ASAP even when multiple \n> requests are queued already (which makes complete sense IMHO). This requirement \n> shall not get lost and I think there is a way to make it work.\n> \n> 3. Get the tests to pass on rkisp1, rpi and ipu3\n> I guess this will take a while and involve different parties, so let's not wait\n> until everything works everywhere...\n> \n> 4. Make the tests mandatory (details tbd)\n> \n> This series contains part 1.\n> \n> One change of thought is, that we add the ability to push requests to delayed\n> controls for a given sequence number and to re-push for the \n> same sequence number. This has the benefit, that we don't have to decide \n> early on if we operate in strict per-frame-controls mode (sometimes called \n> \"android mode\") or in \"raspberry pi\" mode.\n> \n> Stefan Klug (12):\n> libcamera: lc-compliance: Add controls param to start() function\n> libcamera: lc-compliance: Add TimeSheet class\n> libcamera: lc-compliance: Add initial set of per-frame-control tests\n> libcamera: delayed_controls: Update unit tests to expected semantics\n> libcamera: delayed_controls: Rename class members\n> libcamera: delayed_controls: Rework delayed controls implementation\n> libcamera: delayed_controls: Add some logging\n> libcamera: delayed_controls: Add ctrls list parameter to reset()\n> function\n> pipeline: rkisp1: Move call to setSensorControls.emit()\n> pipeline: rkisp1: Add more debug logging\n> pipeline: rkisp1: Fix per-frame-controls in manual mode\n> pipeline: rkisp1: Apply initial controls\n> \n> include/libcamera/internal/delayed_controls.h | 20 +-\n> include/libcamera/ipa/rkisp1.mojom | 7 +-\n> src/apps/lc-compliance/capture_test.cpp | 46 +++\n> src/apps/lc-compliance/meson.build | 2 +\n> src/apps/lc-compliance/per_frame_controls.cpp | 316 ++++++++++++++++++\n> src/apps/lc-compliance/per_frame_controls.h | 43 +++\n> src/apps/lc-compliance/simple_capture.cpp | 4 +-\n> src/apps/lc-compliance/simple_capture.h | 2 +-\n> src/apps/lc-compliance/time_sheet.cpp | 145 ++++++++\n> src/apps/lc-compliance/time_sheet.h | 55 +++\n> src/ipa/rkisp1/algorithms/agc.cpp | 4 +-\n> src/ipa/rkisp1/rkisp1.cpp | 49 ++-\n> src/libcamera/delayed_controls.cpp | 235 ++++++++++---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 11 +-\n> test/delayed_controls.cpp | 203 ++++++++---\n> 15 files changed, 1021 insertions(+), 121 deletions(-)\n> create mode 100644 src/apps/lc-compliance/per_frame_controls.cpp\n> create mode 100644 src/apps/lc-compliance/per_frame_controls.h\n> create mode 100644 src/apps/lc-compliance/time_sheet.cpp\n> create mode 100644 src/apps/lc-compliance/time_sheet.h\n> \n> -- \n> 2.40.1\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 2F737BD1F1\n\tfor ;\n\tWed, 13 Mar 2024 11:51:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 27E0362C80;\n\tWed, 13 Mar 2024 12:51:50 +0100 (CET)","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 6EEC062C80\n\tfor ;\n\tWed, 13 Mar 2024 12:51:48 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 91342720;\n\tWed, 13 Mar 2024 12:51:25 +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=\"bWZnm9o8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1710330685;\n\tbh=I6od3xo+7w4H/czD+/bZkZll+zjsapYVmJllb4Y/Sac=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=bWZnm9o8s0ZD4iPrtPi9PgHEUELIw3ynRZrF2PMwNcK+3L/HCp3snrRzrZyC3cCOc\n\t4yWeR3BmbY6qE/RlmL2HmQAhGGvm1IRHL3WBFNzOgV1H0anll2f9E2pU3VjRc8YHpu\n\t/TxB0icxuKUstbEwyUscHdM/UddxhjPTZ9RQCCxg=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240313105645.120317-1-stefan.klug@ideasonboard.com>","References":"<20240313105645.120317-1-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH 00/12] Preparation for per-frame-controls and initial\n\ttests","From":"Kieran Bingham ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 13 Mar 2024 11:51:45 +0000","Message-ID":"<171033070503.252503.7634953262791642046@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}}]