[{"id":20077,"web_url":"https://patchwork.libcamera.org/comment/20077/","msgid":"","date":"2021-10-06T01:35:16","subject":"Re: [libcamera-devel] [PATCH v2 06/17] android: capabilities:\n\tInitialize camera state when building properties","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, Sep 07, 2021 at 09:40:56PM +0200, Jacopo Mondi wrote:\n> Now that building the list of supported stream configuration requires\n> applying a configuration to the Camera, re-initialize the camera\n> controls by applying a configuration generated for the Viewfinder stream\n> role before building the list of static metadata.\n> \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Paul Elder \n> Reviewed-by: Umang Jain \n> ---\n> src/android/camera_capabilities.cpp | 22 +++++++++++++++++++---\n> 1 file changed, 19 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 4e82f12a904e..9bfc7ebc6334 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -394,11 +394,14 @@ int CameraCapabilities::initialize(std::shared_ptr camera,\n> \t}\n> \n> \tret = initializeStreamConfigurations();\n> -\tcamera_->release();\n> -\tif (ret)\n> +\tif (ret) {\n> +\t\tcamera_->release();\n> \t\treturn ret;\n> +\t}\n> \n> -\treturn initializeStaticMetadata();\n> +\tret = initializeStaticMetadata();\n> +\tcamera_->release();\n> +\treturn ret;\n> }\n> \n> std::vector\n> @@ -682,6 +685,19 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t\treturn -EINVAL;\n> \t}\n> \n> +\t/*\n> +\t * Generate and apply a new configuration for the Viewfinder role to\n> +\t * collect control limits and properties from a known state.\n> +\t */\n> +\tstd::unique_ptr cameraConfig =\n> +\t\tcamera_->generateConfiguration({ StreamRole::Viewfinder });\n\nQuite interestingly, we had no guarantee that the camera would be\nconfigured with the viewfinder role when reaching this point.\n\nReviewed-by: Laurent Pinchart \n\n> +\tint ret = camera_->configure(cameraConfig.get());\n> +\tif (ret) {\n> +\t\tLOG(HAL, Error) << \"Failed to initialize the camera state\";\n> +\t\tstaticMetadata_.reset();\n> +\t\treturn ret;\n> +\t}\n> +\n> \tconst ControlInfoMap &controlsInfo = camera_->controls();\n> \tconst ControlList &properties = camera_->properties();\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 443D9C3243\n\tfor ;\n\tWed, 6 Oct 2021 01:35:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B816F602DC;\n\tWed, 6 Oct 2021 03:35:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F92E602DC\n\tfor ;\n\tWed, 6 Oct 2021 03:35:24 +0200 (CEST)","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 CB2F2581;\n\tWed, 6 Oct 2021 03:35:23 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"BO82qdxN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1633484124;\n\tbh=3d1LCchTiNxnAAH9TJNjchYnYe3hrf9MWQvulTBvb9Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BO82qdxN1/7G0ifYbKlyyyr8QoIyOhPoS10Jsd1qQnIz5OPBmZOET8eWOGAOSiqy7\n\tr7BMvrkE7exrNzoS6h/BEBVBfXw+9KKcyv/CMYnUO5XHABO6dLvrbfSxAF5Gg6uEeW\n\tjCEQkMDcPa8QefMH+t8MyeqVey7uMOfzBHkY4C+o=","Date":"Wed, 6 Oct 2021 04:35:16 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210907194107.803730-1-jacopo@jmondi.org>\n\t<20210907194107.803730-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210907194107.803730-7-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 06/17] android: capabilities:\n\tInitialize camera state when building properties","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":20089,"web_url":"https://patchwork.libcamera.org/comment/20089/","msgid":"<20211006074430.kgak277wdtyvki2k@uno.localdomain>","date":"2021-10-06T07:44:30","subject":"Re: [libcamera-devel] [PATCH v2 06/17] android: capabilities:\n\tInitialize camera state when building properties","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Oct 06, 2021 at 04:35:16AM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Tue, Sep 07, 2021 at 09:40:56PM +0200, Jacopo Mondi wrote:\n> > Now that building the list of supported stream configuration requires\n> > applying a configuration to the Camera, re-initialize the camera\n> > controls by applying a configuration generated for the Viewfinder stream\n> > role before building the list of static metadata.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > Reviewed-by: Paul Elder \n> > Reviewed-by: Umang Jain \n> > ---\n> > src/android/camera_capabilities.cpp | 22 +++++++++++++++++++---\n> > 1 file changed, 19 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index 4e82f12a904e..9bfc7ebc6334 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -394,11 +394,14 @@ int CameraCapabilities::initialize(std::shared_ptr camera,\n> > \t}\n> >\n> > \tret = initializeStreamConfigurations();\n> > -\tcamera_->release();\n> > -\tif (ret)\n> > +\tif (ret) {\n> > +\t\tcamera_->release();\n> > \t\treturn ret;\n> > +\t}\n> >\n> > -\treturn initializeStaticMetadata();\n> > +\tret = initializeStaticMetadata();\n> > +\tcamera_->release();\n> > +\treturn ret;\n> > }\n> >\n> > std::vector\n> > @@ -682,6 +685,19 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \t\treturn -EINVAL;\n> > \t}\n> >\n> > +\t/*\n> > +\t * Generate and apply a new configuration for the Viewfinder role to\n> > +\t * collect control limits and properties from a known state.\n> > +\t */\n> > +\tstd::unique_ptr cameraConfig =\n> > +\t\tcamera_->generateConfiguration({ StreamRole::Viewfinder });\n>\n> Quite interestingly, we had no guarantee that the camera would be\n> configured with the viewfinder role when reaching this point.\n\nAnd, had done by Javier in the other initialize*() functions I should\nprobably check for if (!cameraConfig).\n\n>\n> Reviewed-by: Laurent Pinchart \n\nThanks\n j\n\n>\n> > +\tint ret = camera_->configure(cameraConfig.get());\n> > +\tif (ret) {\n> > +\t\tLOG(HAL, Error) << \"Failed to initialize the camera state\";\n> > +\t\tstaticMetadata_.reset();\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > \tconst ControlInfoMap &controlsInfo = camera_->controls();\n> > \tconst ControlList &properties = camera_->properties();\n> >\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 CB368C3243\n\tfor ;\n\tWed, 6 Oct 2021 07:43:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5CD0A691BB;\n\tWed, 6 Oct 2021 09:43:44 +0200 (CEST)","from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3723D691AB\n\tfor ;\n\tWed, 6 Oct 2021 09:43:43 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id C314D200008;\n\tWed, 6 Oct 2021 07:43:42 +0000 (UTC)"],"Date":"Wed, 6 Oct 2021 09:44:30 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20211006074430.kgak277wdtyvki2k@uno.localdomain>","References":"<20210907194107.803730-1-jacopo@jmondi.org>\n\t<20210907194107.803730-7-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 06/17] android: capabilities:\n\tInitialize camera state when building properties","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\" "}}]