[libcamera-devel] [RFC PATCH 1/3] libcamera: Add ColorSpace class

[Naushir Patuck]

Hi David,

Thank you for your work.

On Tue, 8 Jun 2021 at 15:44, David Plowman <david.plowman at raspberrypi.com>
wrote:

> This class represents a colour space by defining its YCbCr encoding,
> the transfer (gamma) function is uses, and whether the output is full
> or limited range.
> ---
>  include/libcamera/color_space.h | 94 +++++++++++++++++++++++++++++++++
>  include/libcamera/meson.build   |  1 +
>  src/libcamera/color_space.cpp   | 43 +++++++++++++++
>  src/libcamera/meson.build       |  1 +
>  4 files changed, 139 insertions(+)
>  create mode 100644 include/libcamera/color_space.h
>  create mode 100644 src/libcamera/color_space.cpp
>
> diff --git a/include/libcamera/color_space.h
> b/include/libcamera/color_space.h
> new file mode 100644
> index 00000000..3d990f99
> --- /dev/null
> +++ b/include/libcamera/color_space.h
> @@ -0,0 +1,94 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> + *
> + * color_space.h - color space definitions
> + */
> +
> +#ifndef __LIBCAMERA_COLOR_SPACE_H__
> +#define __LIBCAMERA_COLOR_SPACE_H__
> +
> +#include <string>
> +
> +namespace libcamera {
> +
> +class ColorSpace
> +{
> +public:
> +       enum class Encoding : int {
> +               UNDEFINED,
> +               RAW,
> +               REC601,
> +               REC709,
> +               REC2020,
> +               VIDEO,
> +       };
> +
> +       enum class TransferFunction : int {
> +               UNDEFINED,
> +               IDENTITY,
> +               SRGB,
> +               REC709,
> +       };
> +
> +       enum class Range : int {
> +               UNDEFINED,
> +               FULL,
> +               LIMITED,
> +       };
> +
> +       constexpr ColorSpace(Encoding e, TransferFunction t, Range r)
> +               : encoding(e), transferFunction(t), range(r)
> +       {
> +       }
> +
> +       constexpr ColorSpace()
> +               : ColorSpace(Encoding::UNDEFINED,
> TransferFunction::UNDEFINED, Range::UNDEFINED)
> +       {
> +       }
> +
> +       static const ColorSpace UNDEFINED;
> +       static const ColorSpace RAW;
> +       static const ColorSpace JFIF;
> +       static const ColorSpace SMPTE170M;
> +       static const ColorSpace REC709;
> +       static const ColorSpace REC2020;
> +       static const ColorSpace VIDEO;
> +
> +       Encoding encoding;
> +       TransferFunction transferFunction;
> +       Range range;
> +
> +       bool isFullyDefined() const
> +       {
> +               return encoding != Encoding::UNDEFINED &&
> +                      transferFunction != TransferFunction::UNDEFINED &&
> +                      range != Range::UNDEFINED;
> +       }
> +
> +       const std::string toString() const;
> +};
> +
> +constexpr ColorSpace ColorSpace::UNDEFINED = { Encoding::UNDEFINED,
> TransferFunction::UNDEFINED, Range::UNDEFINED };
>

Do you think we need ColorSpace::UNDEFINED?  Not sure why anyone would need
to use it?


> +constexpr ColorSpace ColorSpace::RAW = { Encoding::RAW,
> TransferFunction::IDENTITY, Range::FULL };
> +constexpr ColorSpace ColorSpace::JFIF = { Encoding::REC601,
> TransferFunction::SRGB, Range::FULL };
> +constexpr ColorSpace ColorSpace::SMPTE170M = { Encoding::REC601,
> TransferFunction::REC709, Range::LIMITED };
> +constexpr ColorSpace ColorSpace::REC709 = { Encoding::REC709,
> TransferFunction::REC709, Range::LIMITED };
> +constexpr ColorSpace ColorSpace::REC2020 = { Encoding::REC2020,
> TransferFunction::REC709, Range::LIMITED };
> +constexpr ColorSpace ColorSpace::VIDEO = { Encoding::VIDEO,
> TransferFunction::REC709, Range::LIMITED };
> +
> +static inline bool operator==(const ColorSpace &lhs, const ColorSpace
> &rhs)
> +{
> +       return lhs.encoding == rhs.encoding &&
> +              lhs.transferFunction == rhs.transferFunction &&
> +              lhs.range == rhs.range;
> +}
> +
> +static inline bool operator!=(const ColorSpace &lhs, const ColorSpace
> &rhs)
> +{
> +       return !(lhs == rhs);
> +}
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_COLOR_SPACE_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 086c958b..7d61d285 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -6,6 +6,7 @@ libcamera_public_headers = files([
>      'camera.h',
>      'camera_manager.h',
>      'class.h',
> +    'color_space.h',
>      'compiler.h',
>      'controls.h',
>      'file_descriptor.h',
> diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp
> new file mode 100644
> index 00000000..888762ae
> --- /dev/null
> +++ b/src/libcamera/color_space.cpp
> @@ -0,0 +1,43 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited
> + *
> + * color_space.cpp - color spaces.
> + */
> +
> +#include <libcamera/color_space.h>
> +
> +/**
> + * \file color_space.h
> + * \brief Class and enums to represent colour spaces.
> + */
> +
> +namespace libcamera {
> +
> +const std::string ColorSpace::toString() const
> +{
> +       static const char *encodings[] = {
> +               "UNDEFINED",
> +               "RAW",
> +               "REC601",
> +               "REC709",
> +               "REC2020",
> +       };
> +       static const char *transferFunctions[] = {
> +               "UNDEFINED",
> +               "IDENTITY",
> +               "SRGB",
> +               "REC709",
> +       };
> +       static const char *ranges[] = {
> +               "UNDEFINED",
> +               "FULL",
> +               "LIMITED",
> +       };
> +
> +       return std::string(encodings[static_cast<int>(encoding)]) + "+" +
> +
> std::string(transferFunctions[static_cast<int>(transferFunction)]) + "+" +
> +              std::string(ranges[static_cast<int>(range)]);
>

I wonder if using stringstream might be more efficient here?

Regards,
Naush



> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 7e19a177..f7b48a32 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -11,6 +11,7 @@ libcamera_sources = files([
>      'camera_sensor.cpp',
>      'camera_sensor_properties.cpp',
>      'class.cpp',
> +    'color_space.cpp',
>      'controls.cpp',
>      'control_serializer.cpp',
>      'control_validator.cpp',
> --
> 2.20.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210614/d2291d34/attachment-0001.htm>