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

13212 diff mbox series

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.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 include/libcamera/color_space.h |  94 +++++++++++++++++
 include/libcamera/meson.build   |   1 +
 src/libcamera/color_space.cpp   | 180 ++++++++++++++++++++++++++++++++
 src/libcamera/meson.build       |   1 +
 4 files changed, 276 insertions(+)
 create mode 100644 include/libcamera/color_space.h
 create mode 100644 src/libcamera/color_space.cpp

Hi David,

Thank you for your patch.

On Thu, 5 Aug 2021 at 15:22, David Plowman <david.plowman@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.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  include/libcamera/color_space.h |  94 +++++++++++++++++
>  include/libcamera/meson.build   |   1 +
>  src/libcamera/color_space.cpp   | 180 ++++++++++++++++++++++++++++++++
>  src/libcamera/meson.build       |   1 +
>  4 files changed, 276 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 };
> +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 5b25ef84..7a8a04e5 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -3,6 +3,7 @@
>  libcamera_public_headers = files([
>      'camera.h',
>      'camera_manager.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..c40264db
> --- /dev/null
> +++ b/src/libcamera/color_space.cpp
> @@ -0,0 +1,180 @@
> +/* 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 {
> +
> +/**
> + * \class ColorSpace
> + * \brief Class to describe a color space.
> + *
> + * The color space class defines the encodings of the color primaries, the
> + * transfer function associated with the color space, and the range
> (sometimes
> + * also referred to as the quantisation) of the color space.
> + *
> + * Certain combinations of these fields form well-known standard color
> spaces,
> + * such as "JFIF" or "REC709", though there is flexibility to leave some
> or all
> + * of them undefined too.
> + */
> +
> +/**
> + * \enum ColorSpace::Encoding
> + * \brief The encoding used for the color primaries.
> + *
> + * \var ColorSpace::Encoding::UNDEFINED
> + * \brief The encoding for the colour primaries is not specified.
> + * \var ColorSpace::Encoding::RAW
> + * \brief These are raw colours from the sensor.
> + * \var ColorSpace::Encoding::REC601
> + * \brief REC601 colour primaries.
> + * \var ColorSpace::Encoding::REC709
> + * \brief Rec709 colour primaries.
> + * \var ColorSpace::Encoding::REC2020
> + * \brief REC2020 colour primaries.
> + * \var ColorSpace::Encoding::VIDEO
> + * \brief A place-holder for video streams which will be resolved to one
> + * of REC601, REC709 or REC2020 once the video resolution is known.
> + */
> +
> +/**
> + * \enum ColorSpace::TransferFunction
> + * \brief The transfer function used for this colour space.
> + *
> + * \var ColorSpace::TransferFunction::UNDEFINED
> + * \brief The transfer function is not specified.
> + * \var ColorSpace::TransferFunction::IDENTITY
> + * \brief This color space uses an identity transfer function.
> + * \var ColorSpace::TransferFunction::SRGB
> + * \brief sRGB transfer function.
> + * \var ColorSpace::TransferFunction::REC709
> + * \brief Rec709 transfer function.
> + */
> +
> +/**
> + * \enum ColorSpace::Range
> + * \brief The range (sometimes "quantisation") for this color space.
> + *
> + * \var ColorSpace::Range::UNDEFINED
> + * \brief The range is not specified.
> + * \var ColorSpace::Range::FULL
> + * \brief This color space uses full range pixel values.
> + * \var ColorSpace::Range::LIMITED
> + * \brief This color space uses limited range pixel values.
> + */
> +
> +/**
> + * \fn ColorSpace::ColorSpace(Encoding e, TransferFunction t, Range r)
> + * \brief Construct a ColorSpace from explicit values
> + * \param[in] e The encoding for the color primaries
> + * \param[in] t The transfer function for the color space
> + * \param[in] r The range of the pixel values in this color space
> + */
> +
> +/**
> + * \fn ColorSpace::ColorSpace()
> + * \brief Construct a color space with undefined encoding, transfer
> function
> + * and range
> + */
> +
> +/**
> + * \fn ColorSpace::isFullyDefined() const
> + * \brief Return whether all the fields of the color space are defined.
> + */
> +
> +/**
> + * \brief Assemble and return a readable string representation of the
> + * ColorSpace
> + * \return A string describing the ColorSpace
> + */
> +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)]);
> +}
>

