[{"id":14023,"web_url":"https://patchwork.libcamera.org/comment/14023/","msgid":"<20201201184142.GT4569@pendragon.ideasonboard.com>","date":"2020-12-01T18:41:42","subject":"Re: [libcamera-devel] [PATCH v4 4/5] libcamera: controls: Improve\n\tdocumentation for ExposureTime and AnalogueGain","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Tue, Dec 01, 2020 at 05:55:35PM +0000, David Plowman wrote:\n> Setting these controls \"fixes\" them and the AE may not change them;\n> setting them back to zero returns them to the control of the AE\n> algorithm.\n> \n> Signed-off-by: David Plowman \n> ---\n> src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> 1 file changed, 14 insertions(+)\n> \n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index a883e27e..fba1f545 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -125,8 +125,15 @@ controls:\n> Exposure time (shutter speed) for the frame applied in the sensor\n> device. This value is specified in micro-seconds.\n> \n> + Setting this value means that it is now fixed and the AE algorithm may\n> + not change it. Setting it back to zero returns it to the control of the\n> + AE algorithm.\n> +\n> \\sa AnalogueGain AeEnable\n> \n> + \\todo Consider how setting the exposure time interacts with other AE\n> + features, such as aperture, aperture/shutter priority modes etc.\n\nNo need for the additional indentation. I'd also expand this a bit to\nmention AeEnable. Maybe something along the lines of the following ?\n\n \\todo Document the interactions between AeEnable and setting a fixed\n value for this control. Consider interactions with other AE features,\n such as aperture and aperture/shutter priority mode, and decide if\n control of which features should be automatically adjusted shouldn't\n better be handled through a separate AE mode control.\n\nIf you're fine with this, I'll use this text when applying, there's no\nneed to resend this patch.\n\n> +\n> - AnalogueGain:\n> type: float\n> description: |\n> @@ -134,8 +141,15 @@ controls:\n> The value of the control specifies the gain multiplier applied to all\n> colour channels. This value cannot be lower than 1.0.\n> \n> + Setting this value means that it is now fixed and the AE algorithm may\n> + not change it. Setting it back to zero returns it to the control of the\n> + AE algorithm.\n> +\n> \\sa ExposureTime AeEnable\n> \n> + \\todo Consider how setting the analogue gain interacts with other AE\n> + features, such as aperture, aperture/shutter priority modes etc.\n> +\n> - Brightness:\n> type: float\n> description: |","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 0EA0EBE177\n\tfor ;\n\tTue, 1 Dec 2020 18:41:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B36C63501;\n\tTue, 1 Dec 2020 19:41:53 +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 9F7A863460\n\tfor ;\n\tTue, 1 Dec 2020 19:41:51 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 23AD3DBD;\n\tTue, 1 Dec 2020 19:41:51 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OGz6yBTv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606848111;\n\tbh=sb0GqofDkJjjCVyJHGrAa2S4BSKQ++/6fxXgDkTfEt8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OGz6yBTvurSrzv0DCVW8CHewnlqAlzVeR/zK3dFJSV84FnLEhYEZwvnatMUQ8r7LP\n\tqmmp29pTFooIGNtUuJlble3TiKaL8lbailal54b0d0X6dSSafXlqK+o8iCXtMfS3+H\n\tAK6wcCJ+8yKvV4EXFHbD6XeCbli84QHPA97xKzvo=","Date":"Tue, 1 Dec 2020 20:41:42 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"<20201201184142.GT4569@pendragon.ideasonboard.com>","References":"<20201201175536.11093-1-david.plowman@raspberrypi.com>\n\t<20201201175536.11093-5-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201201175536.11093-5-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 4/5] libcamera: controls: Improve\n\tdocumentation for ExposureTime and AnalogueGain","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14030,"web_url":"https://patchwork.libcamera.org/comment/14030/","msgid":"","date":"2020-12-01T19:55:02","subject":"Re: [libcamera-devel] [PATCH v4 4/5] libcamera: controls: Improve\n\tdocumentation for ExposureTime and AnalogueGain","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nThanks very much for the review. All sounds great to me!\n\nBest regards and thank you\n\nDavid\n\nOn Tue, 1 Dec 2020 at 18:41, Laurent Pinchart\n wrote:\n>\n> Hi David,\n>\n> Thank you for the patch.\n>\n> On Tue, Dec 01, 2020 at 05:55:35PM +0000, David Plowman wrote:\n> > Setting these controls \"fixes\" them and the AE may not change them;\n> > setting them back to zero returns them to the control of the AE\n> > algorithm.\n> >\n> > Signed-off-by: David Plowman \n> > ---\n> > src/libcamera/control_ids.yaml | 14 ++++++++++++++\n> > 1 file changed, 14 insertions(+)\n> >\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index a883e27e..fba1f545 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -125,8 +125,15 @@ controls:\n> > Exposure time (shutter speed) for the frame applied in the sensor\n> > device. This value is specified in micro-seconds.\n> >\n> > + Setting this value means that it is now fixed and the AE algorithm may\n> > + not change it. Setting it back to zero returns it to the control of the\n> > + AE algorithm.\n> > +\n> > \\sa AnalogueGain AeEnable\n> >\n> > + \\todo Consider how setting the exposure time interacts with other AE\n> > + features, such as aperture, aperture/shutter priority modes etc.\n>\n> No need for the additional indentation. I'd also expand this a bit to\n> mention AeEnable. Maybe something along the lines of the following ?\n>\n> \\todo Document the interactions between AeEnable and setting a fixed\n> value for this control. Consider interactions with other AE features,\n> such as aperture and aperture/shutter priority mode, and decide if\n> control of which features should be automatically adjusted shouldn't\n> better be handled through a separate AE mode control.\n>\n> If you're fine with this, I'll use this text when applying, there's no\n> need to resend this patch.\n>\n> > +\n> > - AnalogueGain:\n> > type: float\n> > description: |\n> > @@ -134,8 +141,15 @@ controls:\n> > The value of the control specifies the gain multiplier applied to all\n> > colour channels. This value cannot be lower than 1.0.\n> >\n> > + Setting this value means that it is now fixed and the AE algorithm may\n> > + not change it. Setting it back to zero returns it to the control of the\n> > + AE algorithm.\n> > +\n> > \\sa ExposureTime AeEnable\n> >\n> > + \\todo Consider how setting the analogue gain interacts with other AE\n> > + features, such as aperture, aperture/shutter priority modes etc.\n> > +\n> > - Brightness:\n> > type: float\n> > description: |\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 36E65BE176\n\tfor ;\n\tTue, 1 Dec 2020 19:55:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A76B363505;\n\tTue, 1 Dec 2020 20:55:16 +0100 (CET)","from mail-oi1-x244.google.com (mail-oi1-x244.google.com\n\t[IPv6:2607:f8b0:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E9EC63460\n\tfor ;\n\tTue, 1 Dec 2020 20:55:15 +0100 (CET)","by mail-oi1-x244.google.com with SMTP id v202so3080251oia.9\n\tfor ;\n\tTue, 01 Dec 2020 11:55:15 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"NaU7NyzK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=DPMjVMNgQ5NWTAoZenSOAllvmjJX5PRqx5r63xSslXk=;\n\tb=NaU7NyzKruHG2iMbfjCvhnwiwDtP1GGJiYMdl2iTxwKDzmyxXGqMn8OZt1e3DHyLZQ\n\tLEVUNFLLtX05CS0JEysXBCFoy3AwSmLqSrD0snGsavveWh/5pkmk7kto1dVrSqLVqQ3m\n\tPqeUa0gORV2l6anwpYvb+x7JDJ6IXkLxv9o6lw6gco1anSTWwVAl6asZrpBtGcEePYXc\n\tRvmFZ26Cf7RR8FH+2PWiHL2bX8BtoN0UY2OnhSmHi0H4aZOdQ5O+HedRiY0VI6ok2C4l\n\t9TyP9uUwhQdFroWkb6SW9uIBEk+a00MQMRI6855GL9nMPaWY5fBqi4JjSp47KoX1k1SL\n\tnY6Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=DPMjVMNgQ5NWTAoZenSOAllvmjJX5PRqx5r63xSslXk=;\n\tb=YAaYCnefBncLVkFC2Cr17VQDKV5NedoKWbsMmRp99FFpAQwZpG/QOqeX8DfebZ5g5O\n\tV3TxPelc3ex0Ycbu5XnwM6TFcx4gUmaSdN08zoJwk+9Sc5zvZ+qISbi9Os5xWq/7Qpcq\n\tbsPWlO7W2d59pacHvhX6wA6xhdOocye99rmidKOBqB/0R+Xj/LCGNhZS58L/ZjEwRI9p\n\tTOfclTNY9W6S6zjWwIz9juQFxvCrNpqXB1Zv3GhHfnV/HzEY/tleWP52dZXeSLA/L2Hq\n\tiUTNLj0bNF7HwQD1mMJ+pG40gDZIeJWdGC4fR8KtJ6aJUllscpcYWgFEFtx5MOEPW6bC\n\tw1nw==","X-Gm-Message-State":"AOAM533ktqdoa56l4UCF6p+WqMH2YDSZrbPmx8ka7JXmzVwDUBx6iAW+\n\tZnAt4qN8Mx6zUWaLSj3qRwacPDaiBihELOrNOz93XlQEKZhPzQ==","X-Google-Smtp-Source":"ABdhPJzTiFdFhGpUlQN2LLkR1FS/2pdIFL9/TyAy+waD4j9t95YVvV+3iCVyIeMqtG/NUdNnSVU7Blnbcur4ZNzqhkE=","X-Received":"by 2002:aca:5114:: with SMTP id\n\tf20mr2741684oib.107.1606852514025; \n\tTue, 01 Dec 2020 11:55:14 -0800 (PST)","MIME-Version":"1.0","References":"<20201201175536.11093-1-david.plowman@raspberrypi.com>\n\t<20201201175536.11093-5-david.plowman@raspberrypi.com>\n\t<20201201184142.GT4569@pendragon.ideasonboard.com>","In-Reply-To":"<20201201184142.GT4569@pendragon.ideasonboard.com>","From":"David Plowman ","Date":"Tue, 1 Dec 2020 19:55:02 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v4 4/5] libcamera: controls: Improve\n\tdocumentation for ExposureTime and AnalogueGain","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]