[{"id":27555,"web_url":"https://patchwork.libcamera.org/comment/27555/","msgid":"<CAHW6GYKbT1c1WEfN3_mnVO185DAm5V_=_RJv5czwOPsGhKdu+A@mail.gmail.com>","date":"2023-07-17T09:28:43","subject":"Re: [libcamera-devel] [PATCH v2 2/9] libcamera: camera: Introduce\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 this patch!\n\nOn Fri, 14 Jul 2023 at 15:16, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n> Introduce CameraConfiguration::Orientation in the CameraConfiguration\n> class.\n>\n> The Orienation enumeration describes the possible 2D transformations\n\ns/Orienation/Orientation/\n\nI have some semantic difficulties deciding whether an orientation is a\ntransformation, or strictly only the outcome of a transformation...\nbut honestly, I'm fine to go with this!!\n\n> that can be applied to an image using two basic plane transformations.\n>\n> The enumeration values follow the ones defined by the EXIF specification at\n> revision 2.32, Tag 274 'orientation'.\n>\n> The newly introduced filed is meant to replace\n> CameraConfiguration::transform which is not removed yet not to break\n> compilation.\n>\n> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> ---\n>  include/libcamera/camera.h | 17 +++++++++\n>  src/libcamera/camera.cpp   | 76 +++++++++++++++++++++++++++++++++++++-\n>  2 files changed, 92 insertions(+), 1 deletion(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 004bc89455f5..8132ef2506a4 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -8,6 +8,7 @@\n>  #pragma once\n>\n>  #include <initializer_list>\n> +#include <iostream>\n>  #include <memory>\n>  #include <set>\n>  #include <stdint.h>\n> @@ -39,6 +40,18 @@ public:\n>                 Invalid,\n>         };\n>\n> +       enum Orientation {\n> +               /* EXIF tag 274 starts from '1' */\n> +               rotate0 = 1,\n> +               rotate0Flip,\n> +               rotate180,\n> +               rotate180Flip,\n> +               rotate90Flip,\n> +               rotate270,\n> +               rotate270Flip,\n> +               rotate90,\n> +       };\n> +\n\nIf it weren't for the world external to libcamera, of course it would\nhave been convenient to use the same internal representation as\ntransforms do - it would simplify our code slightly. But I see that it\nmay be convenient to have it tie up with the defined EXIF values too,\nso I guess I'm fine with this.\n\nI wonder slightly whether it might be worth duplicating the enum names\nto make the correspondence explicit, either here (so we'd add\n\"identity = 1\" here) or in the transform class (so we'd have \"rotate0\n= 0\" there). But maybe that's messy, so I'm not too fussed.\n\n>         using iterator = std::vector<StreamConfiguration>::iterator;\n>         using const_iterator = std::vector<StreamConfiguration>::const_iterator;\n>\n> @@ -67,6 +80,7 @@ public:\n>         std::size_t size() const;\n>\n>         Transform transform;\n> +       Orientation orientation;\n>\n>  protected:\n>         CameraConfiguration();\n> @@ -83,6 +97,9 @@ protected:\n>         std::vector<StreamConfiguration> config_;\n>  };\n>\n> +std::ostream &operator<<(std::ostream &out,\n> +                        const CameraConfiguration::Orientation &orientation);\n> +\n>  class Camera final : public Object, public std::enable_shared_from_this<Camera>,\n>                      public Extensible\n>  {\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 0eecee766f00..78aa11e72dd4 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -145,6 +145,44 @@ LOG_DECLARE_CATEGORY(Camera)\n>   * The configuration is invalid and can't be adjusted automatically\n>   */\n>\n> +/**\n> + * \\enum CameraConfiguration::Orientation\n> + * \\brief The image orientation in a memory buffer\n> + *\n> + * The Orientation enumeration describes the orientation of the images\n> + * produced by the camera pipeline as they get received by the application\n> + * inside memory buffers.\n> + *\n> + * All the possible 2D planes transformation of an image are expressed as the\n\ns/planes transformation/plane transformations/\n\n> + * combination of two basic operations:\n> + *\n> + *   'a': rotate the image 90 degrees clockwise\n> + *   'b': flip the image horizontally (mirroring)\n> + *\n> + * plus an identity operator 'e'.\n> + *\n> + * The composition of operations 'a' and 'b' according to the canonical function\n> + * composition notion 'b * a' (where 'a' is applied first, then 'b' is applied\n> + * next) gives origin to a symmetric group of order 8, or dihedral group.\n> + *\n> + * See https://en.wikipedia.org/wiki/Dihedral_group#/media/File:Dih4_cycle_graph.svg\n> + * as an example of the 2D plane transformation and how they originate from\n> + * the composition of the 'a' and 'b' operations.\n> + *\n> + * The image orientation expressed using the Orientation enumeration can be\n> + * then inferred by applying a multiple of a 90 degrees rotation from the origin\n> + * and the applying any horizontal mirroring to a base image assume to be\n> + * naturally oriented (no rotation and no mirroring applied).\n> + *\n> + * The enumeration numerical values follow the ones defined by the EXIF\n> + * Specification version 2.32, Tag 274 \"Orientation\", while the names of the\n> + * enumerated values report the rotation and mirroring operations performed.\n> + *\n> + * In example Orientation::rotate90Flip describes the image transformation\n> + * obtained by rotating 90 degrees clockwise first and the applying an\n\ns/the/then/\n\n> + * horizontal mirroring.\n> + */\n> +\n>  /**\n>   * \\typedef CameraConfiguration::iterator\n>   * \\brief Iterator for the stream configurations in the camera configuration\n> @@ -160,7 +198,7 @@ LOG_DECLARE_CATEGORY(Camera)\n>   * \\brief Create an empty camera configuration\n>   */\n>  CameraConfiguration::CameraConfiguration()\n> -       : transform(Transform::Identity), config_({})\n> +       : transform(Transform::Identity), orientation(rotate0), config_({})\n>  {\n>  }\n>\n> @@ -317,6 +355,27 @@ std::size_t CameraConfiguration::size() const\n>         return config_.size();\n>  }\n>\n> +/**\n> + * \\brief Prints human-friendly names for Orientation items\n> + * \\param[in] out The output stream\n> + * \\param[in] orientation The Orientation item\n> + * \\return The output stream \\a out\n> + */\n> +std::ostream &operator<<(std::ostream &out,\n> +                        const CameraConfiguration::Orientation &orientation)\n> +{\n> +       constexpr std::array<const char *, 9> orientationNames = {\n> +               \"\", /* Orientation starts counting from 1. */\n> +               \"rotate0\", \"rotate0Flip\",\n> +               \"rotate180\", \"rotate180Flip\",\n> +               \"rotate90Flip\", \"rotate270\",\n> +               \"rotate270Flip\", \"rotate90\",\n> +       };\n> +\n> +       out << orientationNames[orientation];\n> +       return out;\n> +}\n> +\n>  /**\n>   * \\enum CameraConfiguration::ColorSpaceFlag\n>   * \\brief Specify the behaviour of validateColorSpaces\n> @@ -404,6 +463,21 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\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> + *\n> + * The orientation field is a user-specified 2D plane transformation that\n> + * specifies how the application wants the camera images to be rotated in\n> + * the memory buffers.\n> + *\n> + * If the application requested orientation cannot be obtained the validate()\n> + * function will Adjust this field to the actual orientation of the images\n> + * as produced by the camera pipeline.\n> + *\n> + * By default the orientation filed is set to Orientation::rotate0.\n\ns/filed/field/\n\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\n\nThanks!\nDavid\n\n> + */\n> +\n>  /**\n>   * \\var CameraConfiguration::config_\n>   * \\brief The vector of stream configurations\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 99902BDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 17 Jul 2023 09:28:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0EFD761E2B;\n\tMon, 17 Jul 2023 11:28:57 +0200 (CEST)","from mail-qv1-xf2a.google.com (mail-qv1-xf2a.google.com\n\t[IPv6:2607:f8b0:4864:20::f2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9CF3060381\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Jul 2023 11:28:55 +0200 (CEST)","by mail-qv1-xf2a.google.com with SMTP id\n\t6a1803df08f44-636274ce31eso31564116d6.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 17 Jul 2023 02:28:55 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1689586137;\n\tbh=UiNy/P9kVR0HD06fxT5P9qtAcAtlz6b+v5TX3kydaY0=;\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=ZFaz61hPui7O0JfiHZ9sjmvO03WekMy4Jgra9rUbKJDYJB3ZH35uDo5l9/ShD7/UO\n\tvsEGcP0WyHWtXVKI4qnyagDFoc26tNmYz6sHBa2QGtV2GFI7FIVQpKa9jl2U5wQW9z\n\tqS9sSXTDc1yed9PLwS+ueciMEl2sTVg+qzoNhnThfEnNKscDSoE38nxIvRSmSmTL+b\n\tpZz+WkNRURXpPkd/EWrviMKQ78yhWKLRljlRwgmrEik82u8qIs0oQGqp7hA8FEXgCg\n\tQcWUEYEyYE6NcWN4jPQ1iR86qxy7ZniepkR4xxbYPqIotMEiWeMKC2SbcMthhMULvk\n\tSPs4rXLeq+nzg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1689586134; x=1692178134;\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=dq6wfBctBmtFzk6Y0HVVRqV+1Ki+pWsM5g9TPwDa7Mg=;\n\tb=CD2zHWn+dZmqI1HkmIxRtCqd7v2jJ4SBeN3JO9bkd55JE/vs0xn3IJa7NsWqeXVCe2\n\tfqNvb7tUo0KImPmRVSeqxGvtgCZ73HcO7g1vyJ275L+umqZJGKqySCLMmO9pnAGbSJdt\n\t0li0CFes/lqrjVIktmN+3oX66Zt+IjUCjky+vDxZn5aJ4YoEjlSQUf/R69yyCTjBGCZ8\n\tnyfd3PRG9cdeMbjnMQ6omhXOwwiYtR08jvzON6bt4XaOly6RIGqZ52Jvm/oe/b664d97\n\tNtIRI9cg6BUQBVBgiZalgi44SqJGdFhQgz4KwPO6pRUPZjtula/p0eahMKXCpDsqm0J2\n\tDwvg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"CD2zHWn+\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1689586134; x=1692178134;\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=dq6wfBctBmtFzk6Y0HVVRqV+1Ki+pWsM5g9TPwDa7Mg=;\n\tb=fEpjdjxsrgSn/dj9cNOCH/WR/TUjg66bLrkzCXgW1Ng3+/wHQrJyxubUT3royCjdVA\n\t1cAKZv96yKJIRXLlFGM1fQ2wk9Ar9gG8JfUJxV37Dmuha1GhJviQ1GSnREb5nZjpvqQM\n\tAUmvSni0wTVDDdWrB+c2giWpBCdMO0QIUX34Ex2oxoNWq+bWQE4CV121yVbwNleh8knN\n\txINyHmp22KnIsxMkwQBqGr+j84SeWKvN/0EJZvKkqLvyXw0T0vcyuBdmQT7uyQnEy7Iz\n\tki6IokU8wTcc02hf073BfoiAVARKcfhgzPdMXZ4KUp6VonSM3Y/Z4PpMYFePY4zZAy14\n\tiHOw==","X-Gm-Message-State":"ABy/qLZUQxy8ucXdk0SgHtc8YNE8e79il1HKTHVx7vwB6+3iNTAuLri9\n\tLTwSHTlewiUWcFFaQnmq7kkawUX/KoOGawOHz+tyFg==","X-Google-Smtp-Source":"APBJJlHFxGQwUcsgCEY6sA9Tyq56a9YLeBkE86GJ6xSSZnpkJ9Cf4QeZx4oKweTLQU/X+nkELFJpz0fWP5EGsBQsnUM=","X-Received":"by 2002:a05:6214:5609:b0:635:eae1:414 with SMTP id\n\tmg9-20020a056214560900b00635eae10414mr10228568qvb.50.1689586134352;\n\tMon, 17 Jul 2023 02:28:54 -0700 (PDT)","MIME-Version":"1.0","References":"<20230714141549.11085-1-jacopo.mondi@ideasonboard.com>\n\t<20230714141549.11085-3-jacopo.mondi@ideasonboard.com>","In-Reply-To":"<20230714141549.11085-3-jacopo.mondi@ideasonboard.com>","Date":"Mon, 17 Jul 2023 10:28:43 +0100","Message-ID":"<CAHW6GYKbT1c1WEfN3_mnVO185DAm5V_=_RJv5czwOPsGhKdu+A@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/9] libcamera: camera: Introduce\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>"}}]