[{"id":30030,"web_url":"https://patchwork.libcamera.org/comment/30030/","msgid":"<nr2e35i4kj62pashgbrfbnxuxxe2vvo3u3oa5hu2ge6kmy3mos@wf3ytwkln7bs>","date":"2024-06-21T08:27:23","subject":"Re: [PATCH 1/2] qcam: rotate the viewfinder output as per camera\n\tproperties","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Joel\n\nOn Thu, Jun 20, 2024 at 04:36:06PM GMT, Joel Selvaraj wrote:\n> Devicetrees may specify the rotation at which the camera is\n> mounted in the hardware. If available, this gets populated in camera\n> properties. Rotate the viewfinder output accordingly in qcam qt and\n> opengl renderers.\n>\n> Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>\n> ---\n>  include/libcamera/orientation.h |  2 ++\n>  src/apps/qcam/main_window.cpp   |  9 ++++++-\n>  src/apps/qcam/viewfinder.h      |  4 ++-\n>  src/apps/qcam/viewfinder_gl.cpp | 46 ++++++++++++++++++++++++++++++---\n>  src/apps/qcam/viewfinder_gl.h   |  4 ++-\n>  src/apps/qcam/viewfinder_qt.cpp | 18 +++++++++++--\n>  src/apps/qcam/viewfinder_qt.h   |  5 +++-\n>  src/libcamera/orientation.cpp   | 34 ++++++++++++++++++++++++\n>  8 files changed, 113 insertions(+), 9 deletions(-)\n>\n> diff --git a/include/libcamera/orientation.h b/include/libcamera/orientation.h\n> index a3b40e63..e32f5eb8 100644\n> --- a/include/libcamera/orientation.h\n> +++ b/include/libcamera/orientation.h\n> @@ -25,6 +25,8 @@ enum class Orientation {\n>\n>  Orientation orientationFromRotation(int angle, bool *success = nullptr);\n>\n> +int rotationFromOrientation(const Orientation &orientation, bool *success = nullptr);\n> +\n>  std::ostream &operator<<(std::ostream &out, const Orientation &orientation);\n>\n>  } /* namespace libcamera */\n> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\n> index d515beed..18c94cf3 100644\n> --- a/src/apps/qcam/main_window.cpp\n> +++ b/src/apps/qcam/main_window.cpp\n> @@ -363,6 +363,7 @@ void MainWindow::toggleCapture(bool start)\n>  int MainWindow::startCapture()\n>  {\n>  \tstd::vector<StreamRole> roles = StreamKeyValueParser::roles(options_[OptStream]);\n> +\tOrientation orientation = Orientation::Rotate0;\n>  \tint ret;\n>\n>  \t/* Verify roles are supported. */\n> @@ -392,6 +393,12 @@ int MainWindow::startCapture()\n>  \t\treturn -EINVAL;\n>  \t}\n>\n> +\t/* Get orientation provided by camera kernel driver */\n> +\tconst ControlList &properties = camera_->properties();\n> +\tconst auto &rotation = properties.get(properties::Rotation);\n> +\tif (rotation)\n> +\t\torientation = orientationFromRotation(*rotation);\n> +\n>  \tStreamConfiguration &vfConfig = config_->at(0);\n>\n>  \t/* Use a format supported by the viewfinder if available. */\n> @@ -444,7 +451,7 @@ int MainWindow::startCapture()\n>  \tret = viewfinder_->setFormat(vfConfig.pixelFormat,\n>  \t\t\t\t     QSize(vfConfig.size.width, vfConfig.size.height),\n>  \t\t\t\t     vfConfig.colorSpace.value_or(ColorSpace::Sycc),\n> -\t\t\t\t     vfConfig.stride);\n> +\t\t\t\t     vfConfig.stride, orientation);\n>  \tif (ret < 0) {\n>  \t\tqInfo() << \"Failed to set viewfinder format\";\n>  \t\treturn ret;\n> diff --git a/src/apps/qcam/viewfinder.h b/src/apps/qcam/viewfinder.h\n> index 914f88ec..f17cc4a1 100644\n> --- a/src/apps/qcam/viewfinder.h\n> +++ b/src/apps/qcam/viewfinder.h\n> @@ -14,6 +14,7 @@\n>  #include <libcamera/color_space.h>\n>  #include <libcamera/formats.h>\n>  #include <libcamera/framebuffer.h>\n> +#include <libcamera/orientation.h>\n>\n>  class Image;\n>\n> @@ -26,7 +27,8 @@ public:\n>\n>  \tvirtual int setFormat(const libcamera::PixelFormat &format, const QSize &size,\n>  \t\t\t      const libcamera::ColorSpace &colorSpace,\n> -\t\t\t      unsigned int stride) = 0;\n> +\t\t\t      unsigned int stride,\n> +\t\t\t      const libcamera::Orientation &orientation) = 0;\n>  \tvirtual void render(libcamera::FrameBuffer *buffer, Image *image) = 0;\n>  \tvirtual void stop() = 0;\n>\n> diff --git a/src/apps/qcam/viewfinder_gl.cpp b/src/apps/qcam/viewfinder_gl.cpp\n> index 9d2a6960..c4b10e1e 100644\n> --- a/src/apps/qcam/viewfinder_gl.cpp\n> +++ b/src/apps/qcam/viewfinder_gl.cpp\n> @@ -77,7 +77,7 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const\n>\n>  int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, const QSize &size,\n>  \t\t\t    const libcamera::ColorSpace &colorSpace,\n> -\t\t\t    unsigned int stride)\n> +\t\t\t    unsigned int stride, const libcamera::Orientation &orientation)\n>  {\n>  \tif (format != format_ || colorSpace != colorSpace_) {\n>  \t\t/*\n> @@ -101,6 +101,7 @@ int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, const QSize &s\n>\n>  \tsize_ = size;\n>  \tstride_ = stride;\n> +\torientation_ = orientation;\n>\n>  \tupdateGeometry();\n>  \treturn 0;\n> @@ -511,7 +512,7 @@ void ViewFinderGL::initializeGL()\n>  \tglEnable(GL_TEXTURE_2D);\n>  \tglDisable(GL_DEPTH_TEST);\n>\n> -\tstatic const GLfloat coordinates[2][4][2]{\n> +\tGLfloat coordinates[2][4][2]{\n>  \t\t{\n>  \t\t\t/* Vertex coordinates */\n>  \t\t\t{ -1.0f, -1.0f },\n> @@ -528,6 +529,41 @@ void ViewFinderGL::initializeGL()\n>  \t\t},\n>  \t};\n>\n> +\tswitch (orientation_) {\n> +\tcase libcamera::Orientation::Rotate90:\n> +\t\tcoordinates[0][0][0] = -1.0f;\n> +\t\tcoordinates[0][0][1] = +1.0f;\n> +\t\tcoordinates[0][1][0] = +1.0f;\n> +\t\tcoordinates[0][1][1] = +1.0f;\n> +\t\tcoordinates[0][2][0] = +1.0f;\n> +\t\tcoordinates[0][2][1] = -1.0f;\n> +\t\tcoordinates[0][3][0] = -1.0f;\n> +\t\tcoordinates[0][3][1] = -1.0f;\n> +\t\tbreak;\n> +\tcase libcamera::Orientation::Rotate180:\n> +\t\tcoordinates[0][0][0] = +1.0f;\n> +\t\tcoordinates[0][0][1] = +1.0f;\n> +\t\tcoordinates[0][1][0] = +1.0f;\n> +\t\tcoordinates[0][1][1] = -1.0f;\n> +\t\tcoordinates[0][2][0] = -1.0f;\n> +\t\tcoordinates[0][2][1] = -1.0f;\n> +\t\tcoordinates[0][3][0] = -1.0f;\n> +\t\tcoordinates[0][3][1] = +1.0f;\n> +\t\tbreak;\n> +\tcase libcamera::Orientation::Rotate270:\n> +\t\tcoordinates[0][0][0] = +1.0f;\n> +\t\tcoordinates[0][0][1] = -1.0f;\n> +\t\tcoordinates[0][1][0] = -1.0f;\n> +\t\tcoordinates[0][1][1] = -1.0f;\n> +\t\tcoordinates[0][2][0] = -1.0f;\n> +\t\tcoordinates[0][2][1] = +1.0f;\n> +\t\tcoordinates[0][3][0] = +1.0f;\n> +\t\tcoordinates[0][3][1] = +1.0f;\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tbreak;\n> +\t}\n> +\n>  \tvertexBuffer_.create();\n>  \tvertexBuffer_.bind();\n>  \tvertexBuffer_.allocate(coordinates, sizeof(coordinates));\n> @@ -831,5 +867,9 @@ void ViewFinderGL::resizeGL(int w, int h)\n>\n>  QSize ViewFinderGL::sizeHint() const\n>  {\n> -\treturn size_.isValid() ? size_ : QSize(640, 480);\n> +\tif (orientation_ == libcamera::Orientation::Rotate90 ||\n> +\t    orientation_ == libcamera::Orientation::Rotate270)\n> +\t\treturn size_.isValid() ? size_.transposed() : QSize(480, 640);\n> +\telse\n> +\t\treturn size_.isValid() ? size_ : QSize(640, 480);\n>  }\n> diff --git a/src/apps/qcam/viewfinder_gl.h b/src/apps/qcam/viewfinder_gl.h\n> index 23744b41..1fc0f827 100644\n> --- a/src/apps/qcam/viewfinder_gl.h\n> +++ b/src/apps/qcam/viewfinder_gl.h\n> @@ -39,7 +39,8 @@ public:\n>\n>  \tint setFormat(const libcamera::PixelFormat &format, const QSize &size,\n>  \t\t      const libcamera::ColorSpace &colorSpace,\n> -\t\t      unsigned int stride) override;\n> +\t\t      unsigned int stride,\n> +\t\t      const libcamera::Orientation &orientation) override;\n>  \tvoid render(libcamera::FrameBuffer *buffer, Image *image) override;\n>  \tvoid stop() override;\n>\n> @@ -68,6 +69,7 @@ private:\n>  \tlibcamera::FrameBuffer *buffer_;\n>  \tlibcamera::PixelFormat format_;\n>  \tlibcamera::ColorSpace colorSpace_;\n> +\tlibcamera::Orientation orientation_;\n>  \tQSize size_;\n>  \tunsigned int stride_;\n>  \tImage *image_;\n> diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp\n> index 4821c27d..583a74f7 100644\n> --- a/src/apps/qcam/viewfinder_qt.cpp\n> +++ b/src/apps/qcam/viewfinder_qt.cpp\n> @@ -57,7 +57,7 @@ const QList<libcamera::PixelFormat> &ViewFinderQt::nativeFormats() const\n>\n>  int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, const QSize &size,\n>  \t\t\t    [[maybe_unused]] const libcamera::ColorSpace &colorSpace,\n> -\t\t\t    unsigned int stride)\n> +\t\t\t    unsigned int stride, const libcamera::Orientation &orientation)\n>  {\n>  \timage_ = QImage();\n>\n> @@ -80,6 +80,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, const QSize &s\n>\n>  \tformat_ = format;\n>  \tsize_ = size;\n> +\torientation_ = orientation;\n> +\n> +\tbool success;\n> +\tint angle = libcamera::rotationFromOrientation(orientation, &success);\n> +\tif (!success) {\n> +\t\tqWarning() << \"Unsupported orientation\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\ttransform_ = QTransform().rotate(angle);\n>\n>  \tupdateGeometry();\n>  \treturn 0;\n> @@ -148,6 +157,7 @@ void ViewFinderQt::paintEvent(QPaintEvent *)\n>\n>  \t/* If we have an image, draw it. */\n>  \tif (!image_.isNull()) {\n> +\t\timage_ = image_.transformed(transform_);\n>  \t\tpainter.drawImage(rect(), image_, image_.rect());\n>  \t\treturn;\n>  \t}\n> @@ -179,5 +189,9 @@ void ViewFinderQt::paintEvent(QPaintEvent *)\n>\n>  QSize ViewFinderQt::sizeHint() const\n>  {\n> -\treturn size_.isValid() ? size_ : QSize(640, 480);\n> +\tif (orientation_ == libcamera::Orientation::Rotate90 ||\n> +\t    orientation_ == libcamera::Orientation::Rotate270)\n> +\t\treturn size_.isValid() ? size_.transposed() : QSize(480, 640);\n> +\telse\n> +\t\treturn size_.isValid() ? size_ : QSize(640, 480);\n>  }\n> diff --git a/src/apps/qcam/viewfinder_qt.h b/src/apps/qcam/viewfinder_qt.h\n> index 4f4b9f11..309b39e5 100644\n> --- a/src/apps/qcam/viewfinder_qt.h\n> +++ b/src/apps/qcam/viewfinder_qt.h\n> @@ -33,7 +33,8 @@ public:\n>\n>  \tint setFormat(const libcamera::PixelFormat &format, const QSize &size,\n>  \t\t      const libcamera::ColorSpace &colorSpace,\n> -\t\t      unsigned int stride) override;\n> +\t\t      unsigned int stride,\n> +\t\t      const libcamera::Orientation &orientation) override;\n>  \tvoid render(libcamera::FrameBuffer *buffer, Image *image) override;\n>  \tvoid stop() override;\n>\n> @@ -51,6 +52,8 @@ private:\n>\n>  \tlibcamera::PixelFormat format_;\n>  \tQSize size_;\n> +\tQTransform transform_;\n> +\tlibcamera::Orientation orientation_;\n>\n>  \t/* Camera stopped icon */\n>  \tQSize vfSize_;\n> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp\n> index 47fd6a32..7afb37a8 100644\n> --- a/src/libcamera/orientation.cpp\n> +++ b/src/libcamera/orientation.cpp\n> @@ -92,6 +92,40 @@ Orientation orientationFromRotation(int angle, bool *success)\n>  \treturn Orientation::Rotate0;\n>  }\n>\n> +/**\n> + * \\brief Return the rotation angle for a given orientation\n\nI'm a bit unconfortable about having this in the API, as it might\nresult in something more complex than what it actually looks like.\n\nAll the 2d plane transforms expressed by Orientation include\nan optional horizontal flip, something that cannot be expressed with\nan angular rotation only.\n\nLook at the example images we have in the Orientation class\ndocumentation. If you only report the rotation angle (*)\nyou won't be able to identify if an image has been flipped or not, and\nthe result might not be what you expect.\n\n(*) Also expressing the rotation angle only is less trivial than what\none might expect. First of all, if you want to return a rotation\nangle, you have to specify what the rotation describes:\n\n1) The image rotation in the memory buffers\n2) The correction to be applied to display the image in its natural\norientation\n\nThe Orientation class describes the image rotation in memory buffers,\nso I presume that's what you're expressing here, but it has to be made\nexplicit in the documentation of the function. The direction of the\nrotation has to be specified as well. All of this is documented in the\nOrientation class, but if this function is made part of the API, it\nhas to be specified here as well.\n\nI would suggest to infer the rotation angle from Orientation in the\napplication and explicitly say that mirroring is not corrected there.\n\nThanks\n  j\n\n> + * \\param[in] orientation The orientation to convert to a rotation angle\n> + * \\param[out] success Set to `true` if the given orientation is valid,\n> + * otherwise `false`\n> + * \\return The rotation angle corresponding to the given orientation\n> + * if \\a success was set to `true`, otherwise 0.\n> + */\n> +int rotationFromOrientation(const Orientation &orientation, bool *success)\n> +{\n> +\tif (success != nullptr)\n> +\t\t*success = true;\n> +\n> +\tswitch (orientation) {\n> +\tcase Orientation::Rotate0:\n> +\tcase Orientation::Rotate0Mirror:\n> +\t\treturn 0;\n> +\tcase Orientation::Rotate90:\n> +\tcase Orientation::Rotate90Mirror:\n> +\t\treturn 90;\n> +\tcase Orientation::Rotate180:\n> +\tcase Orientation::Rotate180Mirror:\n> +\t\treturn 180;\n> +\tcase Orientation::Rotate270:\n> +\tcase Orientation::Rotate270Mirror:\n> +\t\treturn 270;\n> +\t}\n> +\n> +\tif (success != nullptr)\n> +\t\t*success = false;\n> +\n> +\treturn 0;\n> +}\n> +\n>  /**\n>   * \\brief Prints human-friendly names for Orientation items\n>   * \\param[in] out The output stream\n> --\n> 2.45.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 46070BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 21 Jun 2024 08:27:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 158F9654A8;\n\tFri, 21 Jun 2024 10:27:28 +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 5938C619F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Jun 2024 10:27:26 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7E14B524;\n\tFri, 21 Jun 2024 10:27:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cIlWV2tj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718958426;\n\tbh=jo4p4AgGyMyDmCBuWePJqRFEaealqcr6QgNKs9p6djU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cIlWV2tjc4RPlMS+afjLjqvuYAXEwRvZ0HgFNUtKq6BmANFWkhamN5wkAAXq+Wj92\n\tPk9GMjkmCHZG0Nscd74qmWNarV1dQG3tA11SGjcVM5jJR79KBmzkOP1PPQFYiEHK+Y\n\tENsjvjLP8XhQEeNQM59PMPw+Pn+5vvoyg340VMZQ=","Date":"Fri, 21 Jun 2024 10:27:23 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Joel Selvaraj <joelselvaraj.oss@gmail.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] qcam: rotate the viewfinder output as per camera\n\tproperties","Message-ID":"<nr2e35i4kj62pashgbrfbnxuxxe2vvo3u3oa5hu2ge6kmy3mos@wf3ytwkln7bs>","References":"<20240620213607.32583-1-joelselvaraj.oss@gmail.com>\n\t<20240620213607.32583-2-joelselvaraj.oss@gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240620213607.32583-2-joelselvaraj.oss@gmail.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30032,"web_url":"https://patchwork.libcamera.org/comment/30032/","msgid":"<478b62b7-5c32-4953-84d4-d2a2118ed988@gmail.com>","date":"2024-06-21T12:04:46","subject":"Re: [PATCH 1/2] qcam: rotate the viewfinder output as per camera\n\tproperties","submitter":{"id":201,"url":"https://patchwork.libcamera.org/api/people/201/","name":"Joel Selvaraj","email":"joelselvaraj.oss@gmail.com"},"content":"Hi Jacopo Mondi,\n\nOn 6/21/24 03:27, Jacopo Mondi wrote:\n>> diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp\n>> index 47fd6a32..7afb37a8 100644\n>> --- a/src/libcamera/orientation.cpp\n>> +++ b/src/libcamera/orientation.cpp\n>> @@ -92,6 +92,40 @@ Orientation orientationFromRotation(int angle, bool *success)\n>>   \treturn Orientation::Rotate0;\n>>   }\n>>\n>> +/**\n>> + * \\brief Return the rotation angle for a given orientation\n> \n> I'm a bit unconfortable about having this in the API, as it might\n> result in something more complex than what it actually looks like.\n> \n> All the 2d plane transforms expressed by Orientation include\n> an optional horizontal flip, something that cannot be expressed with\n> an angular rotation only.\n> \n> Look at the example images we have in the Orientation class\n> documentation. If you only report the rotation angle (*)\n> you won't be able to identify if an image has been flipped or not, and\n> the result might not be what you expect.\n> \n> (*) Also expressing the rotation angle only is less trivial than what\n> one might expect. First of all, if you want to return a rotation\n> angle, you have to specify what the rotation describes:\n> \n> 1) The image rotation in the memory buffers\n> 2) The correction to be applied to display the image in its natural\n> orientation\n> \n> The Orientation class describes the image rotation in memory buffers,\n> so I presume that's what you're expressing here, but it has to be made\n> explicit in the documentation of the function. The direction of the\n> rotation has to be specified as well. All of this is documented in the\n> Orientation class, but if this function is made part of the API, it\n> has to be specified here as well.\n> \n> I would suggest to infer the rotation angle from Orientation in the\n> application and explicitly say that mirroring is not corrected there. \n\nRight... I was a bit hesitant when I worked on this API. I will remove \nthis and infer the angle in the application directly in v2.\n\nRegards,\nJoel Selvaraj\n>    j\n> \n>> + * \\param[in] orientation The orientation to convert to a rotation angle\n>> + * \\param[out] success Set to `true` if the given orientation is valid,\n>> + * otherwise `false`\n>> + * \\return The rotation angle corresponding to the given orientation\n>> + * if \\a success was set to `true`, otherwise 0.\n>> + */\n>> +int rotationFromOrientation(const Orientation &orientation, bool *success)\n>> +{\n>> +\tif (success != nullptr)\n>> +\t\t*success = true;\n>> +\n>> +\tswitch (orientation) {\n>> +\tcase Orientation::Rotate0:\n>> +\tcase Orientation::Rotate0Mirror:\n>> +\t\treturn 0;\n>> +\tcase Orientation::Rotate90:\n>> +\tcase Orientation::Rotate90Mirror:\n>> +\t\treturn 90;\n>> +\tcase Orientation::Rotate180:\n>> +\tcase Orientation::Rotate180Mirror:\n>> +\t\treturn 180;\n>> +\tcase Orientation::Rotate270:\n>> +\tcase Orientation::Rotate270Mirror:\n>> +\t\treturn 270;\n>> +\t}\n>> +\n>> +\tif (success != nullptr)\n>> +\t\t*success = false;\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>>   /**\n>>    * \\brief Prints human-friendly names for Orientation items\n>>    * \\param[in] out The output stream\n>> --\n>> 2.45.2\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 09BCBBD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 21 Jun 2024 12:04:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A4F46654A9;\n\tFri, 21 Jun 2024 14:04:50 +0200 (CEST)","from mail-ot1-x334.google.com (mail-ot1-x334.google.com\n\t[IPv6:2607:f8b0:4864:20::334])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 576CC654A2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Jun 2024 14:04:49 +0200 (CEST)","by mail-ot1-x334.google.com with SMTP id\n\t46e09a7af769-7009a3976ebso38017a34.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Jun 2024 05:04:49 -0700 (PDT)","from [192.168.0.98] ([67.6.32.220]) by smtp.gmail.com with ESMTPSA\n\tid 586e51a60fabf-25cd4987fa4sm372207fac.15.2024.06.21.05.04.46\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tFri, 21 Jun 2024 05:04:47 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"QxWYsMY5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20230601; t=1718971488; x=1719576288;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=ZnQlrt5etPp1vNBnBcUs14y1PdBA821vTuZpJecBucE=;\n\tb=QxWYsMY5x2L1AQ2qy6bH8ld0HeAj3O7tkg9038uTUfRL3jA6IA7DdBv6t9uVXB3BEu\n\toXY70AfK3GE7j53W5rmbgOjQwMxmUci3CfutRgIeUl14t+ZMoY/W+6PTLOgg4zgEq20a\n\tv2ZelKciLPpwoUAaQqRfgOdMd+ySZVTQiV5/tsLwGq6zuZZtjzSzL5nlEhkTzQaQgiBU\n\tiHlrlMVl0k+Ln6oq2VPRWuvYnySHXW04LzjX7cO978nRfRjQ1+RBpsvgpZagAC7fPxro\n\td4WvIUQhZDHrbsQNx0dVywEPs6B1rMftzWs9Cfc2ELaJJspEdjB6NM2OUHz6NobEeg5F\n\tLznQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1718971488; x=1719576288;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;\n\tbh=ZnQlrt5etPp1vNBnBcUs14y1PdBA821vTuZpJecBucE=;\n\tb=c3TYzTiMLOlcYgzjNIQKtEwY69LsktYLhicAh3NoSa67UJDPrx7zsAMLedLtjNj0yO\n\tIiSU1x3LnA2WjqQDZifsl65UYQhztEA81pkxVHJozdouGzydOj2mO2wHt78zdGAiutpp\n\t/UsP5a/pxUxPEQDYLai79erMZ6SmhuPH3g+C42cj9LqBYUBLxFzBelF1LlGDVYvkAbAl\n\trqlkE4HgCFQe4blbptpftJlni2xA05UjZuTxSjMaPXZAETbOJjqFxOUGBnxIaCrDCOzs\n\tKRlL4hKsDCiSbWtFC4BFMTeWR0Q3LGINt0iKJW6wE7jtFnoXPOXvTuLnj/QDR8M3oGVO\n\taWwA==","X-Gm-Message-State":"AOJu0YyLuwCJjBO/62bpnmfpgHjdZkt8rNOEyoPnkHztDS60x9OyjX3A\n\tIVwK/312BVOzqmEDKnsflNV0xT6dqUkYGsXowJCMm6U5hHXzY875YvR6U+ci","X-Google-Smtp-Source":"AGHT+IEp2GUa16grHnyuNNcQppZuujwV27Lh4eSO/EJzDlTSw9h7WBUcQFLiDbsfVW33uVSXKXPkWw==","X-Received":"by 2002:a05:6870:ec8e:b0:254:d417:34ff with SMTP id\n\t586e51a60fabf-25c94da7916mr8544262fac.4.1718971487837; \n\tFri, 21 Jun 2024 05:04:47 -0700 (PDT)","Message-ID":"<478b62b7-5c32-4953-84d4-d2a2118ed988@gmail.com>","Date":"Fri, 21 Jun 2024 07:04:46 -0500","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/2] qcam: rotate the viewfinder output as per camera\n\tproperties","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240620213607.32583-1-joelselvaraj.oss@gmail.com>\n\t<20240620213607.32583-2-joelselvaraj.oss@gmail.com>\n\t<nr2e35i4kj62pashgbrfbnxuxxe2vvo3u3oa5hu2ge6kmy3mos@wf3ytwkln7bs>","Content-Language":"en-US","From":"Joel Selvaraj <joelselvaraj.oss@gmail.com>","In-Reply-To":"<nr2e35i4kj62pashgbrfbnxuxxe2vvo3u3oa5hu2ge6kmy3mos@wf3ytwkln7bs>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30034,"web_url":"https://patchwork.libcamera.org/comment/30034/","msgid":"<20240621132443.GK30640@pendragon.ideasonboard.com>","date":"2024-06-21T13:24:43","subject":"Re: [PATCH 1/2] qcam: rotate the viewfinder output as per camera\n\tproperties","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Jun 21, 2024 at 10:27:23AM +0200, Jacopo Mondi wrote:\n> Hi Joel\n> \n> On Thu, Jun 20, 2024 at 04:36:06PM GMT, Joel Selvaraj wrote:\n> > Devicetrees may specify the rotation at which the camera is\n> > mounted in the hardware. If available, this gets populated in camera\n> > properties. Rotate the viewfinder output accordingly in qcam qt and\n> > opengl renderers.\n> >\n> > Signed-off-by: Joel Selvaraj <joelselvaraj.oss@gmail.com>\n> > ---\n> >  include/libcamera/orientation.h |  2 ++\n> >  src/apps/qcam/main_window.cpp   |  9 ++++++-\n> >  src/apps/qcam/viewfinder.h      |  4 ++-\n> >  src/apps/qcam/viewfinder_gl.cpp | 46 ++++++++++++++++++++++++++++++---\n> >  src/apps/qcam/viewfinder_gl.h   |  4 ++-\n> >  src/apps/qcam/viewfinder_qt.cpp | 18 +++++++++++--\n> >  src/apps/qcam/viewfinder_qt.h   |  5 +++-\n> >  src/libcamera/orientation.cpp   | 34 ++++++++++++++++++++++++\n> >  8 files changed, 113 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/include/libcamera/orientation.h b/include/libcamera/orientation.h\n> > index a3b40e63..e32f5eb8 100644\n> > --- a/include/libcamera/orientation.h\n> > +++ b/include/libcamera/orientation.h\n> > @@ -25,6 +25,8 @@ enum class Orientation {\n> >\n> >  Orientation orientationFromRotation(int angle, bool *success = nullptr);\n> >\n> > +int rotationFromOrientation(const Orientation &orientation, bool *success = nullptr);\n> > +\n> >  std::ostream &operator<<(std::ostream &out, const Orientation &orientation);\n> >\n> >  } /* namespace libcamera */\n> > diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\n> > index d515beed..18c94cf3 100644\n> > --- a/src/apps/qcam/main_window.cpp\n> > +++ b/src/apps/qcam/main_window.cpp\n> > @@ -363,6 +363,7 @@ void MainWindow::toggleCapture(bool start)\n> >  int MainWindow::startCapture()\n> >  {\n> >  \tstd::vector<StreamRole> roles = StreamKeyValueParser::roles(options_[OptStream]);\n> > +\tOrientation orientation = Orientation::Rotate0;\n> >  \tint ret;\n> >\n> >  \t/* Verify roles are supported. */\n> > @@ -392,6 +393,12 @@ int MainWindow::startCapture()\n> >  \t\treturn -EINVAL;\n> >  \t}\n> >\n> > +\t/* Get orientation provided by camera kernel driver */\n\nThe orientation comes from libcamera. It may be provided by kernel\ndrivers in some cases, but that's an implementation detail. I would\nonly mention here that we get the camera orientation.\n\n> > +\tconst ControlList &properties = camera_->properties();\n> > +\tconst auto &rotation = properties.get(properties::Rotation);\n> > +\tif (rotation)\n> > +\t\torientation = orientationFromRotation(*rotation);\n\nI think it would be more efficient to do this a bit differently. While\nmost cameras can't rotate by 90° or 270°, rotation by 180° is not\nuncommon, and it would be useful to make use of that feature if the\ncamera supports it.\n\nYou can set the CameraConfiguration::orientation member to\nOrientation::Rotate0 before calling validate() on the configuration.\nAfter validating it, the member will be adjusted to the orientation you\nwill get in the buffer. If the camera can correct the mounting rotation\n(for instance when the camera is mounted upside-down and the hardware is\ncapable of 180° rotation) then you will get Orientation::Rotate0 back\nafter validate(). Otherwise you will get another value that tells you\nhow the images will be oriented in the buffers. You can then pass that\nvalue to the viewfinder to correct the orientation by rotating the\nimages when displaying.\n\n> > +\n> >  \tStreamConfiguration &vfConfig = config_->at(0);\n> >\n> >  \t/* Use a format supported by the viewfinder if available. */\n> > @@ -444,7 +451,7 @@ int MainWindow::startCapture()\n> >  \tret = viewfinder_->setFormat(vfConfig.pixelFormat,\n> >  \t\t\t\t     QSize(vfConfig.size.width, vfConfig.size.height),\n> >  \t\t\t\t     vfConfig.colorSpace.value_or(ColorSpace::Sycc),\n> > -\t\t\t\t     vfConfig.stride);\n> > +\t\t\t\t     vfConfig.stride, orientation);\n> >  \tif (ret < 0) {\n> >  \t\tqInfo() << \"Failed to set viewfinder format\";\n> >  \t\treturn ret;\n> > diff --git a/src/apps/qcam/viewfinder.h b/src/apps/qcam/viewfinder.h\n> > index 914f88ec..f17cc4a1 100644\n> > --- a/src/apps/qcam/viewfinder.h\n> > +++ b/src/apps/qcam/viewfinder.h\n> > @@ -14,6 +14,7 @@\n> >  #include <libcamera/color_space.h>\n> >  #include <libcamera/formats.h>\n> >  #include <libcamera/framebuffer.h>\n> > +#include <libcamera/orientation.h>\n> >\n> >  class Image;\n> >\n> > @@ -26,7 +27,8 @@ public:\n> >\n> >  \tvirtual int setFormat(const libcamera::PixelFormat &format, const QSize &size,\n> >  \t\t\t      const libcamera::ColorSpace &colorSpace,\n> > -\t\t\t      unsigned int stride) = 0;\n> > +\t\t\t      unsigned int stride,\n> > +\t\t\t      const libcamera::Orientation &orientation) = 0;\n\nlibcamera::Orientation is a simple enum, so you can pass it by value\ninstead of const reference.\n\n> >  \tvirtual void render(libcamera::FrameBuffer *buffer, Image *image) = 0;\n> >  \tvirtual void stop() = 0;\n> >\n> > diff --git a/src/apps/qcam/viewfinder_gl.cpp b/src/apps/qcam/viewfinder_gl.cpp\n> > index 9d2a6960..c4b10e1e 100644\n> > --- a/src/apps/qcam/viewfinder_gl.cpp\n> > +++ b/src/apps/qcam/viewfinder_gl.cpp\n> > @@ -77,7 +77,7 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const\n> >\n> >  int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, const QSize &size,\n> >  \t\t\t    const libcamera::ColorSpace &colorSpace,\n> > -\t\t\t    unsigned int stride)\n> > +\t\t\t    unsigned int stride, const libcamera::Orientation &orientation)\n> >  {\n> >  \tif (format != format_ || colorSpace != colorSpace_) {\n> >  \t\t/*\n> > @@ -101,6 +101,7 @@ int ViewFinderGL::setFormat(const libcamera::PixelFormat &format, const QSize &s\n> >\n> >  \tsize_ = size;\n> >  \tstride_ = stride;\n> > +\torientation_ = orientation;\n> >\n> >  \tupdateGeometry();\n> >  \treturn 0;\n> > @@ -511,7 +512,7 @@ void ViewFinderGL::initializeGL()\n> >  \tglEnable(GL_TEXTURE_2D);\n> >  \tglDisable(GL_DEPTH_TEST);\n> >\n> > -\tstatic const GLfloat coordinates[2][4][2]{\n> > +\tGLfloat coordinates[2][4][2]{\n> >  \t\t{\n> >  \t\t\t/* Vertex coordinates */\n> >  \t\t\t{ -1.0f, -1.0f },\n> > @@ -528,6 +529,41 @@ void ViewFinderGL::initializeGL()\n> >  \t\t},\n> >  \t};\n> >\n> > +\tswitch (orientation_) {\n> > +\tcase libcamera::Orientation::Rotate90:\n> > +\t\tcoordinates[0][0][0] = -1.0f;\n> > +\t\tcoordinates[0][0][1] = +1.0f;\n> > +\t\tcoordinates[0][1][0] = +1.0f;\n> > +\t\tcoordinates[0][1][1] = +1.0f;\n> > +\t\tcoordinates[0][2][0] = +1.0f;\n> > +\t\tcoordinates[0][2][1] = -1.0f;\n> > +\t\tcoordinates[0][3][0] = -1.0f;\n> > +\t\tcoordinates[0][3][1] = -1.0f;\n> > +\t\tbreak;\n> > +\tcase libcamera::Orientation::Rotate180:\n> > +\t\tcoordinates[0][0][0] = +1.0f;\n> > +\t\tcoordinates[0][0][1] = +1.0f;\n> > +\t\tcoordinates[0][1][0] = +1.0f;\n> > +\t\tcoordinates[0][1][1] = -1.0f;\n> > +\t\tcoordinates[0][2][0] = -1.0f;\n> > +\t\tcoordinates[0][2][1] = -1.0f;\n> > +\t\tcoordinates[0][3][0] = -1.0f;\n> > +\t\tcoordinates[0][3][1] = +1.0f;\n> > +\t\tbreak;\n> > +\tcase libcamera::Orientation::Rotate270:\n> > +\t\tcoordinates[0][0][0] = +1.0f;\n> > +\t\tcoordinates[0][0][1] = -1.0f;\n> > +\t\tcoordinates[0][1][0] = -1.0f;\n> > +\t\tcoordinates[0][1][1] = -1.0f;\n> > +\t\tcoordinates[0][2][0] = -1.0f;\n> > +\t\tcoordinates[0][2][1] = +1.0f;\n> > +\t\tcoordinates[0][3][0] = +1.0f;\n> > +\t\tcoordinates[0][3][1] = +1.0f;\n> > +\t\tbreak;\n> > +\tdefault:\n> > +\t\tbreak;\n> > +\t}\n\nI'll have a look at how to improve this :-)\n\n> > +\n> >  \tvertexBuffer_.create();\n> >  \tvertexBuffer_.bind();\n> >  \tvertexBuffer_.allocate(coordinates, sizeof(coordinates));\n> > @@ -831,5 +867,9 @@ void ViewFinderGL::resizeGL(int w, int h)\n> >\n> >  QSize ViewFinderGL::sizeHint() const\n> >  {\n> > -\treturn size_.isValid() ? size_ : QSize(640, 480);\n> > +\tif (orientation_ == libcamera::Orientation::Rotate90 ||\n> > +\t    orientation_ == libcamera::Orientation::Rotate270)\n> > +\t\treturn size_.isValid() ? size_.transposed() : QSize(480, 640);\n> > +\telse\n> > +\t\treturn size_.isValid() ? size_ : QSize(640, 480);\n\nThis would avoid hardcoding the default VGA size twice:\n\t\n\tQSize size = size_.isValid() ? size_ : QSize(640, 480);\n\n\tif (orientation_ == libcamera::Orientation::Rotate90 ||\n\t    orientation_ == libcamera::Orientation::Rotate270)\n\t\tsize.transpose();\n\n\treturn size;\n\n> >  }\n> > diff --git a/src/apps/qcam/viewfinder_gl.h b/src/apps/qcam/viewfinder_gl.h\n> > index 23744b41..1fc0f827 100644\n> > --- a/src/apps/qcam/viewfinder_gl.h\n> > +++ b/src/apps/qcam/viewfinder_gl.h\n> > @@ -39,7 +39,8 @@ public:\n> >\n> >  \tint setFormat(const libcamera::PixelFormat &format, const QSize &size,\n> >  \t\t      const libcamera::ColorSpace &colorSpace,\n> > -\t\t      unsigned int stride) override;\n> > +\t\t      unsigned int stride,\n> > +\t\t      const libcamera::Orientation &orientation) override;\n> >  \tvoid render(libcamera::FrameBuffer *buffer, Image *image) override;\n> >  \tvoid stop() override;\n> >\n> > @@ -68,6 +69,7 @@ private:\n> >  \tlibcamera::FrameBuffer *buffer_;\n> >  \tlibcamera::PixelFormat format_;\n> >  \tlibcamera::ColorSpace colorSpace_;\n> > +\tlibcamera::Orientation orientation_;\n> >  \tQSize size_;\n> >  \tunsigned int stride_;\n> >  \tImage *image_;\n> > diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp\n> > index 4821c27d..583a74f7 100644\n> > --- a/src/apps/qcam/viewfinder_qt.cpp\n> > +++ b/src/apps/qcam/viewfinder_qt.cpp\n> > @@ -57,7 +57,7 @@ const QList<libcamera::PixelFormat> &ViewFinderQt::nativeFormats() const\n> >\n> >  int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, const QSize &size,\n> >  \t\t\t    [[maybe_unused]] const libcamera::ColorSpace &colorSpace,\n> > -\t\t\t    unsigned int stride)\n> > +\t\t\t    unsigned int stride, const libcamera::Orientation &orientation)\n> >  {\n> >  \timage_ = QImage();\n> >\n> > @@ -80,6 +80,15 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format, const QSize &s\n> >\n> >  \tformat_ = format;\n> >  \tsize_ = size;\n> > +\torientation_ = orientation;\n> > +\n> > +\tbool success;\n> > +\tint angle = libcamera::rotationFromOrientation(orientation, &success);\n> > +\tif (!success) {\n> > +\t\tqWarning() << \"Unsupported orientation\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\ttransform_ = QTransform().rotate(angle);\n> >\n> >  \tupdateGeometry();\n> >  \treturn 0;\n> > @@ -148,6 +157,7 @@ void ViewFinderQt::paintEvent(QPaintEvent *)\n> >\n> >  \t/* If we have an image, draw it. */\n> >  \tif (!image_.isNull()) {\n> > +\t\timage_ = image_.transformed(transform_);\n\nThis is quite inefficient, as it will result in a copy of the image. The\nQPainter class supports applying transformations when drawing, could you\ntry it out ?\n\n> >  \t\tpainter.drawImage(rect(), image_, image_.rect());\n> >  \t\treturn;\n> >  \t}\n> > @@ -179,5 +189,9 @@ void ViewFinderQt::paintEvent(QPaintEvent *)\n> >\n> >  QSize ViewFinderQt::sizeHint() const\n> >  {\n> > -\treturn size_.isValid() ? size_ : QSize(640, 480);\n> > +\tif (orientation_ == libcamera::Orientation::Rotate90 ||\n> > +\t    orientation_ == libcamera::Orientation::Rotate270)\n> > +\t\treturn size_.isValid() ? size_.transposed() : QSize(480, 640);\n> > +\telse\n> > +\t\treturn size_.isValid() ? size_ : QSize(640, 480);\n\nSame comment as above.\n\n> >  }\n> > diff --git a/src/apps/qcam/viewfinder_qt.h b/src/apps/qcam/viewfinder_qt.h\n> > index 4f4b9f11..309b39e5 100644\n> > --- a/src/apps/qcam/viewfinder_qt.h\n> > +++ b/src/apps/qcam/viewfinder_qt.h\n> > @@ -33,7 +33,8 @@ public:\n> >\n> >  \tint setFormat(const libcamera::PixelFormat &format, const QSize &size,\n> >  \t\t      const libcamera::ColorSpace &colorSpace,\n> > -\t\t      unsigned int stride) override;\n> > +\t\t      unsigned int stride,\n> > +\t\t      const libcamera::Orientation &orientation) override;\n> >  \tvoid render(libcamera::FrameBuffer *buffer, Image *image) override;\n> >  \tvoid stop() override;\n> >\n> > @@ -51,6 +52,8 @@ private:\n> >\n> >  \tlibcamera::PixelFormat format_;\n> >  \tQSize size_;\n> > +\tQTransform transform_;\n> > +\tlibcamera::Orientation orientation_;\n> >\n> >  \t/* Camera stopped icon */\n> >  \tQSize vfSize_;\n> > diff --git a/src/libcamera/orientation.cpp b/src/libcamera/orientation.cpp\n> > index 47fd6a32..7afb37a8 100644\n> > --- a/src/libcamera/orientation.cpp\n> > +++ b/src/libcamera/orientation.cpp\n> > @@ -92,6 +92,40 @@ Orientation orientationFromRotation(int angle, bool *success)\n> >  \treturn Orientation::Rotate0;\n> >  }\n> >\n> > +/**\n> > + * \\brief Return the rotation angle for a given orientation\n> \n> I'm a bit unconfortable about having this in the API, as it might\n> result in something more complex than what it actually looks like.\n> \n> All the 2d plane transforms expressed by Orientation include\n> an optional horizontal flip, something that cannot be expressed with\n> an angular rotation only.\n> \n> Look at the example images we have in the Orientation class\n> documentation. If you only report the rotation angle (*)\n> you won't be able to identify if an image has been flipped or not, and\n> the result might not be what you expect.\n> \n> (*) Also expressing the rotation angle only is less trivial than what\n> one might expect. First of all, if you want to return a rotation\n> angle, you have to specify what the rotation describes:\n> \n> 1) The image rotation in the memory buffers\n> 2) The correction to be applied to display the image in its natural\n> orientation\n> \n> The Orientation class describes the image rotation in memory buffers,\n> so I presume that's what you're expressing here, but it has to be made\n> explicit in the documentation of the function. The direction of the\n> rotation has to be specified as well. All of this is documented in the\n> Orientation class, but if this function is made part of the API, it\n> has to be specified here as well.\n> \n> I would suggest to infer the rotation angle from Orientation in the\n> application and explicitly say that mirroring is not corrected there.\n\nI agree. libcamera reports the image orientation, and it's up to the\napplication to decide what to then do. Different applications may have\ndifferent needs, so it's best to move this to the application side.\n\nOn a side note, if we kept this in libcamera, the patch would have\nneeded to be split in two, with one patch to add the new function, and a\nseparate patch for qcam. We try to stick to the \"one change, one patch\"\nrule.\n\n> > + * \\param[in] orientation The orientation to convert to a rotation angle\n> > + * \\param[out] success Set to `true` if the given orientation is valid,\n> > + * otherwise `false`\n> > + * \\return The rotation angle corresponding to the given orientation\n> > + * if \\a success was set to `true`, otherwise 0.\n> > + */\n> > +int rotationFromOrientation(const Orientation &orientation, bool *success)\n> > +{\n> > +\tif (success != nullptr)\n> > +\t\t*success = true;\n> > +\n> > +\tswitch (orientation) {\n> > +\tcase Orientation::Rotate0:\n> > +\tcase Orientation::Rotate0Mirror:\n> > +\t\treturn 0;\n> > +\tcase Orientation::Rotate90:\n> > +\tcase Orientation::Rotate90Mirror:\n> > +\t\treturn 90;\n> > +\tcase Orientation::Rotate180:\n> > +\tcase Orientation::Rotate180Mirror:\n> > +\t\treturn 180;\n> > +\tcase Orientation::Rotate270:\n> > +\tcase Orientation::Rotate270Mirror:\n> > +\t\treturn 270;\n> > +\t}\n> > +\n> > +\tif (success != nullptr)\n> > +\t\t*success = false;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> >  /**\n> >   * \\brief Prints human-friendly names for Orientation items\n> >   * \\param[in] out The output stream","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 30D92BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 21 Jun 2024 13:25:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 467C9654A3;\n\tFri, 21 Jun 2024 15:25:08 +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 BE1AB654A2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Jun 2024 15:25:05 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DFF918E1;\n\tFri, 21 Jun 2024 15:24:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MCFZPEej\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718976286;\n\tbh=Z76Vd/xIyL/Z5Pd/iw3bQbZ+TVzu62A9WU1VKnKdMNo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MCFZPEejXqxPKKRO8Pt8jwz0JLWOjWLyva5pXTNyFOSNT9WfpxqkS8iSASM42bCCS\n\tdt/dntVkLDJEzorOYENKlwql0g27UGizBK0nbU7GEZmMfEyImXvuGeUxV39pHJeTXx\n\tOszdTSV2AAJ1ahah6+PRDxSnuj5EgLjeXqgZSjCk=","Date":"Fri, 21 Jun 2024 16:24:43 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Joel Selvaraj <joelselvaraj.oss@gmail.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] qcam: rotate the viewfinder output as per camera\n\tproperties","Message-ID":"<20240621132443.GK30640@pendragon.ideasonboard.com>","References":"<20240620213607.32583-1-joelselvaraj.oss@gmail.com>\n\t<20240620213607.32583-2-joelselvaraj.oss@gmail.com>\n\t<nr2e35i4kj62pashgbrfbnxuxxe2vvo3u3oa5hu2ge6kmy3mos@wf3ytwkln7bs>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<nr2e35i4kj62pashgbrfbnxuxxe2vvo3u3oa5hu2ge6kmy3mos@wf3ytwkln7bs>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]