From patchwork Tue Sep 23 19:06:52 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 24439 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 11F38BDB1C for ; Tue, 23 Sep 2025 19:07:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 580AD6B5FF; Tue, 23 Sep 2025 21:07:05 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="S2OhudSN"; dkim-atps=neutral Received: from sea.source.kernel.org (sea.source.kernel.org [IPv6:2600:3c0a:e001:78e:0:1991:8:25]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CBB36B5A2 for ; Tue, 23 Sep 2025 21:07:02 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id B49F144130 for ; Tue, 23 Sep 2025 19:07:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AFB1CC4CEF5; Tue, 23 Sep 2025 19:06:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758654420; bh=HnqliwoujGcaYuaSJXBGDv+pr/yNHgJ4qCdL/dh4d3A=; h=From:To:Cc:Subject:Date:From; b=S2OhudSNeRCV2SXEDh8xg/imsCxL1yjOU4YtwjQPJSR3Qbpoe336E37VGSPQJoP8M Awcr0ySsxvNSy9Vv3pZfnxrFVlvXWZBC/Iwhr4+P9+R7n5W8uPJP8HGEWT31B4buiS 7AnHtp1byi9gaQwB4DnvrYvW+1uYV/0V0zmzSEV+meGEAUzihvtq1Nt7bcL0OOcVC6 64+IIFkSR5Nvkj3qOvxkxCsWDyfAV/YwcE9WY0ZFjBzQrikpYYqzrRZmvYyQMyM+Lk kdiZWLMKPambRc61TD+iKOMglCcW1EUS3RkFsjz2Pf6hDigZZHDYK7YXlhGynRtdsI 8w6Qwb/rWs0ig== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede Subject: [PATCH 0/5] ipa: software_isp: AGC: Fox AGC oscillation bug Date: Tue, 23 Sep 2025 21:06:52 +0200 Message-ID: <20250923190657.21453-1-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Hi All, Due to a combination of not having correct control-delay information for various sensors as well as the generic nature of the simple-pipeline + software-ISP meaning that any pipeline delays are unknown it is impossible to get reliable control-delay values. Wrong control-delay values can lead to pretty bad oscilation. An example oscilation which I observed while debugging this went as follows: 1. gain is set at 910 of 1000 max 2. a new frame is processed and is found to not be bright enough, gain gets changed to 1000 3. the gain gets applied 1 frame earlier then expected 4. a new frame with gain 1000 is processed and is now found to be too bright (gain overshoot), the AGC code starts with the old gain of 910 due to the control-delay issue and decreases this to 820, this gets applied 1 frame earlier then expected 5. a new frame with gain 820 get processed, this frame is not bright enough so the gain gets increased from 1000 to 1000 (clipped at max), this gets applied 1 frame earlier then expected 6. a new frame with gain 1000 is processed and is now found to be too bright (gain overshoot), the AGC code starts with the old gain of 820 due to the control-delay issue and decreases this to 740, this gets applied 1 frame earlier then expected ... This repeats over and over increasing the oscilation (by decreasing the low gain value) until the gain bounces between gain-min and gain max leading to pretty bad flickering. This series fixes this by remembering the last set gain and exposure values instead of using the delayed-ctrls values, combined with skipping a fixed number of frames after changing values. This effectively reverts commit bb1aa92eb9ee ("libcamera: software_isp: Initialize exposure+gain before agc calculations"). That commit was added to fix the initial gain and exposure starting at 0, this series initializes the cached values with the actual sensor values on the first frame to avoid re-introducing this problem. This deviates from how other ISPs do things which is not ideal, but atm it is not possible to fix the wrong control-delay values, so this is the best we can do. Regards, Hans Hans de Goede (5): ipa: software_isp: Fix context_.configuration.agc.againMin init ipa: software_isp: AGC: do not lower gain below default gain value ipa: software_isp: AGC: Raise exposure or gain not both at the same time ipa: software_isp: AGC: Only use integers for exposure calculations ipa: software_isp: AGC: Stop using delayed control for previous values src/ipa/simple/algorithms/agc.cpp | 53 +++++++++++++++++++++---------- src/ipa/simple/algorithms/agc.h | 3 +- src/ipa/simple/ipa_context.h | 8 ++++- src/ipa/simple/soft_simple.cpp | 11 +++++-- 4 files changed, 53 insertions(+), 22 deletions(-) Tested-by: Milan Zamazal Tested-by: Kieran Bingham