[{"id":27559,"web_url":"https://patchwork.libcamera.org/comment/27559/","msgid":"<CAHW6GYKmvSh9oZcFJB48oQHsw2dUwtBnhqhQLr3FuRseArKG1A@mail.gmail.com>","date":"2023-07-17T11:43:43","subject":"Re: [libcamera-devel] [PATCH v2 8/9] libcamera: Move to use\n\tCameraConfiguration::orientation","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for the patch.\n\nOn Fri, 14 Jul 2023 at 15:16, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n> Replace the usage of CameraConfiguration::transform with the newly\n> introduced CameraConfiguration::orientation.\n>\n> Rework and rename the CameraSensor::validateTransform(transform) to\n> CameraSensor::computeTransform(orientation), that given the desired\n> image orientation computes the Transform that pipeline handlers should\n> apply to the sensor to obtain it.\n>\n> Port all pipeline handlers to use the newly introduced function.\n>\n> This commit breaks existing applications as it removes the public\n> CameraConfiguration::transform in favour of\n> CameraConfiguration::orientation.\n>\n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  include/libcamera/camera.h                    |  1 -\n>  include/libcamera/internal/camera_sensor.h    |  3 +-\n>  src/libcamera/camera.cpp                      | 15 +--\n>  src/libcamera/camera_sensor.cpp               | 92 +++++++++----------\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>  10 files changed, 65 insertions(+), 83 deletions(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 55359d53f2ab..d937ffc24f54 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -80,7 +80,6 @@ public:\n>         bool empty() const;\n>         std::size_t size() const;\n>\n> -       Transform transform;\n>         Orientation orientation;\n>\n>  protected:\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 02b4b4d25e6d..1d47a2b1a500 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -14,6 +14,7 @@\n>  #include <libcamera/base/class.h>\n>  #include <libcamera/base/log.h>\n>\n> +#include <libcamera/camera.h>\n>  #include <libcamera/control_ids.h>\n>  #include <libcamera/controls.h>\n>  #include <libcamera/geometry.h>\n> @@ -71,7 +72,7 @@ public:\n>\n>         CameraLens *focusLens() { return focusLens_.get(); }\n>\n> -       Transform validateTransform(Transform *transform) const;\n> +       Transform computeTransform(CameraConfiguration::Orientation *orientation) const;\n>\n>  protected:\n>         std::string logPrefix() const override;\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index af842e70dcb0..3d52d1a0019a 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -224,7 +224,7 @@ LOG_DECLARE_CATEGORY(Camera)\n>   * \\brief Create an empty camera configuration\n>   */\n>  CameraConfiguration::CameraConfiguration()\n> -       : transform(Transform::Identity), orientation(rotate0), config_({})\n> +       : orientation(rotate0), config_({})\n>  {\n>  }\n>\n> @@ -476,19 +476,6 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n>         return status;\n>  }\n>\n> -/**\n> - * \\var CameraConfiguration::transform\n> - * \\brief User-specified transform to be applied to the image\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.\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> - * may adjust this field at its discretion if the selection is not supported.\n> - */\n> -\n>  /**\n>   * \\var CameraConfiguration::orientation\n>   * \\brief The desired orientation of the images produced by the camera\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 3ba364c44a40..ac81ea3889f7 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -465,7 +465,7 @@ int CameraSensor::initProperties()\n>\n>                 /*\n>                  * Cache the Transform associated with the camera mounting\n> -                * rotation for later use in validateTransform().\n> +                * rotation for later use in computeTransform().\n>                  */\n>                 bool success;\n>                 rotationTransform_ = transformFromRotation(propertyValue, &success);\n> @@ -1024,69 +1024,65 @@ void CameraSensor::updateControlInfo()\n>   */\n>\n>  /**\n> - * \\brief Validate a transform request against the sensor capabilities\n> - * \\param[inout] transform The requested transformation, updated to match\n> - * the sensor capabilities\n> + * \\brief Compute the Transform that gives the requested \\a orientation\n> + * \\param[inout] orientation The desired image orientation\n>   *\n> - * The input \\a transform is the transform that the caller wants, and it is\n> - * adjusted according to the capabilities of the sensor to represent the\n> - * \"nearest\" transform that can actually be delivered.\n> + * This function computes the Transform that the pipeline handler should apply\n> + * to the CameraSensor to obtain the requested \\a orientation.\n>   *\n> - * The returned Transform is the transform applied to the sensor in order to\n> - * produce the input \\a transform, It is also validated against the sensor's\n> - * ability to perform horizontal and vertical flips.\n> + * The intended caller of this function is the validate() implementation of\n> + * pipeline handlers, that pass in the application requested\n> + * CameraConfiguration::orientation and obtain a Transform to apply to the\n> + * camera sensor, likely at configure() time.\n>   *\n> - * For example, if the requested \\a transform is Transform::Identity and the\n> - * sensor rotation is 180 degrees, the output transform will be\n> - * Transform::Rot180 to correct the images so that they appear to have\n> - * Transform::Identity, but only if the sensor can apply horizontal and vertical\n> - * flips.\n> + * If the requested \\a orientation cannot be obtained, the \\a orientation\n> + * parameter is adjusted to report the current image orientation and\n> + * Transform::Identity is returned.\n>   *\n> - * \\return A Transform instance that represents which transformation has been\n> - * applied to the camera sensor\n> + * If the requested \\a orientation can be obtained, the function computes a\n> + * Transform and does not adjust \\a orientation.\n> + *\n> + * Pipeline handlers are expected to verify if \\a orientation has been\n> + * adjusted by this function and set the CameraConfiguration::status to\n> + * Adjusted accordingly.\n> + *\n> + * \\return A Transform instance that applied to the CameraSensor produces images\n> + * with \\a orientation\n>   */\n> -Transform CameraSensor::validateTransform(Transform *transform) const\n> +Transform CameraSensor::computeTransform(CameraConfiguration::Orientation *orientation) const\n>  {\n> +       CameraConfiguration::Orientation mountingOrientation =\n> +                               transformToOrientation(rotationTransform_);\n> +\n>         /*\n> -        * Combine the requested transform to compensate the sensor mounting\n> -        * rotation.\n> +        * We cannot do any flips: we cannot change the native camera mounting\n> +        * orientation.\n>          */\n> -       Transform combined = *transform * rotationTransform_;\n> +       if (!supportFlips_) {\n> +               *orientation = mountingOrientation;\n> +               return Transform::Identity;\n> +       }\n>\n>         /*\n> -        * We combine the platform and user transform, but must \"adjust away\"\n> -        * any combined result that includes a transposition, as we can't do\n> -        * those. In this case, flipping only the transpose bit is helpful to\n> -        * applications - they either get the transform they requested, or have\n> -        * to do a simple transpose themselves (they don't have to worry about\n> -        * the other possible cases).\n> +        * Now compute the required transform to obtain 'orientation' starting\n> +        * from the mounting rotation.\n> +        *\n> +        * As a note:\n> +        *      orientation / mountingOrientation = transform\n> +        *      mountingOrientation * transform = orientation\n>          */\n> -       if (!!(combined & Transform::Transpose)) {\n> -               /*\n> -                * Flipping the transpose bit in \"transform\" flips it in the\n> -                * combined result too (as it's the last thing that happens),\n> -                * which is of course clearing it.\n> -                */\n> -               *transform ^= Transform::Transpose;\n> -               combined &= ~Transform::Transpose;\n> -       }\n> +       Transform transform = *orientation / mountingOrientation;\n>\n>         /*\n> -        * We also check if the sensor doesn't do h/vflips at all, in which\n> -        * case we clear them, and the application will have to do everything.\n> +        * If transform contains any Transpose we cannot do it, so adjust\n> +        * 'orientation' to report the image native orientation and return Identity.\n>          */\n> -       if (!supportFlips_ && !!combined) {\n> -               /*\n> -                * If the sensor can do no transforms, then combined must be\n> -                * changed to the identity. The only user transform that gives\n> -                * rise to this is the inverse of the rotation. (Recall that\n> -                * combined = transform * rotationTransform.)\n> -                */\n> -               *transform = -rotationTransform_;\n> -               combined = Transform::Identity;\n> +       if (!!(transform & Transform::Transpose)) {\n> +               *orientation = mountingOrientation;\n> +               return Transform::Identity;\n\nI guess I wonder if in future we'll encounter ISPs that can do\ntranspose, but I'm happy to ignore the question for now!\n\n>         }\n>\n> -       return combined;\n> +       return transform;\n>  }\n>\n>  std::string CameraSensor::logPrefix() const\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index a81c817a1255..fa4bd0bb73e2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -187,9 +187,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n>          * rotation and store the final combined transform that configure() will\n>          * need to apply to the sensor to save us working it out again.\n>          */\n> -       Transform requestedTransform = transform;\n> -       combinedTransform_ = data_->cio2_.sensor()->validateTransform(&transform);\n> -       if (transform != requestedTransform)\n> +       Orientation requestedOrientation = orientation;\n> +       combinedTransform_ = data_->cio2_.sensor()->computeTransform(&orientation);\n> +       if (orientation != requestedOrientation)\n>                 status = Adjusted;\n>\n>         /* Cap the number of entries to the available streams. */\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 6efa79f29f66..586b46d64630 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -481,9 +481,9 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>                 status = Adjusted;\n>         }\n>\n> -       Transform requestedTransform = transform;\n> -       Transform combined = sensor->validateTransform(&transform);\n> -       if (transform != requestedTransform)\n> +       Orientation requestedOrientation = orientation;\n> +       combinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> +       if (orientation != requestedOrientation)\n>                 status = Adjusted;\n>\n>         /*\n> @@ -595,8 +595,6 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>         if (sensorFormat_.size.isNull())\n>                 sensorFormat_.size = sensor->resolution();\n>\n> -       combinedTransform_ = combined;\n> -\n>         return status;\n>  }\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 179a5b81a516..5d541d71defe 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -254,9 +254,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>          * rotation and store the final combined transform that configure() will\n>          * need to apply to the sensor to save us working it out again.\n>          */\n> -       Transform requestedTransform = transform;\n> -       combinedTransform_ = data_->sensor_->validateTransform(&transform);\n> -       if (transform != requestedTransform)\n> +       Orientation requestedOrientation = orientation;\n> +       combinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> +       if (orientation != requestedOrientation)\n>                 status = Adjusted;\n\nI wonder slightly about restoring the behaviour the Pi PH had\npreviously, but I'm happy to ignore it for now and will add a further\ncommit at a later date if it is needed.\n\nBut overall I think this is a good improvement and shows that\norientations and transforms make sense together (putting aside my\nwobbles over transform ordering, though I think those do need\naddressing).\n\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks!\nDavid\n\n>\n>         std::vector<CameraData::StreamParams> rawStreams, outStreams;\n> @@ -1193,7 +1193,8 @@ int CameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::Config\n>         }\n>\n>         /* Always send the user transform to the IPA. */\n> -       params.transform = static_cast<unsigned int>(config->transform);\n> +       params.transform =\n> +               static_cast<unsigned int>(transformFromOrientation(config->orientation));\n>\n>         /* Ready the IPA - it must know about the sensor resolution. */\n>         ret = ipa_->configure(sensorInfo_, params, result);\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 05ba76bce630..911051b28498 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -889,9 +889,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>         if (config_.empty())\n>                 return Invalid;\n>\n> -       Transform requestedTransform = transform;\n> -       combinedTransform_ = sensor->validateTransform(&transform);\n> -       if (transform != requestedTransform)\n> +       Orientation requestedOrientation = orientation;\n> +       combinedTransform_ = sensor->computeTransform(&orientation);\n> +       if (orientation != requestedOrientation)\n>                 status = Adjusted;\n>\n>         /* Cap the number of entries to the available streams. */\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 02a6117c7955..6f9cf0dca6f5 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -112,8 +112,8 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n>         if (config_.empty())\n>                 return Invalid;\n>\n> -       if (transform != Transform::Identity) {\n> -               transform = Transform::Identity;\n> +       if (orientation != CameraConfiguration::rotate0) {\n> +               orientation = CameraConfiguration::rotate0;\n>                 status = Adjusted;\n>         }\n>\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 00e6f4c6a51f..26401ab92d15 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -128,8 +128,8 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()\n>         if (config_.empty())\n>                 return Invalid;\n>\n> -       if (transform != Transform::Identity) {\n> -               transform = Transform::Identity;\n> +       if (orientation != CameraConfiguration::rotate0) {\n> +               orientation = CameraConfiguration::rotate0;\n>                 status = Adjusted;\n>         }\n>\n> --\n> 2.40.1\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 5843EBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Jul 2023 11:43:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E583628C0;\n\tMon, 17 Jul 2023 13:43:56 +0200 (CEST)","from mail-oa1-x31.google.com (mail-oa1-x31.google.com\n\t[IPv6:2001:4860:4864:20::31])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 923AB60383\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Jul 2023 13:43:55 +0200 (CEST)","by mail-oa1-x31.google.com with SMTP id\n\t586e51a60fabf-1b06da65bdbso3294840fac.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Jul 2023 04:43:55 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1689594236;\n\tbh=HIqhh7YY192IAg9dq7tvGGLmwOHN51xtP2lar0eE6HI=;\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=APwyiCssxEtApVQZ0P8mJqWQDJKypfpSE8ujXuk0wW1uCpNgU8e+BDz3uQWUkLNWu\n\tCSjt1TKSr6rmELp0LEylAn8axR9sTVJ29Bn+NA4s/kyWob242m2ILjja88bUO+N90g\n\tpEzvJx0Y/Tbwv4jiZkMkkp9Vifkn3N/ZM6t7CPQ11hb2lBKKmLVtzEqzDQFcWOMzW/\n\tpiDwGreKaFN/CKA5DXpA1X0hrzxI9+fAwftJrxChgjqrm57HSESo1eXvxjTVV1q/Ej\n\tqf45ob7X97cjwUxUazGmfE3iQEfh97Kxe8wnJfZYdKMVN7McoOXQ9rdZMmM1/qePiq\n\tYuVIZGSHBIWoQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1689594234; x=1692186234;\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=lYFFloR8Hyh3Pr7c+TxyQZI9IIJpxvdDv6j96UFHvwQ=;\n\tb=GbdkPMISBwD3mDPoG9B5ZUPoKbYHC562+AzF99iS6DPQhGs6hhHtj3IaIKdd7oicsd\n\tOAt5/MkDZWVAC6L6lNwWKUgFBhieTqlx3tnSno4JKcX3Qr3a2b4YI3AazdVzuajYTsJc\n\t3StDnr4Gp6Ha1woXIUXSlw2yVpT1Y8O2NpE1hwKLDAckkk5GxOWwtr/59pWRgVX3OjbA\n\tupb+ir4N/9H8pXCRx0QzOBZjqEcLr2WJ+G17VzJ5L/oG6vFaqig+3UAOUxZuC2gyUBoO\n\tN7eURZ8QYO1W5E3A3t1mcRWgpkWjpccVvHy/7hi1XXTiKzS+dC6pwAuQWbpM6bw10ZjW\n\tBg7Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"GbdkPMIS\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1689594234; x=1692186234;\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=lYFFloR8Hyh3Pr7c+TxyQZI9IIJpxvdDv6j96UFHvwQ=;\n\tb=Rh+v2XVMllyxN4yYe+Zz/42CpX3xyZ8T0AST81oxQGQgwbN4xQIJ/2ZDGvIauWrla/\n\t64knsAk4JNe/q4KFlGXyfTbF4CPV4WUuReJTzVtH9aYKCOlzvkoKi1wWaox/hmZ2q2HQ\n\tD6Ic067aKiNDyBkwMHep4PvJr55oCmC76vmZy2iEsM7oKk+8E8G2VyqOUzVWP7ZBP9XQ\n\t3ERzYCexWGD9tdF6mkIoRndJOnx8A7An6LPzO7MMsj15pAO/ljopDG1nGPttPa2eB+d2\n\tEBg5tH7tAdR7FwwP6iXVeu/hp83IaYsyogYYFByCa1zyEjLyvZLDyw0aNvXNIKiF6MuK\n\thicw==","X-Gm-Message-State":"ABy/qLYm31wyv2hsLW+j91zi7nkB7ooDMckqhkK9iKJ2dz6FbNMbpNRM\n\thIRO+wcbxb89vVyQzdrTp0HQT/8J3n9W+BKPz7GDdA==","X-Google-Smtp-Source":"APBJJlGsjpWViSB4NQpNS5lLEQX/VScmzanlsfUPUohyht9o/vaHyJ56ROlTIIKq7n+e6hkkYx36ae6DglJfSYPXDHo=","X-Received":"by 2002:a05:6870:fb87:b0:19e:7d07:ab9 with SMTP id\n\tkv7-20020a056870fb8700b0019e7d070ab9mr10846082oab.53.1689594234187;\n\tMon, 17 Jul 2023 04:43:54 -0700 (PDT)","MIME-Version":"1.0","References":"<20230714141549.11085-1-jacopo.mondi@ideasonboard.com>\n\t<20230714141549.11085-9-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230714141549.11085-9-jacopo.mondi@ideasonboard.com>","Date":"Mon, 17 Jul 2023 12:43:43 +0100","Message-ID":"<CAHW6GYKmvSh9oZcFJB48oQHsw2dUwtBnhqhQLr3FuRseArKG1A@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 8/9] libcamera: Move to use\n\tCameraConfiguration::orientation","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>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27563,"web_url":"https://patchwork.libcamera.org/comment/27563/","msgid":"<dobeishhzxv6ouax4ougzczq4gfprcofczrbrnwtc2lrjbldwc@i2eaefvhw5t7>","date":"2023-07-17T13:29:59","subject":"Re: [libcamera-devel] [PATCH v2 8/9] libcamera: Move to use\n\tCameraConfiguration::orientation","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi David\n\nOn Mon, Jul 17, 2023 at 12:43:43PM +0100, David Plowman via libcamera-devel wrote:\n> Hi Jacopo\n>\n> Thanks for the patch.\n>\n> On Fri, 14 Jul 2023 at 15:16, Jacopo Mondi\n> <jacopo.mondi@ideasonboard.com> wrote:\n> >\n> > Replace the usage of CameraConfiguration::transform with the newly\n> > introduced CameraConfiguration::orientation.\n> >\n> > Rework and rename the CameraSensor::validateTransform(transform) to\n> > CameraSensor::computeTransform(orientation), that given the desired\n> > image orientation computes the Transform that pipeline handlers should\n> > apply to the sensor to obtain it.\n> >\n> > Port all pipeline handlers to use the newly introduced function.\n> >\n> > This commit breaks existing applications as it removes the public\n> > CameraConfiguration::transform in favour of\n> > CameraConfiguration::orientation.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > ---\n> >  include/libcamera/camera.h                    |  1 -\n> >  include/libcamera/internal/camera_sensor.h    |  3 +-\n> >  src/libcamera/camera.cpp                      | 15 +--\n> >  src/libcamera/camera_sensor.cpp               | 92 +++++++++----------\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> >  10 files changed, 65 insertions(+), 83 deletions(-)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 55359d53f2ab..d937ffc24f54 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -80,7 +80,6 @@ public:\n> >         bool empty() const;\n> >         std::size_t size() const;\n> >\n> > -       Transform transform;\n> >         Orientation orientation;\n> >\n> >  protected:\n> > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > index 02b4b4d25e6d..1d47a2b1a500 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -14,6 +14,7 @@\n> >  #include <libcamera/base/class.h>\n> >  #include <libcamera/base/log.h>\n> >\n> > +#include <libcamera/camera.h>\n> >  #include <libcamera/control_ids.h>\n> >  #include <libcamera/controls.h>\n> >  #include <libcamera/geometry.h>\n> > @@ -71,7 +72,7 @@ public:\n> >\n> >         CameraLens *focusLens() { return focusLens_.get(); }\n> >\n> > -       Transform validateTransform(Transform *transform) const;\n> > +       Transform computeTransform(CameraConfiguration::Orientation *orientation) const;\n> >\n> >  protected:\n> >         std::string logPrefix() const override;\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index af842e70dcb0..3d52d1a0019a 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -224,7 +224,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> >   * \\brief Create an empty camera configuration\n> >   */\n> >  CameraConfiguration::CameraConfiguration()\n> > -       : transform(Transform::Identity), orientation(rotate0), config_({})\n> > +       : orientation(rotate0), config_({})\n> >  {\n> >  }\n> >\n> > @@ -476,19 +476,6 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> >         return status;\n> >  }\n> >\n> > -/**\n> > - * \\var CameraConfiguration::transform\n> > - * \\brief User-specified transform to be applied to the image\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.\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> > - * may adjust this field at its discretion if the selection is not supported.\n> > - */\n> > -\n> >  /**\n> >   * \\var CameraConfiguration::orientation\n> >   * \\brief The desired orientation of the images produced by the camera\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 3ba364c44a40..ac81ea3889f7 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -465,7 +465,7 @@ int CameraSensor::initProperties()\n> >\n> >                 /*\n> >                  * Cache the Transform associated with the camera mounting\n> > -                * rotation for later use in validateTransform().\n> > +                * rotation for later use in computeTransform().\n> >                  */\n> >                 bool success;\n> >                 rotationTransform_ = transformFromRotation(propertyValue, &success);\n> > @@ -1024,69 +1024,65 @@ void CameraSensor::updateControlInfo()\n> >   */\n> >\n> >  /**\n> > - * \\brief Validate a transform request against the sensor capabilities\n> > - * \\param[inout] transform The requested transformation, updated to match\n> > - * the sensor capabilities\n> > + * \\brief Compute the Transform that gives the requested \\a orientation\n> > + * \\param[inout] orientation The desired image orientation\n> >   *\n> > - * The input \\a transform is the transform that the caller wants, and it is\n> > - * adjusted according to the capabilities of the sensor to represent the\n> > - * \"nearest\" transform that can actually be delivered.\n> > + * This function computes the Transform that the pipeline handler should apply\n> > + * to the CameraSensor to obtain the requested \\a orientation.\n> >   *\n> > - * The returned Transform is the transform applied to the sensor in order to\n> > - * produce the input \\a transform, It is also validated against the sensor's\n> > - * ability to perform horizontal and vertical flips.\n> > + * The intended caller of this function is the validate() implementation of\n> > + * pipeline handlers, that pass in the application requested\n> > + * CameraConfiguration::orientation and obtain a Transform to apply to the\n> > + * camera sensor, likely at configure() time.\n> >   *\n> > - * For example, if the requested \\a transform is Transform::Identity and the\n> > - * sensor rotation is 180 degrees, the output transform will be\n> > - * Transform::Rot180 to correct the images so that they appear to have\n> > - * Transform::Identity, but only if the sensor can apply horizontal and vertical\n> > - * flips.\n> > + * If the requested \\a orientation cannot be obtained, the \\a orientation\n> > + * parameter is adjusted to report the current image orientation and\n> > + * Transform::Identity is returned.\n> >   *\n> > - * \\return A Transform instance that represents which transformation has been\n> > - * applied to the camera sensor\n> > + * If the requested \\a orientation can be obtained, the function computes a\n> > + * Transform and does not adjust \\a orientation.\n> > + *\n> > + * Pipeline handlers are expected to verify if \\a orientation has been\n> > + * adjusted by this function and set the CameraConfiguration::status to\n> > + * Adjusted accordingly.\n> > + *\n> > + * \\return A Transform instance that applied to the CameraSensor produces images\n> > + * with \\a orientation\n> >   */\n> > -Transform CameraSensor::validateTransform(Transform *transform) const\n> > +Transform CameraSensor::computeTransform(CameraConfiguration::Orientation *orientation) const\n> >  {\n> > +       CameraConfiguration::Orientation mountingOrientation =\n> > +                               transformToOrientation(rotationTransform_);\n> > +\n> >         /*\n> > -        * Combine the requested transform to compensate the sensor mounting\n> > -        * rotation.\n> > +        * We cannot do any flips: we cannot change the native camera mounting\n> > +        * orientation.\n> >          */\n> > -       Transform combined = *transform * rotationTransform_;\n> > +       if (!supportFlips_) {\n> > +               *orientation = mountingOrientation;\n> > +               return Transform::Identity;\n> > +       }\n> >\n> >         /*\n> > -        * We combine the platform and user transform, but must \"adjust away\"\n> > -        * any combined result that includes a transposition, as we can't do\n> > -        * those. In this case, flipping only the transpose bit is helpful to\n> > -        * applications - they either get the transform they requested, or have\n> > -        * to do a simple transpose themselves (they don't have to worry about\n> > -        * the other possible cases).\n> > +        * Now compute the required transform to obtain 'orientation' starting\n> > +        * from the mounting rotation.\n> > +        *\n> > +        * As a note:\n> > +        *      orientation / mountingOrientation = transform\n> > +        *      mountingOrientation * transform = orientation\n> >          */\n> > -       if (!!(combined & Transform::Transpose)) {\n> > -               /*\n> > -                * Flipping the transpose bit in \"transform\" flips it in the\n> > -                * combined result too (as it's the last thing that happens),\n> > -                * which is of course clearing it.\n> > -                */\n> > -               *transform ^= Transform::Transpose;\n> > -               combined &= ~Transform::Transpose;\n> > -       }\n> > +       Transform transform = *orientation / mountingOrientation;\n> >\n> >         /*\n> > -        * We also check if the sensor doesn't do h/vflips at all, in which\n> > -        * case we clear them, and the application will have to do everything.\n> > +        * If transform contains any Transpose we cannot do it, so adjust\n> > +        * 'orientation' to report the image native orientation and return Identity.\n> >          */\n> > -       if (!supportFlips_ && !!combined) {\n> > -               /*\n> > -                * If the sensor can do no transforms, then combined must be\n> > -                * changed to the identity. The only user transform that gives\n> > -                * rise to this is the inverse of the rotation. (Recall that\n> > -                * combined = transform * rotationTransform.)\n> > -                */\n> > -               *transform = -rotationTransform_;\n> > -               combined = Transform::Identity;\n> > +       if (!!(transform & Transform::Transpose)) {\n> > +               *orientation = mountingOrientation;\n> > +               return Transform::Identity;\n>\n> I guess I wonder if in future we'll encounter ISPs that can do\n> transpose, but I'm happy to ignore the question for now!\n>\n\nThey won't rely on the CameraSensor class to do transform then :)\n\n> >         }\n> >\n> > -       return combined;\n> > +       return transform;\n> >  }\n> >\n> >  std::string CameraSensor::logPrefix() const\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index a81c817a1255..fa4bd0bb73e2 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -187,9 +187,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> >          * rotation and store the final combined transform that configure() will\n> >          * need to apply to the sensor to save us working it out again.\n> >          */\n> > -       Transform requestedTransform = transform;\n> > -       combinedTransform_ = data_->cio2_.sensor()->validateTransform(&transform);\n> > -       if (transform != requestedTransform)\n> > +       Orientation requestedOrientation = orientation;\n> > +       combinedTransform_ = data_->cio2_.sensor()->computeTransform(&orientation);\n> > +       if (orientation != requestedOrientation)\n> >                 status = Adjusted;\n> >\n> >         /* Cap the number of entries to the available streams. */\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 6efa79f29f66..586b46d64630 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -481,9 +481,9 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> >                 status = Adjusted;\n> >         }\n> >\n> > -       Transform requestedTransform = transform;\n> > -       Transform combined = sensor->validateTransform(&transform);\n> > -       if (transform != requestedTransform)\n> > +       Orientation requestedOrientation = orientation;\n> > +       combinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> > +       if (orientation != requestedOrientation)\n> >                 status = Adjusted;\n> >\n> >         /*\n> > @@ -595,8 +595,6 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> >         if (sensorFormat_.size.isNull())\n> >                 sensorFormat_.size = sensor->resolution();\n> >\n> > -       combinedTransform_ = combined;\n> > -\n> >         return status;\n> >  }\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index 179a5b81a516..5d541d71defe 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -254,9 +254,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> >          * rotation and store the final combined transform that configure() will\n> >          * need to apply to the sensor to save us working it out again.\n> >          */\n> > -       Transform requestedTransform = transform;\n> > -       combinedTransform_ = data_->sensor_->validateTransform(&transform);\n> > -       if (transform != requestedTransform)\n> > +       Orientation requestedOrientation = orientation;\n> > +       combinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> > +       if (orientation != requestedOrientation)\n> >                 status = Adjusted;\n>\n> I wonder slightly about restoring the behaviour the Pi PH had\n> previously, but I'm happy to ignore it for now and will add a further\n> commit at a later date if it is needed.\n\nSorry, how is it different now ?\n\n>\n> But overall I think this is a good improvement and shows that\n> orientations and transforms make sense together (putting aside my\n> wobbles over transform ordering, though I think those do need\n> addressing).\n>\n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n>\n> Thanks!\n> David\n>\n> >\n> >         std::vector<CameraData::StreamParams> rawStreams, outStreams;\n> > @@ -1193,7 +1193,8 @@ int CameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::Config\n> >         }\n> >\n> >         /* Always send the user transform to the IPA. */\n> > -       params.transform = static_cast<unsigned int>(config->transform);\n> > +       params.transform =\n> > +               static_cast<unsigned int>(transformFromOrientation(config->orientation));\n> >\n> >         /* Ready the IPA - it must know about the sensor resolution. */\n> >         ret = ipa_->configure(sensorInfo_, params, result);\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 05ba76bce630..911051b28498 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -889,9 +889,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >         if (config_.empty())\n> >                 return Invalid;\n> >\n> > -       Transform requestedTransform = transform;\n> > -       combinedTransform_ = sensor->validateTransform(&transform);\n> > -       if (transform != requestedTransform)\n> > +       Orientation requestedOrientation = orientation;\n> > +       combinedTransform_ = sensor->computeTransform(&orientation);\n> > +       if (orientation != requestedOrientation)\n> >                 status = Adjusted;\n> >\n> >         /* Cap the number of entries to the available streams. */\n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 02a6117c7955..6f9cf0dca6f5 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -112,8 +112,8 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n> >         if (config_.empty())\n> >                 return Invalid;\n> >\n> > -       if (transform != Transform::Identity) {\n> > -               transform = Transform::Identity;\n> > +       if (orientation != CameraConfiguration::rotate0) {\n> > +               orientation = CameraConfiguration::rotate0;\n> >                 status = Adjusted;\n> >         }\n> >\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 00e6f4c6a51f..26401ab92d15 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -128,8 +128,8 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()\n> >         if (config_.empty())\n> >                 return Invalid;\n> >\n> > -       if (transform != Transform::Identity) {\n> > -               transform = Transform::Identity;\n> > +       if (orientation != CameraConfiguration::rotate0) {\n> > +               orientation = CameraConfiguration::rotate0;\n> >                 status = Adjusted;\n> >         }\n> >\n> > --\n> > 2.40.1\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 D9BAABDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Jul 2023 13:30:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 36B2C628C0;\n\tMon, 17 Jul 2023 15:30:09 +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 C5B1360383\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Jul 2023 15:30:07 +0200 (CEST)","from ideasonboard.com (mob-5-90-54-150.net.vodafone.it\n\t[5.90.54.150])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E5167331B;\n\tMon, 17 Jul 2023 15:29:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1689600609;\n\tbh=i6y1PYiriy1EcFOeUtvvkDOgBub/7FUfvqdHqhe/8QY=;\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=RZX+pUVVqKZYwebeedpCviLdVJlDPdsGIFZ7bfWspQByp2dkLKCxAqmhqY5PGtIhw\n\tZOlf1NrQbrMExfSP31MPjbdcvsgOd5Yjupe7RQVpHkdslSjLNBa7khTOYpU11tZoS8\n\tH4TZR0G0klwjFrswMxHcdQLji6U/OtrYLnOJgtIbGD/dma5UErFx7DzMDs+rWes7JT\n\tTdxfY17DciGf1oz2wHt+uWB5pKl9ypViFawbkV9R4tUf/HVAaSvI+AeBjLEjyPWX5+\n\tuKoOAyg3KfMy3vuUDTgBxCeobODBS8oivUBNccGuVjMZAXZBTJbeKp27NnFTfcVfwS\n\tOzw4OlG1iOjrg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1689600554;\n\tbh=i6y1PYiriy1EcFOeUtvvkDOgBub/7FUfvqdHqhe/8QY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ALFtYDQep/bEkzq31xQ0HcBFAo14Agew3fhB5Va00ZHE4E7+KxXEcugoM5jmMELIe\n\tNt3Wwhor9+SCkZAuD9JKNO2nZ55VZMbrvqk8QS5zd3g4lmctg3IRxq9KWBqxIYLKq3\n\tYgLJPwhDWyaTvJ+yoAospsoDt6fqVJipTjSyH01Q="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ALFtYDQe\"; dkim-atps=neutral","Date":"Mon, 17 Jul 2023 15:29:59 +0200","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<dobeishhzxv6ouax4ougzczq4gfprcofczrbrnwtc2lrjbldwc@i2eaefvhw5t7>","References":"<20230714141549.11085-1-jacopo.mondi@ideasonboard.com>\n\t<20230714141549.11085-9-jacopo.mondi@ideasonboard.com>\n\t<CAHW6GYKmvSh9oZcFJB48oQHsw2dUwtBnhqhQLr3FuRseArKG1A@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAHW6GYKmvSh9oZcFJB48oQHsw2dUwtBnhqhQLr3FuRseArKG1A@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 8/9] libcamera: Move to use\n\tCameraConfiguration::orientation","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27566,"web_url":"https://patchwork.libcamera.org/comment/27566/","msgid":"<CAHW6GYJ=bPczHYCt5i17CrHpF83c3Po_RXwtYSUzb=4Vh+Cw9Q@mail.gmail.com>","date":"2023-07-17T13:42:27","subject":"Re: [libcamera-devel] [PATCH v2 8/9] libcamera: Move to use\n\tCameraConfiguration::orientation","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"HI Jacopo\n\nOn Mon, 17 Jul 2023 at 14:30, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n> Hi David\n>\n> On Mon, Jul 17, 2023 at 12:43:43PM +0100, David Plowman via libcamera-devel wrote:\n> > Hi Jacopo\n> >\n> > Thanks for the patch.\n> >\n> > On Fri, 14 Jul 2023 at 15:16, Jacopo Mondi\n> > <jacopo.mondi@ideasonboard.com> wrote:\n> > >\n> > > Replace the usage of CameraConfiguration::transform with the newly\n> > > introduced CameraConfiguration::orientation.\n> > >\n> > > Rework and rename the CameraSensor::validateTransform(transform) to\n> > > CameraSensor::computeTransform(orientation), that given the desired\n> > > image orientation computes the Transform that pipeline handlers should\n> > > apply to the sensor to obtain it.\n> > >\n> > > Port all pipeline handlers to use the newly introduced function.\n> > >\n> > > This commit breaks existing applications as it removes the public\n> > > CameraConfiguration::transform in favour of\n> > > CameraConfiguration::orientation.\n> > >\n> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > > ---\n> > >  include/libcamera/camera.h                    |  1 -\n> > >  include/libcamera/internal/camera_sensor.h    |  3 +-\n> > >  src/libcamera/camera.cpp                      | 15 +--\n> > >  src/libcamera/camera_sensor.cpp               | 92 +++++++++----------\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> > >  10 files changed, 65 insertions(+), 83 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > index 55359d53f2ab..d937ffc24f54 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -80,7 +80,6 @@ public:\n> > >         bool empty() const;\n> > >         std::size_t size() const;\n> > >\n> > > -       Transform transform;\n> > >         Orientation orientation;\n> > >\n> > >  protected:\n> > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > > index 02b4b4d25e6d..1d47a2b1a500 100644\n> > > --- a/include/libcamera/internal/camera_sensor.h\n> > > +++ b/include/libcamera/internal/camera_sensor.h\n> > > @@ -14,6 +14,7 @@\n> > >  #include <libcamera/base/class.h>\n> > >  #include <libcamera/base/log.h>\n> > >\n> > > +#include <libcamera/camera.h>\n> > >  #include <libcamera/control_ids.h>\n> > >  #include <libcamera/controls.h>\n> > >  #include <libcamera/geometry.h>\n> > > @@ -71,7 +72,7 @@ public:\n> > >\n> > >         CameraLens *focusLens() { return focusLens_.get(); }\n> > >\n> > > -       Transform validateTransform(Transform *transform) const;\n> > > +       Transform computeTransform(CameraConfiguration::Orientation *orientation) const;\n> > >\n> > >  protected:\n> > >         std::string logPrefix() const override;\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index af842e70dcb0..3d52d1a0019a 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -224,7 +224,7 @@ LOG_DECLARE_CATEGORY(Camera)\n> > >   * \\brief Create an empty camera configuration\n> > >   */\n> > >  CameraConfiguration::CameraConfiguration()\n> > > -       : transform(Transform::Identity), orientation(rotate0), config_({})\n> > > +       : orientation(rotate0), config_({})\n> > >  {\n> > >  }\n> > >\n> > > @@ -476,19 +476,6 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> > >         return status;\n> > >  }\n> > >\n> > > -/**\n> > > - * \\var CameraConfiguration::transform\n> > > - * \\brief User-specified transform to be applied to the image\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.\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> > > - * may adjust this field at its discretion if the selection is not supported.\n> > > - */\n> > > -\n> > >  /**\n> > >   * \\var CameraConfiguration::orientation\n> > >   * \\brief The desired orientation of the images produced by the camera\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index 3ba364c44a40..ac81ea3889f7 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -465,7 +465,7 @@ int CameraSensor::initProperties()\n> > >\n> > >                 /*\n> > >                  * Cache the Transform associated with the camera mounting\n> > > -                * rotation for later use in validateTransform().\n> > > +                * rotation for later use in computeTransform().\n> > >                  */\n> > >                 bool success;\n> > >                 rotationTransform_ = transformFromRotation(propertyValue, &success);\n> > > @@ -1024,69 +1024,65 @@ void CameraSensor::updateControlInfo()\n> > >   */\n> > >\n> > >  /**\n> > > - * \\brief Validate a transform request against the sensor capabilities\n> > > - * \\param[inout] transform The requested transformation, updated to match\n> > > - * the sensor capabilities\n> > > + * \\brief Compute the Transform that gives the requested \\a orientation\n> > > + * \\param[inout] orientation The desired image orientation\n> > >   *\n> > > - * The input \\a transform is the transform that the caller wants, and it is\n> > > - * adjusted according to the capabilities of the sensor to represent the\n> > > - * \"nearest\" transform that can actually be delivered.\n> > > + * This function computes the Transform that the pipeline handler should apply\n> > > + * to the CameraSensor to obtain the requested \\a orientation.\n> > >   *\n> > > - * The returned Transform is the transform applied to the sensor in order to\n> > > - * produce the input \\a transform, It is also validated against the sensor's\n> > > - * ability to perform horizontal and vertical flips.\n> > > + * The intended caller of this function is the validate() implementation of\n> > > + * pipeline handlers, that pass in the application requested\n> > > + * CameraConfiguration::orientation and obtain a Transform to apply to the\n> > > + * camera sensor, likely at configure() time.\n> > >   *\n> > > - * For example, if the requested \\a transform is Transform::Identity and the\n> > > - * sensor rotation is 180 degrees, the output transform will be\n> > > - * Transform::Rot180 to correct the images so that they appear to have\n> > > - * Transform::Identity, but only if the sensor can apply horizontal and vertical\n> > > - * flips.\n> > > + * If the requested \\a orientation cannot be obtained, the \\a orientation\n> > > + * parameter is adjusted to report the current image orientation and\n> > > + * Transform::Identity is returned.\n> > >   *\n> > > - * \\return A Transform instance that represents which transformation has been\n> > > - * applied to the camera sensor\n> > > + * If the requested \\a orientation can be obtained, the function computes a\n> > > + * Transform and does not adjust \\a orientation.\n> > > + *\n> > > + * Pipeline handlers are expected to verify if \\a orientation has been\n> > > + * adjusted by this function and set the CameraConfiguration::status to\n> > > + * Adjusted accordingly.\n> > > + *\n> > > + * \\return A Transform instance that applied to the CameraSensor produces images\n> > > + * with \\a orientation\n> > >   */\n> > > -Transform CameraSensor::validateTransform(Transform *transform) const\n> > > +Transform CameraSensor::computeTransform(CameraConfiguration::Orientation *orientation) const\n> > >  {\n> > > +       CameraConfiguration::Orientation mountingOrientation =\n> > > +                               transformToOrientation(rotationTransform_);\n> > > +\n> > >         /*\n> > > -        * Combine the requested transform to compensate the sensor mounting\n> > > -        * rotation.\n> > > +        * We cannot do any flips: we cannot change the native camera mounting\n> > > +        * orientation.\n> > >          */\n> > > -       Transform combined = *transform * rotationTransform_;\n> > > +       if (!supportFlips_) {\n> > > +               *orientation = mountingOrientation;\n> > > +               return Transform::Identity;\n> > > +       }\n> > >\n> > >         /*\n> > > -        * We combine the platform and user transform, but must \"adjust away\"\n> > > -        * any combined result that includes a transposition, as we can't do\n> > > -        * those. In this case, flipping only the transpose bit is helpful to\n> > > -        * applications - they either get the transform they requested, or have\n> > > -        * to do a simple transpose themselves (they don't have to worry about\n> > > -        * the other possible cases).\n> > > +        * Now compute the required transform to obtain 'orientation' starting\n> > > +        * from the mounting rotation.\n> > > +        *\n> > > +        * As a note:\n> > > +        *      orientation / mountingOrientation = transform\n> > > +        *      mountingOrientation * transform = orientation\n> > >          */\n> > > -       if (!!(combined & Transform::Transpose)) {\n> > > -               /*\n> > > -                * Flipping the transpose bit in \"transform\" flips it in the\n> > > -                * combined result too (as it's the last thing that happens),\n> > > -                * which is of course clearing it.\n> > > -                */\n> > > -               *transform ^= Transform::Transpose;\n> > > -               combined &= ~Transform::Transpose;\n> > > -       }\n> > > +       Transform transform = *orientation / mountingOrientation;\n> > >\n> > >         /*\n> > > -        * We also check if the sensor doesn't do h/vflips at all, in which\n> > > -        * case we clear them, and the application will have to do everything.\n> > > +        * If transform contains any Transpose we cannot do it, so adjust\n> > > +        * 'orientation' to report the image native orientation and return Identity.\n> > >          */\n> > > -       if (!supportFlips_ && !!combined) {\n> > > -               /*\n> > > -                * If the sensor can do no transforms, then combined must be\n> > > -                * changed to the identity. The only user transform that gives\n> > > -                * rise to this is the inverse of the rotation. (Recall that\n> > > -                * combined = transform * rotationTransform.)\n> > > -                */\n> > > -               *transform = -rotationTransform_;\n> > > -               combined = Transform::Identity;\n> > > +       if (!!(transform & Transform::Transpose)) {\n> > > +               *orientation = mountingOrientation;\n> > > +               return Transform::Identity;\n> >\n> > I guess I wonder if in future we'll encounter ISPs that can do\n> > transpose, but I'm happy to ignore the question for now!\n> >\n>\n> They won't rely on the CameraSensor class to do transform then :)\n\nYes, I guess that's a fair point!\n\n>\n> > >         }\n> > >\n> > > -       return combined;\n> > > +       return transform;\n> > >  }\n> > >\n> > >  std::string CameraSensor::logPrefix() const\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index a81c817a1255..fa4bd0bb73e2 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -187,9 +187,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> > >          * rotation and store the final combined transform that configure() will\n> > >          * need to apply to the sensor to save us working it out again.\n> > >          */\n> > > -       Transform requestedTransform = transform;\n> > > -       combinedTransform_ = data_->cio2_.sensor()->validateTransform(&transform);\n> > > -       if (transform != requestedTransform)\n> > > +       Orientation requestedOrientation = orientation;\n> > > +       combinedTransform_ = data_->cio2_.sensor()->computeTransform(&orientation);\n> > > +       if (orientation != requestedOrientation)\n> > >                 status = Adjusted;\n> > >\n> > >         /* Cap the number of entries to the available streams. */\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 6efa79f29f66..586b46d64630 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -481,9 +481,9 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > >                 status = Adjusted;\n> > >         }\n> > >\n> > > -       Transform requestedTransform = transform;\n> > > -       Transform combined = sensor->validateTransform(&transform);\n> > > -       if (transform != requestedTransform)\n> > > +       Orientation requestedOrientation = orientation;\n> > > +       combinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> > > +       if (orientation != requestedOrientation)\n> > >                 status = Adjusted;\n> > >\n> > >         /*\n> > > @@ -595,8 +595,6 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > >         if (sensorFormat_.size.isNull())\n> > >                 sensorFormat_.size = sensor->resolution();\n> > >\n> > > -       combinedTransform_ = combined;\n> > > -\n> > >         return status;\n> > >  }\n> > >\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > index 179a5b81a516..5d541d71defe 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > @@ -254,9 +254,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > >          * rotation and store the final combined transform that configure() will\n> > >          * need to apply to the sensor to save us working it out again.\n> > >          */\n> > > -       Transform requestedTransform = transform;\n> > > -       combinedTransform_ = data_->sensor_->validateTransform(&transform);\n> > > -       if (transform != requestedTransform)\n> > > +       Orientation requestedOrientation = orientation;\n> > > +       combinedTransform_ = data_->sensor_->computeTransform(&orientation);\n> > > +       if (orientation != requestedOrientation)\n> > >                 status = Adjusted;\n> >\n> > I wonder slightly about restoring the behaviour the Pi PH had\n> > previously, but I'm happy to ignore it for now and will add a further\n> > commit at a later date if it is needed.\n>\n> Sorry, how is it different now ?\n\nPreviously things were arranged on the Pi so that the residual\ntransform for the application to do would always just be Transpose,\nbut with the orientation/transform classes I can easily arrange that\nfor myself (if I still want to). So I am fine with the code here.\n\nDavid\n\n>\n> >\n> > But overall I think this is a good improvement and shows that\n> > orientations and transforms make sense together (putting aside my\n> > wobbles over transform ordering, though I think those do need\n> > addressing).\n> >\n> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> >\n> > Thanks!\n> > David\n> >\n> > >\n> > >         std::vector<CameraData::StreamParams> rawStreams, outStreams;\n> > > @@ -1193,7 +1193,8 @@ int CameraData::configureIPA(const CameraConfiguration *config, ipa::RPi::Config\n> > >         }\n> > >\n> > >         /* Always send the user transform to the IPA. */\n> > > -       params.transform = static_cast<unsigned int>(config->transform);\n> > > +       params.transform =\n> > > +               static_cast<unsigned int>(transformFromOrientation(config->orientation));\n> > >\n> > >         /* Ready the IPA - it must know about the sensor resolution. */\n> > >         ret = ipa_->configure(sensorInfo_, params, result);\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 05ba76bce630..911051b28498 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -889,9 +889,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> > >         if (config_.empty())\n> > >                 return Invalid;\n> > >\n> > > -       Transform requestedTransform = transform;\n> > > -       combinedTransform_ = sensor->validateTransform(&transform);\n> > > -       if (transform != requestedTransform)\n> > > +       Orientation requestedOrientation = orientation;\n> > > +       combinedTransform_ = sensor->computeTransform(&orientation);\n> > > +       if (orientation != requestedOrientation)\n> > >                 status = Adjusted;\n> > >\n> > >         /* Cap the number of entries to the available streams. */\n> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > index 02a6117c7955..6f9cf0dca6f5 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > @@ -112,8 +112,8 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n> > >         if (config_.empty())\n> > >                 return Invalid;\n> > >\n> > > -       if (transform != Transform::Identity) {\n> > > -               transform = Transform::Identity;\n> > > +       if (orientation != CameraConfiguration::rotate0) {\n> > > +               orientation = CameraConfiguration::rotate0;\n> > >                 status = Adjusted;\n> > >         }\n> > >\n> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > index 00e6f4c6a51f..26401ab92d15 100644\n> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > @@ -128,8 +128,8 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()\n> > >         if (config_.empty())\n> > >                 return Invalid;\n> > >\n> > > -       if (transform != Transform::Identity) {\n> > > -               transform = Transform::Identity;\n> > > +       if (orientation != CameraConfiguration::rotate0) {\n> > > +               orientation = CameraConfiguration::rotate0;\n> > >                 status = Adjusted;\n> > >         }\n> > >\n> > > --\n> > > 2.40.1\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 F085BBDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Jul 2023 13:42:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E039628C1;\n\tMon, 17 Jul 2023 15:42:40 +0200 (CEST)","from mail-oi1-x232.google.com (mail-oi1-x232.google.com\n\t[IPv6:2607:f8b0:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 29EFB61E2C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Jul 2023 15:42:39 +0200 (CEST)","by mail-oi1-x232.google.com with SMTP id\n\t5614622812f47-3a43cbb4343so1717861b6e.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Jul 2023 06:42:39 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1689601360;\n\tbh=LAHKDkSh+TxJzj4X1pdpy0Ap9SktxFcx/J/+Vd0OmVY=;\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=gJT2T/U7aH+1lQeQoLQlj29TkL5pEAxMUknrnWeD8EVbDre2e7xAOqrwqtaSN3H+P\n\twNQe0W64wtMv1zlXbHM9MIni3Vh80ybwuPjv2Sh3LlYQDV/kn8JRrkJQPHoSGU9PF+\n\tuXTG+LP8frBr7RVbqzVK87vaLYRfvQ7YJpvSMhkCDyi+tJYGBrx8a1Qc5VtkOxI8QL\n\tLgj0ZBTfQEffv62VnL6Wvee4QxHy3chuk4hLB1JatuoG+UOl499oEkmcCnRvRJ79ot\n\tBMpyXyHBZa20Y91KlxiA0+joG96HfTCGKHcEPYkf5siCgX7B6Phsv5DrdhNww2kimh\n\tPXY08raMgk6LA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1689601358; x=1692193358;\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=o79KOp5j0WSvJueuB5JfwqlINhlWk+UaAzgQ7M0VC4M=;\n\tb=CVQwFdRKo3WI6E5UXP9Sia5nG8+zwXGsMyNuYsmgrHPRav/3SiLp8vbzP/+GeVhkai\n\t+n/cLfCnvU6Gh6lJiPvBiYHZnD/s+vJW+zd8N8+wgZMp6qZlMtSe0wOpTODBbYwpK4SU\n\twpKtHq4G+J9sRySQACngpp8EmfBbpsL8hPtNQoo+8Omrpy95ibITkF5g8t4nGyslSHVf\n\teQQwda3ZvTbEQr4K+6yzeW9Uq2uuQ20QPlCQ0tM36UFx5fzSG2kyRPI9YyDdtLLHpQ0u\n\tU6eWteafmxpi1Yn/diVN5+dEipq4F1A0h4de96jYuw1fCdLEfEr8Ju9RETLzNWvZeBn1\n\tZk8Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"CVQwFdRK\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1689601358; x=1692193358;\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=o79KOp5j0WSvJueuB5JfwqlINhlWk+UaAzgQ7M0VC4M=;\n\tb=JULJEqsIeGA1B8SQaL0s2YkgvcUdxFAulRb6YWdQV3A3ApzxXmCotHKcbqHg+2omy9\n\tkN2kb+287tbj17GW7MqquSSj31NFzMVmXjnzD+PHGRfrFglA67o2FZbnQte+SkTfmndr\n\tO8iQG7f9G+SffM6v4U7Wo1kZFOssVIxTUB0VH1jgECnqDRSKocJXIzLmi/8gZ9hYoe2D\n\tXiJtookjdQh1rPNQ54vI6knxAwgCMerFxdRNwqEigP7tXGBt9w9Xk/5SXnX1LClKahRP\n\tcnqviMXjK77gG/DYAdpj26bNtaWARnQw9iKxj2bKz1731qgARVuYVlkdL6oNB6PJfPpe\n\tPQFg==","X-Gm-Message-State":"ABy/qLax16Y1tOFoC2KtEoBlyJiEnXCXqIrrUTsICztE02cgRbVBfJzr\n\tfNgr4Xu95viBxDAQl/tTF/31QbZz4Ma8eRci6wc4aQmOsDSFoLrF","X-Google-Smtp-Source":"APBJJlFqDNqfGwOmg5mUY88KMMzbtqHAo3yr4TRxLf0YC4sH7ZnuxHzobkrnoMKEqtbkg84oCxR7Cv+ZhkFDAPQz3Lo=","X-Received":"by 2002:a05:6808:171c:b0:3a3:4314:8dc0 with SMTP id\n\tbc28-20020a056808171c00b003a343148dc0mr10766248oib.5.1689601357746;\n\tMon, 17 Jul 2023 06:42:37 -0700 (PDT)","MIME-Version":"1.0","References":"<20230714141549.11085-1-jacopo.mondi@ideasonboard.com>\n\t<20230714141549.11085-9-jacopo.mondi@ideasonboard.com>\n\t<CAHW6GYKmvSh9oZcFJB48oQHsw2dUwtBnhqhQLr3FuRseArKG1A@mail.gmail.com>\n\t<dobeishhzxv6ouax4ougzczq4gfprcofczrbrnwtc2lrjbldwc@i2eaefvhw5t7>","In-Reply-To":"<dobeishhzxv6ouax4ougzczq4gfprcofczrbrnwtc2lrjbldwc@i2eaefvhw5t7>","Date":"Mon, 17 Jul 2023 14:42:27 +0100","Message-ID":"<CAHW6GYJ=bPczHYCt5i17CrHpF83c3Po_RXwtYSUzb=4Vh+Cw9Q@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 8/9] libcamera: Move to use\n\tCameraConfiguration::orientation","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>","From":"David Plowman via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"David Plowman <david.plowman@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]