[{"id":38482,"web_url":"https://patchwork.libcamera.org/comment/38482/","msgid":"<85fr5eaoio.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-04-01T11:49:51","subject":"Re: [PATCH 1/2] rkisp1: Add disco mode","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Isaac,\n\nthank you for the patch.\n\nIsaac Scott writes:\n\n> Allow users of the rkisp1 pipeline handler to adjust the level of funk\n> in their streams to their heart's content.\n\nConsidering recent Kieran's work, this change should be integrated into\nlibipa rather than being rkisp1 specific. Especially software ISP could\nbenefit from the change, working around the problems caused by applying\nscaling too early and white balance too late.\n\n> No non-funky changes intended by this patch.\n>\n> Signed-off-by: Isaac \"Moog Modular\" Scott \n> ---\n> src/ipa/rkisp1/algorithms/awb.cpp | 25 +++++++++++++++++++++++++\n> src/ipa/rkisp1/ipa_context.h | 4 ++++\n> src/libcamera/control_ids_core.yaml | 17 +++++++++++++++++\n> 3 files changed, 46 insertions(+)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index f83da545b..749add143 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -9,6 +9,7 @@\n> \n> #include \n> #include \n> +#include \n\ncmath should be included instead.\n\n> #include \n> \n> @@ -94,6 +95,8 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData)\n> \n> \tcmap[&controls::ColourGains] = ControlInfo(0.0f, 3.996f,\n> \t\t\t\t\t\t Span{ { 1.0f, 1.0f } });\n\nPlease insert one or more blank lines here.\n\n> +\tcmap[&controls::DiscoMode] = ControlInfo(false, true, false);\n> +\tcmap[&controls::Funk] = ControlInfo(0, 10, 0);\n\nI really don't like the naming here. While your use case is music\nvideos, we're in a camera business here. How about VideoEnhancementMode\nand AwbOscillationFactor?\n\n> \tif (!tuningData.contains(\"algorithm\"))\n> \t\tLOG(RkISP1Awb, Info) << \"No AWB algorithm specified.\"\n> @@ -175,6 +178,19 @@ void Awb::queueRequest(IPAContext &context,\n> \n> \tframeContext.awb.autoEnabled = awb.autoEnabled;\n> \n> +\tconst auto &discoModeRockin = controls.get(controls::DiscoMode);\n\n`auto' is generally unclear about the type and makes the code hard to\nunderstand, please use a specific type declaration. (I work on a patch\nseries to remove the remaining uses of `auto' from libcamera.)\n\nOther than that the change LGTM. The math below is too complicated to\nreview and I suppose the controls descriptions were generated by AI,\nwhich must be absolutely right. If you can post v2, addressing the more\nimportant issues pointed out above and adding some tests to make sure\nvariables are named correctly, before 3 AM the change can be included in\nthe next release.\n\n> +\tif (discoModeRockin && (*discoModeRockin == true) != awb.discoMode) {\n> +\t\tawb.discoMode = *discoModeRockin;\n> +\t}\n> +\n> +\tconst auto &funkRequired = controls.get(controls::Funk);\n> +\tif (funkRequired) {\n> +\t\tawb.funkMagnitude = *funkRequired;\n> +\t}\n> +\n> +\tframeContext.awb.discoMode = awb.discoMode;\n> +\tframeContext.awb.funkMagnitude = awb.funkMagnitude;\n> +\n> \tif (awb.autoEnabled)\n> \t\treturn;\n> \n> @@ -227,6 +243,12 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n> \tauto gainConfig = params->block();\n> \tgainConfig.setEnabled(true);\n> \n> +\tif (frameContext.awb.discoMode) {\n> +\t\tfloat funk = static_cast(frameContext.awb.funkMagnitude) * 3.0f + 1.0f;\n> +\t\tframeContext.awb.gains.r() = (sin(funk * (frame % 100) / 100) + 1.0f);\n> +\t\tframeContext.awb.gains.b() = (cos(funk * (frame % 100) / 100) + 1.0f);\n> +\t}\n> +\n> \tgainConfig->gain_green_b = std::clamp(256 * frameContext.awb.gains.g(), 0, 0x3ff);\n> \tgainConfig->gain_blue = std::clamp(256 * frameContext.awb.gains.b(), 0, 0x3ff);\n> \tgainConfig->gain_red = std::clamp(256 * frameContext.awb.gains.r(), 0, 0x3ff);\n> @@ -300,6 +322,9 @@ void Awb::process(IPAContext &context,\n> \t\t});\n> \tmetadata.set(controls::ColourTemperature, frameContext.awb.temperatureK);\n> \n> +\tmetadata.set(controls::DiscoMode, frameContext.awb.discoMode);\n> +\tmetadata.set(controls::Funk, frameContext.awb.funkMagnitude);\n> +\n> \tif (!stats || !(stats->meas_type & RKISP1_CIF_ISP_STAT_AWB)) {\n> \t\tLOG(RkISP1Awb, Error) << \"AWB data is missing in statistics\";\n> \t\treturn;\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index e61391bb1..48e567a95 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -110,6 +110,8 @@ struct IPAActiveState {\n> \t\tAwbState automatic;\n> \n> \t\tbool autoEnabled;\n> +\t\tbool discoMode;\n> +\t\tunsigned int funkMagnitude;\n> \t} awb;\n> \n> \tstruct {\n> @@ -178,6 +180,8 @@ struct IPAFrameContext : public FrameContext {\n> \t\tRGB gains;\n> \t\tbool autoEnabled;\n> \t\tunsigned int temperatureK;\n> +\t\tbool discoMode;\n> +\t\tunsigned int funkMagnitude;\n> \t} awb;\n> \n> \tstruct {\n> diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml\n> index 89991d03d..7a25aad45 100644\n> --- a/src/libcamera/control_ids_core.yaml\n> +++ b/src/libcamera/control_ids_core.yaml\n> @@ -1376,4 +1376,21 @@ controls:\n> The nominal range is [-180, 180], where 0° leaves hues unchanged and the\n> range wraps around continuously, with 180° == -180°.\n> \n> + - DiscoMode:\n> + type: bool\n> + direction: inout\n> + description: |\n> + Enable / disable the feeling of being in your favourite discotheque,\n> + stabilising and pushing the boundaries of funk to maximal levels.\n> +\n> + The DiscoMode control can be fine-tuned to the user's desired level\n> + of funk, depending on whether they prefer a gentle or aggressive\n> + vibe in their streams.\n> +\n> + - Funk:\n> + type: int32_t\n> + direction: inout\n> + description: |\n> + Adjust the funk intensity level, from 0 (enough to do a relaxed two-step)\n> + to 10 (maximal, earth-shattering, weapons-grade funk).\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 EFAD8BEFBE\n\tfor ;\n\tWed, 1 Apr 2026 11:50:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E8F0762D54;\n\tWed, 1 Apr 2026 13:49:59 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0254262CCA\n\tfor ;\n\tWed, 1 Apr 2026 13:49:57 +0200 (CEST)","from mail-qk1-f197.google.com (mail-qk1-f197.google.com\n\t[209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-687-FnhrTZGQM9uH-yMCc72HXg-1; Wed, 01 Apr 2026 07:49:55 -0400","by mail-qk1-f197.google.com with SMTP id\n\taf79cd13be357-8cd7fc27cf7so1982164585a.1\n\tfor ;\n\tWed, 01 Apr 2026 04:49:55 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\taf79cd13be357-8d028095097sm1122869185a.46.2026.04.01.04.49.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 01 Apr 2026 04:49:53 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"bTAHCrdg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1775044196;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=fEI2u4h3+gWjyksHKcrdacJVBSlzp7Wpm/XGSlPfXZw=;\n\tb=bTAHCrdgRujWvPeAYpmSY1R0WR+fhSe6GZNwUUixmCO/pa4KMr82rPA7pvKj7I6+UNAk6P\n\tYnVR0dGLntbNL8yGoGGPgAc9m6t+KSkk+LDhfvNe8QmP459ypLKqfYcy9bvlxPqc1DT/ER\n\tUfvzZE+SCh3mmpHNYBaslkUCJKG0f8Q=","X-MC-Unique":"FnhrTZGQM9uH-yMCc72HXg-1","X-Mimecast-MFC-AGG-ID":"FnhrTZGQM9uH-yMCc72HXg_1775044195","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1775044195; x=1775648995;\n\th=content-transfer-encoding:mime-version:user-agent:message-id:date\n\t:references:in-reply-to:subject:cc:to:from:x-gm-gg\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=R147RAXtTDvrm/nHfhuvhcEvPQpDVjSsyhW3SuYOzeE=;\n\tb=iluW1xN13lM8Br7MXktEGwqw+FLqXVt4GJSDkMk66a1RkuJMzvuzN/VE168K5MyXOU\n\tU3tlIbH/oB+fGTrzcqzanXHBoBvVrtKkpxThmLDIQ4jlzYtKlvSJC26nWWIzERzSuYrT\n\t6OJuFh/MPW13QZWdXtZ7MTTIEMpprT2RYLJVlYoRsxsx6QCCVsIY+jCkESLg8vw+ot7k\n\tTzAMBpGrXm4pIRMv8LyxZGzvwyVrDkt7xmPy0FSedhYDBOhRW2dr9AKdseYMGQOim5Hy\n\t9iuUh42mje9Bd1IWCoCgQVKRO1QHhDvCRL5xqd2m97lpVnDkZroagrzL2NnAwIRdO5Jv\n\tq5ow==","X-Gm-Message-State":"AOJu0Yxpe50an8YGjIwmWMRt+DenPCJnSY892awl683l6PEm/JkvpTzX\n\tybW4LS1KUMjLti3IjEgVUp04FPRP8TfAg5c2pDWaTser+n9+ZsLaBHgV7ok03K+X+ga3wRc9Vm3\n\tKnIs6qvtuLsL+QHT/K0qVFz0V0eO2En5VDj+47DqqKUAZ6AfFe6hlMdP+X6B2aLpmkFQqQC3aAQ\n\t2ITPXIesDD5rwSYbULwXV4MkcdoD1eiSFvzVVl2ElTQxlxIkE52gr4fv1Jgwo=","X-Gm-Gg":"ATEYQzy+sPMt4tNObRPkjRcT65BBErkvRYONCVtdb00XZyOlZdoLwvTUbaMMDLG3MWe\n\tKHo9jubOD1c18mBvn2GvuyS7ElfwutHZZSI8WCpIDgqWZE4Xk2wyDzZNepOEn6y93eI1g8PDjbI\n\ttsz9Il74ONETaAYEluAIqj88R0PvQqPsIvhvGH8Fbg1HDdwptM7Z3bBtYxVd+Z4nA8o0pggEGFe\n\thvsWizN+Fe1kabhmucjoNlSFjvYudASNxBjzFBHiz79fKc3J5sOQoSaMia+tIbepy/jpUy1bWE4\n\tv7oNa77pKwIdSLWmEP3fkB/yI7wK4JGh6HIU3oLuh182QX3sPbgXK0aLuqznVvJQBQPlKg5DvAb\n\tPo//AVcGK/eZKwJCIWf4cr5z+163OEWsBTLhyKvpYwZwu86L/V4B+jnN5obPxm7BwoJhRTKC43/\n\tI=","X-Received":["by 2002:a05:620a:44cc:b0:8cf:e152:592d with SMTP id\n\taf79cd13be357-8d1b5af164amr478886285a.21.1775044194601; \n\tWed, 01 Apr 2026 04:49:54 -0700 (PDT)","by 2002:a05:620a:44cc:b0:8cf:e152:592d with SMTP id\n\taf79cd13be357-8d1b5af164amr478882185a.21.1775044193969; \n\tWed, 01 Apr 2026 04:49:53 -0700 (PDT)"],"From":"Milan Zamazal ","To":"Isaac Scott ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] rkisp1: Add disco mode","In-Reply-To":"<20260401071650.116344-4-isaac.scott@ideasonboard.com> (Isaac\n\tScott's message of \"Wed, 1 Apr 2026 08:16:39 +0100\")","References":"<20260401071650.116344-2-isaac.scott@ideasonboard.com>\n\t<20260401071650.116344-4-isaac.scott@ideasonboard.com>","Date":"Wed, 01 Apr 2026 13:49:51 +0200","Message-ID":"<85fr5eaoio.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"TF6riPI0Dkh6NPg5JoAauJ0F8_WgGg2n_TLok4PreyI_1775044195","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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\" "}}]