Message ID | 20220830074725.1059643-6-umang.jain@ideasonboard.com |
---|---|
State | Accepted |
Delegated to: | Umang Jain |
Headers | show |
Series |
|
Related | show |
On Tue, Aug 30, 2022 at 01:17:23PM +0530, Umang Jain via libcamera-devel wrote: > The CameraConfiguration::validateColorSpaces() function performs color > space validation on a camera configuration, by validating the color > space of each stream individually, and optionally ensuring that all > streams share the same color space. The individual validation is very > basic, limited to ensuring that raw formats use a raw color space. > > Color spaces are more constrained than that: > > - The Y'CbCr encoding and quantization range for RGB formats must be > YcbcrEncoding::None and Range::Full respectively. > - The Y'CbCr encoding for YUV formats must not be YcbcrEncoding::None. > > Instead of open-coding these constraints in the validateColorSpaces() > function, create a new ColorSpace::adjust() function to centralize color > space validation and adjustment, and use it in validateColorSpaces(). > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/color_space.h | 4 ++ > src/libcamera/camera.cpp | 43 ++++++-------- > src/libcamera/color_space.cpp | 101 ++++++++++++++++++++++++++++++++ > 3 files changed, 122 insertions(+), 26 deletions(-) > > diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h > index f493f72d..6d6c2829 100644 > --- a/include/libcamera/color_space.h > +++ b/include/libcamera/color_space.h > @@ -12,6 +12,8 @@ > > namespace libcamera { > > +class PixelFormat; > + > class ColorSpace > { > public: > @@ -61,6 +63,8 @@ public: > static std::string toString(const std::optional<ColorSpace> &colorSpace); > > static std::optional<ColorSpace> fromString(const std::string &str); > + > + bool adjust(PixelFormat format); > }; > > bool operator==(const ColorSpace &lhs, const ColorSpace &rhs); > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index a5c3aabe..9fe29ca9 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -317,17 +317,6 @@ std::size_t CameraConfiguration::size() const > return config_.size(); > } > > -namespace { > - > -bool isRaw(const PixelFormat &pixFmt) > -{ > - const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); > - return info.isValid() && > - info.colourEncoding == PixelFormatInfo::ColourEncodingRAW; > -} > - > -} /* namespace */ > - > /** > * \enum CameraConfiguration::ColorSpaceFlag > * \brief Specify the behaviour of validateColorSpaces > @@ -368,29 +357,31 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF > Status status = Valid; > > /* > - * Set all raw streams to the Raw color space, and make a note of the largest > - * non-raw stream with a defined color space (if there is one). > + * Set all raw streams to the Raw color space, and make a note of the > + * largest non-raw stream with a defined color space (if there is one). > */ > - int index = -1; > + std::optional<ColorSpace> colorSpace; > + > for (auto [i, cfg] : utils::enumerate(config_)) { > - if (isRaw(cfg.pixelFormat)) { > - if (cfg.colorSpace != ColorSpace::Raw) { > - cfg.colorSpace = ColorSpace::Raw; > - status = Adjusted; > - } > - } else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size)) { > - index = i; > - } > + if (!cfg.colorSpace) > + continue; > + > + if (cfg.colorSpace->adjust(cfg.pixelFormat)) > + status = Adjusted; > + > + if (cfg.colorSpace != ColorSpace::Raw && > + (!colorSpace || cfg.size > config_[i].size)) > + colorSpace = cfg.colorSpace; > } > > - if (index < 0 || !(flags & ColorSpaceFlag::StreamsShareColorSpace)) > + if (!colorSpace || !(flags & ColorSpaceFlag::StreamsShareColorSpace)) > return status; > > /* Make all output color spaces the same, if requested. */ > for (auto &cfg : config_) { > - if (!isRaw(cfg.pixelFormat) && > - cfg.colorSpace != config_[index].colorSpace) { > - cfg.colorSpace = config_[index].colorSpace; > + if (cfg.colorSpace != ColorSpace::Raw && > + cfg.colorSpace != colorSpace) { > + cfg.colorSpace = colorSpace; > status = Adjusted; > } > } > diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp > index ec34620a..7356bf7d 100644 > --- a/src/libcamera/color_space.cpp > +++ b/src/libcamera/color_space.cpp > @@ -16,6 +16,8 @@ > > #include <libcamera/base/utils.h> > > +#include "libcamera/internal/formats.h" > + > /** > * \file color_space.h > * \brief Class and enums to represent color spaces > @@ -398,6 +400,105 @@ std::optional<ColorSpace> ColorSpace::fromString(const std::string &str) > return colorSpace; > } > > +/** > + * \brief Adjust the color space to match a pixel format > + * \param[in] format The pixel format > + * > + * Not all combinations of pixel formats and color spaces make sense. For > + * instance, nobody uses a limited quantization range with raw Bayer formats, > + * and the YcbcrEncoding::None encoding isn't valid for YUV formats. This > + * function adjusts the ColorSpace to make it compatible with the given \a > + * format, by applying the following rules: > + * > + * - The color space for RAW formats must be Raw. > + * - The Y'CbCr encoding and quantization range for RGB formats must be > + * YcbcrEncoding::None and Range::Full respectively. > + * - The Y'CbCr encoding for YUV formats must not be YcbcrEncoding::None. The > + * best encoding is in that case guessed based on the primaries and transfer > + * function. > + * > + * \return True if the color space has been adjusted, or false if it was > + * already compatible with the format and hasn't been changed > + */ > +bool ColorSpace::adjust(PixelFormat format) > +{ > + const PixelFormatInfo &info = PixelFormatInfo::info(format); > + bool adjusted = false; > + > + switch (info.colourEncoding) { > + case PixelFormatInfo::ColourEncodingRAW: > + /* Raw formats must use the raw color space. */ > + if (*this != ColorSpace::Raw) { > + *this = ColorSpace::Raw; > + adjusted = true; > + } > + break; > + > + case PixelFormatInfo::ColourEncodingRGB: > + /* > + * RGB formats can't have a Y'CbCr encoding, and must use full > + * range quantization. > + */ > + if (ycbcrEncoding != YcbcrEncoding::None) { > + ycbcrEncoding = YcbcrEncoding::None; > + adjusted = true; > + } > + > + if (range != Range::Full) { > + range = Range::Full; > + adjusted = true; > + } > + break; > + > + case PixelFormatInfo::ColourEncodingYUV: > + if (ycbcrEncoding != YcbcrEncoding::None) > + break; > + > + /* > + * YUV formats must have a Y'CbCr encoding. Infer the most > + * probable option from the transfer function and primaries. > + */ > + switch (transferFunction) { > + case TransferFunction::Linear: > + /* > + * Linear YUV is not used in any standard color space, > + * pick the widely supported and used Rec601 as default. > + */ > + ycbcrEncoding = YcbcrEncoding::Rec601; > + break; > + > + case TransferFunction::Rec709: > + switch (primaries) { > + /* Raw should never happen. */ > + case Primaries::Raw: > + case Primaries::Smpte170m: > + ycbcrEncoding = YcbcrEncoding::Rec601; > + break; > + case Primaries::Rec709: > + ycbcrEncoding = YcbcrEncoding::Rec709; > + break; > + case Primaries::Rec2020: > + ycbcrEncoding = YcbcrEncoding::Rec2020; > + break; > + } > + break; > + > + case TransferFunction::Srgb: > + /* > + * Only the sYCC color space uses the sRGB transfer > + * function, the corresponding encoding is Rec601. > + */ > + ycbcrEncoding = YcbcrEncoding::Rec601; > + break; > + } > + > + adjusted = true; > + break; > + } > + > + return adjusted; > +} > + > /** > * \brief Compare color spaces for equality > * \return True if the two color spaces are identical, false otherwise > -- > 2.37.2 >
diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h index f493f72d..6d6c2829 100644 --- a/include/libcamera/color_space.h +++ b/include/libcamera/color_space.h @@ -12,6 +12,8 @@ namespace libcamera { +class PixelFormat; + class ColorSpace { public: @@ -61,6 +63,8 @@ public: static std::string toString(const std::optional<ColorSpace> &colorSpace); static std::optional<ColorSpace> fromString(const std::string &str); + + bool adjust(PixelFormat format); }; bool operator==(const ColorSpace &lhs, const ColorSpace &rhs); diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index a5c3aabe..9fe29ca9 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -317,17 +317,6 @@ std::size_t CameraConfiguration::size() const return config_.size(); } -namespace { - -bool isRaw(const PixelFormat &pixFmt) -{ - const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt); - return info.isValid() && - info.colourEncoding == PixelFormatInfo::ColourEncodingRAW; -} - -} /* namespace */ - /** * \enum CameraConfiguration::ColorSpaceFlag * \brief Specify the behaviour of validateColorSpaces @@ -368,29 +357,31 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF Status status = Valid; /* - * Set all raw streams to the Raw color space, and make a note of the largest - * non-raw stream with a defined color space (if there is one). + * Set all raw streams to the Raw color space, and make a note of the + * largest non-raw stream with a defined color space (if there is one). */ - int index = -1; + std::optional<ColorSpace> colorSpace; + for (auto [i, cfg] : utils::enumerate(config_)) { - if (isRaw(cfg.pixelFormat)) { - if (cfg.colorSpace != ColorSpace::Raw) { - cfg.colorSpace = ColorSpace::Raw; - status = Adjusted; - } - } else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size)) { - index = i; - } + if (!cfg.colorSpace) + continue; + + if (cfg.colorSpace->adjust(cfg.pixelFormat)) + status = Adjusted; + + if (cfg.colorSpace != ColorSpace::Raw && + (!colorSpace || cfg.size > config_[i].size)) + colorSpace = cfg.colorSpace; } - if (index < 0 || !(flags & ColorSpaceFlag::StreamsShareColorSpace)) + if (!colorSpace || !(flags & ColorSpaceFlag::StreamsShareColorSpace)) return status; /* Make all output color spaces the same, if requested. */ for (auto &cfg : config_) { - if (!isRaw(cfg.pixelFormat) && - cfg.colorSpace != config_[index].colorSpace) { - cfg.colorSpace = config_[index].colorSpace; + if (cfg.colorSpace != ColorSpace::Raw && + cfg.colorSpace != colorSpace) { + cfg.colorSpace = colorSpace; status = Adjusted; } } diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp index ec34620a..7356bf7d 100644 --- a/src/libcamera/color_space.cpp +++ b/src/libcamera/color_space.cpp @@ -16,6 +16,8 @@ #include <libcamera/base/utils.h> +#include "libcamera/internal/formats.h" + /** * \file color_space.h * \brief Class and enums to represent color spaces @@ -398,6 +400,105 @@ std::optional<ColorSpace> ColorSpace::fromString(const std::string &str) return colorSpace; } +/** + * \brief Adjust the color space to match a pixel format + * \param[in] format The pixel format + * + * Not all combinations of pixel formats and color spaces make sense. For + * instance, nobody uses a limited quantization range with raw Bayer formats, + * and the YcbcrEncoding::None encoding isn't valid for YUV formats. This + * function adjusts the ColorSpace to make it compatible with the given \a + * format, by applying the following rules: + * + * - The color space for RAW formats must be Raw. + * - The Y'CbCr encoding and quantization range for RGB formats must be + * YcbcrEncoding::None and Range::Full respectively. + * - The Y'CbCr encoding for YUV formats must not be YcbcrEncoding::None. The + * best encoding is in that case guessed based on the primaries and transfer + * function. + * + * \return True if the color space has been adjusted, or false if it was + * already compatible with the format and hasn't been changed + */ +bool ColorSpace::adjust(PixelFormat format) +{ + const PixelFormatInfo &info = PixelFormatInfo::info(format); + bool adjusted = false; + + switch (info.colourEncoding) { + case PixelFormatInfo::ColourEncodingRAW: + /* Raw formats must use the raw color space. */ + if (*this != ColorSpace::Raw) { + *this = ColorSpace::Raw; + adjusted = true; + } + break; + + case PixelFormatInfo::ColourEncodingRGB: + /* + * RGB formats can't have a Y'CbCr encoding, and must use full + * range quantization. + */ + if (ycbcrEncoding != YcbcrEncoding::None) { + ycbcrEncoding = YcbcrEncoding::None; + adjusted = true; + } + + if (range != Range::Full) { + range = Range::Full; + adjusted = true; + } + break; + + case PixelFormatInfo::ColourEncodingYUV: + if (ycbcrEncoding != YcbcrEncoding::None) + break; + + /* + * YUV formats must have a Y'CbCr encoding. Infer the most + * probable option from the transfer function and primaries. + */ + switch (transferFunction) { + case TransferFunction::Linear: + /* + * Linear YUV is not used in any standard color space, + * pick the widely supported and used Rec601 as default. + */ + ycbcrEncoding = YcbcrEncoding::Rec601; + break; + + case TransferFunction::Rec709: + switch (primaries) { + /* Raw should never happen. */ + case Primaries::Raw: + case Primaries::Smpte170m: + ycbcrEncoding = YcbcrEncoding::Rec601; + break; + case Primaries::Rec709: + ycbcrEncoding = YcbcrEncoding::Rec709; + break; + case Primaries::Rec2020: + ycbcrEncoding = YcbcrEncoding::Rec2020; + break; + } + break; + + case TransferFunction::Srgb: + /* + * Only the sYCC color space uses the sRGB transfer + * function, the corresponding encoding is Rec601. + */ + ycbcrEncoding = YcbcrEncoding::Rec601; + break; + } + + adjusted = true; + break; + } + + return adjusted; +} + /** * \brief Compare color spaces for equality * \return True if the two color spaces are identical, false otherwise