Hi Jacopo, On Mon, Sep 02, 2019 at 09:48:30PM +0200, Jacopo Mondi wrote: > On Mon, Sep 02, 2019 at 07:49:37PM +0300, Laurent Pinchart wrote: > > On Mon, Sep 02, 2019 at 06:40:31PM +0200, Jacopo Mondi wrote: > >> On Mon, Sep 02, 2019 at 02:38:40PM +0100, Rob Herring wrote: > >>> On Thu, Aug 29, 2019 at 02:46:40PM +0200, Jacopo Mondi wrote: > >>>> On Tue, Aug 27, 2019 at 03:21:26PM +0300, Laurent Pinchart wrote: > >>>>> On Tue, Aug 27, 2019 at 11:23:27AM +0200, Jacopo Mondi wrote: > >>>>>> Add the 'location' device property, used to specify the camera device > >>>>>> mounting position. The property is particularly meaningful for mobile > >>>>>> devices with a well defined usage orientation. > >>>>>> > >>>>>> Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx> > >>>>>> --- > >>>>>> .../devicetree/bindings/media/video-interfaces.txt | 10 ++++++++++ > >>>>>> 1 file changed, 10 insertions(+) > >>>>>> > >>>>>> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt > >>>>>> index f884ada0bffc..865f4142f432 100644 > >>>>>> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt > >>>>>> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt > >>>>>> @@ -89,6 +89,16 @@ Optional properties > >>>>>> but a number of degrees counter clockwise. Typical values are 0 and 180 > >>>>>> (upside down). > >>>>>> > >>>>>> +- location: The camera sensor mounting location, expressed as a position > >>>>>> + relative to the usage orientation of the device the sensor is installed on. > >>>>> > >>>>> DT bindings being ABIs, we need to be precise and thorough there. One > >>>>> particular point that bothers me is that the property is named location, > >>>>> and its description refers to camera sensor mounting location. > >>>>> > >>>>> I see two options to fix this. One of them is to rename the property to > >>>>> camera-location, but that would limit its future usage for other types > >>>>> of devices. The other one is to document the property as applying to a > >>>>> "device" instead of a "camera sensor", and add one sentence stating that > >>>>> this property is valid for camera sensors only. > >>>>> > >>>>> This will require finding another name for the device that the device is > >>>>> mounted on though, as using device twice would be very confusing. > >>>> > >>>> I had the same concern, but I cannot find another term to convey > >>>> this... suggestions? > >>> > >>> For accelerometers and/or gyroscopes, we already have 'mount-matrix'. > >> > >> Pardon my ignorance, but I could not find it documented. Some binding > >> files refers to an iio/mount-matrix.txt file which I cannot find. Has > >> it been removed? Anyway, some individual bindings report examples of > >> mount matrices (ie > >> Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt) > >> > >>> That would replace 'rotation'. Maybe we can do something similar here? > >> > >> I think 'rotation' simply expressed as degrees is fine here, our > >> problem was to find a term that would make it possible to specify that > >> rotation/location are applied to a 'device' mounted on a 'device'. > >> Sakari suggested on irc to use 'system' in place of the second > >> 'device' and that seems to work well to me. > >> > >> Or were you suggesting to use a construct similar to mount-matrix for > >> a purpose I didn't get :) ? > > > > mount-matrix would allow exposing the rotation in a more generic way, > > and if there are standard DT bindings, I think this would make sense, > > especially given that we could have device with camera sensors not > > mounted parallel to the device side. > > After reading this entry you pointed me to > https://lore.kernel.org/patchwork/patch/1044786/ > I understand why Rob suggested that, but it seems a bit far fetched to > me... The devices that could make use of 'mount-matrix', such as the > ones IIO deal with, sample data in a 3D space, and to compensate their > mounting displacement a rotation matrix is required. > > Do you see use cases for compensating mounting displacement in a 3-d > space for a camera ? I'm not saying it's not possible, but seems -a > bit- far fetched. 360° cameras come to mind, they can use multiple sensors mounted with different angles. > If we consider cameras as always mounted parallel to the device used > to take picturea, the correction you could apply to the possible mounting > displacements are just rotations along the origin (or the z axis you want > to consider a 3 dimensional space) and requiring to supply a 2-dimensions > rotation matrix (with only a few combinations of 0, 1 and -1, if we only > accept multiples of 90 degrees as we agreed) it seems a bit an overkill. > > True, we could also handle image flipping, not just rotation with a rotation > matrix but an image sensor mounted flipped upside down kind of defeat its > purpose, doesn't it ? (it would point to the inside of the device :) > > When we'll have devices with movable sensors like the one Pavel > linked, rotation matrices could be used to report the current camera > position maybe, but to represent a mounting displacement compensation, > for devices working in a 2 dimensional space and with fixed rotations > of 90 degrees multiples it seems an overkill to ask to developers to > me. > > That said, there are devices out there which do things I cannot even > imagine, and maybe could actually compensate rotations in the 3D > space, sample 3D point maps or other advanced things. Indeed they > could use a rotation matrix if they need to, but the purpose of > 'rotation' is much more humble and even if it could be represented > with 'mount-matrix' it seems to me a non justifiable effort. Most devices will only need to report a 0, 90, 180 or 270 rotation angle along an axis perpendicular to the device. For those, mount-matrix is indeed a bit overkill, and a rotation property would be enough. However, when we will need to express more than those simple configurations, mount-matrix will become needed, and all of a sudden we will have multiple ways to express the same information. That's what I would like to avoid by going for mount-matrix already. -- Regards, Laurent Pinchart
