[{"id":24789,"web_url":"https://patchwork.libcamera.org/comment/24789/","msgid":"<b26d12f1-e5ed-873d-b379-0720ababb538@ideasonboard.com>","date":"2022-08-26T09:15:47","subject":"Re: [libcamera-devel] [PATCH v2.1 4/6] libcamera: color_space: Move\n\tcolor space adjustment to ColorSpace class","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for v2.1\n\nOn 8/25/22 9:36 PM, Laurent Pinchart wrote:\n> From: Umang Jain <umang.jain@ideasonboard.com>\n>\n> The CameraConfiguration::validateColorSpaces() function performs color\n> space validation on a camera configuration, by validating the color\n> space of each stream individually, and optionally ensuring that all\n> streams share the same color space. The individual validation is very\n> basic, limited to ensuring that raw formats use a raw color space.\n>\n> Color spaces are more constrained than that:\n>\n> - The Y'CbCr encoding and quantization range for RGB formats must be\n>    YcbcrEncoding::None and Range::Full respectively.\n> - The Y'CbCr encoding for YUV formats must not be YcbcrEncoding::None.\n>\n> Instead of open-coding these constraints in the validateColorSpaces()\n> function, create a new ColorSpace::adjust() function to centralize color\n> space validation and adjustment, and use it in validateColorSpaces().\n>\n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n> Hi Umang,\n>\n> I thought it would be easier to send a patch than explaining exactly\n> what I was envisioning in the review.\n\nNice :-)\n> ---\n>   include/libcamera/color_space.h |   4 ++\n>   src/libcamera/camera.cpp        |  43 ++++++--------\n>   src/libcamera/color_space.cpp   | 101 ++++++++++++++++++++++++++++++++\n>   3 files changed, 122 insertions(+), 26 deletions(-)\n>\n> diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h\n> index 8030a264c66f..33e1e6999de1 100644\n> --- a/include/libcamera/color_space.h\n> +++ b/include/libcamera/color_space.h\n> @@ -12,6 +12,8 @@\n>   \n>   namespace libcamera {\n>   \n> +class PixelFormat;\n> +\n>   class ColorSpace\n>   {\n>   public:\n> @@ -59,6 +61,8 @@ public:\n>   \n>   \tstd::string toString() const;\n>   \tstatic std::string toString(const std::optional<ColorSpace> &colorSpace);\n> +\n> +\tbool adjust(PixelFormat format);\n>   };\n>   \n>   bool operator==(const ColorSpace &lhs, const ColorSpace &rhs);\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index a5c3aabeddbb..9fe29ca9dca5 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -317,17 +317,6 @@ std::size_t CameraConfiguration::size() const\n>   \treturn config_.size();\n>   }\n>   \n> -namespace {\n> -\n> -bool isRaw(const PixelFormat &pixFmt)\n> -{\n> -\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> -\treturn info.isValid() &&\n> -\t       info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> -}\n> -\n> -} /* namespace */\n> -\n>   /**\n>    * \\enum CameraConfiguration::ColorSpaceFlag\n>    * \\brief Specify the behaviour of validateColorSpaces\n> @@ -368,29 +357,31 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n>   \tStatus status = Valid;\n>   \n>   \t/*\n> -\t * Set all raw streams to the Raw color space, and make a note of the largest\n> -\t * non-raw stream with a defined color space (if there is one).\n> +\t * Set all raw streams to the Raw color space, and make a note of the\n> +\t * largest non-raw stream with a defined color space (if there is one).\n>   \t */\n> -\tint index = -1;\n> +\tstd::optional<ColorSpace> colorSpace;\n> +\n>   \tfor (auto [i, cfg] : utils::enumerate(config_)) {\n> -\t\tif (isRaw(cfg.pixelFormat)) {\n> -\t\t\tif (cfg.colorSpace != ColorSpace::Raw) {\n> -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> -\t\t\t\tstatus = Adjusted;\n> -\t\t\t}\n> -\t\t} else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size)) {\n> -\t\t\tindex = i;\n> -\t\t}\n> +\t\tif (!cfg.colorSpace)\n> +\t\t\tcontinue;\n> +\n> +\t\tif (cfg.colorSpace->adjust(cfg.pixelFormat))\n> +\t\t\tstatus = Adjusted;\n> +\n> +\t\tif (cfg.colorSpace != ColorSpace::Raw &&\n> +\t\t    (!colorSpace || cfg.size > config_[i].size))\n> +\t\t\tcolorSpace = cfg.colorSpace;\n>   \t}\n>   \n> -\tif (index < 0 || !(flags & ColorSpaceFlag::StreamsShareColorSpace))\n> +\tif (!colorSpace || !(flags & ColorSpaceFlag::StreamsShareColorSpace))\n>   \t\treturn status;\n>   \n>   \t/* Make all output color spaces the same, if requested. */\n>   \tfor (auto &cfg : config_) {\n> -\t\tif (!isRaw(cfg.pixelFormat) &&\n> -\t\t    cfg.colorSpace != config_[index].colorSpace) {\n> -\t\t\tcfg.colorSpace = config_[index].colorSpace;\n> +\t\tif (cfg.colorSpace != ColorSpace::Raw &&\n> +\t\t    cfg.colorSpace != colorSpace) {\n> +\t\t\tcfg.colorSpace = colorSpace;\n>   \t\t\tstatus = Adjusted;\n>   \t\t}\n>   \t}\n\nThis is already looking so much cleaner :-)\n> diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp\n> index cb47acf9727b..eefcf7616766 100644\n> --- a/src/libcamera/color_space.cpp\n> +++ b/src/libcamera/color_space.cpp\n> @@ -13,6 +13,8 @@\n>   #include <sstream>\n>   #include <utility>\n>   \n> +#include \"libcamera/internal/formats.h\"\n> +\n>   /**\n>    * \\file color_space.h\n>    * \\brief Class and enums to represent color spaces\n> @@ -203,6 +205,105 @@ std::string ColorSpace::toString() const\n>   \treturn ss.str();\n>   }\n>   \n> +/**\n> + * \\brief Adjust the color space to match a pixel format\n> + * \\param[in] format The pixel format\n> + *\n> + * Not all combinations of pixel formats and color spaces make sense. For\n> + * instance, nobody uses a limited quantization range with raw Bayer formats,\n> + * and the YcbcrEncoding::None encoding isn't valid for YUV formats. This\n> + * function adjusts the ColorSpace to make it compatible with the given \\a\n> + * format, by applying the following rules:\n> + *\n> + * - The color space for RAW formats must be Raw.\n> + * - The Y'CbCr encoding and quantization range for RGB formats must be\n> + *   YcbcrEncoding::None and Range::Full respectively.\n> + * - The Y'CbCr encoding for YUV formats must not be YcbcrEncoding::None. The\n> + *   best encoding is in that case guessed based on the primaries and transfer\n> + *   function.\n> + *\n> + * \\return True if the color space has been adjusted, or false if it was\n> + * already compatible with the format and hasn't been changed\n> + */\n> +bool ColorSpace::adjust(PixelFormat format)\n\nAh, ,my version of Colorspace::adjust API was terrible :S\n\nv2.1 looks very good!\n\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n\n> +{\n> +\tconst PixelFormatInfo &info = PixelFormatInfo::info(format);\n> +\tbool adjusted = false;\n> +\n> +\tswitch (info.colourEncoding) {\n> +\tcase PixelFormatInfo::ColourEncodingRAW:\n> +\t\t/* Raw formats must use the raw color space. */\n> +\t\tif (*this != ColorSpace::Raw) {\n> +\t\t\t*this = ColorSpace::Raw;\n> +\t\t\tadjusted = true;\n> +\t\t}\n> +\t\tbreak;\n> +\n> +\tcase PixelFormatInfo::ColourEncodingRGB:\n> +\t\t/*\n> +\t\t * RGB formats can't have a Y'CbCr encoding, and must use full\n> +\t\t * range quantization.\n> +\t\t */\n> +\t\tif (ycbcrEncoding != YcbcrEncoding::None) {\n> +\t\t\tycbcrEncoding = YcbcrEncoding::None;\n> +\t\t\tadjusted = true;\n> +\t\t}\n> +\n> +\t\tif (range != Range::Full) {\n> +\t\t\trange = Range::Full;\n> +\t\t\tadjusted = true;\n> +\t\t}\n> +\t\tbreak;\n> +\n> +\tcase PixelFormatInfo::ColourEncodingYUV:\n> +\t\tif (ycbcrEncoding != YcbcrEncoding::None)\n> +\t\t\tbreak;\n> +\n> +\t\t/*\n> +\t\t * YUV formats must have a Y'CbCr encoding. Infer the most\n> +\t\t * probable option from the transfer function and primaries.\n> +\t\t */\n> +\t\tswitch (transferFunction) {\n> +\t\tcase TransferFunction::Linear:\n> +\t\t\t/*\n> +\t\t\t * Linear YUV is not used in any standard color space,\n> +\t\t\t * pick the widely supported and used Rec601 as default.\n> +\t\t\t */\n> +\t\t\tycbcrEncoding = YcbcrEncoding::Rec601;\n> +\t\t\tbreak;\n> +\n> +\t\tcase TransferFunction::Rec709:\n> +\t\t\tswitch (primaries) {\n> +\t\t\t/* Raw should never happen. */\n> +\t\t\tcase Primaries::Raw:\n> +\t\t\tcase Primaries::Smpte170m:\n> +\t\t\t\tycbcrEncoding = YcbcrEncoding::Rec601;\n> +\t\t\t\tbreak;\n> +\t\t\tcase Primaries::Rec709:\n> +\t\t\t\tycbcrEncoding = YcbcrEncoding::Rec709;\n> +\t\t\t\tbreak;\n> +\t\t\tcase Primaries::Rec2020:\n> +\t\t\t\tycbcrEncoding = YcbcrEncoding::Rec2020;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\n> +\t\tcase TransferFunction::Srgb:\n> +\t\t\t/*\n> +\t\t\t * Only the sYCC color space uses the sRGB transfer\n> +\t\t\t * function, the corresponding encoding is Rec601.\n> +\t\t\t */\n> +\t\t\tycbcrEncoding = YcbcrEncoding::Rec601;\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tadjusted = true;\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn adjusted;\n> +}\n> +\n>   /**\n>    * \\brief Assemble and return a readable string representation of an\n>    * optional ColorSpace","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 67B10C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 26 Aug 2022 09:15:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8FAEA61FC0;\n\tFri, 26 Aug 2022 11:15:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5831961F9E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Aug 2022 11:15:54 +0200 (CEST)","from [IPV6:2401:4900:1f3f:806e:6647:8e5c:f441:ca9a] (unknown\n\t[IPv6:2401:4900:1f3f:806e:6647:8e5c:f441:ca9a])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D617B3F1;\n\tFri, 26 Aug 2022 11:15:52 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661505355;\n\tbh=3l7ubFnO0NgSTQYC+UOKLbk8cPk4FCB2iIYctUTcS0o=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=XRhK4pYLR+i0lVUFelCOQ2Q3tR0ZXK2K+PjM590j5kLL4F/4k0rzx4P+deS5SliDO\n\ttrWs0pFDcsiWv1aKz1crbvZTJGKAMDBYlJ+1gVZIe1WJsY8/1ecVBoEO4dkGRNiZ5r\n\tYrnIGQ3L0bUZwm7+cgb4FXLjv8m6VkeohLIz4iP3CkM6LbS+O3dtRwNdvoM7Z4wRpk\n\ttfIukl57ufiXhzQIYomczGzAERNWdLhW+fYFaaxWa+xZLeBpFFZHTfj+uJJFlwCSqs\n\tNyEA7iPi8RPNe7RAvDJVjh37u/dL7hK08LeUcABfqwMQp17UX0PbhlPtEVzeDLjV/s\n\toG/oKyISioh5Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661505354;\n\tbh=3l7ubFnO0NgSTQYC+UOKLbk8cPk4FCB2iIYctUTcS0o=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=hy2Iioxa0juodICm/2C7TFYdkeL1rWxWP8cxD+PUZiA2bJ8ZSZ4WekkvbyyFL5WnO\n\tFzl+2d296/5UMS9giT68Ow52DpIbsfAxos0AP0Rx94DAmuA8Ve4YgsO+MQjjLbckWm\n\tif9LyjbD6Vq75gjV359p29B9ZJpRIWBYkiG6niCU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hy2Iioxa\"; dkim-atps=neutral","Message-ID":"<b26d12f1-e5ed-873d-b379-0720ababb538@ideasonboard.com>","Date":"Fri, 26 Aug 2022 14:45:47 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.12.0","Content-Language":"en-US","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220824162425.71087-5-umang.jain@ideasonboard.com>\n\t<20220825160647.10436-1-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220825160647.10436-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2.1 4/6] libcamera: color_space: Move\n\tcolor space adjustment to ColorSpace class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Umang Jain via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Umang Jain <umang.jain@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":24791,"web_url":"https://patchwork.libcamera.org/comment/24791/","msgid":"<YwihONcDP0YAZl/h@pendragon.ideasonboard.com>","date":"2022-08-26T10:32:24","subject":"Re: [libcamera-devel] [PATCH v2.1 4/6] libcamera: color_space: Move\n\tcolor space adjustment to ColorSpace class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Fri, Aug 26, 2022 at 02:45:47PM +0530, Umang Jain wrote:\n> On 8/25/22 9:36 PM, Laurent Pinchart wrote:\n> > From: Umang Jain <umang.jain@ideasonboard.com>\n> >\n> > The CameraConfiguration::validateColorSpaces() function performs color\n> > space validation on a camera configuration, by validating the color\n> > space of each stream individually, and optionally ensuring that all\n> > streams share the same color space. The individual validation is very\n> > basic, limited to ensuring that raw formats use a raw color space.\n> >\n> > Color spaces are more constrained than that:\n> >\n> > - The Y'CbCr encoding and quantization range for RGB formats must be\n> >    YcbcrEncoding::None and Range::Full respectively.\n> > - The Y'CbCr encoding for YUV formats must not be YcbcrEncoding::None.\n> >\n> > Instead of open-coding these constraints in the validateColorSpaces()\n> > function, create a new ColorSpace::adjust() function to centralize color\n> > space validation and adjustment, and use it in validateColorSpaces().\n> >\n> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> > Hi Umang,\n> >\n> > I thought it would be easier to send a patch than explaining exactly\n> > what I was envisioning in the review.\n> \n> Nice :-)\n> \n> > ---\n> >   include/libcamera/color_space.h |   4 ++\n> >   src/libcamera/camera.cpp        |  43 ++++++--------\n> >   src/libcamera/color_space.cpp   | 101 ++++++++++++++++++++++++++++++++\n> >   3 files changed, 122 insertions(+), 26 deletions(-)\n> >\n> > diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h\n> > index 8030a264c66f..33e1e6999de1 100644\n> > --- a/include/libcamera/color_space.h\n> > +++ b/include/libcamera/color_space.h\n> > @@ -12,6 +12,8 @@\n> >   \n> >   namespace libcamera {\n> >   \n> > +class PixelFormat;\n> > +\n> >   class ColorSpace\n> >   {\n> >   public:\n> > @@ -59,6 +61,8 @@ public:\n> >   \n> >   \tstd::string toString() const;\n> >   \tstatic std::string toString(const std::optional<ColorSpace> &colorSpace);\n> > +\n> > +\tbool adjust(PixelFormat format);\n> >   };\n> >   \n> >   bool operator==(const ColorSpace &lhs, const ColorSpace &rhs);\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index a5c3aabeddbb..9fe29ca9dca5 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -317,17 +317,6 @@ std::size_t CameraConfiguration::size() const\n> >   \treturn config_.size();\n> >   }\n> >   \n> > -namespace {\n> > -\n> > -bool isRaw(const PixelFormat &pixFmt)\n> > -{\n> > -\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> > -\treturn info.isValid() &&\n> > -\t       info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> > -}\n> > -\n> > -} /* namespace */\n> > -\n> >   /**\n> >    * \\enum CameraConfiguration::ColorSpaceFlag\n> >    * \\brief Specify the behaviour of validateColorSpaces\n> > @@ -368,29 +357,31 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF\n> >   \tStatus status = Valid;\n> >   \n> >   \t/*\n> > -\t * Set all raw streams to the Raw color space, and make a note of the largest\n> > -\t * non-raw stream with a defined color space (if there is one).\n> > +\t * Set all raw streams to the Raw color space, and make a note of the\n> > +\t * largest non-raw stream with a defined color space (if there is one).\n> >   \t */\n> > -\tint index = -1;\n> > +\tstd::optional<ColorSpace> colorSpace;\n> > +\n> >   \tfor (auto [i, cfg] : utils::enumerate(config_)) {\n> > -\t\tif (isRaw(cfg.pixelFormat)) {\n> > -\t\t\tif (cfg.colorSpace != ColorSpace::Raw) {\n> > -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> > -\t\t\t\tstatus = Adjusted;\n> > -\t\t\t}\n> > -\t\t} else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size)) {\n> > -\t\t\tindex = i;\n> > -\t\t}\n> > +\t\tif (!cfg.colorSpace)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tif (cfg.colorSpace->adjust(cfg.pixelFormat))\n> > +\t\t\tstatus = Adjusted;\n> > +\n> > +\t\tif (cfg.colorSpace != ColorSpace::Raw &&\n> > +\t\t    (!colorSpace || cfg.size > config_[i].size))\n> > +\t\t\tcolorSpace = cfg.colorSpace;\n> >   \t}\n> >   \n> > -\tif (index < 0 || !(flags & ColorSpaceFlag::StreamsShareColorSpace))\n> > +\tif (!colorSpace || !(flags & ColorSpaceFlag::StreamsShareColorSpace))\n> >   \t\treturn status;\n> >   \n> >   \t/* Make all output color spaces the same, if requested. */\n> >   \tfor (auto &cfg : config_) {\n> > -\t\tif (!isRaw(cfg.pixelFormat) &&\n> > -\t\t    cfg.colorSpace != config_[index].colorSpace) {\n> > -\t\t\tcfg.colorSpace = config_[index].colorSpace;\n> > +\t\tif (cfg.colorSpace != ColorSpace::Raw &&\n> > +\t\t    cfg.colorSpace != colorSpace) {\n> > +\t\t\tcfg.colorSpace = colorSpace;\n> >   \t\t\tstatus = Adjusted;\n> >   \t\t}\n> >   \t}\n> \n> This is already looking so much cleaner :-)\n> \n> > diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp\n> > index cb47acf9727b..eefcf7616766 100644\n> > --- a/src/libcamera/color_space.cpp\n> > +++ b/src/libcamera/color_space.cpp\n> > @@ -13,6 +13,8 @@\n> >   #include <sstream>\n> >   #include <utility>\n> >   \n> > +#include \"libcamera/internal/formats.h\"\n> > +\n> >   /**\n> >    * \\file color_space.h\n> >    * \\brief Class and enums to represent color spaces\n> > @@ -203,6 +205,105 @@ std::string ColorSpace::toString() const\n> >   \treturn ss.str();\n> >   }\n> >   \n> > +/**\n> > + * \\brief Adjust the color space to match a pixel format\n> > + * \\param[in] format The pixel format\n> > + *\n> > + * Not all combinations of pixel formats and color spaces make sense. For\n> > + * instance, nobody uses a limited quantization range with raw Bayer formats,\n> > + * and the YcbcrEncoding::None encoding isn't valid for YUV formats. This\n> > + * function adjusts the ColorSpace to make it compatible with the given \\a\n> > + * format, by applying the following rules:\n> > + *\n> > + * - The color space for RAW formats must be Raw.\n> > + * - The Y'CbCr encoding and quantization range for RGB formats must be\n> > + *   YcbcrEncoding::None and Range::Full respectively.\n> > + * - The Y'CbCr encoding for YUV formats must not be YcbcrEncoding::None. The\n> > + *   best encoding is in that case guessed based on the primaries and transfer\n> > + *   function.\n> > + *\n> > + * \\return True if the color space has been adjusted, or false if it was\n> > + * already compatible with the format and hasn't been changed\n> > + */\n> > +bool ColorSpace::adjust(PixelFormat format)\n> \n> Ah, ,my version of Colorspace::adjust API was terrible :S\n\nNot at all. I've based v2.1 on your work, the good outcome results from\ncollective work :-)\n\n> v2.1 looks very good!\n> \n> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n> \n> > +{\n> > +\tconst PixelFormatInfo &info = PixelFormatInfo::info(format);\n> > +\tbool adjusted = false;\n> > +\n> > +\tswitch (info.colourEncoding) {\n> > +\tcase PixelFormatInfo::ColourEncodingRAW:\n> > +\t\t/* Raw formats must use the raw color space. */\n> > +\t\tif (*this != ColorSpace::Raw) {\n> > +\t\t\t*this = ColorSpace::Raw;\n> > +\t\t\tadjusted = true;\n> > +\t\t}\n> > +\t\tbreak;\n> > +\n> > +\tcase PixelFormatInfo::ColourEncodingRGB:\n> > +\t\t/*\n> > +\t\t * RGB formats can't have a Y'CbCr encoding, and must use full\n> > +\t\t * range quantization.\n> > +\t\t */\n> > +\t\tif (ycbcrEncoding != YcbcrEncoding::None) {\n> > +\t\t\tycbcrEncoding = YcbcrEncoding::None;\n> > +\t\t\tadjusted = true;\n> > +\t\t}\n> > +\n> > +\t\tif (range != Range::Full) {\n> > +\t\t\trange = Range::Full;\n> > +\t\t\tadjusted = true;\n> > +\t\t}\n> > +\t\tbreak;\n> > +\n> > +\tcase PixelFormatInfo::ColourEncodingYUV:\n> > +\t\tif (ycbcrEncoding != YcbcrEncoding::None)\n> > +\t\t\tbreak;\n> > +\n> > +\t\t/*\n> > +\t\t * YUV formats must have a Y'CbCr encoding. Infer the most\n> > +\t\t * probable option from the transfer function and primaries.\n> > +\t\t */\n> > +\t\tswitch (transferFunction) {\n> > +\t\tcase TransferFunction::Linear:\n> > +\t\t\t/*\n> > +\t\t\t * Linear YUV is not used in any standard color space,\n> > +\t\t\t * pick the widely supported and used Rec601 as default.\n> > +\t\t\t */\n> > +\t\t\tycbcrEncoding = YcbcrEncoding::Rec601;\n> > +\t\t\tbreak;\n> > +\n> > +\t\tcase TransferFunction::Rec709:\n> > +\t\t\tswitch (primaries) {\n> > +\t\t\t/* Raw should never happen. */\n> > +\t\t\tcase Primaries::Raw:\n> > +\t\t\tcase Primaries::Smpte170m:\n> > +\t\t\t\tycbcrEncoding = YcbcrEncoding::Rec601;\n> > +\t\t\t\tbreak;\n> > +\t\t\tcase Primaries::Rec709:\n> > +\t\t\t\tycbcrEncoding = YcbcrEncoding::Rec709;\n> > +\t\t\t\tbreak;\n> > +\t\t\tcase Primaries::Rec2020:\n> > +\t\t\t\tycbcrEncoding = YcbcrEncoding::Rec2020;\n> > +\t\t\t\tbreak;\n> > +\t\t\t}\n> > +\t\t\tbreak;\n> > +\n> > +\t\tcase TransferFunction::Srgb:\n> > +\t\t\t/*\n> > +\t\t\t * Only the sYCC color space uses the sRGB transfer\n> > +\t\t\t * function, the corresponding encoding is Rec601.\n> > +\t\t\t */\n> > +\t\t\tycbcrEncoding = YcbcrEncoding::Rec601;\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\n> > +\t\tadjusted = true;\n> > +\t\tbreak;\n> > +\t}\n> > +\n> > +\treturn adjusted;\n> > +}\n> > +\n> >   /**\n> >    * \\brief Assemble and return a readable string representation of an\n> >    * optional ColorSpace","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C825AC3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 26 Aug 2022 10:32:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2648561FC0;\n\tFri, 26 Aug 2022 12:32:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FC7861FA2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 26 Aug 2022 12:32:32 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id ACE562B3;\n\tFri, 26 Aug 2022 12:32:31 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661509953;\n\tbh=dsX0sC1XSiKPi0roH1XAK0hZ6Aj19o9xu5bfcIrlIUo=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Hctln8baVAotLBdBioccpl3/ZxcTki3iqdQmxeCITvnxjDqx/LeZ4c8qV+JyQ19i5\n\tlW3DxGmlw3oo/Gb+FAuF3CgDkS8+kyfsb7x0lYFBJgluQc4Jqb6P/CxPO3kQyZ3fip\n\tC5jC9O/36b3WO1GTV6Q4ZTP4lQ5Dfkm9qyNGxb+nt0clBHjlVemyhVNZSYfr1JcCVS\n\tcK8L8PGNQsnPk31sSxgearJXWe+TuLwBrOb5S7v1CRG5flXJXsnyz6BQw8hNfNaS8d\n\tAnV31uJecokMRaCBr48Zg2aMdyNq+i2H4eZ+mJs3DwPiy4GinfDoKt3Bc/G/fVqM5s\n\tq2OcRPOeofkwg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661509952;\n\tbh=dsX0sC1XSiKPi0roH1XAK0hZ6Aj19o9xu5bfcIrlIUo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BGwOWkRaCmqH5GUdjpz90OehwSeEsA+H9mUrvfoAQpNjkmWEh/P1r23j4HIsxGaOB\n\tS6DPL+qdI6qmfGuPS9yHBnRcvJSqmvWvIwBPwxB4FS/Xu2GntGB8iO5ONoZsZvrkwn\n\tY0P/V0hkam3SOOFYTHi+hU2j5qINA19RWYrbHSRY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"BGwOWkRa\"; dkim-atps=neutral","Date":"Fri, 26 Aug 2022 13:32:24 +0300","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<YwihONcDP0YAZl/h@pendragon.ideasonboard.com>","References":"<20220824162425.71087-5-umang.jain@ideasonboard.com>\n\t<20220825160647.10436-1-laurent.pinchart@ideasonboard.com>\n\t<b26d12f1-e5ed-873d-b379-0720ababb538@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<b26d12f1-e5ed-873d-b379-0720ababb538@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2.1 4/6] libcamera: color_space: Move\n\tcolor space adjustment to ColorSpace class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]