[{"id":28021,"web_url":"https://patchwork.libcamera.org/comment/28021/","msgid":"<20231022225630.GC3336@pendragon.ideasonboard.com>","date":"2023-10-22T22:56:30","subject":"Re: [libcamera-devel] [PATCH v6 0/12] libcamera: Replace\n\tCameraConfiguration::transform","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 patches.\n\nCC'ing David, for feedback on the impact on Raspberry Pi (if any).\n\nOn Thu, Oct 19, 2023 at 04:01:21PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> v5->v6:\n> - Rename enum members\n> - replace images with svg\n> - fix documentation\n> \n> v4->v5:\n> - Add support in Python layer\n> - Rebased on master\n> \n> v3->v4:\n> - Remove unrelated change in 8/10\n> - Remove a double test case\n> - Add David's tags\n> \n> v2->v3:\n> - Move Orientation out of CameraConfiguration to a dedicated file\n> - Change operator*(Transform, Transform)\n> - Address David's comments in documentation and commit messages\n> \n> v1->v2:\n> - Provide a better definitio of Orientation based on two basic operations\n> - Provide functions to combine Orientation and Transpose\n> - Provide tests\n> - Do not adjust properties::Rotation anymore\n> \n> This series proposes to replace the usage to Transform in the public API in\n> favour of a new Orientation type, defined based on the EXIF\n> specification, tag 274, 'orientation'. For reference this is the same as\n> implemented by gstreamer:\n> https://gstreamer.freedesktop.org/documentation/gstreamer/gsttaglist.html?gi-language=c#GST_TAG_IMAGE_ORIENTATION\n> \n> (the names of the enum values are different as gstreamer expresses the\n> \"correction\" while we express the actual orientation in memory buffers).\n> \n> The newly introduced CameraConfiguration::orientation replaces the\n> existing CameraConfiguration::tranform, and it is meant for application to\n> express how they would like the images to be oriented, not to tell libcamera\n> what to do. As an example, passing in 'rotation0' means that the application\n> expects the images to be rotated upright, and doesn't tell libcamera not to\n> apply any rotation like passing in \"Transform::Identity\" did.\n> \n> The value CameraConfiguration::orientation is set to after a validate() also\n> differs in meaning, as instead of reporting \"what applications have to do\n> to obtain what they originally asked for\" it simply reports the actual\n> orientation of the stream: this means that if libcamera cannot fully satisfy the\n> user request it will set ::orientation to report the native images rotation\n> and the CameraConfiguration::status will be set to Adjusted.\n> \n> Handling of 90 and 270 degrees rotation has also changed: as the camera sensor\n> cannot correct rotations that include a transposition, requests for a 90/270\n> corrections are ignored. This makes it clear and less confusing for applications\n> that they have to deal with correction fully by themselves. As an example, with\n> the current implementation if the application requires a Rot270 (HFlip +\n> Transpose) libcamera will do the HFlip and leave transposition to the upper\n> layers. There is no clear advantage in doing so, and display compositors are\n> better suited for handling transpositions and flipping in a single pass instead\n> of having the library try to handle part of that.\n> \n> This series clearly breaks the application API as it removes a member from\n> CameraConfiguration, so it should be introduced probably only when a new release\n> is cut.\n> \n> Tested on RPi imx219 with the newly introduced cam option, flip, mirror and\n> rotation work as expected.\n\nOverall this looks good to me. I've just sent a series of patches on top\nof thes series (numbered 13/12 to 21/12), with the first patches being\nsmall fixups that I propose squashing into the corresponding base patch.\nThe last three patches would require more complex rework of your series\nif we wanted to squash them, so I think we can keep them on top.\n\nI wanted to replace the\n\nOrientation operator*(const Orientation &o, const Transform &t);\n\nwith a\n\nOrientation Transform::operator()(const Orientation &o);\n\nin order to be able to write\n\n\tTransform t = ...;\n\tOrientation o1 = ...,\n\tOrientation o2 = t(o1);\n\nor also\n\n\tOrientation o1 = ...,\n\tOrientation o2 = Transform::HFlip(o1);\n\nwhich I think would be a nicer syntax. However, I couldn't find a way to\nnicely do so :-( Transform is currently an enum class, which is not a\nclass, and thus can't have member operators. The call operator having no\nnon-member (outside of the class definition) variant, I had to try and\nturn Transform into a class. I got stuck at the point where HFlip had to\nbe a static constexpr member of the Transform class (to be able to write\n`Transform::HFlip`), but also an instance of the Transform class (to be\nable to write `Transform::HFlip(o)`). The compiler rightfully complained\nthat I was trying to use a type that hasn't been fully defined yet. I\ngave up for now (but would be very interested into hearing from a C++\nenthousiast on how something similar could be achieved). If we keep the\nmultiplication operator as-is, then your patch 06/12 that inverts the\norder of the operands to the multiplication operator between two\nTransform instances is needed.\n\nIf you're fine with the additional changes, I can handle the squashing\nand pushing. I'll need R-b tags for the last three patches though. As\nfor this series, with the changes on top,\n\nReviewed-by: Laurent Pinchart \n\nAn additional change I would like to see on top would be the rewrite of\nthe Python Transform class in Python, without going through C++\nbindings. This would allow us to drop the transformFromRotation() which\nis used in the Python bindings code only. Any opinion on this ?\n\n> Jacopo Mondi (12):\n> libcamera: camera_sensor: Cache rotationTransform_\n> libcamera: camera: Introduce Orientation\n> Documentation: Add figures to document Orientation\n> libcamera: properties: Make 'Rotation' the mounting rotation\n> libcamera: transform: Add functions to convert Orientation\n> libcamera: transform: Invert operator*() operands\n> libcamera: transform: Add operations with Orientation\n> test: Add unit test for Transform and Orientation\n> py: libcamera: Define and use Orientation\n> libcamera: Use CameraConfiguration::orientation\n> apps: cam: Add option to set stream orientation\n> py: cam: Add option to set stream orientation\n> \n> Documentation/Doxyfile.in | 2 +\n> Documentation/rotation/rotate0.svg | 132 +++++++\n> Documentation/rotation/rotate0Mirror.svg | 135 +++++++\n> Documentation/rotation/rotate180.svg | 135 +++++++\n> Documentation/rotation/rotate180Mirror.svg | 135 +++++++\n> Documentation/rotation/rotate270.svg | 135 +++++++\n> Documentation/rotation/rotate270Mirror.svg | 135 +++++++\n> Documentation/rotation/rotate90.svg | 135 +++++++\n> Documentation/rotation/rotate90Mirror.svg | 135 +++++++\n> include/libcamera/camera.h | 4 +-\n> include/libcamera/internal/camera_sensor.h | 5 +-\n> include/libcamera/meson.build | 1 +\n> include/libcamera/orientation.h | 28 ++\n> include/libcamera/transform.h | 7 +\n> src/apps/cam/camera_session.cpp | 18 +\n> src/apps/cam/main.cpp | 5 +\n> src/apps/cam/main.h | 1 +\n> src/libcamera/camera.cpp | 20 +-\n> src/libcamera/camera_sensor.cpp | 131 ++++---\n> src/libcamera/meson.build | 1 +\n> src/libcamera/orientation.cpp | 79 +++++\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +-\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +-\n> .../pipeline/rpi/common/pipeline_base.cpp | 9 +-\n> src/libcamera/pipeline/simple/simple.cpp | 6 +-\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 +-\n> src/libcamera/pipeline/vimc/vimc.cpp | 4 +-\n> src/libcamera/property_ids.yaml | 8 +-\n> src/libcamera/transform.cpp | 112 +++++-\n> src/py/cam/cam.py | 21 ++\n> src/py/libcamera/py_enums.cpp | 10 +\n> src/py/libcamera/py_main.cpp | 2 +-\n> test/meson.build | 1 +\n> test/transform.cpp | 329 ++++++++++++++++++\n> 34 files changed, 1787 insertions(+), 112 deletions(-)\n> create mode 100644 Documentation/rotation/rotate0.svg\n> create mode 100644 Documentation/rotation/rotate0Mirror.svg\n> create mode 100644 Documentation/rotation/rotate180.svg\n> create mode 100644 Documentation/rotation/rotate180Mirror.svg\n> create mode 100644 Documentation/rotation/rotate270.svg\n> create mode 100644 Documentation/rotation/rotate270Mirror.svg\n> create mode 100644 Documentation/rotation/rotate90.svg\n> create mode 100644 Documentation/rotation/rotate90Mirror.svg\n> create mode 100644 include/libcamera/orientation.h\n> create mode 100644 src/libcamera/orientation.cpp\n> create mode 100644 test/transform.cpp","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 D9C55C0DA4\n\tfor ;\n\tSun, 22 Oct 2023 22:56:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 59B2F6297E;\n\tMon, 23 Oct 2023 00:56:24 +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 E23F86297C\n\tfor ;\n\tMon, 23 Oct 2023 00:56:22 +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 5DDE0749;\n\tMon, 23 Oct 2023 00:56:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1698015384;\n\tbh=kDxTIOSGxYfIcNDEQFvfnE7tOcdwGcZwt5s/8rbhjgw=;\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=utyU1dhbwURWNQrLDgFmcovyRVl/vlroIRoocBTEPHK8EydKHOhRkm+koesTmN2iF\n\tfpZI/An8qzHGDsqbKzbRFXbmETGneyS1uQ7b7aSTq0a/UHl5As3WsPjF/RusvkE8VT\n\tvxFHh0RpSiC38HwmJoRTDNQYs5qHXDIKqJjC+0QHZnXX1NLOF5deNQ9FeOs7ES/s6i\n\tUsDhTTz1Qqxih0TZMXVqzIb715qkBQGUTUfVWNjm9O34RZzcJyQ33BPWTeRnLkO82r\n\tdceQdRC40Diw9DxTd2GWxN7e9qKlmN5R7hB8822pPyEG6DTbg+Q3/Q8sRTvmQRGpU3\n\tb/08D7wDCS3YA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1698015372;\n\tbh=kDxTIOSGxYfIcNDEQFvfnE7tOcdwGcZwt5s/8rbhjgw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VvpW3A92GcE+FooZJCNzbRcRiCI1SV9DyqEQ5AOA5/31ITV2Yxns4AVv6lb/70/7k\n\tGFGnOJqhrIw9LbwMM4FquPp6a6EWeDFsOfH+XnfgKEGWP9u80C9REY84YTD7laXnAz\n\tKqB5Zetsk6hJ6QKm3IUcOYmBZFE52YyWSlRZ/eKU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"VvpW3A92\"; dkim-atps=neutral","Date":"Mon, 23 Oct 2023 01:56:30 +0300","To":"Jacopo Mondi ","Message-ID":"<20231022225630.GC3336@pendragon.ideasonboard.com>","References":"<20231019140133.32090-1-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231019140133.32090-1-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v6 0/12] libcamera: Replace\n\tCameraConfiguration::transform","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":28026,"web_url":"https://patchwork.libcamera.org/comment/28026/","msgid":"","date":"2023-10-23T08:58:10","subject":"Re: [libcamera-devel] [PATCH v6 0/12] libcamera: Replace\n\tCameraConfiguration::transform","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Mon, Oct 23, 2023 at 01:56:30AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Hi Jacopo,\n>\n> Thank you for the patches.\n>\n> CC'ing David, for feedback on the impact on Raspberry Pi (if any).\n>\n> On Thu, Oct 19, 2023 at 04:01:21PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> > v5->v6:\n> > - Rename enum members\n> > - replace images with svg\n> > - fix documentation\n> >\n> > v4->v5:\n> > - Add support in Python layer\n> > - Rebased on master\n> >\n> > v3->v4:\n> > - Remove unrelated change in 8/10\n> > - Remove a double test case\n> > - Add David's tags\n> >\n> > v2->v3:\n> > - Move Orientation out of CameraConfiguration to a dedicated file\n> > - Change operator*(Transform, Transform)\n> > - Address David's comments in documentation and commit messages\n> >\n> > v1->v2:\n> > - Provide a better definitio of Orientation based on two basic operations\n> > - Provide functions to combine Orientation and Transpose\n> > - Provide tests\n> > - Do not adjust properties::Rotation anymore\n> >\n> > This series proposes to replace the usage to Transform in the public API in\n> > favour of a new Orientation type, defined based on the EXIF\n> > specification, tag 274, 'orientation'. For reference this is the same as\n> > implemented by gstreamer:\n> > https://gstreamer.freedesktop.org/documentation/gstreamer/gsttaglist.html?gi-language=c#GST_TAG_IMAGE_ORIENTATION\n> >\n> > (the names of the enum values are different as gstreamer expresses the\n> > \"correction\" while we express the actual orientation in memory buffers).\n> >\n> > The newly introduced CameraConfiguration::orientation replaces the\n> > existing CameraConfiguration::tranform, and it is meant for application to\n> > express how they would like the images to be oriented, not to tell libcamera\n> > what to do. As an example, passing in 'rotation0' means that the application\n> > expects the images to be rotated upright, and doesn't tell libcamera not to\n> > apply any rotation like passing in \"Transform::Identity\" did.\n> >\n> > The value CameraConfiguration::orientation is set to after a validate() also\n> > differs in meaning, as instead of reporting \"what applications have to do\n> > to obtain what they originally asked for\" it simply reports the actual\n> > orientation of the stream: this means that if libcamera cannot fully satisfy the\n> > user request it will set ::orientation to report the native images rotation\n> > and the CameraConfiguration::status will be set to Adjusted.\n> >\n> > Handling of 90 and 270 degrees rotation has also changed: as the camera sensor\n> > cannot correct rotations that include a transposition, requests for a 90/270\n> > corrections are ignored. This makes it clear and less confusing for applications\n> > that they have to deal with correction fully by themselves. As an example, with\n> > the current implementation if the application requires a Rot270 (HFlip +\n> > Transpose) libcamera will do the HFlip and leave transposition to the upper\n> > layers. There is no clear advantage in doing so, and display compositors are\n> > better suited for handling transpositions and flipping in a single pass instead\n> > of having the library try to handle part of that.\n> >\n> > This series clearly breaks the application API as it removes a member from\n> > CameraConfiguration, so it should be introduced probably only when a new release\n> > is cut.\n> >\n> > Tested on RPi imx219 with the newly introduced cam option, flip, mirror and\n> > rotation work as expected.\n>\n> Overall this looks good to me. I've just sent a series of patches on top\n> of thes series (numbered 13/12 to 21/12), with the first patches being\n> small fixups that I propose squashing into the corresponding base patch.\n> The last three patches would require more complex rework of your series\n> if we wanted to squash them, so I think we can keep them on top.\n>\n> I wanted to replace the\n>\n> Orientation operator*(const Orientation &o, const Transform &t);\n>\n> with a\n>\n> Orientation Transform::operator()(const Orientation &o);\n>\n> in order to be able to write\n>\n> \tTransform t = ...;\n> \tOrientation o1 = ...,\n> \tOrientation o2 = t(o1);\n>\n> or also\n>\n> \tOrientation o1 = ...,\n> \tOrientation o2 = Transform::HFlip(o1);\n>\n> which I think would be a nicer syntax. However, I couldn't find a way to\n> nicely do so :-( Transform is currently an enum class, which is not a\n> class, and thus can't have member operators. The call operator having no\n> non-member (outside of the class definition) variant, I had to try and\n> turn Transform into a class. I got stuck at the point where HFlip had to\n> be a static constexpr member of the Transform class (to be able to write\n> `Transform::HFlip`), but also an instance of the Transform class (to be\n> able to write `Transform::HFlip(o)`). The compiler rightfully complained\n> that I was trying to use a type that hasn't been fully defined yet. I\n> gave up for now (but would be very interested into hearing from a C++\n> enthousiast on how something similar could be achieved). If we keep the\n> multiplication operator as-is, then your patch 06/12 that inverts the\n> order of the operands to the multiplication operator between two\n> Transform instances is needed.\n>\n> If you're fine with the additional changes, I can handle the squashing\n> and pushing. I'll need R-b tags for the last three patches though. As\n> for this series, with the changes on top,\n\nMinor comments apart, I'm fine with all of this.\n\nWith a few questions clarified, feel free to squash in what's\nsquashable and push\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> Reviewed-by: Laurent Pinchart \n>\n> An additional change I would like to see on top would be the rewrite of\n> the Python Transform class in Python, without going through C++\n> bindings. This would allow us to drop the transformFromRotation() which\n> is used in the Python bindings code only. Any opinion on this ?\n>\n> > Jacopo Mondi (12):\n> > libcamera: camera_sensor: Cache rotationTransform_\n> > libcamera: camera: Introduce Orientation\n> > Documentation: Add figures to document Orientation\n> > libcamera: properties: Make 'Rotation' the mounting rotation\n> > libcamera: transform: Add functions to convert Orientation\n> > libcamera: transform: Invert operator*() operands\n> > libcamera: transform: Add operations with Orientation\n> > test: Add unit test for Transform and Orientation\n> > py: libcamera: Define and use Orientation\n> > libcamera: Use CameraConfiguration::orientation\n> > apps: cam: Add option to set stream orientation\n> > py: cam: Add option to set stream orientation\n> >\n> > Documentation/Doxyfile.in | 2 +\n> > Documentation/rotation/rotate0.svg | 132 +++++++\n> > Documentation/rotation/rotate0Mirror.svg | 135 +++++++\n> > Documentation/rotation/rotate180.svg | 135 +++++++\n> > Documentation/rotation/rotate180Mirror.svg | 135 +++++++\n> > Documentation/rotation/rotate270.svg | 135 +++++++\n> > Documentation/rotation/rotate270Mirror.svg | 135 +++++++\n> > Documentation/rotation/rotate90.svg | 135 +++++++\n> > Documentation/rotation/rotate90Mirror.svg | 135 +++++++\n> > include/libcamera/camera.h | 4 +-\n> > include/libcamera/internal/camera_sensor.h | 5 +-\n> > include/libcamera/meson.build | 1 +\n> > include/libcamera/orientation.h | 28 ++\n> > include/libcamera/transform.h | 7 +\n> > src/apps/cam/camera_session.cpp | 18 +\n> > src/apps/cam/main.cpp | 5 +\n> > src/apps/cam/main.h | 1 +\n> > src/libcamera/camera.cpp | 20 +-\n> > src/libcamera/camera_sensor.cpp | 131 ++++---\n> > src/libcamera/meson.build | 1 +\n> > src/libcamera/orientation.cpp | 79 +++++\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +-\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +-\n> > .../pipeline/rpi/common/pipeline_base.cpp | 9 +-\n> > src/libcamera/pipeline/simple/simple.cpp | 6 +-\n> > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 +-\n> > src/libcamera/pipeline/vimc/vimc.cpp | 4 +-\n> > src/libcamera/property_ids.yaml | 8 +-\n> > src/libcamera/transform.cpp | 112 +++++-\n> > src/py/cam/cam.py | 21 ++\n> > src/py/libcamera/py_enums.cpp | 10 +\n> > src/py/libcamera/py_main.cpp | 2 +-\n> > test/meson.build | 1 +\n> > test/transform.cpp | 329 ++++++++++++++++++\n> > 34 files changed, 1787 insertions(+), 112 deletions(-)\n> > create mode 100644 Documentation/rotation/rotate0.svg\n> > create mode 100644 Documentation/rotation/rotate0Mirror.svg\n> > create mode 100644 Documentation/rotation/rotate180.svg\n> > create mode 100644 Documentation/rotation/rotate180Mirror.svg\n> > create mode 100644 Documentation/rotation/rotate270.svg\n> > create mode 100644 Documentation/rotation/rotate270Mirror.svg\n> > create mode 100644 Documentation/rotation/rotate90.svg\n> > create mode 100644 Documentation/rotation/rotate90Mirror.svg\n> > create mode 100644 include/libcamera/orientation.h\n> > create mode 100644 src/libcamera/orientation.cpp\n> > create mode 100644 test/transform.cpp\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 E9592C0DA4\n\tfor ;\n\tMon, 23 Oct 2023 08:58:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A5C8B6297E;\n\tMon, 23 Oct 2023 10:58:16 +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 96AC160556\n\tfor ;\n\tMon, 23 Oct 2023 10:58:14 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1143DB53;\n\tMon, 23 Oct 2023 10:58:04 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1698051496;\n\tbh=owDBMyB2JQ5k3/ICRci83/GNU2y2EQL7CWMIy8mGy2s=;\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=r/eMEG9viJSXz0Qp8QRrCWSYMUiMfmbxAddZ7COP6WUkIXjv8Ew5Iup9REb+/oZUE\n\tUat0vyrk2Pu0kARN4/lstDpSA57tcLx+jQ6acQBq0PyCcynwF7uSOXV4rlHeo4dNIL\n\tJx5497fDfD9oRFToWyO2CFt4WSNG55W+RKNlFAM704YoXsOi0MyorKT/qKzx3Al0Nk\n\t4GAvMC3Lg1ErmtuRr5nJNkDLJRdozeVUBVYeCNYfUJVDnjDv0eATXoJBI9MalS9w8S\n\tQJWCdFIIJsr/Tjo1YIbCWUY2qJ6ANNsQ2Arel77vyxcVnE5JT1spsdnZHuXtKj9cWk\n\t+vlcg8y6Ds/Ag==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1698051484;\n\tbh=owDBMyB2JQ5k3/ICRci83/GNU2y2EQL7CWMIy8mGy2s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=p+2AU9dAMVnoKb8YBbUM0J+h463PyKglX8hFgsZc7oldRIoY5ENyWHQ0pH+YsGih4\n\tjbvnEeCNzE6r9c6oZjXTUx2PEK9oLAbX4mxmm0IP29oVnmD9eW2/7ZIDkV7z8WEu9Z\n\tquDij3CCIXX+VrKl3+hEOkG0u42AtOEyMCWv3KFQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"p+2AU9dA\"; dkim-atps=neutral","Date":"Mon, 23 Oct 2023 10:58:10 +0200","To":"Laurent Pinchart ","Message-ID":"","References":"<20231019140133.32090-1-jacopo.mondi@ideasonboard.com>\n\t<20231022225630.GC3336@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231022225630.GC3336@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v6 0/12] libcamera: Replace\n\tCameraConfiguration::transform","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\" "}},{"id":28055,"web_url":"https://patchwork.libcamera.org/comment/28055/","msgid":"","date":"2023-11-03T19:41:44","subject":"Re: [libcamera-devel] [PATCH v6 0/12] libcamera: Replace\n\tCameraConfiguration::transform","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2023. október 23., hétfő 0:56 keltezéssel, Laurent Pinchart via libcamera-devel írta:\n\n> [...]\n> I wanted to replace the\n> \n> Orientation operator*(const Orientation &o, const Transform &t);\n> \n> with a\n> \n> Orientation Transform::operator()(const Orientation &o);\n> \n> in order to be able to write\n> \n> \tTransform t = ...;\n> \tOrientation o1 = ...,\n> \tOrientation o2 = t(o1);\n> \n> or also\n> \n> \tOrientation o1 = ...,\n> \tOrientation o2 = Transform::HFlip(o1);\n> \n> which I think would be a nicer syntax. However, I couldn't find a way to\n> nicely do so :-( Transform is currently an enum class, which is not a\n> class, and thus can't have member operators. The call operator having no\n> non-member (outside of the class definition) variant, I had to try and\n> turn Transform into a class. I got stuck at the point where HFlip had to\n> be a static constexpr member of the Transform class (to be able to write\n> `Transform::HFlip`), but also an instance of the Transform class (to be\n> able to write `Transform::HFlip(o)`). The compiler rightfully complained\n> that I was trying to use a type that hasn't been fully defined yet. I\n> gave up for now (but would be very interested into hearing from a C++\n> enthousiast on how something similar could be achieved). If we keep the\n> multiplication operator as-is, then your patch 06/12 that inverts the\n> order of the operands to the multiplication operator between two\n> Transform instances is needed.\n> [...]\n\nI believe something like the following can work: https://gcc.godbolt.org/z/ecqoGTGGs\nIt's a lot of code compared to the enum...\n\nOn a slightly related note, I think using a pipe is also not bad: \"o1 | t1 | t2 | t3 ...\".\n\n\nRegards,\nBarnabás Pőcze","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 1B710BD16B\n\tfor ;\n\tFri, 3 Nov 2023 19:42:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54D5A62999;\n\tFri, 3 Nov 2023 20:42:02 +0100 (CET)","from mail-40131.protonmail.ch (mail-40131.protonmail.ch\n\t[185.70.40.131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6EFC461DC4\n\tfor ;\n\tFri, 3 Nov 2023 20:41:59 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1699040522;\n\tbh=Fr0DjvXAwXhXh6/Mnt/qNkcY9W2xTZsynlMyFGlg0ys=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=H8318Szh7hSi5DJ2dvCpq3fefk/2U6UJ/Ikg80OY2Lh5h0jSLkqxyKcqmwUEVVndB\n\tF0aL76JTvCErwWkZImILw5Z16/bwlkcfhOrPXlszOYjNY36OSbiuGVZiMHZfx26n4d\n\t3n3m0ZmXRiIpmCq6dsQ8FljnDEAC4vfbo+c8SHDM8nHjIO8tJiZywZdeZF13+eVG+D\n\t7d7vagEQtvPv5nbIf5Y7xm5SQIY0B6RDz89j6ENJZPtdZ582wxbsFovw1URgqvktwo\n\tuXxur0XtM+t6P7EkUB46B+Mldr7F8jmvpNr2J8DFF0LAJ7UOYGrsHOAMjC/ZEp1wm/\n\tAnTrRSXOQyYmg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1699040518; x=1699299718;\n\tbh=Fr0DjvXAwXhXh6/Mnt/qNkcY9W2xTZsynlMyFGlg0ys=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=AsOSpG4imwMof4M1pZgmQ+decvDFvFXxZLvk0wR/doXQEuG4HXbWdJ+dqJL9sfRqb\n\tOkNrHEVdVObvx9q2g8F/4QzQP6W0lo1TTaTRjZuvHWQEbdRU7oKSI/dk1U+62/3Rvg\n\t0gphFA9oX5HSYPysiDsRltBdqLF5/xKpVzbfhoG51hnS3cvxNd2a3/rFl4I5GFropZ\n\tl2MrL1XL+JQbbF1Id3z0/FfeYVco+NqgS1PVp2PrEJjkTSeC4YtyaSbaJuONArbFPA\n\tPS8n0psKZwgfFDkwqWj3XU5apfdHwiDfP4ETpsNDTd+7ZXvqUjoxR9CLjyFDUApufC\n\t6gQDTt8DB+3IA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"AsOSpG4i\"; dkim-atps=neutral","Date":"Fri, 03 Nov 2023 19:41:44 +0000","To":"Laurent Pinchart ","Message-ID":"","In-Reply-To":"<20231022225630.GC3336@pendragon.ideasonboard.com>","References":"<20231019140133.32090-1-jacopo.mondi@ideasonboard.com>\n\t<20231022225630.GC3336@pendragon.ideasonboard.com>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v6 0/12] libcamera: Replace\n\tCameraConfiguration::transform","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":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]