[libcamera-devel] [PATCH v2 2/9] libcamera: camera: Introduce CameraConfiguration::orientation

[David Plowman]

Hi Jacopo

Thanks for this patch!

On Fri, 14 Jul 2023 at 15:16, Jacopo Mondi
<jacopo.mondi at ideasonboard.com> wrote:
>
> Introduce CameraConfiguration::Orientation in the CameraConfiguration
> class.
>
> The Orienation enumeration describes the possible 2D transformations

s/Orienation/Orientation/

I have some semantic difficulties deciding whether an orientation is a
transformation, or strictly only the outcome of a transformation...
but honestly, I'm fine to go with this!!

> that can be applied to an image using two basic plane transformations.
>
> The enumeration values follow the ones defined by the EXIF specification at
> revision 2.32, Tag 274 'orientation'.
>
> The newly introduced filed is meant to replace
> CameraConfiguration::transform which is not removed yet not to break
> compilation.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
>  include/libcamera/camera.h | 17 +++++++++
>  src/libcamera/camera.cpp   | 76 +++++++++++++++++++++++++++++++++++++-
>  2 files changed, 92 insertions(+), 1 deletion(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 004bc89455f5..8132ef2506a4 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -8,6 +8,7 @@
>  #pragma once
>
>  #include <initializer_list>
> +#include <iostream>
>  #include <memory>
>  #include <set>
>  #include <stdint.h>
> @@ -39,6 +40,18 @@ public:
>                 Invalid,
>         };
>
> +       enum Orientation {
> +               /* EXIF tag 274 starts from '1' */
> +               rotate0 = 1,
> +               rotate0Flip,
> +               rotate180,
> +               rotate180Flip,
> +               rotate90Flip,
> +               rotate270,
> +               rotate270Flip,
> +               rotate90,
> +       };
> +

If it weren't for the world external to libcamera, of course it would
have been convenient to use the same internal representation as
transforms do - it would simplify our code slightly. But I see that it
may be convenient to have it tie up with the defined EXIF values too,
so I guess I'm fine with this.

I wonder slightly whether it might be worth duplicating the enum names
to make the correspondence explicit, either here (so we'd add
"identity = 1" here) or in the transform class (so we'd have "rotate0
= 0" there). But maybe that's messy, so I'm not too fussed.

>         using iterator = std::vector<StreamConfiguration>::iterator;
>         using const_iterator = std::vector<StreamConfiguration>::const_iterator;
>
> @@ -67,6 +80,7 @@ public:
>         std::size_t size() const;
>
>         Transform transform;
> +       Orientation orientation;
>
>  protected:
>         CameraConfiguration();
> @@ -83,6 +97,9 @@ protected:
>         std::vector<StreamConfiguration> config_;
>  };
>
> +std::ostream &operator<<(std::ostream &out,
> +                        const CameraConfiguration::Orientation &orientation);
> +
>  class Camera final : public Object, public std::enable_shared_from_this<Camera>,
>                      public Extensible
>  {
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 0eecee766f00..78aa11e72dd4 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -145,6 +145,44 @@ LOG_DECLARE_CATEGORY(Camera)
>   * The configuration is invalid and can't be adjusted automatically
>   */
>
> +/**
> + * \enum CameraConfiguration::Orientation
> + * \brief The image orientation in a memory buffer
> + *
> + * The Orientation enumeration describes the orientation of the images
> + * produced by the camera pipeline as they get received by the application
> + * inside memory buffers.
> + *
> + * All the possible 2D planes transformation of an image are expressed as the

s/planes transformation/plane transformations/

> + * combination of two basic operations:
> + *
> + *   'a': rotate the image 90 degrees clockwise
> + *   'b': flip the image horizontally (mirroring)
> + *
> + * plus an identity operator 'e'.
> + *
> + * The composition of operations 'a' and 'b' according to the canonical function
> + * composition notion 'b * a' (where 'a' is applied first, then 'b' is applied
> + * next) gives origin to a symmetric group of order 8, or dihedral group.
> + *
> + * See https://en.wikipedia.org/wiki/Dihedral_group#/media/File:Dih4_cycle_graph.svg
> + * as an example of the 2D plane transformation and how they originate from
> + * the composition of the 'a' and 'b' operations.
> + *
> + * The image orientation expressed using the Orientation enumeration can be
> + * then inferred by applying a multiple of a 90 degrees rotation from the origin
> + * and the applying any horizontal mirroring to a base image assume to be
> + * naturally oriented (no rotation and no mirroring applied).
> + *
> + * The enumeration numerical values follow the ones defined by the EXIF
> + * Specification version 2.32, Tag 274 "Orientation", while the names of the
> + * enumerated values report the rotation and mirroring operations performed.
> + *
> + * In example Orientation::rotate90Flip describes the image transformation
> + * obtained by rotating 90 degrees clockwise first and the applying an

s/the/then/

> + * horizontal mirroring.
> + */
> +
>  /**
>   * \typedef CameraConfiguration::iterator
>   * \brief Iterator for the stream configurations in the camera configuration
> @@ -160,7 +198,7 @@ LOG_DECLARE_CATEGORY(Camera)
>   * \brief Create an empty camera configuration
>   */
>  CameraConfiguration::CameraConfiguration()
> -       : transform(Transform::Identity), config_({})
> +       : transform(Transform::Identity), orientation(rotate0), config_({})
>  {
>  }
>
> @@ -317,6 +355,27 @@ std::size_t CameraConfiguration::size() const
>         return config_.size();
>  }
>
> +/**
> + * \brief Prints human-friendly names for Orientation items
> + * \param[in] out The output stream
> + * \param[in] orientation The Orientation item
> + * \return The output stream \a out
> + */
> +std::ostream &operator<<(std::ostream &out,
> +                        const CameraConfiguration::Orientation &orientation)
> +{
> +       constexpr std::array<const char *, 9> orientationNames = {
> +               "", /* Orientation starts counting from 1. */
> +               "rotate0", "rotate0Flip",
> +               "rotate180", "rotate180Flip",
> +               "rotate90Flip", "rotate270",
> +               "rotate270Flip", "rotate90",
> +       };
> +
> +       out << orientationNames[orientation];
> +       return out;
> +}
> +
>  /**
>   * \enum CameraConfiguration::ColorSpaceFlag
>   * \brief Specify the behaviour of validateColorSpaces
> @@ -404,6 +463,21 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF
>   * may adjust this field at its discretion if the selection is not supported.
>   */
>
> +/**
> + * \var CameraConfiguration::orientation
> + * \brief The desired orientation of the images produced by the camera
> + *
> + * The orientation field is a user-specified 2D plane transformation that
> + * specifies how the application wants the camera images to be rotated in
> + * the memory buffers.
> + *
> + * If the application requested orientation cannot be obtained the validate()
> + * function will Adjust this field to the actual orientation of the images
> + * as produced by the camera pipeline.
> + *
> + * By default the orientation filed is set to Orientation::rotate0.

s/filed/field/

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David

> + */
> +
>  /**
>   * \var CameraConfiguration::config_
>   * \brief The vector of stream configurations
> --
> 2.40.1
>