[libcamera-devel] [PATCH 04/23] libcamera: properties: Add rotation property
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Feb 13 22:01:50 CET 2020
Hi Jacopo,
On Wed, Feb 05, 2020 at 11:51:05AM +0100, jacopo mondi wrote:
> On Tue, Feb 04, 2020 at 01:42:49AM +0200, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > This looks really good. The description is quite long, but that's
> > unavoidable if we want to be both precise and generic enough to cover
> > all use cases.
> >
> > Please see below for a few minor comments.
> >
> > On Mon, Jan 13, 2020 at 05:42:26PM +0100, Jacopo Mondi wrote:
> > > The rotation property describes the rotation of the camera sensor.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > > src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++
> > > 1 file changed, 309 insertions(+)
> > >
> > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > > index aaadcbd3e52b..243af7bd0a03 100644
> > > --- a/src/libcamera/property_ids.yaml
> > > +++ b/src/libcamera/property_ids.yaml
> > > @@ -25,4 +25,313 @@ controls:
> > > description: |
> > > The camera is attached to the device in a way that allows it to
> > > be moved freely
> > > +
> > > + - Rotation:
> > > + type: int32_t
> > > + description: |
> > > + The camera rotation is expressed as the angular difference in degrees
> > > + between two reference systems, one implicitly defined by the camera
> >
> > I wouldn't say "implicitly defined" here, as the reference system is
> > defined below, quite explicitly :-) How about "one relative to the
> > camera module" ?
>
> Would you keep "camera module intrinsics characteristics" or just
> "camera module" ?
>
> I would prefer the former, as the coordintate system is defined by the
> sensor's pixel array displacement.
I would prefer the latter actually, I'm not sure what "relative to the
camera module intrinsic characteristics" actually mean.
> > > + module intrinsics characteristics, and one artificially defined on the
> > > + external world scene to be captured when projected on the image sensor
> > > + pixel array.
> > > +
> > > + A camera sensor has an implicitly defined 2-dimensional reference
> > > + system 'Rc' defined by its pixel array scan-out order, with its origin
> >
> > "scan-out" is mostly used for displays, for camera sensors the usual
> > term is "read-out".
>
> Ack
>
> > > + posed at pixel address (0,0), the x-axis progressing from there towards
> >
> > Maybe "placed" instead of "posed" ? The latter doesn't match a
> > definition I can find, but I may be wrong.
> >
> > Pixels don't have addresses, they have coordinates.
>
> I've seen address used in many sensor chip manuals, I think I took it
> from there, but I can indeed change it.
>
> > Unless I'm mistaken axes are usually named with capital letters.
> >
> > > + the last scanned out column of the pixel array and the y-axis
> > > + progressing towards the last scanned out line.
> >
> > To match the above,
> >
> > "A camera sensor has a 2-dimensional reference system 'Rc' defined by
> > its pixel array read-out order. The origin is set to the first pixel
> > being read out, the X-axis points along the column read-out direction
> > towards the last columns, and the Y-axis along the row read-out
> > direction towards the last row."
>
> I'm not sure I'm super happy with the expression
>
> "The X-axis points along the column read-out direction towards the
> last column."
>
> if compared to:
>
> "The X-axis progressing from the here [the first read-out pixel]
> towards the last read-out column."
Did you mean "there" instead of "the here" ? I find the second a bit
strange (otherwise I wouldn't have commented on it :-)), but it may be a
matter of personal taste.
Kieran, what can your native English skills teach us ?
> > > +
> > > + A typical example for a sensor with a (2592x1944) pixel array matrix
> >
> > You can drop the parentheses here, they're used when expressing
> > coordinate tuples, but not for sizes.
>
> Ack
>
> > > + observed from the front is
> > > +
> > > + (2592) x-axis 0
> >
> > s/x/X/ (and below, and same for y)
>
> Ack
>
> > > + <------------------------+ 0
> > > + .......... ... ..........!
> > > + .......... ... ..........! y-axis
> > > + ... !
> > > + .......... ... ..........!
> > > + .......... ... ..........! (1944)
> > > + V
> >
> > 2592 and 1944 should be 2591 and 1943 respectively as you count from 0.
> > The parentheses are also not needed.
>
> Ack
>
> > Typically sensors also have dark and active boundary rows and columns,
> > but that may be too much details to include here.
>
> Not a detail I would include here. And, by the way, I the number I
> have used here have been taken from the full pixel array size of a
> sensor producing 2560x1920 images. They include black and calibration
> pixels.
>
> > > + The external world scene reference system scene 'Rs' is defined as a
> >
> > s/system scene/system/ ?
>
> Ack
>
> > > + 2-dimensional reference system on the parallel plane posed in front
> > > + of the camera module's focal plane, with its origin placed on the
> >
> > This is a bit complicated, how about simply saying that the reference
> > system is defined on the focal plane of the sensor ?
>
> Ack
>
> > > + visible top-left corner, the x-axis progressing towards the right from
> > > + there and the y-axis progressing towards the bottom of the visible
> > > + scene.
> >
> > Shouldn't we point out that the terms top, left, right and bottom are
> > intentionally not defined ?
> >
> > With the above comments applied here,
> >
> > "The external world scene reference system 'Rs' is a 2-dimensional
> > reference system on the focal plane of the camera module. The origin is
> > placed on the top-left corner of the visible scene, the X-axis points
> > towards the right, and the Y-axis points towards the bottom of the
> > scene. The top, bottom, left and right directions are intentionally not
> > defined and depend on the environment in which the camera is used."
>
> Ack
>
> > > +
> > > + A typical example of a (very common) picture of a shark swimming from
> > > + left to right is
> >
> > "from left to right, as seen from the camera, is"
>
> Ack
>
> > > +
> > > + x-axis
> > > + (0,0)---------------------->
> > > + !
> > > + !
> > > + ! |\____)\___
> > > + ! ) _____ __`<
> > > + ! |/ )/
> > > + !
> > > + V
> > > + y-axis
> > > +
> > > + With the reference plane posed in front of the camera module and
> > > + parallel to its focal plane
> > > +
> > > + !
> > > + / !
> > > + / !
> > > + / !
> > > + _ / !
> > > + +-/ \-+ / !
> > > + | (o) | ! 'Rs' reference plane
> > > + +-----+ \ !
> > > + \ !
> > > + \ !
> > > + \ !
> > > + \ !
> > > + !
> > > +
> > > + When projected on the sensor's pixel array, the image and the associated
> > > + reference system 'Rs' are typically inverted, due to the camera module's
> >
> > Maybe "typically (but not always)" ?
> >
> > > + lens optical inversion effect.
> > > +
> > > + Assuming the above represented scene of the swimming shark, the lens
> > > + inversion projects on the sensor pixel array the reference plane 'Rp'
> >
> > I got confused for a second, wondering why only the Y axis was inverted,
> > until I realized the this drawing shows the scene projected onto the
> > camera module as seen when looking at the camera module. I think we
> > should mention this explicitly:
>
> Indeed, thanks for pointing it out. I inverted the position of the
> observer without even noticing.
>
> > "Assuming the above represented scene of the swimming shark, the lens
> > inversion projects the scene and its reference system onto the sensor
> > pixel array as follows, seen from the front of the camera sensor."
>
> Ack
>
> > > +
> > > + y-axis
> > > + ^
> > > + !
> > > + ! |\_____)\__
> > > + ! ) ____ ___.<
> > > + ! |/ )/
> > > + !
> > > + !
> > > + (0,0)---------------------->
> > > + x-axis
> >
> > "Note the shark being upside-down.
> >
> > The resulting projected reference system is named 'Rp'."
> >
> > > +
> > > + The camera rotation property is then defined as the angular difference
> > > + in counterclockwise direction between the origin of the camera reference
> >
> > Nitpicking again, an angular difference can't be defined between
> > origins, as origins are points. It should be defined between reference
> > systems.
> >
> > We should specify that the rotation is expressed in degrees as a number
> > in the [0, 360[ range.
>
> true
>
> > > + system 'Rc', defined by the camera sensor scan-out direction and its
> > > + mounting position, and the origin of the projected scene reference
> > > + system 'Rp', result of the optical projection of the scene reference
> > > + system 'Rs' on the sensor pixel array.
> >
> > I don't think we need to repeat the definitions of the two reference
> > systems.
>
> I was not sure it was needed, but it seemed to me it was not too heavy
> to read, but it can be dropped indeed
>
> > "The camera rotation property is then defined as the angular difference
> > in the counterclockwise direction between the camera reference system
> > 'Rc' and the projected scene reference system 'Rp'. It is expressed in
> > degrees as a number in the range [0, 360[."
>
> Thanks, works well
>
> > > +
> > > + Examples
> > > +
> > > + 0 degrees camera rotation
> > > +
> > > + y-Rp
> > > + y-Rc ^
> > > + ^ !
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! (0,0)---------------------->
> > > + ! x-Rp
> > > + 0 +------------------------------------->
> > > + 0 x-Rc
> >
> > The X-axis 0 should be aligned with the +, or the Y-axis 0 moved one
> > line up. You may want to use the same notation for the Rc and Rp
> > reference systems ('(0,0)' or split coordinates in both cases).
>
> I am not sure why I used two different notations for the camera and
> the projected scene reference systems. I will use a single one
>
> > Should the x-Rc axis have the same length as the x-Rp axis ?
>
> They could, let's see how the drawing looks like.
>
> > > +
> > > +
> > > + x-Rc 0
> > > + <------------------------+ 0
> > > + x-Rp !
> > > + <-----------------------(0,0) !
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! V
> > > + ! y-Rc
> > > + V
> > > + y-Rp
> > > +
> > > + 90 degrees camera rotation
> > > +
> > > + 0 y-Rc
> > > + 0 +----------------------->
> > > + !
> > > + ! y-Rp
> > > + ! ^
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! (0,0)---------------------->
> > > + ! x-Rp
> > > + !
> > > + V
> > > + x-Rc
> > > +
> > > + 180 degrees camera rotation
> > > +
> > > + x-Cr 0
> >
> > s/Cr/Rc/
> >
> > > + <------------------------+ 0
> > > + y-Rp !
> > > + ^ !
> > > + ! ! y-Cr
> > > + ! !
> > > + ! !
> > > + ! V
> > > + !
> > > + !
> > > + (0,0)--------------------->
> > > + x-Rp
> > > +
> > > + 270 degrees camera rotation
> > > +
> > > + 0 y-Rc
> > > + 0 +----------------------->
> > > + ! x-Rp
> > > + ! <-----------------------(0,0
> >
> > Missing ) at the end of the line.
> >
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! !
> > > + ! V
> > > + ! y-Rp
> > > + !
> > > + !
> > > + V
> > > + x-Rc
> > > +
> > > +
> > > +
> > > + Example one - Webcam
> > > +
> > > + A camera module installed on the user facing part of a laptop screen
> > > + casing used for video calls. The captured images are meant to be
> > > + displayed in landscape mode (width > height) on the laptop screen.
> > > +
> > > + The camera is typically mounted 180 degrees rotated to compensate the
> > > + lens optical inversion effect.
> >
> > I understand what you mean here, but can be confusing for the reader.
> > The direction of the Rc and Rp reference systems depicted below
> > correspond to the 0° rotation above, and here the text states 180°. How
> > about writing "The camera sensor is typically mounted upside-down to
> > compensate ..." ?
>
> Ack
>
> > > +
> > > + y-Rp
> > > + y-Rc ^
> > > + ^ !
> > > + ! ! |\_____)\__
> > > + ! ! ) ____ ___.<
> > > + ! ! |/ )/
> > > + ! !
> > > + ! !
> > > + ! (0,0)---------------------->
> > > + ! x-Rp
> > > + 0 +------------------------------------->
> > > + 0 x-Rc
> > > +
> > > + The two reference systems are aligned, the resulting camera rotation is
> > > + 0 degrees, no rotation correction should be applied to the resulting
> >
> > s/should/needs to/ ?
>
> ack
>
> > > + image once captured to memory buffers to correctly display it to users.
> > > +
> > > + +--------------------------+
> > > + ! |\____)\___ !
> > > + ! ) _____ __`< !
> > > + ! |/ )/ !
> > > + +--------------------------+
> > > +
> > > + If the camera module is not mounted 180 degrees rotated to compensate
> >
> > And here, for the reason explained above, "is not mounted upside-down to
> > compensate ..." ?
> >
> > > + the lens optical inversion, the two reference system will result not
> >
> > s/system/systems/
> >
> > > + aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'
> >
> > s/will result not aligned/will not be aligned/ ?
>
> ack on all the above comments
>
> > > + plane.
> >
> > As above, we should probably not use plane to refer to Rc and Rp, as
> > they're defined as reference systems. How about the following ?
> >
> > "If the camera sensor is not mounted upside-down to compensate for the
> > lens optical inversion, the two reference systems will not be aligned,
> > with 'Rp' being rotated 180 degrees relatively to 'Rc'."
>
> It's more clear, thanks
>
> > > + x-Rc 0
> > > + <------------------------+ 0
> > > + y-Rp !
> > > + ^ !
> > > + ! ! y-Rc
> > > + ! |\_____)\__ !
> > > + ! ) ____ ___.< !
> > > + ! |/ )/ V
> > > + !
> > > + !
> > > + (0,0)--------------------->
> > > + x-Rp
> > > +
> > > + The image once captured to memory will then be 180 degrees rotated
> >
> > "will then be rotated by 180 degrees" ?
>
> matter of tastes I guess, but ok
>
> > > +
> > > + +--------------------------+
> > > + ! __/(_____/| !
> > > + ! >.___ ____ ( !
> > > + ! \( \| !
> > > + +--------------------------+
> >
> > Should we add two lines above and below the shark to keep the aspect
> > ration ?
> >
> > > +
> > > + A software rotation correction of 180 degrees should be applied to
> > > + correctly display the image.
> > > +
> > > + +--------------------------+
> > > + ! |\____)\___ !
> > > + ! ) _____ __`< !
> > > + ! |/ )/ !
> > > + +--------------------------+
> >
> > Here too.
> >
> > > +
> > > + Example two - Phone camera
> > > +
> > > + A camera installed on the back-side of a mobile device facing away from
> >
> > s/back-side/back side/
> >
> > > + the user. The captured images are meant to be displayed in portrait mode
> > > + (height > width) to match the device screen orientation and the device
> > > + usage orientation used when taking the picture.
> > > +
> > > + The camera is typically mounted with its pixel array longer side aligned
> >
> > s/camera/camera sensor/
> >
> > > + to the device longer side, 180 degrees rotated to compensate the lens
> >
> > s/compensate/compensate for/ ?
> >
> > > + optical inversion effect.
> > > +
> > > + 0 y-Rc
> > > + 0 +----------------------->
> > > + !
> > > + ! y-Rp
> > > + ! ^
> > > + ! !
> > > + ! ! |\_____)\__
> > > + ! ! ) ____ ___.<
> > > + ! ! |/ )/
> > > + ! !
> > > + ! !
> > > + ! (0,0)---------------------->
> > > + ! x-Rp
> > > + !
> > > + !
> > > + V
> > > + x-Rc
> > > +
> > > + The two reference systems are not aligned and the 'Rp' reference
> > > + system is 90 degrees rotated in counterclockwise direction in respect
> >
> > "... is rotated by 90 degrees in the counterclockwise direction
> > relatively to the ..."
> >
> > > + to the 'Rc' reference system.
> > > +
> > > + The image, when captured to memory buffer will be rotated.
> >
> > To match the previous example,
> >
> > "The image once captured to memory will be rotated."
> >
> > > +
> > > + +---------------------------------------+
> > > + | _ _ |
> > > + | \ / |
> > > + | | | |
> > > + | | | |
> > > + | | > |
> > > + | < | |
> > > + | | | |
> > > + | . |
> > > + | V |
> > > + +---------------------------------------+
> > > +
> > > + A correction of 90 degrees in counterclockwise direction has to be
> > > + applied to correctly display the image in portrait mode on the device
> > > + screen.
> > > +
> > > + +-----------------+
> > > + | |
> > > + | |
> > > + | |
> > > + | |
> > > + | |
> > > + | |
> > > + | |\____)\___ |
> > > + | ) _____ __`< |
> > > + | |/ )/ |
> > > + | |
> > > + | |
> > > + | |
> > > + | |
> > > + | |
> > > + +-----------------+
> >
> > I had to think a bit to check if the sharks are in the right
> > orientation, and they are :-) I would however reduce the frame a little
> > bit to match the size of the references systems above.
>
> I received the opposed comments from Andrey and I agree with him that
> we should try to keep the image proportions similar to the ones above
> to avoid the idea of cropping. In my view, the coordinate system does
> not represent the size of the captured image, but I can work to make
> them look like it.
>
> > > ...
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list