[{"id":3449,"web_url":"https://patchwork.libcamera.org/comment/3449/","msgid":"<20200114231643.GC942549@oden.dyn.berto.se>","date":"2020-01-14T23:16:43","subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for a great effort in writing this up!\n\nOn 2020-01-13 17:42:26 +0100, Jacopo Mondi wrote:\n> The rotation property describes the rotation of the camera sensor.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThis matches my view of our discussions, nice work!\n\nMy only concern is that a shark is swimming in front of the laptop \nwebcam example ;-)\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++\n>  1 file changed, 309 insertions(+)\n> \n> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> index aaadcbd3e52b..243af7bd0a03 100644\n> --- a/src/libcamera/property_ids.yaml\n> +++ b/src/libcamera/property_ids.yaml\n> @@ -25,4 +25,313 @@ controls:\n>            description: |\n>              The camera is attached to the device in a way that allows it to\n>              be moved freely\n> +\n> +  - Rotation:\n> +      type: int32_t\n> +      description: |\n> +        The camera rotation is expressed as the angular difference in degrees\n> +        between two reference systems, one implicitly defined by the camera\n> +        module intrinsics characteristics, and one artificially defined on the\n> +        external world scene to be captured when projected on the image sensor\n> +        pixel array.\n> +\n> +        A camera sensor has an implicitly defined 2-dimensional reference\n> +        system 'Rc' defined by its pixel array scan-out order, with its origin\n> +        posed at pixel address (0,0), the x-axis progressing from there towards\n> +        the last scanned out column of the pixel array and the y-axis\n> +        progressing towards the last scanned out line.\n> +\n> +        A typical example for a sensor with a (2592x1944) pixel array matrix\n> +        observed from the front is\n> +\n> +                   (2592)      x-axis          0\n> +                      <------------------------+ 0\n> +                      .......... ... ..........!\n> +                      .......... ... ..........! y-axis\n> +                                 ...           !\n> +                      .......... ... ..........!\n> +                      .......... ... ..........! (1944)\n> +                                               V\n> +\n> +        The external world scene reference system scene 'Rs' is defined as a\n> +        2-dimensional reference system on the parallel plane posed in front\n> +        of the camera module's focal plane, with its origin placed on the\n> +        visible top-left corner, the x-axis progressing towards the right from\n> +        there and the y-axis progressing towards the bottom of the visible\n> +        scene.\n> +\n> +        A typical example of a (very common) picture of a shark swimming from\n> +        left to right is\n> +\n> +                                 x-axis\n> +                   (0,0)---------------------->\n> +                     !\n> +                     !\n> +                     !       |\\____)\\___\n> +                     !       ) _____  __`<\n> +                     !       |/     )/\n> +                     !\n> +                     V\n> +                   y-axis\n> +\n> +        With the reference plane posed in front of the camera module and\n> +        parallel to its focal plane\n> +\n> +                                   !\n> +                                 / !\n> +                                /  !\n> +                               /   !\n> +                        _     /    !\n> +                     +-/ \\-+ /     !\n> +                     | (o) |       ! 'Rs' reference plane\n> +                     +-----+ \\     !\n> +                              \\    !\n> +                               \\   !\n> +                                \\  !\n> +                                 \\ !\n> +                                   !\n> +\n> +        When projected on the sensor's pixel array, the image and the associated\n> +        reference system 'Rs' are typically inverted, due to the camera module's\n> +        lens optical inversion effect.\n> +\n> +        Assuming the above represented scene of the swimming shark, the lens\n> +        inversion projects on the sensor pixel array the reference plane 'Rp'\n> +\n> +                  y-axis\n> +                     ^\n> +                     !\n> +                     !       |\\_____)\\__\n> +                     !       ) ____  ___.<\n> +                     !       |/    )/\n> +                     !\n> +                     !\n> +                   (0,0)---------------------->\n> +                                 x-axis\n> +\n> +        The camera rotation property is then defined as the angular difference\n> +        in counterclockwise direction between the origin of the camera reference\n> +        system 'Rc', defined by the camera sensor scan-out direction and its\n> +        mounting position, and the origin of the projected scene reference\n> +        system 'Rp', result of the optical projection of the scene reference\n> +        system 'Rs' on the sensor pixel array.\n> +\n> +        Examples\n> +\n> +        0 degrees camera rotation\n> +\n> +                          y-Rp\n> +                    y-Rc   ^\n> +                     ^     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !   (0,0)---------------------->\n> +                     !                 x-Rp\n> +                   0 +------------------------------------->\n> +                      0            x-Rc\n> +\n> +\n> +                                     x-Rc          0\n> +                           <------------------------+ 0\n> +                                x-Rp                !\n> +                     <-----------------------(0,0)  !\n> +                                               !    !\n> +                                               !    !\n> +                                               !    !\n> +                                               !    V\n> +                                               !  y-Rc\n> +                                               V\n> +                                             y-Rp\n> +\n> +        90 degrees camera rotation\n> +\n> +                      0        y-Rc\n> +                   0 +----------------------->\n> +                     !\n> +                     !    y-Rp\n> +                     !     ^\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !   (0,0)---------------------->\n> +                     !                 x-Rp\n> +                     !\n> +                     V\n> +                   x-Rc\n> +\n> +        180 degrees camera rotation\n> +\n> +                                   x-Cr          0\n> +                         <------------------------+ 0\n> +                     y-Rp                         !\n> +                      ^                           !\n> +                      !                           ! y-Cr\n> +                      !                           !\n> +                      !                           !\n> +                      !                           V\n> +                      !\n> +                      !\n> +                    (0,0)--------------------->\n> +                                   x-Rp\n> +\n> +        270 degrees camera rotation\n> +\n> +                      0        y-Rc\n> +                   0 +----------------------->\n> +                     !            x-Rp\n> +                     ! <-----------------------(0,0\n> +                     !                           !\n> +                     !                           !\n> +                     !                           !\n> +                     !                           !\n> +                     !                           !\n> +                     !                           V\n> +                     !                         y-Rp\n> +                     !\n> +                     !\n> +                     V\n> +                   x-Rc\n> +\n> +\n> +\n> +        Example one - Webcam\n> +\n> +        A camera module installed on the user facing part of a laptop screen\n> +        casing used for video calls. The captured images are meant to be\n> +        displayed in landscape mode (width > height) on the laptop screen.\n> +\n> +        The camera is typically mounted 180 degrees rotated to compensate the\n> +        lens optical inversion effect.\n> +\n> +                          y-Rp\n> +                    y-Rc   ^\n> +                     ^     !\n> +                     !     !       |\\_____)\\__\n> +                     !     !       ) ____  ___.<\n> +                     !     !       |/    )/\n> +                     !     !\n> +                     !     !\n> +                     !   (0,0)---------------------->\n> +                     !                 x-Rp\n> +                   0 +------------------------------------->\n> +                      0                x-Rc\n> +\n> +        The two reference systems are aligned, the resulting camera rotation is\n> +        0 degrees, no rotation correction should be applied to the resulting\n> +        image once captured to memory buffers to correctly display it to users.\n> +\n> +                     +--------------------------+\n> +                     !       |\\____)\\___        !\n> +                     !       ) _____  __`<      !\n> +                     !       |/     )/          !\n> +                     +--------------------------+\n> +\n> +        If the camera module is not mounted 180 degrees rotated to compensate\n> +        the lens optical inversion, the two reference system will result not\n> +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'\n> +        plane.\n> +\n> +                                   x-Rc          0\n> +                         <------------------------+ 0\n> +                     y-Rp                         !\n> +                      ^                           !\n> +                      !                           ! y-Rc\n> +                      !      |\\_____)\\__          !\n> +                      !      ) ____  ___.<        !\n> +                      !      |/    )/             V\n> +                      !\n> +                      !\n> +                    (0,0)--------------------->\n> +                                   x-Rp\n> +\n> +        The image once captured to memory will then be 180 degrees rotated\n> +\n> +                     +--------------------------+\n> +                     !          __/(_____/|     !\n> +                     !        >.___  ____ (     !\n> +                     !             \\(    \\|     !\n> +                     +--------------------------+\n> +\n> +        A software rotation correction of 180 degrees should be applied to\n> +        correctly display the image.\n> +\n> +                     +--------------------------+\n> +                     !       |\\____)\\___        !\n> +                     !       ) _____  __`<      !\n> +                     !       |/     )/          !\n> +                     +--------------------------+\n> +\n> +        Example two - Phone camera\n> +\n> +        A camera installed on the back-side of a mobile device facing away from\n> +        the user. The captured images are meant to be displayed in portrait mode\n> +        (height > width) to match the device screen orientation and the device\n> +        usage orientation used when taking the picture.\n> +\n> +        The camera is typically mounted with its pixel array longer side aligned\n> +        to the device longer side, 180 degrees rotated to compensate the lens\n> +        optical inversion effect.\n> +\n> +                      0        y-Rc\n> +                   0 +----------------------->\n> +                     !\n> +                     !    y-Rp\n> +                     !     ^\n> +                     !     !\n> +                     !     !       |\\_____)\\__\n> +                     !     !       ) ____  ___.<\n> +                     !     !       |/    )/\n> +                     !     !\n> +                     !     !\n> +                     !   (0,0)---------------------->\n> +                     !                 x-Rp\n> +                     !\n> +                     !\n> +                     V\n> +                   x-Rc\n> +\n> +        The two reference systems are not aligned and the 'Rp' reference\n> +        system is 90 degrees rotated in counterclockwise direction in respect\n> +        to the 'Rc' reference system.\n> +\n> +        The image, when captured to memory buffer will be rotated.\n> +\n> +                     +---------------------------------------+\n> +                     |                  _ _                  |\n> +                     |                 \\   /                 |\n> +                     |                  | |                  |\n> +                     |                  | |                  |\n> +                     |                  |  >                 |\n> +                     |                 <  |                  |\n> +                     |                  | |                  |\n> +                     |                    .                  |\n> +                     |                   V                   |\n> +                     +---------------------------------------+\n> +\n> +        A correction of 90 degrees in counterclockwise direction has to be\n> +        applied to correctly display the image in portrait mode on the device\n> +        screen.\n> +\n> +                                +-----------------+\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |  |\\____)\\___    |\n> +                                |  ) _____  __`<  |\n> +                                |  |/     )/      |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                +-----------------+\n>  ...\n> -- \n> 2.24.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9F49B60705\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 15 Jan 2020 00:16:45 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id j1so16331600lja.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 14 Jan 2020 15:16:45 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tl12sm8163700lji.52.2020.01.14.15.16.43\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 14 Jan 2020 15:16:44 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=R7DLNrat9o/E0J3s3D58hte5YWH/MIc70Y9qGzdvVj4=;\n\tb=Ahv+CIqHH3Ks1I9erEaq++Lxxkbe53qratam9zjcYuhIKKJUrb27FMG5dcX2PkPJ7M\n\tbunGKU8H1AtpYrV8yzRCmHMGCBv1i06oYa1p0Yx4dIcfkCz+qvVXzcH4CQmDs0OweXkc\n\tXTUtWpYCeuoOtVjVUQEYUchrLP+2jk6TWbJumEaJp5cfux19oFffErk0ddWmDQjHA2Pn\n\thFSwZ8KCc1RK4tdtjmrgn5UXocZPQuVwn64BO5RF4Xu9CRK2tI+VY+ehINF8gNZu9sui\n\tiUxGc+xLKjjc5e/3dR9ptYrWcJabcIkSyEThgBq5iLNXkCbHKohrG3X0sQjbk14725JF\n\tzKjA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=R7DLNrat9o/E0J3s3D58hte5YWH/MIc70Y9qGzdvVj4=;\n\tb=C8NRyQnUHyC0SH3rpkjpzp1yPPC0FK7tMKUCufXfMz2/KNqgHW2MXjDmRn9c/Dt0E1\n\tW+ilRMVIEu9FgWcJ7Lnr/7eAAZDLE4rJzALEU+9FXk5dgok33heVM7KiBX56/2ZWmJIc\n\tzpROMUMCU7fauepHSOGavWc+zr4adkOyV5uPDSZDsRFBojgCDpwxisiNPDPw6Ko+6Yru\n\tKrA05vcGygMrpfYDO7XQhtAAUCRpSnr+DKtYjh0sdjN24yYwMoc9MY84O/HHAxgPwd+K\n\tFG2O/fSFjoDdV2amjPAoLV6c2JW87E6SqdCmR/46NMJ37JWDuWY7+ENeLmo94EX+2boc\n\t6uUg==","X-Gm-Message-State":"APjAAAXyfmvFUIU2atZZqjg+gk+G5lUM+/J8sBRs+S4GkGcGH1mRJT5u\n\tZVkCo1n4V9J0ongaAft7iDtSyQ==","X-Google-Smtp-Source":"APXvYqzQtro4OfpWgC25B24SWhNzAgtjnteOTbfl1ZSrcNCMJEPiiC4ywYSjpGM1Fpx3JDERNVPGYg==","X-Received":"by 2002:a05:651c:214:: with SMTP id\n\ty20mr15477639ljn.139.1579043804791; \n\tTue, 14 Jan 2020 15:16:44 -0800 (PST)","Date":"Wed, 15 Jan 2020 00:16:43 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200114231643.GC942549@oden.dyn.berto.se>","References":"<20200113164245.52535-1-jacopo@jmondi.org>\n\t<20200113164245.52535-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200113164245.52535-5-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 14 Jan 2020 23:16:45 -0000"}},{"id":3609,"web_url":"https://patchwork.libcamera.org/comment/3609/","msgid":"<20200203234249.GG4722@pendragon.ideasonboard.com>","date":"2020-02-03T23:42:49","subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nThis looks really good. The description is quite long, but that's\nunavoidable if we want to be both precise and generic enough to cover\nall use cases.\n\nPlease see below for a few minor comments.\n\nOn Mon, Jan 13, 2020 at 05:42:26PM +0100, Jacopo Mondi wrote:\n> The rotation property describes the rotation of the camera sensor.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++\n>  1 file changed, 309 insertions(+)\n> \n> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> index aaadcbd3e52b..243af7bd0a03 100644\n> --- a/src/libcamera/property_ids.yaml\n> +++ b/src/libcamera/property_ids.yaml\n> @@ -25,4 +25,313 @@ controls:\n>            description: |\n>              The camera is attached to the device in a way that allows it to\n>              be moved freely\n> +\n> +  - Rotation:\n> +      type: int32_t\n> +      description: |\n> +        The camera rotation is expressed as the angular difference in degrees\n> +        between two reference systems, one implicitly defined by the camera\n\nI wouldn't say \"implicitly defined\" here, as the reference system is\ndefined below, quite explicitly :-) How about \"one relative to the\ncamera module\" ?\n\n> +        module intrinsics characteristics, and one artificially defined on the\n> +        external world scene to be captured when projected on the image sensor\n> +        pixel array.\n> +\n> +        A camera sensor has an implicitly defined 2-dimensional reference\n> +        system 'Rc' defined by its pixel array scan-out order, with its origin\n\n\"scan-out\" is mostly used for displays, for camera sensors the usual\nterm is \"read-out\".\n\n> +        posed at pixel address (0,0), the x-axis progressing from there towards\n\nMaybe \"placed\" instead of \"posed\" ? The latter doesn't match a\ndefinition I can find, but I may be wrong.\n\nPixels don't have addresses, they have coordinates.\n\nUnless I'm mistaken axes are usually named with capital letters.\n\n> +        the last scanned out column of the pixel array and the y-axis\n> +        progressing towards the last scanned out line.\n\nTo match the above,\n\n\"A camera sensor has a 2-dimensional reference system 'Rc' defined by\nits pixel array read-out order. The origin is set to the first pixel\nbeing read out, the X-axis points along the column read-out direction\ntowards the last columns, and the Y-axis along the row read-out\ndirection towards the last row.\"\n\n> +\n> +        A typical example for a sensor with a (2592x1944) pixel array matrix\n\nYou can drop the parentheses here, they're used when expressing\ncoordinate tuples, but not for sizes.\n\n> +        observed from the front is\n> +\n> +                   (2592)      x-axis          0\n\ns/x/X/ (and below, and same for y)\n\n> +                      <------------------------+ 0\n> +                      .......... ... ..........!\n> +                      .......... ... ..........! y-axis\n> +                                 ...           !\n> +                      .......... ... ..........!\n> +                      .......... ... ..........! (1944)\n> +                                               V\n\n2592 and 1944 should be 2591 and 1943 respectively as you count from 0.\nThe parentheses are also not needed.\n\nTypically sensors also have dark and active boundary rows and columns,\nbut that may be too much details to include here.\n\n> +        The external world scene reference system scene 'Rs' is defined as a\n\ns/system scene/system/ ?\n\n> +        2-dimensional reference system on the parallel plane posed in front\n> +        of the camera module's focal plane, with its origin placed on the\n\nThis is a bit complicated, how about simply saying that the reference\nsystem is defined on the focal plane of the sensor ?\n\n> +        visible top-left corner, the x-axis progressing towards the right from\n> +        there and the y-axis progressing towards the bottom of the visible\n> +        scene.\n\nShouldn't we point out that the terms top, left, right and bottom are\nintentionally not defined ?\n\nWith the above comments applied here,\n\n\"The external world scene reference system 'Rs' is a 2-dimensional\nreference system on the focal plane of the camera module. The origin is\nplaced on the top-left corner of the visible scene, the X-axis points\ntowards the right, and the Y-axis points towards the bottom of the\nscene. The top, bottom, left and right directions are intentionally not\ndefined and depend on the environment in which the camera is used.\"\n\n> +\n> +        A typical example of a (very common) picture of a shark swimming from\n> +        left to right is\n\n\"from left to right, as seen from the camera, is\"\n\n> +\n> +                                 x-axis\n> +                   (0,0)---------------------->\n> +                     !\n> +                     !\n> +                     !       |\\____)\\___\n> +                     !       ) _____  __`<\n> +                     !       |/     )/\n> +                     !\n> +                     V\n> +                   y-axis\n> +\n> +        With the reference plane posed in front of the camera module and\n> +        parallel to its focal plane\n> +\n> +                                   !\n> +                                 / !\n> +                                /  !\n> +                               /   !\n> +                        _     /    !\n> +                     +-/ \\-+ /     !\n> +                     | (o) |       ! 'Rs' reference plane\n> +                     +-----+ \\     !\n> +                              \\    !\n> +                               \\   !\n> +                                \\  !\n> +                                 \\ !\n> +                                   !\n> +\n> +        When projected on the sensor's pixel array, the image and the associated\n> +        reference system 'Rs' are typically inverted, due to the camera module's\n\nMaybe \"typically (but not always)\" ?\n\n> +        lens optical inversion effect.\n> +\n> +        Assuming the above represented scene of the swimming shark, the lens\n> +        inversion projects on the sensor pixel array the reference plane 'Rp'\n\nI got confused for a second, wondering why only the Y axis was inverted,\nuntil I realized the this drawing shows the scene projected onto the\ncamera module as seen when looking at the camera module. I think we\nshould mention this explicitly:\n\n\"Assuming the above represented scene of the swimming shark, the lens\ninversion projects the scene and its reference system onto the sensor\npixel array as follows, seen from the front of the camera sensor.\"\n\n> +\n> +                  y-axis\n> +                     ^\n> +                     !\n> +                     !       |\\_____)\\__\n> +                     !       ) ____  ___.<\n> +                     !       |/    )/\n> +                     !\n> +                     !\n> +                   (0,0)---------------------->\n> +                                 x-axis\n\n\"Note the shark being upside-down.\n\nThe resulting projected reference system is named 'Rp'.\"\n\n> +\n> +        The camera rotation property is then defined as the angular difference\n> +        in counterclockwise direction between the origin of the camera reference\n\nNitpicking again, an angular difference can't be defined between\norigins, as origins are points. It should be defined between reference\nsystems.\n\nWe should specify that the rotation is expressed in degrees as a number\nin the [0, 360[ range.\n\n> +        system 'Rc', defined by the camera sensor scan-out direction and its\n> +        mounting position, and the origin of the projected scene reference\n> +        system 'Rp', result of the optical projection of the scene reference\n> +        system 'Rs' on the sensor pixel array.\n\nI don't think we need to repeat the definitions of the two reference\nsystems.\n\n\"The camera rotation property is then defined as the angular difference\nin the counterclockwise direction between the camera reference system\n'Rc' and the projected scene reference system 'Rp'. It is expressed in\ndegrees as a number in the range [0, 360[.\"\n\n> +\n> +        Examples\n> +\n> +        0 degrees camera rotation\n> +\n> +                          y-Rp\n> +                    y-Rc   ^\n> +                     ^     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !   (0,0)---------------------->\n> +                     !                 x-Rp\n> +                   0 +------------------------------------->\n> +                      0            x-Rc\n\nThe X-axis 0 should be aligned with the +, or the Y-axis 0 moved one\nline up. You may want to use the same notation for the Rc and Rp\nreference systems ('(0,0)' or split coordinates in both cases).\n\nShould the x-Rc axis have the same length as the x-Rp axis ?\n\n> +\n> +\n> +                                     x-Rc          0\n> +                           <------------------------+ 0\n> +                                x-Rp                !\n> +                     <-----------------------(0,0)  !\n> +                                               !    !\n> +                                               !    !\n> +                                               !    !\n> +                                               !    V\n> +                                               !  y-Rc\n> +                                               V\n> +                                             y-Rp\n> +\n> +        90 degrees camera rotation\n> +\n> +                      0        y-Rc\n> +                   0 +----------------------->\n> +                     !\n> +                     !    y-Rp\n> +                     !     ^\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !     !\n> +                     !   (0,0)---------------------->\n> +                     !                 x-Rp\n> +                     !\n> +                     V\n> +                   x-Rc\n> +\n> +        180 degrees camera rotation\n> +\n> +                                   x-Cr          0\n\ns/Cr/Rc/\n\n> +                         <------------------------+ 0\n> +                     y-Rp                         !\n> +                      ^                           !\n> +                      !                           ! y-Cr\n> +                      !                           !\n> +                      !                           !\n> +                      !                           V\n> +                      !\n> +                      !\n> +                    (0,0)--------------------->\n> +                                   x-Rp\n> +\n> +        270 degrees camera rotation\n> +\n> +                      0        y-Rc\n> +                   0 +----------------------->\n> +                     !            x-Rp\n> +                     ! <-----------------------(0,0\n\nMissing ) at the end of the line.\n\n> +                     !                           !\n> +                     !                           !\n> +                     !                           !\n> +                     !                           !\n> +                     !                           !\n> +                     !                           V\n> +                     !                         y-Rp\n> +                     !\n> +                     !\n> +                     V\n> +                   x-Rc\n> +\n> +\n> +\n> +        Example one - Webcam\n> +\n> +        A camera module installed on the user facing part of a laptop screen\n> +        casing used for video calls. The captured images are meant to be\n> +        displayed in landscape mode (width > height) on the laptop screen.\n> +\n> +        The camera is typically mounted 180 degrees rotated to compensate the\n> +        lens optical inversion effect.\n\nI understand what you mean here, but can be confusing for the reader.\nThe direction of the Rc and Rp reference systems depicted below\ncorrespond to the 0° rotation above, and here the text states 180°. How\nabout writing \"The camera sensor is typically mounted upside-down to\ncompensate ...\" ?\n\n> +\n> +                          y-Rp\n> +                    y-Rc   ^\n> +                     ^     !\n> +                     !     !       |\\_____)\\__\n> +                     !     !       ) ____  ___.<\n> +                     !     !       |/    )/\n> +                     !     !\n> +                     !     !\n> +                     !   (0,0)---------------------->\n> +                     !                 x-Rp\n> +                   0 +------------------------------------->\n> +                      0                x-Rc\n> +\n> +        The two reference systems are aligned, the resulting camera rotation is\n> +        0 degrees, no rotation correction should be applied to the resulting\n\ns/should/needs to/ ?\n\n> +        image once captured to memory buffers to correctly display it to users.\n> +\n> +                     +--------------------------+\n> +                     !       |\\____)\\___        !\n> +                     !       ) _____  __`<      !\n> +                     !       |/     )/          !\n> +                     +--------------------------+\n> +\n> +        If the camera module is not mounted 180 degrees rotated to compensate\n\nAnd here, for the reason explained above, \"is not mounted upside-down to\ncompensate ...\" ?\n\n> +        the lens optical inversion, the two reference system will result not\n\ns/system/systems/\n\n> +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'\n\ns/will result not aligned/will not be aligned/ ?\n\n> +        plane.\n\nAs above, we should probably not use plane to refer to Rc and Rp, as\nthey're defined as reference systems. How about the following ?\n\n\"If the camera sensor is not mounted upside-down to compensate for the\nlens optical inversion, the two reference systems will not be aligned,\nwith 'Rp' being rotated 180 degrees relatively to 'Rc'.\"\n\n> +                                   x-Rc          0\n> +                         <------------------------+ 0\n> +                     y-Rp                         !\n> +                      ^                           !\n> +                      !                           ! y-Rc\n> +                      !      |\\_____)\\__          !\n> +                      !      ) ____  ___.<        !\n> +                      !      |/    )/             V\n> +                      !\n> +                      !\n> +                    (0,0)--------------------->\n> +                                   x-Rp\n> +\n> +        The image once captured to memory will then be 180 degrees rotated\n\n\"will then be rotated by 180 degrees\" ?\n\n> +\n> +                     +--------------------------+\n> +                     !          __/(_____/|     !\n> +                     !        >.___  ____ (     !\n> +                     !             \\(    \\|     !\n> +                     +--------------------------+\n\nShould we add two lines above and below the shark to keep the aspect\nration ?\n\n> +\n> +        A software rotation correction of 180 degrees should be applied to\n> +        correctly display the image.\n> +\n> +                     +--------------------------+\n> +                     !       |\\____)\\___        !\n> +                     !       ) _____  __`<      !\n> +                     !       |/     )/          !\n> +                     +--------------------------+\n\nHere too.\n\n> +\n> +        Example two - Phone camera\n> +\n> +        A camera installed on the back-side of a mobile device facing away from\n\ns/back-side/back side/\n\n> +        the user. The captured images are meant to be displayed in portrait mode\n> +        (height > width) to match the device screen orientation and the device\n> +        usage orientation used when taking the picture.\n> +\n> +        The camera is typically mounted with its pixel array longer side aligned\n\ns/camera/camera sensor/\n\n> +        to the device longer side, 180 degrees rotated to compensate the lens\n\ns/compensate/compensate for/ ?\n\n> +        optical inversion effect.\n> +\n> +                      0        y-Rc\n> +                   0 +----------------------->\n> +                     !\n> +                     !    y-Rp\n> +                     !     ^\n> +                     !     !\n> +                     !     !       |\\_____)\\__\n> +                     !     !       ) ____  ___.<\n> +                     !     !       |/    )/\n> +                     !     !\n> +                     !     !\n> +                     !   (0,0)---------------------->\n> +                     !                 x-Rp\n> +                     !\n> +                     !\n> +                     V\n> +                   x-Rc\n> +\n> +        The two reference systems are not aligned and the 'Rp' reference\n> +        system is 90 degrees rotated in counterclockwise direction in respect\n\n\"... is rotated by 90 degrees in the counterclockwise direction\nrelatively to the ...\"\n\n> +        to the 'Rc' reference system.\n> +\n> +        The image, when captured to memory buffer will be rotated.\n\nTo match the previous example,\n\n\"The image once captured to memory will be rotated.\"\n\n> +\n> +                     +---------------------------------------+\n> +                     |                  _ _                  |\n> +                     |                 \\   /                 |\n> +                     |                  | |                  |\n> +                     |                  | |                  |\n> +                     |                  |  >                 |\n> +                     |                 <  |                  |\n> +                     |                  | |                  |\n> +                     |                    .                  |\n> +                     |                   V                   |\n> +                     +---------------------------------------+\n> +\n> +        A correction of 90 degrees in counterclockwise direction has to be\n> +        applied to correctly display the image in portrait mode on the device\n> +        screen.\n> +\n> +                                +-----------------+\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |  |\\____)\\___    |\n> +                                |  ) _____  __`<  |\n> +                                |  |/     )/      |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                |                 |\n> +                                +-----------------+\n\nI had to think a bit to check if the sharks are in the right\norientation, and they are :-) I would however reduce the frame a little\nbit to match the size of the references systems above.\n\n>  ...\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","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 59DB360446\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  4 Feb 2020 00:43:04 +0100 (CET)","from pendragon.ideasonboard.com\n\t(64.177-245-81.adsl-dyn.isp.belgacom.be [81.245.177.64])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C830354A;\n\tTue,  4 Feb 2020 00:43:03 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1580773383;\n\tbh=AfWLRLrUlFKfia0sLTd9CELTIbyK3HsDt0H2Z5cnY6Y=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qrLHkzfWfxtwIyCJ1XBnnPl7A8lYxQeNTYHv+G2oddsUcvUkiP3UcidBjT8HOus04\n\tSeVx3UNpQUoFGiYQKKWPvARP0WA/L+QIzSplqTCs3zfYInWf3UsD18cV7m1h74hSNt\n\tZdlCwkDrtANjv7n5bar73DHTvwuABzw242KLYxKI=","Date":"Tue, 4 Feb 2020 01:42:49 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200203234249.GG4722@pendragon.ideasonboard.com>","References":"<20200113164245.52535-1-jacopo@jmondi.org>\n\t<20200113164245.52535-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200113164245.52535-5-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 03 Feb 2020 23:43:04 -0000"}},{"id":3612,"web_url":"https://patchwork.libcamera.org/comment/3612/","msgid":"<20200205105105.243nbpreess656xj@vino>","date":"2020-02-05T10:51:05","subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Feb 04, 2020 at 01:42:49AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> This looks really good. The description is quite long, but that's\n> unavoidable if we want to be both precise and generic enough to cover\n> all use cases.\n>\n> Please see below for a few minor comments.\n>\n> On Mon, Jan 13, 2020 at 05:42:26PM +0100, Jacopo Mondi wrote:\n> > The rotation property describes the rotation of the camera sensor.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++\n> >  1 file changed, 309 insertions(+)\n> >\n> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> > index aaadcbd3e52b..243af7bd0a03 100644\n> > --- a/src/libcamera/property_ids.yaml\n> > +++ b/src/libcamera/property_ids.yaml\n> > @@ -25,4 +25,313 @@ controls:\n> >            description: |\n> >              The camera is attached to the device in a way that allows it to\n> >              be moved freely\n> > +\n> > +  - Rotation:\n> > +      type: int32_t\n> > +      description: |\n> > +        The camera rotation is expressed as the angular difference in degrees\n> > +        between two reference systems, one implicitly defined by the camera\n>\n> I wouldn't say \"implicitly defined\" here, as the reference system is\n> defined below, quite explicitly :-) How about \"one relative to the\n> camera module\" ?\n>\n\nWould you keep \"camera module intrinsics characteristics\" or just\n\"camera module\" ?\n\nI would prefer the former, as the coordintate system is defined by the\nsensor's pixel array displacement.\n\n> > +        module intrinsics characteristics, and one artificially defined on the\n> > +        external world scene to be captured when projected on the image sensor\n> > +        pixel array.\n> > +\n> > +        A camera sensor has an implicitly defined 2-dimensional reference\n> > +        system 'Rc' defined by its pixel array scan-out order, with its origin\n>\n> \"scan-out\" is mostly used for displays, for camera sensors the usual\n> term is \"read-out\".\n>\n\nAck\n\n> > +        posed at pixel address (0,0), the x-axis progressing from there towards\n>\n> Maybe \"placed\" instead of \"posed\" ? The latter doesn't match a\n> definition I can find, but I may be wrong.\n>\n> Pixels don't have addresses, they have coordinates.\n\nI've seen address used in many sensor chip manuals, I think I took it\nfrom there, but I can indeed change it.\n\n>\n> Unless I'm mistaken axes are usually named with capital letters.\n>\n> > +        the last scanned out column of the pixel array and the y-axis\n> > +        progressing towards the last scanned out line.\n>\n> To match the above,\n>\n> \"A camera sensor has a 2-dimensional reference system 'Rc' defined by\n> its pixel array read-out order. The origin is set to the first pixel\n> being read out, the X-axis points along the column read-out direction\n> towards the last columns, and the Y-axis along the row read-out\n> direction towards the last row.\"\n>\n\nI'm not sure I'm super happy with the expression\n\n\"The X-axis points along the column read-out direction towards the\nlast column.\"\n\nif compared to:\n\n\"The X-axis progressing from the here [the first read-out pixel]\ntowards the last read-out column.\"\n\n> > +\n> > +        A typical example for a sensor with a (2592x1944) pixel array matrix\n>\n> You can drop the parentheses here, they're used when expressing\n> coordinate tuples, but not for sizes.\n>\n\nAck\n\n> > +        observed from the front is\n> > +\n> > +                   (2592)      x-axis          0\n>\n> s/x/X/ (and below, and same for y)\n>\n\nAck\n\n> > +                      <------------------------+ 0\n> > +                      .......... ... ..........!\n> > +                      .......... ... ..........! y-axis\n> > +                                 ...           !\n> > +                      .......... ... ..........!\n> > +                      .......... ... ..........! (1944)\n> > +                                               V\n>\n> 2592 and 1944 should be 2591 and 1943 respectively as you count from 0.\n> The parentheses are also not needed.\n>\n\nAck\n\n> Typically sensors also have dark and active boundary rows and columns,\n> but that may be too much details to include here.\n>\n\nNot a detail I would include here. And, by the way, I the number I\nhave used here have been taken from the full pixel array size of a\nsensor producing 2560x1920 images. They include black and calibration\npixels.\n\n> > +        The external world scene reference system scene 'Rs' is defined as a\n>\n> s/system scene/system/ ?\n>\n\nAck\n\n> > +        2-dimensional reference system on the parallel plane posed in front\n> > +        of the camera module's focal plane, with its origin placed on the\n>\n> This is a bit complicated, how about simply saying that the reference\n> system is defined on the focal plane of the sensor ?\n>\n\nAck\n\n> > +        visible top-left corner, the x-axis progressing towards the right from\n> > +        there and the y-axis progressing towards the bottom of the visible\n> > +        scene.\n>\n> Shouldn't we point out that the terms top, left, right and bottom are\n> intentionally not defined ?\n>\n> With the above comments applied here,\n>\n> \"The external world scene reference system 'Rs' is a 2-dimensional\n> reference system on the focal plane of the camera module. The origin is\n> placed on the top-left corner of the visible scene, the X-axis points\n> towards the right, and the Y-axis points towards the bottom of the\n> scene. The top, bottom, left and right directions are intentionally not\n> defined and depend on the environment in which the camera is used.\"\n>\n\nAck\n\n> > +\n> > +        A typical example of a (very common) picture of a shark swimming from\n> > +        left to right is\n>\n> \"from left to right, as seen from the camera, is\"\n>\n\nAck\n\n> > +\n> > +                                 x-axis\n> > +                   (0,0)---------------------->\n> > +                     !\n> > +                     !\n> > +                     !       |\\____)\\___\n> > +                     !       ) _____  __`<\n> > +                     !       |/     )/\n> > +                     !\n> > +                     V\n> > +                   y-axis\n> > +\n> > +        With the reference plane posed in front of the camera module and\n> > +        parallel to its focal plane\n> > +\n> > +                                   !\n> > +                                 / !\n> > +                                /  !\n> > +                               /   !\n> > +                        _     /    !\n> > +                     +-/ \\-+ /     !\n> > +                     | (o) |       ! 'Rs' reference plane\n> > +                     +-----+ \\     !\n> > +                              \\    !\n> > +                               \\   !\n> > +                                \\  !\n> > +                                 \\ !\n> > +                                   !\n> > +\n> > +        When projected on the sensor's pixel array, the image and the associated\n> > +        reference system 'Rs' are typically inverted, due to the camera module's\n>\n> Maybe \"typically (but not always)\" ?\n>\n> > +        lens optical inversion effect.\n> > +\n> > +        Assuming the above represented scene of the swimming shark, the lens\n> > +        inversion projects on the sensor pixel array the reference plane 'Rp'\n>\n> I got confused for a second, wondering why only the Y axis was inverted,\n> until I realized the this drawing shows the scene projected onto the\n> camera module as seen when looking at the camera module. I think we\n> should mention this explicitly:\n>\n\nIndeed, thanks for pointing it out. I inverted the position of the\nobserver without even noticing.\n\n> \"Assuming the above represented scene of the swimming shark, the lens\n> inversion projects the scene and its reference system onto the sensor\n> pixel array as follows, seen from the front of the camera sensor.\"\n>\n\nAck\n\n> > +\n> > +                  y-axis\n> > +                     ^\n> > +                     !\n> > +                     !       |\\_____)\\__\n> > +                     !       ) ____  ___.<\n> > +                     !       |/    )/\n> > +                     !\n> > +                     !\n> > +                   (0,0)---------------------->\n> > +                                 x-axis\n>\n> \"Note the shark being upside-down.\n>\n> The resulting projected reference system is named 'Rp'.\"\n>\n> > +\n> > +        The camera rotation property is then defined as the angular difference\n> > +        in counterclockwise direction between the origin of the camera reference\n>\n> Nitpicking again, an angular difference can't be defined between\n> origins, as origins are points. It should be defined between reference\n> systems.\n>\n> We should specify that the rotation is expressed in degrees as a number\n> in the [0, 360[ range.\n>\n\ntrue\n\n> > +        system 'Rc', defined by the camera sensor scan-out direction and its\n> > +        mounting position, and the origin of the projected scene reference\n> > +        system 'Rp', result of the optical projection of the scene reference\n> > +        system 'Rs' on the sensor pixel array.\n>\n> I don't think we need to repeat the definitions of the two reference\n> systems.\n>\n\nI was not sure it was needed, but it seemed to me it was not too heavy\nto read, but it can be dropped indeed\n\n> \"The camera rotation property is then defined as the angular difference\n> in the counterclockwise direction between the camera reference system\n> 'Rc' and the projected scene reference system 'Rp'. It is expressed in\n> degrees as a number in the range [0, 360[.\"\n>\n\nThanks, works well\n\n> > +\n> > +        Examples\n> > +\n> > +        0 degrees camera rotation\n> > +\n> > +                          y-Rp\n> > +                    y-Rc   ^\n> > +                     ^     !\n> > +                     !     !\n> > +                     !     !\n> > +                     !     !\n> > +                     !     !\n> > +                     !     !\n> > +                     !   (0,0)---------------------->\n> > +                     !                 x-Rp\n> > +                   0 +------------------------------------->\n> > +                      0            x-Rc\n>\n> The X-axis 0 should be aligned with the +, or the Y-axis 0 moved one\n> line up. You may want to use the same notation for the Rc and Rp\n> reference systems ('(0,0)' or split coordinates in both cases).\n>\n\nI am not sure why I used two different notations for the camera and\nthe projected scene reference systems. I will use a single one\n\n> Should the x-Rc axis have the same length as the x-Rp axis ?\n>\n\nThey could, let's see how the drawing looks like.\n\n> > +\n> > +\n> > +                                     x-Rc          0\n> > +                           <------------------------+ 0\n> > +                                x-Rp                !\n> > +                     <-----------------------(0,0)  !\n> > +                                               !    !\n> > +                                               !    !\n> > +                                               !    !\n> > +                                               !    V\n> > +                                               !  y-Rc\n> > +                                               V\n> > +                                             y-Rp\n> > +\n> > +        90 degrees camera rotation\n> > +\n> > +                      0        y-Rc\n> > +                   0 +----------------------->\n> > +                     !\n> > +                     !    y-Rp\n> > +                     !     ^\n> > +                     !     !\n> > +                     !     !\n> > +                     !     !\n> > +                     !     !\n> > +                     !     !\n> > +                     !     !\n> > +                     !   (0,0)---------------------->\n> > +                     !                 x-Rp\n> > +                     !\n> > +                     V\n> > +                   x-Rc\n> > +\n> > +        180 degrees camera rotation\n> > +\n> > +                                   x-Cr          0\n>\n> s/Cr/Rc/\n>\n> > +                         <------------------------+ 0\n> > +                     y-Rp                         !\n> > +                      ^                           !\n> > +                      !                           ! y-Cr\n> > +                      !                           !\n> > +                      !                           !\n> > +                      !                           V\n> > +                      !\n> > +                      !\n> > +                    (0,0)--------------------->\n> > +                                   x-Rp\n> > +\n> > +        270 degrees camera rotation\n> > +\n> > +                      0        y-Rc\n> > +                   0 +----------------------->\n> > +                     !            x-Rp\n> > +                     ! <-----------------------(0,0\n>\n> Missing ) at the end of the line.\n>\n> > +                     !                           !\n> > +                     !                           !\n> > +                     !                           !\n> > +                     !                           !\n> > +                     !                           !\n> > +                     !                           V\n> > +                     !                         y-Rp\n> > +                     !\n> > +                     !\n> > +                     V\n> > +                   x-Rc\n> > +\n> > +\n> > +\n> > +        Example one - Webcam\n> > +\n> > +        A camera module installed on the user facing part of a laptop screen\n> > +        casing used for video calls. The captured images are meant to be\n> > +        displayed in landscape mode (width > height) on the laptop screen.\n> > +\n> > +        The camera is typically mounted 180 degrees rotated to compensate the\n> > +        lens optical inversion effect.\n>\n> I understand what you mean here, but can be confusing for the reader.\n> The direction of the Rc and Rp reference systems depicted below\n> correspond to the 0° rotation above, and here the text states 180°. How\n> about writing \"The camera sensor is typically mounted upside-down to\n> compensate ...\" ?\n>\n\nAck\n\n> > +\n> > +                          y-Rp\n> > +                    y-Rc   ^\n> > +                     ^     !\n> > +                     !     !       |\\_____)\\__\n> > +                     !     !       ) ____  ___.<\n> > +                     !     !       |/    )/\n> > +                     !     !\n> > +                     !     !\n> > +                     !   (0,0)---------------------->\n> > +                     !                 x-Rp\n> > +                   0 +------------------------------------->\n> > +                      0                x-Rc\n> > +\n> > +        The two reference systems are aligned, the resulting camera rotation is\n> > +        0 degrees, no rotation correction should be applied to the resulting\n>\n> s/should/needs to/ ?\n>\n\nack\n\n> > +        image once captured to memory buffers to correctly display it to users.\n> > +\n> > +                     +--------------------------+\n> > +                     !       |\\____)\\___        !\n> > +                     !       ) _____  __`<      !\n> > +                     !       |/     )/          !\n> > +                     +--------------------------+\n> > +\n> > +        If the camera module is not mounted 180 degrees rotated to compensate\n>\n> And here, for the reason explained above, \"is not mounted upside-down to\n> compensate ...\" ?\n>\n> > +        the lens optical inversion, the two reference system will result not\n>\n> s/system/systems/\n>\n> > +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'\n>\n> s/will result not aligned/will not be aligned/ ?\n>\n\nack on all the above comments\n\n> > +        plane.\n>\n> As above, we should probably not use plane to refer to Rc and Rp, as\n> they're defined as reference systems. How about the following ?\n>\n> \"If the camera sensor is not mounted upside-down to compensate for the\n> lens optical inversion, the two reference systems will not be aligned,\n> with 'Rp' being rotated 180 degrees relatively to 'Rc'.\"\n>\n\nIt's more clear, thanks\n\n> > +                                   x-Rc          0\n> > +                         <------------------------+ 0\n> > +                     y-Rp                         !\n> > +                      ^                           !\n> > +                      !                           ! y-Rc\n> > +                      !      |\\_____)\\__          !\n> > +                      !      ) ____  ___.<        !\n> > +                      !      |/    )/             V\n> > +                      !\n> > +                      !\n> > +                    (0,0)--------------------->\n> > +                                   x-Rp\n> > +\n> > +        The image once captured to memory will then be 180 degrees rotated\n>\n> \"will then be rotated by 180 degrees\" ?\n>\n\nmatter of tastes I guess, but ok\n\n> > +\n> > +                     +--------------------------+\n> > +                     !          __/(_____/|     !\n> > +                     !        >.___  ____ (     !\n> > +                     !             \\(    \\|     !\n> > +                     +--------------------------+\n>\n> Should we add two lines above and below the shark to keep the aspect\n> ration ?\n>\n> > +\n> > +        A software rotation correction of 180 degrees should be applied to\n> > +        correctly display the image.\n> > +\n> > +                     +--------------------------+\n> > +                     !       |\\____)\\___        !\n> > +                     !       ) _____  __`<      !\n> > +                     !       |/     )/          !\n> > +                     +--------------------------+\n>\n> Here too.\n>\n> > +\n> > +        Example two - Phone camera\n> > +\n> > +        A camera installed on the back-side of a mobile device facing away from\n>\n> s/back-side/back side/\n>\n> > +        the user. The captured images are meant to be displayed in portrait mode\n> > +        (height > width) to match the device screen orientation and the device\n> > +        usage orientation used when taking the picture.\n> > +\n> > +        The camera is typically mounted with its pixel array longer side aligned\n>\n> s/camera/camera sensor/\n>\n> > +        to the device longer side, 180 degrees rotated to compensate the lens\n>\n> s/compensate/compensate for/ ?\n>\n> > +        optical inversion effect.\n> > +\n> > +                      0        y-Rc\n> > +                   0 +----------------------->\n> > +                     !\n> > +                     !    y-Rp\n> > +                     !     ^\n> > +                     !     !\n> > +                     !     !       |\\_____)\\__\n> > +                     !     !       ) ____  ___.<\n> > +                     !     !       |/    )/\n> > +                     !     !\n> > +                     !     !\n> > +                     !   (0,0)---------------------->\n> > +                     !                 x-Rp\n> > +                     !\n> > +                     !\n> > +                     V\n> > +                   x-Rc\n> > +\n> > +        The two reference systems are not aligned and the 'Rp' reference\n> > +        system is 90 degrees rotated in counterclockwise direction in respect\n>\n> \"... is rotated by 90 degrees in the counterclockwise direction\n> relatively to the ...\"\n>\n> > +        to the 'Rc' reference system.\n> > +\n> > +        The image, when captured to memory buffer will be rotated.\n>\n> To match the previous example,\n>\n> \"The image once captured to memory will be rotated.\"\n>\n> > +\n> > +                     +---------------------------------------+\n> > +                     |                  _ _                  |\n> > +                     |                 \\   /                 |\n> > +                     |                  | |                  |\n> > +                     |                  | |                  |\n> > +                     |                  |  >                 |\n> > +                     |                 <  |                  |\n> > +                     |                  | |                  |\n> > +                     |                    .                  |\n> > +                     |                   V                   |\n> > +                     +---------------------------------------+\n> > +\n> > +        A correction of 90 degrees in counterclockwise direction has to be\n> > +        applied to correctly display the image in portrait mode on the device\n> > +        screen.\n> > +\n> > +                                +-----------------+\n> > +                                |                 |\n> > +                                |                 |\n> > +                                |                 |\n> > +                                |                 |\n> > +                                |                 |\n> > +                                |                 |\n> > +                                |  |\\____)\\___    |\n> > +                                |  ) _____  __`<  |\n> > +                                |  |/     )/      |\n> > +                                |                 |\n> > +                                |                 |\n> > +                                |                 |\n> > +                                |                 |\n> > +                                |                 |\n> > +                                +-----------------+\n>\n> I had to think a bit to check if the sharks are in the right\n> orientation, and they are :-) I would however reduce the frame a little\n> bit to match the size of the references systems above.\n>\n\nI received the opposed comments from Andrey and I agree with him that\nwe should try to keep the image proportions similar to the ones above\nto avoid the idea of cropping. In my view, the coordinate system does\nnot represent the size of the captured image, but I can work to make\nthem look like it.\n\nThanks\n  j\n\n> >  ...\n>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F1CF60441\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  5 Feb 2020 11:51:16 +0100 (CET)","from vino (mob-31-157-166-105.net.vodafone.it [31.157.166.105])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 76BB5E0004;\n\tWed,  5 Feb 2020 10:51:15 +0000 (UTC)"],"X-Originating-IP":"31.157.166.105","Date":"Wed, 5 Feb 2020 11:51:05 +0100","From":"jacopo mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200205105105.243nbpreess656xj@vino>","References":"<20200113164245.52535-1-jacopo@jmondi.org>\n\t<20200113164245.52535-5-jacopo@jmondi.org>\n\t<20200203234249.GG4722@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200203234249.GG4722@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Wed, 05 Feb 2020 10:51:16 -0000"}},{"id":3746,"web_url":"https://patchwork.libcamera.org/comment/3746/","msgid":"<20200213210150.GN29760@pendragon.ideasonboard.com>","date":"2020-02-13T21:01:50","subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","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, Feb 05, 2020 at 11:51:05AM +0100, jacopo mondi wrote:\n> On Tue, Feb 04, 2020 at 01:42:49AM +0200, Laurent Pinchart wrote:\n> > Hi Jacopo,\n> >\n> > Thank you for the patch.\n> >\n> > This looks really good. The description is quite long, but that's\n> > unavoidable if we want to be both precise and generic enough to cover\n> > all use cases.\n> >\n> > Please see below for a few minor comments.\n> >\n> > On Mon, Jan 13, 2020 at 05:42:26PM +0100, Jacopo Mondi wrote:\n> > > The rotation property describes the rotation of the camera sensor.\n> > >\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > > ---\n> > >  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++\n> > >  1 file changed, 309 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> > > index aaadcbd3e52b..243af7bd0a03 100644\n> > > --- a/src/libcamera/property_ids.yaml\n> > > +++ b/src/libcamera/property_ids.yaml\n> > > @@ -25,4 +25,313 @@ controls:\n> > >            description: |\n> > >              The camera is attached to the device in a way that allows it to\n> > >              be moved freely\n> > > +\n> > > +  - Rotation:\n> > > +      type: int32_t\n> > > +      description: |\n> > > +        The camera rotation is expressed as the angular difference in degrees\n> > > +        between two reference systems, one implicitly defined by the camera\n> >\n> > I wouldn't say \"implicitly defined\" here, as the reference system is\n> > defined below, quite explicitly :-) How about \"one relative to the\n> > camera module\" ?\n> \n> Would you keep \"camera module intrinsics characteristics\" or just\n> \"camera module\" ?\n> \n> I would prefer the former, as the coordintate system is defined by the\n> sensor's pixel array displacement.\n\nI would prefer the latter actually, I'm not sure what \"relative to the\ncamera module intrinsic characteristics\" actually mean.\n\n> > > +        module intrinsics characteristics, and one artificially defined on the\n> > > +        external world scene to be captured when projected on the image sensor\n> > > +        pixel array.\n> > > +\n> > > +        A camera sensor has an implicitly defined 2-dimensional reference\n> > > +        system 'Rc' defined by its pixel array scan-out order, with its origin\n> >\n> > \"scan-out\" is mostly used for displays, for camera sensors the usual\n> > term is \"read-out\".\n> \n> Ack\n> \n> > > +        posed at pixel address (0,0), the x-axis progressing from there towards\n> >\n> > Maybe \"placed\" instead of \"posed\" ? The latter doesn't match a\n> > definition I can find, but I may be wrong.\n> >\n> > Pixels don't have addresses, they have coordinates.\n> \n> I've seen address used in many sensor chip manuals, I think I took it\n> from there, but I can indeed change it.\n> \n> > Unless I'm mistaken axes are usually named with capital letters.\n> >\n> > > +        the last scanned out column of the pixel array and the y-axis\n> > > +        progressing towards the last scanned out line.\n> >\n> > To match the above,\n> >\n> > \"A camera sensor has a 2-dimensional reference system 'Rc' defined by\n> > its pixel array read-out order. The origin is set to the first pixel\n> > being read out, the X-axis points along the column read-out direction\n> > towards the last columns, and the Y-axis along the row read-out\n> > direction towards the last row.\"\n> \n> I'm not sure I'm super happy with the expression\n> \n> \"The X-axis points along the column read-out direction towards the\n> last column.\"\n> \n> if compared to:\n> \n> \"The X-axis progressing from the here [the first read-out pixel]\n> towards the last read-out column.\"\n\nDid you mean \"there\" instead of \"the here\" ? I find the second a bit\nstrange (otherwise I wouldn't have commented on it :-)), but it may be a\nmatter of personal taste.\n\nKieran, what can your native English skills teach us ?\n\n> > > +\n> > > +        A typical example for a sensor with a (2592x1944) pixel array matrix\n> >\n> > You can drop the parentheses here, they're used when expressing\n> > coordinate tuples, but not for sizes.\n> \n> Ack\n> \n> > > +        observed from the front is\n> > > +\n> > > +                   (2592)      x-axis          0\n> >\n> > s/x/X/ (and below, and same for y)\n> \n> Ack\n> \n> > > +                      <------------------------+ 0\n> > > +                      .......... ... ..........!\n> > > +                      .......... ... ..........! y-axis\n> > > +                                 ...           !\n> > > +                      .......... ... ..........!\n> > > +                      .......... ... ..........! (1944)\n> > > +                                               V\n> >\n> > 2592 and 1944 should be 2591 and 1943 respectively as you count from 0.\n> > The parentheses are also not needed.\n> \n> Ack\n> \n> > Typically sensors also have dark and active boundary rows and columns,\n> > but that may be too much details to include here.\n> \n> Not a detail I would include here. And, by the way, I the number I\n> have used here have been taken from the full pixel array size of a\n> sensor producing 2560x1920 images. They include black and calibration\n> pixels.\n> \n> > > +        The external world scene reference system scene 'Rs' is defined as a\n> >\n> > s/system scene/system/ ?\n> \n> Ack\n> \n> > > +        2-dimensional reference system on the parallel plane posed in front\n> > > +        of the camera module's focal plane, with its origin placed on the\n> >\n> > This is a bit complicated, how about simply saying that the reference\n> > system is defined on the focal plane of the sensor ?\n> \n> Ack\n> \n> > > +        visible top-left corner, the x-axis progressing towards the right from\n> > > +        there and the y-axis progressing towards the bottom of the visible\n> > > +        scene.\n> >\n> > Shouldn't we point out that the terms top, left, right and bottom are\n> > intentionally not defined ?\n> >\n> > With the above comments applied here,\n> >\n> > \"The external world scene reference system 'Rs' is a 2-dimensional\n> > reference system on the focal plane of the camera module. The origin is\n> > placed on the top-left corner of the visible scene, the X-axis points\n> > towards the right, and the Y-axis points towards the bottom of the\n> > scene. The top, bottom, left and right directions are intentionally not\n> > defined and depend on the environment in which the camera is used.\"\n> \n> Ack\n> \n> > > +\n> > > +        A typical example of a (very common) picture of a shark swimming from\n> > > +        left to right is\n> >\n> > \"from left to right, as seen from the camera, is\"\n> \n> Ack\n> \n> > > +\n> > > +                                 x-axis\n> > > +                   (0,0)---------------------->\n> > > +                     !\n> > > +                     !\n> > > +                     !       |\\____)\\___\n> > > +                     !       ) _____  __`<\n> > > +                     !       |/     )/\n> > > +                     !\n> > > +                     V\n> > > +                   y-axis\n> > > +\n> > > +        With the reference plane posed in front of the camera module and\n> > > +        parallel to its focal plane\n> > > +\n> > > +                                   !\n> > > +                                 / !\n> > > +                                /  !\n> > > +                               /   !\n> > > +                        _     /    !\n> > > +                     +-/ \\-+ /     !\n> > > +                     | (o) |       ! 'Rs' reference plane\n> > > +                     +-----+ \\     !\n> > > +                              \\    !\n> > > +                               \\   !\n> > > +                                \\  !\n> > > +                                 \\ !\n> > > +                                   !\n> > > +\n> > > +        When projected on the sensor's pixel array, the image and the associated\n> > > +        reference system 'Rs' are typically inverted, due to the camera module's\n> >\n> > Maybe \"typically (but not always)\" ?\n> >\n> > > +        lens optical inversion effect.\n> > > +\n> > > +        Assuming the above represented scene of the swimming shark, the lens\n> > > +        inversion projects on the sensor pixel array the reference plane 'Rp'\n> >\n> > I got confused for a second, wondering why only the Y axis was inverted,\n> > until I realized the this drawing shows the scene projected onto the\n> > camera module as seen when looking at the camera module. I think we\n> > should mention this explicitly:\n> \n> Indeed, thanks for pointing it out. I inverted the position of the\n> observer without even noticing.\n> \n> > \"Assuming the above represented scene of the swimming shark, the lens\n> > inversion projects the scene and its reference system onto the sensor\n> > pixel array as follows, seen from the front of the camera sensor.\"\n> \n> Ack\n> \n> > > +\n> > > +                  y-axis\n> > > +                     ^\n> > > +                     !\n> > > +                     !       |\\_____)\\__\n> > > +                     !       ) ____  ___.<\n> > > +                     !       |/    )/\n> > > +                     !\n> > > +                     !\n> > > +                   (0,0)---------------------->\n> > > +                                 x-axis\n> >\n> > \"Note the shark being upside-down.\n> >\n> > The resulting projected reference system is named 'Rp'.\"\n> >\n> > > +\n> > > +        The camera rotation property is then defined as the angular difference\n> > > +        in counterclockwise direction between the origin of the camera reference\n> >\n> > Nitpicking again, an angular difference can't be defined between\n> > origins, as origins are points. It should be defined between reference\n> > systems.\n> >\n> > We should specify that the rotation is expressed in degrees as a number\n> > in the [0, 360[ range.\n> \n> true\n> \n> > > +        system 'Rc', defined by the camera sensor scan-out direction and its\n> > > +        mounting position, and the origin of the projected scene reference\n> > > +        system 'Rp', result of the optical projection of the scene reference\n> > > +        system 'Rs' on the sensor pixel array.\n> >\n> > I don't think we need to repeat the definitions of the two reference\n> > systems.\n> \n> I was not sure it was needed, but it seemed to me it was not too heavy\n> to read, but it can be dropped indeed\n> \n> > \"The camera rotation property is then defined as the angular difference\n> > in the counterclockwise direction between the camera reference system\n> > 'Rc' and the projected scene reference system 'Rp'. It is expressed in\n> > degrees as a number in the range [0, 360[.\"\n> \n> Thanks, works well\n> \n> > > +\n> > > +        Examples\n> > > +\n> > > +        0 degrees camera rotation\n> > > +\n> > > +                          y-Rp\n> > > +                    y-Rc   ^\n> > > +                     ^     !\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !   (0,0)---------------------->\n> > > +                     !                 x-Rp\n> > > +                   0 +------------------------------------->\n> > > +                      0            x-Rc\n> >\n> > The X-axis 0 should be aligned with the +, or the Y-axis 0 moved one\n> > line up. You may want to use the same notation for the Rc and Rp\n> > reference systems ('(0,0)' or split coordinates in both cases).\n> \n> I am not sure why I used two different notations for the camera and\n> the projected scene reference systems. I will use a single one\n> \n> > Should the x-Rc axis have the same length as the x-Rp axis ?\n> \n> They could, let's see how the drawing looks like.\n> \n> > > +\n> > > +\n> > > +                                     x-Rc          0\n> > > +                           <------------------------+ 0\n> > > +                                x-Rp                !\n> > > +                     <-----------------------(0,0)  !\n> > > +                                               !    !\n> > > +                                               !    !\n> > > +                                               !    !\n> > > +                                               !    V\n> > > +                                               !  y-Rc\n> > > +                                               V\n> > > +                                             y-Rp\n> > > +\n> > > +        90 degrees camera rotation\n> > > +\n> > > +                      0        y-Rc\n> > > +                   0 +----------------------->\n> > > +                     !\n> > > +                     !    y-Rp\n> > > +                     !     ^\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !   (0,0)---------------------->\n> > > +                     !                 x-Rp\n> > > +                     !\n> > > +                     V\n> > > +                   x-Rc\n> > > +\n> > > +        180 degrees camera rotation\n> > > +\n> > > +                                   x-Cr          0\n> >\n> > s/Cr/Rc/\n> >\n> > > +                         <------------------------+ 0\n> > > +                     y-Rp                         !\n> > > +                      ^                           !\n> > > +                      !                           ! y-Cr\n> > > +                      !                           !\n> > > +                      !                           !\n> > > +                      !                           V\n> > > +                      !\n> > > +                      !\n> > > +                    (0,0)--------------------->\n> > > +                                   x-Rp\n> > > +\n> > > +        270 degrees camera rotation\n> > > +\n> > > +                      0        y-Rc\n> > > +                   0 +----------------------->\n> > > +                     !            x-Rp\n> > > +                     ! <-----------------------(0,0\n> >\n> > Missing ) at the end of the line.\n> >\n> > > +                     !                           !\n> > > +                     !                           !\n> > > +                     !                           !\n> > > +                     !                           !\n> > > +                     !                           !\n> > > +                     !                           V\n> > > +                     !                         y-Rp\n> > > +                     !\n> > > +                     !\n> > > +                     V\n> > > +                   x-Rc\n> > > +\n> > > +\n> > > +\n> > > +        Example one - Webcam\n> > > +\n> > > +        A camera module installed on the user facing part of a laptop screen\n> > > +        casing used for video calls. The captured images are meant to be\n> > > +        displayed in landscape mode (width > height) on the laptop screen.\n> > > +\n> > > +        The camera is typically mounted 180 degrees rotated to compensate the\n> > > +        lens optical inversion effect.\n> >\n> > I understand what you mean here, but can be confusing for the reader.\n> > The direction of the Rc and Rp reference systems depicted below\n> > correspond to the 0° rotation above, and here the text states 180°. How\n> > about writing \"The camera sensor is typically mounted upside-down to\n> > compensate ...\" ?\n> \n> Ack\n> \n> > > +\n> > > +                          y-Rp\n> > > +                    y-Rc   ^\n> > > +                     ^     !\n> > > +                     !     !       |\\_____)\\__\n> > > +                     !     !       ) ____  ___.<\n> > > +                     !     !       |/    )/\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !   (0,0)---------------------->\n> > > +                     !                 x-Rp\n> > > +                   0 +------------------------------------->\n> > > +                      0                x-Rc\n> > > +\n> > > +        The two reference systems are aligned, the resulting camera rotation is\n> > > +        0 degrees, no rotation correction should be applied to the resulting\n> >\n> > s/should/needs to/ ?\n> \n> ack\n> \n> > > +        image once captured to memory buffers to correctly display it to users.\n> > > +\n> > > +                     +--------------------------+\n> > > +                     !       |\\____)\\___        !\n> > > +                     !       ) _____  __`<      !\n> > > +                     !       |/     )/          !\n> > > +                     +--------------------------+\n> > > +\n> > > +        If the camera module is not mounted 180 degrees rotated to compensate\n> >\n> > And here, for the reason explained above, \"is not mounted upside-down to\n> > compensate ...\" ?\n> >\n> > > +        the lens optical inversion, the two reference system will result not\n> >\n> > s/system/systems/\n> >\n> > > +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'\n> >\n> > s/will result not aligned/will not be aligned/ ?\n> \n> ack on all the above comments\n> \n> > > +        plane.\n> >\n> > As above, we should probably not use plane to refer to Rc and Rp, as\n> > they're defined as reference systems. How about the following ?\n> >\n> > \"If the camera sensor is not mounted upside-down to compensate for the\n> > lens optical inversion, the two reference systems will not be aligned,\n> > with 'Rp' being rotated 180 degrees relatively to 'Rc'.\"\n> \n> It's more clear, thanks\n> \n> > > +                                   x-Rc          0\n> > > +                         <------------------------+ 0\n> > > +                     y-Rp                         !\n> > > +                      ^                           !\n> > > +                      !                           ! y-Rc\n> > > +                      !      |\\_____)\\__          !\n> > > +                      !      ) ____  ___.<        !\n> > > +                      !      |/    )/             V\n> > > +                      !\n> > > +                      !\n> > > +                    (0,0)--------------------->\n> > > +                                   x-Rp\n> > > +\n> > > +        The image once captured to memory will then be 180 degrees rotated\n> >\n> > \"will then be rotated by 180 degrees\" ?\n> \n> matter of tastes I guess, but ok\n> \n> > > +\n> > > +                     +--------------------------+\n> > > +                     !          __/(_____/|     !\n> > > +                     !        >.___  ____ (     !\n> > > +                     !             \\(    \\|     !\n> > > +                     +--------------------------+\n> >\n> > Should we add two lines above and below the shark to keep the aspect\n> > ration ?\n> >\n> > > +\n> > > +        A software rotation correction of 180 degrees should be applied to\n> > > +        correctly display the image.\n> > > +\n> > > +                     +--------------------------+\n> > > +                     !       |\\____)\\___        !\n> > > +                     !       ) _____  __`<      !\n> > > +                     !       |/     )/          !\n> > > +                     +--------------------------+\n> >\n> > Here too.\n> >\n> > > +\n> > > +        Example two - Phone camera\n> > > +\n> > > +        A camera installed on the back-side of a mobile device facing away from\n> >\n> > s/back-side/back side/\n> >\n> > > +        the user. The captured images are meant to be displayed in portrait mode\n> > > +        (height > width) to match the device screen orientation and the device\n> > > +        usage orientation used when taking the picture.\n> > > +\n> > > +        The camera is typically mounted with its pixel array longer side aligned\n> >\n> > s/camera/camera sensor/\n> >\n> > > +        to the device longer side, 180 degrees rotated to compensate the lens\n> >\n> > s/compensate/compensate for/ ?\n> >\n> > > +        optical inversion effect.\n> > > +\n> > > +                      0        y-Rc\n> > > +                   0 +----------------------->\n> > > +                     !\n> > > +                     !    y-Rp\n> > > +                     !     ^\n> > > +                     !     !\n> > > +                     !     !       |\\_____)\\__\n> > > +                     !     !       ) ____  ___.<\n> > > +                     !     !       |/    )/\n> > > +                     !     !\n> > > +                     !     !\n> > > +                     !   (0,0)---------------------->\n> > > +                     !                 x-Rp\n> > > +                     !\n> > > +                     !\n> > > +                     V\n> > > +                   x-Rc\n> > > +\n> > > +        The two reference systems are not aligned and the 'Rp' reference\n> > > +        system is 90 degrees rotated in counterclockwise direction in respect\n> >\n> > \"... is rotated by 90 degrees in the counterclockwise direction\n> > relatively to the ...\"\n> >\n> > > +        to the 'Rc' reference system.\n> > > +\n> > > +        The image, when captured to memory buffer will be rotated.\n> >\n> > To match the previous example,\n> >\n> > \"The image once captured to memory will be rotated.\"\n> >\n> > > +\n> > > +                     +---------------------------------------+\n> > > +                     |                  _ _                  |\n> > > +                     |                 \\   /                 |\n> > > +                     |                  | |                  |\n> > > +                     |                  | |                  |\n> > > +                     |                  |  >                 |\n> > > +                     |                 <  |                  |\n> > > +                     |                  | |                  |\n> > > +                     |                    .                  |\n> > > +                     |                   V                   |\n> > > +                     +---------------------------------------+\n> > > +\n> > > +        A correction of 90 degrees in counterclockwise direction has to be\n> > > +        applied to correctly display the image in portrait mode on the device\n> > > +        screen.\n> > > +\n> > > +                                +-----------------+\n> > > +                                |                 |\n> > > +                                |                 |\n> > > +                                |                 |\n> > > +                                |                 |\n> > > +                                |                 |\n> > > +                                |                 |\n> > > +                                |  |\\____)\\___    |\n> > > +                                |  ) _____  __`<  |\n> > > +                                |  |/     )/      |\n> > > +                                |                 |\n> > > +                                |                 |\n> > > +                                |                 |\n> > > +                                |                 |\n> > > +                                |                 |\n> > > +                                +-----------------+\n> >\n> > I had to think a bit to check if the sharks are in the right\n> > orientation, and they are :-) I would however reduce the frame a little\n> > bit to match the size of the references systems above.\n> \n> I received the opposed comments from Andrey and I agree with him that\n> we should try to keep the image proportions similar to the ones above\n> to avoid the idea of cropping. In my view, the coordinate system does\n> not represent the size of the captured image, but I can work to make\n> them look like it.\n> \n> > >  ...\n> >\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D8EEB60962\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 13 Feb 2020 22:02:07 +0100 (CET)","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 3A210504;\n\tThu, 13 Feb 2020 22:02:07 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1581627727;\n\tbh=p+4a+TSPM6FNe2cIzgpIOZNLGBU1X7hcepxLW56Zu2o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=stkCK/ABuwBlMZ4W+CBY5IIZIEfNXA0eB45BPR9qEzOxqis3mECp74PvEZI9NCWpj\n\tqnZaGRacUqgyS2jZPexFEvfz/hkceQbre7nLJMCe/RQ/EzGNy+tUe3ekhAZXne3IpA\n\tVyakAAcPkMY66et92Rb+4iKcnEEQ16UAPWUaSWEc=","Date":"Thu, 13 Feb 2020 23:01:50 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"jacopo mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200213210150.GN29760@pendragon.ideasonboard.com>","References":"<20200113164245.52535-1-jacopo@jmondi.org>\n\t<20200113164245.52535-5-jacopo@jmondi.org>\n\t<20200203234249.GG4722@pendragon.ideasonboard.com>\n\t<20200205105105.243nbpreess656xj@vino>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200205105105.243nbpreess656xj@vino>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 13 Feb 2020 21:02:08 -0000"}},{"id":3747,"web_url":"https://patchwork.libcamera.org/comment/3747/","msgid":"<589c37c1-d474-07c8-34e1-e5fbcb2246f0@ideasonboard.com>","date":"2020-02-13T21:46:22","subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 13/02/2020 21:01, Laurent Pinchart wrote:\n> Hi Jacopo,\n> \n> On Wed, Feb 05, 2020 at 11:51:05AM +0100, jacopo mondi wrote:\n>> On Tue, Feb 04, 2020 at 01:42:49AM +0200, Laurent Pinchart wrote:\n>>> Hi Jacopo,\n>>>\n>>> Thank you for the patch.\n>>>\n>>> This looks really good. The description is quite long, but that's\n>>> unavoidable if we want to be both precise and generic enough to cover\n>>> all use cases.\n>>>\n>>> Please see below for a few minor comments.\n>>>\n>>> On Mon, Jan 13, 2020 at 05:42:26PM +0100, Jacopo Mondi wrote:\n>>>> The rotation property describes the rotation of the camera sensor.\n>>>>\n>>>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>>>> ---\n>>>>  src/libcamera/property_ids.yaml | 309 ++++++++++++++++++++++++++++++++\n>>>>  1 file changed, 309 insertions(+)\n>>>>\n>>>> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n>>>> index aaadcbd3e52b..243af7bd0a03 100644\n>>>> --- a/src/libcamera/property_ids.yaml\n>>>> +++ b/src/libcamera/property_ids.yaml\n>>>> @@ -25,4 +25,313 @@ controls:\n>>>>            description: |\n>>>>              The camera is attached to the device in a way that allows it to\n>>>>              be moved freely\n>>>> +\n>>>> +  - Rotation:\n>>>> +      type: int32_t\n>>>> +      description: |\n>>>> +        The camera rotation is expressed as the angular difference in degrees\n>>>> +        between two reference systems, one implicitly defined by the camera\n>>>\n>>> I wouldn't say \"implicitly defined\" here, as the reference system is\n>>> defined below, quite explicitly :-) How about \"one relative to the\n>>> camera module\" ?\n>>\n>> Would you keep \"camera module intrinsics characteristics\" or just\n>> \"camera module\" ?\n>>\n>> I would prefer the former, as the coordintate system is defined by the\n>> sensor's pixel array displacement.\n> \n> I would prefer the latter actually, I'm not sure what \"relative to the\n> camera module intrinsic characteristics\" actually mean.\n> \n>>>> +        module intrinsics characteristics, and one artificially defined on the\n>>>> +        external world scene to be captured when projected on the image sensor\n>>>> +        pixel array.\n>>>> +\n>>>> +        A camera sensor has an implicitly defined 2-dimensional reference\n>>>> +        system 'Rc' defined by its pixel array scan-out order, with its origin\n>>>\n>>> \"scan-out\" is mostly used for displays, for camera sensors the usual\n>>> term is \"read-out\".\n>>\n>> Ack\n>>\n>>>> +        posed at pixel address (0,0), the x-axis progressing from there towards\n>>>\n>>> Maybe \"placed\" instead of \"posed\" ? The latter doesn't match a\n>>> definition I can find, but I may be wrong.\n\n>>>\n>>> Pixels don't have addresses, they have coordinates.\n\nI'd use 'positioned at coordinate 0,0' or such then ...\n\nThe origin hasn't been 'placed'.\n\nYou could also use \"located at ...\"\n\n\n>> I've seen address used in many sensor chip manuals, I think I took it\n>> from there, but I can indeed change it.\n>>\n>>> Unless I'm mistaken axes are usually named with capital letters.\n>>>\n>>>> +        the last scanned out column of the pixel array and the y-axis\n>>>> +        progressing towards the last scanned out line.\n>>>\n>>> To match the above,\n>>>\n>>> \"A camera sensor has a 2-dimensional reference system 'Rc' defined by\n>>> its pixel array read-out order. The origin is set to the first pixel\n>>> being read out, the X-axis points along the column read-out direction\n>>> towards the last columns, and the Y-axis along the row read-out\n>>> direction towards the last row.\"\n>>\n>> I'm not sure I'm super happy with the expression\n>>\n>> \"The X-axis points along the column read-out direction towards the\n>> last column.\"\n>>\n>> if compared to:\n>>\n>> \"The X-axis progressing from the here [the first read-out pixel]\n>> towards the last read-out column.\"\n> \n> Did you mean \"there\" instead of \"the here\" ? I find the second a bit\n> strange (otherwise I wouldn't have commented on it :-)), but it may be a\n> matter of personal taste.\n> \n> Kieran, what can your native English skills teach us ?\n\n\nWe'd never really use \"the here\" ... but in the sentences above \"there\"\ndoesn't make sense either...\n\nI think I'm missing the original context statement...\n\n\n> \n>>>> +\n>>>> +        A typical example for a sensor with a (2592x1944) pixel array matrix\n>>>\n>>> You can drop the parentheses here, they're used when expressing\n>>> coordinate tuples, but not for sizes.\n>>\n>> Ack\n>>\n>>>> +        observed from the front is\n>>>> +\n>>>> +                   (2592)      x-axis          0\n>>>\n>>> s/x/X/ (and below, and same for y)\n>>\n>> Ack\n>>\n>>>> +                      <------------------------+ 0\n>>>> +                      .......... ... ..........!\n>>>> +                      .......... ... ..........! y-axis\n>>>> +                                 ...           !\n>>>> +                      .......... ... ..........!\n>>>> +                      .......... ... ..........! (1944)\n>>>> +                                               V\n>>>\n>>> 2592 and 1944 should be 2591 and 1943 respectively as you count from 0.\n>>> The parentheses are also not needed.\n>>\n>> Ack\n>>\n>>> Typically sensors also have dark and active boundary rows and columns,\n>>> but that may be too much details to include here.\n>>\n>> Not a detail I would include here. And, by the way, I the number I\n>> have used here have been taken from the full pixel array size of a\n>> sensor producing 2560x1920 images. They include black and calibration\n>> pixels.\n>>\n>>>> +        The external world scene reference system scene 'Rs' is defined as a\n>>>\n>>> s/system scene/system/ ?\n>>\n>> Ack\n>>\n>>>> +        2-dimensional reference system on the parallel plane posed in front\n>>>> +        of the camera module's focal plane, with its origin placed on the\n>>>\n>>> This is a bit complicated, how about simply saying that the reference\n>>> system is defined on the focal plane of the sensor ?\n>>\n>> Ack\n>>\n>>>> +        visible top-left corner, the x-axis progressing towards the right from\n>>>> +        there and the y-axis progressing towards the bottom of the visible\n>>>> +        scene.\n>>>\n>>> Shouldn't we point out that the terms top, left, right and bottom are\n>>> intentionally not defined ?\n>>>\n>>> With the above comments applied here,\n>>>\n>>> \"The external world scene reference system 'Rs' is a 2-dimensional\n>>> reference system on the focal plane of the camera module. The origin is\n>>> placed on the top-left corner of the visible scene, the X-axis points\n>>> towards the right, and the Y-axis points towards the bottom of the\n>>> scene. The top, bottom, left and right directions are intentionally not\n>>> defined and depend on the environment in which the camera is used.\"\n>>\n>> Ack\n>>\n>>>> +\n>>>> +        A typical example of a (very common) picture of a shark swimming from\n>>>> +        left to right is\n>>>\n>>> \"from left to right, as seen from the camera, is\"\n>>\n>> Ack\n>>\n>>>> +\n>>>> +                                 x-axis\n>>>> +                   (0,0)---------------------->\n>>>> +                     !\n>>>> +                     !\n>>>> +                     !       |\\____)\\___\n>>>> +                     !       ) _____  __`<\n>>>> +                     !       |/     )/\n>>>> +                     !\n>>>> +                     V\n>>>> +                   y-axis\n>>>> +\n>>>> +        With the reference plane posed in front of the camera module and\n>>>> +        parallel to its focal plane\n>>>> +\n>>>> +                                   !\n>>>> +                                 / !\n>>>> +                                /  !\n>>>> +                               /   !\n>>>> +                        _     /    !\n>>>> +                     +-/ \\-+ /     !\n>>>> +                     | (o) |       ! 'Rs' reference plane\n>>>> +                     +-----+ \\     !\n>>>> +                              \\    !\n>>>> +                               \\   !\n>>>> +                                \\  !\n>>>> +                                 \\ !\n>>>> +                                   !\n>>>> +\n>>>> +        When projected on the sensor's pixel array, the image and the associated\n>>>> +        reference system 'Rs' are typically inverted, due to the camera module's\n>>>\n>>> Maybe \"typically (but not always)\" ?\n>>>\n>>>> +        lens optical inversion effect.\n>>>> +\n>>>> +        Assuming the above represented scene of the swimming shark, the lens\n>>>> +        inversion projects on the sensor pixel array the reference plane 'Rp'\n>>>\n>>> I got confused for a second, wondering why only the Y axis was inverted,\n>>> until I realized the this drawing shows the scene projected onto the\n>>> camera module as seen when looking at the camera module. I think we\n>>> should mention this explicitly:\n>>\n>> Indeed, thanks for pointing it out. I inverted the position of the\n>> observer without even noticing.\n>>\n>>> \"Assuming the above represented scene of the swimming shark, the lens\n>>> inversion projects the scene and its reference system onto the sensor\n>>> pixel array as follows, seen from the front of the camera sensor.\"\n>>\n>> Ack\n>>\n>>>> +\n>>>> +                  y-axis\n>>>> +                     ^\n>>>> +                     !\n>>>> +                     !       |\\_____)\\__\n>>>> +                     !       ) ____  ___.<\n>>>> +                     !       |/    )/\n>>>> +                     !\n>>>> +                     !\n>>>> +                   (0,0)---------------------->\n>>>> +                                 x-axis\n>>>\n>>> \"Note the shark being upside-down.\n>>>\n>>> The resulting projected reference system is named 'Rp'.\"\n>>>\n>>>> +\n>>>> +        The camera rotation property is then defined as the angular difference\n>>>> +        in counterclockwise direction between the origin of the camera reference\n\ns/counterclockwise/counter-clockwise/\n\n\n>>>\n>>> Nitpicking again, an angular difference can't be defined between\n>>> origins, as origins are points. It should be defined between reference\n>>> systems.\n>>>\n>>> We should specify that the rotation is expressed in degrees as a number\n>>> in the [0, 360[ range.\n>>\n>> true\n>>\n>>>> +        system 'Rc', defined by the camera sensor scan-out direction and its\n>>>> +        mounting position, and the origin of the projected scene reference\n>>>> +        system 'Rp', result of the optical projection of the scene reference\n>>>> +        system 'Rs' on the sensor pixel array.\n>>>\n>>> I don't think we need to repeat the definitions of the two reference\n>>> systems.\n>>\n>> I was not sure it was needed, but it seemed to me it was not too heavy\n>> to read, but it can be dropped indeed\n>>\n>>> \"The camera rotation property is then defined as the angular difference\n>>> in the counterclockwise direction between the camera reference system\n>>> 'Rc' and the projected scene reference system 'Rp'. It is expressed in\n>>> degrees as a number in the range [0, 360[.\"\n>>\n>> Thanks, works well\n>>\n>>>> +\n>>>> +        Examples\n>>>> +\n>>>> +        0 degrees camera rotation\n>>>> +\n>>>> +                          y-Rp\n>>>> +                    y-Rc   ^\n>>>> +                     ^     !\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !   (0,0)---------------------->\n>>>> +                     !                 x-Rp\n>>>> +                   0 +------------------------------------->\n>>>> +                      0            x-Rc\n>>>\n>>> The X-axis 0 should be aligned with the +, or the Y-axis 0 moved one\n>>> line up. You may want to use the same notation for the Rc and Rp\n>>> reference systems ('(0,0)' or split coordinates in both cases).\n>>\n>> I am not sure why I used two different notations for the camera and\n>> the projected scene reference systems. I will use a single one\n>>\n>>> Should the x-Rc axis have the same length as the x-Rp axis ?\n>>\n>> They could, let's see how the drawing looks like.\n>>\n>>>> +\n>>>> +\n>>>> +                                     x-Rc          0\n>>>> +                           <------------------------+ 0\n>>>> +                                x-Rp                !\n>>>> +                     <-----------------------(0,0)  !\n>>>> +                                               !    !\n>>>> +                                               !    !\n>>>> +                                               !    !\n>>>> +                                               !    V\n>>>> +                                               !  y-Rc\n>>>> +                                               V\n>>>> +                                             y-Rp\n>>>> +\n>>>> +        90 degrees camera rotation\n>>>> +\n>>>> +                      0        y-Rc\n>>>> +                   0 +----------------------->\n>>>> +                     !\n>>>> +                     !    y-Rp\n>>>> +                     !     ^\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !   (0,0)---------------------->\n>>>> +                     !                 x-Rp\n>>>> +                     !\n>>>> +                     V\n>>>> +                   x-Rc\n>>>> +\n>>>> +        180 degrees camera rotation\n>>>> +\n>>>> +                                   x-Cr          0\n>>>\n>>> s/Cr/Rc/\n>>>\n>>>> +                         <------------------------+ 0\n>>>> +                     y-Rp                         !\n>>>> +                      ^                           !\n>>>> +                      !                           ! y-Cr\n>>>> +                      !                           !\n>>>> +                      !                           !\n>>>> +                      !                           V\n>>>> +                      !\n>>>> +                      !\n>>>> +                    (0,0)--------------------->\n>>>> +                                   x-Rp\n>>>> +\n>>>> +        270 degrees camera rotation\n>>>> +\n>>>> +                      0        y-Rc\n>>>> +                   0 +----------------------->\n>>>> +                     !            x-Rp\n>>>> +                     ! <-----------------------(0,0\n>>>\n>>> Missing ) at the end of the line.\n>>>\n>>>> +                     !                           !\n>>>> +                     !                           !\n>>>> +                     !                           !\n>>>> +                     !                           !\n>>>> +                     !                           !\n>>>> +                     !                           V\n>>>> +                     !                         y-Rp\n>>>> +                     !\n>>>> +                     !\n>>>> +                     V\n>>>> +                   x-Rc\n>>>> +\n>>>> +\n>>>> +\n>>>> +        Example one - Webcam\n>>>> +\n>>>> +        A camera module installed on the user facing part of a laptop screen\n>>>> +        casing used for video calls. The captured images are meant to be\n>>>> +        displayed in landscape mode (width > height) on the laptop screen.\n>>>> +\n>>>> +        The camera is typically mounted 180 degrees rotated to compensate the\n>>>> +        lens optical inversion effect.\n>>>\n>>> I understand what you mean here, but can be confusing for the reader.\n>>> The direction of the Rc and Rp reference systems depicted below\n>>> correspond to the 0° rotation above, and here the text states 180°. How\n>>> about writing \"The camera sensor is typically mounted upside-down to\n>>> compensate ...\" ?\n>>\n>> Ack\n>>\n>>>> +\n>>>> +                          y-Rp\n>>>> +                    y-Rc   ^\n>>>> +                     ^     !\n>>>> +                     !     !       |\\_____)\\__\n>>>> +                     !     !       ) ____  ___.<\n>>>> +                     !     !       |/    )/\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !   (0,0)---------------------->\n>>>> +                     !                 x-Rp\n>>>> +                   0 +------------------------------------->\n>>>> +                      0                x-Rc\n>>>> +\n>>>> +        The two reference systems are aligned, the resulting camera rotation is\n>>>> +        0 degrees, no rotation correction should be applied to the resulting\n>>>\n>>> s/should/needs to/ ?\n>>\n>> ack\n>>\n>>>> +        image once captured to memory buffers to correctly display it to users.\n>>>> +\n>>>> +                     +--------------------------+\n>>>> +                     !       |\\____)\\___        !\n>>>> +                     !       ) _____  __`<      !\n>>>> +                     !       |/     )/          !\n>>>> +                     +--------------------------+\n>>>> +\n>>>> +        If the camera module is not mounted 180 degrees rotated to compensate\n>>>\n>>> And here, for the reason explained above, \"is not mounted upside-down to\n>>> compensate ...\" ?\n>>>\n>>>> +        the lens optical inversion, the two reference system will result not\n>>>\n>>> s/system/systems/\n>>>\n>>>> +        aligned, with 'Rp' plane 180 degrees rotated in respect to the 'Rc'\n>>>\n>>> s/will result not aligned/will not be aligned/ ?\n>>\n>> ack on all the above comments\n>>\n>>>> +        plane.\n>>>\n>>> As above, we should probably not use plane to refer to Rc and Rp, as\n>>> they're defined as reference systems. How about the following ?\n>>>\n>>> \"If the camera sensor is not mounted upside-down to compensate for the\n>>> lens optical inversion, the two reference systems will not be aligned,\n>>> with 'Rp' being rotated 180 degrees relatively to 'Rc'.\"\n>>\n>> It's more clear, thanks\n>>\n>>>> +                                   x-Rc          0\n>>>> +                         <------------------------+ 0\n>>>> +                     y-Rp                         !\n>>>> +                      ^                           !\n>>>> +                      !                           ! y-Rc\n>>>> +                      !      |\\_____)\\__          !\n>>>> +                      !      ) ____  ___.<        !\n>>>> +                      !      |/    )/             V\n>>>> +                      !\n>>>> +                      !\n>>>> +                    (0,0)--------------------->\n>>>> +                                   x-Rp\n>>>> +\n>>>> +        The image once captured to memory will then be 180 degrees rotated\n>>>\n>>> \"will then be rotated by 180 degrees\" ?\n>>\n>> matter of tastes I guess, but ok\n>>\n>>>> +\n>>>> +                     +--------------------------+\n>>>> +                     !          __/(_____/|     !\n>>>> +                     !        >.___  ____ (     !\n>>>> +                     !             \\(    \\|     !\n>>>> +                     +--------------------------+\n>>>\n>>> Should we add two lines above and below the shark to keep the aspect\n>>> ration ?\n>>>\n>>>> +\n>>>> +        A software rotation correction of 180 degrees should be applied to\n>>>> +        correctly display the image.\n>>>> +\n>>>> +                     +--------------------------+\n>>>> +                     !       |\\____)\\___        !\n>>>> +                     !       ) _____  __`<      !\n>>>> +                     !       |/     )/          !\n>>>> +                     +--------------------------+\n>>>\n>>> Here too.\n>>>\n>>>> +\n>>>> +        Example two - Phone camera\n>>>> +\n>>>> +        A camera installed on the back-side of a mobile device facing away from\n>>>\n>>> s/back-side/back side/\n>>>\n>>>> +        the user. The captured images are meant to be displayed in portrait mode\n>>>> +        (height > width) to match the device screen orientation and the device\n>>>> +        usage orientation used when taking the picture.\n>>>> +\n>>>> +        The camera is typically mounted with its pixel array longer side aligned\n>>>\n>>> s/camera/camera sensor/\n>>>\n>>>> +        to the device longer side, 180 degrees rotated to compensate the lens\n>>>\n>>> s/compensate/compensate for/ ?\n>>>\n>>>> +        optical inversion effect.\n>>>> +\n>>>> +                      0        y-Rc\n>>>> +                   0 +----------------------->\n>>>> +                     !\n>>>> +                     !    y-Rp\n>>>> +                     !     ^\n>>>> +                     !     !\n>>>> +                     !     !       |\\_____)\\__\n>>>> +                     !     !       ) ____  ___.<\n>>>> +                     !     !       |/    )/\n>>>> +                     !     !\n>>>> +                     !     !\n>>>> +                     !   (0,0)---------------------->\n>>>> +                     !                 x-Rp\n>>>> +                     !\n>>>> +                     !\n>>>> +                     V\n>>>> +                   x-Rc\n>>>> +\n>>>> +        The two reference systems are not aligned and the 'Rp' reference\n>>>> +        system is 90 degrees rotated in counterclockwise direction in respect\n>>>\n>>> \"... is rotated by 90 degrees in the counterclockwise direction\n>>> relatively to the ...\"\n\ns/counterclockwise/counter-clockwise/\n\n\n>>>\n>>>> +        to the 'Rc' reference system.\n>>>> +\n>>>> +        The image, when captured to memory buffer will be rotated.\n>>>\n>>> To match the previous example,\n>>>\n>>> \"The image once captured to memory will be rotated.\"\n>>>\n>>>> +\n>>>> +                     +---------------------------------------+\n>>>> +                     |                  _ _                  |\n>>>> +                     |                 \\   /                 |\n>>>> +                     |                  | |                  |\n>>>> +                     |                  | |                  |\n>>>> +                     |                  |  >                 |\n>>>> +                     |                 <  |                  |\n>>>> +                     |                  | |                  |\n>>>> +                     |                    .                  |\n>>>> +                     |                   V                   |\n>>>> +                     +---------------------------------------+\n>>>> +\n>>>> +        A correction of 90 degrees in counterclockwise direction has to be\n\nAnd again\n\n\n>>>> +        applied to correctly display the image in portrait mode on the device\n>>>> +        screen.\n>>>> +\n>>>> +                                +-----------------+\n>>>> +                                |                 |\n>>>> +                                |                 |\n>>>> +                                |                 |\n>>>> +                                |                 |\n>>>> +                                |                 |\n>>>> +                                |                 |\n>>>> +                                |  |\\____)\\___    |\n>>>> +                                |  ) _____  __`<  |\n>>>> +                                |  |/     )/      |\n>>>> +                                |                 |\n>>>> +                                |                 |\n>>>> +                                |                 |\n>>>> +                                |                 |\n>>>> +                                |                 |\n>>>> +                                +-----------------+\n>>>\n>>> I had to think a bit to check if the sharks are in the right\n>>> orientation, and they are :-) I would however reduce the frame a little\n>>> bit to match the size of the references systems above.\n>>\n>> I received the opposed comments from Andrey and I agree with him that\n>> we should try to keep the image proportions similar to the ones above\n>> to avoid the idea of cropping. In my view, the coordinate system does\n>> not represent the size of the captured image, but I can work to make\n>> them look like it.\n>>\n>>>>  ...\n>>>\n>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AA94E60962\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 13 Feb 2020 22:46:25 +0100 (CET)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 06C14504;\n\tThu, 13 Feb 2020 22:46:24 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1581630385;\n\tbh=PabJeD9Gmkq+QXqJyoebXZDKLOhsshqmrJSzU1GUfps=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=u5AqivvqrRX0+0WeJk6Q3vMdQA7spxQCwnbnNzuj5YZBPZDPfVjcbUEY78IjXWF99\n\tu07uHn1zIM+pg8n9/TU+mbteaXfKH3RlWmcaPeX0EnFw3OC4TMeb4MccxgZa3M3TuY\n\toyabxelAuUdNE23OBmhOQEYQLBFD3cuBc6ShEjtk=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tjacopo mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20200113164245.52535-1-jacopo@jmondi.org>\n\t<20200113164245.52535-5-jacopo@jmondi.org>\n\t<20200203234249.GG4722@pendragon.ideasonboard.com>\n\t<20200205105105.243nbpreess656xj@vino>\n\t<20200213210150.GN29760@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<589c37c1-d474-07c8-34e1-e5fbcb2246f0@ideasonboard.com>","Date":"Thu, 13 Feb 2020 21:46:22 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200213210150.GN29760@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 04/23] libcamera: properties: Add\n\trotation property","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 13 Feb 2020 21:46:25 -0000"}}]