[{"id":26266,"web_url":"https://patchwork.libcamera.org/comment/26266/","msgid":"<20230119171431.frqduxnbrtdv3wgi@uno.localdomain>","date":"2023-01-19T17:14:31","subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: rkisp1: Raise maximum\n\tanalogue gain","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Mikhail\n\nOn Thu, Jan 19, 2023 at 06:59:05PM +0300, Mikhail Rudenko via libcamera-devel wrote:\n> Omnivision OV4689 sensor driver exposes maximum analogue gain of\n> 16x. Raise kMaxAnalogueGain to 16.0, so that the full gain range can\n> be used.\n>\n> Signed-off-by: Mikhail Rudenko \n> ---\n> src/ipa/rkisp1/algorithms/agc.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> index e3470e25..e4cb2fc7 100644\n> --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> @@ -38,7 +38,7 @@ LOG_DEFINE_CATEGORY(RkISP1Agc)\n>\n> /* Limits for analogue gain values */\n> static constexpr double kMinAnalogueGain = 1.0;\n> -static constexpr double kMaxAnalogueGain = 8.0;\n> +static constexpr double kMaxAnalogueGain = 16.0;\n\nI'm very surprised we have such an hard limit..\n\nShould it be made configurable ? Should we allow the sensor tuning\nfile to provide this value ? Not something required for you to do to\nfix this ofc\n\n>\n> /* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n> static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n> --\n> 2.39.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 EC7F7C3240\n\tfor ;\n\tThu, 19 Jan 2023 17:14:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A8256625E6;\n\tThu, 19 Jan 2023 18:14:35 +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 5E45861EFE\n\tfor ;\n\tThu, 19 Jan 2023 18:14:34 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 018CA501;\n\tThu, 19 Jan 2023 18:14:33 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674148475;\n\tbh=K48CzPH5sF7ojHpsdaj8FYapnBNZoBlhoxdRJZRaa8o=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=0afsjf4YHhwMlqAuvYssfdkKTzbhStnvjUG3ksy/7CjYdMK9Rl3BAtscAthIWzc+4\n\tANl0fTzwctxKMn1B6BXJIN83euNSZxPxfwD32F4AvZguXecJqGKVL4TY3UKtAqnmxb\n\tB6Y08eFSXL+Iatfk3BYej6ZYCvcp6TLFyCXHIinvzRplBfuSHbHEHV/TcrpDrzdwcp\n\th+4LfCq7K/E66+rdthytMq2wHr7xAjzu8rwH8TaY+A7qi2uOS8WcmV1XQp1zf5KsI1\n\tiWNy+bMDg3PpMw0svc8X9duSV3H6yuBKXkC4oXLMX1Dx/LPz0173bm3OH73Vt5pxYp\n\tA3rNBZiVE6K0Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674148474;\n\tbh=K48CzPH5sF7ojHpsdaj8FYapnBNZoBlhoxdRJZRaa8o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NsZwOsRT2xeM0dNiuhVdZKo21YP1B++UAailtZOlRj3t4tbkMcFxhGUTxl1X1Uxhe\n\tZ/asN9FYZqbUwnghzpJgk2pSwYYOuzMc7y/rj08iBhF242aOcU9R/dDyOs1tO6jIJG\n\tSArRkU7Ao0BYvQavzyh7GjqHq7lERIr4RDXjydJE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"NsZwOsRT\"; dkim-atps=neutral","Date":"Thu, 19 Jan 2023 18:14:31 +0100","To":"Mikhail Rudenko ","Message-ID":"<20230119171431.frqduxnbrtdv3wgi@uno.localdomain>","References":"<20230119155905.464995-1-mike.rudenko@gmail.com>\n\t<20230119155905.464995-5-mike.rudenko@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230119155905.464995-5-mike.rudenko@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: rkisp1: Raise maximum\n\tanalogue gain","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26267,"web_url":"https://patchwork.libcamera.org/comment/26267/","msgid":"<20230119172635.qy22jc6mooqdbyab@uno.localdomain>","date":"2023-01-19T17:26:35","subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: rkisp1: Raise maximum\n\tanalogue gain","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hello again\n\nOn Thu, Jan 19, 2023 at 06:14:31PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> Hi Mikhail\n>\n> On Thu, Jan 19, 2023 at 06:59:05PM +0300, Mikhail Rudenko via libcamera-devel wrote:\n> > Omnivision OV4689 sensor driver exposes maximum analogue gain of\n> > 16x. Raise kMaxAnalogueGain to 16.0, so that the full gain range can\n> > be used.\n> >\n> > Signed-off-by: Mikhail Rudenko \n> > ---\n> > src/ipa/rkisp1/algorithms/agc.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> > index e3470e25..e4cb2fc7 100644\n> > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> > @@ -38,7 +38,7 @@ LOG_DEFINE_CATEGORY(RkISP1Agc)\n> >\n> > /* Limits for analogue gain values */\n> > static constexpr double kMinAnalogueGain = 1.0;\n> > -static constexpr double kMaxAnalogueGain = 8.0;\n> > +static constexpr double kMaxAnalogueGain = 16.0;\n>\n> I'm very surprised we have such an hard limit..\n>\n> Should it be made configurable ? Should we allow the sensor tuning\n> file to provide this value ? Not something required for you to do to\n> fix this ofc\n>\n\nIn facts, this is already overriden using the sensor's provided max\ngain as returned from the CameraHelper\n\nsrc/ipa/rkisp1/rkisp1.cpp: context_.configuration.sensor.maxAnalogueGain = camHelper_->gain(maxGain);\n\nBut we limit it to 8.0\n\nsrc/ipa/rkisp1/algorithms/agc.cpp: double maxAnalogueGain = std::min(configuration.sensor.maxAnalogueGain,\nsrc/ipa/rkisp1/algorithms/agc.cpp- kMaxAnalogueGain);\n\nShould the camera sensor/helper be let express their max gain as they\nlike ?\n\n\n> >\n> > /* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n> > static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n> > --\n> > 2.39.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 AEB01BD87C\n\tfor ;\n\tThu, 19 Jan 2023 17:26:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F1869625D8;\n\tThu, 19 Jan 2023 18:26:39 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0B1D61EFE\n\tfor ;\n\tThu, 19 Jan 2023 18:26:38 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 08EF5501;\n\tThu, 19 Jan 2023 18:26:37 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674149200;\n\tbh=1RHCCCfrmWWHPD+piHzrK1aJ9jv+/QKjnV2wyj2Cv5E=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=phL+nKx+nElsVNQw/1+Y8pXKiET0Ceda2nnnE804mlUq43wqfGp1APaIw1CPAcTTY\n\tLMlIel0dUaFMTzeHYE0LvDSR/7fmTAqCv9gYucAbasp+vW9gP73+BqcujU8alqJEcs\n\tPchtqB9BDfw/3oeSoN+N8OR8bUKzj61wajL+IRTDM/EKgh2pwiC6l0v6ldDKpGUxz2\n\tEENQ08ld18QPdhftLbVT3bw6J1ieZq7MjQk1jhdWTecjiuRmSTVwek3d4qZqXuCQWz\n\t0cCzfv7QF22gpdFGIeyxWx2H9jkjAyYc6tVLFaMayDDwc+veSR45E/CyRgfLdZAG/4\n\tEMH27quqEza2w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674149198;\n\tbh=1RHCCCfrmWWHPD+piHzrK1aJ9jv+/QKjnV2wyj2Cv5E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JbX4odwzJgl97JGSgNqkPPh581vhKXZAomGMwAugAXur1cyGve6E/hHIJ18CHiFAu\n\tRwVKn7NLd0iWJTCZ80cTWdSUSwGNxS/QKVNsltTsjym9Cmdc1R2npPcUvFpqLQ53gT\n\tAjAh3zI8K8pBhzgyLgKGxudhxi2lTkMKNc5Kaq/E="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JbX4odwz\"; dkim-atps=neutral","Date":"Thu, 19 Jan 2023 18:26:35 +0100","To":"Jacopo Mondi ","Message-ID":"<20230119172635.qy22jc6mooqdbyab@uno.localdomain>","References":"<20230119155905.464995-1-mike.rudenko@gmail.com>\n\t<20230119155905.464995-5-mike.rudenko@gmail.com>\n\t<20230119171431.frqduxnbrtdv3wgi@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230119171431.frqduxnbrtdv3wgi@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: rkisp1: Raise maximum\n\tanalogue gain","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org,\n\tMikhail Rudenko ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26298,"web_url":"https://patchwork.libcamera.org/comment/26298/","msgid":"<875yd11dm1.fsf@gmail.com>","date":"2023-01-20T14:27:52","subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: rkisp1: Raise maximum\n\tanalogue gain","submitter":{"id":146,"url":"https://patchwork.libcamera.org/api/people/146/","name":"Mikhail Rudenko","email":"mike.rudenko@gmail.com"},"content":"Hi Jacopo,\n\nOn 2023-01-19 at 18:26 +01, Jacopo Mondi wrote:\n\n> Hello again\n>\n> On Thu, Jan 19, 2023 at 06:14:31PM +0100, Jacopo Mondi via libcamera-devel wrote:\n>> Hi Mikhail\n>>\n>> On Thu, Jan 19, 2023 at 06:59:05PM +0300, Mikhail Rudenko via libcamera-devel wrote:\n>> > Omnivision OV4689 sensor driver exposes maximum analogue gain of\n>> > 16x. Raise kMaxAnalogueGain to 16.0, so that the full gain range can\n>> > be used.\n>> >\n>> > Signed-off-by: Mikhail Rudenko \n>> > ---\n>> > src/ipa/rkisp1/algorithms/agc.cpp | 2 +-\n>> > 1 file changed, 1 insertion(+), 1 deletion(-)\n>> >\n>> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n>> > index e3470e25..e4cb2fc7 100644\n>> > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n>> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n>> > @@ -38,7 +38,7 @@ LOG_DEFINE_CATEGORY(RkISP1Agc)\n>> >\n>> > /* Limits for analogue gain values */\n>> > static constexpr double kMinAnalogueGain = 1.0;\n>> > -static constexpr double kMaxAnalogueGain = 8.0;\n>> > +static constexpr double kMaxAnalogueGain = 16.0;\n>>\n>> I'm very surprised we have such an hard limit..\n>>\n>> Should it be made configurable ? Should we allow the sensor tuning\n>> file to provide this value ? Not something required for you to do to\n>> fix this ofc\n>>\n>\n> In facts, this is already overriden using the sensor's provided max\n> gain as returned from the CameraHelper\n>\n> src/ipa/rkisp1/rkisp1.cpp: context_.configuration.sensor.maxAnalogueGain = camHelper_->gain(maxGain);\n>\n> But we limit it to 8.0\n>\n> src/ipa/rkisp1/algorithms/agc.cpp: double maxAnalogueGain = std::min(configuration.sensor.maxAnalogueGain,\n> src/ipa/rkisp1/algorithms/agc.cpp- kMaxAnalogueGain);\n>\n> Should the camera sensor/helper be let express their max gain as they\n> like ?\n\nDo I understand correctly that you suggest dropping 4/4 as it is and\nremoving kMaxAnalogueGain and analogue gain limiting in\nsrc/ipa/rkisp1/algorithms/agc.cpp instead?\n\nBest regards,\nMikhail\n\n>\n>> >\n>> > /* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n>> > static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n>> > --\n>> > 2.39.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 8EA96BE175\n\tfor ;\n\tFri, 20 Jan 2023 14:31:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0A59A625E4;\n\tFri, 20 Jan 2023 15:31:55 +0100 (CET)","from mail-lj1-x229.google.com (mail-lj1-x229.google.com\n\t[IPv6:2a00:1450:4864:20::229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A7E4D61EFD\n\tfor ;\n\tFri, 20 Jan 2023 15:31:53 +0100 (CET)","by mail-lj1-x229.google.com with SMTP id y18so5653950ljk.11\n\tfor ;\n\tFri, 20 Jan 2023 06:31:53 -0800 (PST)","from razdolb (93-80-66-125.broadband.corbina.ru. [93.80.66.125])\n\tby smtp.gmail.com with ESMTPSA id\n\ti18-20020a2ea372000000b0028b94f21814sm1121989ljn.95.2023.01.20.06.31.51\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 20 Jan 2023 06:31:51 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674225115;\n\tbh=n52krcu37L015JPuBn4X/Lr45Bm+rLgUB/9SL0LKFzs=;\n\th=References:To:Date:In-reply-to:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=tgGP/PkHnaQTnb60hHp/h3fr4pbzCMPcyxVobyvY7OsVFcotURe1NinLhoe5CTqAk\n\tSNOVh008dWcMvRIzTu/rnxmBGbxEyHBOS0gBgG644BKTt5QDN9IG+aIXK3U5umA901\n\tIShpCMrVSwpeQcit8GzdAy2rSyrskx/9Gzt/1ctexSKdLwWui5M6QbmFvrMAyP/cCO\n\t6vDtjXBCpoUOwR7Wqa0MWaQg4zWFVkJTzzatvpAmB7esbC2ogC/Dk7ul64vTQamIkZ\n\t4XitZHYR0uMI1Yb4l6JX0Oz30IpbhJ5HJK/6qUJAbP8GOrEyt1VZohGx0POMA6MkMc\n\t7q6ibGh10V5cw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;\n\th=mime-version:message-id:in-reply-to:date:subject:cc:to:from\n\t:user-agent:references:from:to:cc:subject:date:message-id:reply-to;\n\tbh=cwoGwUCnPauRhhOX1pu0oNPq9XZNb553yh10R2mTwyo=;\n\tb=WvlWH2ifJEXhdmzaKH4E1SJH753LD9VpSlmN3lD0N3lSSA2y/tjid/IhQfyXfO4ETv\n\t3Mn6PWBlm74YFrncVKzCyHWjmjCG8kY9luWynC17gMm+zoguK9yaQL36LimwJBg4KBzD\n\t13Mg9DED77NeVKJQhHbW4CgfKogYeA/J91TPgckNXVFF1EhBHZgFPweHbAjs9wH2n055\n\tceejZUxFu7Q+U7hwTr2QBIrcFZINCfBIBpeNkJKAXP+lynw3oN1aPcaB8slk0bccGabT\n\tO2tGJPxxf+dCuWXf2HcDemT6wE2wTCtusqu22X1MmrmPN7sFnRAZJ8mWhXQFnl9NZqRn\n\t9Dzw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"WvlWH2if\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=mime-version:message-id:in-reply-to:date:subject:cc:to:from\n\t:user-agent:references:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=cwoGwUCnPauRhhOX1pu0oNPq9XZNb553yh10R2mTwyo=;\n\tb=5epJfqA2v8RplFsvOY1OH1EvWV1Nk8rBRjV7BBvR0tJedxLrPXL9xWfWPh0SK7TbdV\n\t2KP4qxS9pOYnjO9nnqkdr44NXDXO9/wJ9dXuEp26r0ctp4ePqxhUxql+zxnVctrQ/tXH\n\tZ3vfBTfDb94z78WdrDFyZN7Lqf60DgI9ZcER0H3jc9vQkQULN4X56hmZYsWGK0ToQUum\n\tOY19z/T/VXA5mXu/Dq2JRuJl2+v4qzFWy7OdhR0kW4ByazeNytYOxwbV4Vu1c+tSfhzB\n\t0axdPlUG55QuSKu1m61+6vFV7eGnB4Dhy3f/7/nirxkxH7hd4EkL324+A4uDufbZTc2K\n\tZP/A==","X-Gm-Message-State":"AFqh2kogUvX45EDKUTz0eut9bWNW83T45o0ohDh7IisGbkrpefw7KKsY\n\ttEiLuW3Vx0X6h1MOrgcB43LVqzyYMQo=","X-Google-Smtp-Source":"AMrXdXumalOJOFUM7CEcVNB1BSmLTMDkISqGOmTPj5KF2uVhQ7rKR8sXBpCunMxRLA/qv6KDQbdA+g==","X-Received":"by 2002:a2e:b6cf:0:b0:283:4fff:7aeb with SMTP id\n\tm15-20020a2eb6cf000000b002834fff7aebmr4320328ljo.30.1674225112536; \n\tFri, 20 Jan 2023 06:31:52 -0800 (PST)","References":"<20230119155905.464995-1-mike.rudenko@gmail.com>\n\t<20230119155905.464995-5-mike.rudenko@gmail.com>\n\t<20230119171431.frqduxnbrtdv3wgi@uno.localdomain>\n\t<20230119172635.qy22jc6mooqdbyab@uno.localdomain>","User-agent":"mu4e 1.9.0; emacs 28.2","To":"Jacopo Mondi ","Date":"Fri, 20 Jan 2023 17:27:52 +0300","In-reply-to":"<20230119172635.qy22jc6mooqdbyab@uno.localdomain>","Message-ID":"<875yd11dm1.fsf@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: rkisp1: Raise maximum\n\tanalogue gain","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","From":"Mikhail Rudenko via libcamera-devel\n\t","Reply-To":"Mikhail Rudenko ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26299,"web_url":"https://patchwork.libcamera.org/comment/26299/","msgid":"<20230120150909.osirih6o2fior5xo@uno.localdomain>","date":"2023-01-20T15:09:09","subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: rkisp1: Raise maximum\n\tanalogue gain","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Mikhail\n\nOn Fri, Jan 20, 2023 at 05:27:52PM +0300, Mikhail Rudenko via libcamera-devel wrote:\n>\n> Hi Jacopo,\n>\n> On 2023-01-19 at 18:26 +01, Jacopo Mondi wrote:\n>\n> > Hello again\n> >\n> > On Thu, Jan 19, 2023 at 06:14:31PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> >> Hi Mikhail\n> >>\n> >> On Thu, Jan 19, 2023 at 06:59:05PM +0300, Mikhail Rudenko via libcamera-devel wrote:\n> >> > Omnivision OV4689 sensor driver exposes maximum analogue gain of\n> >> > 16x. Raise kMaxAnalogueGain to 16.0, so that the full gain range can\n> >> > be used.\n> >> >\n> >> > Signed-off-by: Mikhail Rudenko \n> >> > ---\n> >> > src/ipa/rkisp1/algorithms/agc.cpp | 2 +-\n> >> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >> >\n> >> > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\n> >> > index e3470e25..e4cb2fc7 100644\n> >> > --- a/src/ipa/rkisp1/algorithms/agc.cpp\n> >> > +++ b/src/ipa/rkisp1/algorithms/agc.cpp\n> >> > @@ -38,7 +38,7 @@ LOG_DEFINE_CATEGORY(RkISP1Agc)\n> >> >\n> >> > /* Limits for analogue gain values */\n> >> > static constexpr double kMinAnalogueGain = 1.0;\n> >> > -static constexpr double kMaxAnalogueGain = 8.0;\n> >> > +static constexpr double kMaxAnalogueGain = 16.0;\n> >>\n> >> I'm very surprised we have such an hard limit..\n> >>\n> >> Should it be made configurable ? Should we allow the sensor tuning\n> >> file to provide this value ? Not something required for you to do to\n> >> fix this ofc\n> >>\n> >\n> > In facts, this is already overriden using the sensor's provided max\n> > gain as returned from the CameraHelper\n> >\n> > src/ipa/rkisp1/rkisp1.cpp: context_.configuration.sensor.maxAnalogueGain = camHelper_->gain(maxGain);\n> >\n> > But we limit it to 8.0\n> >\n> > src/ipa/rkisp1/algorithms/agc.cpp: double maxAnalogueGain = std::min(configuration.sensor.maxAnalogueGain,\n> > src/ipa/rkisp1/algorithms/agc.cpp- kMaxAnalogueGain);\n> >\n> > Should the camera sensor/helper be let express their max gain as they\n> > like ?\n>\n> Do I understand correctly that you suggest dropping 4/4 as it is and\n> removing kMaxAnalogueGain and analogue gain limiting in\n> src/ipa/rkisp1/algorithms/agc.cpp instead?\n\nLooking at the history, that values is there since the very beginning,\nwhich makes me wonder if it's just a leftover from early developments.\n\nWe should let the sensor express it's maximum gain value in my\nopinion (and give it a way to tune it from configuration file\neventually, but not for this patch).\n\nSame for kMinAnalogueValue.\n\nTrue that\n\n static constexpr double kMinAnalogueGain = 1.0;\n static constexpr double kMaxAnalogueGain = 8.0;\n\n\tdouble minAnalogueGain = std::max(configuration.sensor.minAnalogueGain,\n\t\t\t\t\t kMinAnalogueGain);\n\tdouble maxAnalogueGain = std::min(configuration.sensor.maxAnalogueGain,\n\t\t\t\t\t kMaxAnalogueGain);\n\n\tdouble stepGain = std::clamp(exposureValue / shutterTime,\n\t\t\t\t minAnalogueGain, maxAnalogueGain);\n\nGuarantees that the CameraSensorHelper receives gain guaranteed to be\nwithin a know interval. If we remove such clamp we would possibly feed\nto CameraSensorHelper values not previously tested (and which only\ndepend on the sensor driver implementation).\n\nI'm looking at\n uint32_t CameraSensorHelper::gainCode(double gain) const\n\nand I'm trying to figure out if it's safe or we need a safety clamp\nmechanism\n\n>\n> Best regards,\n> Mikhail\n>\n> >\n> >> >\n> >> > /* \\todo Honour the FrameDurationLimits control instead of hardcoding a limit */\n> >> > static constexpr utils::Duration kMaxShutterSpeed = 60ms;\n> >> > --\n> >> > 2.39.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 B0616C3240\n\tfor ;\n\tFri, 20 Jan 2023 15:09:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C432625D8;\n\tFri, 20 Jan 2023 16:09:14 +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 ACCA761EFD\n\tfor ;\n\tFri, 20 Jan 2023 16:09:12 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 18D43514;\n\tFri, 20 Jan 2023 16:09:12 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674227354;\n\tbh=7hfalXxLzPvHGvfhGg3bTrebSPTYPUULxWQ4K3vd7zg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=H7nmriQgxv6qyOvzNp7GfvcwcmI7+THHap1HfhfRfavCM4aqszvra9pqrHi0SQzpv\n\tHnYA/R7bvB/8X4Uj1xiLoS8bdGZdiXZWG3ItjN0Le1ihYwJEHNmfU2RRC1zxtY844R\n\tDYGnAIrQPWuScl+rhHZjLLogC3fh6+CTaq5foZp/3YGZtvkD2Juj85bICD9QLT5mTO\n\t45CfOJTcWgmvgcpdR5Yi6nlDKMglXE0fPvz696RxJcE3BP9l/ZwnHAAkmGKUGr4iAi\n\tNvDV0MiCFq/YorEQP88HPkougOAGGDaTCMf7UdjNt7PMI8Sqv34VuHASlHfPYujQM1\n\tWm+nVzqknY0PA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674227352;\n\tbh=7hfalXxLzPvHGvfhGg3bTrebSPTYPUULxWQ4K3vd7zg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EGNRLsVzGWphSucn2n2OxzzzvEWUt4If6zCTOBU7y7UzZlWmaOS4aahU0GVXCyGA5\n\tuwvx1U+8z8f3eFBwdeOnNev0U39B0dJDSovNM+SAomQQQxtpsJwv57rdLSgMpYbSAD\n\t9HP2b/DSSPr8oX4yxPPynYuhmlkNoEV31KDUDZ2c="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EGNRLsVz\"; dkim-atps=neutral","Date":"Fri, 20 Jan 2023 16:09:09 +0100","To":"Mikhail Rudenko ","Message-ID":"<20230120150909.osirih6o2fior5xo@uno.localdomain>","References":"<20230119155905.464995-1-mike.rudenko@gmail.com>\n\t<20230119155905.464995-5-mike.rudenko@gmail.com>\n\t<20230119171431.frqduxnbrtdv3wgi@uno.localdomain>\n\t<20230119172635.qy22jc6mooqdbyab@uno.localdomain>\n\t<875yd11dm1.fsf@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<875yd11dm1.fsf@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: rkisp1: Raise maximum\n\tanalogue gain","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]