<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for your work.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 20 Oct 2021 at 12:08, David Plowman <<a href="mailto:david.plowman@raspberrypi.com">david.plowman@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This class represents a color space by defining its color primaries,<br>
YCbCr encoding, the transfer (gamma) function it uses, and whether the<br>
output is full or limited range.<br>
<br>
Signed-off-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
---<br>
include/libcamera/color_space.h | 88 +++++++++++<br>
include/libcamera/meson.build | 1 +<br>
src/libcamera/color_space.cpp | 256 ++++++++++++++++++++++++++++++++<br>
src/libcamera/meson.build | 1 +<br>
4 files changed, 346 insertions(+)<br>
create mode 100644 include/libcamera/color_space.h<br>
create mode 100644 src/libcamera/color_space.cpp<br>
<br>
diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h<br>
new file mode 100644<br>
index 00000000..2af9da31<br>
--- /dev/null<br>
+++ b/include/libcamera/color_space.h<br>
@@ -0,0 +1,88 @@<br>
+/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
+/*<br>
+ * Copyright (C) 2021, Raspberry Pi (Trading) Limited<br>
+ *<br>
+ * color_space.h - color space definitions<br>
+ */<br>
+<br>
+#ifndef __LIBCAMERA_COLOR_SPACE_H__<br>
+#define __LIBCAMERA_COLOR_SPACE_H__<br>
+<br>
+#include <string><br>
+<br>
+namespace libcamera {<br>
+<br>
+class ColorSpace<br>
+{<br>
+public:<br>
+ enum class Primaries : int {<br>
+ Undefined,<br>
+ Raw,<br>
+ Smpte170m,<br>
+ Rec709,<br>
+ Rec2020,<br>
+ };<br>
+<br>
+ enum class YcbcrEncoding : int {<br>
+ Undefined,<br>
+ Rec601,<br>
+ Rec709,<br>
+ Rec2020,<br>
+ };<br>
+<br>
+ enum class TransferFunction : int {<br>
+ Undefined,<br>
+ Linear,<br>
+ Srgb,<br>
+ Rec709,<br>
+ };<br>
+<br>
+ enum class Range : int {<br>
+ Undefined,<br>
+ Full,<br>
+ Limited,<br>
+ };<br>
+<br>
+ constexpr ColorSpace()<br>
+ : ColorSpace(Primaries::Undefined, YcbcrEncoding::Undefined, TransferFunction::Undefined, Range::Undefined)<br>
+ {<br>
+ }<br>
+<br>
+ constexpr ColorSpace(Primaries p, YcbcrEncoding e, TransferFunction t, Range r)<br>
+ : primaries(p), ycbcrEncoding(e), transferFunction(t), range(r)<br>
+ {<br>
+ }<br>
+<br>
+ static const ColorSpace Undefined;<br>
+ static const ColorSpace Raw;<br>
+ static const ColorSpace Jpeg;<br>
+ static const ColorSpace Smpte170m;<br>
+ static const ColorSpace Rec709;<br>
+ static const ColorSpace Rec2020;<br>
+<br>
+ Primaries primaries;<br>
+ YcbcrEncoding ycbcrEncoding;<br>
+ TransferFunction transferFunction;<br>
+ Range range;<br>
+<br>
+ bool isFullyDefined() const;<br></blockquote><div><br></div><div>Other classes seem to use an isValid() member function. Should we do the same here?</div><div>Granted, this is not exactly the same, a false return value from isFullyDefined() might be</div><div>allowed for a valid colourspace?</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ const std::string toString() const;<br>
+};<br>
+<br>
+constexpr ColorSpace ColorSpace::Undefined = { Primaries::Undefined, YcbcrEncoding::Undefined, TransferFunction::Undefined, Range::Undefined };<br>
+constexpr ColorSpace ColorSpace::Raw = { Primaries::Raw, YcbcrEncoding::Rec601, TransferFunction::Linear, Range::Full };<br>
+constexpr ColorSpace ColorSpace::Jpeg = { Primaries::Rec709, YcbcrEncoding::Rec601, TransferFunction::Srgb, Range::Full };<br>
+constexpr ColorSpace ColorSpace::Smpte170m = { Primaries::Smpte170m, YcbcrEncoding::Rec601, TransferFunction::Rec709, Range::Limited };<br>
+constexpr ColorSpace ColorSpace::Rec709 = { Primaries::Rec709, YcbcrEncoding::Rec709, TransferFunction::Rec709, Range::Limited };<br>
+constexpr ColorSpace ColorSpace::Rec2020 = { Primaries::Rec2020, YcbcrEncoding::Rec2020, TransferFunction::Rec709, Range::Limited };<br>
+<br>
+bool operator==(const ColorSpace &lhs, const ColorSpace &rhs);<br>
+static inline bool operator!=(const ColorSpace &lhs, const ColorSpace &rhs)<br>
+{<br>
+ return !(lhs == rhs);<br>
+}<br>
+<br>
+} /* namespace libcamera */<br>
+<br>
+#endif /* __LIBCAMERA_COLOR_SPACE_H__ */<br>
diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build<br>
index 7155ff20..131e1740 100644<br>
--- a/include/libcamera/meson.build<br>
+++ b/include/libcamera/meson.build<br>
@@ -5,6 +5,7 @@ libcamera_include_dir = 'libcamera' / 'libcamera'<br>
libcamera_public_headers = files([<br>
'camera.h',<br>
'camera_manager.h',<br>
+ 'color_space.h',<br>
'compiler.h',<br>
'controls.h',<br>
'file_descriptor.h',<br>
diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp<br>
new file mode 100644<br>
index 00000000..aec4107f<br>
--- /dev/null<br>
+++ b/src/libcamera/color_space.cpp<br>
@@ -0,0 +1,256 @@<br>
+/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
+/*<br>
+ * Copyright (C) 2021, Raspberry Pi (Trading) Limited<br>
+ *<br>
+ * color_space.cpp - color spaces.<br>
+ */<br>
+<br>
+#include <libcamera/color_space.h><br>
+<br>
+#include <algorithm><br>
+#include <sstream><br>
+#include <vector><br>
+<br>
+/**<br>
+ * \file color_space.h<br>
+ * \brief Class and enums to represent color spaces<br>
+ */<br>
+<br>
+namespace libcamera {<br>
+<br>
+/**<br>
+ * \class ColorSpace<br>
+ * \brief Class to describe a color space<br>
+ *<br>
+ * The ColorSpace class defines the color primaries, the Y'CbCr encoding,<br>
+ * the transfer function associated with the color space, and the range<br>
+ * (sometimes also referred to as the quantisation) of the color space.<br>
+ *<br>
+ * Certain combinations of these fields form well-known standard color<br>
+ * spaces such as "JPEG" or "REC709". Applications must not request color<br>
+ * spaces with undefined fields, but the "Undefined" value may be<br>
+ * returned if the camera drivers decide to use a color space that is<br>
+ * not recognised by the ColorSpace class.<br>
+ *<br>
+ * For more information on the specific color spaces described here, please<br>
+ * see:<br>
+ *<br>
+ * <a href="<a href="https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-srgb" rel="noreferrer" target="_blank">https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-srgb</a>">sRGB</a> and <a href="<a href="https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-jpeg" rel="noreferrer" target="_blank">https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-jpeg</a>">JPEG</a><br>
+>* <a href="<a href="https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-smpte-170m" rel="noreferrer" target="_blank">https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-smpte-170m</a>">SMPTE 170M</a><br>
+ * <a href="<a href="https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-rec709" rel="noreferrer" target="_blank">https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-rec709</a>">Rec.709</a><br>
+ * <a href="<a href="https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-bt2020" rel="noreferrer" target="_blank">https://www.kernel.org/doc/html/v4.8/media/uapi/v4l/pixfmt-007.html#col-bt2020</a>">Rec.2020</a><br>
+ */<br>
+<br>
+/**<br>
+ * \enum ColorSpace::Primaries<br>
+ * \brief The color primaries for this color space<br>
+ *<br>
+ * \var ColorSpace::Primaries::Undefined<br>
+ * \brief The color primaries are undefined<br>
+ * \var ColorSpace::Primaries::Raw<br>
+ * \brief These are raw colors directly from a sensor<br>
+ * \var ColorSpace::Primaries::Smpte170m<br>
+ * \brief SMPTE 170M color primaries<br>
+ * \var ColorSpace::Primaries::Rec709<br>
+ * \brief Rec.709 color primaries<br>
+ * \var ColorSpace::Primaries::Rec2020<br>
+ * \brief Rec.2020 color primaries<br>
+ */<br>
+<br>
+/**<br>
+ * \enum ColorSpace::YcbcrEncoding<br>
+ * \brief The Y'CbCr encoding<br>
+ *<br>
+ * \var ColorSpace::YcbcrEncoding::Undefined<br>
+ * \brief The Y'CbCr encoding is undefined<br>
+ * \var ColorSpace::YcbcrEncoding::Rec601<br>
+ * \brief Rec.601 Y'CbCr encoding<br>
+ * \var ColorSpace::YcbcrEncoding::Rec709<br>
+ * \brief Rec.709 Y'CbCr encoding<br>
+ * \var ColorSpace::YcbcrEncoding::Rec2020<br>
+ * \brief Rec.2020 Y'CbCr encoding<br>
+ */<br>
+<br>
+/**<br>
+ * \enum ColorSpace::TransferFunction<br>
+ * \brief The transfer function used for this color space<br>
+ *<br>
+ * \var ColorSpace::TransferFunction::Undefined<br>
+ * \brief The transfer function is not specified<br>
+ * \var ColorSpace::TransferFunction::Linear<br>
+ * \brief This color space uses a linear (identity) transfer function<br>
+ * \var ColorSpace::TransferFunction::Srgb<br>
+ * \brief sRGB transfer function<br>
+ * \var ColorSpace::TransferFunction::Rec709<br>
+ * \brief Rec709 transfer function<br>
+ */<br>
+<br>
+/**<br>
+ * \enum ColorSpace::Range<br>
+ * \brief The range (sometimes "quantisation") for this color space<br>
+ *<br>
+ * \var ColorSpace::Range::Undefined<br>
+ * \brief The range is not specified<br>
+ * \var ColorSpace::Range::Full<br>
+ * \brief This color space uses full range pixel values<br>
+ * \var ColorSpace::Range::Limited<br>
+ * \brief This color space uses limited range pixel values, being<br>
+ * 16 to 235 for Y' and 16 to 240 for Cb and Cr<br></blockquote><div><br></div><div>Perhaps add the range for 10-bit as well?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ */<br>
+<br>
+/**<br>
+ * \fn ColorSpace::ColorSpace(Encoding e, TransferFunction t, Range r)<br>
+ * \brief Construct a ColorSpace from explicit values<br>
+ * \param[in] e The Y'CbCr encoding<br>
+ * \param[in] t The transfer function for the color space<br>
+ * \param[in] r The range of the pixel values in this color space<br>
+ */<br>
+<br>
+/**<br>
+ * \fn ColorSpace::ColorSpace()<br>
+ * \brief Construct a color space with undefined encoding, transfer function<br>
+ * and range<br>
+ */<br>
+<br>
+/**<br>
+ * \brief Check if all the fields of the color space are defined<br>
+ * \return Return true if all the fields of the color space are defined,<br>
+ * otherwise false<br>
+ */<br>
+bool ColorSpace::isFullyDefined() const<br>
+{<br>
+ return primaries != Primaries::Undefined &&<br>
+ ycbcrEncoding != YcbcrEncoding::Undefined &&<br>
+ transferFunction != TransferFunction::Undefined &&<br>
+ range != Range::Undefined;<br>
+}<br>
+<br>
+/**<br>
+ * \brief Assemble and return a readable string representation of the<br>
+ * ColorSpace<br>
+ * \return A string describing the ColorSpace<br>
+ */<br>
+const std::string ColorSpace::toString() const<br>
+{<br>
+ /* Print out a brief name only for standard color sapces. */<br>
+<br>
+ static const std::vector<std::pair<ColorSpace, const char *>> colorSpaceNames = {<br>
+ { ColorSpace::Undefined, "Undefined" },<br>
+ { ColorSpace::Raw, "Raw" },<br>
+ { ColorSpace::Jpeg, "Jpeg" },<br>
+ { ColorSpace::Smpte170m, "Smpte170m" },<br>
+ { ColorSpace::Rec709, "Rec709" },<br>
+ { ColorSpace::Rec2020, "Rec2020" },<br>
+ };<br>
+ auto it = std::find_if(colorSpaceNames.begin(), colorSpaceNames.end(),<br>
+ [this](const auto &item) {<br>
+ return *this == item.first;<br>
+ });<br></blockquote><div><br></div><div>Perhaps this might be better done with a std::map or even embedding the string</div><div>into a member variable? Not too fussed either way.</div><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ if (it != colorSpaceNames.end())<br>
+ return std::string(it->second);<br>
+<br>
+ static const char *primariesNames[] = {<br>
+ "Undefined",<br>
+ "Raw",<br>
+ "Smpte170m",<br>
+ "Rec709",<br>
+ "Rec2020",<br>
+ };<br>
+ static const char *encodingNames[] = {<br>
+ "Undefined",<br>
+ "Rec601",<br>
+ "Rec709",<br>
+ "Rec2020",<br>
+ };<br>
+ static const char *transferFunctionNames[] = {<br>
+ "Undefined",<br>
+ "Linear",<br>
+ "Srgb",<br>
+ "Rec709",<br>
+ };<br>
+ static const char *rangeNames[] = {<br>
+ "Undefined",<br>
+ "Full",<br>
+ "Limited",<br>
+ };<br>
+<br>
+ std::stringstream ss;<br>
+ ss << std::string(primariesNames[static_cast<int>(primaries)]) << "/"<br>
+ << std::string(encodingNames[static_cast<int>(ycbcrEncoding)]) << "/"<br>
+ << std::string(transferFunctionNames[static_cast<int>(transferFunction)]) << "/"<br>
+ << std::string(rangeNames[static_cast<int>(range)]);<br>
+<br>
+ return ss.str();<br>
+}<br>
+<br>
+/**<br>
+ * \var ColorSpace::primaries<br>
+ * \brief The color primaries<br>
+ */<br>
+<br>
+/**<br>
+ * \var ColorSpace::ycbcrEncoding<br>
+ * \brief The Y'CbCr encoding<br>
+ */<br>
+<br>
+/**<br>
+ * \var ColorSpace::transferFunction<br>
+ * \brief The transfer function for this color space<br>
+ */<br>
+<br>
+/**<br>
+ * \var ColorSpace::range<br>
+ * \brief The pixel range used by this color space<br>
+ */<br>
+<br>
+/**<br>
+ * \var ColorSpace::Undefined<br>
+ * \brief A constant representing a fully undefined color space<br>
+ */<br>
+<br>
+/**<br>
+ * \var ColorSpace::Raw<br>
+ * \brief A constant representing a raw color space (from a sensor)<br>
+ */<br>
+<br>
+/**<br>
+ * \var ColorSpace::Jpeg<br>
+ * \brief A constant representing the JPEG color space used for<br>
+ * encoding JPEG images (and regarded as being the same as the sRGB<br>
+ * color space)<br>
+ */<br>
+<br>
+/**<br>
+ * \var ColorSpace::Smpte170m<br>
+ * \brief A constant representing the SMPTE170M color space<br>
+ */<br>
+<br>
+/**<br>
+ * \var ColorSpace::Rec709<br>
+ * \brief A constant representing the Rec.709 color space<br>
+ */<br>
+<br>
+/**<br>
+ * \var ColorSpace::Rec2020<br>
+ * \brief A constant representing the Rec.2020 color space<br>
+ */<br>
+<br>
+/**<br>
+ * \brief Compare color spaces for equality<br>
+ * \return True if the two color spaces are identical, false otherwise<br>
+ */<br>
+bool operator==(const ColorSpace &lhs, const ColorSpace &rhs)<br>
+{<br>
+ return lhs.primaries == rhs.primaries &&<br>
+ lhs.ycbcrEncoding == rhs.ycbcrEncoding &&<br>
+ lhs.transferFunction == rhs.transferFunction &&<br>
+ lhs.range == rhs.range;<br>
+}<br>
+<br>
+/**<br>
+ * \fn bool operator!=(const ColorSpace &lhs, const ColorSpace &rhs)<br>
+ * \brief Compare color spaces for inequality<br>
+ * \return True if the two color spaces are not identical, false otherwise<br>
+ */<br>
+<br>
+} /* namespace libcamera */<br>
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build<br>
index 243dd3c1..8dc5d39d 100644<br>
--- a/src/libcamera/meson.build<br>
+++ b/src/libcamera/meson.build<br>
@@ -8,6 +8,7 @@ libcamera_sources = files([<br>
'camera_manager.cpp',<br>
'camera_sensor.cpp',<br>
'camera_sensor_properties.cpp',<br>
+ 'color_space.cpp',<br>
'controls.cpp',<br>
'control_serializer.cpp',<br>
'control_validator.cpp',<br>
-- <br>
2.20.1<br>
<br>
</blockquote></div></div>