[{"id":4739,"web_url":"https://patchwork.libcamera.org/comment/4739/","msgid":"<20200505140206.589f54ae@coco.lan>","date":"2020-05-05T12:02:06","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":47,"url":"https://patchwork.libcamera.org/api/people/47/","name":"Mauro Carvalho Chehab","email":"mchehab@kernel.org"},"content":"Em Fri, 17 Apr 2020 14:41:01 +0200\nJacopo Mondi escreveu:\n\n> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n> control. The newly added read-only control reports the camera device\n> mounting position.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n> 1 file changed, 32 insertions(+)\n> \n> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> index e39f84d2447f..01a9042d53a6 100644\n> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n> value down. A value of zero stops the motion if one is in progress\n> and has no effect otherwise.\n> \n> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n> + This read-only control describes the camera sensor location by reporting\n> + its mounting position on the device where the camera is installed. The\n> + control value is constant and not modifiable by software. This control is\n> + particularly meaningful for devices which have a well defined orientation,\n> + such as phones, laptops and portable devices since the camera location is\n> + expressed as a position relative to the device's intended usage orientation.\n> + For example, a camera sensor installed on the user-facing side of a phone,\n> + a tablet or a laptop device is said to be installed in the\n> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n> + opposite the front one are said to be installed in the\n> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n> + the device or attached in a way that allows them to move freely, such as\n> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n> + location.\n> +\n> +\n> +\n> +.. flat-table::\n> + :header-rows: 0\n> + :stub-columns: 0\n> +\n> + * - ``\n``\n> + - The camera sensor is located on the front side of the device.\n> + * - ``V4L2_LOCATION_BACK``\n> + - The camera sensor is located on the back side of the device.\n> + * - ``V4L2_LOCATION_EXTERNAL``\n> + - The camera sensor is not directly attached to the device and is\n> + freely movable.\n\nI guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\nfor two reasons:\n\n1) newer devices may all top of the line mobile devices now are coming\n with multiple camera sensors at the same side. So, just saying that\n the location is front or back is not enough. A location syscall would\n need have something more to better identify the location.\n It probably doesn't need to be something fancy, but, at least, on a\n device with 3 back sensors, I would call them as:\n\n\tV4L2_LOCATION_BACK_1\n\tV4L2_LOCATION_BACK_2\n\tV4L2_LOCATION_BACK_3\n\n And add some comment at the control documentation that would allow to\n uniquely number the other ones, like:\n\n\t\"when multiple sensors are present at the same side, sensors\n\t will be numbered considering the ``(x,y)`` coordinates of the center\n\t of each sensor, starting from the topmost, leftmost position.\n\n\t She first sensor will be the topmost sensor column at the leftmost\n\t side. The other sensors that will have the same ``y`` coordinate,\n\t counting from the left to the right, then increment the ``y`` and\n\t parse the next column again until all sensors are numbered.\"\n\n2) There are also some devices that has a movable sensor, that can either\n be taking a picture from the front or from the back, like those:\n\n\thttps://www.youtube.com/watch?v=br6G7MrkRUc\n\n On such case, the control should not be read-only, as one may need to\n change this control in order to select if a sensor would either be on\n FRONT or on BACK position.\n\n For such kind of sensors (when we start supporting them), we could \n for example call them like:\n\n\tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_1\n\tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_2\n\tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_1\n\tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_2\n\n And add rename the other definitions to:\n\n\tV4L2_LOCATION_FIXED_FRONT_1\n\tV4L2_LOCATION_FIXED_BACK_1\n\nOk, nobody has yet attempted to upstream code for such devices,\nso, we, for now, we don't need to add more than those 3 types.\n\nBut, IMO, we would change the sensors description in a way that it\nwould be easier to add support for more than one sensor per location\nin the future, like:\n\n\t* - ``V4L2_LOCATION_FIXED_FRONT_1``\n - The camera sensor is fixed, being the first sensor\n\t located on the front side of the device.\n\t* - ``V4L2_LOCATION_FIXED_BACK_1``\n\t - The camera sensor is fixed, being the first sensor\n\t located on the back side of the device.\n\t* - ``V4L2_LOCATION_EXTERNAL``\n - The camera sensor is not directly attached to the device\n\t and is freely movable.\n\n\t.. note:: Please submit a patch upstream if you need to have\n\t\t more than one sensor either at front or back position.\n\nThis would make a lot easier when someone upstream patches requiring \nto locate more than one sensor location, or to support flipping\nsensors.\n\nThanks,\nMauro","headers":{"Return-Path":"","Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[IPv6:2607:7c80:54:e::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD33761690\n\tfor ;\n\tTue, 5 May 2020 14:02:29 +0200 (CEST)","from ip5f5ad5c5.dynamic.kabel-deutschland.de ([95.90.213.197]\n\thelo=coco.lan)\n\tby bombadil.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat\n\tLinux)) id 1jVwH6-0007oX-QN; Tue, 05 May 2020 12:02:13 +0000"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=infradead.org; s=bombadil.20170209; h=Content-Transfer-Encoding:\n\tContent-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To:\n\tFrom:Date:Sender:Reply-To:Content-ID:Content-Description;\n\tbh=2B9EF7ZG6VLHghS2dcFJsnZMeN5hh2i7ksYJc+3yuLo=;\n\tb=cgIFq+/faqY7SWT5B1hvhRV0/Z\n\trY0ZZWwacq+tOQdMpWH5K6Fbd5TqhFiI9GU7x6itjRc+g3zA1EVbnAsICYWdS+McKJpUQxUgq8WNM\n\tCIre+30w2EaDcvCkPYccWHN+BbuJFgauAVod+Y3MjbcvdB5qwt3dWMPH/o8SqK8j2GkAmpS2l7Vdv\n\t4Coh7ZLZiY2Cx+dPDBs9bIfoSKosZWxa828zskLcvUp00E0N+RLJnERWpfn5KY2dT2sg+3MwDK3xV\n\tFeOpJjQ81DeSqX3fHpFPBlECYUMyDD6ns0EwC1UEGaZtywNqdu/vEW+uTpgxnkZ3ntcYvKKeMBAyF\n\tqQHMJXwQ==;","Date":"Tue, 5 May 2020 14:02:06 +0200","From":"Mauro Carvalho Chehab ","To":"Jacopo Mondi ","Cc":"Hans Verkuil , Sakari Ailus\n\t, Laurent Pinchart\n\t, tfiga@google.com, pavel@ucw.cz, \n\tlinux-media@vger.kernel.org (open list:MEDIA INPUT INFRASTRUCTURE \n\t(V4L/DVB)), libcamera-devel@lists.libcamera.org","Message-ID":"<20200505140206.589f54ae@coco.lan>","In-Reply-To":"<20200417124110.72313-3-jacopo@jmondi.org>","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>","X-Mailer":"Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-redhat-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Tue, 05 May 2020 12:02:35 -0000"}},{"id":4740,"web_url":"https://patchwork.libcamera.org/comment/4740/","msgid":"","date":"2020-05-05T12:21:38","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":43,"url":"https://patchwork.libcamera.org/api/people/43/","name":"Hans Verkuil","email":"hverkuil-cisco@xs4all.nl"},"content":"On 05/05/2020 14:02, Mauro Carvalho Chehab wrote:\n> Em Fri, 17 Apr 2020 14:41:01 +0200\n> Jacopo Mondi escreveu:\n> \n>> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n>> control. The newly added read-only control reports the camera device\n>> mounting position.\n>>\n>> Signed-off-by: Jacopo Mondi \n>> ---\n>> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n>> 1 file changed, 32 insertions(+)\n>>\n>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n>> index e39f84d2447f..01a9042d53a6 100644\n>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n>> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n>> value down. A value of zero stops the motion if one is in progress\n>> and has no effect otherwise.\n>> \n>> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n>> + This read-only control describes the camera sensor location by reporting\n>> + its mounting position on the device where the camera is installed. The\n>> + control value is constant and not modifiable by software. This control is\n>> + particularly meaningful for devices which have a well defined orientation,\n>> + such as phones, laptops and portable devices since the camera location is\n>> + expressed as a position relative to the device's intended usage orientation.\n>> + For example, a camera sensor installed on the user-facing side of a phone,\n>> + a tablet or a laptop device is said to be installed in the\n>> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n>> + opposite the front one are said to be installed in the\n>> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n>> + the device or attached in a way that allows them to move freely, such as\n>> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n>> + location.\n>> +\n>> +\n>> +\n>> +.. flat-table::\n>> + :header-rows: 0\n>> + :stub-columns: 0\n>> +\n>> + * - ``\n> ``\n>> + - The camera sensor is located on the front side of the device.\n>> + * - ``V4L2_LOCATION_BACK``\n>> + - The camera sensor is located on the back side of the device.\n>> + * - ``V4L2_LOCATION_EXTERNAL``\n>> + - The camera sensor is not directly attached to the device and is\n>> + freely movable.\n> \n> I guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\n> for two reasons:\n> \n> 1) newer devices may all top of the line mobile devices now are coming\n> with multiple camera sensors at the same side. So, just saying that\n> the location is front or back is not enough. A location syscall would\n> need have something more to better identify the location.\n> It probably doesn't need to be something fancy, but, at least, on a\n> device with 3 back sensors, I would call them as:\n> \n> \tV4L2_LOCATION_BACK_1\n> \tV4L2_LOCATION_BACK_2\n> \tV4L2_LOCATION_BACK_3\n> \n> And add some comment at the control documentation that would allow to\n> uniquely number the other ones, like:\n> \n> \t\"when multiple sensors are present at the same side, sensors\n> \t will be numbered considering the ``(x,y)`` coordinates of the center\n> \t of each sensor, starting from the topmost, leftmost position.\n> \n> \t She first sensor will be the topmost sensor column at the leftmost\n> \t side. The other sensors that will have the same ``y`` coordinate,\n> \t counting from the left to the right, then increment the ``y`` and\n> \t parse the next column again until all sensors are numbered.\"\n\nI think this isn't a good idea. In most cases you do not care about this.\n\nAnd if you do care about this, then wouldn't it be better to do that through\na new control where you provide the precise coordinates in e.g. mm?\n\nBACK_1/2/3 really doesn't tell you anything other than that there are three\nsensors on the back, but we knew that already.\n\nIf we need support for the precise location in the future, then let's do that\nright and not try to shoehorn into something that wasn't meant for it.\n\n> \n> 2) There are also some devices that has a movable sensor, that can either\n> be taking a picture from the front or from the back, like those:\n> \n> \thttps://www.youtube.com/watch?v=br6G7MrkRUc\n> \n> On such case, the control should not be read-only, as one may need to\n> change this control in order to select if a sensor would either be on\n> FRONT or on BACK position.\n> \n> For such kind of sensors (when we start supporting them), we could \n> for example call them like:\n> \n> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_1\n> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_2\n> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_1\n> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_2\n\nI don't like this. If the driver can tell when the position changes, then it\ncan update the control's value (it's still read-only because userspace\ncan't write to it, but that doesn't mean it can't be updated). So there is\nno need to call it 'MOVABLE', you just report the correct location. And with\nQUERYMENU you can tell that it is movable since multiple possible locations\nare reported (BACK and FRONT in this example). If it is fixed, then QUERYMENU\nwill report only a single location.\n\nThis might have some consequences for the DT bindings, though. Not sure\nhow to represent this there.\n\nIf the driver cannot tell what the position is, then it makes no sense for\nthe driver to expose this location control since it clearly is something that\nhas to be hardcoded in userspace. I.e., there is no point for userspace to\nwrite to the control and then read back what it wrote :-)\n\nSo I disagree that there is a need for FIXED vs MOVABLE, this can be\nrepresented nicely with the current proposal.\n\nRegards,\n\n\tHans\n\n> \n> And add rename the other definitions to:\n> \n> \tV4L2_LOCATION_FIXED_FRONT_1\n> \tV4L2_LOCATION_FIXED_BACK_1\n> \n> Ok, nobody has yet attempted to upstream code for such devices,\n> so, we, for now, we don't need to add more than those 3 types.\n> \n> But, IMO, we would change the sensors description in a way that it\n> would be easier to add support for more than one sensor per location\n> in the future, like:\n> \n> \t* - ``V4L2_LOCATION_FIXED_FRONT_1``\n> - The camera sensor is fixed, being the first sensor\n> \t located on the front side of the device.\n> \t* - ``V4L2_LOCATION_FIXED_BACK_1``\n> \t - The camera sensor is fixed, being the first sensor\n> \t located on the back side of the device.\n> \t* - ``V4L2_LOCATION_EXTERNAL``\n> - The camera sensor is not directly attached to the device\n> \t and is freely movable.\n> \n> \t.. note:: Please submit a patch upstream if you need to have\n> \t\t more than one sensor either at front or back position.\n> \n> This would make a lot easier when someone upstream patches requiring \n> to locate more than one sensor location, or to support flipping\n> sensors.\n> \n> Thanks,\n> Mauro\n>","headers":{"Return-Path":"","Received":["from lb1-smtp-cloud8.xs4all.net (lb1-smtp-cloud8.xs4all.net\n\t[194.109.24.21])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BA29B61690\n\tfor ;\n\tTue, 5 May 2020 14:21:43 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud8.xs4all.net with ESMTPA\n\tid VwZujUiXihEkrVwZxjxgSF; Tue, 05 May 2020 14:21:43 +0200"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"I7SKt9ox\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1588681303; bh=VSlWp6YY5vmqNA0KvHBjNJgstMab0HWtn4YGAMMnP+E=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=I7SKt9ox17hU5a+tH7Yal7vNqAYfj9JJ9N4wF098LQRPS6e0FiaTju9InlpzZ9vS7\n\twmYaIxHHN9CDTFQdbQz9b5f5ziElG+i6HhjTR+w/IgO/m5CLjDlVMUidMpQ/TJ73El\n\tiaBhqN/cPkRfwX94IkCFScWfsxZM1gZsS+Nk/ilHy1q4sHGva5zkjSHJ/MTcmCbpdS\n\t9aVU4eVvK+252myKcZb5r8nhWN4LGE6vyuNsrXxcixZ+rxorZuxGYQx4+xlKO7w7AZ\n\t/uuKSJMlJMviP78+mwvyxxH7WTnOnsAx3owuiQehXXwiUBCIbsEqeSaq2CBhbtSa3W\n\tE9A1JmTaTGDVw==","To":"Mauro Carvalho Chehab ,\n\tJacopo Mondi ","Cc":"Sakari Ailus ,\n\tLaurent Pinchart , tfiga@google.com,\n\tpavel@ucw.cz, \"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>","From":"Hans Verkuil ","Message-ID":"","Date":"Tue, 5 May 2020 14:21:38 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.5.0","MIME-Version":"1.0","In-Reply-To":"<20200505140206.589f54ae@coco.lan>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfJ2jDJbsB6hOechOyjEE0pn5Kt1dWq8hx0v9oUdDo5YhMH05mQ2JPsHEzse0mXMEEOLAK6GEUcZvRC4CqmgnwxDB0BJIGMlkQv6jc59LFr00cgmv85+G\n\tyZJMgwtYXXceXLMSFMCTOoVCacwSr6QgXc03xH3L/L64mpCljAit5ptrfkBEjQZRWM7khWgCQaLxi9OweUkQNPiq5E2akyQSecRvEa1zESUDCdFKRswXaCwa\n\tRhVk9vyxOhUFIaqtawVLavxQPIMMjU3Pyc5tN3krPGMtP1VLC3lf2OFDAsa160N7XYVepOc4KDn3s9G6nwojq2SfypCr0hsd9oiPS0ZP+e4SK3YHcds1qXRS\n\tFTVnmWTauYIdVuIl24KurDi8K4vsau1J3YEmFMdZ1eMToiCkzhnyzkw37IeIcJt5YpywTJ/qnwjLEWfQeNXSX01MOzQcpA==","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Tue, 05 May 2020 12:21:44 -0000"}},{"id":4742,"web_url":"https://patchwork.libcamera.org/comment/4742/","msgid":"<20200505165826.1ce8bb0d@coco.lan>","date":"2020-05-05T14:58:26","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":47,"url":"https://patchwork.libcamera.org/api/people/47/","name":"Mauro Carvalho Chehab","email":"mchehab@kernel.org"},"content":"Em Tue, 5 May 2020 14:21:38 +0200\nHans Verkuil escreveu:\n\n> On 05/05/2020 14:02, Mauro Carvalho Chehab wrote:\n> > Em Fri, 17 Apr 2020 14:41:01 +0200\n> > Jacopo Mondi escreveu:\n> > \n> >> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n> >> control. The newly added read-only control reports the camera device\n> >> mounting position.\n> >>\n> >> Signed-off-by: Jacopo Mondi \n> >> ---\n> >> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n> >> 1 file changed, 32 insertions(+)\n> >>\n> >> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >> index e39f84d2447f..01a9042d53a6 100644\n> >> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n> >> value down. A value of zero stops the motion if one is in progress\n> >> and has no effect otherwise.\n> >> \n> >> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n> >> + This read-only control describes the camera sensor location by reporting\n> >> + its mounting position on the device where the camera is installed. The\n> >> + control value is constant and not modifiable by software. This control is\n> >> + particularly meaningful for devices which have a well defined orientation,\n> >> + such as phones, laptops and portable devices since the camera location is\n> >> + expressed as a position relative to the device's intended usage orientation.\n> >> + For example, a camera sensor installed on the user-facing side of a phone,\n> >> + a tablet or a laptop device is said to be installed in the\n> >> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n> >> + opposite the front one are said to be installed in the\n> >> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n> >> + the device or attached in a way that allows them to move freely, such as\n> >> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n> >> + location.\n> >> +\n> >> +\n> >> +\n> >> +.. flat-table::\n> >> + :header-rows: 0\n> >> + :stub-columns: 0\n> >> +\n> >> + * - `` \n> > `` \n> >> + - The camera sensor is located on the front side of the device.\n> >> + * - ``V4L2_LOCATION_BACK``\n> >> + - The camera sensor is located on the back side of the device.\n> >> + * - ``V4L2_LOCATION_EXTERNAL``\n> >> + - The camera sensor is not directly attached to the device and is\n> >> + freely movable. \n> > \n> > I guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\n> > for two reasons:\n> > \n> > 1) newer devices may all top of the line mobile devices now are coming\n> > with multiple camera sensors at the same side. So, just saying that\n> > the location is front or back is not enough. A location syscall would\n> > need have something more to better identify the location.\n> > It probably doesn't need to be something fancy, but, at least, on a\n> > device with 3 back sensors, I would call them as:\n> > \n> > \tV4L2_LOCATION_BACK_1\n> > \tV4L2_LOCATION_BACK_2\n> > \tV4L2_LOCATION_BACK_3\n> > \n> > And add some comment at the control documentation that would allow to\n> > uniquely number the other ones, like:\n> > \n> > \t\"when multiple sensors are present at the same side, sensors\n> > \t will be numbered considering the ``(x,y)`` coordinates of the center\n> > \t of each sensor, starting from the topmost, leftmost position.\n> > \n> > \t She first sensor will be the topmost sensor column at the leftmost\n> > \t side. The other sensors that will have the same ``y`` coordinate,\n> > \t counting from the left to the right, then increment the ``y`` and\n> > \t parse the next column again until all sensors are numbered.\" \n> \n> I think this isn't a good idea. In most cases you do not care about this.\n\nTrue, because on most cases, the userspace is hardcoded to open, let's say,\nvideo0 for the front sensor or video1 for the back sensor.\n\nThis control only makes sense if the userspace is generic enough to accept\nsensors on different positions, identifying them at runtime.\n\nWith the current proposal, userspace can only work with 2 sensors, as, if\nthere's a third sensor, userspace won't know how to pick the right one.\n\nFor instance, let's assume a car with 4 sensors, one on each side of\nthe car (right, front); (left, front); (right; back); (left; back).\n\nWith the current proposal, userspace can't do anything if it wants\nto identify the (right, back) camera.\n\n> And if you do care about this, then wouldn't it be better to do that through\n> a new control where you provide the precise coordinates in e.g. mm?\n> \n> BACK_1/2/3 really doesn't tell you anything other than that there are three\n> sensors on the back, but we knew that already.\n\nNo, if we define some criteria about how sensors should be accounted for\n(something similar to the text I drafted), the location will be defined.\n\nWith the above text, for example, a device with 3 sensors horizontally\naligned, the arrangement will be:\n\n- sensor 1 is on the left;\n- sensor 2 in the middle;\n- sensor 3 is on the right.\n\nOk, I agree that writing a text with such criteria sucks, and maybe\njust numbering from 1 to n may not be the best thing to do. Yet,\nadding coordinates in mm would be just too much information, IMHO.\n\n> If we need support for the precise location in the future, then let's do that\n> right and not try to shoehorn into something that wasn't meant for it.\n\nAssuming that all the problems we have so far are to support devices with\n2 sensors, by the time we add support for a third sensor, we'll end having\na new ioctl for the same thing: to specify the sensors locations.\n\nWe know the drill: having two controls for the same thing makes userspace\nmore complex and will require backward-compatibility code at the kernel\nand at userspace. That's what I want to avoid.\n\nI'm open to other suggestions that won't limit the usage of this control\nfor devices with just (up to) two sensors.\n\n> \n> > \n> > 2) There are also some devices that has a movable sensor, that can either\n> > be taking a picture from the front or from the back, like those:\n> > \n> > \thttps://www.youtube.com/watch?v=br6G7MrkRUc\n> > \n> > On such case, the control should not be read-only, as one may need to\n> > change this control in order to select if a sensor would either be on\n> > FRONT or on BACK position.\n> > \n> > For such kind of sensors (when we start supporting them), we could \n> > for example call them like:\n> > \n> > \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_1\n> > \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_2\n> > \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_1\n> > \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_2 \n> \n> I don't like this. If the driver can tell when the position changes, then it\n> can update the control's value (it's still read-only because userspace\n> can't write to it, but that doesn't mean it can't be updated).\n\nWhy userspace can't set it? I mean, if the camera is movable, it\nshould be up to the application to select the sensor between FRONT\nand BACK.\n\nBtw, this is a case where I clearly see value on this ioctl: all cameras\nwith flippable sensors need a control to switch the sensor's position,\neven if the sensor device is hardcoded on some application.\n\n> So there is\n> no need to call it 'MOVABLE', you just report the correct location. And with\n> QUERYMENU you can tell that it is movable since multiple possible locations\n> are reported (BACK and FRONT in this example). If it is fixed, then QUERYMENU\n> will report only a single location.\n> \n> This might have some consequences for the DT bindings, though. Not sure\n> how to represent this there.\n\nI guess DT should contain the default value when the device is turned\noff. \n\n> If the driver cannot tell what the position is, then it makes no sense for\n> the driver to expose this location control since it clearly is something that\n> has to be hardcoded in userspace. I.e., there is no point for userspace to\n> write to the control and then read back what it wrote :-)\n\nActually there is. When you command a device to switch position, it may\ntake some time to move the sensor, and such operation may even fail.\n\nSo, reading back the position is probably mandatory.\n\nThat reminds that it may actually have a third position, to warn\nthat the sensor was blocked.\n\nAlso, some flip sensors may have another position (a \"closed\"\nposition).\n\n> So I disagree that there is a need for FIXED vs MOVABLE, this can be\n> represented nicely with the current proposal.\n\nThanks,\nMauro","headers":{"Return-Path":"","Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[IPv6:2607:7c80:54:e::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A6AA61690\n\tfor ;\n\tTue, 5 May 2020 16:58:44 +0200 (CEST)","from ip5f5ad5c5.dynamic.kabel-deutschland.de ([95.90.213.197]\n\thelo=coco.lan)\n\tby bombadil.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat\n\tLinux)) id 1jVz1j-0007KT-HA; Tue, 05 May 2020 14:58:31 +0000"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=infradead.org\n\theader.i=@infradead.org\n\theader.b=\"cGKUcs6t\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=infradead.org; s=bombadil.20170209; h=Content-Transfer-Encoding:\n\tContent-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To:\n\tFrom:Date:Sender:Reply-To:Content-ID:Content-Description;\n\tbh=dJ7YPxbG/35TsFosSCkDdIiJQ2l+1QDAnondLu+lvRA=;\n\tb=cGKUcs6ttWG13oWo07vW6bUV5B\n\tuN5JdpPO5QdY7jLp6VhIUL9du7JcesGMPa5E4AoNMpGPjw1rK4NO4Gk3yPbK7P1k/u4zci/ezzat9\n\tjNoayE1THObRo1+85lLWOlsWx7Gk2O+IwAxJ6TuPQWPTcfE7Zz/TvBjn3xfXi0lIFB052ir3ps2ea\n\tPeF1Y+tM2/LAj+QNrqiZCi4Vgvc+Xd2FSjbAQhmY54JobPZG2AwISGYcagHkEGuZwpf3JYeaDI74q\n\tvHY1QMpQhvxSsOeZcbUeCPjYI/r6OegyABOuegcNx2Eg6cNlZPEQYaOv/DaAAXo6AD3hHLZQ2yRNx\n\tCnIFsI3w==;","Date":"Tue, 5 May 2020 16:58:26 +0200","From":"Mauro Carvalho Chehab ","To":"Hans Verkuil ","Cc":"Jacopo Mondi , Sakari Ailus\n\t, Laurent Pinchart\n\t, tfiga@google.com, pavel@ucw.cz,\n\t\"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","Message-ID":"<20200505165826.1ce8bb0d@coco.lan>","In-Reply-To":"","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t","X-Mailer":"Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-redhat-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Tue, 05 May 2020 14:58:45 -0000"}},{"id":4745,"web_url":"https://patchwork.libcamera.org/comment/4745/","msgid":"","date":"2020-05-06T08:04:39","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":43,"url":"https://patchwork.libcamera.org/api/people/43/","name":"Hans Verkuil","email":"hverkuil-cisco@xs4all.nl"},"content":"On 05/05/2020 16:58, Mauro Carvalho Chehab wrote:\n> Em Tue, 5 May 2020 14:21:38 +0200\n> Hans Verkuil escreveu:\n> \n>> On 05/05/2020 14:02, Mauro Carvalho Chehab wrote:\n>>> Em Fri, 17 Apr 2020 14:41:01 +0200\n>>> Jacopo Mondi escreveu:\n>>> \n>>>> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n>>>> control. The newly added read-only control reports the camera device\n>>>> mounting position.\n>>>>\n>>>> Signed-off-by: Jacopo Mondi \n>>>> ---\n>>>> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n>>>> 1 file changed, 32 insertions(+)\n>>>>\n>>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n>>>> index e39f84d2447f..01a9042d53a6 100644\n>>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n>>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n>>>> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n>>>> value down. A value of zero stops the motion if one is in progress\n>>>> and has no effect otherwise.\n>>>> \n>>>> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n>>>> + This read-only control describes the camera sensor location by reporting\n>>>> + its mounting position on the device where the camera is installed. The\n>>>> + control value is constant and not modifiable by software. This control is\n>>>> + particularly meaningful for devices which have a well defined orientation,\n>>>> + such as phones, laptops and portable devices since the camera location is\n>>>> + expressed as a position relative to the device's intended usage orientation.\n>>>> + For example, a camera sensor installed on the user-facing side of a phone,\n>>>> + a tablet or a laptop device is said to be installed in the\n>>>> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n>>>> + opposite the front one are said to be installed in the\n>>>> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n>>>> + the device or attached in a way that allows them to move freely, such as\n>>>> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n>>>> + location.\n>>>> +\n>>>> +\n>>>> +\n>>>> +.. flat-table::\n>>>> + :header-rows: 0\n>>>> + :stub-columns: 0\n>>>> +\n>>>> + * - `` \n>>> `` \n>>>> + - The camera sensor is located on the front side of the device.\n>>>> + * - ``V4L2_LOCATION_BACK``\n>>>> + - The camera sensor is located on the back side of the device.\n>>>> + * - ``V4L2_LOCATION_EXTERNAL``\n>>>> + - The camera sensor is not directly attached to the device and is\n>>>> + freely movable. \n>>>\n>>> I guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\n>>> for two reasons:\n>>>\n>>> 1) newer devices may all top of the line mobile devices now are coming\n>>> with multiple camera sensors at the same side. So, just saying that\n>>> the location is front or back is not enough. A location syscall would\n>>> need have something more to better identify the location.\n>>> It probably doesn't need to be something fancy, but, at least, on a\n>>> device with 3 back sensors, I would call them as:\n>>>\n>>> \tV4L2_LOCATION_BACK_1\n>>> \tV4L2_LOCATION_BACK_2\n>>> \tV4L2_LOCATION_BACK_3\n>>>\n>>> And add some comment at the control documentation that would allow to\n>>> uniquely number the other ones, like:\n>>>\n>>> \t\"when multiple sensors are present at the same side, sensors\n>>> \t will be numbered considering the ``(x,y)`` coordinates of the center\n>>> \t of each sensor, starting from the topmost, leftmost position.\n>>>\n>>> \t She first sensor will be the topmost sensor column at the leftmost\n>>> \t side. The other sensors that will have the same ``y`` coordinate,\n>>> \t counting from the left to the right, then increment the ``y`` and\n>>> \t parse the next column again until all sensors are numbered.\" \n>>\n>> I think this isn't a good idea. In most cases you do not care about this.\n> \n> True, because on most cases, the userspace is hardcoded to open, let's say,\n> video0 for the front sensor or video1 for the back sensor.\n> \n> This control only makes sense if the userspace is generic enough to accept\n> sensors on different positions, identifying them at runtime.\n> \n> With the current proposal, userspace can only work with 2 sensors, as, if\n> there's a third sensor, userspace won't know how to pick the right one.\n> \n> For instance, let's assume a car with 4 sensors, one on each side of\n> the car (right, front); (left, front); (right; back); (left; back).\n> \n> With the current proposal, userspace can't do anything if it wants\n> to identify the (right, back) camera.\n> \n>> And if you do care about this, then wouldn't it be better to do that through\n>> a new control where you provide the precise coordinates in e.g. mm?\n>>\n>> BACK_1/2/3 really doesn't tell you anything other than that there are three\n>> sensors on the back, but we knew that already.\n> \n> No, if we define some criteria about how sensors should be accounted for\n> (something similar to the text I drafted), the location will be defined.\n> \n> With the above text, for example, a device with 3 sensors horizontally\n> aligned, the arrangement will be:\n> \n> - sensor 1 is on the left;\n> - sensor 2 in the middle;\n> - sensor 3 is on the right.\n\nOr sensor 2 is below sensor 1 and sensor 3 is to the right of sensor 1.\nIt's meaningless information. If you want to specify the location, then\nbe precise. Especially for stereoscopic sensors (left and right) it is\ngood to know the exact distance between the sensors. Just calling them\n'1' and '2' is not enough.\n\nFor sensors you want to know the plane (back/front) and where they are\non that plane (in the case of more than one sensor). That's separate\ninformation that's only needed in the case of more than one sensor.\n\n> \n> Ok, I agree that writing a text with such criteria sucks, and maybe\n> just numbering from 1 to n may not be the best thing to do. Yet,\n> adding coordinates in mm would be just too much information, IMHO.\n\nWhy? Just numbering them makes no sense, it's useless information.\n\n> \n>> If we need support for the precise location in the future, then let's do that\n>> right and not try to shoehorn into something that wasn't meant for it.\n> \n> Assuming that all the problems we have so far are to support devices with\n> 2 sensors, by the time we add support for a third sensor, we'll end having\n> a new ioctl for the same thing: to specify the sensors locations.\n\nIt's just a control, nothing more.\n\nIn most cases all you need to know is if it is a front or back sensor. In\nsome cases you need to know more: e.g. my Samsung Note 10+ has three sensors\non the back in a vertical row (wide, telephoto, ultrawide), and two sensors\nfor 3D to the right of them. For those last two you need to know the exact\nposition relative to one another. For the other sensors all you need to know\nis that they are back sensors.\n\n> \n> We know the drill: having two controls for the same thing makes userspace\n> more complex and will require backward-compatibility code at the kernel\n> and at userspace. That's what I want to avoid.\n> \n> I'm open to other suggestions that won't limit the usage of this control\n> for devices with just (up to) two sensors.\n\nWhat backward compatibility code are you talking about? I honestly don't see\nthe problem here.\n\n> \n>>\n>>>\n>>> 2) There are also some devices that has a movable sensor, that can either\n>>> be taking a picture from the front or from the back, like those:\n>>>\n>>> \thttps://www.youtube.com/watch?v=br6G7MrkRUc\n>>>\n>>> On such case, the control should not be read-only, as one may need to\n>>> change this control in order to select if a sensor would either be on\n>>> FRONT or on BACK position.\n>>>\n>>> For such kind of sensors (when we start supporting them), we could \n>>> for example call them like:\n>>>\n>>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_1\n>>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_2\n>>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_1\n>>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_2 \n>>\n>> I don't like this. If the driver can tell when the position changes, then it\n>> can update the control's value (it's still read-only because userspace\n>> can't write to it, but that doesn't mean it can't be updated).\n> \n> Why userspace can't set it? I mean, if the camera is movable, it\n> should be up to the application to select the sensor between FRONT\n> and BACK.\n\nAh, right. If you can command the camera to flip from back to front using\na button or something, then yes, it can be writable. Sorry, didn't think of\nthat. I was thinking that the user would manually move the camera and the\nnew position would be detected by the driver and reported in the location\ncontrol.\n\nIn any case, if the location control can be set through the driver by setting\nthis control, then just drop the READ_ONLY flag. If the control is writable,\nthen the sensor is movable. Just document this and we're done.\n\nYou are making this much more complicated than it need to be IMHO.\n\n> \n> Btw, this is a case where I clearly see value on this ioctl: all cameras\n\nIt's a *control*, not a new ioctl.\n\n> with flippable sensors need a control to switch the sensor's position,\n> even if the sensor device is hardcoded on some application.\n> \n>> So there is\n>> no need to call it 'MOVABLE', you just report the correct location. And with\n>> QUERYMENU you can tell that it is movable since multiple possible locations\n>> are reported (BACK and FRONT in this example). If it is fixed, then QUERYMENU\n>> will report only a single location.\n>>\n>> This might have some consequences for the DT bindings, though. Not sure\n>> how to represent this there.\n> \n> I guess DT should contain the default value when the device is turned\n> off. \n> \n>> If the driver cannot tell what the position is, then it makes no sense for\n>> the driver to expose this location control since it clearly is something that\n>> has to be hardcoded in userspace. I.e., there is no point for userspace to\n>> write to the control and then read back what it wrote :-)\n> \n> Actually there is. When you command a device to switch position, it may\n> take some time to move the sensor, and such operation may even fail.\n\nYeah, I forgot about that option.\n\n> \n> So, reading back the position is probably mandatory.\n\nWell, it's a control, so that's standard.\n\n> \n> That reminds that it may actually have a third position, to warn\n> that the sensor was blocked.\n> \n> Also, some flip sensors may have another position (a \"closed\"\n> position).\n\nIt's certainly possible that we need to add new positions to support the\nvarious states of such a movable sensor. But that's no problem. It's just\na menu control, adding new positions is cheap and easy.\n\nI stand by what I said, except that I agree that this control can be\nwritable in some circumstances, and that should be documented.\n\nI strongly disagree with the notion of BACK_1/2/3 and FRONT_1/2/3: it adds\nno meaningful information. If you have multiple sensors and in order to use\nthem the application needs to know the relative positions (most likely for\n3D sensors), then provide the precise position. The unit for that should\nprobably be micrometer since millimeter is most likely not precise enough\n(at least looking at the depth sensors on my camera).\n\nRegards,\n\n\tHans\n\n> \n>> So I disagree that there is a need for FIXED vs MOVABLE, this can be\n>> represented nicely with the current proposal.\n> \n> Thanks,\n> Mauro\n>","headers":{"Return-Path":"","Received":["from lb3-smtp-cloud7.xs4all.net (lb3-smtp-cloud7.xs4all.net\n\t[194.109.24.31])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 04BCA603EA\n\tfor ;\n\tWed, 6 May 2020 10:04:43 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud7.xs4all.net with ESMTPA\n\tid WF2ljLpVxtKAsWF2pjUE29; Wed, 06 May 2020 10:04:43 +0200"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"kz31dokA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1588752283; bh=ltd4qst7D7I3x++9bUGIH+BMfFhVv6azIH/osw8vUo8=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=kz31dokAX32wzyITYud1/0i4FWTn2y48BhLBm/tgqnadvLmPSKpeMn0j+Oeg6anca\n\ti0LONeaB3tSWU3Qw0jbKUhcs8TwxFux9yiYuLwxYIxyyYcPpQg2OKpohwUnICM3v+G\n\t9ZUaooaH4bBoiU/QWLC5GJzoLfmvGoFOWWHv7FCNO2lhjfLnYMISeGSvz7oOZNlzYJ\n\tcendCDk//fjHleoca7WkoBnYn8dnpblCxSqrYIZRZLELsbifplzkujORScoaKYfmrh\n\tRHuIUWFRGAgwm9cvmnxqdU60L1g+hV6jwLH6vXk5LC87NPZvKLGTzBEf/87dXgLWLA\n\tYDCAGk8vBiYpA==","To":"Mauro Carvalho Chehab ","Cc":"Jacopo Mondi ,\n\tSakari Ailus ,\n\tLaurent Pinchart , tfiga@google.com,\n\tpavel@ucw.cz, \"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200505165826.1ce8bb0d@coco.lan>","From":"Hans Verkuil ","Message-ID":"","Date":"Wed, 6 May 2020 10:04:39 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.5.0","MIME-Version":"1.0","In-Reply-To":"<20200505165826.1ce8bb0d@coco.lan>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfLsWrudtSIG2n2n8LA8HaVFfI8HgBuvIxtXEOL/qNLE7F2LgakRAAFyCV+C/Mgo5YAJBU6CGfdJmo/174ET0RRVMoQjHyuFsuoXDw5eRjDA0kMHf314r\n\t6QZYJsF054po8qz1Dob+Q3vLFSXYUoVxkt0S9N3hzSUE+lKLrITH4+PfSmfRJQBHKxfBBKzxFKjJ4VSlrTAUgnTvscVAYR9wDFpHncjUaQXWSO21ZK4XfFhp\n\tTtn6KrEA1JRM+xAQjk2F34zzVppd4OqBtYg6Dksq7q9PfgSdvWG5gDfywmZvLUb4zrQ3kBx2DMdEq2jSbhCIDiFhefOHiJJ/Epfeyh4H1Dc/8UlY1ir/uGwJ\n\tMDmJCT6QPIE8yM6rqHxCpj/HfYDu3876i17cthi4ScYr/pvlW1KIBycTmcles40vDoG5wr1g7gH4zA8ga+MuTf/GC6zsJg==","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Wed, 06 May 2020 08:04:44 -0000"}},{"id":4746,"web_url":"https://patchwork.libcamera.org/comment/4746/","msgid":"<20200506113909.1489bd1e@coco.lan>","date":"2020-05-06T09:39:09","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":47,"url":"https://patchwork.libcamera.org/api/people/47/","name":"Mauro Carvalho Chehab","email":"mchehab@kernel.org"},"content":"Em Wed, 6 May 2020 10:04:39 +0200\nHans Verkuil escreveu:\n\n> On 05/05/2020 16:58, Mauro Carvalho Chehab wrote:\n> > Em Tue, 5 May 2020 14:21:38 +0200\n> > Hans Verkuil escreveu:\n> > \n> >> On 05/05/2020 14:02, Mauro Carvalho Chehab wrote: \n> >>> Em Fri, 17 Apr 2020 14:41:01 +0200\n> >>> Jacopo Mondi escreveu:\n> >>> \n> >>>> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n> >>>> control. The newly added read-only control reports the camera device\n> >>>> mounting position.\n> >>>>\n> >>>> Signed-off-by: Jacopo Mondi \n> >>>> ---\n> >>>> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n> >>>> 1 file changed, 32 insertions(+)\n> >>>>\n> >>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >>>> index e39f84d2447f..01a9042d53a6 100644\n> >>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >>>> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n> >>>> value down. A value of zero stops the motion if one is in progress\n> >>>> and has no effect otherwise.\n> >>>> \n> >>>> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n> >>>> + This read-only control describes the camera sensor location by reporting\n> >>>> + its mounting position on the device where the camera is installed. The\n> >>>> + control value is constant and not modifiable by software. This control is\n> >>>> + particularly meaningful for devices which have a well defined orientation,\n> >>>> + such as phones, laptops and portable devices since the camera location is\n> >>>> + expressed as a position relative to the device's intended usage orientation.\n> >>>> + For example, a camera sensor installed on the user-facing side of a phone,\n> >>>> + a tablet or a laptop device is said to be installed in the\n> >>>> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n> >>>> + opposite the front one are said to be installed in the\n> >>>> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n> >>>> + the device or attached in a way that allows them to move freely, such as\n> >>>> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n> >>>> + location.\n> >>>> +\n> >>>> +\n> >>>> +\n> >>>> +.. flat-table::\n> >>>> + :header-rows: 0\n> >>>> + :stub-columns: 0\n> >>>> +\n> >>>> + * - `` \n> >>> `` \n> >>>> + - The camera sensor is located on the front side of the device.\n> >>>> + * - ``V4L2_LOCATION_BACK``\n> >>>> + - The camera sensor is located on the back side of the device.\n> >>>> + * - ``V4L2_LOCATION_EXTERNAL``\n> >>>> + - The camera sensor is not directly attached to the device and is\n> >>>> + freely movable. \n> >>>\n> >>> I guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\n> >>> for two reasons:\n> >>>\n> >>> 1) newer devices may all top of the line mobile devices now are coming\n> >>> with multiple camera sensors at the same side. So, just saying that\n> >>> the location is front or back is not enough. A location syscall would\n> >>> need have something more to better identify the location.\n> >>> It probably doesn't need to be something fancy, but, at least, on a\n> >>> device with 3 back sensors, I would call them as:\n> >>>\n> >>> \tV4L2_LOCATION_BACK_1\n> >>> \tV4L2_LOCATION_BACK_2\n> >>> \tV4L2_LOCATION_BACK_3\n> >>>\n> >>> And add some comment at the control documentation that would allow to\n> >>> uniquely number the other ones, like:\n> >>>\n> >>> \t\"when multiple sensors are present at the same side, sensors\n> >>> \t will be numbered considering the ``(x,y)`` coordinates of the center\n> >>> \t of each sensor, starting from the topmost, leftmost position.\n> >>>\n> >>> \t She first sensor will be the topmost sensor column at the leftmost\n> >>> \t side. The other sensors that will have the same ``y`` coordinate,\n> >>> \t counting from the left to the right, then increment the ``y`` and\n> >>> \t parse the next column again until all sensors are numbered.\" \n> >>\n> >> I think this isn't a good idea. In most cases you do not care about this. \n> > \n> > True, because on most cases, the userspace is hardcoded to open, let's say,\n> > video0 for the front sensor or video1 for the back sensor.\n> > \n> > This control only makes sense if the userspace is generic enough to accept\n> > sensors on different positions, identifying them at runtime.\n> > \n> > With the current proposal, userspace can only work with 2 sensors, as, if\n> > there's a third sensor, userspace won't know how to pick the right one.\n> > \n> > For instance, let's assume a car with 4 sensors, one on each side of\n> > the car (right, front); (left, front); (right; back); (left; back).\n> > \n> > With the current proposal, userspace can't do anything if it wants\n> > to identify the (right, back) camera.\n> > \n> >> And if you do care about this, then wouldn't it be better to do that through\n> >> a new control where you provide the precise coordinates in e.g. mm?\n> >>\n> >> BACK_1/2/3 really doesn't tell you anything other than that there are three\n> >> sensors on the back, but we knew that already. \n> > \n> > No, if we define some criteria about how sensors should be accounted for\n> > (something similar to the text I drafted), the location will be defined.\n> > \n> > With the above text, for example, a device with 3 sensors horizontally\n> > aligned, the arrangement will be:\n> > \n> > - sensor 1 is on the left;\n> > - sensor 2 in the middle;\n> > - sensor 3 is on the right. \n> \n> Or sensor 2 is below sensor 1 and sensor 3 is to the right of sensor 1.\n> It's meaningless information. If you want to specify the location, then\n> be precise. Especially for stereoscopic sensors (left and right) it is\n> good to know the exact distance between the sensors. Just calling them\n> '1' and '2' is not enough.\n> \n> For sensors you want to know the plane (back/front) and where they are\n> on that plane (in the case of more than one sensor). That's separate\n> information that's only needed in the case of more than one sensor.\n> \n> > \n> > Ok, I agree that writing a text with such criteria sucks, and maybe\n> > just numbering from 1 to n may not be the best thing to do. Yet,\n> > adding coordinates in mm would be just too much information, IMHO. \n> \n> Why? Just numbering them makes no sense, it's useless information.\n> \n> > \n> >> If we need support for the precise location in the future, then let's do that\n> >> right and not try to shoehorn into something that wasn't meant for it. \n> > \n> > Assuming that all the problems we have so far are to support devices with\n> > 2 sensors, by the time we add support for a third sensor, we'll end having\n> > a new ioctl for the same thing: to specify the sensors locations. \n> \n> It's just a control, nothing more.\n> \n> In most cases all you need to know is if it is a front or back sensor. In\n> some cases you need to know more: e.g. my Samsung Note 10+ has three sensors\n> on the back in a vertical row (wide, telephoto, ultrawide), and two sensors\n> for 3D to the right of them. For those last two you need to know the exact\n> position relative to one another. For the other sensors all you need to know\n> is that they are back sensors.\n> \n> > \n> > We know the drill: having two controls for the same thing makes userspace\n> > more complex and will require backward-compatibility code at the kernel\n> > and at userspace. That's what I want to avoid.\n> > \n> > I'm open to other suggestions that won't limit the usage of this control\n> > for devices with just (up to) two sensors. \n> \n> What backward compatibility code are you talking about? I honestly don't see\n> the problem here.\n\nRight now, we're adding an API that assumes that the video node may have\nonly up to 2 sensors, and that would cover just one small subset of usecases\n(see more below). If it has anything more than that, this control won't help.\n\nOne day (probably soon enough, as there are several devices with more than\ntwo sensors already), we'll end adding a proper support for it, and this \ncontrol will become obsoleted, requiring us to think about backward\ncompatibility issues when this control become deprecated.\n\nThat's why I prefer spending some time finding a better way to report it,\navoiding the need of having to do some deprecation logic anytime soon.\n\n> >>>\n> >>> 2) There are also some devices that has a movable sensor, that can either\n> >>> be taking a picture from the front or from the back, like those:\n> >>>\n> >>> \thttps://www.youtube.com/watch?v=br6G7MrkRUc\n> >>>\n> >>> On such case, the control should not be read-only, as one may need to\n> >>> change this control in order to select if a sensor would either be on\n> >>> FRONT or on BACK position.\n> >>>\n> >>> For such kind of sensors (when we start supporting them), we could \n> >>> for example call them like:\n> >>>\n> >>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_1\n> >>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_2\n> >>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_1\n> >>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_2 \n> >>\n> >> I don't like this. If the driver can tell when the position changes, then it\n> >> can update the control's value (it's still read-only because userspace\n> >> can't write to it, but that doesn't mean it can't be updated). \n> > \n> > Why userspace can't set it? I mean, if the camera is movable, it\n> > should be up to the application to select the sensor between FRONT\n> > and BACK. \n> \n> Ah, right. If you can command the camera to flip from back to front using\n> a button or something, then yes, it can be writable. Sorry, didn't think of\n> that. I was thinking that the user would manually move the camera and the\n> new position would be detected by the driver and reported in the location\n> control.\n> \n> In any case, if the location control can be set through the driver by setting\n> this control, then just drop the READ_ONLY flag. If the control is writable,\n> then the sensor is movable. Just document this and we're done.\n\nWorks for me.\n\n> You are making this much more complicated than it need to be IMHO.\n> \n> > \n> > Btw, this is a case where I clearly see value on this ioctl: all cameras \n> \n> It's a *control*, not a new ioctl.\n> \n> > with flippable sensors need a control to switch the sensor's position,\n> > even if the sensor device is hardcoded on some application.\n> > \n> >> So there is\n> >> no need to call it 'MOVABLE', you just report the correct location. And with\n> >> QUERYMENU you can tell that it is movable since multiple possible locations\n> >> are reported (BACK and FRONT in this example). If it is fixed, then QUERYMENU\n> >> will report only a single location.\n> >>\n> >> This might have some consequences for the DT bindings, though. Not sure\n> >> how to represent this there. \n> > \n> > I guess DT should contain the default value when the device is turned\n> > off. \n> > \n> >> If the driver cannot tell what the position is, then it makes no sense for\n> >> the driver to expose this location control since it clearly is something that\n> >> has to be hardcoded in userspace. I.e., there is no point for userspace to\n> >> write to the control and then read back what it wrote :-) \n> > \n> > Actually there is. When you command a device to switch position, it may\n> > take some time to move the sensor, and such operation may even fail. \n> \n> Yeah, I forgot about that option.\n> \n> > \n> > So, reading back the position is probably mandatory. \n> \n> Well, it's a control, so that's standard.\n> \n> > \n> > That reminds that it may actually have a third position, to warn\n> > that the sensor was blocked.\n> > \n> > Also, some flip sensors may have another position (a \"closed\"\n> > position). \n> \n> It's certainly possible that we need to add new positions to support the\n> various states of such a movable sensor. But that's no problem. It's just\n> a menu control, adding new positions is cheap and easy.\n> \n> I stand by what I said, except that I agree that this control can be\n> writable in some circumstances, and that should be documented\n> \n> I strongly disagree with the notion of BACK_1/2/3 and FRONT_1/2/3: it adds\n> no meaningful information. \n\nOk, but if this control would just say where a sensor is mounted\n(front, back or unknown/external), naming it as \"LOCATION\" seems too\nambitious ;-)\n\nWhat it is actually trying to report is the angle of the sensor, with\nregards to the front position, adding currently two possible angles:\n0 degrees (front) or 180 degrees (back).\n\nSo, I would call it, instead, as V4L2_CID_CAMERA_VIEWING_ANGLE\n(or something similar to it).\n\nHaving just two pre-defined angles (front/back) only works fine on\ndevices like cell-phones or tablets, where the sensor cannot be\non some other angle.\n\nIf you mount cameras on a larger device, like a car, you may have\nsome cameras mounted with different angles, for example, the front\ncameras could have been mounted with -45, 0 and 45 degrees, in order\nto cover a larger region.\n\nSo, if that would be ok for you, I can live with a\n\nV4L2_CID_CAMERA_VIEWING_ANGLE (or some similar name) that will\nspecify the angle where the sensor is mounted (for fixed sensors),\nor the current angle, in case of movable ones, being RO for fixed\nsensors and RW for movable ones.\n\nLet's postpone discussions for a LOCATION control once this\nwould be needed by some driver.\n\n> If you have multiple sensors and in order to use\n> them the application needs to know the relative positions (most likely for\n> 3D sensors), then provide the precise position. The unit for that should\n> probably be micrometer since millimeter is most likely not precise enough\n> (at least looking at the depth sensors on my camera).\n\nI can see two different types of usage for a real localization control:\n\n1) 3D sensors - for that, micrometer is probably a better measure;\n2) multiple sensors, each covering a different view. That could be,\n for example, a back camera on the left side or on at the right side\n of a car. It could also be several sensors at the same side on a long\n product inspection line.\n\nFor (2), the distance between sensors could be several meters. So,\nperhaps we'll need to either add two different LOCATION controls,\none for 3D and another one for multiple 2D sensors spread to cover\na larger region.\n\nIn any case, let's postpone any further discussions about that until\nwhen we have someone needing it.\n\n\nThanks,\nMauro","headers":{"Return-Path":"","Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[IPv6:2607:7c80:54:e::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2EE8E603EA\n\tfor ;\n\tWed, 6 May 2020 11:39:35 +0200 (CEST)","from ip5f5ad5c5.dynamic.kabel-deutschland.de ([95.90.213.197]\n\thelo=coco.lan)\n\tby bombadil.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat\n\tLinux)) id 1jWGWH-0001BC-QN; Wed, 06 May 2020 09:39:14 +0000"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=infradead.org\n\theader.i=@infradead.org\n\theader.b=\"cAaDvPHP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=infradead.org; s=bombadil.20170209; h=Content-Transfer-Encoding:\n\tContent-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To:\n\tFrom:Date:Sender:Reply-To:Content-ID:Content-Description;\n\tbh=YDrKAqf/aibq0Ju0nXvSzE5O8lbrElqg6pf9RUuQkwk=;\n\tb=cAaDvPHPlTCedycNoCl4x/dGDe\n\trODCOx+jYXaVHSWJ0eKsBNWqRqNcaK8J1r3bLX1SGEVi6YrcRsZPSqXTTCDW2si4fdp10IMVKYLjP\n\tmM5hEyvYELs1URdtP8rUbqYgBDi7Gq1uKc7GLFtQYf2IK7Vl5aS28pdw0B2YKnpD1cJ/eSftr3gZK\n\t5VGMKcnmwG21PEAhcNLS1eKHDsA3mTn3tmORMPLFw4HrOCIIjEoHReopD8SLLJisDBTQ2zVdYHP7r\n\tmOuLEBqh55GfBaVX/zZWeWS5U8pY73pU6M0xvZeY0cPk5IFz1a3n5VYZeXKQKpKrzTchGLVePTM6a\n\tIlUqGWpA==;","Date":"Wed, 6 May 2020 11:39:09 +0200","From":"Mauro Carvalho Chehab ","To":"Hans Verkuil ","Cc":"Jacopo Mondi , Sakari Ailus\n\t, Laurent Pinchart\n\t, tfiga@google.com, pavel@ucw.cz,\n\t\"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","Message-ID":"<20200506113909.1489bd1e@coco.lan>","In-Reply-To":"","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200505165826.1ce8bb0d@coco.lan>\n\t","X-Mailer":"Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-redhat-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Wed, 06 May 2020 09:39:36 -0000"}},{"id":4747,"web_url":"https://patchwork.libcamera.org/comment/4747/","msgid":"<20200506104723.l3wojjyefvazohpd@uno.localdomain>","date":"2020-05-06T10:47:23","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hello,\n\nOn Tue, May 05, 2020 at 02:21:38PM +0200, Hans Verkuil wrote:\n> On 05/05/2020 14:02, Mauro Carvalho Chehab wrote:\n> > Em Fri, 17 Apr 2020 14:41:01 +0200\n> > Jacopo Mondi escreveu:\n> >\n> >> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n> >> control. The newly added read-only control reports the camera device\n> >> mounting position.\n> >>\n> >> Signed-off-by: Jacopo Mondi \n> >> ---\n> >> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n> >> 1 file changed, 32 insertions(+)\n> >>\n> >> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >> index e39f84d2447f..01a9042d53a6 100644\n> >> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n> >> value down. A value of zero stops the motion if one is in progress\n> >> and has no effect otherwise.\n> >>\n> >> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n> >> + This read-only control describes the camera sensor location by reporting\n> >> + its mounting position on the device where the camera is installed. The\n> >> + control value is constant and not modifiable by software. This control is\n> >> + particularly meaningful for devices which have a well defined orientation,\n> >> + such as phones, laptops and portable devices since the camera location is\n> >> + expressed as a position relative to the device's intended usage orientation.\n> >> + For example, a camera sensor installed on the user-facing side of a phone,\n> >> + a tablet or a laptop device is said to be installed in the\n> >> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n> >> + opposite the front one are said to be installed in the\n> >> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n> >> + the device or attached in a way that allows them to move freely, such as\n> >> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n> >> + location.\n> >> +\n> >> +\n> >> +\n> >> +.. flat-table::\n> >> + :header-rows: 0\n> >> + :stub-columns: 0\n> >> +\n> >> + * - ``\n> > ``\n> >> + - The camera sensor is located on the front side of the device.\n> >> + * - ``V4L2_LOCATION_BACK``\n> >> + - The camera sensor is located on the back side of the device.\n> >> + * - ``V4L2_LOCATION_EXTERNAL``\n> >> + - The camera sensor is not directly attached to the device and is\n> >> + freely movable.\n> >\n> > I guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\n> > for two reasons:\n> >\n> > 1) newer devices may all top of the line mobile devices now are coming\n> > with multiple camera sensors at the same side. So, just saying that\n> > the location is front or back is not enough. A location syscall would\n> > need have something more to better identify the location.\n> > It probably doesn't need to be something fancy, but, at least, on a\n> > device with 3 back sensors, I would call them as:\n> >\n> > \tV4L2_LOCATION_BACK_1\n> > \tV4L2_LOCATION_BACK_2\n> > \tV4L2_LOCATION_BACK_3\n> >\n> > And add some comment at the control documentation that would allow to\n> > uniquely number the other ones, like:\n> >\n> > \t\"when multiple sensors are present at the same side, sensors\n> > \t will be numbered considering the ``(x,y)`` coordinates of the center\n> > \t of each sensor, starting from the topmost, leftmost position.\n> >\n> > \t She first sensor will be the topmost sensor column at the leftmost\n> > \t side. The other sensors that will have the same ``y`` coordinate,\n> > \t counting from the left to the right, then increment the ``y`` and\n> > \t parse the next column again until all sensors are numbered.\"\n>\n> I think this isn't a good idea. In most cases you do not care about this.\n>\n> And if you do care about this, then wouldn't it be better to do that through\n> a new control where you provide the precise coordinates in e.g. mm?\n>\n> BACK_1/2/3 really doesn't tell you anything other than that there are three\n> sensors on the back, but we knew that already.\n>\n> If we need support for the precise location in the future, then let's do that\n> right and not try to shoehorn into something that wasn't meant for it.\n\nI think the best move forward to describe movable cameras and such\nwould be to provide a 3D rotation matrix, along the lines of what iio\nhas in the 'mount-matrix' property as suggested by Rob and Laurent in\nthe review of the series.\n\nBefore going the 'easy' way with this proeprty that just allow to\nenumerate fixed locations I considered the idea, but we're still\nmissing a unique definition for the device usage orientation that the\nrotation matrix would be defined for.\n\nThis property implements a mechanism that covers most of devices out\nthere and all devices in mainline. The properties defined here are the\nmost basic ones, and could be combined and expanded to provide more\nprecise definition is someone needs to do so (expecially downstream),\nbut the important part is that the mechanism to retrieve the\ninformation is in place.\n\n>\n> >\n> > 2) There are also some devices that has a movable sensor, that can either\n> > be taking a picture from the front or from the back, like those:\n> >\n> > \thttps://www.youtube.com/watch?v=br6G7MrkRUc\n> >\n> > On such case, the control should not be read-only, as one may need to\n> > change this control in order to select if a sensor would either be on\n> > FRONT or on BACK position.\n> >\n> > For such kind of sensors (when we start supporting them), we could\n> > for example call them like:\n> >\n> > \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_1\n> > \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_2\n> > \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_1\n> > \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_2\n>\n> I don't like this. If the driver can tell when the position changes, then it\n> can update the control's value (it's still read-only because userspace\n> can't write to it, but that doesn't mean it can't be updated). So there is\n\nYes, the control is read-only as userspace cannot modify it, but\ndrivers could update it after having gathered its value from firmware.\n\n> no need to call it 'MOVABLE', you just report the correct location. And with\n> QUERYMENU you can tell that it is movable since multiple possible locations\n> are reported (BACK and FRONT in this example). If it is fixed, then QUERYMENU\n> will report only a single location.\n>\n> This might have some consequences for the DT bindings, though. Not sure\n> how to represent this there.\n>\n> If the driver cannot tell what the position is, then it makes no sense for\n> the driver to expose this location control since it clearly is something that\n> has to be hardcoded in userspace. I.e., there is no point for userspace to\n> write to the control and then read back what it wrote :-)\n>\n> So I disagree that there is a need for FIXED vs MOVABLE, this can be\n> represented nicely with the current proposal.\n\nJust to add that enumerating several BACK_x (or FRONT_x) location\nwould not help at all userspace, which should be again capable of\nrecognizing what '_x' conveys.\n\nWhat userspace could be interested in is something like \"find\nall BACK cameras\" then \"use the one with the highest resolution\". The\nhere presented property allows filtering cameras in the system, but\nshould not try to uniquely identify them, as userspace would most\nlikely combine several filtering criteria to get to the \"right\" camera.\n\nThanks\n j\n\n>\n> Regards,\n>\n> \tHans\n>\n> >\n> > And add rename the other definitions to:\n> >\n> > \tV4L2_LOCATION_FIXED_FRONT_1\n> > \tV4L2_LOCATION_FIXED_BACK_1\n> >\n> > Ok, nobody has yet attempted to upstream code for such devices,\n> > so, we, for now, we don't need to add more than those 3 types.\n> >\n> > But, IMO, we would change the sensors description in a way that it\n> > would be easier to add support for more than one sensor per location\n> > in the future, like:\n> >\n> > \t* - ``V4L2_LOCATION_FIXED_FRONT_1``\n> > - The camera sensor is fixed, being the first sensor\n> > \t located on the front side of the device.\n> > \t* - ``V4L2_LOCATION_FIXED_BACK_1``\n> > \t - The camera sensor is fixed, being the first sensor\n> > \t located on the back side of the device.\n> > \t* - ``V4L2_LOCATION_EXTERNAL``\n> > - The camera sensor is not directly attached to the device\n> > \t and is freely movable.\n> >\n> > \t.. note:: Please submit a patch upstream if you need to have\n> > \t\t more than one sensor either at front or back position.\n> >\n> > This would make a lot easier when someone upstream patches requiring\n> > to locate more than one sensor location, or to support flipping\n> > sensors.\n> >\n> > Thanks,\n> > Mauro\n> >\n>","headers":{"Return-Path":"","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 988FE603F2\n\tfor ;\n\tWed, 6 May 2020 12:44:15 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 0778840009;\n\tWed, 6 May 2020 10:44:11 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 6 May 2020 12:47:23 +0200","From":"Jacopo Mondi ","To":"Hans Verkuil ","Cc":"Mauro Carvalho Chehab ,\n\tSakari Ailus ,\n\tLaurent Pinchart ,\n\ttfiga@google.com, pavel@ucw.cz,\n\t\"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","Message-ID":"<20200506104723.l3wojjyefvazohpd@uno.localdomain>","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Wed, 06 May 2020 10:44:15 -0000"}},{"id":4748,"web_url":"https://patchwork.libcamera.org/comment/4748/","msgid":"<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>","date":"2020-05-06T11:07:30","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hello,\n\nOn Wed, May 06, 2020 at 11:39:09AM +0200, Mauro Carvalho Chehab wrote:\n> Em Wed, 6 May 2020 10:04:39 +0200\n> Hans Verkuil escreveu:\n>\n> > On 05/05/2020 16:58, Mauro Carvalho Chehab wrote:\n> > > Em Tue, 5 May 2020 14:21:38 +0200\n> > > Hans Verkuil escreveu:\n> > >\n> > >> On 05/05/2020 14:02, Mauro Carvalho Chehab wrote:\n> > >>> Em Fri, 17 Apr 2020 14:41:01 +0200\n> > >>> Jacopo Mondi escreveu:\n> > >>>\n> > >>>> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n> > >>>> control. The newly added read-only control reports the camera device\n> > >>>> mounting position.\n> > >>>>\n> > >>>> Signed-off-by: Jacopo Mondi \n> > >>>> ---\n> > >>>> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n> > >>>> 1 file changed, 32 insertions(+)\n> > >>>>\n> > >>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> > >>>> index e39f84d2447f..01a9042d53a6 100644\n> > >>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> > >>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> > >>>> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n> > >>>> value down. A value of zero stops the motion if one is in progress\n> > >>>> and has no effect otherwise.\n> > >>>>\n> > >>>> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n> > >>>> + This read-only control describes the camera sensor location by reporting\n> > >>>> + its mounting position on the device where the camera is installed. The\n> > >>>> + control value is constant and not modifiable by software. This control is\n> > >>>> + particularly meaningful for devices which have a well defined orientation,\n> > >>>> + such as phones, laptops and portable devices since the camera location is\n> > >>>> + expressed as a position relative to the device's intended usage orientation.\n> > >>>> + For example, a camera sensor installed on the user-facing side of a phone,\n> > >>>> + a tablet or a laptop device is said to be installed in the\n> > >>>> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n> > >>>> + opposite the front one are said to be installed in the\n> > >>>> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n> > >>>> + the device or attached in a way that allows them to move freely, such as\n> > >>>> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n> > >>>> + location.\n> > >>>> +\n> > >>>> +\n> > >>>> +\n> > >>>> +.. flat-table::\n> > >>>> + :header-rows: 0\n> > >>>> + :stub-columns: 0\n> > >>>> +\n> > >>>> + * - ``\n> > >>> ``\n> > >>>> + - The camera sensor is located on the front side of the device.\n> > >>>> + * - ``V4L2_LOCATION_BACK``\n> > >>>> + - The camera sensor is located on the back side of the device.\n> > >>>> + * - ``V4L2_LOCATION_EXTERNAL``\n> > >>>> + - The camera sensor is not directly attached to the device and is\n> > >>>> + freely movable.\n> > >>>\n> > >>> I guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\n> > >>> for two reasons:\n> > >>>\n> > >>> 1) newer devices may all top of the line mobile devices now are coming\n> > >>> with multiple camera sensors at the same side. So, just saying that\n> > >>> the location is front or back is not enough. A location syscall would\n> > >>> need have something more to better identify the location.\n> > >>> It probably doesn't need to be something fancy, but, at least, on a\n> > >>> device with 3 back sensors, I would call them as:\n> > >>>\n> > >>> \tV4L2_LOCATION_BACK_1\n> > >>> \tV4L2_LOCATION_BACK_2\n> > >>> \tV4L2_LOCATION_BACK_3\n> > >>>\n> > >>> And add some comment at the control documentation that would allow to\n> > >>> uniquely number the other ones, like:\n> > >>>\n> > >>> \t\"when multiple sensors are present at the same side, sensors\n> > >>> \t will be numbered considering the ``(x,y)`` coordinates of the center\n> > >>> \t of each sensor, starting from the topmost, leftmost position.\n> > >>>\n> > >>> \t She first sensor will be the topmost sensor column at the leftmost\n> > >>> \t side. The other sensors that will have the same ``y`` coordinate,\n> > >>> \t counting from the left to the right, then increment the ``y`` and\n> > >>> \t parse the next column again until all sensors are numbered.\"\n> > >>\n> > >> I think this isn't a good idea. In most cases you do not care about this.\n> > >\n> > > True, because on most cases, the userspace is hardcoded to open, let's say,\n> > > video0 for the front sensor or video1 for the back sensor.\n> > >\n> > > This control only makes sense if the userspace is generic enough to accept\n> > > sensors on different positions, identifying them at runtime.\n> > >\n> > > With the current proposal, userspace can only work with 2 sensors, as, if\n> > > there's a third sensor, userspace won't know how to pick the right one.\n> > >\n> > > For instance, let's assume a car with 4 sensors, one on each side of\n> > > the car (right, front); (left, front); (right; back); (left; back).\n> > >\n> > > With the current proposal, userspace can't do anything if it wants\n> > > to identify the (right, back) camera.\n> > >\n> > >> And if you do care about this, then wouldn't it be better to do that through\n> > >> a new control where you provide the precise coordinates in e.g. mm?\n> > >>\n> > >> BACK_1/2/3 really doesn't tell you anything other than that there are three\n> > >> sensors on the back, but we knew that already.\n> > >\n> > > No, if we define some criteria about how sensors should be accounted for\n> > > (something similar to the text I drafted), the location will be defined.\n> > >\n> > > With the above text, for example, a device with 3 sensors horizontally\n> > > aligned, the arrangement will be:\n> > >\n> > > - sensor 1 is on the left;\n> > > - sensor 2 in the middle;\n> > > - sensor 3 is on the right.\n> >\n> > Or sensor 2 is below sensor 1 and sensor 3 is to the right of sensor 1.\n> > It's meaningless information. If you want to specify the location, then\n> > be precise. Especially for stereoscopic sensors (left and right) it is\n> > good to know the exact distance between the sensors. Just calling them\n> > '1' and '2' is not enough.\n> >\n> > For sensors you want to know the plane (back/front) and where they are\n> > on that plane (in the case of more than one sensor). That's separate\n> > information that's only needed in the case of more than one sensor.\n> >\n> > >\n> > > Ok, I agree that writing a text with such criteria sucks, and maybe\n> > > just numbering from 1 to n may not be the best thing to do. Yet,\n> > > adding coordinates in mm would be just too much information, IMHO.\n> >\n> > Why? Just numbering them makes no sense, it's useless information.\n> >\n> > >\n> > >> If we need support for the precise location in the future, then let's do that\n> > >> right and not try to shoehorn into something that wasn't meant for it.\n> > >\n> > > Assuming that all the problems we have so far are to support devices with\n> > > 2 sensors, by the time we add support for a third sensor, we'll end having\n> > > a new ioctl for the same thing: to specify the sensors locations.\n> >\n> > It's just a control, nothing more.\n> >\n> > In most cases all you need to know is if it is a front or back sensor. In\n> > some cases you need to know more: e.g. my Samsung Note 10+ has three sensors\n> > on the back in a vertical row (wide, telephoto, ultrawide), and two sensors\n> > for 3D to the right of them. For those last two you need to know the exact\n> > position relative to one another. For the other sensors all you need to know\n> > is that they are back sensors.\n> >\n> > >\n> > > We know the drill: having two controls for the same thing makes userspace\n> > > more complex and will require backward-compatibility code at the kernel\n> > > and at userspace. That's what I want to avoid.\n> > >\n> > > I'm open to other suggestions that won't limit the usage of this control\n> > > for devices with just (up to) two sensors.\n> >\n> > What backward compatibility code are you talking about? I honestly don't see\n> > the problem here.\n>\n> Right now, we're adding an API that assumes that the video node may have\n> only up to 2 sensors, and that would cover just one small subset of usecases\n> (see more below). If it has anything more than that, this control won't help.\n\nI don't agree the number of sensor is limited to 2. This property does\nnot identify sensors, it describes one more thing that userspace might\nuse to filter cameras. I was actually suprised nothing like this\nexisted in Linux when I started looking into this issue, as this seems\nto me quite basic information that a generic enough userspace\napplication would like to be able to retrieve.\n\nTL;DR: you can describe as many BACK cameras you want, the 'location'\ngives you -one- filtering criteria more, that's it.\n\n>\n> One day (probably soon enough, as there are several devices with more than\n> two sensors already), we'll end adding a proper support for it, and this\n> control will become obsoleted, requiring us to think about backward\n> compatibility issues when this control become deprecated.\n>\n> That's why I prefer spending some time finding a better way to report it,\n> avoiding the need of having to do some deprecation logic anytime soon.\n>\n\nAs said and discussed during the review of this series, a 3-d rotation\nmatrix is probably the right direction. I refrained from taking that\npath because:\n1) 99% of devices are interested in reporting FRONT/BACK or some\nspecialization of those. Asking dt to provide a 9 entries matrix to\nsay \"FRONT\" seemed an overkill.\n2) There is no consensus on how the reference plane should be defined,\ngiven the wide range of devices that we target. This is a separate\ndiscussion on itself, and given it took 6 months to get to the point\nof considering these basic properties, I'm a bit skeptical such a\ndiscussion would have taken less than that.\n\n> > >>>\n> > >>> 2) There are also some devices that has a movable sensor, that can either\n> > >>> be taking a picture from the front or from the back, like those:\n> > >>>\n> > >>> \thttps://www.youtube.com/watch?v=br6G7MrkRUc\n> > >>>\n> > >>> On such case, the control should not be read-only, as one may need to\n> > >>> change this control in order to select if a sensor would either be on\n> > >>> FRONT or on BACK position.\n> > >>>\n> > >>> For such kind of sensors (when we start supporting them), we could\n> > >>> for example call them like:\n> > >>>\n> > >>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_1\n> > >>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_2\n> > >>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_1\n> > >>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_2\n> > >>\n> > >> I don't like this. If the driver can tell when the position changes, then it\n> > >> can update the control's value (it's still read-only because userspace\n> > >> can't write to it, but that doesn't mean it can't be updated).\n> > >\n> > > Why userspace can't set it? I mean, if the camera is movable, it\n> > > should be up to the application to select the sensor between FRONT\n> > > and BACK.\n> >\n> > Ah, right. If you can command the camera to flip from back to front using\n> > a button or something, then yes, it can be writable. Sorry, didn't think of\n> > that. I was thinking that the user would manually move the camera and the\n> > new position would be detected by the driver and reported in the location\n> > control.\n> >\n> > In any case, if the location control can be set through the driver by setting\n> > this control, then just drop the READ_ONLY flag. If the control is writable,\n> > then the sensor is movable. Just document this and we're done.\n>\n> Works for me.\n>\n\nThis makes sense, it will impact bindings in the sense that it now\nbecomes possible to specify several locations to which select from,\nwhich will become the items of the menu control (with some rule that\nsays \"the first is the default\" or such). If more than one location is\nspecified the control is RW, RO otherwise.\n\n> > You are making this much more complicated than it need to be IMHO.\n> >\n> > >\n> > > Btw, this is a case where I clearly see value on this ioctl: all cameras\n> >\n> > It's a *control*, not a new ioctl.\n> >\n> > > with flippable sensors need a control to switch the sensor's position,\n> > > even if the sensor device is hardcoded on some application.\n> > >\n> > >> So there is\n> > >> no need to call it 'MOVABLE', you just report the correct location. And with\n> > >> QUERYMENU you can tell that it is movable since multiple possible locations\n> > >> are reported (BACK and FRONT in this example). If it is fixed, then QUERYMENU\n> > >> will report only a single location.\n> > >>\n> > >> This might have some consequences for the DT bindings, though. Not sure\n> > >> how to represent this there.\n> > >\n> > > I guess DT should contain the default value when the device is turned\n> > > off.\n> > >\n> > >> If the driver cannot tell what the position is, then it makes no sense for\n> > >> the driver to expose this location control since it clearly is something that\n> > >> has to be hardcoded in userspace. I.e., there is no point for userspace to\n> > >> write to the control and then read back what it wrote :-)\n> > >\n> > > Actually there is. When you command a device to switch position, it may\n> > > take some time to move the sensor, and such operation may even fail.\n> >\n> > Yeah, I forgot about that option.\n> >\n> > >\n> > > So, reading back the position is probably mandatory.\n> >\n> > Well, it's a control, so that's standard.\n> >\n> > >\n> > > That reminds that it may actually have a third position, to warn\n> > > that the sensor was blocked.\n> > >\n> > > Also, some flip sensors may have another position (a \"closed\"\n> > > position).\n> >\n> > It's certainly possible that we need to add new positions to support the\n> > various states of such a movable sensor. But that's no problem. It's just\n> > a menu control, adding new positions is cheap and easy.\n> >\n> > I stand by what I said, except that I agree that this control can be\n> > writable in some circumstances, and that should be documented\n> >\n> > I strongly disagree with the notion of BACK_1/2/3 and FRONT_1/2/3: it adds\n> > no meaningful information.\n>\n> Ok, but if this control would just say where a sensor is mounted\n> (front, back or unknown/external), naming it as \"LOCATION\" seems too\n> ambitious ;-)\n>\n> What it is actually trying to report is the angle of the sensor, with\n> regards to the front position, adding currently two possible angles:\n> 0 degrees (front) or 180 degrees (back).\n>\n> So, I would call it, instead, as V4L2_CID_CAMERA_VIEWING_ANGLE\n> (or something similar to it).\n\n_ORIENTATION might be the right word, I'm fine to reserve _LOCATION\nfor a more precise property if that helps moving forward.\n\n>\n> Having just two pre-defined angles (front/back) only works fine on\n> devices like cell-phones or tablets, where the sensor cannot be\n> on some other angle.\n>\n> If you mount cameras on a larger device, like a car, you may have\n> some cameras mounted with different angles, for example, the front\n> cameras could have been mounted with -45, 0 and 45 degrees, in order\n> to cover a larger region.\n\nI considered that case, but I expect those very specific usages to be\ncovered by downstream extensions of the property supported values. I\nwish we had a .dts to describe a car in mainlien, but I would be happy\nenough to provide a standard mechanism for people to use downstream\neventually, instead of adding custom properties, or taking shortcuts\nlike it mostly happens today.\n\n>\n> So, if that would be ok for you, I can live with a\n>\n> V4L2_CID_CAMERA_VIEWING_ANGLE (or some similar name) that will\n> specify the angle where the sensor is mounted (for fixed sensors),\n> or the current angle, in case of movable ones, being RO for fixed\n> sensors and RW for movable ones.\n>\n> Let's postpone discussions for a LOCATION control once this\n> would be needed by some driver.\n\nWould V4L2_CID_CAMERA_ORIENTATION work ?\n\nI could:\n1) rename dt-proeprty and control to use orientation\n2) specify multiple locations could be entered, the first one being\nthe \"default\" (eg. device turned off) location\n3) make am RW control if multiple entries have been specified, a RO\notherwise.\n\nAck ?\n\nThanks\n j","headers":{"Return-Path":"","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F7CC603F2\n\tfor ;\n\tWed, 6 May 2020 13:05:10 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id D8710200021;\n\tWed, 6 May 2020 11:04:18 +0000 (UTC)"],"Date":"Wed, 6 May 2020 13:07:30 +0200","From":"Jacopo Mondi ","To":"Mauro Carvalho Chehab ","Cc":"Hans Verkuil ,\n\tSakari Ailus ,\n\tLaurent Pinchart ,\n\ttfiga@google.com, pavel@ucw.cz,\n\t\"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","Message-ID":"<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200505165826.1ce8bb0d@coco.lan>\n\t\n\t<20200506113909.1489bd1e@coco.lan>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200506113909.1489bd1e@coco.lan>","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Wed, 06 May 2020 11:05:10 -0000"}},{"id":4749,"web_url":"https://patchwork.libcamera.org/comment/4749/","msgid":"<20200506130913.7df929ff@coco.lan>","date":"2020-05-06T11:09:13","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":47,"url":"https://patchwork.libcamera.org/api/people/47/","name":"Mauro Carvalho Chehab","email":"mchehab@kernel.org"},"content":"Em Wed, 6 May 2020 12:47:23 +0200\nJacopo Mondi escreveu:\n\n> Hello,\n> \n> On Tue, May 05, 2020 at 02:21:38PM +0200, Hans Verkuil wrote:\n> > On 05/05/2020 14:02, Mauro Carvalho Chehab wrote: \n> > > Em Fri, 17 Apr 2020 14:41:01 +0200\n> > > Jacopo Mondi escreveu:\n> > > \n> > >> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n> > >> control. The newly added read-only control reports the camera device\n> > >> mounting position.\n> > >>\n> > >> Signed-off-by: Jacopo Mondi \n> > >> ---\n> > >> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n> > >> 1 file changed, 32 insertions(+)\n> > >>\n> > >> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> > >> index e39f84d2447f..01a9042d53a6 100644\n> > >> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> > >> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> > >> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n> > >> value down. A value of zero stops the motion if one is in progress\n> > >> and has no effect otherwise.\n> > >>\n> > >> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n> > >> + This read-only control describes the camera sensor location by reporting\n> > >> + its mounting position on the device where the camera is installed. The\n> > >> + control value is constant and not modifiable by software. This control is\n> > >> + particularly meaningful for devices which have a well defined orientation,\n> > >> + such as phones, laptops and portable devices since the camera location is\n> > >> + expressed as a position relative to the device's intended usage orientation.\n> > >> + For example, a camera sensor installed on the user-facing side of a phone,\n> > >> + a tablet or a laptop device is said to be installed in the\n> > >> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n> > >> + opposite the front one are said to be installed in the\n> > >> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n> > >> + the device or attached in a way that allows them to move freely, such as\n> > >> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n> > >> + location.\n> > >> +\n> > >> +\n> > >> +\n> > >> +.. flat-table::\n> > >> + :header-rows: 0\n> > >> + :stub-columns: 0\n> > >> +\n> > >> + * - `` \n> > > `` \n> > >> + - The camera sensor is located on the front side of the device.\n> > >> + * - ``V4L2_LOCATION_BACK``\n> > >> + - The camera sensor is located on the back side of the device.\n> > >> + * - ``V4L2_LOCATION_EXTERNAL``\n> > >> + - The camera sensor is not directly attached to the device and is\n> > >> + freely movable. \n> > >\n> > > I guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\n> > > for two reasons:\n> > >\n> > > 1) newer devices may all top of the line mobile devices now are coming\n> > > with multiple camera sensors at the same side. So, just saying that\n> > > the location is front or back is not enough. A location syscall would\n> > > need have something more to better identify the location.\n> > > It probably doesn't need to be something fancy, but, at least, on a\n> > > device with 3 back sensors, I would call them as:\n> > >\n> > > \tV4L2_LOCATION_BACK_1\n> > > \tV4L2_LOCATION_BACK_2\n> > > \tV4L2_LOCATION_BACK_3\n> > >\n> > > And add some comment at the control documentation that would allow to\n> > > uniquely number the other ones, like:\n> > >\n> > > \t\"when multiple sensors are present at the same side, sensors\n> > > \t will be numbered considering the ``(x,y)`` coordinates of the center\n> > > \t of each sensor, starting from the topmost, leftmost position.\n> > >\n> > > \t She first sensor will be the topmost sensor column at the leftmost\n> > > \t side. The other sensors that will have the same ``y`` coordinate,\n> > > \t counting from the left to the right, then increment the ``y`` and\n> > > \t parse the next column again until all sensors are numbered.\" \n> >\n> > I think this isn't a good idea. In most cases you do not care about this.\n> >\n> > And if you do care about this, then wouldn't it be better to do that through\n> > a new control where you provide the precise coordinates in e.g. mm?\n> >\n> > BACK_1/2/3 really doesn't tell you anything other than that there are three\n> > sensors on the back, but we knew that already.\n> >\n> > If we need support for the precise location in the future, then let's do that\n> > right and not try to shoehorn into something that wasn't meant for it. \n> \n> I think the best move forward to describe movable cameras and such\n> would be to provide a 3D rotation matrix, along the lines of what iio\n> has in the 'mount-matrix' property as suggested by Rob and Laurent in\n> the review of the series.\n> \n> Before going the 'easy' way with this proeprty that just allow to\n> enumerate fixed locations I considered the idea, but we're still\n> missing a unique definition for the device usage orientation that the\n> rotation matrix would be defined for.\n> \n> This property implements a mechanism that covers most of devices out\n> there and all devices in mainline. The properties defined here are the\n> most basic ones, and could be combined and expanded to provide more\n> precise definition is someone needs to do so (expecially downstream),\n> but the important part is that the mechanism to retrieve the\n> information is in place.\n\nI had some discussions with Laurent about that.\n\nYeah, a 3D rotation matrix could work. Another option would be to\nname this as CID_LENS_FACING, use about the same definition as on\nAndroid:\n\n https://jmondi.org/android_metadata_tags/docs.html#static_android.lens.poseTranslation\n\nThe definition there is arguable (as some devices may have back\nscreens nowadays), but a name like that is what this control\nreally does, as it doesn't neither provide a rotation matrix\nnor a camera location.\n\nStarting with a \"read-only\" control sound OK to me, but I would\nadd some note about flippable changes that can be changed in\nruntime between back/front position.\n\nSomething like:\n\n.. note:\n\n\tSensors that could have it side flipped is currently out\n\tof the scope of this control. Some changes on the behavior\n\tof this control may change when support for such kind of\n\tdevices would be added upstream.\n\nThanks,\nMauro","headers":{"Return-Path":"","Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[IPv6:2607:7c80:54:e::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E6F81603F2\n\tfor ;\n\tWed, 6 May 2020 13:09:29 +0200 (CEST)","from ip5f5ad5c5.dynamic.kabel-deutschland.de ([95.90.213.197]\n\thelo=coco.lan)\n\tby bombadil.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat\n\tLinux)) id 1jWHvR-0005bJ-Ir; Wed, 06 May 2020 11:09:17 +0000"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=infradead.org\n\theader.i=@infradead.org\n\theader.b=\"bit31eS1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=infradead.org; s=bombadil.20170209; h=Content-Transfer-Encoding:\n\tContent-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To:\n\tFrom:Date:Sender:Reply-To:Content-ID:Content-Description;\n\tbh=tklJkt/qMJNRgdzu1RQc4mu57ElnC8FdgyYmjzia984=;\n\tb=bit31eS1mdznnVmStMpZfj95GT\n\tuC9McBtkeUpQhcrdr2uTrmsEl9nZh35JRb2GrqF5siinKuC7kYZu7NZwJ+2KQie9Voxgq1BHXGeKP\n\tn7iBRU4kx6YQ/mhD51fDGyUJFvxfYp6gOmVvdjQeBlV4D56IjJRFSwjVSwCCnikS1A/nNJ/vGfI+Y\n\tEENGLHZ+qQSV/1nXAw7vWA46s/rp3Mh+nBfLPps3zRXce9uIfJDTwpyZdAy/870I1n4VB24A047ai\n\tIVglKm/wErfGv6vhbsRauMGNlCXn20Ut7aytxwNZG/fs2dGUebyy5ACzWrzQM9vt2Csb1zBqJOBV1\n\tMSLEi2hw==;","Date":"Wed, 6 May 2020 13:09:13 +0200","From":"Mauro Carvalho Chehab ","To":"Jacopo Mondi ","Cc":"Hans Verkuil , Sakari Ailus\n\t, Laurent Pinchart\n\t, tfiga@google.com, pavel@ucw.cz,\n\t\"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","Message-ID":"<20200506130913.7df929ff@coco.lan>","In-Reply-To":"<20200506104723.l3wojjyefvazohpd@uno.localdomain>","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200506104723.l3wojjyefvazohpd@uno.localdomain>","X-Mailer":"Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-redhat-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Wed, 06 May 2020 11:09:30 -0000"}},{"id":4750,"web_url":"https://patchwork.libcamera.org/comment/4750/","msgid":"<20200506132847.03860fce@coco.lan>","date":"2020-05-06T11:28:47","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":47,"url":"https://patchwork.libcamera.org/api/people/47/","name":"Mauro Carvalho Chehab","email":"mchehab@kernel.org"},"content":"Em Wed, 6 May 2020 13:07:30 +0200\nJacopo Mondi escreveu:\n\n> > So, if that would be ok for you, I can live with a\n> >\n> > V4L2_CID_CAMERA_VIEWING_ANGLE (or some similar name) that will\n> > specify the angle where the sensor is mounted (for fixed sensors),\n> > or the current angle, in case of movable ones, being RO for fixed\n> > sensors and RW for movable ones.\n> >\n> > Let's postpone discussions for a LOCATION control once this\n> > would be needed by some driver. \n> \n> Would V4L2_CID_CAMERA_ORIENTATION work ?\n\nYeah, either V4L2_CID_CAMERA_ORIENTATION or CID_LENS_FACING would\nequally work (although I would prefer the one with a shorter name).\n\n> \n> I could:\n> 1) rename dt-proeprty and control to use orientation\n> 2) specify multiple locations could be entered, the first one being\n> the \"default\" (eg. device turned off) location\n> 3) make am RW control if multiple entries have been specified, a RO\n> otherwise.\n> \n> Ack ?\n\nYeah, that would work for me. \n\nThanks,\nMauro","headers":{"Return-Path":"","Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[IPv6:2607:7c80:54:e::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A37B7603F2\n\tfor ;\n\tWed, 6 May 2020 13:29:01 +0200 (CEST)","from ip5f5ad5c5.dynamic.kabel-deutschland.de ([95.90.213.197]\n\thelo=coco.lan)\n\tby bombadil.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat\n\tLinux)) id 1jWIEN-0002fb-3T; Wed, 06 May 2020 11:28:51 +0000"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=infradead.org\n\theader.i=@infradead.org\n\theader.b=\"XJmLHyy7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=infradead.org; s=bombadil.20170209; h=Content-Transfer-Encoding:\n\tContent-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To:\n\tFrom:Date:Sender:Reply-To:Content-ID:Content-Description;\n\tbh=dVhoKCrJYU8ynbHvWDZWWaj7USYLeiy+4xjGqbhr0P4=;\n\tb=XJmLHyy750kaWjLK5KGJEbzB8w\n\ttMZK5SRgk24msGKRrrVupuCfakUl+a+9WXtsH9aF9Efw8ni7uOnby62kLwzCnTud0KFL2j4FbOiBc\n\tcOwHD7W3NpiCG6QsgPqvtQKCHN/lLMt+vefzYtPTwR57hrnRSX05OJch2BjXl4LwN7VwCZAId8WCD\n\tShzSW07Qbxj3nxwhUKlnHlKFKnMQJ0s7M11xQl82U6FuyrR12j/jmB4kahYShatNusxUlBtQS8ZYN\n\tmdmAC0RN3+qC3TQN6K/EPfdpBcn2odmpNQ3BH82fJlbIfCWPbfQaDvSlQJu4T/3PTNTZBzDxRU0ea\n\tLoVV9IGw==;","Date":"Wed, 6 May 2020 13:28:47 +0200","From":"Mauro Carvalho Chehab ","To":"Jacopo Mondi ","Cc":"Hans Verkuil , Sakari Ailus\n\t, Laurent Pinchart\n\t, tfiga@google.com, pavel@ucw.cz,\n\t\"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","Message-ID":"<20200506132847.03860fce@coco.lan>","In-Reply-To":"<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200505165826.1ce8bb0d@coco.lan>\n\t\n\t<20200506113909.1489bd1e@coco.lan>\n\t<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>","X-Mailer":"Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-redhat-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Wed, 06 May 2020 11:29:01 -0000"}},{"id":4752,"web_url":"https://patchwork.libcamera.org/comment/4752/","msgid":"<20200506154741.GB15206@pendragon.ideasonboard.com>","date":"2020-05-06T15:47:41","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, May 06, 2020 at 01:07:30PM +0200, Jacopo Mondi wrote:\n> On Wed, May 06, 2020 at 11:39:09AM +0200, Mauro Carvalho Chehab wrote:\n> > Em Wed, 6 May 2020 10:04:39 +0200 Hans Verkuil escreveu:\n> >> On 05/05/2020 16:58, Mauro Carvalho Chehab wrote:\n> >>> Em Tue, 5 May 2020 14:21:38 +0200 Hans Verkuil escreveu:\n> >>>> On 05/05/2020 14:02, Mauro Carvalho Chehab wrote:\n> >>>>> Em Fri, 17 Apr 2020 14:41:01 +0200 Jacopo Mondi escreveu:\n> >>>>>> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n> >>>>>> control. The newly added read-only control reports the camera device\n> >>>>>> mounting position.\n> >>>>>>\n> >>>>>> Signed-off-by: Jacopo Mondi \n> >>>>>> ---\n> >>>>>> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n> >>>>>> 1 file changed, 32 insertions(+)\n> >>>>>>\n> >>>>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >>>>>> index e39f84d2447f..01a9042d53a6 100644\n> >>>>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >>>>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> >>>>>> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n> >>>>>> value down. A value of zero stops the motion if one is in progress\n> >>>>>> and has no effect otherwise.\n> >>>>>>\n> >>>>>> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n> >>>>>> + This read-only control describes the camera sensor location by reporting\n> >>>>>> + its mounting position on the device where the camera is installed. The\n> >>>>>> + control value is constant and not modifiable by software. This control is\n> >>>>>> + particularly meaningful for devices which have a well defined orientation,\n> >>>>>> + such as phones, laptops and portable devices since the camera location is\n> >>>>>> + expressed as a position relative to the device's intended usage orientation.\n> >>>>>> + For example, a camera sensor installed on the user-facing side of a phone,\n> >>>>>> + a tablet or a laptop device is said to be installed in the\n> >>>>>> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n> >>>>>> + opposite the front one are said to be installed in the\n> >>>>>> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n> >>>>>> + the device or attached in a way that allows them to move freely, such as\n> >>>>>> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n> >>>>>> + location.\n> >>>>>> +\n> >>>>>> +\n> >>>>>> +\n> >>>>>> +.. flat-table::\n> >>>>>> + :header-rows: 0\n> >>>>>> + :stub-columns: 0\n> >>>>>> +\n> >>>>>> + * - ``\n> >>>>> ``\n> >>>>>> + - The camera sensor is located on the front side of the device.\n> >>>>>> + * - ``V4L2_LOCATION_BACK``\n> >>>>>> + - The camera sensor is located on the back side of the device.\n> >>>>>> + * - ``V4L2_LOCATION_EXTERNAL``\n> >>>>>> + - The camera sensor is not directly attached to the device and is\n> >>>>>> + freely movable.\n> >>>>>\n> >>>>> I guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\n> >>>>> for two reasons:\n> >>>>>\n> >>>>> 1) newer devices may all top of the line mobile devices now are coming\n> >>>>> with multiple camera sensors at the same side. So, just saying that\n> >>>>> the location is front or back is not enough. A location syscall would\n> >>>>> need have something more to better identify the location.\n> >>>>> It probably doesn't need to be something fancy, but, at least, on a\n> >>>>> device with 3 back sensors, I would call them as:\n> >>>>>\n> >>>>> \tV4L2_LOCATION_BACK_1\n> >>>>> \tV4L2_LOCATION_BACK_2\n> >>>>> \tV4L2_LOCATION_BACK_3\n> >>>>>\n> >>>>> And add some comment at the control documentation that would allow to\n> >>>>> uniquely number the other ones, like:\n> >>>>>\n> >>>>> \t\"when multiple sensors are present at the same side, sensors\n> >>>>> \t will be numbered considering the ``(x,y)`` coordinates of the center\n> >>>>> \t of each sensor, starting from the topmost, leftmost position.\n> >>>>>\n> >>>>> \t She first sensor will be the topmost sensor column at the leftmost\n> >>>>> \t side. The other sensors that will have the same ``y`` coordinate,\n> >>>>> \t counting from the left to the right, then increment the ``y`` and\n> >>>>> \t parse the next column again until all sensors are numbered.\"\n> >>>>\n> >>>> I think this isn't a good idea. In most cases you do not care about this.\n> >>>\n> >>> True, because on most cases, the userspace is hardcoded to open, let's say,\n> >>> video0 for the front sensor or video1 for the back sensor.\n> >>>\n> >>> This control only makes sense if the userspace is generic enough to accept\n> >>> sensors on different positions, identifying them at runtime.\n> >>>\n> >>> With the current proposal, userspace can only work with 2 sensors, as, if\n> >>> there's a third sensor, userspace won't know how to pick the right one.\n> >>>\n> >>> For instance, let's assume a car with 4 sensors, one on each side of\n> >>> the car (right, front); (left, front); (right; back); (left; back).\n> >>>\n> >>> With the current proposal, userspace can't do anything if it wants\n> >>> to identify the (right, back) camera.\n> >>>\n> >>>> And if you do care about this, then wouldn't it be better to do that through\n> >>>> a new control where you provide the precise coordinates in e.g. mm?\n> >>>>\n> >>>> BACK_1/2/3 really doesn't tell you anything other than that there are three\n> >>>> sensors on the back, but we knew that already.\n> >>>\n> >>> No, if we define some criteria about how sensors should be accounted for\n> >>> (something similar to the text I drafted), the location will be defined.\n> >>>\n> >>> With the above text, for example, a device with 3 sensors horizontally\n> >>> aligned, the arrangement will be:\n> >>>\n> >>> - sensor 1 is on the left;\n> >>> - sensor 2 in the middle;\n> >>> - sensor 3 is on the right.\n> >>\n> >> Or sensor 2 is below sensor 1 and sensor 3 is to the right of sensor 1.\n> >> It's meaningless information. If you want to specify the location, then\n> >> be precise. Especially for stereoscopic sensors (left and right) it is\n> >> good to know the exact distance between the sensors. Just calling them\n> >> '1' and '2' is not enough.\n> >>\n> >> For sensors you want to know the plane (back/front) and where they are\n> >> on that plane (in the case of more than one sensor). That's separate\n> >> information that's only needed in the case of more than one sensor.\n> >>\n> >>>\n> >>> Ok, I agree that writing a text with such criteria sucks, and maybe\n> >>> just numbering from 1 to n may not be the best thing to do. Yet,\n> >>> adding coordinates in mm would be just too much information, IMHO.\n> >>\n> >> Why? Just numbering them makes no sense, it's useless information.\n> >>\n> >>>> If we need support for the precise location in the future, then let's do that\n> >>>> right and not try to shoehorn into something that wasn't meant for it.\n> >>>\n> >>> Assuming that all the problems we have so far are to support devices with\n> >>> 2 sensors, by the time we add support for a third sensor, we'll end having\n> >>> a new ioctl for the same thing: to specify the sensors locations.\n> >>\n> >> It's just a control, nothing more.\n> >>\n> >> In most cases all you need to know is if it is a front or back sensor. In\n> >> some cases you need to know more: e.g. my Samsung Note 10+ has three sensors\n> >> on the back in a vertical row (wide, telephoto, ultrawide), and two sensors\n> >> for 3D to the right of them. For those last two you need to know the exact\n> >> position relative to one another. For the other sensors all you need to know\n> >> is that they are back sensors.\n> >>\n> >>> We know the drill: having two controls for the same thing makes userspace\n> >>> more complex and will require backward-compatibility code at the kernel\n> >>> and at userspace. That's what I want to avoid.\n> >>>\n> >>> I'm open to other suggestions that won't limit the usage of this control\n> >>> for devices with just (up to) two sensors.\n> >>\n> >> What backward compatibility code are you talking about? I honestly don't see\n> >> the problem here.\n> >\n> > Right now, we're adding an API that assumes that the video node may have\n> > only up to 2 sensors, and that would cover just one small subset of usecases\n> > (see more below). If it has anything more than that, this control won't help.\n> \n> I don't agree the number of sensor is limited to 2. This property does\n> not identify sensors, it describes one more thing that userspace might\n> use to filter cameras. I was actually suprised nothing like this\n> existed in Linux when I started looking into this issue, as this seems\n> to me quite basic information that a generic enough userspace\n> application would like to be able to retrieve.\n> \n> TL;DR: you can describe as many BACK cameras you want, the 'location'\n> gives you -one- filtering criteria more, that's it.\n> \n> > One day (probably soon enough, as there are several devices with more than\n> > two sensors already), we'll end adding a proper support for it, and this\n> > control will become obsoleted, requiring us to think about backward\n> > compatibility issues when this control become deprecated.\n> >\n> > That's why I prefer spending some time finding a better way to report it,\n> > avoiding the need of having to do some deprecation logic anytime soon.\n> \n> As said and discussed during the review of this series, a 3-d rotation\n> matrix is probably the right direction. I refrained from taking that\n> path because:\n> 1) 99% of devices are interested in reporting FRONT/BACK or some\n> specialization of those. Asking dt to provide a 9 entries matrix to\n> say \"FRONT\" seemed an overkill.\n> 2) There is no consensus on how the reference plane should be defined,\n> given the wide range of devices that we target. This is a separate\n> discussion on itself, and given it took 6 months to get to the point\n> of considering these basic properties, I'm a bit skeptical such a\n> discussion would have taken less than that.\n> \n> >>>>>\n> >>>>> 2) There are also some devices that has a movable sensor, that can either\n> >>>>> be taking a picture from the front or from the back, like those:\n> >>>>>\n> >>>>> \thttps://www.youtube.com/watch?v=br6G7MrkRUc\n> >>>>>\n> >>>>> On such case, the control should not be read-only, as one may need to\n> >>>>> change this control in order to select if a sensor would either be on\n> >>>>> FRONT or on BACK position.\n> >>>>>\n> >>>>> For such kind of sensors (when we start supporting them), we could\n> >>>>> for example call them like:\n> >>>>>\n> >>>>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_1\n> >>>>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_2\n> >>>>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_1\n> >>>>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_2\n> >>>>\n> >>>> I don't like this. If the driver can tell when the position changes, then it\n> >>>> can update the control's value (it's still read-only because userspace\n> >>>> can't write to it, but that doesn't mean it can't be updated).\n> >>>\n> >>> Why userspace can't set it? I mean, if the camera is movable, it\n> >>> should be up to the application to select the sensor between FRONT\n> >>> and BACK.\n> >>\n> >> Ah, right. If you can command the camera to flip from back to front using\n> >> a button or something, then yes, it can be writable. Sorry, didn't think of\n> >> that. I was thinking that the user would manually move the camera and the\n> >> new position would be detected by the driver and reported in the location\n> >> control.\n> >>\n> >> In any case, if the location control can be set through the driver by setting\n> >> this control, then just drop the READ_ONLY flag. If the control is writable,\n> >> then the sensor is movable. Just document this and we're done.\n> >\n> > Works for me.\n> \n> This makes sense, it will impact bindings in the sense that it now\n> becomes possible to specify several locations to which select from,\n> which will become the items of the menu control (with some rule that\n> says \"the first is the default\" or such). If more than one location is\n> specified the control is RW, RO otherwise.\n> \n> >> You are making this much more complicated than it need to be IMHO.\n> >>\n> >>> Btw, this is a case where I clearly see value on this ioctl: all cameras\n> >>\n> >> It's a *control*, not a new ioctl.\n> >>\n> >>> with flippable sensors need a control to switch the sensor's position,\n> >>> even if the sensor device is hardcoded on some application.\n> >>>\n> >>>> So there is\n> >>>> no need to call it 'MOVABLE', you just report the correct location. And with\n> >>>> QUERYMENU you can tell that it is movable since multiple possible locations\n> >>>> are reported (BACK and FRONT in this example). If it is fixed, then QUERYMENU\n> >>>> will report only a single location.\n> >>>>\n> >>>> This might have some consequences for the DT bindings, though. Not sure\n> >>>> how to represent this there.\n> >>>\n> >>> I guess DT should contain the default value when the device is turned\n> >>> off.\n> >>>\n> >>>> If the driver cannot tell what the position is, then it makes no sense for\n> >>>> the driver to expose this location control since it clearly is something that\n> >>>> has to be hardcoded in userspace. I.e., there is no point for userspace to\n> >>>> write to the control and then read back what it wrote :-)\n> >>>\n> >>> Actually there is. When you command a device to switch position, it may\n> >>> take some time to move the sensor, and such operation may even fail.\n> >>\n> >> Yeah, I forgot about that option.\n> >>\n> >>>\n> >>> So, reading back the position is probably mandatory.\n> >>\n> >> Well, it's a control, so that's standard.\n> >>\n> >>>\n> >>> That reminds that it may actually have a third position, to warn\n> >>> that the sensor was blocked.\n> >>>\n> >>> Also, some flip sensors may have another position (a \"closed\"\n> >>> position).\n> >>\n> >> It's certainly possible that we need to add new positions to support the\n> >> various states of such a movable sensor. But that's no problem. It's just\n> >> a menu control, adding new positions is cheap and easy.\n> >>\n> >> I stand by what I said, except that I agree that this control can be\n> >> writable in some circumstances, and that should be documented\n> >>\n> >> I strongly disagree with the notion of BACK_1/2/3 and FRONT_1/2/3: it adds\n> >> no meaningful information.\n> >\n> > Ok, but if this control would just say where a sensor is mounted\n> > (front, back or unknown/external), naming it as \"LOCATION\" seems too\n> > ambitious ;-)\n> >\n> > What it is actually trying to report is the angle of the sensor, with\n> > regards to the front position, adding currently two possible angles:\n> > 0 degrees (front) or 180 degrees (back).\n> >\n> > So, I would call it, instead, as V4L2_CID_CAMERA_VIEWING_ANGLE\n> > (or something similar to it).\n> \n> _ORIENTATION might be the right word, I'm fine to reserve _LOCATION\n> for a more precise property if that helps moving forward.\n> \n> > Having just two pre-defined angles (front/back) only works fine on\n> > devices like cell-phones or tablets, where the sensor cannot be\n> > on some other angle.\n> >\n> > If you mount cameras on a larger device, like a car, you may have\n> > some cameras mounted with different angles, for example, the front\n> > cameras could have been mounted with -45, 0 and 45 degrees, in order\n> > to cover a larger region.\n> \n> I considered that case, but I expect those very specific usages to be\n> covered by downstream extensions of the property supported values. I\n> wish we had a .dts to describe a car in mainlien, but I would be happy\n> enough to provide a standard mechanism for people to use downstream\n> eventually, instead of adding custom properties, or taking shortcuts\n> like it mostly happens today.\n> \n> > So, if that would be ok for you, I can live with a\n> >\n> > V4L2_CID_CAMERA_VIEWING_ANGLE (or some similar name) that will\n> > specify the angle where the sensor is mounted (for fixed sensors),\n> > or the current angle, in case of movable ones, being RO for fixed\n> > sensors and RW for movable ones.\n> >\n> > Let's postpone discussions for a LOCATION control once this\n> > would be needed by some driver.\n> \n> Would V4L2_CID_CAMERA_ORIENTATION work ?\n> \n> I could:\n> 1) rename dt-proeprty and control to use orientation\n> 2) specify multiple locations could be entered, the first one being\n> the \"default\" (eg. device turned off) location\n> 3) make am RW control if multiple entries have been specified, a RO\n> otherwise.\n\nI would refrain from doing 2) and 3) at this point. We have no idea how\nwe will control those devices, as we haven't worked with them, and we\ndon't know whether flipping the camera could be done through the V4L2\nsubsystem or would need to involve other APIs. Designing APIs that can't\nbe tested has so far not been a great success. It's easy to specify the\nDT property as a single value and the control as read-only and ease\nthose restrictions later, it will be more difficult to start with the\nread-write case and then change it to something else if we realize it\nwas a bad idea.","headers":{"Return-Path":"","Received":["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 2F2DB603F2\n\tfor ;\n\tWed, 6 May 2020 17:47:47 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 74E28542;\n\tWed, 6 May 2020 17:47:46 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hu7/BjAJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1588780066;\n\tbh=xKyuqGZpJAkeeoGZz6G2JEo//dgEHFtxij9vI5Zt/Us=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hu7/BjAJZ767Lt9sWd12c/UbbzfPzXMjYCnQ+8UbYdbcF/c3O1acMkAOXNJ6NdYWp\n\tcITemgTaRngArtaBa33V92rUR7151d4gbd31VfGgFdDW5bsu7g7Hv++KFl/jGssAW/\n\tnsKnlSIOmIgq2ssfHKqZVns2lkxzJP4+dWoCYRug=","Date":"Wed, 6 May 2020 18:47:41 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Mauro Carvalho Chehab ,\n\tHans Verkuil ,\n\tSakari Ailus , tfiga@google.com,\n\tpavel@ucw.cz, \"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","Message-ID":"<20200506154741.GB15206@pendragon.ideasonboard.com>","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200505165826.1ce8bb0d@coco.lan>\n\t\n\t<20200506113909.1489bd1e@coco.lan>\n\t<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Wed, 06 May 2020 15:47:47 -0000"}},{"id":4758,"web_url":"https://patchwork.libcamera.org/comment/4758/","msgid":"<20200507122958.2ot7aqtowcutiobe@uno.localdomain>","date":"2020-05-07T12:29:58","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, May 06, 2020 at 06:47:41PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Wed, May 06, 2020 at 01:07:30PM +0200, Jacopo Mondi wrote:\n> > On Wed, May 06, 2020 at 11:39:09AM +0200, Mauro Carvalho Chehab wrote:\n> > > Em Wed, 6 May 2020 10:04:39 +0200 Hans Verkuil escreveu:\n> > >> On 05/05/2020 16:58, Mauro Carvalho Chehab wrote:\n> > >>> Em Tue, 5 May 2020 14:21:38 +0200 Hans Verkuil escreveu:\n> > >>>> On 05/05/2020 14:02, Mauro Carvalho Chehab wrote:\n> > >>>>> Em Fri, 17 Apr 2020 14:41:01 +0200 Jacopo Mondi escreveu:\n> > >>>>>> Add documentation for the V4L2_CID_CAMERA_SENSOR_LOCATION camera\n> > >>>>>> control. The newly added read-only control reports the camera device\n> > >>>>>> mounting position.\n> > >>>>>>\n> > >>>>>> Signed-off-by: Jacopo Mondi \n> > >>>>>> ---\n> > >>>>>> .../media/v4l/ext-ctrls-camera.rst | 32 +++++++++++++++++++\n> > >>>>>> 1 file changed, 32 insertions(+)\n> > >>>>>>\n> > >>>>>> diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> > >>>>>> index e39f84d2447f..01a9042d53a6 100644\n> > >>>>>> --- a/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> > >>>>>> +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-camera.rst\n> > >>>>>> @@ -510,6 +510,38 @@ enum v4l2_scene_mode -\n> > >>>>>> value down. A value of zero stops the motion if one is in progress\n> > >>>>>> and has no effect otherwise.\n> > >>>>>>\n> > >>>>>> +``V4L2_CID_CAMERA_SENSOR_LOCATION (integer)``\n> > >>>>>> + This read-only control describes the camera sensor location by reporting\n> > >>>>>> + its mounting position on the device where the camera is installed. The\n> > >>>>>> + control value is constant and not modifiable by software. This control is\n> > >>>>>> + particularly meaningful for devices which have a well defined orientation,\n> > >>>>>> + such as phones, laptops and portable devices since the camera location is\n> > >>>>>> + expressed as a position relative to the device's intended usage orientation.\n> > >>>>>> + For example, a camera sensor installed on the user-facing side of a phone,\n> > >>>>>> + a tablet or a laptop device is said to be installed in the\n> > >>>>>> + ``V4L2_LOCATION_FRONT`` location while camera sensors installed on the side\n> > >>>>>> + opposite the front one are said to be installed in the\n> > >>>>>> + ``V4L2_LOCATION_BACK`` location. Camera sensors not directly attached to\n> > >>>>>> + the device or attached in a way that allows them to move freely, such as\n> > >>>>>> + webcams and digital cameras, are said to have the ``V4L2_LOCATION_EXTERNAL``\n> > >>>>>> + location.\n> > >>>>>> +\n> > >>>>>> +\n> > >>>>>> +\n> > >>>>>> +.. flat-table::\n> > >>>>>> + :header-rows: 0\n> > >>>>>> + :stub-columns: 0\n> > >>>>>> +\n> > >>>>>> + * - ``\n> > >>>>> ``\n> > >>>>>> + - The camera sensor is located on the front side of the device.\n> > >>>>>> + * - ``V4L2_LOCATION_BACK``\n> > >>>>>> + - The camera sensor is located on the back side of the device.\n> > >>>>>> + * - ``V4L2_LOCATION_EXTERNAL``\n> > >>>>>> + - The camera sensor is not directly attached to the device and is\n> > >>>>>> + freely movable.\n> > >>>>>\n> > >>>>> I guess I mentioned this already, but IMHO this ioctl is somewhat flawed,\n> > >>>>> for two reasons:\n> > >>>>>\n> > >>>>> 1) newer devices may all top of the line mobile devices now are coming\n> > >>>>> with multiple camera sensors at the same side. So, just saying that\n> > >>>>> the location is front or back is not enough. A location syscall would\n> > >>>>> need have something more to better identify the location.\n> > >>>>> It probably doesn't need to be something fancy, but, at least, on a\n> > >>>>> device with 3 back sensors, I would call them as:\n> > >>>>>\n> > >>>>> \tV4L2_LOCATION_BACK_1\n> > >>>>> \tV4L2_LOCATION_BACK_2\n> > >>>>> \tV4L2_LOCATION_BACK_3\n> > >>>>>\n> > >>>>> And add some comment at the control documentation that would allow to\n> > >>>>> uniquely number the other ones, like:\n> > >>>>>\n> > >>>>> \t\"when multiple sensors are present at the same side, sensors\n> > >>>>> \t will be numbered considering the ``(x,y)`` coordinates of the center\n> > >>>>> \t of each sensor, starting from the topmost, leftmost position.\n> > >>>>>\n> > >>>>> \t She first sensor will be the topmost sensor column at the leftmost\n> > >>>>> \t side. The other sensors that will have the same ``y`` coordinate,\n> > >>>>> \t counting from the left to the right, then increment the ``y`` and\n> > >>>>> \t parse the next column again until all sensors are numbered.\"\n> > >>>>\n> > >>>> I think this isn't a good idea. In most cases you do not care about this.\n> > >>>\n> > >>> True, because on most cases, the userspace is hardcoded to open, let's say,\n> > >>> video0 for the front sensor or video1 for the back sensor.\n> > >>>\n> > >>> This control only makes sense if the userspace is generic enough to accept\n> > >>> sensors on different positions, identifying them at runtime.\n> > >>>\n> > >>> With the current proposal, userspace can only work with 2 sensors, as, if\n> > >>> there's a third sensor, userspace won't know how to pick the right one.\n> > >>>\n> > >>> For instance, let's assume a car with 4 sensors, one on each side of\n> > >>> the car (right, front); (left, front); (right; back); (left; back).\n> > >>>\n> > >>> With the current proposal, userspace can't do anything if it wants\n> > >>> to identify the (right, back) camera.\n> > >>>\n> > >>>> And if you do care about this, then wouldn't it be better to do that through\n> > >>>> a new control where you provide the precise coordinates in e.g. mm?\n> > >>>>\n> > >>>> BACK_1/2/3 really doesn't tell you anything other than that there are three\n> > >>>> sensors on the back, but we knew that already.\n> > >>>\n> > >>> No, if we define some criteria about how sensors should be accounted for\n> > >>> (something similar to the text I drafted), the location will be defined.\n> > >>>\n> > >>> With the above text, for example, a device with 3 sensors horizontally\n> > >>> aligned, the arrangement will be:\n> > >>>\n> > >>> - sensor 1 is on the left;\n> > >>> - sensor 2 in the middle;\n> > >>> - sensor 3 is on the right.\n> > >>\n> > >> Or sensor 2 is below sensor 1 and sensor 3 is to the right of sensor 1.\n> > >> It's meaningless information. If you want to specify the location, then\n> > >> be precise. Especially for stereoscopic sensors (left and right) it is\n> > >> good to know the exact distance between the sensors. Just calling them\n> > >> '1' and '2' is not enough.\n> > >>\n> > >> For sensors you want to know the plane (back/front) and where they are\n> > >> on that plane (in the case of more than one sensor). That's separate\n> > >> information that's only needed in the case of more than one sensor.\n> > >>\n> > >>>\n> > >>> Ok, I agree that writing a text with such criteria sucks, and maybe\n> > >>> just numbering from 1 to n may not be the best thing to do. Yet,\n> > >>> adding coordinates in mm would be just too much information, IMHO.\n> > >>\n> > >> Why? Just numbering them makes no sense, it's useless information.\n> > >>\n> > >>>> If we need support for the precise location in the future, then let's do that\n> > >>>> right and not try to shoehorn into something that wasn't meant for it.\n> > >>>\n> > >>> Assuming that all the problems we have so far are to support devices with\n> > >>> 2 sensors, by the time we add support for a third sensor, we'll end having\n> > >>> a new ioctl for the same thing: to specify the sensors locations.\n> > >>\n> > >> It's just a control, nothing more.\n> > >>\n> > >> In most cases all you need to know is if it is a front or back sensor. In\n> > >> some cases you need to know more: e.g. my Samsung Note 10+ has three sensors\n> > >> on the back in a vertical row (wide, telephoto, ultrawide), and two sensors\n> > >> for 3D to the right of them. For those last two you need to know the exact\n> > >> position relative to one another. For the other sensors all you need to know\n> > >> is that they are back sensors.\n> > >>\n> > >>> We know the drill: having two controls for the same thing makes userspace\n> > >>> more complex and will require backward-compatibility code at the kernel\n> > >>> and at userspace. That's what I want to avoid.\n> > >>>\n> > >>> I'm open to other suggestions that won't limit the usage of this control\n> > >>> for devices with just (up to) two sensors.\n> > >>\n> > >> What backward compatibility code are you talking about? I honestly don't see\n> > >> the problem here.\n> > >\n> > > Right now, we're adding an API that assumes that the video node may have\n> > > only up to 2 sensors, and that would cover just one small subset of usecases\n> > > (see more below). If it has anything more than that, this control won't help.\n> >\n> > I don't agree the number of sensor is limited to 2. This property does\n> > not identify sensors, it describes one more thing that userspace might\n> > use to filter cameras. I was actually suprised nothing like this\n> > existed in Linux when I started looking into this issue, as this seems\n> > to me quite basic information that a generic enough userspace\n> > application would like to be able to retrieve.\n> >\n> > TL;DR: you can describe as many BACK cameras you want, the 'location'\n> > gives you -one- filtering criteria more, that's it.\n> >\n> > > One day (probably soon enough, as there are several devices with more than\n> > > two sensors already), we'll end adding a proper support for it, and this\n> > > control will become obsoleted, requiring us to think about backward\n> > > compatibility issues when this control become deprecated.\n> > >\n> > > That's why I prefer spending some time finding a better way to report it,\n> > > avoiding the need of having to do some deprecation logic anytime soon.\n> >\n> > As said and discussed during the review of this series, a 3-d rotation\n> > matrix is probably the right direction. I refrained from taking that\n> > path because:\n> > 1) 99% of devices are interested in reporting FRONT/BACK or some\n> > specialization of those. Asking dt to provide a 9 entries matrix to\n> > say \"FRONT\" seemed an overkill.\n> > 2) There is no consensus on how the reference plane should be defined,\n> > given the wide range of devices that we target. This is a separate\n> > discussion on itself, and given it took 6 months to get to the point\n> > of considering these basic properties, I'm a bit skeptical such a\n> > discussion would have taken less than that.\n> >\n> > >>>>>\n> > >>>>> 2) There are also some devices that has a movable sensor, that can either\n> > >>>>> be taking a picture from the front or from the back, like those:\n> > >>>>>\n> > >>>>> \thttps://www.youtube.com/watch?v=br6G7MrkRUc\n> > >>>>>\n> > >>>>> On such case, the control should not be read-only, as one may need to\n> > >>>>> change this control in order to select if a sensor would either be on\n> > >>>>> FRONT or on BACK position.\n> > >>>>>\n> > >>>>> For such kind of sensors (when we start supporting them), we could\n> > >>>>> for example call them like:\n> > >>>>>\n> > >>>>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_1\n> > >>>>> \tV4L2_LOCATION_MOVABLE_IN_BACK_POSITION_2\n> > >>>>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_1\n> > >>>>> \tV4L2_LOCATION_MOVABLE_IN_FRONT_POSITION_2\n> > >>>>\n> > >>>> I don't like this. If the driver can tell when the position changes, then it\n> > >>>> can update the control's value (it's still read-only because userspace\n> > >>>> can't write to it, but that doesn't mean it can't be updated).\n> > >>>\n> > >>> Why userspace can't set it? I mean, if the camera is movable, it\n> > >>> should be up to the application to select the sensor between FRONT\n> > >>> and BACK.\n> > >>\n> > >> Ah, right. If you can command the camera to flip from back to front using\n> > >> a button or something, then yes, it can be writable. Sorry, didn't think of\n> > >> that. I was thinking that the user would manually move the camera and the\n> > >> new position would be detected by the driver and reported in the location\n> > >> control.\n> > >>\n> > >> In any case, if the location control can be set through the driver by setting\n> > >> this control, then just drop the READ_ONLY flag. If the control is writable,\n> > >> then the sensor is movable. Just document this and we're done.\n> > >\n> > > Works for me.\n> >\n> > This makes sense, it will impact bindings in the sense that it now\n> > becomes possible to specify several locations to which select from,\n> > which will become the items of the menu control (with some rule that\n> > says \"the first is the default\" or such). If more than one location is\n> > specified the control is RW, RO otherwise.\n> >\n> > >> You are making this much more complicated than it need to be IMHO.\n> > >>\n> > >>> Btw, this is a case where I clearly see value on this ioctl: all cameras\n> > >>\n> > >> It's a *control*, not a new ioctl.\n> > >>\n> > >>> with flippable sensors need a control to switch the sensor's position,\n> > >>> even if the sensor device is hardcoded on some application.\n> > >>>\n> > >>>> So there is\n> > >>>> no need to call it 'MOVABLE', you just report the correct location. And with\n> > >>>> QUERYMENU you can tell that it is movable since multiple possible locations\n> > >>>> are reported (BACK and FRONT in this example). If it is fixed, then QUERYMENU\n> > >>>> will report only a single location.\n> > >>>>\n> > >>>> This might have some consequences for the DT bindings, though. Not sure\n> > >>>> how to represent this there.\n> > >>>\n> > >>> I guess DT should contain the default value when the device is turned\n> > >>> off.\n> > >>>\n> > >>>> If the driver cannot tell what the position is, then it makes no sense for\n> > >>>> the driver to expose this location control since it clearly is something that\n> > >>>> has to be hardcoded in userspace. I.e., there is no point for userspace to\n> > >>>> write to the control and then read back what it wrote :-)\n> > >>>\n> > >>> Actually there is. When you command a device to switch position, it may\n> > >>> take some time to move the sensor, and such operation may even fail.\n> > >>\n> > >> Yeah, I forgot about that option.\n> > >>\n> > >>>\n> > >>> So, reading back the position is probably mandatory.\n> > >>\n> > >> Well, it's a control, so that's standard.\n> > >>\n> > >>>\n> > >>> That reminds that it may actually have a third position, to warn\n> > >>> that the sensor was blocked.\n> > >>>\n> > >>> Also, some flip sensors may have another position (a \"closed\"\n> > >>> position).\n> > >>\n> > >> It's certainly possible that we need to add new positions to support the\n> > >> various states of such a movable sensor. But that's no problem. It's just\n> > >> a menu control, adding new positions is cheap and easy.\n> > >>\n> > >> I stand by what I said, except that I agree that this control can be\n> > >> writable in some circumstances, and that should be documented\n> > >>\n> > >> I strongly disagree with the notion of BACK_1/2/3 and FRONT_1/2/3: it adds\n> > >> no meaningful information.\n> > >\n> > > Ok, but if this control would just say where a sensor is mounted\n> > > (front, back or unknown/external), naming it as \"LOCATION\" seems too\n> > > ambitious ;-)\n> > >\n> > > What it is actually trying to report is the angle of the sensor, with\n> > > regards to the front position, adding currently two possible angles:\n> > > 0 degrees (front) or 180 degrees (back).\n> > >\n> > > So, I would call it, instead, as V4L2_CID_CAMERA_VIEWING_ANGLE\n> > > (or something similar to it).\n> >\n> > _ORIENTATION might be the right word, I'm fine to reserve _LOCATION\n> > for a more precise property if that helps moving forward.\n> >\n> > > Having just two pre-defined angles (front/back) only works fine on\n> > > devices like cell-phones or tablets, where the sensor cannot be\n> > > on some other angle.\n> > >\n> > > If you mount cameras on a larger device, like a car, you may have\n> > > some cameras mounted with different angles, for example, the front\n> > > cameras could have been mounted with -45, 0 and 45 degrees, in order\n> > > to cover a larger region.\n> >\n> > I considered that case, but I expect those very specific usages to be\n> > covered by downstream extensions of the property supported values. I\n> > wish we had a .dts to describe a car in mainlien, but I would be happy\n> > enough to provide a standard mechanism for people to use downstream\n> > eventually, instead of adding custom properties, or taking shortcuts\n> > like it mostly happens today.\n> >\n> > > So, if that would be ok for you, I can live with a\n> > >\n> > > V4L2_CID_CAMERA_VIEWING_ANGLE (or some similar name) that will\n> > > specify the angle where the sensor is mounted (for fixed sensors),\n> > > or the current angle, in case of movable ones, being RO for fixed\n> > > sensors and RW for movable ones.\n> > >\n> > > Let's postpone discussions for a LOCATION control once this\n> > > would be needed by some driver.\n> >\n> > Would V4L2_CID_CAMERA_ORIENTATION work ?\n> >\n> > I could:\n> > 1) rename dt-proeprty and control to use orientation\n> > 2) specify multiple locations could be entered, the first one being\n> > the \"default\" (eg. device turned off) location\n> > 3) make am RW control if multiple entries have been specified, a RO\n> > otherwise.\n>\n> I would refrain from doing 2) and 3) at this point. We have no idea how\n> we will control those devices, as we haven't worked with them, and we\n> don't know whether flipping the camera could be done through the V4L2\n> subsystem or would need to involve other APIs. Designing APIs that can't\n> be tested has so far not been a great success. It's easy to specify the\n> DT property as a single value and the control as read-only and ease\n> those restrictions later, it will be more difficult to start with the\n> read-write case and then change it to something else if we realize it\n> was a bad idea.\n>\n\nSure we don't have use cases at hand.. I'm fine post-poning then.\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AF0F7603EA\n\tfor ;\n\tThu, 7 May 2020 14:26:49 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 899B41C000B;\n\tThu, 7 May 2020 12:26:46 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 7 May 2020 14:29:58 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"Mauro Carvalho Chehab ,\n\tHans Verkuil ,\n\tSakari Ailus , tfiga@google.com,\n\tpavel@ucw.cz, \"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","Message-ID":"<20200507122958.2ot7aqtowcutiobe@uno.localdomain>","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200505165826.1ce8bb0d@coco.lan>\n\t\n\t<20200506113909.1489bd1e@coco.lan>\n\t<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>\n\t<20200506154741.GB15206@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200506154741.GB15206@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Thu, 07 May 2020 12:26:49 -0000"}},{"id":4759,"web_url":"https://patchwork.libcamera.org/comment/4759/","msgid":"<20200507123649.q3ecciyxulzisn62@uno.localdomain>","date":"2020-05-07T12:36:49","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Mauro,\n\nOn Wed, May 06, 2020 at 01:28:47PM +0200, Mauro Carvalho Chehab wrote:\n> Em Wed, 6 May 2020 13:07:30 +0200\n> Jacopo Mondi escreveu:\n>\n> > > So, if that would be ok for you, I can live with a\n> > >\n> > > V4L2_CID_CAMERA_VIEWING_ANGLE (or some similar name) that will\n> > > specify the angle where the sensor is mounted (for fixed sensors),\n> > > or the current angle, in case of movable ones, being RO for fixed\n> > > sensors and RW for movable ones.\n> > >\n> > > Let's postpone discussions for a LOCATION control once this\n> > > would be needed by some driver.\n> >\n> > Would V4L2_CID_CAMERA_ORIENTATION work ?\n>\n> Yeah, either V4L2_CID_CAMERA_ORIENTATION or CID_LENS_FACING would\n> equally work (although I would prefer the one with a shorter name).\n>\n\nYeah, CID_LENS_FACING is nice and shorter, but I would refrain from\npolluting the LENS_ namespace, this control applies to the whole camera\nmodule, so I would keep it in the CAMERA_ namespace... And\n'orientation' gives a nice match with the DT property, which I would\nnot call 'facing' or 'facing_side' as 'orientation' seems more\nappropriate as a dt-property name to me..\n\n> >\n> > I could:\n> > 1) rename dt-proeprty and control to use orientation\n> > 2) specify multiple locations could be entered, the first one being\n> > the \"default\" (eg. device turned off) location\n> > 3) make am RW control if multiple entries have been specified, a RO\n> > otherwise.\n> >\n> > Ack ?\n>\n> Yeah, that would work for me.\n>\n> Thanks,\n> Mauro","headers":{"Return-Path":"","Received":["from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 303D0603EA\n\tfor ;\n\tThu, 7 May 2020 14:33:39 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id CDCA82000D;\n\tThu, 7 May 2020 12:33:36 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 7 May 2020 14:36:49 +0200","From":"Jacopo Mondi ","To":"Mauro Carvalho Chehab ","Cc":"Hans Verkuil ,\n\tSakari Ailus ,\n\tLaurent Pinchart ,\n\ttfiga@google.com, pavel@ucw.cz,\n\t\"open list:MEDIA INPUT INFRASTRUCTURE (V4L/DVB)\"\n\t, libcamera-devel@lists.libcamera.org","Message-ID":"<20200507123649.q3ecciyxulzisn62@uno.localdomain>","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200505165826.1ce8bb0d@coco.lan>\n\t\n\t<20200506113909.1489bd1e@coco.lan>\n\t<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>\n\t<20200506132847.03860fce@coco.lan>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200506132847.03860fce@coco.lan>","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Thu, 07 May 2020 12:33:39 -0000"}},{"id":4760,"web_url":"https://patchwork.libcamera.org/comment/4760/","msgid":"<20200507160530.21cf5922@coco.lan>","date":"2020-05-07T14:05:54","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":47,"url":"https://patchwork.libcamera.org/api/people/47/","name":"Mauro Carvalho Chehab","email":"mchehab@kernel.org"},"content":"Em Thu, 7 May 2020 14:36:49 +0200\nJacopo Mondi escreveu:\n\n> Hi Mauro,\n> \n> On Wed, May 06, 2020 at 01:28:47PM +0200, Mauro Carvalho Chehab wrote:\n> > Em Wed, 6 May 2020 13:07:30 +0200\n> > Jacopo Mondi escreveu:\n> > \n> > > > So, if that would be ok for you, I can live with a\n> > > >\n> > > > V4L2_CID_CAMERA_VIEWING_ANGLE (or some similar name) that will\n> > > > specify the angle where the sensor is mounted (for fixed sensors),\n> > > > or the current angle, in case of movable ones, being RO for fixed\n> > > > sensors and RW for movable ones.\n> > > >\n> > > > Let's postpone discussions for a LOCATION control once this\n> > > > would be needed by some driver. \n> > >\n> > > Would V4L2_CID_CAMERA_ORIENTATION work ? \n> >\n> > Yeah, either V4L2_CID_CAMERA_ORIENTATION or CID_LENS_FACING would\n> > equally work (although I would prefer the one with a shorter name).\n> > \n> \n> Yeah, CID_LENS_FACING is nice and shorter, but I would refrain from\n> polluting the LENS_ namespace, this control applies to the whole camera\n> module, so I would keep it in the CAMERA_ namespace... And\n> 'orientation' gives a nice match with the DT property, which I would\n> not call 'facing' or 'facing_side' as 'orientation' seems more\n> appropriate as a dt-property name to me..\n\nOk. V4L2_CID_CAMERA_ORIENTATION works for me.\n\nThanks,\nMauro","headers":{"Return-Path":"","Received":["from bombadil.infradead.org (bombadil.infradead.org\n\t[IPv6:2607:7c80:54:e::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A856603EA\n\tfor ;\n\tThu, 7 May 2020 16:06:18 +0200 (CEST)","from ip5f5ad5c5.dynamic.kabel-deutschland.de ([95.90.213.197]\n\thelo=coco.lan)\n\tby bombadil.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat\n\tLinux)) id 1jWh9z-0002UW-Nd; Thu, 07 May 2020 14:06:00 +0000"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=infradead.org; s=bombadil.20170209; h=Content-Transfer-Encoding:\n\tContent-Type:MIME-Version:References:In-Reply-To:Message-ID:Subject:Cc:To:\n\tFrom:Date:Sender:Reply-To:Content-ID:Content-Description;\n\tbh=VdMq/Jc7qCK1txecCcqfPFUFad+GzEPdwS+Cdig9ARE=;\n\tb=GafhsXpWf7nec1WgDx5zoTea0V\n\tXcoaJ0nwre6dEwF02YkufHpl1aIMEdmHc4stcKlvshEHs93tdtsXznmrnGrPIm5Has30xjzQFTIbd\n\t0RK3yPNrLbDiIdYAdlsSzD0NT8+d8gM8Wyyv8sSI2tu3EdbC/9/9hdhJAZ5XcTmxOY5EyXntgpQqn\n\t3+JoLyhL6BsMfXcGUnLRMUvbYIasz/QYkOKGaxy2dRBwTiDU+NZZLUE0PQoh1yE+f1iLUdi1SP5Lh\n\t595FMxf9/3nKspvS/xi6sz/gIbidFWOl8eD0BnMQu9Sfe1sJ+K5HKDsOSt/IyGLgIEExG/QJbTje2\n\tilBP7/MA==;","Date":"Thu, 7 May 2020 16:05:54 +0200","From":"Mauro Carvalho Chehab ","To":"Jacopo Mondi ","Cc":"Hans Verkuil , Sakari Ailus\n\t, Laurent Pinchart\n\t, tfiga@google.com, pavel@ucw.cz, \n\tlinux-media@vger.kernel.org, libcamera-devel@lists.libcamera.org","Message-ID":"<20200507160530.21cf5922@coco.lan>","In-Reply-To":"<20200507123649.q3ecciyxulzisn62@uno.localdomain>","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200505165826.1ce8bb0d@coco.lan>\n\t\n\t<20200506113909.1489bd1e@coco.lan>\n\t<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>\n\t<20200506132847.03860fce@coco.lan>\n\t<20200507123649.q3ecciyxulzisn62@uno.localdomain>","X-Mailer":"Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-redhat-linux-gnu)","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Thu, 07 May 2020 14:06:23 -0000"}},{"id":4761,"web_url":"https://patchwork.libcamera.org/comment/4761/","msgid":"","date":"2020-05-07T14:09:20","subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","submitter":{"id":43,"url":"https://patchwork.libcamera.org/api/people/43/","name":"Hans Verkuil","email":"hverkuil-cisco@xs4all.nl"},"content":"On 07/05/2020 16:05, Mauro Carvalho Chehab wrote:\n> Em Thu, 7 May 2020 14:36:49 +0200\n> Jacopo Mondi escreveu:\n> \n>> Hi Mauro,\n>>\n>> On Wed, May 06, 2020 at 01:28:47PM +0200, Mauro Carvalho Chehab wrote:\n>>> Em Wed, 6 May 2020 13:07:30 +0200\n>>> Jacopo Mondi escreveu:\n>>> \n>>>>> So, if that would be ok for you, I can live with a\n>>>>>\n>>>>> V4L2_CID_CAMERA_VIEWING_ANGLE (or some similar name) that will\n>>>>> specify the angle where the sensor is mounted (for fixed sensors),\n>>>>> or the current angle, in case of movable ones, being RO for fixed\n>>>>> sensors and RW for movable ones.\n>>>>>\n>>>>> Let's postpone discussions for a LOCATION control once this\n>>>>> would be needed by some driver. \n>>>>\n>>>> Would V4L2_CID_CAMERA_ORIENTATION work ? \n>>>\n>>> Yeah, either V4L2_CID_CAMERA_ORIENTATION or CID_LENS_FACING would\n>>> equally work (although I would prefer the one with a shorter name).\n>>> \n>>\n>> Yeah, CID_LENS_FACING is nice and shorter, but I would refrain from\n>> polluting the LENS_ namespace, this control applies to the whole camera\n>> module, so I would keep it in the CAMERA_ namespace... And\n>> 'orientation' gives a nice match with the DT property, which I would\n>> not call 'facing' or 'facing_side' as 'orientation' seems more\n>> appropriate as a dt-property name to me..\n> \n> Ok. V4L2_CID_CAMERA_ORIENTATION works for me.\n\nFor me as well.\n\nRegards,\n\n\tHans","headers":{"Return-Path":"","Received":["from lb3-smtp-cloud9.xs4all.net (lb3-smtp-cloud9.xs4all.net\n\t[194.109.24.30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 79569603EA\n\tfor ;\n\tThu, 7 May 2020 16:09:25 +0200 (CEST)","from cust-b5b5937f ([IPv6:fc0c:c16d:66b8:757f:c639:739b:9d66:799d])\n\tby smtp-cloud9.xs4all.net with ESMTPA\n\tid WhDFjvEgs8hmdWhDIjFl8j; Thu, 07 May 2020 16:09:25 +0200"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"tPNTOCcv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=s1;\n\tt=1588860565; bh=lLmr+AD5eotjWpiEMpcgSEGCJpCEHy7KCO1D7jiFKl0=;\n\th=Subject:To:From:Message-ID:Date:MIME-Version:Content-Type:From:\n\tSubject;\n\tb=tPNTOCcvntbnLaO2xPBwU5fXUOFUyRyWsloVEHbnWxa6UQg2ATQ5gzoq805wRoRPd\n\tZSsEs1PJkVgCNwZp9MerPRoFEnKqr/UUUb8z3L6YPMNbdriYe4fdZTGIu3WIivvXyn\n\tSU9pon0zlLuuy5t0t14Pfn/NLl8FamU5ZzBpggxvKO1SXnHoMX2GrKEO/ciCYmEUTN\n\tO7CHlfm/Cf++6qAXXOV/XgpARgXSBf56pZyl1XyShceEIH6WG9ujTB+hFmfaCr1Jr2\n\taT/LzprS9as6RT/csQkexRk3DKOe13x7EdvYs30P0P9V8Vn1DgBOY2kJ8GszbKzoN7\n\tJiMAep/WmKhZQ==","To":"Mauro Carvalho Chehab ,\n\tJacopo Mondi ","Cc":"Sakari Ailus ,\n\tLaurent Pinchart , tfiga@google.com,\n\tpavel@ucw.cz, linux-media@vger.kernel.org,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200417124110.72313-1-jacopo@jmondi.org>\n\t<20200417124110.72313-3-jacopo@jmondi.org>\n\t<20200505140206.589f54ae@coco.lan>\n\t\n\t<20200505165826.1ce8bb0d@coco.lan>\n\t\n\t<20200506113909.1489bd1e@coco.lan>\n\t<20200506110730.rvhxoh74u3rmemtw@uno.localdomain>\n\t<20200506132847.03860fce@coco.lan>\n\t<20200507123649.q3ecciyxulzisn62@uno.localdomain>\n\t<20200507160530.21cf5922@coco.lan>","From":"Hans Verkuil ","Message-ID":"","Date":"Thu, 7 May 2020 16:09:20 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.5.0","MIME-Version":"1.0","In-Reply-To":"<20200507160530.21cf5922@coco.lan>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-CMAE-Envelope":"MS4wfM3DOzZjUNu0tKgWV5ag7IFciT/xB/yn8j1RAE947PHPVMXO3kMS+FtQ0vPBoKLpbYxngFIam8phUyPkfX13182ooV8f/24beVAl5GjFzHo9DjUzoeUM\n\tYPWlSb1NnFPfSOsgrFmoqwkJ/Y2zvAOnShRw5NuTiprzs4l/ofT4VwxIIUJB0QPHoN0aJtLL90fA64oBkSGKERE4DjcRRN3kK/p5BGZeTmExgWmloeQzCiYq\n\tjavEZ7Yi9DoqrT2mnlfJJGrIuNODwNVw2YJme4PPqkzC66sLn1B6elw1wft/0fnrhLuUarjqdOkZ/1U/G2HVWZmCyPqwR3WUSWdgPK3RBlz4/WkYWifdubmK\n\te2a+0khQjBCd2GxGxGR5gi4gwCYT9Xlpw+qeU5Rfafk9H1B5JcNPwJB2+iwmTD0Tqs75JjufMSQF+mKhteK0jEVBmFa5kA==","Subject":"Re: [libcamera-devel] [PATCH v9 02/11] media: v4l2-ctrl: Document\n\tV4L2_CID_CAMERA_SENSOR_LOCATION","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","X-List-Received-Date":"Thu, 07 May 2020 14:09:25 -0000"}}]