[{"id":21841,"web_url":"https://patchwork.libcamera.org/comment/21841/","msgid":"<20211221090039.nfrno56vmvzgvbaz@uno.localdomain>","date":"2021-12-21T09:00:39","subject":"Re: [libcamera-devel] [PATCH v2 6/6] android: Add CONTROL_MODE_OFF\n\tin template and static metadata","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Mon, Dec 20, 2021 at 05:26:29PM -0600, Paul Elder wrote:\n> Add CONTROL_MODE_OFF to the available control modes in static metadata,\n> if both AE off and AWB off are available. Also set CONTROL_MODE_OFF in\n> the manual template.\n>\n> Signed-off-by: Paul Elder \n> Reviewed-by: Kieran Bingham \n> ---\n> src/android/camera_capabilities.cpp | 18 +++++++++++++++++-\n> 1 file changed, 17 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index ea2aaf58..a2e42a5c 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -996,7 +996,17 @@ int CameraCapabilities::initializeStaticMetadata()\n> \tstaticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> \t\t\t\t awbLockAvailable);\n>\n> -\tchar availableControlModes = ANDROID_CONTROL_MODE_AUTO;\n> +\t/*\n> +\t * \\todo Get this from some combination of the available AE and AWB\n> +\t * modes\n> +\t */\n> +\tstd::vector availableControlModes = { ANDROID_CONTROL_MODE_AUTO };\n> +\tif (staticMetadata_->entryContains(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> +\t\t\t\t\t\t ANDROID_CONTROL_AE_MODE_OFF) &&\n> +\t staticMetadata_->entryContains(ANDROID_CONTROL_AWB_AVAILABLE_MODES,\n> +\t\t\t\t\t\t ANDROID_CONTROL_AWB_MODE_OFF)) {\n> +\t\tavailableControlModes.push_back(ANDROID_CONTROL_MODE_OFF);\n> +\t}\n\nThis makes sense\n\n> \tstaticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,\n> \t\t\t\t availableControlModes);\n>\n> @@ -1452,6 +1462,12 @@ std::unique_ptr CameraCapabilities::requestTemplateManual() cons\n> \tif (!manualTemplate)\n> \t\treturn nullptr;\n>\n> +\tif (staticMetadata_->entryContains(ANDROID_CONTROL_AVAILABLE_MODES,\n> +\t\t\t\t\t\t ANDROID_CONTROL_MODE_OFF)) {\n> +\t\tuint8_t mode = ANDROID_CONTROL_MODE_OFF;\n> +\t\tmanualTemplate->appendEntry(ANDROID_CONTROL_MODE, mode);\n> +\t}\n> +\n\nI would wait. The manual template is broken, it is created with the\npreview one (which has AE on and AWB set to auto). I wonder if this\ncreates more confusion than anything. We'll have to rewrite this part\nanyway.\n\nWhat I would be more interested in is knowing if reporting\nCONTROL_MODE in dynamic metadata is required, now that we potentially\nadvertise CONTROL_MODE_OFF too.\n\nThanks\n j\n\n> \treturn manualTemplate;\n> }\n>\n> --\n> 2.27.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 9E5A1BF415\n\tfor ;\n\tTue, 21 Dec 2021 08:59:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 08579608ED;\n\tTue, 21 Dec 2021 09:59:47 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DC98960113\n\tfor ;\n\tTue, 21 Dec 2021 09:59:44 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 5B7D7C000B;\n\tTue, 21 Dec 2021 08:59:44 +0000 (UTC)"],"Date":"Tue, 21 Dec 2021 10:00:39 +0100","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20211221090039.nfrno56vmvzgvbaz@uno.localdomain>","References":"<20211220232629.1485890-1-paul.elder@ideasonboard.com>\n\t<20211220232629.1485890-7-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211220232629.1485890-7-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] android: Add CONTROL_MODE_OFF\n\tin template and static metadata","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21862,"web_url":"https://patchwork.libcamera.org/comment/21862/","msgid":"<20211221225833.GJ2742@pyrite.rasen.tech>","date":"2021-12-21T22:58:33","subject":"Re: [libcamera-devel] [PATCH v2 6/6] android: Add CONTROL_MODE_OFF\n\tin template and static metadata","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Dec 21, 2021 at 10:00:39AM +0100, Jacopo Mondi wrote:\n> Hi Paul,\n> \n> On Mon, Dec 20, 2021 at 05:26:29PM -0600, Paul Elder wrote:\n> > Add CONTROL_MODE_OFF to the available control modes in static metadata,\n> > if both AE off and AWB off are available. Also set CONTROL_MODE_OFF in\n> > the manual template.\n> >\n> > Signed-off-by: Paul Elder \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > src/android/camera_capabilities.cpp | 18 +++++++++++++++++-\n> > 1 file changed, 17 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index ea2aaf58..a2e42a5c 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -996,7 +996,17 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \tstaticMetadata_->addEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> > \t\t\t\t awbLockAvailable);\n> >\n> > -\tchar availableControlModes = ANDROID_CONTROL_MODE_AUTO;\n> > +\t/*\n> > +\t * \\todo Get this from some combination of the available AE and AWB\n> > +\t * modes\n> > +\t */\n> > +\tstd::vector availableControlModes = { ANDROID_CONTROL_MODE_AUTO };\n> > +\tif (staticMetadata_->entryContains(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> > +\t\t\t\t\t\t ANDROID_CONTROL_AE_MODE_OFF) &&\n> > +\t staticMetadata_->entryContains(ANDROID_CONTROL_AWB_AVAILABLE_MODES,\n> > +\t\t\t\t\t\t ANDROID_CONTROL_AWB_MODE_OFF)) {\n> > +\t\tavailableControlModes.push_back(ANDROID_CONTROL_MODE_OFF);\n> > +\t}\n> \n> This makes sense\n> \n> > \tstaticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,\n> > \t\t\t\t availableControlModes);\n> >\n> > @@ -1452,6 +1462,12 @@ std::unique_ptr CameraCapabilities::requestTemplateManual() cons\n> > \tif (!manualTemplate)\n> > \t\treturn nullptr;\n> >\n> > +\tif (staticMetadata_->entryContains(ANDROID_CONTROL_AVAILABLE_MODES,\n> > +\t\t\t\t\t\t ANDROID_CONTROL_MODE_OFF)) {\n> > +\t\tuint8_t mode = ANDROID_CONTROL_MODE_OFF;\n> > +\t\tmanualTemplate->appendEntry(ANDROID_CONTROL_MODE, mode);\n> > +\t}\n> > +\n> \n> I would wait. The manual template is broken, it is created with the\n> preview one (which has AE on and AWB set to auto). I wonder if this\n> creates more confusion than anything. We'll have to rewrite this part\n> anyway.\n\nIt's not really broken. There's a guard against requesting a manual\ntemplate when manual sensor isn't supported, and our manual sensor\nvalidator currently never returns true. It will at the very end once\neverything for FULL is merged in.\n\nSo I think it's okay to put this in now, since it ought to be sufficient\n(perhaps with the exception of AF), so later when we do enable FULL (and\nthe required capability validators), it'll Just Work.\n\n> \n> What I would be more interested in is knowing if reporting\n> CONTROL_MODE in dynamic metadata is required, now that we potentially\n> advertise CONTROL_MODE_OFF too.\n\nWe always report it as an available request key.\n\nActually, I just realized I forgot to plumb this into the result\nmetadata. So this patch will be moved out of this series anyway.\n\n\nPaul\n\n> \n> > \treturn manualTemplate;\n> > }\n> >\n> > --\n> > 2.27.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 6BCF4BF415\n\tfor ;\n\tTue, 21 Dec 2021 22:58:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE4FB608F9;\n\tTue, 21 Dec 2021 23:58:40 +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 87702605A8\n\tfor ;\n\tTue, 21 Dec 2021 23:58:39 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2604:2d80:ad90:fb00:96fd:8874:873:6c16])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B210B881;\n\tTue, 21 Dec 2021 23:58:38 +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=\"DqsAv5U+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1640127519;\n\tbh=yJRv4qJVEY6QyIHYHgbrdQSdBELeby4aSHKI45L9Vf4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DqsAv5U+Zy2hyxaNRseUs7K0kR8O64lQPiDkFq/ygistFiSzmis/bit1DiDZU1usw\n\tPsdjnbrZxJ/qQLMk6JXET1TQjHFXl2Z2bZHdtOeuDKeWAUwyys2uFRqTOWCpK6WRJO\n\tFYC3H/GSFuS/+z01zQ3TGAeeDblqqIatN8hauuBw=","Date":"Tue, 21 Dec 2021 16:58:33 -0600","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi ","Message-ID":"<20211221225833.GJ2742@pyrite.rasen.tech>","References":"<20211220232629.1485890-1-paul.elder@ideasonboard.com>\n\t<20211220232629.1485890-7-paul.elder@ideasonboard.com>\n\t<20211221090039.nfrno56vmvzgvbaz@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20211221090039.nfrno56vmvzgvbaz@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] android: Add CONTROL_MODE_OFF\n\tin template and static metadata","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]