From patchwork Tue Sep 27 02:36:09 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17411 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 22771C0DA4 for ; Tue, 27 Sep 2022 02:37:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3AFA662262; Tue, 27 Sep 2022 04:37:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1664246222; bh=fjqaNsWAWStNtmIo7Jn1JdOW3/gkF+Lrv512r6lSPZ8=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=K5DQwsBPlCdw7sXsBNxbhOR+8LXicNoNWh87YPPnS/QAf7Ep/goN9jiSf3YykAePD dj9+ulrmCXmPsHIUcjeOhNdG4p3IcPtYeMAxzPDFevU67FRAckJCw/iCQ5aZVUuFvV VT1XufKedbA1ZTYMVG6gjjQXz9kM1Aux5z+WVEPtXh2sPfzg5SamIaCqnZJ+fzmjlL f5lVhbNS0+vGW1C/wRQlNNT7HQTmcahU550rdFmzEY7cIOu6BWEwyxybh/Bry3hSMg 0KPlIHRRewxqOmGAPsZt/ih7WZtf3PZ85reMARMplFtxNL7p2TZ8VjTBIJUymwgIuI WkJhgJU5lzeCA== 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 2E22061F79 for ; Tue, 27 Sep 2022 04:37:00 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="k7XAvaec"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7648747C for ; Tue, 27 Sep 2022 04:36:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1664246219; bh=fjqaNsWAWStNtmIo7Jn1JdOW3/gkF+Lrv512r6lSPZ8=; h=From:To:Subject:Date:From; b=k7XAvaecO1LV2rr9mSAUgrKvsZbieG6XUy+S7Md5SovyaBTG1eLw+niMYRgFRJ8Vv 4c7WvUplCViVh3zZ90cKDRPD4jDmp0JZhreG6lr1kKu8iJwVutijgqiwV+YRgw50L8 oqUFwFlaL+m69bqRQ4Krr2XLeyCz0hwhXOJyZvz4= To: libcamera-devel@lists.libcamera.org Date: Tue, 27 Sep 2022 05:36:09 +0300 Message-Id: <20220927023642.12341-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v5 00/33] ipa: Frame context queue, IPU3 & RkISP consolidation, and RkISP1 improvements 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: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Hello, Here's the v5 of the "frame context queue" series. It looks very much like v4, with review comments taken into account, and a few additional changes: - The rename of IPAFrameContext to IPU3FrameContext and RkISP1FrameContext got dropped. I'm not convinced it's a good idea, and while I could change my mind, I think we should then rename all the other structures accordingly in one go. - Patches 17/33 and 18/33 have been added. They integrate review comments from v4, and I've kept them separate to ease review and discussions. Patches 01/33 and 02/33 are small preliminary drive-by cleanups. The gist of the frame context queue introduction is in patches 03/33 to 08/33 that also update the IPA algorithm operations to pass the new objects around. There shouldn't be anything really new there, small changes are listed in individual changelogs. The next patches, from 09/33 to 11/33, port the IPU3 IPA module to the FCQueue class. They should be similar to the implementation in v3 that was spread within the patches that updated the IPA algorithm operations. I have dropped the other changes to the IPU3 IPA module, not because they were wrong, but because the series was growing big already. I will rebase them on top of this v4 and submit them separately (unless someone beats me to it). Patches 12/33 to 16/33 then performs the same for the RkISP1 IPA module. I have also dropped the lens-related patches from v3, as I wanted to focus on the frame context queue first. Those patches will also be rebased on top, they're not lost. As mentioned above, the next two patches are new in v5. Patch 17/33 passes the FCQueue size as a template argument instead of a constructor argument. I've kept it separate as I'm not entirely sure it's the right option, I have a feeling we may have use cases for computing the queue size at runtime instead of compile time, and using a template argument doesn't bring that much as far as I can tell. Patch 18/33 disables copy-construction of the frame contexts, which shouldn't be controversial. The next 7 patches, from 19/33 to 25/34, port the RkISP1 algorithms to the frame context API. 19/33 starts by dropping the frameCount member variable of the active state as we can now use the frame number passed to the algorithm operations, and the next patches port the algorithms one by one, showcasing how data can be split between the active state and frame context. Different schemes may be possible, maybe with less duplication of member variables between the active state and frame context. I'm particularly interested in feedback on this. The last patch of this group, 25/33, documents the scheme used here. Finally, patches 26/33 to 33/33 improve the AWB implementation of the RkISP1 IPA module. Patch 26/33 in particular shows how the frame context allows fixing a defect of the current implementation. The rest are improvements that are detailed in the individual commit messages. We don't need to merge this series in one go. It is organized as three sets of patches (01/33 to 18/33 to introduce and use the frame context queue, 19/33 to 25/33 to showcase its usage in the RkISP1 IPA module, and 26/33 to 33/33 to improve the RkISP1 AWB). Each set is quite self-contained, but they are based on each other in that order. There is still work to do, in particular in the documentation, but I didn't want to spend more time on it before getting an approval of the general approach. Another point I'm not completely happy about is how the FCQueue alloc() and get() functions handle underruns, and actually how the whole underrun mechanism will be implemented. We need to experiment on top of this series to figure it out. Jacopo Mondi (1): ipa: rkisp1: Remove unused class member Kieran Bingham (6): ipa: libipa: Provide a common base for frame contexts ipa: libipa: algorithm: prepare(): Pass frame and frame Context ipa: libipa: algorithm: process(): Pass frame number ipa: libipa: algorithm: queueRequest(): Pass frame context ipa: rkisp1: Rename frameContext to activeState ipa: rkisp1: Convert to use the FCQueue Laurent Pinchart (24): ipa: ipu3: Fix style of Doxygen comment blocks ipa: ipu3: af: Pass context reference to afIsOutOfFocus() ipa: libipa: Pass a reference instead of pointer to Algorithm::process() ipa: ipu3: Use base FrameContext class ipa: ipu3: Use the FCQueue ipa: ipu3: Pass controls to algorithm's queueRequest() handler ipa: rkisp1: Sort documentation of the IPA context ipa: rkisp1: Use base FrameContext class ipa: libipa: Pass FCQueue size as template argument ipa: Disable copy-construction of IPAContext ipa: rkisp1: Use frame number passed to Algorithm::prepare() ipa: rkisp1: agc: Store per-frame information in frame context ipa: rkisp1: awb: Store per-frame information in frame context ipa: rkisp1: cproc: Store per-frame information in frame context ipa: rkisp1: dpf: Store per-frame information in frame context ipa: rkisp1: filter: Store per-frame information in frame context ipa: rkisp1: Document the active state and frame context ipa: rkisp1: awb: Use frame context to fix gains calculations ipa: rkisp1: awb: Store color temperature as an integer ipa: rkisp1: awb: Log means, gains and temperature in debug message ipa: rkisp1: awb: Prevent RGB means from being negative ipa: rkisp1: awb: Clamp gains to prevent divisions by zero ipa: rkisp1: awb: Freeze AWB when means are too small ipa: rkisp1: awb: Remove bias from gain calculation Quentin Schulz (1): ipa: rkisp1: awb: Add support for RGB means Umang Jain (1): ipa: libipa: Introduce FrameContextQueue src/ipa/ipu3/algorithms/af.cpp | 34 +-- src/ipa/ipu3/algorithms/af.h | 9 +- src/ipa/ipu3/algorithms/agc.cpp | 10 +- src/ipa/ipu3/algorithms/agc.h | 5 +- src/ipa/ipu3/algorithms/awb.cpp | 8 +- src/ipa/ipu3/algorithms/awb.h | 7 +- src/ipa/ipu3/algorithms/blc.cpp | 10 +- src/ipa/ipu3/algorithms/blc.h | 4 +- src/ipa/ipu3/algorithms/tone_mapping.cpp | 18 +- src/ipa/ipu3/algorithms/tone_mapping.h | 6 +- src/ipa/ipu3/ipa_context.cpp | 37 +--- src/ipa/ipu3/ipa_context.h | 25 +-- src/ipa/ipu3/ipu3.cpp | 37 ++-- src/ipa/libipa/algorithm.cpp | 4 + src/ipa/libipa/algorithm.h | 7 +- src/ipa/libipa/fc_queue.cpp | 140 ++++++++++++ src/ipa/libipa/fc_queue.h | 117 ++++++++++ src/ipa/libipa/meson.build | 2 + src/ipa/rkisp1/algorithms/agc.cpp | 46 ++-- src/ipa/rkisp1/algorithms/agc.h | 10 +- src/ipa/rkisp1/algorithms/awb.cpp | 269 ++++++++++++++++------- src/ipa/rkisp1/algorithms/awb.h | 12 +- src/ipa/rkisp1/algorithms/blc.cpp | 6 +- src/ipa/rkisp1/algorithms/blc.h | 4 +- src/ipa/rkisp1/algorithms/cproc.cpp | 52 +++-- src/ipa/rkisp1/algorithms/cproc.h | 5 +- src/ipa/rkisp1/algorithms/dpcc.cpp | 6 +- src/ipa/rkisp1/algorithms/dpcc.h | 4 +- src/ipa/rkisp1/algorithms/dpf.cpp | 33 +-- src/ipa/rkisp1/algorithms/dpf.h | 5 +- src/ipa/rkisp1/algorithms/filter.cpp | 49 +++-- src/ipa/rkisp1/algorithms/filter.h | 5 +- src/ipa/rkisp1/algorithms/gsl.cpp | 6 +- src/ipa/rkisp1/algorithms/gsl.h | 4 +- src/ipa/rkisp1/algorithms/lsc.cpp | 5 +- src/ipa/rkisp1/algorithms/lsc.h | 4 +- src/ipa/rkisp1/ipa_context.cpp | 219 +++++++++++++----- src/ipa/rkisp1/ipa_context.h | 64 +++++- src/ipa/rkisp1/rkisp1.cpp | 55 +++-- 39 files changed, 998 insertions(+), 345 deletions(-) create mode 100644 src/ipa/libipa/fc_queue.cpp create mode 100644 src/ipa/libipa/fc_queue.h