From patchwork Tue Aug 30 07:47:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 17254 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 B4E77C3272 for ; Tue, 30 Aug 2022 07:47:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7666F61FC1; Tue, 30 Aug 2022 09:47:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1661845670; 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:Cc: From; b=RlnV7NcqpTDgPX/N13wM5k/eb5LGeXKQgF334lqsO+roa2C5a1FUz3gczbcG+0F0N sRCt6gvAr/zOsV4UhY+xjLbHJu8DM07pAR1OSvp8DcLkaX8VfML9fWwJLFmkHc3V7K M10iVThTu6PL19Fcp1iKd9Fl4aYkqqskzu1qWtJYQ84huxovKmBYxeAg4LX3H9OdN6 xHSWjP42/6K6UNiC5bcdpQJT5hgZPVl+vUJzqzahI5bS71AmPpvU0D0G4jIzNdGuEo kEcQP1ug7Nm13F0CkeZJMGmXylhMxTcllS3tsBwYlRt8ya4ga3YdGtECZP0EkAegRP CIx39c9R+ClrQ== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 87ED761FB9 for ; Tue, 30 Aug 2022 09:47:48 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="dFpnJSv4"; 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 18CD4481; Tue, 30 Aug 2022 09:47:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1661845668; bh=psmaUBKUBww47lbfze0f1NvxjgvL30karPZR5FG9adM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=dFpnJSv4E4/dS2e2OAqgnQmNEHXzJ9XpRknFD06TAoCgRf8JEJGmpAxq+PQPMt+JF FOs+BKzyTjILM9jKiBK9J5YIz2PmPnRI53YakM03hCY72p2h4FOkQkI6T1c4Ao5K+6 g7qqjXJEpCvAaYrlsPCNiLmAzWBnEODNQ9uCVMn0= To: libcamera-devel@lists.libcamera.org Date: Tue, 30 Aug 2022 13:17:23 +0530 Message-Id: <20220830074725.1059643-6-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20220830074725.1059643-1-umang.jain@ideasonboard.com> References: <20220830074725.1059643-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 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 Cc: rishikeshdonadkar@gmail.com 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 Reviewed-by: Paul Elder --- 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