From patchwork Wed Jan 21 13:39:07 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 25906 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 B22DDC3220 for ; Wed, 21 Jan 2026 13:39:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D555E61FC4; Wed, 21 Jan 2026 14:39:21 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="NQMxdnn2"; dkim-atps=neutral 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 368F161F9F for ; Wed, 21 Jan 2026 14:39:20 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:8b90:6146:64f5:1ba3]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 2C834B3; Wed, 21 Jan 2026 14:38:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1769002728; bh=hEqb5IxHKUxrHhm4fXa+wP+tHGIv6yqW/0VBYCFTJ/0=; h=From:To:Cc:Subject:Date:From; b=NQMxdnn2JuM7KRRKEVV0XGSBeLZpzEonN3fuUS8C8TAWOZ1aBGMnAClJ1PnM9Y0zz D1FnOCAQtT0TOlkU1By64Jh7nqqaeA1J04u2cOYDuzFUXyfyOWCDBvHCXhMgwhAlWt 6YOEOPqD6WZR+syud38jcPhHKXQvz0/OS9VFGaTQ= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2] ipa: rkisp1: cproc: Fix brightness and contrast value handling Date: Wed, 21 Jan 2026 14:39:07 +0100 Message-ID: <20260121133910.34925-1-stefan.klug@ideasonboard.com> 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" By reverse engineering it was discovered that the transfer curve applied by the CPROC module is Yout = Yin * contrast + brightness/2. However the most common expectation on contrast is to not change the middle gray of an image. So the expected curve is: Yout = (Yin - 0.5) * contrast + 0.5 + brightness Map the user expectation to the hardware by changing the brightness value written to the hardware. This also includes multiplying the requested brightness value by two. Note: The limits of the brightness control are left as is even though they can not be reached without changing the contrast. But limiting them to [0.5, 0.5] would cut off parts of the combined control space of contrast + brightness. Signed-off-by: Stefan Klug --- This patch needs to be applied on top of the quantization series: https://patchwork.libcamera.org/project/libcamera/list/?series=5708 Changes in v2: - Moved calculation of brightness into helper function, to be able to reuse it inside configure() - Moved calculation of actualBrightness from process() to the helper function - Added documentation for the new context variables - Removed unnecessary curly braces - Improved comment --- src/ipa/rkisp1/algorithms/cproc.cpp | 58 ++++++++++++++++++++++------- src/ipa/rkisp1/ipa_context.cpp | 17 +++++++-- src/ipa/rkisp1/ipa_context.h | 3 ++ 3 files changed, 61 insertions(+), 17 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp index 7484e4780094..e0f6ac9816f7 100644 --- a/src/ipa/rkisp1/algorithms/cproc.cpp +++ b/src/ipa/rkisp1/algorithms/cproc.cpp @@ -46,6 +46,33 @@ constexpr float kDefaultSaturation = 1.0f; */ constexpr float kHueScale = -90.0f; +void applyRequestedBrightness(IPAActiveState &activeState) +{ + auto &cproc = activeState.cproc; + + /* + * The CPROC module applies the transfer function + * + * Yout = Yin * contrast + brightness/2 + * + * The calculations are done one bit wider than the input data to + * account for the maximum contrast of 2x without clamping. Brightness + * is applied after and therefore affects the output only with half the + * value. + * + * Most users expect that changing contrast doesn't change the middle + * gray and that the brightness value is normalized to one. So they + * expect a transfer function of + * + * Yout = (Yin - 0.5) * contrast + 0.5 + brightness + * + * To implement above formula with the given hardware we need adjust the + * hardware brightness value depending on the contrast. + */ + cproc.brightness = (1 - cproc.contrast.value()) + 2 * cproc.requestedBrightness; + cproc.actualBrightness = (cproc.brightness.value() - (1 - cproc.contrast.value())) / 2; +} + } /* namespace */ /** @@ -76,8 +103,10 @@ int ColorProcessing::configure(IPAContext &context, { auto &cproc = context.activeState.cproc; - cproc.brightness = BrightnessQ(kDefaultBrightness); + cproc.requestedBrightness = kDefaultBrightness; cproc.contrast = ContrastQ(kDefaultContrast); + applyRequestedBrightness(context.activeState); + cproc.hue = HueQ(kDefaultHue); cproc.saturation = SaturationQ(kDefaultSaturation); @@ -98,17 +127,6 @@ void ColorProcessing::queueRequest(IPAContext &context, if (frame == 0) update = true; - const auto &brightness = controls.get(controls::Brightness); - if (brightness) { - BrightnessQ value = *brightness; - if (cproc.brightness != value) { - cproc.brightness = value; - update = true; - } - - LOG(RkISP1CProc, Debug) << "Set brightness to " << value; - } - const auto &contrast = controls.get(controls::Contrast); if (contrast) { ContrastQ value = *contrast; @@ -120,6 +138,19 @@ void ColorProcessing::queueRequest(IPAContext &context, LOG(RkISP1CProc, Debug) << "Set contrast to " << value; } + const auto &brightness = controls.get(controls::Brightness); + if (brightness) + cproc.requestedBrightness = *brightness; + + if (update || brightness) { + BrightnessQ old = cproc.brightness; + applyRequestedBrightness(context.activeState); + if (cproc.brightness != old) + update = true; + + LOG(RkISP1CProc, Debug) << "Set brightness to " << cproc.actualBrightness; + } + const auto &hue = controls.get(controls::Hue); if (hue) { /* Scale the Hue from ]-90, +90] */ @@ -144,6 +175,7 @@ void ColorProcessing::queueRequest(IPAContext &context, } frameContext.cproc.brightness = cproc.brightness; + frameContext.cproc.actualBrightness = cproc.actualBrightness; frameContext.cproc.contrast = cproc.contrast; frameContext.cproc.hue = cproc.hue; frameContext.cproc.saturation = cproc.saturation; @@ -179,7 +211,7 @@ void ColorProcessing::process([[maybe_unused]] IPAContext &context, [[maybe_unused]] const rkisp1_stat_buffer *stats, ControlList &metadata) { - metadata.set(controls::Brightness, frameContext.cproc.brightness.value()); + metadata.set(controls::Brightness, frameContext.cproc.actualBrightness); metadata.set(controls::Contrast, frameContext.cproc.contrast.value()); metadata.set(controls::Hue, frameContext.cproc.hue.value() * kHueScale); metadata.set(controls::Saturation, frameContext.cproc.saturation.value()); diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index 15cb0afe9fe8..f9d0aff058bc 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -232,9 +232,15 @@ namespace libcamera::ipa::rkisp1 { /** * \var IPAActiveState::cproc * \brief State for the Color Processing algorithm + * + * \var IPAActiveState::cproc.requestedBrightness + * \brief Brightness level requested by the user + * + * \var IPAActiveState::cproc.actualBrightness + * \brief Brightness level after contrast compensation * - * \struct IPAActiveState::cproc.brightness - * \brief Brightness level + * \var IPAActiveState::cproc.brightness + * \brief Hardware brightness level * * \var IPAActiveState::cproc.contrast * \brief Contrast level @@ -400,8 +406,11 @@ namespace libcamera::ipa::rkisp1 { * \var IPAFrameContext::cproc * \brief Color Processing parameters for this frame * - * \struct IPAFrameContext::cproc.brightness - * \brief Brightness level + * \var IPAFrameContext::cproc.actualBrightness + * \brief Brightness level after contrast compensation + * + * \var IPAFrameContext::cproc.brightness + * \brief Hardware brightness level * * \var IPAFrameContext::cproc.contrast * \brief Contrast level diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 80b035044cda..72ec64532dd8 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -122,6 +122,8 @@ struct IPAActiveState { } ccm; struct { + float requestedBrightness; + float actualBrightness; BrightnessQ brightness; ContrastQ contrast; HueQ hue; @@ -181,6 +183,7 @@ struct IPAFrameContext : public FrameContext { } awb; struct { + float actualBrightness; BrightnessQ brightness; ContrastQ contrast; HueQ hue;