[{"id":31960,"web_url":"https://patchwork.libcamera.org/comment/31960/","msgid":"<87frofne7u.fsf@redhat.com>","date":"2024-10-29T15:03:33","subject":"Re: [PATCH 1/2] libcamera: software_isp: Initialize exposure+gain\n\tbefore agc calculations","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Stanislaw,\n\nthank you for the fix.\n\nStanislaw Gruszka writes:\n\n> On my setup, since commit fb8ad13d (\"libcamera: software_isp: Move exposure+gain\n> to an algorithm module\"), at start camera output stays very dark for dozen\n> of seconds, and then later slowly gets to normal. This is because existing\n> sensor exposure+gain settings are not used at start. We save initial\n> values in frameContext but in the agc algorithm we use IPA context.\n\nI think this was indeed just a mistake.\n\nReviewed-by: Milan Zamazal \n\n> Fix the problem by using in frameContext sensor values, since we already\n> use those in blc algorithm and change exposure type to int32_t to\n> unnecessary castings.\n>\n> Signed-off-by: Stanislaw Gruszka \n> ---\n> src/ipa/simple/algorithms/agc.cpp | 8 ++++----\n> src/ipa/simple/algorithms/agc.h | 2 +-\n> src/ipa/simple/algorithms/blc.h | 2 +-\n> src/ipa/simple/ipa_context.h | 2 +-\n> src/ipa/simple/soft_simple.cpp | 4 ++--\n> 5 files changed, 9 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n> index df92edd7..72aade14 100644\n> --- a/src/ipa/simple/algorithms/agc.cpp\n> +++ b/src/ipa/simple/algorithms/agc.cpp\n> @@ -39,7 +39,7 @@ Agc::Agc()\n> {\n> }\n> \n> -void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> +void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV)\n> {\n> \t/*\n> \t * kExpDenominator of 10 gives ~10% increment/decrement;\n> @@ -50,8 +50,8 @@ void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> \tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> \n> \tdouble next;\n> -\tint32_t &exposure = context.activeState.agc.exposure;\n> -\tdouble &again = context.activeState.agc.again;\n> +\tint32_t &exposure = frameContext.sensor.exposure;\n> +\tdouble &again = frameContext.sensor.gain;\n> \n> \tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> \t\tnext = exposure * kExpNumeratorUp / kExpDenominator;\n> @@ -129,7 +129,7 @@ void Agc::process(IPAContext &context,\n> \t}\n> \n> \tfloat exposureMSV = (denom == 0 ? 0 : static_cast(num) / denom);\n> -\tupdateExposure(context, exposureMSV);\n> +\tupdateExposure(context, frameContext, exposureMSV);\n> }\n> \n> REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h\n> index ad5fca9f..112d9f5a 100644\n> --- a/src/ipa/simple/algorithms/agc.h\n> +++ b/src/ipa/simple/algorithms/agc.h\n> @@ -25,7 +25,7 @@ public:\n> \t\t ControlList &metadata) override;\n> \n> private:\n> -\tvoid updateExposure(IPAContext &context, double exposureMSV);\n> +\tvoid updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV);\n> };\n> \n> } /* namespace ipa::soft::algorithms */\n> diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h\n> index 2cf2a877..67c688ae 100644\n> --- a/src/ipa/simple/algorithms/blc.h\n> +++ b/src/ipa/simple/algorithms/blc.h\n> @@ -27,7 +27,7 @@ public:\n> \t\t ControlList &metadata) override;\n> \n> private:\n> -\tuint32_t exposure_;\n> +\tint32_t exposure_;\n> \tdouble gain_;\n> };\n> \n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index fd121eeb..c510c436 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -53,7 +53,7 @@ struct IPAActiveState {\n> \n> struct IPAFrameContext : public FrameContext {\n> \tstruct {\n> -\t\tuint32_t exposure;\n> +\t\tint32_t exposure;\n> \t\tdouble gain;\n> \t} sensor;\n> };\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index c8ad55a2..dfacd6aa 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -310,8 +310,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> \n> \tControlList ctrls(sensorInfoMap_);\n> \n> -\tauto &againNew = context_.activeState.agc.again;\n> -\tctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);\n> +\tauto &againNew = frameContext.sensor.gain;\n> +\tctrls.set(V4L2_CID_EXPOSURE, frameContext.sensor.exposure);\n> \tctrls.set(V4L2_CID_ANALOGUE_GAIN,\n> \t\t static_cast(camHelper_ ? camHelper_->gainCode(againNew) : againNew));","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 B1979C3220\n\tfor ;\n\tTue, 29 Oct 2024 15:03:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE0BD65399;\n\tTue, 29 Oct 2024 16:03:44 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE4E360366\n\tfor ;\n\tTue, 29 Oct 2024 16:03:42 +0100 (CET)","from mail-lf1-f69.google.com (mail-lf1-f69.google.com\n\t[209.85.167.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-190-rXdDx7y1M0SiSy92tYIFBg-1; Tue, 29 Oct 2024 11:03:40 -0400","by mail-lf1-f69.google.com with SMTP id\n\t2adb3069b0e04-539ebb67c28so3693792e87.3\n\tfor ;\n\tTue, 29 Oct 2024 08:03:40 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-4318b567838sm180610935e9.23.2024.10.29.08.03.34\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 29 Oct 2024 08:03:35 -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=\"X2wmsng7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1730214221;\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\tin-reply-to:in-reply-to:references:references;\n\tbh=aFtueyEZOKWd3OdGcett4BhQu2T/67dWh3RJUncpt5o=;\n\tb=X2wmsng7ebYkV9robIaoqd2bj8eUbP64fa2fbrl/3mihXr5fSKWkL2IP0xUNR7z38S4DfC\n\tY1DUSSpl3NVx0hIlJwW68vyIUuEks1wN2ppJuBvqd9wcAxt9NxppmyKa6/+tNtuzW/qGMe\n\tNZ+l56zzYz65FWYw/y/x5GehEfmLQ3I=","X-MC-Unique":"rXdDx7y1M0SiSy92tYIFBg-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1730214218; x=1730819018;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=aFtueyEZOKWd3OdGcett4BhQu2T/67dWh3RJUncpt5o=;\n\tb=RdXzywdWkUIarwEe2xveQknDde1susjisH5q0pshBz/N4nONd3678uBHRDWSg6Y6Pz\n\tJHMvXZz9xUrGABhPoeBIZw2KXklyJwAKkK4FhsYFzMjMevMqY25WLQ8CzoyVooWJmolc\n\tF5FYRZ/wTKOCGwhbwP8m6Mmyse+nGr4CAfZoVd4EGEw8rty2BpwIEAJrROFMCQIFmXK2\n\ths655TJwz5X/btlFRsdKU1mUpWaHhT4ajjHwneKShcTDYCCpT2p8i41jqKaRnWtrSiCW\n\tYCRdhd+VuTGBo//nN5ZP3N9jnacI5L9SyVABKnC0mN+BZnI3bcw9eVFk5GfZrwgwaN0g\n\tMUuQ==","X-Gm-Message-State":"AOJu0Yxeg0aEFhFw3XJi3g2sNqXFIA5Ui4csPfr2aZiHSD4LpJmnnAPj\n\tyZcKciNA+9Qeu/Otnnd78pJMn2gnqUZQqV/0I86nWx4eFI8iZqVtUCEVCsYOABv/hfjAhckrAmh\n\twYfcSvbKwliQwcdN9jmUg6f3UCGw3O000W1hkfa+DLcbb4ruHK+QF5z4PxvZrpGQjshL8JFpNL4\n\tCV/aTLKlkdS4DLVxDkbXtr7TTGpOAAPhts1OT8Wi5gRHJ9KYGOKJUIg+w=","X-Received":["by 2002:a05:6512:10d1:b0:53b:1f7a:9bfd with SMTP id\n\t2adb3069b0e04-53b34a343d5mr6488925e87.52.1730214216884; \n\tTue, 29 Oct 2024 08:03:36 -0700 (PDT)","by 2002:a05:6512:10d1:b0:53b:1f7a:9bfd with SMTP id\n\t2adb3069b0e04-53b34a343d5mr6488840e87.52.1730214215892; \n\tTue, 29 Oct 2024 08:03:35 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEm9JbOdPOMoDdfjmxkV6WBxBztFsO1zqp6IvVEPkmmyNDL9J1VYK9NohXet/NP7wofEmbxIg==","From":"Milan Zamazal ","To":"Stanislaw Gruszka ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] libcamera: software_isp: Initialize exposure+gain\n\tbefore agc calculations","In-Reply-To":"<20241029112501.140252-1-stanislaw.gruszka@linux.intel.com>\n\t(Stanislaw Gruszka's message of \"Tue, 29 Oct 2024 12:25:00 +0100\")","References":"<20241029112501.140252-1-stanislaw.gruszka@linux.intel.com>","Date":"Tue, 29 Oct 2024 16:03:33 +0100","Message-ID":"<87frofne7u.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","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\" "}},{"id":32120,"web_url":"https://patchwork.libcamera.org/comment/32120/","msgid":"","date":"2024-11-12T11:38:07","subject":"Re: [PATCH 1/2] libcamera: software_isp: Initialize exposure+gain\n\tbefore agc calculations","submitter":{"id":140,"url":"https://patchwork.libcamera.org/api/people/140/","name":"Robert Mader","email":"robert.mader@collabora.com"},"content":"Thanks for the patch!\n\nTested-by: Robert Mader \n\nWould love to see it land so \nhttps://patchwork.libcamera.org/patch/21865/ can be rebased and land on top\n\nOn 29.10.24 12:25, Stanislaw Gruszka wrote:\n> On my setup, since commit fb8ad13d (\"libcamera: software_isp: Move exposure+gain\n> to an algorithm module\"), at start camera output stays very dark for dozen\n> of seconds, and then later slowly gets to normal. This is because existing\n> sensor exposure+gain settings are not used at start. We save initial\n> values in frameContext but in the agc algorithm we use IPA context.\n>\n> Fix the problem by using in frameContext sensor values, since we already\n> use those in blc algorithm and change exposure type to int32_t to\n> unnecessary castings.\n>\n> Signed-off-by: Stanislaw Gruszka \n> ---\n> src/ipa/simple/algorithms/agc.cpp | 8 ++++----\n> src/ipa/simple/algorithms/agc.h | 2 +-\n> src/ipa/simple/algorithms/blc.h | 2 +-\n> src/ipa/simple/ipa_context.h | 2 +-\n> src/ipa/simple/soft_simple.cpp | 4 ++--\n> 5 files changed, 9 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n> index df92edd7..72aade14 100644\n> --- a/src/ipa/simple/algorithms/agc.cpp\n> +++ b/src/ipa/simple/algorithms/agc.cpp\n> @@ -39,7 +39,7 @@ Agc::Agc()\n> {\n> }\n> \n> -void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> +void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV)\n> {\n> \t/*\n> \t * kExpDenominator of 10 gives ~10% increment/decrement;\n> @@ -50,8 +50,8 @@ void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> \tstatic constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> \n> \tdouble next;\n> -\tint32_t &exposure = context.activeState.agc.exposure;\n> -\tdouble &again = context.activeState.agc.again;\n> +\tint32_t &exposure = frameContext.sensor.exposure;\n> +\tdouble &again = frameContext.sensor.gain;\n> \n> \tif (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> \t\tnext = exposure * kExpNumeratorUp / kExpDenominator;\n> @@ -129,7 +129,7 @@ void Agc::process(IPAContext &context,\n> \t}\n> \n> \tfloat exposureMSV = (denom == 0 ? 0 : static_cast(num) / denom);\n> -\tupdateExposure(context, exposureMSV);\n> +\tupdateExposure(context, frameContext, exposureMSV);\n> }\n> \n> REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h\n> index ad5fca9f..112d9f5a 100644\n> --- a/src/ipa/simple/algorithms/agc.h\n> +++ b/src/ipa/simple/algorithms/agc.h\n> @@ -25,7 +25,7 @@ public:\n> \t\t ControlList &metadata) override;\n> \n> private:\n> -\tvoid updateExposure(IPAContext &context, double exposureMSV);\n> +\tvoid updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV);\n> };\n> \n> } /* namespace ipa::soft::algorithms */\n> diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h\n> index 2cf2a877..67c688ae 100644\n> --- a/src/ipa/simple/algorithms/blc.h\n> +++ b/src/ipa/simple/algorithms/blc.h\n> @@ -27,7 +27,7 @@ public:\n> \t\t ControlList &metadata) override;\n> \n> private:\n> -\tuint32_t exposure_;\n> +\tint32_t exposure_;\n> \tdouble gain_;\n> };\n> \n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index fd121eeb..c510c436 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -53,7 +53,7 @@ struct IPAActiveState {\n> \n> struct IPAFrameContext : public FrameContext {\n> \tstruct {\n> -\t\tuint32_t exposure;\n> +\t\tint32_t exposure;\n> \t\tdouble gain;\n> \t} sensor;\n> };\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index c8ad55a2..dfacd6aa 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -310,8 +310,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> \n> \tControlList ctrls(sensorInfoMap_);\n> \n> -\tauto &againNew = context_.activeState.agc.again;\n> -\tctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);\n> +\tauto &againNew = frameContext.sensor.gain;\n> +\tctrls.set(V4L2_CID_EXPOSURE, frameContext.sensor.exposure);\n> \tctrls.set(V4L2_CID_ANALOGUE_GAIN,\n> \t\t static_cast(camHelper_ ? camHelper_->gainCode(againNew) : againNew));\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 0CC3ABE173\n\tfor ;\n\tTue, 12 Nov 2024 11:38:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3B8C1657B0;\n\tTue, 12 Nov 2024 12:38:17 +0100 (CET)","from sender4-op-o12.zoho.com (sender4-op-o12.zoho.com\n\t[136.143.188.12])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EE680657B0\n\tfor ;\n\tTue, 12 Nov 2024 12:38:14 +0100 (CET)","by mx.zohomail.com with SMTPS id 1731411489790836.5451721927462;\n\tTue, 12 Nov 2024 03:38:09 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=collabora.com\n\theader.i=robert.mader@collabora.com header.b=\"gA1Srm8a\"; \n\tdkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1731411491; cv=none; \n\td=zohomail.com; s=zohoarc; \n\tb=AEOKNKms5isse/KKUlIrUy5b9ld2sLcg6qrR0sY/1TCyqCp19AzUez3easCSfxRHy02LSfSzEXcPgDG2r022Mt2dZ8YfAQBBzdYmLP1K6oY6v0z+9bCKwd+mdItLt3fFIssK/eeYqDH8QKMZZ92D3x8PMzFQ1hNjBn4jDmJYlgg=","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; \n\ts=zohoarc; t=1731411491;\n\th=Content-Type:Content-Transfer-Encoding:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To:Cc;\n\tbh=mi5ILgLwn24xfZEa3Z8oFigENUrFhAYO0k2w+dAbr0g=; \n\tb=IyHDZEYypo0fT4YSqPecCE23JXftwcfmxjeM1ImGv0bsM2LvFuCkZw1+ci/ppzBdzS0THVqwIAeQI1Kyw93zUrliovJJe302i6N0EyUSSfkzpHZhOOYn7/Kdnx87i6bPPX1hfqbAJ+CEjIKIpytm534Tnnpoi+yQvwfWSogpGXQ=","ARC-Authentication-Results":"i=1; mx.zohomail.com;\n\tdkim=pass header.i=collabora.com;\n\tspf=pass smtp.mailfrom=robert.mader@collabora.com;\n\tdmarc=pass header.from=","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1731411491;\n\ts=zohomail; d=collabora.com; i=robert.mader@collabora.com;\n\th=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To:Cc;\n\tbh=mi5ILgLwn24xfZEa3Z8oFigENUrFhAYO0k2w+dAbr0g=;\n\tb=gA1Srm8acDjp+3WGr8kYZawcotYAoWoQHtdbXwVduFFPJfBt+VgVq1aID8mgcTGs\n\tqHkESWnREO3z35LSliUpBK7hEW4raEdaM4HeH7Jqpaw0u6Afx5yP5qRWXrl4C0nslXP\n\tREcjpkYSRBkWiHaCGMTsEkrhsmvmoRKKjnNUHnF0=","Message-ID":"","Date":"Tue, 12 Nov 2024 12:38:07 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/2] libcamera: software_isp: Initialize exposure+gain\n\tbefore agc calculations","To":"libcamera-devel@lists.libcamera.org","References":"<20241029112501.140252-1-stanislaw.gruszka@linux.intel.com>","Content-Language":"en-US, de-DE","From":"Robert Mader ","In-Reply-To":"<20241029112501.140252-1-stanislaw.gruszka@linux.intel.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}},{"id":32471,"web_url":"https://patchwork.libcamera.org/comment/32471/","msgid":"<173291805743.4100433.3148351906432225229@ping.linuxembedded.co.uk>","date":"2024-11-29T22:07:37","subject":"Re: [PATCH 1/2] libcamera: software_isp: Initialize exposure+gain\n\tbefore agc calculations","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stanislaw Gruszka (2024-10-29 11:25:00)\n> On my setup, since commit fb8ad13d (\"libcamera: software_isp: Move exposure+gain\n> to an algorithm module\"), at start camera output stays very dark for dozen\n> of seconds, and then later slowly gets to normal. This is because existing\n> sensor exposure+gain settings are not used at start. We save initial\n> values in frameContext but in the agc algorithm we use IPA context.\n> \n> Fix the problem by using in frameContext sensor values, since we already\n> use those in blc algorithm and change exposure type to int32_t to\n> unnecessary castings.\n> \n> Signed-off-by: Stanislaw Gruszka \n\nTested-by: Kieran Bingham \nReviewed-by: Kieran Bingham \n\n> ---\n> src/ipa/simple/algorithms/agc.cpp | 8 ++++----\n> src/ipa/simple/algorithms/agc.h | 2 +-\n> src/ipa/simple/algorithms/blc.h | 2 +-\n> src/ipa/simple/ipa_context.h | 2 +-\n> src/ipa/simple/soft_simple.cpp | 4 ++--\n> 5 files changed, 9 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n> index df92edd7..72aade14 100644\n> --- a/src/ipa/simple/algorithms/agc.cpp\n> +++ b/src/ipa/simple/algorithms/agc.cpp\n> @@ -39,7 +39,7 @@ Agc::Agc()\n> {\n> }\n> \n> -void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> +void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV)\n> {\n> /*\n> * kExpDenominator of 10 gives ~10% increment/decrement;\n> @@ -50,8 +50,8 @@ void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> \n> double next;\n> - int32_t &exposure = context.activeState.agc.exposure;\n> - double &again = context.activeState.agc.again;\n> + int32_t &exposure = frameContext.sensor.exposure;\n> + double &again = frameContext.sensor.gain;\n> \n> if (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> next = exposure * kExpNumeratorUp / kExpDenominator;\n> @@ -129,7 +129,7 @@ void Agc::process(IPAContext &context,\n> }\n> \n> float exposureMSV = (denom == 0 ? 0 : static_cast(num) / denom);\n> - updateExposure(context, exposureMSV);\n> + updateExposure(context, frameContext, exposureMSV);\n> }\n> \n> REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h\n> index ad5fca9f..112d9f5a 100644\n> --- a/src/ipa/simple/algorithms/agc.h\n> +++ b/src/ipa/simple/algorithms/agc.h\n> @@ -25,7 +25,7 @@ public:\n> ControlList &metadata) override;\n> \n> private:\n> - void updateExposure(IPAContext &context, double exposureMSV);\n> + void updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV);\n> };\n> \n> } /* namespace ipa::soft::algorithms */\n> diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h\n> index 2cf2a877..67c688ae 100644\n> --- a/src/ipa/simple/algorithms/blc.h\n> +++ b/src/ipa/simple/algorithms/blc.h\n> @@ -27,7 +27,7 @@ public:\n> ControlList &metadata) override;\n> \n> private:\n> - uint32_t exposure_;\n> + int32_t exposure_;\n> double gain_;\n> };\n> \n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index fd121eeb..c510c436 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -53,7 +53,7 @@ struct IPAActiveState {\n> \n> struct IPAFrameContext : public FrameContext {\n> struct {\n> - uint32_t exposure;\n> + int32_t exposure;\n> double gain;\n> } sensor;\n> };\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index c8ad55a2..dfacd6aa 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -310,8 +310,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> \n> ControlList ctrls(sensorInfoMap_);\n> \n> - auto &againNew = context_.activeState.agc.again;\n> - ctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);\n> + auto &againNew = frameContext.sensor.gain;\n> + ctrls.set(V4L2_CID_EXPOSURE, frameContext.sensor.exposure);\n> ctrls.set(V4L2_CID_ANALOGUE_GAIN,\n> static_cast(camHelper_ ? camHelper_->gainCode(againNew) : againNew));\n> \n> -- \n> 2.43.0\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 6786EBE175\n\tfor ;\n\tFri, 29 Nov 2024 22:07:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B38E66040;\n\tFri, 29 Nov 2024 23:07:42 +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 B5B6466025\n\tfor ;\n\tFri, 29 Nov 2024 23:07:40 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CBA5880A;\n\tFri, 29 Nov 2024 23:07:15 +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=\"H4KE9UbI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732918035;\n\tbh=hl7pC+1FOYff6ljKmn56yPe6FZVJxk3CqcfRJ05Fzhk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=H4KE9UbI+jaoEOQvqGZwLy6XL0nyR6/FNxLUVs70R4zdQC0uVzV3MYmz4FHwVbMBN\n\tH760sg3hPul/err1Blb4jZxa/x+rU7qcXipKFpB2DexaKXVqvYQw9NZ5l2GI+xW43N\n\tIuxN0JdNrRG2KpokTxrKS5bpIhF1pbSDUQ6DpSk8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241029112501.140252-1-stanislaw.gruszka@linux.intel.com>","References":"<20241029112501.140252-1-stanislaw.gruszka@linux.intel.com>","Subject":"Re: [PATCH 1/2] libcamera: software_isp: Initialize exposure+gain\n\tbefore agc calculations","From":"Kieran Bingham ","Cc":"Milan Zamazal ","To":"Stanislaw Gruszka ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 29 Nov 2024 22:07:37 +0000","Message-ID":"<173291805743.4100433.3148351906432225229@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\" "}},{"id":32474,"web_url":"https://patchwork.libcamera.org/comment/32474/","msgid":"<173292093336.4100433.9154575370770127950@ping.linuxembedded.co.uk>","date":"2024-11-29T22:55:33","subject":"Re: [PATCH 1/2] libcamera: software_isp: Initialize exposure+gain\n\tbefore agc calculations","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Robert,\n\nQuoting Robert Mader (2024-11-12 11:38:07)\n> Thanks for the patch!\n> \n> Tested-by: Robert Mader \n> \n> Would love to see it land so \n> https://patchwork.libcamera.org/patch/21865/ can be rebased and land on top\n\nHave you tested https://patchwork.libcamera.org/patch/21865/ ? Could you\nsend a tag (RB or TB) for that one please?\n\nRegards\n--\nKieran\n\n\n\n> \n> On 29.10.24 12:25, Stanislaw Gruszka wrote:\n> > On my setup, since commit fb8ad13d (\"libcamera: software_isp: Move exposure+gain\n> > to an algorithm module\"), at start camera output stays very dark for dozen\n> > of seconds, and then later slowly gets to normal. This is because existing\n> > sensor exposure+gain settings are not used at start. We save initial\n> > values in frameContext but in the agc algorithm we use IPA context.\n> >\n> > Fix the problem by using in frameContext sensor values, since we already\n> > use those in blc algorithm and change exposure type to int32_t to\n> > unnecessary castings.\n> >\n> > Signed-off-by: Stanislaw Gruszka \n> > ---\n> > src/ipa/simple/algorithms/agc.cpp | 8 ++++----\n> > src/ipa/simple/algorithms/agc.h | 2 +-\n> > src/ipa/simple/algorithms/blc.h | 2 +-\n> > src/ipa/simple/ipa_context.h | 2 +-\n> > src/ipa/simple/soft_simple.cpp | 4 ++--\n> > 5 files changed, 9 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp\n> > index df92edd7..72aade14 100644\n> > --- a/src/ipa/simple/algorithms/agc.cpp\n> > +++ b/src/ipa/simple/algorithms/agc.cpp\n> > @@ -39,7 +39,7 @@ Agc::Agc()\n> > {\n> > }\n> > \n> > -void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> > +void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV)\n> > {\n> > /*\n> > * kExpDenominator of 10 gives ~10% increment/decrement;\n> > @@ -50,8 +50,8 @@ void Agc::updateExposure(IPAContext &context, double exposureMSV)\n> > static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1;\n> > \n> > double next;\n> > - int32_t &exposure = context.activeState.agc.exposure;\n> > - double &again = context.activeState.agc.again;\n> > + int32_t &exposure = frameContext.sensor.exposure;\n> > + double &again = frameContext.sensor.gain;\n> > \n> > if (exposureMSV < kExposureOptimal - kExposureSatisfactory) {\n> > next = exposure * kExpNumeratorUp / kExpDenominator;\n> > @@ -129,7 +129,7 @@ void Agc::process(IPAContext &context,\n> > }\n> > \n> > float exposureMSV = (denom == 0 ? 0 : static_cast(num) / denom);\n> > - updateExposure(context, exposureMSV);\n> > + updateExposure(context, frameContext, exposureMSV);\n> > }\n> > \n> > REGISTER_IPA_ALGORITHM(Agc, \"Agc\")\n> > diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h\n> > index ad5fca9f..112d9f5a 100644\n> > --- a/src/ipa/simple/algorithms/agc.h\n> > +++ b/src/ipa/simple/algorithms/agc.h\n> > @@ -25,7 +25,7 @@ public:\n> > ControlList &metadata) override;\n> > \n> > private:\n> > - void updateExposure(IPAContext &context, double exposureMSV);\n> > + void updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV);\n> > };\n> > \n> > } /* namespace ipa::soft::algorithms */\n> > diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h\n> > index 2cf2a877..67c688ae 100644\n> > --- a/src/ipa/simple/algorithms/blc.h\n> > +++ b/src/ipa/simple/algorithms/blc.h\n> > @@ -27,7 +27,7 @@ public:\n> > ControlList &metadata) override;\n> > \n> > private:\n> > - uint32_t exposure_;\n> > + int32_t exposure_;\n> > double gain_;\n> > };\n> > \n> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> > index fd121eeb..c510c436 100644\n> > --- a/src/ipa/simple/ipa_context.h\n> > +++ b/src/ipa/simple/ipa_context.h\n> > @@ -53,7 +53,7 @@ struct IPAActiveState {\n> > \n> > struct IPAFrameContext : public FrameContext {\n> > struct {\n> > - uint32_t exposure;\n> > + int32_t exposure;\n> > double gain;\n> > } sensor;\n> > };\n> > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> > index c8ad55a2..dfacd6aa 100644\n> > --- a/src/ipa/simple/soft_simple.cpp\n> > +++ b/src/ipa/simple/soft_simple.cpp\n> > @@ -310,8 +310,8 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> > \n> > ControlList ctrls(sensorInfoMap_);\n> > \n> > - auto &againNew = context_.activeState.agc.again;\n> > - ctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure);\n> > + auto &againNew = frameContext.sensor.gain;\n> > + ctrls.set(V4L2_CID_EXPOSURE, frameContext.sensor.exposure);\n> > ctrls.set(V4L2_CID_ANALOGUE_GAIN,\n> > static_cast(camHelper_ ? camHelper_->gainCode(againNew) : againNew));\n> > \n> \n> -- \n> Robert Mader\n> Consultant Software Developer\n> \n> Collabora Ltd.\n> Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK\n> Registered in England & Wales, no. 5513718\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 1EC80C3237\n\tfor ;\n\tFri, 29 Nov 2024 22:55:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3FF066041;\n\tFri, 29 Nov 2024 23:55:37 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6D07E66025\n\tfor ;\n\tFri, 29 Nov 2024 23:55:36 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 975C080A;\n\tFri, 29 Nov 2024 23:55:11 +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=\"eldp2/+M\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1732920911;\n\tbh=rqB221fxW1kVmMU1vi8GNikz3wVKHH1nL/938bOG6eg=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=eldp2/+McHqzV2PbYG8/sirbB7HI3r1F2ug5zTDJ2y+Soinpgm1Mx5lTU6NCkAXR7\n\tWpNfwEKVDnc0TwB4xFwI5XBylk7wcIi+dfD5KDZ+NB41PiPBXgaK0864mk6c/JYQLI\n\ttpATWRhm+IRB33uPG5M0D1KsExX5AnFC00nLwE5Q=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20241029112501.140252-1-stanislaw.gruszka@linux.intel.com>\n\t","Subject":"Re: [PATCH 1/2] libcamera: software_isp: Initialize exposure+gain\n\tbefore agc calculations","From":"Kieran Bingham ","To":"Robert Mader ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 29 Nov 2024 22:55:33 +0000","Message-ID":"<173292093336.4100433.9154575370770127950@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\" "}}]