From patchwork Mon Aug 29 16:37:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 17232 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id E535AC327D for ; Mon, 29 Aug 2022 16:38:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8709C61FC3; Mon, 29 Aug 2022 18:38:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1661791093; bh=psmaUBKUBww47lbfze0f1NvxjgvL30karPZR5FG9adM=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=h6gcpvkbvrzqjtLJ1NrDjbD/1xd49WrcsHY99uyOBr4bpFycyyfSba4frkI4GB19z nbN8sbk1vIIbd5avitv5wY/Zc98MvaOWAl5XG9gJcfLiz87eECL6B0whFgm26ZkmFV T0Qyi3RHWKURtfBakDTaxAdZKPBVU6g0UvJXGERu1fQ77SAYtlTU76RgcazJe+wnv8 2scHBFBsj3OZoApVpFrfaK6pKFscQMMhyaqqlTjQhVJtPPdmLQhTrKT06jbDKgKG2y zQtTRENAWeEhXRnc0/D9C5lNVd1Kr+RNtJTNUBWWGPKvGsDZOy1opfuutL+zBgTbos gX/Q8Csztdlzw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4ED7461FBA for ; Mon, 29 Aug 2022 18:38:11 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="DMLUJFJ0"; dkim-atps=neutral Received: from umang.jainideasonboard.com (unknown [IPv6:2401:4900:1f3f:1548:78ac:4a3:edc3:c28a]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 16058505; Mon, 29 Aug 2022 18:38:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1661791091; bh=psmaUBKUBww47lbfze0f1NvxjgvL30karPZR5FG9adM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DMLUJFJ0aNDr6U/B83K4NmUiy2fI9meRJEJIMNgb5Pm1tVRilEks5m3kX+3ZdCYr3 6Nw3n0l2r90dc91fAvsUwlKqLXLvgIzSsMC53X5tMV4wHiq2eG2o7tUh9ARxvIqP+Q nYY+69oHfBxWSBGSNqVGyJ7EqAeBSNgC58u9rU+U= To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Aug 2022 22:07:40 +0530 Message-Id: <20220829163742.1006102-6-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20220829163742.1006102-1-umang.jain@ideasonboard.com> References: <20220829163742.1006102-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 5/7] libcamera: color_space: Move color space adjustment to ColorSpace class X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Umang Jain via libcamera-devel From: Umang Jain Reply-To: Umang Jain Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 Signed-off-by: Laurent Pinchart Reviewed-by: Umang Jain --- 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); static std::optional 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; + 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 +#include "libcamera/internal/formats.h" + /** * \file color_space.h * \brief Class and enums to represent color spaces @@ -398,6 +400,105 @@ std::optional 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