[{"id":18447,"web_url":"https://patchwork.libcamera.org/comment/18447/","msgid":"<YQSxDrTmpYkyoru0@pendragon.ideasonboard.com>","date":"2021-07-31T02:10:22","subject":"Re: [libcamera-devel] [PATCH v3 2/4] android: Disallow external\n\tlocation in HAL config","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Fri, Jul 30, 2021 at 04:31:52PM +0530, Umang Jain wrote:\n> Error out on any camera's location if set to \"external\",\n> in the HAL configuration file. The HAL configuration file\n> is only meant for the integrated cameras present on the system.\n\nThis makes sense, but I'd change the rationale a little bit. We may want\nto use the HAL configuration file in the future to also specify\ninformation for external cameras (not sure what the use cases would be,\nbut someone attaching an external camera to an Android device may want\nto tweak properties through the file). However, in that case, libcamera\nwill report the location as external already, so there's no need for the\nconfiguration file to override the location. Overriding the location to\nexternal would only be needed for cameras that libcamera report as front\nor back, and this wouldn't make sense.\n\nYou could write the commit message as\n\n\"Error out on any camera's location if set to \"external\", in the HAL\nconfiguration file. The HAL configuration file is meant to override the\nlocation property, and overriding an internal camera location to\nexternal doesn't make sense.\"\n\n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  src/android/camera_hal_config.cpp | 2 --\n>  1 file changed, 2 deletions(-)\n> \n> diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> index 833cf4ba..7126aba4 100644\n> --- a/src/android/camera_hal_config.cpp\n> +++ b/src/android/camera_hal_config.cpp\n> @@ -127,8 +127,6 @@ int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfig\n>  \t\tcameraConfigData->facing = CAMERA_FACING_FRONT;\n>  \telse if (location == \"back\")\n>  \t\tcameraConfigData->facing = CAMERA_FACING_BACK;\n> -\telse if (location == \"external\")\n> -\t\tcameraConfigData->facing = CAMERA_FACING_EXTERNAL;\n>  \telse\n>  \t\treturn -EINVAL;\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 EBFC4C322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 31 Jul 2021 02:10:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F1F3B687C3;\n\tSat, 31 Jul 2021 04:10:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 65031687BA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 31 Jul 2021 04:10:32 +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 DB2B42A3;\n\tSat, 31 Jul 2021 04:10:31 +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=\"eVq4FNvB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627697432;\n\tbh=ZY2GWtibKeYCZEBSnPdyWUEWis5MQl37XRmSwed9Xac=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eVq4FNvB2scPdycvZmmFyJGTBePh0vS5hMGPCP+w9b/8Zd68jFBoxM7XuXzQ5h+lR\n\tQ2KfIe+mqf1l2ToAXNrgCFS3ToycqhnRVicFP+NE1LubOV83vgSIB8QQfZH1XiXzz0\n\tM56+Wgc5nAqmtwKjSQVqbLtq1zgPbHKEIkXV2c8k=","Date":"Sat, 31 Jul 2021 05:10:22 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<YQSxDrTmpYkyoru0@pendragon.ideasonboard.com>","References":"<20210730110154.181370-1-umang.jain@ideasonboard.com>\n\t<20210730110154.181370-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210730110154.181370-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/4] android: Disallow external\n\tlocation in HAL config","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18483,"web_url":"https://patchwork.libcamera.org/comment/18483/","msgid":"<20210802070834.GU2167@pyrite.rasen.tech>","date":"2021-08-02T07:08:34","subject":"Re: [libcamera-devel] [PATCH v3 2/4] android: Disallow external\n\tlocation in HAL config","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Fri, Jul 30, 2021 at 04:31:52PM +0530, Umang Jain wrote:\n> Error out on any camera's location if set to \"external\",\n> in the HAL configuration file. The HAL configuration file\n> is only meant for the integrated cameras present on the system.\n> \n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> ---\n>  src/android/camera_hal_config.cpp | 2 --\n>  1 file changed, 2 deletions(-)\n> \n> diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> index 833cf4ba..7126aba4 100644\n> --- a/src/android/camera_hal_config.cpp\n> +++ b/src/android/camera_hal_config.cpp\n> @@ -127,8 +127,6 @@ int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfig\n>  \t\tcameraConfigData->facing = CAMERA_FACING_FRONT;\n>  \telse if (location == \"back\")\n>  \t\tcameraConfigData->facing = CAMERA_FACING_BACK;\n> -\telse if (location == \"external\")\n> -\t\tcameraConfigData->facing = CAMERA_FACING_EXTERNAL;\n>  \telse\n>  \t\treturn -EINVAL;\n>  \n> -- \n> 2.31.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 47344C3232\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  2 Aug 2021 07:08:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 071AF6026E;\n\tMon,  2 Aug 2021 09:08:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E38C86026E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  2 Aug 2021 09:08:41 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B167D4A3;\n\tMon,  2 Aug 2021 09:08:40 +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=\"eIAWHvS5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627888121;\n\tbh=cfWtH5fEooRUviDldNT5mZMAr07fEOVtb/Avf4LFvU0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eIAWHvS5RFsW/E3avTpBuqYAUzytHeTZH2YxX2WHTQMYbjNwONmSUZ0qnjCDT0W1J\n\tarRnLts/oEpLP7+0ceVQhTLt7+esDk9Jsw3beIFYFVraRy+05BQgSY2gfAQkoA3RjF\n\tqUr00WgSmNRVJy7jwhlg7ZA+RrFpFh+9CCjRHS3Y=","Date":"Mon, 2 Aug 2021 16:08:34 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<20210802070834.GU2167@pyrite.rasen.tech>","References":"<20210730110154.181370-1-umang.jain@ideasonboard.com>\n\t<20210730110154.181370-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210730110154.181370-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/4] android: Disallow external\n\tlocation in HAL config","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18493,"web_url":"https://patchwork.libcamera.org/comment/18493/","msgid":"<20210802165619.wemkgrj72gmins7v@uno.localdomain>","date":"2021-08-02T16:56:19","subject":"Re: [libcamera-devel] [PATCH v3 2/4] android: Disallow external\n\tlocation in HAL config","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Umang,\n\nOn Fri, Jul 30, 2021 at 04:31:52PM +0530, Umang Jain wrote:\n> Error out on any camera's location if set to \"external\",\n> in the HAL configuration file. The HAL configuration file\n> is only meant for the integrated cameras present on the system.\n>\n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n\nI think that's fair for the time being\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n> ---\n>  src/android/camera_hal_config.cpp | 2 --\n>  1 file changed, 2 deletions(-)\n>\n> diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp\n> index 833cf4ba..7126aba4 100644\n> --- a/src/android/camera_hal_config.cpp\n> +++ b/src/android/camera_hal_config.cpp\n> @@ -127,8 +127,6 @@ int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfig\n>  \t\tcameraConfigData->facing = CAMERA_FACING_FRONT;\n>  \telse if (location == \"back\")\n>  \t\tcameraConfigData->facing = CAMERA_FACING_BACK;\n> -\telse if (location == \"external\")\n> -\t\tcameraConfigData->facing = CAMERA_FACING_EXTERNAL;\n>  \telse\n>  \t\treturn -EINVAL;\n>\n> --\n> 2.31.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A0B93C3236\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon,  2 Aug 2021 16:55:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C0D0687EB;\n\tMon,  2 Aug 2021 18:55:34 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EE113687CB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  2 Aug 2021 18:55:32 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 58098100003;\n\tMon,  2 Aug 2021 16:55:32 +0000 (UTC)"],"Date":"Mon, 2 Aug 2021 18:56:19 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<20210802165619.wemkgrj72gmins7v@uno.localdomain>","References":"<20210730110154.181370-1-umang.jain@ideasonboard.com>\n\t<20210730110154.181370-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210730110154.181370-3-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 2/4] android: Disallow external\n\tlocation in HAL config","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]