Perhaps using std::stringstream would be better here?
Apart from that

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


> +
> +/**
> + * \var ColorSpace::encoding
> + * \brief The encoding of the color primaries
> + */
> +
> +/**
> + * \var ColorSpace::transferFunction
> + * \brief The transfer function for this color space.
> + */
> +
> +/**
> + * \var ColorSpace::range
> + * \brief The pixel range used by this color space.
> + */
> +
> +/**
> + * \var ColorSpace::UNDEFINED
> + * \brief A constant representing a fully undefined color space.
> + */
> +
> +/**
> + * \var ColorSpace::RAW
> + * \brief A constant representing a raw color space (from a sensor).
> + */
> +
> +/**
> + * \var ColorSpace::JFIF
> + * \brief A constant representing the JFIF color space usually used for
> + * encoding JPEG images.
> + */
> +
> +/**
> + * \var ColorSpace::SMPTE170M
> + * \brief A constant representing the SMPTE170M color space (sometimes
> also
> + * referred to as "full range BT601").
> + */
> +
> +/**
> + * \var ColorSpace::REC709
> + * \brief A constant representing the REC709 color space.
> + */
> +
> +/**
> + * \var ColorSpace::REC2020
> + * \brief A constant representing the REC2020 color space.
> + */
> +
> +/**
> + * \var ColorSpace::VIDEO
> + * \brief A constant that video streams can use to indicate the "default"
> + * color space for a video of this resolution, once that is is known. For
> + * exmample, SD streams would interpret this as SMPTE170M, HD streams as
> + * REC709 and ultra HD as REC2020.
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 4f085801..e0748840 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -8,6 +8,7 @@ libcamera_sources = files([
>      'camera_manager.cpp',
>      'camera_sensor.cpp',
>      'camera_sensor_properties.cpp',
> +    'color_space.cpp',
>      'controls.cpp',
>      'control_serializer.cpp',
>      'control_validator.cpp',
> --
> 2.20.1
>
>

Hi David,

Thanks for the patch.

On Thu, Aug 05, 2021 at 03:21:52PM +0100, David Plowman 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.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  include/libcamera/color_space.h |  94 +++++++++++++++++
>  include/libcamera/meson.build   |   1 +
>  src/libcamera/color_space.cpp   | 180 ++++++++++++++++++++++++++++++++
>  src/libcamera/meson.build       |   1 +
>  4 files changed, 276 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,
> +	};

I suppose we'll just ignore the other values that V4L2 has that we don't
have?

> +
> +	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 };
> +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);
> +}

What's wrong with putting these two functions in the cpp file, and
leaving a funtion prototype in the class definition?

Otherwise, looks good to me.


Paul

> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_COLOR_SPACE_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 5b25ef84..7a8a04e5 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -3,6 +3,7 @@
>  libcamera_public_headers = files([
>      'camera.h',
>      'camera_manager.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..c40264db
> --- /dev/null
> +++ b/src/libcamera/color_space.cpp
> @@ -0,0 +1,180 @@
> +/* 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 {
> +
> +/**
> + * \class ColorSpace
> + * \brief Class to describe a color space.
> + *
> + * The color space class defines the encodings of the color primaries, the
> + * transfer function associated with the color space, and the range (sometimes
> + * also referred to as the quantisation) of the color space.
> + *
> + * Certain combinations of these fields form well-known standard color spaces,
> + * such as "JFIF" or "REC709", though there is flexibility to leave some or all
> + * of them undefined too.
> + */
> +
> +/**
> + * \enum ColorSpace::Encoding
> + * \brief The encoding used for the color primaries.
> + *
> + * \var ColorSpace::Encoding::UNDEFINED
> + * \brief The encoding for the colour primaries is not specified.
> + * \var ColorSpace::Encoding::RAW
> + * \brief These are raw colours from the sensor.
> + * \var ColorSpace::Encoding::REC601
> + * \brief REC601 colour primaries.
> + * \var ColorSpace::Encoding::REC709
> + * \brief Rec709 colour primaries.
> + * \var ColorSpace::Encoding::REC2020
> + * \brief REC2020 colour primaries.
> + * \var ColorSpace::Encoding::VIDEO
> + * \brief A place-holder for video streams which will be resolved to one
> + * of REC601, REC709 or REC2020 once the video resolution is known.
> + */
> +
> +/**
> + * \enum ColorSpace::TransferFunction
> + * \brief The transfer function used for this colour space.
> + *
> + * \var ColorSpace::TransferFunction::UNDEFINED
> + * \brief The transfer function is not specified.
> + * \var ColorSpace::TransferFunction::IDENTITY
> + * \brief This color space uses an identity transfer function.
> + * \var ColorSpace::TransferFunction::SRGB
> + * \brief sRGB transfer function.
> + * \var ColorSpace::TransferFunction::REC709
> + * \brief Rec709 transfer function.
> + */
> +
> +/**
> + * \enum ColorSpace::Range
> + * \brief The range (sometimes "quantisation") for this color space.
> + *
> + * \var ColorSpace::Range::UNDEFINED
> + * \brief The range is not specified.
> + * \var ColorSpace::Range::FULL
> + * \brief This color space uses full range pixel values.
> + * \var ColorSpace::Range::LIMITED
> + * \brief This color space uses limited range pixel values.
> + */
> +
> +/**
> + * \fn ColorSpace::ColorSpace(Encoding e, TransferFunction t, Range r)
> + * \brief Construct a ColorSpace from explicit values
> + * \param[in] e The encoding for the color primaries
> + * \param[in] t The transfer function for the color space
> + * \param[in] r The range of the pixel values in this color space
> + */
> +
> +/**
> + * \fn ColorSpace::ColorSpace()
> + * \brief Construct a color space with undefined encoding, transfer function
> + * and range
> + */
> +
> +/**
> + * \fn ColorSpace::isFullyDefined() const
> + * \brief Return whether all the fields of the color space are defined.
> + */
> +
> +/**
> + * \brief Assemble and return a readable string representation of the
> + * ColorSpace
> + * \return A string describing the ColorSpace
> + */
> +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)]);
> +}
> +
> +/**
> + * \var ColorSpace::encoding
> + * \brief The encoding of the color primaries
> + */
> +
> +/**
> + * \var ColorSpace::transferFunction
> + * \brief The transfer function for this color space.
> + */
> +
> +/**
> + * \var ColorSpace::range
> + * \brief The pixel range used by this color space.
> + */
> +
> +/**
> + * \var ColorSpace::UNDEFINED
> + * \brief A constant representing a fully undefined color space.
> + */
> +
> +/**
> + * \var ColorSpace::RAW
> + * \brief A constant representing a raw color space (from a sensor).
> + */
> +
> +/**
> + * \var ColorSpace::JFIF
> + * \brief A constant representing the JFIF color space usually used for
> + * encoding JPEG images.
> + */
> +
> +/**
> + * \var ColorSpace::SMPTE170M
> + * \brief A constant representing the SMPTE170M color space (sometimes also
> + * referred to as "full range BT601").
> + */
> +
> +/**
> + * \var ColorSpace::REC709
> + * \brief A constant representing the REC709 color space.
> + */
> +
> +/**
> + * \var ColorSpace::REC2020
> + * \brief A constant representing the REC2020 color space.
> + */
> +
> +/**
> + * \var ColorSpace::VIDEO
> + * \brief A constant that video streams can use to indicate the "default"
> + * color space for a video of this resolution, once that is is known. For
> + * exmample, SD streams would interpret this as SMPTE170M, HD streams as
> + * REC709 and ultra HD as REC2020.
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 4f085801..e0748840 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -8,6 +8,7 @@ libcamera_sources = files([
>      'camera_manager.cpp',
>      'camera_sensor.cpp',
>      'camera_sensor_properties.cpp',
> +    'color_space.cpp',
>      'controls.cpp',
>      'control_serializer.cpp',
>      'control_validator.cpp',
> -- 
> 2.20.1
>

Hi Paul,

On Thu, Sep 23, 2021 at 03:21:10PM +0900, paul.elder@ideasonboard.com wrote:
> On Thu, Aug 05, 2021 at 03:21:52PM +0100, David Plowman 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.
> > 
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  include/libcamera/color_space.h |  94 +++++++++++++++++
> >  include/libcamera/meson.build   |   1 +
> >  src/libcamera/color_space.cpp   | 180 ++++++++++++++++++++++++++++++++
> >  src/libcamera/meson.build       |   1 +
> >  4 files changed, 276 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,
> > +	};
> 
> I suppose we'll just ignore the other values that V4L2 has that we don't
> have?
> 
> > +
> > +	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 };
> > +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);
> > +}
> 
> What's wrong with putting these two functions in the cpp file, and
> leaving a funtion prototype in the class definition?

It's a matter of inlining. For operator!=(), for instance, there should
be very little (or no ?) drawback to inlining it, it won't increase code
size, the compiler will be able to optimize it, and there should be no
backward compatibility concerns as I don't see how the function
definition could change.

(On a side note, I wish we could use the C++20 spaceship <=>)

> Otherwise, looks good to me.
> 
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_COLOR_SPACE_H__ */
> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > index 5b25ef84..7a8a04e5 100644
> > --- a/include/libcamera/meson.build
> > +++ b/include/libcamera/meson.build
> > @@ -3,6 +3,7 @@
> >  libcamera_public_headers = files([
> >      'camera.h',
> >      'camera_manager.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..c40264db
> > --- /dev/null
> > +++ b/src/libcamera/color_space.cpp
> > @@ -0,0 +1,180 @@
> > +/* 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 {
> > +
> > +/**
> > + * \class ColorSpace
> > + * \brief Class to describe a color space.
> > + *
> > + * The color space class defines the encodings of the color primaries, the
> > + * transfer function associated with the color space, and the range (sometimes
> > + * also referred to as the quantisation) of the color space.
> > + *
> > + * Certain combinations of these fields form well-known standard color spaces,
> > + * such as "JFIF" or "REC709", though there is flexibility to leave some or all
> > + * of them undefined too.
> > + */
> > +
> > +/**
> > + * \enum ColorSpace::Encoding
> > + * \brief The encoding used for the color primaries.
> > + *
> > + * \var ColorSpace::Encoding::UNDEFINED
> > + * \brief The encoding for the colour primaries is not specified.
> > + * \var ColorSpace::Encoding::RAW
> > + * \brief These are raw colours from the sensor.
> > + * \var ColorSpace::Encoding::REC601
> > + * \brief REC601 colour primaries.
> > + * \var ColorSpace::Encoding::REC709
> > + * \brief Rec709 colour primaries.
> > + * \var ColorSpace::Encoding::REC2020
> > + * \brief REC2020 colour primaries.
> > + * \var ColorSpace::Encoding::VIDEO
> > + * \brief A place-holder for video streams which will be resolved to one
> > + * of REC601, REC709 or REC2020 once the video resolution is known.
> > + */
> > +
> > +/**
> > + * \enum ColorSpace::TransferFunction
> > + * \brief The transfer function used for this colour space.
> > + *
> > + * \var ColorSpace::TransferFunction::UNDEFINED
> > + * \brief The transfer function is not specified.
> > + * \var ColorSpace::TransferFunction::IDENTITY
> > + * \brief This color space uses an identity transfer function.
> > + * \var ColorSpace::TransferFunction::SRGB
> > + * \brief sRGB transfer function.
> > + * \var ColorSpace::TransferFunction::REC709
> > + * \brief Rec709 transfer function.
> > + */
> > +
> > +/**
> > + * \enum ColorSpace::Range
> > + * \brief The range (sometimes "quantisation") for this color space.
> > + *
> > + * \var ColorSpace::Range::UNDEFINED
> > + * \brief The range is not specified.
> > + * \var ColorSpace::Range::FULL
> > + * \brief This color space uses full range pixel values.
> > + * \var ColorSpace::Range::LIMITED
> > + * \brief This color space uses limited range pixel values.
> > + */
> > +
> > +/**
> > + * \fn ColorSpace::ColorSpace(Encoding e, TransferFunction t, Range r)
> > + * \brief Construct a ColorSpace from explicit values
> > + * \param[in] e The encoding for the color primaries
> > + * \param[in] t The transfer function for the color space
> > + * \param[in] r The range of the pixel values in this color space
> > + */
> > +
> > +/**
> > + * \fn ColorSpace::ColorSpace()
> > + * \brief Construct a color space with undefined encoding, transfer function
> > + * and range
> > + */
> > +
> > +/**
> > + * \fn ColorSpace::isFullyDefined() const
> > + * \brief Return whether all the fields of the color space are defined.
> > + */
> > +
> > +/**
> > + * \brief Assemble and return a readable string representation of the
> > + * ColorSpace
> > + * \return A string describing the ColorSpace
> > + */
> > +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)]);
> > +}
> > +
> > +/**
> > + * \var ColorSpace::encoding
> > + * \brief The encoding of the color primaries
> > + */
> > +
> > +/**
> > + * \var ColorSpace::transferFunction
> > + * \brief The transfer function for this color space.
> > + */
> > +
> > +/**
> > + * \var ColorSpace::range
> > + * \brief The pixel range used by this color space.
> > + */
> > +
> > +/**
> > + * \var ColorSpace::UNDEFINED
> > + * \brief A constant representing a fully undefined color space.
> > + */
> > +
> > +/**
> > + * \var ColorSpace::RAW
> > + * \brief A constant representing a raw color space (from a sensor).
> > + */
> > +
> > +/**
> > + * \var ColorSpace::JFIF
> > + * \brief A constant representing the JFIF color space usually used for
> > + * encoding JPEG images.
> > + */
> > +
> > +/**
> > + * \var ColorSpace::SMPTE170M
> > + * \brief A constant representing the SMPTE170M color space (sometimes also
> > + * referred to as "full range BT601").
> > + */
> > +
> > +/**
> > + * \var ColorSpace::REC709
> > + * \brief A constant representing the REC709 color space.
> > + */
> > +
> > +/**
> > + * \var ColorSpace::REC2020
> > + * \brief A constant representing the REC2020 color space.
> > + */
> > +
> > +/**
> > + * \var ColorSpace::VIDEO
> > + * \brief A constant that video streams can use to indicate the "default"
> > + * color space for a video of this resolution, once that is is known. For
> > + * exmample, SD streams would interpret this as SMPTE170M, HD streams as
> > + * REC709 and ultra HD as REC2020.
> > + */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 4f085801..e0748840 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -8,6 +8,7 @@ libcamera_sources = files([
> >      'camera_manager.cpp',
> >      'camera_sensor.cpp',
> >      'camera_sensor_properties.cpp',
> > +    'color_space.cpp',
> >      'controls.cpp',
> >      'control_serializer.cpp',
> >      'control_validator.cpp',