[{"id":15090,"web_url":"https://patchwork.libcamera.org/comment/15090/","msgid":"","date":"2021-02-11T13:24:19","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your work.\n\nOn 2021-02-11 17:55:26 +0900, Paul Elder wrote:\n> Instead of choosing some arbitrary location for the sensor when its\n> location is unknown, set it explicitly to unknown.\n> \n> Signed-off-by: Paul Elder \n> ---\n> src/libcamera/camera_sensor.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index c9e8d49b..474055ba 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> \t\t\tbreak;\n> \t\t}\n> \t} else {\n> -\t\tpropertyValue = properties::CameraLocationExternal;\n> +\t\tpropertyValue = properties::CameraLocationUnknown;\n\nI wonder if it would not make more sens to just set the location to \nfront here? What additional use-case do we cover by adding the unkown \nlocation?\n\nIf we want to highlight we don't know where a camera is would it not be \nbetter to LOG() that we don't know but assume front. I'm thinking from \nan application point of view is it not kind of messy to have to deal \nwith a firmware description that is incomplete? I guess all users will \ndo what you do in this series for the HAL and default it to something \nelse.\n\nIf you do opt to keep the addition of CameraLocationUnknown you should \nalso update cam utility to handle the new location value.\n\n> \t}\n> \tproperties_.set(properties::Location, propertyValue);\n> \n> -- \n> 2.27.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 8DB52BD160\n\tfor ;\n\tThu, 11 Feb 2021 13:24:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F2A761664;\n\tThu, 11 Feb 2021 14:24:23 +0100 (CET)","from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com\n\t[IPv6:2a00:1450:4864:20::12a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A3010601B5\n\tfor ;\n\tThu, 11 Feb 2021 14:24:21 +0100 (CET)","by mail-lf1-x12a.google.com with SMTP id j19so8006801lfr.12\n\tfor ;\n\tThu, 11 Feb 2021 05:24:21 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tj186sm643505lfj.25.2021.02.11.05.24.20\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 11 Feb 2021 05:24:20 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"hjgj+Zvl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=QXEze0dUQwPMyLfJ/Q7qhytPLDRf+07LlpHttP0Kb7c=;\n\tb=hjgj+ZvlMg93EU5nErgUkvY4J3gxrsOLGM877pe7vBCsD3CUSR6P1EzkAmHp944aMX\n\tuKHwteg9uxuRvQOdZx9O1FAYO8pZ3oZyd2HrlSAagZhng7ieuMgKDOxaUfy66oeNvEI3\n\tAI1IyJxddILgkYIMhCOrCsUY0HVfVf4WyrI0Mj+NMrP+gvmCu423UeGjJi0i2w6lXwGI\n\tZmHkILYOBtCCgR2HIGvML5mN9XaPvi1rT13bSPBRuOdDvMRHYqQDgXF7jyVEQKwOR95l\n\tfglkU0ADtcHh3SnFaQ4E+BjAUnH89g4aEiHMQ2RDtLjyBJACLaA1v+zwIbQ/z8zTm3uJ\n\tbiFQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=QXEze0dUQwPMyLfJ/Q7qhytPLDRf+07LlpHttP0Kb7c=;\n\tb=R94vExEgAShX9JH+tDLyzJTekmIBGbBs+HURLmUDU3CUotN+xciRKeutkCCSYWi0nx\n\tEkZroHuDzoPGJ1C2/xENf8SAqH1Hjd4RNpTwreFc8LNYvksB5VN49k/j1MpL/mXD8M8J\n\tbe72v++uNIdVQ2Vhpei7Kmn4zvljUHPq2HigKvb4tQzPkFq+VtVQnRbE63/HV8Nf6DF/\n\tky6neP7C91RgAo0gscqv+WzV+p1pUnmtljNiTNyyNFJ+Z7RGel3zEn9ZegB6CfgAhrS5\n\tdT4I23X6NVJ18Z6MTipBAOBgdz45/cCKRCEyYOwWiyJR6aHw0s9urxbuVI1sCGweQT6y\n\td+UA==","X-Gm-Message-State":"AOAM531TF+C+Doc7gfBFef3C1b1Pku9cMcryHqN+syjb8Ttz92VRR85Z\n\tKcwZzn7LUEabLBSDym+IomzPHhPGi8ZRnQ==","X-Google-Smtp-Source":"ABdhPJwLxRFXNY7b5dVaQY37h5HcFUCG/n3Jx4PfLC5BmSv9MtIhGGIVxuGcurMa+c3nDKJZAGwG0Q==","X-Received":"by 2002:a19:4c07:: with SMTP id z7mr4421358lfa.455.1613049860882;\n\tThu, 11 Feb 2021 05:24:20 -0800 (PST)","Date":"Thu, 11 Feb 2021 14:24:19 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Paul Elder ","Message-ID":"","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210211085527.44667-3-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15092,"web_url":"https://patchwork.libcamera.org/comment/15092/","msgid":"","date":"2021-02-11T13:52:12","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Paul,\n\nOn 11/02/2021 08:55, Paul Elder wrote:\n> Instead of choosing some arbitrary location for the sensor when its\n> location is unknown, set it explicitly to unknown.\n\nThis probably confirms that we should always expect the property to be\ninitialised to something.\n\nI guess if this were initialised to zero by default, then this would\nbecome implicit, but being explicit is nice all the same.\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Paul Elder \n> ---\n> src/libcamera/camera_sensor.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index c9e8d49b..474055ba 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> \t\t\tbreak;\n> \t\t}\n> \t} else {\n> -\t\tpropertyValue = properties::CameraLocationExternal;\n> +\t\tpropertyValue = properties::CameraLocationUnknown;\n> \t}\n> \tproperties_.set(properties::Location, propertyValue);\n> \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 1CB5DBD160\n\tfor ;\n\tThu, 11 Feb 2021 13:52:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E06A063733;\n\tThu, 11 Feb 2021 14:52:17 +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 69291601B5\n\tfor ;\n\tThu, 11 Feb 2021 14:52:16 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 03A9241;\n\tThu, 11 Feb 2021 14:52:15 +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=\"koLpmbrW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613051536;\n\tbh=4jQefyrxliHUBM3Xe/GhWJocYrg4jdcQGVQ6T4+xCMQ=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=koLpmbrWgnS7oeOaYDgjAyOwTuxZg4+Dj9VdRoIEwWt/KXYcHCrMff6gGZUnokGnG\n\tJLCgvjS/sX+QKvv/ypvV2iT1i59CF/lI3OFleinWoq01eL48CKWfP9XB03oX9UedWw\n\tJZWgTjb6hPapDZemQ5MIf57kfUY+KmnpcSudfmv0=","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Thu, 11 Feb 2021 13:52:12 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20210211085527.44667-3-paul.elder@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15109,"web_url":"https://patchwork.libcamera.org/comment/15109/","msgid":"","date":"2021-02-11T21:44:44","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Thu, Feb 11, 2021 at 02:24:19PM +0100, Niklas Söderlund wrote:\n> On 2021-02-11 17:55:26 +0900, Paul Elder wrote:\n> > Instead of choosing some arbitrary location for the sensor when its\n> > location is unknown, set it explicitly to unknown.\n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > src/libcamera/camera_sensor.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index c9e8d49b..474055ba 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > \t\t\tbreak;\n> > \t\t}\n> > \t} else {\n> > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> \n> I wonder if it would not make more sens to just set the location to \n> front here? What additional use-case do we cover by adding the unkown \n> location?\n>\n> If we want to highlight we don't know where a camera is would it not be \n> better to LOG() that we don't know but assume front. I'm thinking from \n> an application point of view is it not kind of messy to have to deal \n> with a firmware description that is incomplete? I guess all users will \n> do what you do in this series for the HAL and default it to something \n> else.\n\nIsn't it better to let the application decide though, instead of\npretending we know ? The application could then decide how to deal with\nthe situation depending on its use cases, which are not known to\nlibcamera.\n\n> If you do opt to keep the addition of CameraLocationUnknown you should \n> also update cam utility to handle the new location value.\n\nYes, that should be part of this series.\n\nReviewed-by: Laurent Pinchart \n\n> > \t}\n> > \tproperties_.set(properties::Location, propertyValue);\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 9DEC6BD160\n\tfor ;\n\tThu, 11 Feb 2021 21:45:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0629C6374D;\n\tThu, 11 Feb 2021 22:45:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 102CA601B5\n\tfor ;\n\tThu, 11 Feb 2021 22:45:10 +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 75CE041;\n\tThu, 11 Feb 2021 22:45:09 +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=\"Sslv8D2K\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613079909;\n\tbh=gp92vchKeOiQWvp2ZUmogAbNvU6mRQC0G/GrD6dtBcM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Sslv8D2KnGfU8Ja4++TRmG6JF4pDvoi6/xUsKNbGHtvuIZZOtgjqkhiMgaR/SGApA\n\tY+lUNA2EVHWHzu2S2pr47KtKKXxrxRzSwEf/igARAwJuPE3EGftByDhlxV1/gp1Zfy\n\tiKmoSvPgXaLOZYz6JJ12jB6hOmT9qViI8BGdiRLQ=","Date":"Thu, 11 Feb 2021 23:44:44 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15111,"web_url":"https://patchwork.libcamera.org/comment/15111/","msgid":"","date":"2021-02-11T21:58:24","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2021-02-11 23:44:44 +0200, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Thu, Feb 11, 2021 at 02:24:19PM +0100, Niklas Söderlund wrote:\n> > On 2021-02-11 17:55:26 +0900, Paul Elder wrote:\n> > > Instead of choosing some arbitrary location for the sensor when its\n> > > location is unknown, set it explicitly to unknown.\n> > > \n> > > Signed-off-by: Paul Elder \n> > > ---\n> > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > \n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index c9e8d49b..474055ba 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > \t\t\tbreak;\n> > > \t\t}\n> > > \t} else {\n> > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> > \n> > I wonder if it would not make more sens to just set the location to \n> > front here? What additional use-case do we cover by adding the unkown \n> > location?\n> >\n> > If we want to highlight we don't know where a camera is would it not be \n> > better to LOG() that we don't know but assume front. I'm thinking from \n> > an application point of view is it not kind of messy to have to deal \n> > with a firmware description that is incomplete? I guess all users will \n> > do what you do in this series for the HAL and default it to something \n> > else.\n> \n> Isn't it better to let the application decide though, instead of\n> pretending we know ? The application could then decide how to deal with\n> the situation depending on its use cases, which are not known to\n> libcamera.\n\nI'd say it depends :-)\n\nDown the road I envision the camera location to always be mandatory. \nEither read from firmware or a platform configuration file. If this \nholds true I think adding a unknown location now is just pushing the \nproblem down the road. On the other hand if we think we will have \ncameras with an unknown location and see it as a valid use-case I think \nthis patch is correct.\n\nI'm however not convinced we have a good use-case for a camera with an \nunknown location. Do you have an example of such use-case?\n\n> \n> > If you do opt to keep the addition of CameraLocationUnknown you should \n> > also update cam utility to handle the new location value.\n> \n> Yes, that should be part of this series.\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > > \t}\n> > > \tproperties_.set(properties::Location, propertyValue);\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 63FE2BD162\n\tfor ;\n\tThu, 11 Feb 2021 21:58:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE9DF63754;\n\tThu, 11 Feb 2021 22:58:27 +0100 (CET)","from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com\n\t[IPv6:2a00:1450:4864:20::12e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C02AF63748\n\tfor ;\n\tThu, 11 Feb 2021 22:58:26 +0100 (CET)","by mail-lf1-x12e.google.com with SMTP id f1so10330881lfu.3\n\tfor ;\n\tThu, 11 Feb 2021 13:58:26 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ta2sm1066868ljm.135.2021.02.11.13.58.25\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 11 Feb 2021 13:58:25 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"uMvpabmW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=P4x5w3eCJGv0iM5tAwOAHJS9TQCbqTLmc3Zmzco4K1U=;\n\tb=uMvpabmWXHUAH2a8uIRgHzptQbypaylZTjwA/dQNBVBX6FqnMxi8tMYA7RWSQS3OI1\n\tGKSrSOiz8HEjJMd1xPQNoFb3DiZx85jC+6ECZVSz0aOMDT0zF1yKGCzB5dxdFhXTHvQ0\n\tHBp2apuIBHc0wGmAnqvQz946l9CeO48X4NIsKUIwxDd4vWcloZys9suf4xNXd7suY5Is\n\tDaGCY0PXBenf4h+ATsetWaPAXYeu5nKwLWRiZfRcBU6CzdBGK8IcHLepUfv9jz2/NqDP\n\tRl+q+Ib66tSWbzgRxWPi3kmis0gNhlt+bRQAdVxApGfPaJ52gn4VlPczdpS97WBE0IV5\n\tQ/4g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=P4x5w3eCJGv0iM5tAwOAHJS9TQCbqTLmc3Zmzco4K1U=;\n\tb=feBZvcvR1jJOdg4209COc9srUFquNJK+ZJm0w/C53BUrUarLxF4PT53fCOWJ0PwS+U\n\t6CjpqZ/viQm+gp9MEIXyFcwi51wgGw1CGJvJyHqb/8wfyjt5siMUIX+4VrM2BpGpM9fU\n\tqdyrxQ9U1y5/V8REhj4yqDNn+B3zj/X/2XQ3aAPHsIhMoQQ/KbuHuq4vhnV8nj3f/2MN\n\tVbchVe/hl5i0RfA9G53SgzDtF6H6+YkhrI19Ple6ndlcHKQv2N9TWq2lKlQ9odyrPLMx\n\tnCYzz7St8xHJqxvf2HhPTP+rIcnJBbEM+/V33B91zWvL/WeCe8ZIIMDTBtRM3wnyZK9M\n\tnwng==","X-Gm-Message-State":"AOAM533v8+0NIcIpe0kXxAyIo1KjkicmMpwCBtpyrfQifJfJsTfr8oV4\n\tagyO2w/ZxOymyMnUn4iNhg4LDw==","X-Google-Smtp-Source":"ABdhPJzejxr6zujrUYChSg5fhlEAGJXDdg2D1JlP47qRaUsqOB53oy/2kKjN6ZX72k7ld4PORoUMGw==","X-Received":"by 2002:a05:6512:2202:: with SMTP id\n\th2mr22524lfu.392.1613080706009; \n\tThu, 11 Feb 2021 13:58:26 -0800 (PST)","Date":"Thu, 11 Feb 2021 22:58:24 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15112,"web_url":"https://patchwork.libcamera.org/comment/15112/","msgid":"","date":"2021-02-11T22:26:02","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Feb 11, 2021 at 10:58:24PM +0100, Niklas Söderlund wrote:\n> On 2021-02-11 23:44:44 +0200, Laurent Pinchart wrote:\n> > On Thu, Feb 11, 2021 at 02:24:19PM +0100, Niklas Söderlund wrote:\n> > > On 2021-02-11 17:55:26 +0900, Paul Elder wrote:\n> > > > Instead of choosing some arbitrary location for the sensor when its\n> > > > location is unknown, set it explicitly to unknown.\n> > > > \n> > > > Signed-off-by: Paul Elder \n> > > > ---\n> > > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > \n> > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > > index c9e8d49b..474055ba 100644\n> > > > --- a/src/libcamera/camera_sensor.cpp\n> > > > +++ b/src/libcamera/camera_sensor.cpp\n> > > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > > \t\t\tbreak;\n> > > > \t\t}\n> > > > \t} else {\n> > > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> > > \n> > > I wonder if it would not make more sens to just set the location to \n> > > front here? What additional use-case do we cover by adding the unkown \n> > > location?\n> > >\n> > > If we want to highlight we don't know where a camera is would it not be \n> > > better to LOG() that we don't know but assume front. I'm thinking from \n> > > an application point of view is it not kind of messy to have to deal \n> > > with a firmware description that is incomplete? I guess all users will \n> > > do what you do in this series for the HAL and default it to something \n> > > else.\n> > \n> > Isn't it better to let the application decide though, instead of\n> > pretending we know ? The application could then decide how to deal with\n> > the situation depending on its use cases, which are not known to\n> > libcamera.\n> \n> I'd say it depends :-)\n> \n> Down the road I envision the camera location to always be mandatory. \n> Either read from firmware or a platform configuration file. If this \n> holds true I think adding a unknown location now is just pushing the \n> problem down the road. On the other hand if we think we will have \n> cameras with an unknown location and see it as a valid use-case I think \n> this patch is correct.\n> \n> I'm however not convinced we have a good use-case for a camera with an \n> unknown location. Do you have an example of such use-case?\n\nHow would you describe, for instance, a Raspberry Pi camera sitting on a\ndesk ?\n\n> > > If you do opt to keep the addition of CameraLocationUnknown you should \n> > > also update cam utility to handle the new location value.\n> > \n> > Yes, that should be part of this series.\n> > \n> > Reviewed-by: Laurent Pinchart \n> > \n> > > > \t}\n> > > > \tproperties_.set(properties::Location, propertyValue);\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 CDA07BD160\n\tfor ;\n\tThu, 11 Feb 2021 22:26:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5AB8163752;\n\tThu, 11 Feb 2021 23:26:29 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4CA35601B5\n\tfor ;\n\tThu, 11 Feb 2021 23:26:28 +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 A08A041;\n\tThu, 11 Feb 2021 23:26:27 +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=\"FPdbFVQJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613082387;\n\tbh=arIXxOQiPzwF9fHtexnLp6V2p282g3GMbHenXuIXTc8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FPdbFVQJJPChHh7qxwUlZvrxZOlLwNRzSB2ALiZKPQDT2q0HBx+I9Zekf8TTxrKZK\n\tKwKOf+SOXFoI6RyPGcDK/+7IzbHq4IuuawvfUOXGBWm2fwa7IcGtl6vxGhm8UWdtDy\n\tFaU5yYTvhO5BvuBpnSbIbhKiim8dPGPo2wS8brjo=","Date":"Fri, 12 Feb 2021 00:26:02 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15113,"web_url":"https://patchwork.libcamera.org/comment/15113/","msgid":"","date":"2021-02-11T22:35:58","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2021-02-12 00:26:02 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Thu, Feb 11, 2021 at 10:58:24PM +0100, Niklas Söderlund wrote:\n> > On 2021-02-11 23:44:44 +0200, Laurent Pinchart wrote:\n> > > On Thu, Feb 11, 2021 at 02:24:19PM +0100, Niklas Söderlund wrote:\n> > > > On 2021-02-11 17:55:26 +0900, Paul Elder wrote:\n> > > > > Instead of choosing some arbitrary location for the sensor when its\n> > > > > location is unknown, set it explicitly to unknown.\n> > > > > \n> > > > > Signed-off-by: Paul Elder \n> > > > > ---\n> > > > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > > \n> > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > > > index c9e8d49b..474055ba 100644\n> > > > > --- a/src/libcamera/camera_sensor.cpp\n> > > > > +++ b/src/libcamera/camera_sensor.cpp\n> > > > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > > > \t\t\tbreak;\n> > > > > \t\t}\n> > > > > \t} else {\n> > > > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> > > > \n> > > > I wonder if it would not make more sens to just set the location to \n> > > > front here? What additional use-case do we cover by adding the unkown \n> > > > location?\n> > > >\n> > > > If we want to highlight we don't know where a camera is would it not be \n> > > > better to LOG() that we don't know but assume front. I'm thinking from \n> > > > an application point of view is it not kind of messy to have to deal \n> > > > with a firmware description that is incomplete? I guess all users will \n> > > > do what you do in this series for the HAL and default it to something \n> > > > else.\n> > > \n> > > Isn't it better to let the application decide though, instead of\n> > > pretending we know ? The application could then decide how to deal with\n> > > the situation depending on its use cases, which are not known to\n> > > libcamera.\n> > \n> > I'd say it depends :-)\n> > \n> > Down the road I envision the camera location to always be mandatory. \n> > Either read from firmware or a platform configuration file. If this \n> > holds true I think adding a unknown location now is just pushing the \n> > problem down the road. On the other hand if we think we will have \n> > cameras with an unknown location and see it as a valid use-case I think \n> > this patch is correct.\n> > \n> > I'm however not convinced we have a good use-case for a camera with an \n> > unknown location. Do you have an example of such use-case?\n> \n> How would you describe, for instance, a Raspberry Pi camera sitting on a\n> desk ?\n\nI would describe it as external, just like a UVC camera at the end of a \ncable. If I then make a product of the RPi/UVC camera and mount it in a \nframe I would expect a firmware or other integration work (configuation \nfile?) addition to describe how I mounted it.\n\nIMHO that way libcamera applications can make a better deductions about \nuse-cases then if it has to handle the subtle differences between \nexternal and unknown.\n\n> \n> > > > If you do opt to keep the addition of CameraLocationUnknown you should \n> > > > also update cam utility to handle the new location value.\n> > > \n> > > Yes, that should be part of this series.\n> > > \n> > > Reviewed-by: Laurent Pinchart \n> > > \n> > > > > \t}\n> > > > > \tproperties_.set(properties::Location, propertyValue);\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 12455BD162\n\tfor ;\n\tThu, 11 Feb 2021 22:36:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7079563756;\n\tThu, 11 Feb 2021 23:36:01 +0100 (CET)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9158F601B5\n\tfor ;\n\tThu, 11 Feb 2021 23:36:00 +0100 (CET)","by mail-lf1-x136.google.com with SMTP id b2so10495339lfq.0\n\tfor ;\n\tThu, 11 Feb 2021 14:36:00 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tj16sm1061472ljg.32.2021.02.11.14.35.59\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 11 Feb 2021 14:35:59 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"Zu+uJvJu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=tjmnh6UgY35OPuxHqtOkvWG+jooFhgzTJUPL/vmnPEU=;\n\tb=Zu+uJvJukHIxEiipUxkZe+iezY1A2xhZgKosJ0ad2A/mqwRPigKsMtw7yMhjieILFV\n\ty6olO9I5ZjF95CGmpEbJWSsn4QG9S/tr9ROZyr8FQ4GdPfEWC/ffn0D9ZjDGaDI/YWNK\n\t9v7t9BzGEpQaj0MOvy/xMrfxRkN8UsyfIVhspLaXWoGgKKqWDz3WIzfiRirG5SHd5x53\n\tOES+0qHX8DO7We6edSVScHJZ1b/mpkeop0DolnTUnREOxX1Gup+CkuJTIOPuOF9ufTsb\n\tND35/SYQEdDT17tVpyMJhHQ1u3kv0vk/wz3Sp5IXJRGxE5FIZ02QatweUjm//3dG+Wd7\n\tq/1g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=tjmnh6UgY35OPuxHqtOkvWG+jooFhgzTJUPL/vmnPEU=;\n\tb=OfcBQe3H8/r5TvRXlpTlbgJ1Hp2wQdbVtHq5Wga6fMw9rWMSJaKv/sl9KY7L/dEzme\n\tklBwYFpJInn2f7w1lhw47hgtHL5C8CAXrtADwvcWWzNPoEm2FO172Kh8g903EFrpbzZL\n\tSZlp/08HnAeB+X6BD90A3GchDvHVRkD5U7vzrn64x8em7u12o9f9kEfUPPPsXxqDBJzc\n\tO8qZDdTNFGHl2B3zRLh91M7fK1yr4Sob2ogyfoaXAlBEsnQ9bMLiMIuP+Bl/2iLdRQ9K\n\tI91+8onrRyIQFwew9zg772IQyEdXv8CiolDm2rFq7t0n3giqK8vZuXaGSFPNUQGN7Ao4\n\tWFWQ==","X-Gm-Message-State":"AOAM5319jIZ+VP86XlQ1OJB1UhgXINdUyELVhsvbQA3EbOqBY3/CWRyc\n\txgL/VrkcCgG89Ke+z0gMnhMfOnh/iPNfeQ==","X-Google-Smtp-Source":"ABdhPJwBBDVbu/Ca3NSYlNT/xcWbONcw2o+dsf73iQsyzq1nWLFvrgiwT3EWfWjHDdFhOY/xrfyDaw==","X-Received":"by 2002:a19:208:: with SMTP id 8mr87826lfc.256.1613082959969;\n\tThu, 11 Feb 2021 14:35:59 -0800 (PST)","Date":"Thu, 11 Feb 2021 23:35:58 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15123,"web_url":"https://patchwork.libcamera.org/comment/15123/","msgid":"<20210212045251.GA45982@pyrite.rasen.tech>","date":"2021-02-12T04:52:51","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Feb 11, 2021 at 11:35:58PM +0100, Niklas Söderlund wrote:\n> Hi Laurent,\n> \n> On 2021-02-12 00:26:02 +0200, Laurent Pinchart wrote:\n> > Hi Niklas,\n> > \n> > On Thu, Feb 11, 2021 at 10:58:24PM +0100, Niklas Söderlund wrote:\n> > > On 2021-02-11 23:44:44 +0200, Laurent Pinchart wrote:\n> > > > On Thu, Feb 11, 2021 at 02:24:19PM +0100, Niklas Söderlund wrote:\n> > > > > On 2021-02-11 17:55:26 +0900, Paul Elder wrote:\n> > > > > > Instead of choosing some arbitrary location for the sensor when its\n> > > > > > location is unknown, set it explicitly to unknown.\n> > > > > > \n> > > > > > Signed-off-by: Paul Elder \n> > > > > > ---\n> > > > > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > > > \n> > > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > > > > index c9e8d49b..474055ba 100644\n> > > > > > --- a/src/libcamera/camera_sensor.cpp\n> > > > > > +++ b/src/libcamera/camera_sensor.cpp\n> > > > > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > > > > \t\t\tbreak;\n> > > > > > \t\t}\n> > > > > > \t} else {\n> > > > > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > > > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> > > > > \n> > > > > I wonder if it would not make more sens to just set the location to \n> > > > > front here? What additional use-case do we cover by adding the unkown \n> > > > > location?\n> > > > >\n> > > > > If we want to highlight we don't know where a camera is would it not be \n> > > > > better to LOG() that we don't know but assume front. I'm thinking from \n> > > > > an application point of view is it not kind of messy to have to deal \n> > > > > with a firmware description that is incomplete? I guess all users will \n> > > > > do what you do in this series for the HAL and default it to something \n> > > > > else.\n> > > > \n> > > > Isn't it better to let the application decide though, instead of\n> > > > pretending we know ? The application could then decide how to deal with\n> > > > the situation depending on its use cases, which are not known to\n> > > > libcamera.\n> > > \n> > > I'd say it depends :-)\n> > > \n> > > Down the road I envision the camera location to always be mandatory. \n> > > Either read from firmware or a platform configuration file. If this \n> > > holds true I think adding a unknown location now is just pushing the \n> > > problem down the road. On the other hand if we think we will have \n> > > cameras with an unknown location and see it as a valid use-case I think \n> > > this patch is correct.\n> > > \n> > > I'm however not convinced we have a good use-case for a camera with an \n> > > unknown location. Do you have an example of such use-case?\n> > \n> > How would you describe, for instance, a Raspberry Pi camera sitting on a\n> > desk ?\n> \n> I would describe it as external, just like a UVC camera at the end of a \n> cable. If I then make a product of the RPi/UVC camera and mount it in a \n> frame I would expect a firmware or other integration work (configuation \n> file?) addition to describe how I mounted it.\n> \n> IMHO that way libcamera applications can make a better deductions about \n> use-cases then if it has to handle the subtle differences between \n> external and unknown.\n\nYes, I agree that there really should be a firmware or configuration\nfile or /something/ that indicates the location. The issue that we're\nhaving is what if such configuration is absent. In such case I think\nit's better to have a distintion between some default value and the fact\nthat the location is unknown, and then the application can decide what\nto do in the latter case.\n\n> > \n> > > > > If you do opt to keep the addition of CameraLocationUnknown you should \n> > > > > also update cam utility to handle the new location value.\n\nAh, yes.\n\n\nThanks,\n\nPaul\n\n> > > > Yes, that should be part of this series.\n> > > > \n> > > > Reviewed-by: Laurent Pinchart \n> > > > \n> > > > > > \t}\n> > > > > > \tproperties_.set(properties::Location, propertyValue);\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 8F6B2BD162\n\tfor ;\n\tFri, 12 Feb 2021 04:53:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DAEA36375C;\n\tFri, 12 Feb 2021 05:53:00 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D33A063758\n\tfor ;\n\tFri, 12 Feb 2021 05:52:59 +0100 (CET)","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 E81FA8B5;\n\tFri, 12 Feb 2021 05:52:57 +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=\"MJJQTone\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613105579;\n\tbh=QPs1lq2+cVCpohvUvWUkVxJP8SMO8taSJoyQx9qzqxM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MJJQTone65dt0WanNbTQisxxSegzkjcvHHAthpT6peukSeZqSmgOh9Sl39ByuO1mo\n\tNDqamDrxTWpCPeBOc3KicXr4fGD3OvpJYQDgcQaleYdOYxSOjhpyBP2VjSof6VQXR8\n\tsxS+kh8kXtk8bJ3oLswUTABX3HEyXZZ/NIAntvYQ=","Date":"Fri, 12 Feb 2021 13:52:51 +0900","From":"paul.elder@ideasonboard.com","To":"Niklas =?iso-8859-1?q?S=F6derlund?= ","Message-ID":"<20210212045251.GA45982@pyrite.rasen.tech>","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15156,"web_url":"https://patchwork.libcamera.org/comment/15156/","msgid":"","date":"2021-02-12T15:59:18","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Feb 11, 2021 at 11:35:58PM +0100, Niklas Söderlund wrote:\n> On 2021-02-12 00:26:02 +0200, Laurent Pinchart wrote:\n> > On Thu, Feb 11, 2021 at 10:58:24PM +0100, Niklas Söderlund wrote:\n> > > On 2021-02-11 23:44:44 +0200, Laurent Pinchart wrote:\n> > > > On Thu, Feb 11, 2021 at 02:24:19PM +0100, Niklas Söderlund wrote:\n> > > > > On 2021-02-11 17:55:26 +0900, Paul Elder wrote:\n> > > > > > Instead of choosing some arbitrary location for the sensor when its\n> > > > > > location is unknown, set it explicitly to unknown.\n> > > > > > \n> > > > > > Signed-off-by: Paul Elder \n> > > > > > ---\n> > > > > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > > > \n> > > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > > > > index c9e8d49b..474055ba 100644\n> > > > > > --- a/src/libcamera/camera_sensor.cpp\n> > > > > > +++ b/src/libcamera/camera_sensor.cpp\n> > > > > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > > > > \t\t\tbreak;\n> > > > > > \t\t}\n> > > > > > \t} else {\n> > > > > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > > > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> > > > > \n> > > > > I wonder if it would not make more sens to just set the location to \n> > > > > front here? What additional use-case do we cover by adding the unkown \n> > > > > location?\n> > > > >\n> > > > > If we want to highlight we don't know where a camera is would it not be \n> > > > > better to LOG() that we don't know but assume front. I'm thinking from \n> > > > > an application point of view is it not kind of messy to have to deal \n> > > > > with a firmware description that is incomplete? I guess all users will \n> > > > > do what you do in this series for the HAL and default it to something \n> > > > > else.\n> > > > \n> > > > Isn't it better to let the application decide though, instead of\n> > > > pretending we know ? The application could then decide how to deal with\n> > > > the situation depending on its use cases, which are not known to\n> > > > libcamera.\n> > > \n> > > I'd say it depends :-)\n> > > \n> > > Down the road I envision the camera location to always be mandatory. \n> > > Either read from firmware or a platform configuration file. If this \n> > > holds true I think adding a unknown location now is just pushing the \n> > > problem down the road. On the other hand if we think we will have \n> > > cameras with an unknown location and see it as a valid use-case I think \n> > > this patch is correct.\n> > > \n> > > I'm however not convinced we have a good use-case for a camera with an \n> > > unknown location. Do you have an example of such use-case?\n> > \n> > How would you describe, for instance, a Raspberry Pi camera sitting on a\n> > desk ?\n> \n> I would describe it as external, just like a UVC camera at the end of a \n> cable. If I then make a product of the RPi/UVC camera and mount it in a \n> frame I would expect a firmware or other integration work (configuation \n> file?) addition to describe how I mounted it.\n\nYes, if it was integrated in a product, then it should definitely have a\nlocation set in the firmware.\n\n> IMHO that way libcamera applications can make a better deductions about \n> use-cases then if it has to handle the subtle differences between \n> external and unknown.\n\nWe're back at square one then, how do we tell the difference between a\ncamera that has no fixed/defined location, and a real external camera\nfrom an Android point of view ?\n\n> > > > > If you do opt to keep the addition of CameraLocationUnknown you should \n> > > > > also update cam utility to handle the new location value.\n> > > > \n> > > > Yes, that should be part of this series.\n> > > > \n> > > > Reviewed-by: Laurent Pinchart \n> > > > \n> > > > > > \t}\n> > > > > > \tproperties_.set(properties::Location, propertyValue);\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 67EAEBD160\n\tfor ;\n\tFri, 12 Feb 2021 15:59:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3735C63794;\n\tFri, 12 Feb 2021 16:59:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1D9AB63781\n\tfor ;\n\tFri, 12 Feb 2021 16:59:44 +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 9558571;\n\tFri, 12 Feb 2021 16:59:43 +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=\"ewt8j+Gp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613145583;\n\tbh=OEVq7w9CPgNb5xkMD3vlRKwdiaU8ogW8WbqTr6pLmFM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ewt8j+GpdlNgUOtwU0ej2zNcJvYxCX1x/4GeKRe6VKu4pqydR0fu3CgR7/9H09tCJ\n\tbpfICIANyghb02qdK1sw5MsELOTL6xIjCqXsGN+3OzRMW7HeGUHHKoRK1PJqiAdYJs\n\tz8wEeoGPlQER3ue5KrKKHQHdgKaay6megpcUpCzQ=","Date":"Fri, 12 Feb 2021 17:59:18 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15164,"web_url":"https://patchwork.libcamera.org/comment/15164/","msgid":"","date":"2021-02-13T10:37:56","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2021-02-12 17:59:18 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Thu, Feb 11, 2021 at 11:35:58PM +0100, Niklas Söderlund wrote:\n> > On 2021-02-12 00:26:02 +0200, Laurent Pinchart wrote:\n> > > On Thu, Feb 11, 2021 at 10:58:24PM +0100, Niklas Söderlund wrote:\n> > > > On 2021-02-11 23:44:44 +0200, Laurent Pinchart wrote:\n> > > > > On Thu, Feb 11, 2021 at 02:24:19PM +0100, Niklas Söderlund wrote:\n> > > > > > On 2021-02-11 17:55:26 +0900, Paul Elder wrote:\n> > > > > > > Instead of choosing some arbitrary location for the sensor when its\n> > > > > > > location is unknown, set it explicitly to unknown.\n> > > > > > > \n> > > > > > > Signed-off-by: Paul Elder \n> > > > > > > ---\n> > > > > > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > > > > \n> > > > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > > > > > index c9e8d49b..474055ba 100644\n> > > > > > > --- a/src/libcamera/camera_sensor.cpp\n> > > > > > > +++ b/src/libcamera/camera_sensor.cpp\n> > > > > > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > > > > > \t\t\tbreak;\n> > > > > > > \t\t}\n> > > > > > > \t} else {\n> > > > > > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > > > > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> > > > > > \n> > > > > > I wonder if it would not make more sens to just set the location to \n> > > > > > front here? What additional use-case do we cover by adding the unkown \n> > > > > > location?\n> > > > > >\n> > > > > > If we want to highlight we don't know where a camera is would it not be \n> > > > > > better to LOG() that we don't know but assume front. I'm thinking from \n> > > > > > an application point of view is it not kind of messy to have to deal \n> > > > > > with a firmware description that is incomplete? I guess all users will \n> > > > > > do what you do in this series for the HAL and default it to something \n> > > > > > else.\n> > > > > \n> > > > > Isn't it better to let the application decide though, instead of\n> > > > > pretending we know ? The application could then decide how to deal with\n> > > > > the situation depending on its use cases, which are not known to\n> > > > > libcamera.\n> > > > \n> > > > I'd say it depends :-)\n> > > > \n> > > > Down the road I envision the camera location to always be mandatory. \n> > > > Either read from firmware or a platform configuration file. If this \n> > > > holds true I think adding a unknown location now is just pushing the \n> > > > problem down the road. On the other hand if we think we will have \n> > > > cameras with an unknown location and see it as a valid use-case I think \n> > > > this patch is correct.\n> > > > \n> > > > I'm however not convinced we have a good use-case for a camera with an \n> > > > unknown location. Do you have an example of such use-case?\n> > > \n> > > How would you describe, for instance, a Raspberry Pi camera sitting on a\n> > > desk ?\n> > \n> > I would describe it as external, just like a UVC camera at the end of a \n> > cable. If I then make a product of the RPi/UVC camera and mount it in a \n> > frame I would expect a firmware or other integration work (configuation \n> > file?) addition to describe how I mounted it.\n> \n> Yes, if it was integrated in a product, then it should definitely have a\n> location set in the firmware.\n> \n> > IMHO that way libcamera applications can make a better deductions about \n> > use-cases then if it has to handle the subtle differences between \n> > external and unknown.\n> \n> We're back at square one then, how do we tell the difference between a\n> camera that has no fixed/defined location, and a real external camera\n> from an Android point of view ?\n\nI think that if we keep the Location property mandatory (as it is today) \nwe should in the end not default it to anything in core but simply \nrefuse to present any camera who does not provide its location. Either \nretrieved either from firmware, configuration file/script or maybe \ndefault in the pipeline handler. Maybe it make sens for UVC to default \nto external if no location is specified for example.\n\nIf we think adding an Unknown location is a good idea I would argue its \nnicer to make the Location property optional and simply not set if we \ndon't know the location.\n\nMy fear is that however we implement a solution to \"we don't know where \nthis camera is located\" that pushes the problem to applications the \nresult will be that most Linux applications simply won't bother and \nLocation will become a \"HAL only property\". Or worse application A will \ndefault Unknown to Front while application B will default it to \nExternal, creating confusion for users.\n\nIf I where a Linux camera application hacker and saw the Location \nproperty will either not be set or set to Unknown for most of the \nplatforms I target (Linux desktops/laptops) I would simply use the \ncamera model name in my UI.\n\nLooking at all the R-b tags this series have I understand I'm in \nminority :-) I will not block this series to be merged, but I think this \nwill be a missed opportunity to make Linux cameras a bit more \nuser-friendly for moms and pops as end users.\n\n> \n> > > > > > If you do opt to keep the addition of CameraLocationUnknown you should \n> > > > > > also update cam utility to handle the new location value.\n> > > > > \n> > > > > Yes, that should be part of this series.\n> > > > > \n> > > > > Reviewed-by: Laurent Pinchart \n> > > > > \n> > > > > > > \t}\n> > > > > > > \tproperties_.set(properties::Location, propertyValue);\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 DEF76BD160\n\tfor ;\n\tSat, 13 Feb 2021 10:38:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B11A637A3;\n\tSat, 13 Feb 2021 11:38:00 +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 B185360D37\n\tfor ;\n\tSat, 13 Feb 2021 11:37:58 +0100 (CET)","by mail-lj1-x229.google.com with SMTP id c17so968816ljn.0\n\tfor ;\n\tSat, 13 Feb 2021 02:37:58 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ty1sm1778098lfd.115.2021.02.13.02.37.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 13 Feb 2021 02:37:57 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"hNyX2hMT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=xgAQbGnPBCRacT1pKXVRDDyvXiQHH6jkVBLwntb/XOU=;\n\tb=hNyX2hMTwOhe8aXAieVQKwvebwm0+ohgFkNbEu2GtYHYUt+GfPgEVA8mfFyMBIBrxG\n\tX8WYHD62qPiI7ab+yUyPrlUsB+HEq7VlLbfxMumP37gPLnguqdwK0zabNiwvSRJRiFLT\n\toLuMkLKcKTOUPllsts8cOP5YYVYh+dc4Zf1pYU8gDLtLxuOiE5YZRh1QKHMfMnCLedxv\n\t7JbGYFoGhNUG9fA01oWX0VD7EenKP9K3G0JrgVmXMNjqkgfLoeJJzG0s4/c/EuwMILs/\n\tUy9oEHVEUGQz4hIySUBNuvASCF8F372hXypJuxao/Y0TXb3I7D6GT6zCX46g15nsZ8Zr\n\tGaLg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=xgAQbGnPBCRacT1pKXVRDDyvXiQHH6jkVBLwntb/XOU=;\n\tb=BZ7wEKtn5dQnIJU0Q8rcVnKTgYz3AmtmpvuzsE14hXz3+gYGUUIazEcFQDyEQmueKk\n\tzcQZVsw/3aJ18CFfnvWeGg1LB86Nm7bH28Qqva9qL90znRAscdYMjSiOUSrADBvQA1XU\n\tv523yIrQOQpSWM2tN62YxfIFaznAv/iW2rC+EmmoObLgxcgb3qHYXD2T6gP8InJ+lUC9\n\tEjLplk4R3iEI8HHzo98Tcp5qsGJt1UI7DNmzWlENPhGfaJpYB/sYyo0bylkX2vFhqsS0\n\thPOAjUbyiuiuMZGr5BnPScFwLGdcu1yPlGKhzW+HjPDAdmiMaZMu3X7XoBATsqKjQPdK\n\t23MQ==","X-Gm-Message-State":"AOAM5306vb9x5C0IRq0t4TAhIEXxyw6uVldJJh9x8uU0Fiexm31JjlTA\n\tMIzjHLFBa/cNnkMQ1TiLFppjYBXHLIdDdQ==","X-Google-Smtp-Source":"ABdhPJyIU71vXySisGVJS4TSw/g0cTHAJzHJ9jmhOz1djza6QgR7vuqQNXldo9+vRx3RxtqU1yRKSQ==","X-Received":"by 2002:a2e:8618:: with SMTP id\n\ta24mr3960546lji.419.1613212677936; \n\tSat, 13 Feb 2021 02:37:57 -0800 (PST)","Date":"Sat, 13 Feb 2021 11:37:56 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>\n\t\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15174,"web_url":"https://patchwork.libcamera.org/comment/15174/","msgid":"","date":"2021-02-16T03:02:10","subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Sat, Feb 13, 2021 at 11:37:56AM +0100, Niklas Söderlund wrote:\n> On 2021-02-12 17:59:18 +0200, Laurent Pinchart wrote:\n> > On Thu, Feb 11, 2021 at 11:35:58PM +0100, Niklas Söderlund wrote:\n> > > On 2021-02-12 00:26:02 +0200, Laurent Pinchart wrote:\n> > > > On Thu, Feb 11, 2021 at 10:58:24PM +0100, Niklas Söderlund wrote:\n> > > > > On 2021-02-11 23:44:44 +0200, Laurent Pinchart wrote:\n> > > > > > On Thu, Feb 11, 2021 at 02:24:19PM +0100, Niklas Söderlund wrote:\n> > > > > > > On 2021-02-11 17:55:26 +0900, Paul Elder wrote:\n> > > > > > > > Instead of choosing some arbitrary location for the sensor when its\n> > > > > > > > location is unknown, set it explicitly to unknown.\n> > > > > > > > \n> > > > > > > > Signed-off-by: Paul Elder \n> > > > > > > > ---\n> > > > > > > > src/libcamera/camera_sensor.cpp | 2 +-\n> > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > > > > > \n> > > > > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > > > > > > index c9e8d49b..474055ba 100644\n> > > > > > > > --- a/src/libcamera/camera_sensor.cpp\n> > > > > > > > +++ b/src/libcamera/camera_sensor.cpp\n> > > > > > > > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()\n> > > > > > > > \t\t\tbreak;\n> > > > > > > > \t\t}\n> > > > > > > > \t} else {\n> > > > > > > > -\t\tpropertyValue = properties::CameraLocationExternal;\n> > > > > > > > +\t\tpropertyValue = properties::CameraLocationUnknown;\n> > > > > > > \n> > > > > > > I wonder if it would not make more sens to just set the location to \n> > > > > > > front here? What additional use-case do we cover by adding the unkown \n> > > > > > > location?\n> > > > > > >\n> > > > > > > If we want to highlight we don't know where a camera is would it not be \n> > > > > > > better to LOG() that we don't know but assume front. I'm thinking from \n> > > > > > > an application point of view is it not kind of messy to have to deal \n> > > > > > > with a firmware description that is incomplete? I guess all users will \n> > > > > > > do what you do in this series for the HAL and default it to something \n> > > > > > > else.\n> > > > > > \n> > > > > > Isn't it better to let the application decide though, instead of\n> > > > > > pretending we know ? The application could then decide how to deal with\n> > > > > > the situation depending on its use cases, which are not known to\n> > > > > > libcamera.\n> > > > > \n> > > > > I'd say it depends :-)\n> > > > > \n> > > > > Down the road I envision the camera location to always be mandatory. \n> > > > > Either read from firmware or a platform configuration file. If this \n> > > > > holds true I think adding a unknown location now is just pushing the \n> > > > > problem down the road. On the other hand if we think we will have \n> > > > > cameras with an unknown location and see it as a valid use-case I think \n> > > > > this patch is correct.\n> > > > > \n> > > > > I'm however not convinced we have a good use-case for a camera with an \n> > > > > unknown location. Do you have an example of such use-case?\n> > > > \n> > > > How would you describe, for instance, a Raspberry Pi camera sitting on a\n> > > > desk ?\n> > > \n> > > I would describe it as external, just like a UVC camera at the end of a \n> > > cable. If I then make a product of the RPi/UVC camera and mount it in a \n> > > frame I would expect a firmware or other integration work (configuation \n> > > file?) addition to describe how I mounted it.\n> > \n> > Yes, if it was integrated in a product, then it should definitely have a\n> > location set in the firmware.\n> > \n> > > IMHO that way libcamera applications can make a better deductions about \n> > > use-cases then if it has to handle the subtle differences between \n> > > external and unknown.\n> > \n> > We're back at square one then, how do we tell the difference between a\n> > camera that has no fixed/defined location, and a real external camera\n> > from an Android point of view ?\n> \n> I think that if we keep the Location property mandatory (as it is today) \n> we should in the end not default it to anything in core but simply \n> refuse to present any camera who does not provide its location. Either \n> retrieved either from firmware, configuration file/script or maybe \n> default in the pipeline handler. Maybe it make sens for UVC to default \n> to external if no location is specified for example.\n\nFor external cameras, the External location makes sense :-)\n\n> If we think adding an Unknown location is a good idea I would argue its \n> nicer to make the Location property optional and simply not set if we \n> don't know the location.\n\nThat's a good point, there's little point in adding an Unknown location\nwhen we can convey the same meaning by not setting the property.\n\n> My fear is that however we implement a solution to \"we don't know where \n> this camera is located\" that pushes the problem to applications the \n> result will be that most Linux applications simply won't bother and \n> Location will become a \"HAL only property\". Or worse application A will \n> default Unknown to Front while application B will default it to \n> External, creating confusion for users.\n\nThat's the whole point of leaving the decision to applications though,\nthey have different use cases, and should thus be able to react\ndifferently to cameras having an unknown location.\n\n> If I where a Linux camera application hacker and saw the Location \n> property will either not be set or set to Unknown for most of the \n> platforms I target (Linux desktops/laptops) I would simply use the \n> camera model name in my UI.\n\nWe certainly want pipeline handlers to report camera locations. The two\nquestions that need to be answered today is what to do until we get\nthere (from that point of view I'd favour the least intrusive approach,\nwhich will be the easiest to revert or update once a known location\nbecomes mandatory), and if we should accept use cases that have no\nmeaningful way to select a location (a Raspberry Pi with a camera\nhanging off a flat cable for instance). The two questions may call for\ndifferent options (if we're unlucky).\n\n> Looking at all the R-b tags this series have I understand I'm in \n> minority :-) I will not block this series to be merged, but I think this \n> will be a missed opportunity to make Linux cameras a bit more \n> user-friendly for moms and pops as end users.\n\nNo no, I think we should continue the discussion before merging this.\n\n> > > > > > > If you do opt to keep the addition of CameraLocationUnknown you should \n> > > > > > > also update cam utility to handle the new location value.\n> > > > > > \n> > > > > > Yes, that should be part of this series.\n> > > > > > \n> > > > > > Reviewed-by: Laurent Pinchart \n> > > > > > \n> > > > > > > > \t}\n> > > > > > > > \tproperties_.set(properties::Location, propertyValue);\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 DAF2EBD1EF\n\tfor ;\n\tTue, 16 Feb 2021 03:02:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 44704637D1;\n\tTue, 16 Feb 2021 04:02:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C56A860106\n\tfor ;\n\tTue, 16 Feb 2021 04:02:36 +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 31ED78F2;\n\tTue, 16 Feb 2021 04:02:36 +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=\"EMnC/UP1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613444556;\n\tbh=kSA7pnr1PWbW4MQ1s5/wiEgWe3EDL0qp5AR2F847cWc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EMnC/UP1zrqyONeofzyxlEId2goNQj4r6adbdhhqqonHrWwc5cdOmMo9umZMlz/pB\n\tST7t0pvu0uVLerPh4A46qgTHJKpselKLL3++7PNi2qSRAqhgblqjHV1AFzb0QDJy3r\n\trcShWZWGUDx5QKyoX7K8/jciVtp4qkLy5tMJ3v4I=","Date":"Tue, 16 Feb 2021 05:02:10 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20210211085527.44667-1-paul.elder@ideasonboard.com>\n\t<20210211085527.44667-3-paul.elder@ideasonboard.com>\n\t\n\t\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set\n\tdefault sensor location to Unknown","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]