[{"id":27349,"web_url":"https://patchwork.libcamera.org/comment/27349/","msgid":"","date":"2023-06-15T07:41:28","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Adjust\n\tproperties::Rotation","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, 14 Jun 2023 at 12:00, Jacopo Mondi\n wrote:\n>\n> As the CameraSensor::validateTransform() function compensate\n> for the sensor's mounting rotation, the properties::Rotation value\n> should be adjusted to make sure application that receive already\n> \"corrected\" images do not get confused by Rotation still reporting\n> a value.\n>\n> However, as an image sensor can only compensate rotations by applying\n> H/V flips, only correct Rotation when the mounting rotation is 180\n> degrees.\n>\n> Signed-off-by: Jacopo Mondi \n\nThis behaviour seems reasonable to me.\n\nReviewed-by: Naushir Patuck \n\n> ---\n> src/libcamera/camera.cpp | 3 +--\n> src/libcamera/camera_sensor.cpp | 21 ++++++++++++++++++---\n> 2 files changed, 19 insertions(+), 5 deletions(-)\n>\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 99683e498697..3e252f3b8f8d 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -397,8 +397,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> *\n> * The transform is a user-specified 2D plane transform that will be applied\n> * to the camera images by the processing pipeline before being handed to\n> - * the application. This is subsequent to any transform that is already\n> - * required to fix up any platform-defined rotation.\n> + * the application.\n> *\n> * The usual 2D plane transforms are allowed here (horizontal/vertical\n> * flips, multiple of 90-degree rotations etc.), but the validate() function\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 60bf87b49e6e..f3a5aa37149f 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -461,7 +461,17 @@ int CameraSensor::initProperties()\n>\n> const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n> if (rotationControl != controls.end()) {\n> + /*\n> + * validateTransform() compensates for the mounting rotation.\n> + * However, as a camera sensor can only compensate rotations\n> + * by applying H/VFlips, only rotation of 180 degrees are\n> + * automatically compensated. The other valid rotations (Rot90\n> + * and Rot270) require transposition, which the camera sensor\n> + * cannot perform, so leave them untouched.\n> + */\n> propertyValue = rotationControl->second.def().get();\n> + if (propertyValue == 180 && supportFlips_)\n> + propertyValue = 0;\n> properties_.set(properties::Rotation, propertyValue);\n> }\n>\n> @@ -1028,10 +1038,15 @@ void CameraSensor::updateControlInfo()\n> */\n> Transform CameraSensor::validateTransform(Transform *transform) const\n> {\n> - /* Adjust the requested transform to compensate the sensor rotation. */\n> - int32_t rotation = properties().get(properties::Rotation).value_or(0);\n> - bool success;\n> + /* Adjust the requested transform to compensate the sensor mounting rotation. */\n> + const ControlInfoMap &controls = subdev_->controls();\n> + int rotation = 0;\n>\n> + const auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n> + if (rotationControl != controls.end())\n> + rotation = rotationControl->second.def().get();\n> +\n> + bool success;\n> Transform rotationTransform = transformFromRotation(rotation, &success);\n> if (!success)\n> LOG(CameraSensor, Warning) << \"Invalid rotation of \" << rotation\n> --\n> 2.40.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BB9AFBD78E\n\tfor ;\n\tThu, 15 Jun 2023 07:41:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 26342628B6;\n\tThu, 15 Jun 2023 09:41:46 +0200 (CEST)","from mail-yw1-x112f.google.com (mail-yw1-x112f.google.com\n\t[IPv6:2607:f8b0:4864:20::112f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 241CE61E45\n\tfor ;\n\tThu, 15 Jun 2023 09:41:44 +0200 (CEST)","by mail-yw1-x112f.google.com with SMTP id\n\t00721157ae682-56ffd7d7fedso17056547b3.2\n\tfor ;\n\tThu, 15 Jun 2023 00:41:44 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686814906;\n\tbh=x1458TkpAC52Fe0yq4utSWz3yS01DgEiF/PI7pNa+fM=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=cPn+lG+a4DJEmRLhfcOOL9OzQr2aiqrAK9FYzajB/R8ixmzNaq4qLmEkwk4Szy29+\n\tpGmDCUAEF23Xez5kYPyJ0R9VN0P5hhOOfcJx6/p5vdHgBGa0EpcokgiwGxBzVrxJfV\n\tIViGsjDrKKb6BBtvU1L69bRQjFDhfvhnZNL9ywbzQj3LxZlUEsyGRv2V+EpP5xNlrc\n\tPo7usMGGyINQdT4omhYyNZnJ6C0fcC1HBHP4uwAxbab5q75EyRv3/sBYXqcyz3xZMY\n\t68n3nQbyWkPHDLsZqYHW37g8jhbeSzIGtWUUv1yT3Fi38FKceUs2l0iWQlQqtUjYRP\n\tQMuWcgutmgbjw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1686814903; x=1689406903;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=WsilvpY2jvTJgsPTmAvj+EYbxnET6jILOWUlkyOfCtA=;\n\tb=k0+MxOzpu73y/ViCMETHk2UDRFO/NfnsZvb7vKra2LvzKQwlkDShMt5eCKJJpNej9M\n\thMMweesfwmM3/GaE+zfMmVwdIliJuEgJ4ncDWnjIJvlaE/20xbUaZvCzvTv591rGRNX5\n\ts3/LO6L3+pMhwiogmTX1BFu4YdpR68UVBuJLisNt8EAPBQqYBiLtDbCqoJVdrxeEVOz7\n\t5KoPxjlQzpIgfMGesmpP2sTi9m4GJb3AwZK5arKSLg+/PPljbAT8oy+VkRw1+GZxdPap\n\tLMbAlPxUJNj8e8gLX7QMKU4JfF7GqUwATLYWxHJUUIzyCSkr6b+S7aqEtaP21H59GmMV\n\t+/rA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"k0+MxOzp\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1686814903; x=1689406903;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=WsilvpY2jvTJgsPTmAvj+EYbxnET6jILOWUlkyOfCtA=;\n\tb=FxsrYtxbWk9SALVzK+I/0BAij9zIBYG28U1zzyiUrHwii/HQC4ESRgr4VYIZTUiVow\n\tcNKB8w3LKEnbiF9aaDQQSvUoerpV6aAOK/e9w2GUYvOXlMiyzcqXKI6xrddbjBRpWGeT\n\tjE4SVWl+0Ly8+ykQZK9vDFoccd2bMSRaQKmI6Mgpk1jFpKfyGGUiNqqRtFt+qlbUaj8H\n\tG6Wvi8YYAZKFj80f9A76El1pii3XbFuJyTOASnGcOhNzf43lAyTlDSpiMTwdJ91A48Ou\n\t/8WGPGCyzn2RcH7EC06s8TDX35ZIpzIJ0v3zbAJuSpQ6g/E9xIM2ukqrRoO6BuZVWiCt\n\takCw==","X-Gm-Message-State":"AC+VfDz8W+SXKHM3iUqOuzWur1+Q3DEf4esGPn/TMsVOVCImhHMjDtxo\n\t+ljw72d0ns4qMd6se2sX97uz4DJBUEU5VrjnEMPgLsArZzB4ojZgX90=","X-Google-Smtp-Source":"ACHHUZ4D9UD/7zNKEKVy8YApmDVkjG0Qbh1Mdyb8PzlnidSBtHYMm6EL6jR/Msw+FJDlE+ZFVKPrh3qxh450yeYOlt0=","X-Received":"by 2002:a81:6ac3:0:b0:56d:2e22:8b31 with SMTP id\n\tf186-20020a816ac3000000b0056d2e228b31mr4316109ywc.41.1686814902883;\n\tThu, 15 Jun 2023 00:41:42 -0700 (PDT)","MIME-Version":"1.0","References":"<20230614105957.15651-1-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230614105957.15651-1-jacopo.mondi@ideasonboard.com>","Date":"Thu, 15 Jun 2023 08:41:28 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Adjust\n\tproperties::Rotation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27351,"web_url":"https://patchwork.libcamera.org/comment/27351/","msgid":"<5aa576e7-1b56-049d-9c57-798bbd3675d7@ideasonboard.com>","date":"2023-06-15T08:52:53","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Adjust\n\tproperties::Rotation","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn 6/14/23 4:29 PM, Jacopo Mondi via libcamera-devel wrote:\n> As the CameraSensor::validateTransform() function compensate\n> for the sensor's mounting rotation, the properties::Rotation value\n> should be adjusted to make sure application that receive already\n> \"corrected\" images do not get confused by Rotation still reporting\n> a value.\n>\n> However, as an image sensor can only compensate rotations by applying\n> H/V flips, only correct Rotation when the mounting rotation is 180\n> degrees.\n>\n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Umang Jain \n\n> ---\n> src/libcamera/camera.cpp | 3 +--\n> src/libcamera/camera_sensor.cpp | 21 ++++++++++++++++++---\n> 2 files changed, 19 insertions(+), 5 deletions(-)\n>\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 99683e498697..3e252f3b8f8d 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -397,8 +397,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> *\n> * The transform is a user-specified 2D plane transform that will be applied\n> * to the camera images by the processing pipeline before being handed to\n> - * the application. This is subsequent to any transform that is already\n> - * required to fix up any platform-defined rotation.\n> + * the application.\n> *\n> * The usual 2D plane transforms are allowed here (horizontal/vertical\n> * flips, multiple of 90-degree rotations etc.), but the validate() function\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 60bf87b49e6e..f3a5aa37149f 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -461,7 +461,17 @@ int CameraSensor::initProperties()\n>\n> \tconst auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n> \tif (rotationControl != controls.end()) {\n> +\t\t/*\n> +\t\t * validateTransform() compensates for the mounting rotation.\n> +\t\t * However, as a camera sensor can only compensate rotations\n> +\t\t * by applying H/VFlips, only rotation of 180 degrees are\n> +\t\t * automatically compensated. The other valid rotations (Rot90\n> +\t\t * and Rot270) require transposition, which the camera sensor\n> +\t\t * cannot perform, so leave them untouched.\n> +\t\t */\n> \t\tpropertyValue = rotationControl->second.def().get();\n> +\t\tif (propertyValue == 180 && supportFlips_)\n> +\t\t\tpropertyValue = 0;\n> \t\tproperties_.set(properties::Rotation, propertyValue);\n> \t}\n>\n> @@ -1028,10 +1038,15 @@ void CameraSensor::updateControlInfo()\n> */\n> Transform CameraSensor::validateTransform(Transform *transform) const\n> {\n> -\t/* Adjust the requested transform to compensate the sensor rotation. */\n> -\tint32_t rotation = properties().get(properties::Rotation).value_or(0);\n> -\tbool success;\n> +\t/* Adjust the requested transform to compensate the sensor mounting rotation. */\n> +\tconst ControlInfoMap &controls = subdev_->controls();\n> +\tint rotation = 0;\n>\n> +\tconst auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n> +\tif (rotationControl != controls.end())\n> +\t\trotation = rotationControl->second.def().get();\n> +\n> +\tbool success;\n> \tTransform rotationTransform = transformFromRotation(rotation, &success);\n> \tif (!success)\n> \t\tLOG(CameraSensor, Warning) << \"Invalid rotation of \" << rotation\n> --\n> 2.40.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 49EAEBD78E\n\tfor ;\n\tThu, 15 Jun 2023 08:53:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A1741628B6;\n\tThu, 15 Jun 2023 10:53:00 +0200 (CEST)","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 33DF061E45\n\tfor ;\n\tThu, 15 Jun 2023 10:52:59 +0200 (CEST)","from [192.168.1.108] (unknown [103.86.18.143])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 45B819CA;\n\tThu, 15 Jun 2023 10:52:27 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686819180;\n\tbh=paL3R0xkpKnvyAdaKyieUd52z35dXkR1kPX9PgeIWqk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=egJrpVI8+529ZZubnO9o41tv8DJDnwofuZD5DnfwUecAAE62Sp6PIelG/NGRq7Fzk\n\tysF1S48L4Zg8oNOV2zvUwF32fD5MSd6wWNQw4h8RIDkc3PeLzfaJWOhksbSDdUSFIY\n\teQ6YWoiEnsEop9a2FFttNZUktpXn+geHxUTxnnjhgLNWDjvq8L0cgZfJIxxq/T4gSF\n\tBUzTefftvEmCGqHuAXKRBtg425EFikuoe2HsqB8ADtZOVjYByrFjMx5EgvwTygRLSe\n\tb4OHgfPazYexuzmZVJUct/nm7UWu1gF9VJ8Q9HuZOtTspdWvkPfjM+37CyGWk2bYG3\n\tyVT/CmnTbXNQQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686819147;\n\tbh=paL3R0xkpKnvyAdaKyieUd52z35dXkR1kPX9PgeIWqk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=u8hgPSIGS/kKvhD9G7lgU1kXG3dCMCysmJFs4cIErIxU92XZ39VQIDXxBdsiX20s6\n\tvOqK7GEqdlC1VtC9B02zpdJ56sDP6nHyUXM79bF2paemRghPurQ1kfSgVudXkYK1U/\n\t+FalCvUwKwZfCy5EdlYKdgwwODuPDdUAo+QMonOg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"u8hgPSIG\"; dkim-atps=neutral","Message-ID":"<5aa576e7-1b56-049d-9c57-798bbd3675d7@ideasonboard.com>","Date":"Thu, 15 Jun 2023 14:22:53 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.7.1","Content-Language":"en-US","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20230614105957.15651-1-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230614105957.15651-1-jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Adjust\n\tproperties::Rotation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27393,"web_url":"https://patchwork.libcamera.org/comment/27393/","msgid":"<20230619120051.GA31255@pendragon.ideasonboard.com>","date":"2023-06-19T12:00:51","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Adjust\n\tproperties::Rotation","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\nOn Wed, Jun 14, 2023 at 12:59:57PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> As the CameraSensor::validateTransform() function compensate\n> for the sensor's mounting rotation, the properties::Rotation value\n> should be adjusted to make sure application that receive already\n> \"corrected\" images do not get confused by Rotation still reporting\n> a value.\n> \n> However, as an image sensor can only compensate rotations by applying\n> H/V flips, only correct Rotation when the mounting rotation is 180\n> degrees.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/camera.cpp | 3 +--\n> src/libcamera/camera_sensor.cpp | 21 ++++++++++++++++++---\n> 2 files changed, 19 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 99683e498697..3e252f3b8f8d 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -397,8 +397,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> *\n> * The transform is a user-specified 2D plane transform that will be applied\n> * to the camera images by the processing pipeline before being handed to\n> - * the application. This is subsequent to any transform that is already\n> - * required to fix up any platform-defined rotation.\n> + * the application.\n> *\n> * The usual 2D plane transforms are allowed here (horizontal/vertical\n> * flips, multiple of 90-degree rotations etc.), but the validate() function\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 60bf87b49e6e..f3a5aa37149f 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -461,7 +461,17 @@ int CameraSensor::initProperties()\n> \n> \tconst auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n> \tif (rotationControl != controls.end()) {\n> +\t\t/*\n> +\t\t * validateTransform() compensates for the mounting rotation.\n> +\t\t * However, as a camera sensor can only compensate rotations\n> +\t\t * by applying H/VFlips, only rotation of 180 degrees are\n> +\t\t * automatically compensated. The other valid rotations (Rot90\n> +\t\t * and Rot270) require transposition, which the camera sensor\n> +\t\t * cannot perform, so leave them untouched.\n> +\t\t */\n> \t\tpropertyValue = rotationControl->second.def().get();\n> +\t\tif (propertyValue == 180 && supportFlips_)\n> +\t\t\tpropertyValue = 0;\n> \t\tproperties_.set(properties::Rotation, propertyValue);\n> \t}\n> \n> @@ -1028,10 +1038,15 @@ void CameraSensor::updateControlInfo()\n> */\n> Transform CameraSensor::validateTransform(Transform *transform) const\n> {\n> -\t/* Adjust the requested transform to compensate the sensor rotation. */\n> -\tint32_t rotation = properties().get(properties::Rotation).value_or(0);\n> -\tbool success;\n> +\t/* Adjust the requested transform to compensate the sensor mounting rotation. */\n> +\tconst ControlInfoMap &controls = subdev_->controls();\n> +\tint rotation = 0;\n> \n> +\tconst auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n\nHow about caching the rotation value in the CameraSensor class instead\nof looking it up every time ? You could actually even cache the value of\nrotationTransform, simplifying the code in this function.\n\n> +\tif (rotationControl != controls.end())\n> +\t\trotation = rotationControl->second.def().get();\n> +\n> +\tbool success;\n> \tTransform rotationTransform = transformFromRotation(rotation, &success);\n> \tif (!success)\n> \t\tLOG(CameraSensor, Warning) << \"Invalid rotation of \" << rotation","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D2E05BD78E\n\tfor ;\n\tMon, 19 Jun 2023 12:00:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84C65628B1;\n\tMon, 19 Jun 2023 14:00:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 35BC5628B1\n\tfor ;\n\tMon, 19 Jun 2023 14:00:51 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 14755800;\n\tMon, 19 Jun 2023 14:00:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687176052;\n\tbh=OdW74ydbLPILFfzC9Iqse5nzuxNThJ6WFBreqCwHqX0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=saMm3cmT86A0peO/Tw0ETSe2Xwt87Z7QsD8khKkGUkXStjjuItYInVoGbXgv8KyZ+\n\tq+fqUbqczCFQ7EoZfjdmMg4JrVyKM2t/WP52MP0khHBUQfXOjZ5vzrlZ10+6e0RIg0\n\t93tbXN8seW++sKFq7BAB8zk0jdiaGxaf9nUwON0a66dyIYteoF/ax7PD5jFxeJX9mz\n\tFkC4pHmDr5Vc8GFD707ldcU5mo/clCc3uZIB7kM/PvmOIPkssd/UF3YbnrwKOaX614\n\tknGUh+j1JW+VUsIFkiTmayw7vqSAEJWd7WruKelMADaHL6Adpnxoa8HJLPvIQHcMXC\n\tpsZUthVEte/zw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1687176017;\n\tbh=OdW74ydbLPILFfzC9Iqse5nzuxNThJ6WFBreqCwHqX0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AKlgjHEmwN9rPp3TgpK+twUirgU2CctI/T43H8XHN8B9pxRVZsWIEMk6wlI2vpIHt\n\tQS+sBnPYQI9+v/plQSdVyTBkUcjGU9F+JavH5IfFzaoimi6J2QQmYqJ+b3qIAwJtxl\n\t47L00h/lRNYnaZ7mAzQgr4vyg7kwbfu+sSmfxbsU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AKlgjHEm\"; dkim-atps=neutral","Date":"Mon, 19 Jun 2023 15:00:51 +0300","To":"Jacopo Mondi ","Message-ID":"<20230619120051.GA31255@pendragon.ideasonboard.com>","References":"<20230614105957.15651-1-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230614105957.15651-1-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Adjust\n\tproperties::Rotation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27394,"web_url":"https://patchwork.libcamera.org/comment/27394/","msgid":"","date":"2023-06-19T12:20:36","subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Adjust\n\tproperties::Rotation","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\n on top eventually as the patch has been pushed (by me) :)\n\nOn Mon, Jun 19, 2023 at 03:00:51PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Wed, Jun 14, 2023 at 12:59:57PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> > As the CameraSensor::validateTransform() function compensate\n> > for the sensor's mounting rotation, the properties::Rotation value\n> > should be adjusted to make sure application that receive already\n> > \"corrected\" images do not get confused by Rotation still reporting\n> > a value.\n> >\n> > However, as an image sensor can only compensate rotations by applying\n> > H/V flips, only correct Rotation when the mounting rotation is 180\n> > degrees.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/camera.cpp | 3 +--\n> > src/libcamera/camera_sensor.cpp | 21 ++++++++++++++++++---\n> > 2 files changed, 19 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 99683e498697..3e252f3b8f8d 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -397,8 +397,7 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> > *\n> > * The transform is a user-specified 2D plane transform that will be applied\n> > * to the camera images by the processing pipeline before being handed to\n> > - * the application. This is subsequent to any transform that is already\n> > - * required to fix up any platform-defined rotation.\n> > + * the application.\n> > *\n> > * The usual 2D plane transforms are allowed here (horizontal/vertical\n> > * flips, multiple of 90-degree rotations etc.), but the validate() function\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 60bf87b49e6e..f3a5aa37149f 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -461,7 +461,17 @@ int CameraSensor::initProperties()\n> >\n> > \tconst auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n> > \tif (rotationControl != controls.end()) {\n> > +\t\t/*\n> > +\t\t * validateTransform() compensates for the mounting rotation.\n> > +\t\t * However, as a camera sensor can only compensate rotations\n> > +\t\t * by applying H/VFlips, only rotation of 180 degrees are\n> > +\t\t * automatically compensated. The other valid rotations (Rot90\n> > +\t\t * and Rot270) require transposition, which the camera sensor\n> > +\t\t * cannot perform, so leave them untouched.\n> > +\t\t */\n> > \t\tpropertyValue = rotationControl->second.def().get();\n> > +\t\tif (propertyValue == 180 && supportFlips_)\n> > +\t\t\tpropertyValue = 0;\n> > \t\tproperties_.set(properties::Rotation, propertyValue);\n> > \t}\n> >\n> > @@ -1028,10 +1038,15 @@ void CameraSensor::updateControlInfo()\n> > */\n> > Transform CameraSensor::validateTransform(Transform *transform) const\n> > {\n> > -\t/* Adjust the requested transform to compensate the sensor rotation. */\n> > -\tint32_t rotation = properties().get(properties::Rotation).value_or(0);\n> > -\tbool success;\n> > +\t/* Adjust the requested transform to compensate the sensor mounting rotation. */\n> > +\tconst ControlInfoMap &controls = subdev_->controls();\n> > +\tint rotation = 0;\n> >\n> > +\tconst auto &rotationControl = controls.find(V4L2_CID_CAMERA_SENSOR_ROTATION);\n>\n> How about caching the rotation value in the CameraSensor class instead\n> of looking it up every time ? You could actually even cache the value of\n> rotationTransform, simplifying the code in this function.\n>\n> > +\tif (rotationControl != controls.end())\n> > +\t\trotation = rotationControl->second.def().get();\n> > +\n> > +\tbool success;\n> > \tTransform rotationTransform = transformFromRotation(rotation, &success);\n> > \tif (!success)\n> > \t\tLOG(CameraSensor, Warning) << \"Invalid rotation of \" << rotation\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 116EBC3237\n\tfor ;\n\tMon, 19 Jun 2023 12:20:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6FAF0628AC;\n\tMon, 19 Jun 2023 14:20:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1CF34614FD\n\tfor ;\n\tMon, 19 Jun 2023 14:20:40 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:72c3:346:a663:c82d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E6E96547;\n\tMon, 19 Jun 2023 14:20:05 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687177241;\n\tbh=hCchOH0RN9jdqJouR+cXnUD6zPjydcPDedtoopVpOv0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=fFx4EzAD0oCMfxL2a3ZorWn9UV4QakE4GLg8Nn0NxA9LQyR/XbB9rCqfYG2LFmci8\n\tKXdJP0FZyYMgoPeqLUFPyezKq+w+CVuc/DXmQzm55I421Cymnc29gOxQmD9XKleGrB\n\tMucZxLPahT5XG8RGkfkWPcbSDyxy/E/QwgSn/a6YbuIp62JvuqpFSjlEcJyyFIFCCE\n\tG7ZieJSeNYPJTHTFGPtq/79DBSgTRam9N00DJ0PqhkMrkHvP9yL9yRAOz3w9IdCdpO\n\trrIecD6uCHF3hIhFX2A/IWe9zOBV1Z3pg7wScc8elQWq3mKZAxyjliW0cSVsyM1F3S\n\t4kLd/q9fivWqw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1687177206;\n\tbh=hCchOH0RN9jdqJouR+cXnUD6zPjydcPDedtoopVpOv0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ThFkymISWpmFs4rB+DtrnlS9fa7e2V5rv0ET6aolBIGTRtcX+Kfvq5m7shNe7hZph\n\tZncgWmGfbsREp5KYhJu13TlKVD19xVS9PeKjtrnbs6KrSac0v2Um9TfaU9bkLqW4Gi\n\t4Xyzn3jZpGNS1fv6yXUx09FUxazZgNWO07u62sJ0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ThFkymIS\"; dkim-atps=neutral","Date":"Mon, 19 Jun 2023 14:20:36 +0200","To":"Laurent Pinchart ","Message-ID":"","References":"<20230614105957.15651-1-jacopo.mondi@ideasonboard.com>\n\t<20230619120051.GA31255@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230619120051.GA31255@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: camera_sensor: Adjust\n\tproperties::Rotation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]