[{"id":24646,"web_url":"https://patchwork.libcamera.org/comment/24646/","msgid":"<8f9776bf-3c81-cc14-1048-10d877790c97@ideasonboard.com>","date":"2022-08-18T14:01:05","subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","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 the patch and detailed commit message.\n\nOn 8/18/22 02:48, Laurent Pinchart wrote:\n> The JPEG color space is badly name, as the JPEG specification (ITU-T\n> T.81) doesn't define any particular color space:\n>\n> The interchange format does not specify a complete coded image\n> representation. Application-dependent information, e.g. colour\n> space, is outside the scope of this Specification.\n>\n> The JFIF specification (ITU-T T.871) is clearer as it requires ITU-R\n> BT.601 YCbCr encoding and a full quantization range:\n>\n> The interpretations of Y, CB, and CR are derived from the E'Y, E'Cb,\n> and E'Cr signals defined in the 625-line specification of Rec. ITU-R\n> BT.601, but these signals are normalized so as to permit the usage of\n> the full range of 256 levels of the 8-bit binary encoding of the Y\n> component.\n>\n> It however doesn't specify color primaries or a transfer function\n> explicitly. It only mentions the latter when describing the conversion\n> from YCbCr to RGB:\n>\n> The inverse relationship for computing full scale 8-bit per colour\n> channel gamma pre-corrected RGB values (following Rec. ITU-R BT.601\n> gamma pre-correction and colour primary specifications) from YCbCr\n> colours (with 256 levels per component) can be computed as follows:\n> [...]\n>\n> Given that ITU-R BT.601-5 (1995) didn't specify color primaries or a\n> transfer function, and that the later ITU-R BT.601-7 (2011) version\n> specifies color primaries for the 625-line variant that do not match\n> sRGB, the JPEG color space in libcamera is badly named.\n>\n> Rename the color space to sYCC, as its definition matches the sYCC\n> standard, and indicate that it is typically used to encode JPEG images.\n>\n> Signed-off-by: Laurent Pinchart \n\n\nReviewed-by: Umang Jain \n\nCC'ing David from RPi especially (for heads-up) because this patch will \naffects libcamera-apps.\n\n> ---\n>\n> Hans, following our discussion on IRC, I double-check the JFIF standard,\n> and my findings are described in the commit message above. It seems that\n> naming the color space \"JPEG\" in V4L2 is actually incorrect, as JFIF\n> expects the 626-line ITU-R BT.601 chromaticities for the primary colors,\n> which are slightly different than sRGB (the 525-line chromaticities\n> match SMPTE 170M and SMPTE 240M, but not sRGB either). It seems that we\n> are missing a V4L2_COLORSPACE_* value to describe the 625-line ITU-R\n> BT.601 chromaticities, is that an oversight ?\n>\n> ---\n> include/libcamera/color_space.h | 2 +-\n> src/libcamera/color_space.cpp | 28 +++++++++----------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++----\n> src/libcamera/v4l2_device.cpp | 4 +--\n> src/py/libcamera/py_main.cpp | 2 +-\n> 5 files changed, 23 insertions(+), 23 deletions(-)\n>\n> diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h\n> index 0d39fbc02220..8030a264c66f 100644\n> --- a/include/libcamera/color_space.h\n> +++ b/include/libcamera/color_space.h\n> @@ -46,8 +46,8 @@ public:\n> \t}\n> \n> \tstatic const ColorSpace Raw;\n> -\tstatic const ColorSpace Jpeg;\n> \tstatic const ColorSpace Srgb;\n> +\tstatic const ColorSpace Sycc;\n> \tstatic const ColorSpace Smpte170m;\n> \tstatic const ColorSpace Rec709;\n> \tstatic const ColorSpace Rec2020;\n> diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp\n> index caf397607b10..6ace40dcb0fb 100644\n> --- a/src/libcamera/color_space.cpp\n> +++ b/src/libcamera/color_space.cpp\n> @@ -121,8 +121,8 @@ namespace libcamera {\n> * \\brief Assemble and return a readable string representation of the\n> * ColorSpace\n> *\n> - * If the color space matches a standard ColorSpace (such as ColorSpace::Jpeg)\n> - * then the short name of the color space (\"JPEG\") is returned. Otherwise\n> + * If the color space matches a standard ColorSpace (such as ColorSpace::Sycc)\n> + * then the short name of the color space (\"sYCC\") is returned. Otherwise\n> * the four constituent parts of the ColorSpace are assembled into a longer\n> * string.\n> *\n> @@ -134,8 +134,8 @@ std::string ColorSpace::toString() const\n> \n> \tstatic const std::array, 6> colorSpaceNames = { {\n> \t\t{ ColorSpace::Raw, \"RAW\" },\n> -\t\t{ ColorSpace::Jpeg, \"JPEG\" },\n> \t\t{ ColorSpace::Srgb, \"sRGB\" },\n> +\t\t{ ColorSpace::Sycc, \"sYCC\" },\n> \t\t{ ColorSpace::Smpte170m, \"SMPTE170M\" },\n> \t\t{ ColorSpace::Rec709, \"Rec709\" },\n> \t\t{ ColorSpace::Rec2020, \"Rec2020\" },\n> @@ -242,17 +242,6 @@ const ColorSpace ColorSpace::Raw = {\n> \tRange::Full\n> };\n> \n> -/**\n> - * \\brief A constant representing the JPEG color space used for\n> - * encoding JPEG images\n> - */\n> -const ColorSpace ColorSpace::Jpeg = {\n> -\tPrimaries::Rec709,\n> -\tTransferFunction::Srgb,\n> -\tYcbcrEncoding::Rec601,\n> -\tRange::Full\n> -};\n> -\n> /**\n> * \\brief A constant representing the sRGB color space\n> *\n> @@ -266,6 +255,17 @@ const ColorSpace ColorSpace::Srgb = {\n> \tRange::Limited\n> };\n> \n> +/**\n> + * \\brief A constant representing the sYCC color space, typically used for\n> + * encoding JPEG images\n> + */\n> +const ColorSpace ColorSpace::Sycc = {\n> +\tPrimaries::Rec709,\n> +\tTransferFunction::Srgb,\n> +\tYcbcrEncoding::Rec601,\n> +\tRange::Full\n> +};\n> +\n> /**\n> * \\brief A constant representing the SMPTE170M color space\n> */\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index e895584d4fbc..b4094898ca6c 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -593,11 +593,11 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> \t\t\tfmts = data->isp_[Isp::Output0].dev()->formats();\n> \t\t\tpixelFormat = formats::NV12;\n> \t\t\t/*\n> -\t\t\t * Still image codecs usually expect the JPEG color space.\n> +\t\t\t * Still image codecs usually expect the sYCC color space.\n> \t\t\t * Even RGB codecs will be fine as the RGB we get with the\n> -\t\t\t * JPEG color space is the same as sRGB.\n> +\t\t\t * sYCC color space is the same as sRGB.\n> \t\t\t */\n> -\t\t\tcolorSpace = ColorSpace::Jpeg;\n> +\t\t\tcolorSpace = ColorSpace::Sycc;\n> \t\t\t/* Return the largest sensor resolution. */\n> \t\t\tsize = sensorSize;\n> \t\t\tbufferCount = 1;\n> @@ -628,7 +628,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> \t\tcase StreamRole::Viewfinder:\n> \t\t\tfmts = data->isp_[Isp::Output0].dev()->formats();\n> \t\t\tpixelFormat = formats::ARGB8888;\n> -\t\t\tcolorSpace = ColorSpace::Jpeg;\n> +\t\t\tcolorSpace = ColorSpace::Sycc;\n> \t\t\tsize = { 800, 600 };\n> \t\t\tbufferCount = 4;\n> \t\t\toutCount++;\n> @@ -835,7 +835,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t\tformat.size = maxSize;\n> \t\tformat.fourcc = dev->toV4L2PixelFormat(formats::YUV420);\n> \t\t/* No one asked for output, so the color space doesn't matter. */\n> -\t\tformat.colorSpace = ColorSpace::Jpeg;\n> +\t\tformat.colorSpace = ColorSpace::Sycc;\n> \t\tret = dev->setFormat(&format);\n> \t\tif (ret) {\n> \t\t\tLOG(RPI, Error)\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 3fc8438f6579..b22a981ff1a6 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -745,8 +745,8 @@ void V4L2Device::eventAvailable()\n> \n> static const std::map v4l2ToColorSpace = {\n> \t{ V4L2_COLORSPACE_RAW, ColorSpace::Raw },\n> -\t{ V4L2_COLORSPACE_JPEG, ColorSpace::Jpeg },\n> \t{ V4L2_COLORSPACE_SRGB, ColorSpace::Srgb },\n> +\t{ V4L2_COLORSPACE_JPEG, ColorSpace::Sycc },\n> \t{ V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },\n> \t{ V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },\n> \t{ V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },\n> @@ -771,8 +771,8 @@ static const std::map v4l2ToRange = {\n> \n> static const std::vector> colorSpaceToV4l2 = {\n> \t{ ColorSpace::Raw, V4L2_COLORSPACE_RAW },\n> -\t{ ColorSpace::Jpeg, V4L2_COLORSPACE_JPEG },\n> \t{ ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },\n> +\t{ ColorSpace::Sycc, V4L2_COLORSPACE_JPEG },\n> \t{ ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },\n> \t{ ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },\n> \t{ ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },\n> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> index 505cc3dc1a0d..2a58e34a7cdb 100644\n> --- a/src/py/libcamera/py_main.cpp\n> +++ b/src/py/libcamera/py_main.cpp\n> @@ -602,8 +602,8 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t.def_readwrite(\"ycbcrEncoding\", &ColorSpace::ycbcrEncoding)\n> \t\t.def_readwrite(\"range\", &ColorSpace::range)\n> \t\t.def_static(\"Raw\", []() { return ColorSpace::Raw; })\n> -\t\t.def_static(\"Jpeg\", []() { return ColorSpace::Jpeg; })\n> \t\t.def_static(\"Srgb\", []() { return ColorSpace::Srgb; })\n> +\t\t.def_static(\"Sycc\", []() { return ColorSpace::Sycc; })\n> \t\t.def_static(\"Smpte170m\", []() { return ColorSpace::Smpte170m; })\n> \t\t.def_static(\"Rec709\", []() { return ColorSpace::Rec709; })\n> \t\t.def_static(\"Rec2020\", []() { return ColorSpace::Rec2020; });\n>","headers":{"Return-Path":"","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 8FDCBBE173\n\tfor ;\n\tThu, 18 Aug 2022 14:01:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3B5861FC0;\n\tThu, 18 Aug 2022 16:01:13 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9183061FA7\n\tfor ;\n\tThu, 18 Aug 2022 16:01:11 +0200 (CEST)","from [IPV6:2401:4900:1f3f:476e:b28b:2c3c:6f9c:d152] (unknown\n\t[IPv6:2401:4900:1f3f:476e:b28b:2c3c:6f9c:d152])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3536B4A8;\n\tThu, 18 Aug 2022 16:01:10 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660831273;\n\tbh=Dr/6ukX23x8efJtwU3ezb7pOYXGAva8w/jYBmw8Gbac=;\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=qflnH3H1ZS1AKNAg4o6VDwc3BHQbXSi7b33Y7ALnxhPq+aSOPY1k++IosyXjG52oi\n\t73kD8rXMQSdgWIOfXC6OYRGc6JOh1WWwrWkyfTVUsaM8ezpvjE6PgIddtWqsIlCAnL\n\tcV5nQiIkM6DoYGmoRcCiDU0CMsdsNzOv9b4zfdglgAa2FCjeW7Wx4Cnonq5cbp3AIq\n\tnV8NwyjSEAvNaIF/UXijaxahROBkTjKgMHo51gy1deh6LbnpCcMF7ikNx5Pj6bONTO\n\tenvw32JIFOtuTnTfCmsDujzQQsU3FjHCJfxcmsNklOMt0Kspt7mSG69kAfKav7Cdii\n\tDEB4yhtec8o1w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660831271;\n\tbh=Dr/6ukX23x8efJtwU3ezb7pOYXGAva8w/jYBmw8Gbac=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=c4jlMlhl1SRlhKDHxowR3TPAcpHJ0F+MkYjYpjRmZP6h0XO8ExCgrLpM26x+7AWJJ\n\t76Ft56KKMyyBkQy3pxFRSn2YAfq4moiia4Hf1SklfuO+0RKi0b4Ah0EwYjNg9ccLrf\n\tTv46XRBPrqEJ8zOORrKK7R0dRnMMnTWS7WTH+Lv8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"c4jlMlhl\"; dkim-atps=neutral","Message-ID":"<8f9776bf-3c81-cc14-1048-10d877790c97@ideasonboard.com>","Date":"Thu, 18 Aug 2022 19:31:05 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220817211824.20980-1-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220817211824.20980-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Cc":"Hans Verkuil ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24714,"web_url":"https://patchwork.libcamera.org/comment/24714/","msgid":"<8227eee1-074e-6c60-c24f-8d75ad5a3a74@xs4all.nl>","date":"2022-08-19T09:12:56","subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","submitter":{"id":43,"url":"https://patchwork.libcamera.org/api/people/43/","name":"Hans Verkuil","email":"hverkuil-cisco@xs4all.nl"},"content":"On 8/17/22 23:18, Laurent Pinchart wrote:\n> The JPEG color space is badly name, as the JPEG specification (ITU-T\n> T.81) doesn't define any particular color space:\n> \n> The interchange format does not specify a complete coded image\n> representation. Application-dependent information, e.g. colour\n> space, is outside the scope of this Specification.\n> \n> The JFIF specification (ITU-T T.871) is clearer as it requires ITU-R\n> BT.601 YCbCr encoding and a full quantization range:\n> \n> The interpretations of Y, CB, and CR are derived from the E'Y, E'Cb,\n> and E'Cr signals defined in the 625-line specification of Rec. ITU-R\n> BT.601, but these signals are normalized so as to permit the usage of\n> the full range of 256 levels of the 8-bit binary encoding of the Y\n> component.\n> \n> It however doesn't specify color primaries or a transfer function\n> explicitly. It only mentions the latter when describing the conversion\n> from YCbCr to RGB:\n> \n> The inverse relationship for computing full scale 8-bit per colour\n> channel gamma pre-corrected RGB values (following Rec. ITU-R BT.601\n> gamma pre-correction and colour primary specifications) from YCbCr\n> colours (with 256 levels per component) can be computed as follows:\n> [...]\n> \n> Given that ITU-R BT.601-5 (1995) didn't specify color primaries or a\n> transfer function, and that the later ITU-R BT.601-7 (2011) version\n> specifies color primaries for the 625-line variant that do not match\n> sRGB, the JPEG color space in libcamera is badly named.\n> \n> Rename the color space to sYCC, as its definition matches the sYCC\n> standard, and indicate that it is typically used to encode JPEG images.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> \n> Hans, following our discussion on IRC, I double-check the JFIF standard,\n> and my findings are described in the commit message above. It seems that\n> naming the color space \"JPEG\" in V4L2 is actually incorrect, as JFIF\n> expects the 626-line ITU-R BT.601 chromaticities for the primary colors,\n> which are slightly different than sRGB (the 525-line chromaticities\n> match SMPTE 170M and SMPTE 240M, but not sRGB either). It seems that we\n> are missing a V4L2_COLORSPACE_* value to describe the 625-line ITU-R\n> BT.601 chromaticities, is that an oversight ?\n\nWe have that colorspace: V4L2_COLORSPACE_470_SYSTEM_BG, however modern\nSDTV transmissions use V4L2_COLORSPACE_SMPTE170M these days.\n\nThe differences between the two are next to impossible to see.\n\nHowever, I think you misinterpreted the JFIF standard: it does not specify\nthat the BT 601 chromaticities/transfer function should be used, it only\nuses the YCbCr-RGB conversion that BT.601 describes.\n\nNote 3 in section 7 of JFIF is relevant here:\n\n\"NOTE 3 − As this Recommendation | International Standard is based on the prior informally-circulated JFIF version 1.02\nspecification that was produced in 1992, which referenced Rec. ITU-R BT.601 (formerly CCIR 601), it references that\nspecification for definition of the E' Y, E' , and E' signals that correspond to the YCBCR values specified herein. However,\nsince the development of the prior JFIF version 1.02 specification, additional industry specifications have been developed, Rec.\nITU-R BT.601 has been updated, and common industry practice has emerged which often follows the sYCC specification in IEC\n61966-2-1/Amd.1. The difference between the use of the colour interpretation specification in this Recommendation |\nInternational Standard and that of the sYCC specification may be considered negligible in practice. Moreover, as previously\nnoted, the colour space specification herein can provide only a basic level of colour fidelity. The use of supplemental metadata\nsuch as an ICC profile (e.g., as specified in ISO 15076-1) may be necessary to provide a more accurate colour characterization.\"\n\nIn other words, it recommends that sYCC is followed as the default.\n\nNote that I believe that the V4L2_COLORSPACE_JPEG define predates the sYCC standard,\nso there was a good reason why SYCC wasn't used. :-)\n\nI wonder if it would make sense to make a V4L2_COLORSPACE_SYCC as well. Probably\nmore effort than it is worth, though.\n\nRegards,\n\n\tHans\n\n> \n> ---\n> include/libcamera/color_space.h | 2 +-\n> src/libcamera/color_space.cpp | 28 +++++++++----------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++----\n> src/libcamera/v4l2_device.cpp | 4 +--\n> src/py/libcamera/py_main.cpp | 2 +-\n> 5 files changed, 23 insertions(+), 23 deletions(-)\n> \n> diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h\n> index 0d39fbc02220..8030a264c66f 100644\n> --- a/include/libcamera/color_space.h\n> +++ b/include/libcamera/color_space.h\n> @@ -46,8 +46,8 @@ public:\n> \t}\n> \n> \tstatic const ColorSpace Raw;\n> -\tstatic const ColorSpace Jpeg;\n> \tstatic const ColorSpace Srgb;\n> +\tstatic const ColorSpace Sycc;\n> \tstatic const ColorSpace Smpte170m;\n> \tstatic const ColorSpace Rec709;\n> \tstatic const ColorSpace Rec2020;\n> diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp\n> index caf397607b10..6ace40dcb0fb 100644\n> --- a/src/libcamera/color_space.cpp\n> +++ b/src/libcamera/color_space.cpp\n> @@ -121,8 +121,8 @@ namespace libcamera {\n> * \\brief Assemble and return a readable string representation of the\n> * ColorSpace\n> *\n> - * If the color space matches a standard ColorSpace (such as ColorSpace::Jpeg)\n> - * then the short name of the color space (\"JPEG\") is returned. Otherwise\n> + * If the color space matches a standard ColorSpace (such as ColorSpace::Sycc)\n> + * then the short name of the color space (\"sYCC\") is returned. Otherwise\n> * the four constituent parts of the ColorSpace are assembled into a longer\n> * string.\n> *\n> @@ -134,8 +134,8 @@ std::string ColorSpace::toString() const\n> \n> \tstatic const std::array, 6> colorSpaceNames = { {\n> \t\t{ ColorSpace::Raw, \"RAW\" },\n> -\t\t{ ColorSpace::Jpeg, \"JPEG\" },\n> \t\t{ ColorSpace::Srgb, \"sRGB\" },\n> +\t\t{ ColorSpace::Sycc, \"sYCC\" },\n> \t\t{ ColorSpace::Smpte170m, \"SMPTE170M\" },\n> \t\t{ ColorSpace::Rec709, \"Rec709\" },\n> \t\t{ ColorSpace::Rec2020, \"Rec2020\" },\n> @@ -242,17 +242,6 @@ const ColorSpace ColorSpace::Raw = {\n> \tRange::Full\n> };\n> \n> -/**\n> - * \\brief A constant representing the JPEG color space used for\n> - * encoding JPEG images\n> - */\n> -const ColorSpace ColorSpace::Jpeg = {\n> -\tPrimaries::Rec709,\n> -\tTransferFunction::Srgb,\n> -\tYcbcrEncoding::Rec601,\n> -\tRange::Full\n> -};\n> -\n> /**\n> * \\brief A constant representing the sRGB color space\n> *\n> @@ -266,6 +255,17 @@ const ColorSpace ColorSpace::Srgb = {\n> \tRange::Limited\n> };\n> \n> +/**\n> + * \\brief A constant representing the sYCC color space, typically used for\n> + * encoding JPEG images\n> + */\n> +const ColorSpace ColorSpace::Sycc = {\n> +\tPrimaries::Rec709,\n> +\tTransferFunction::Srgb,\n> +\tYcbcrEncoding::Rec601,\n> +\tRange::Full\n> +};\n> +\n> /**\n> * \\brief A constant representing the SMPTE170M color space\n> */\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index e895584d4fbc..b4094898ca6c 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -593,11 +593,11 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> \t\t\tfmts = data->isp_[Isp::Output0].dev()->formats();\n> \t\t\tpixelFormat = formats::NV12;\n> \t\t\t/*\n> -\t\t\t * Still image codecs usually expect the JPEG color space.\n> +\t\t\t * Still image codecs usually expect the sYCC color space.\n> \t\t\t * Even RGB codecs will be fine as the RGB we get with the\n> -\t\t\t * JPEG color space is the same as sRGB.\n> +\t\t\t * sYCC color space is the same as sRGB.\n> \t\t\t */\n> -\t\t\tcolorSpace = ColorSpace::Jpeg;\n> +\t\t\tcolorSpace = ColorSpace::Sycc;\n> \t\t\t/* Return the largest sensor resolution. */\n> \t\t\tsize = sensorSize;\n> \t\t\tbufferCount = 1;\n> @@ -628,7 +628,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> \t\tcase StreamRole::Viewfinder:\n> \t\t\tfmts = data->isp_[Isp::Output0].dev()->formats();\n> \t\t\tpixelFormat = formats::ARGB8888;\n> -\t\t\tcolorSpace = ColorSpace::Jpeg;\n> +\t\t\tcolorSpace = ColorSpace::Sycc;\n> \t\t\tsize = { 800, 600 };\n> \t\t\tbufferCount = 4;\n> \t\t\toutCount++;\n> @@ -835,7 +835,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t\tformat.size = maxSize;\n> \t\tformat.fourcc = dev->toV4L2PixelFormat(formats::YUV420);\n> \t\t/* No one asked for output, so the color space doesn't matter. */\n> -\t\tformat.colorSpace = ColorSpace::Jpeg;\n> +\t\tformat.colorSpace = ColorSpace::Sycc;\n> \t\tret = dev->setFormat(&format);\n> \t\tif (ret) {\n> \t\t\tLOG(RPI, Error)\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 3fc8438f6579..b22a981ff1a6 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -745,8 +745,8 @@ void V4L2Device::eventAvailable()\n> \n> static const std::map v4l2ToColorSpace = {\n> \t{ V4L2_COLORSPACE_RAW, ColorSpace::Raw },\n> -\t{ V4L2_COLORSPACE_JPEG, ColorSpace::Jpeg },\n> \t{ V4L2_COLORSPACE_SRGB, ColorSpace::Srgb },\n> +\t{ V4L2_COLORSPACE_JPEG, ColorSpace::Sycc },\n> \t{ V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },\n> \t{ V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },\n> \t{ V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },\n> @@ -771,8 +771,8 @@ static const std::map v4l2ToRange = {\n> \n> static const std::vector> colorSpaceToV4l2 = {\n> \t{ ColorSpace::Raw, V4L2_COLORSPACE_RAW },\n> -\t{ ColorSpace::Jpeg, V4L2_COLORSPACE_JPEG },\n> \t{ ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },\n> +\t{ ColorSpace::Sycc, V4L2_COLORSPACE_JPEG },\n> \t{ ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },\n> \t{ ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },\n> \t{ ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },\n> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> index 505cc3dc1a0d..2a58e34a7cdb 100644\n> --- a/src/py/libcamera/py_main.cpp\n> +++ b/src/py/libcamera/py_main.cpp\n> @@ -602,8 +602,8 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t.def_readwrite(\"ycbcrEncoding\", &ColorSpace::ycbcrEncoding)\n> \t\t.def_readwrite(\"range\", &ColorSpace::range)\n> \t\t.def_static(\"Raw\", []() { return ColorSpace::Raw; })\n> -\t\t.def_static(\"Jpeg\", []() { return ColorSpace::Jpeg; })\n> \t\t.def_static(\"Srgb\", []() { return ColorSpace::Srgb; })\n> +\t\t.def_static(\"Sycc\", []() { return ColorSpace::Sycc; })\n> \t\t.def_static(\"Smpte170m\", []() { return ColorSpace::Smpte170m; })\n> \t\t.def_static(\"Rec709\", []() { return ColorSpace::Rec709; })\n> \t\t.def_static(\"Rec2020\", []() { return ColorSpace::Rec2020; });\n>","headers":{"Return-Path":"","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 04873C3272\n\tfor ;\n\tFri, 19 Aug 2022 09:12:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B87361FC0;\n\tFri, 19 Aug 2022 11:12:59 +0200 (CEST)","from ewsoutbound.kpnmail.nl (ewsoutbound.kpnmail.nl\n\t[195.121.94.185])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EEE2061FA3\n\tfor ;\n\tFri, 19 Aug 2022 11:12:57 +0200 (CEST)","from smtp.kpnmail.nl (unknown [10.31.155.6])\n\tby ewsoutbound.so.kpn.org (Halon) with ESMTPS\n\tid ef1e4f88-1f9e-11ed-bfe8-005056999439;\n\tFri, 19 Aug 2022 11:11:39 +0200 (CEST)","from [10.47.77.219] (unknown [173.38.220.52])\n\tby smtp.xs4all.nl (Halon) with ESMTPSA\n\tid 1cc9755d-1f9f-11ed-ad28-00505699772e;\n\tFri, 19 Aug 2022 11:12:57 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660900379;\n\tbh=aOqB8CRmlGd8ohl1wzG1TJkJzhU4Wx/4Qcas0ieyRsk=;\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=f02JfZfEkQkuFDjZdFBbB8v/JLhCSUyzGh64CceYifuHGqm+CavluT3DIbqSJndbD\n\tJ80Drq+/nj8FuX9oxj6Fd7/1B2BGkErQsn3C7ZdlYBAVUkQKGjEVAg/gQUTpL5BhNi\n\t91HTFVctnYNDUhM8AidIGYXqeOm368GGAaKLU4v1nBt6kyhseC4Dlud8pGjL4YXGEX\n\tEPF6jRRwGHKpFQbRyCQHpNwiQcNET/wBTB/RsCH4eLYBbwZ6UuicY+RPaKGqU8noUB\n\tu4fQM786/kFSdeHtrgXCnB39e96j9XhEWwmKb1kCDu1Scf0EbwWExAXMBY3qTTn1GZ\n\tCAGSMX47mLkKg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=xs4all.nl; s=xs4all01;\n\th=content-type:from:to:subject:mime-version:date:message-id;\n\tbh=a9lTQx4/w0JI/XMc/YqUWHwK+3knXUnR+HNiIa4nHW4=;\n\tb=UcONq2NRn84dAT0f8HaRfuRqTXwuoBKLJTijBc/a2we5T1dtqrEiS/LD1xGc2BiMIUbn99KkZuyUc\n\tCDKyySmeNJL/gEVTUtxM+Mx7mNgSXw8SvEwCH3nh7ATIp6J7VR0kmPNN6kKD+Jkr5h51hZb2lDFe8L\n\tQpX8lvlh0Q6sa9IBsnV8p2ZB4rnO7C+4eRPrYWUptcleJ80hl6BDAOP5u5ncXGy/jIrzdm5X1IBaA8\n\tixPpVgeOXb4/E9nKywGk5SsiXS1mtKGKQ9WV087sTCHjLNzVbsXVY4Fjyg+8EtS5seRKgZaXxWQT62\n\taglrEwDqoYyZvRmV5MbuX4GMMKQHR9w=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=xs4all.nl header.i=@xs4all.nl\n\theader.b=\"UcONq2NR\"; dkim-atps=neutral","X-KPN-MessageId":"ef1e4f88-1f9e-11ed-bfe8-005056999439","X-KPN-MID":"33|2aL/52DCVmM3ZSQDE09u+mEnVwqUsrBpDm+5j6RDKOF5SW9LMHkTUWsNv8wQ4Hy\n\twqR2Y77hVG+vv+OeUMfub3NpoMEqKKwNOmPX71T56RBQ=","X-KPN-VerifiedSender":"Yes","X-CMASSUN":"33|+TlcE/FwZZqi3PmTWsrPOyeoO0c/3D6F+AttO3nJnXhW0s0vhpatD7qGzD6PSEy\n\t4VJdjgoTsTG1OAgvpcIj7SQ==","X-Originating-IP":"173.38.220.52","Message-ID":"<8227eee1-074e-6c60-c24f-8d75ad5a3a74@xs4all.nl>","Date":"Fri, 19 Aug 2022 11:12:56 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.10.0","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220817211824.20980-1-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220817211824.20980-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Hans Verkuil via libcamera-devel ","Reply-To":"Hans Verkuil ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24722,"web_url":"https://patchwork.libcamera.org/comment/24722/","msgid":"","date":"2022-08-21T20:28:00","subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hans,\n\nOn Fri, Aug 19, 2022 at 11:12:56AM +0200, Hans Verkuil wrote:\n> On 8/17/22 23:18, Laurent Pinchart wrote:\n> > The JPEG color space is badly name, as the JPEG specification (ITU-T\n> > T.81) doesn't define any particular color space:\n> > \n> > The interchange format does not specify a complete coded image\n> > representation. Application-dependent information, e.g. colour\n> > space, is outside the scope of this Specification.\n> > \n> > The JFIF specification (ITU-T T.871) is clearer as it requires ITU-R\n> > BT.601 YCbCr encoding and a full quantization range:\n> > \n> > The interpretations of Y, CB, and CR are derived from the E'Y, E'Cb,\n> > and E'Cr signals defined in the 625-line specification of Rec. ITU-R\n> > BT.601, but these signals are normalized so as to permit the usage of\n> > the full range of 256 levels of the 8-bit binary encoding of the Y\n> > component.\n> > \n> > It however doesn't specify color primaries or a transfer function\n> > explicitly. It only mentions the latter when describing the conversion\n> > from YCbCr to RGB:\n> > \n> > The inverse relationship for computing full scale 8-bit per colour\n> > channel gamma pre-corrected RGB values (following Rec. ITU-R BT.601\n> > gamma pre-correction and colour primary specifications) from YCbCr\n> > colours (with 256 levels per component) can be computed as follows:\n> > [...]\n> > \n> > Given that ITU-R BT.601-5 (1995) didn't specify color primaries or a\n> > transfer function, and that the later ITU-R BT.601-7 (2011) version\n> > specifies color primaries for the 625-line variant that do not match\n> > sRGB, the JPEG color space in libcamera is badly named.\n> > \n> > Rename the color space to sYCC, as its definition matches the sYCC\n> > standard, and indicate that it is typically used to encode JPEG images.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > \n> > Hans, following our discussion on IRC, I double-check the JFIF standard,\n> > and my findings are described in the commit message above. It seems that\n> > naming the color space \"JPEG\" in V4L2 is actually incorrect, as JFIF\n> > expects the 626-line ITU-R BT.601 chromaticities for the primary colors,\n> > which are slightly different than sRGB (the 525-line chromaticities\n> > match SMPTE 170M and SMPTE 240M, but not sRGB either). It seems that we\n> > are missing a V4L2_COLORSPACE_* value to describe the 625-line ITU-R\n> > BT.601 chromaticities, is that an oversight ?\n> \n> We have that colorspace: V4L2_COLORSPACE_470_SYSTEM_BG, however modern\n\nIndeed, my bad.\n\n> SDTV transmissions use V4L2_COLORSPACE_SMPTE170M these days.\n> \n> The differences between the two are next to impossible to see.\n> \n> However, I think you misinterpreted the JFIF standard: it does not specify\n> that the BT 601 chromaticities/transfer function should be used, it only\n> uses the YCbCr-RGB conversion that BT.601 describes.\n> \n> Note 3 in section 7 of JFIF is relevant here:\n> \n> \"NOTE 3 − As this Recommendation | International Standard is based on the prior informally-circulated JFIF version 1.02\n> specification that was produced in 1992, which referenced Rec. ITU-R BT.601 (formerly CCIR 601), it references that\n> specification for definition of the E' Y, E' , and E' signals that correspond to the YCBCR values specified herein. However,\n> since the development of the prior JFIF version 1.02 specification, additional industry specifications have been developed, Rec.\n> ITU-R BT.601 has been updated, and common industry practice has emerged which often follows the sYCC specification in IEC\n> 61966-2-1/Amd.1. The difference between the use of the colour interpretation specification in this Recommendation |\n> International Standard and that of the sYCC specification may be considered negligible in practice. Moreover, as previously\n> noted, the colour space specification herein can provide only a basic level of colour fidelity. The use of supplemental metadata\n> such as an ICC profile (e.g., as specified in ISO 15076-1) may be necessary to provide a more accurate colour characterization.\"\n> \n> In other words, it recommends that sYCC is followed as the default.\n> \n> Note that I believe that the V4L2_COLORSPACE_JPEG define predates the sYCC standard,\n> so there was a good reason why SYCC wasn't used. :-)\n\nThat's possible, I'll stop short of researching this :-)\n\n> I wonder if it would make sense to make a V4L2_COLORSPACE_SYCC as well. Probably\n> more effort than it is worth, though.\n\nFor V4L2 I think it's likely more effort than it's worth, yes. For\nlibcamera we'll use sYCC.\n\n> > ---\n> > include/libcamera/color_space.h | 2 +-\n> > src/libcamera/color_space.cpp | 28 +++++++++----------\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++----\n> > src/libcamera/v4l2_device.cpp | 4 +--\n> > src/py/libcamera/py_main.cpp | 2 +-\n> > 5 files changed, 23 insertions(+), 23 deletions(-)\n> > \n> > diff --git a/include/libcamera/color_space.h b/include/libcamera/color_space.h\n> > index 0d39fbc02220..8030a264c66f 100644\n> > --- a/include/libcamera/color_space.h\n> > +++ b/include/libcamera/color_space.h\n> > @@ -46,8 +46,8 @@ public:\n> > \t}\n> > \n> > \tstatic const ColorSpace Raw;\n> > -\tstatic const ColorSpace Jpeg;\n> > \tstatic const ColorSpace Srgb;\n> > +\tstatic const ColorSpace Sycc;\n> > \tstatic const ColorSpace Smpte170m;\n> > \tstatic const ColorSpace Rec709;\n> > \tstatic const ColorSpace Rec2020;\n> > diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp\n> > index caf397607b10..6ace40dcb0fb 100644\n> > --- a/src/libcamera/color_space.cpp\n> > +++ b/src/libcamera/color_space.cpp\n> > @@ -121,8 +121,8 @@ namespace libcamera {\n> > * \\brief Assemble and return a readable string representation of the\n> > * ColorSpace\n> > *\n> > - * If the color space matches a standard ColorSpace (such as ColorSpace::Jpeg)\n> > - * then the short name of the color space (\"JPEG\") is returned. Otherwise\n> > + * If the color space matches a standard ColorSpace (such as ColorSpace::Sycc)\n> > + * then the short name of the color space (\"sYCC\") is returned. Otherwise\n> > * the four constituent parts of the ColorSpace are assembled into a longer\n> > * string.\n> > *\n> > @@ -134,8 +134,8 @@ std::string ColorSpace::toString() const\n> > \n> > \tstatic const std::array, 6> colorSpaceNames = { {\n> > \t\t{ ColorSpace::Raw, \"RAW\" },\n> > -\t\t{ ColorSpace::Jpeg, \"JPEG\" },\n> > \t\t{ ColorSpace::Srgb, \"sRGB\" },\n> > +\t\t{ ColorSpace::Sycc, \"sYCC\" },\n> > \t\t{ ColorSpace::Smpte170m, \"SMPTE170M\" },\n> > \t\t{ ColorSpace::Rec709, \"Rec709\" },\n> > \t\t{ ColorSpace::Rec2020, \"Rec2020\" },\n> > @@ -242,17 +242,6 @@ const ColorSpace ColorSpace::Raw = {\n> > \tRange::Full\n> > };\n> > \n> > -/**\n> > - * \\brief A constant representing the JPEG color space used for\n> > - * encoding JPEG images\n> > - */\n> > -const ColorSpace ColorSpace::Jpeg = {\n> > -\tPrimaries::Rec709,\n> > -\tTransferFunction::Srgb,\n> > -\tYcbcrEncoding::Rec601,\n> > -\tRange::Full\n> > -};\n> > -\n> > /**\n> > * \\brief A constant representing the sRGB color space\n> > *\n> > @@ -266,6 +255,17 @@ const ColorSpace ColorSpace::Srgb = {\n> > \tRange::Limited\n> > };\n> > \n> > +/**\n> > + * \\brief A constant representing the sYCC color space, typically used for\n> > + * encoding JPEG images\n> > + */\n> > +const ColorSpace ColorSpace::Sycc = {\n> > +\tPrimaries::Rec709,\n> > +\tTransferFunction::Srgb,\n> > +\tYcbcrEncoding::Rec601,\n> > +\tRange::Full\n> > +};\n> > +\n> > /**\n> > * \\brief A constant representing the SMPTE170M color space\n> > */\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index e895584d4fbc..b4094898ca6c 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -593,11 +593,11 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > \t\t\tfmts = data->isp_[Isp::Output0].dev()->formats();\n> > \t\t\tpixelFormat = formats::NV12;\n> > \t\t\t/*\n> > -\t\t\t * Still image codecs usually expect the JPEG color space.\n> > +\t\t\t * Still image codecs usually expect the sYCC color space.\n> > \t\t\t * Even RGB codecs will be fine as the RGB we get with the\n> > -\t\t\t * JPEG color space is the same as sRGB.\n> > +\t\t\t * sYCC color space is the same as sRGB.\n> > \t\t\t */\n> > -\t\t\tcolorSpace = ColorSpace::Jpeg;\n> > +\t\t\tcolorSpace = ColorSpace::Sycc;\n> > \t\t\t/* Return the largest sensor resolution. */\n> > \t\t\tsize = sensorSize;\n> > \t\t\tbufferCount = 1;\n> > @@ -628,7 +628,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > \t\tcase StreamRole::Viewfinder:\n> > \t\t\tfmts = data->isp_[Isp::Output0].dev()->formats();\n> > \t\t\tpixelFormat = formats::ARGB8888;\n> > -\t\t\tcolorSpace = ColorSpace::Jpeg;\n> > +\t\t\tcolorSpace = ColorSpace::Sycc;\n> > \t\t\tsize = { 800, 600 };\n> > \t\t\tbufferCount = 4;\n> > \t\t\toutCount++;\n> > @@ -835,7 +835,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > \t\tformat.size = maxSize;\n> > \t\tformat.fourcc = dev->toV4L2PixelFormat(formats::YUV420);\n> > \t\t/* No one asked for output, so the color space doesn't matter. */\n> > -\t\tformat.colorSpace = ColorSpace::Jpeg;\n> > +\t\tformat.colorSpace = ColorSpace::Sycc;\n> > \t\tret = dev->setFormat(&format);\n> > \t\tif (ret) {\n> > \t\t\tLOG(RPI, Error)\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 3fc8438f6579..b22a981ff1a6 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -745,8 +745,8 @@ void V4L2Device::eventAvailable()\n> > \n> > static const std::map v4l2ToColorSpace = {\n> > \t{ V4L2_COLORSPACE_RAW, ColorSpace::Raw },\n> > -\t{ V4L2_COLORSPACE_JPEG, ColorSpace::Jpeg },\n> > \t{ V4L2_COLORSPACE_SRGB, ColorSpace::Srgb },\n> > +\t{ V4L2_COLORSPACE_JPEG, ColorSpace::Sycc },\n> > \t{ V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },\n> > \t{ V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },\n> > \t{ V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },\n> > @@ -771,8 +771,8 @@ static const std::map v4l2ToRange = {\n> > \n> > static const std::vector> colorSpaceToV4l2 = {\n> > \t{ ColorSpace::Raw, V4L2_COLORSPACE_RAW },\n> > -\t{ ColorSpace::Jpeg, V4L2_COLORSPACE_JPEG },\n> > \t{ ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },\n> > +\t{ ColorSpace::Sycc, V4L2_COLORSPACE_JPEG },\n> > \t{ ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },\n> > \t{ ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },\n> > \t{ ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },\n> > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> > index 505cc3dc1a0d..2a58e34a7cdb 100644\n> > --- a/src/py/libcamera/py_main.cpp\n> > +++ b/src/py/libcamera/py_main.cpp\n> > @@ -602,8 +602,8 @@ PYBIND11_MODULE(_libcamera, m)\n> > \t\t.def_readwrite(\"ycbcrEncoding\", &ColorSpace::ycbcrEncoding)\n> > \t\t.def_readwrite(\"range\", &ColorSpace::range)\n> > \t\t.def_static(\"Raw\", []() { return ColorSpace::Raw; })\n> > -\t\t.def_static(\"Jpeg\", []() { return ColorSpace::Jpeg; })\n> > \t\t.def_static(\"Srgb\", []() { return ColorSpace::Srgb; })\n> > +\t\t.def_static(\"Sycc\", []() { return ColorSpace::Sycc; })\n> > \t\t.def_static(\"Smpte170m\", []() { return ColorSpace::Smpte170m; })\n> > \t\t.def_static(\"Rec709\", []() { return ColorSpace::Rec709; })\n> > \t\t.def_static(\"Rec2020\", []() { return ColorSpace::Rec2020; });","headers":{"Return-Path":"","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 8F3C5BE173\n\tfor ;\n\tSun, 21 Aug 2022 20:28:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C9B9F61FC0;\n\tSun, 21 Aug 2022 22:28:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C90061FA3\n\tfor ;\n\tSun, 21 Aug 2022 22:28:04 +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 B1ED13F1;\n\tSun, 21 Aug 2022 22:28:03 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661113685;\n\tbh=UUScWwzz9Lj9x0/EXYd0X70yNp1zDFhvMjyO8iaoWI0=;\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=aQfC1vyInImNirM8R4lkBhgck0eV0lHWnkFszqYB/FJ76EmlBYkHIiKd4K+G0IXnO\n\to8hjzvVm3x2q4H+RuILe5bYkY0m1nmkKSopuUgv5HIObllUZg6XhAZtj9gpdfWR86Y\n\tREnO5kC+zuF5rp1G9P6b6nfsug9CP0Xmg2NvsuPE0mB2VUtCdyRBdahMegsgAIq5ch\n\tOC0VZ7EDZjp7VG1/7AZFGQHLKqNChZNXyODUQPoAfMoOGPFYfdh24STB1jkSNhLphW\n\tzNbCJ52FmHjpASj8Xc5FxE1d45UTYrW+FRcfPfAcBoeJy5QcvBvvvwf9liSh2WzyTh\n\trrN+rgU42ZLNg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661113683;\n\tbh=UUScWwzz9Lj9x0/EXYd0X70yNp1zDFhvMjyO8iaoWI0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IFg2ErN+R/4z49+6+ScxmbZl75+RXxrmcM8QjEUbIJZwswxZyN6gKTNfBZrAF1UfS\n\tWKg4I9g7J7BDjJPC/pCHXuOSYAAvaiKkojDZJSu7j+OjOvVGC1tVlIG1kjaYkbE+xZ\n\tgpj/pctlD43RmWJFEyh0RnPWTZdzCS8tuWhwYH7s="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"IFg2ErN+\"; dkim-atps=neutral","Date":"Sun, 21 Aug 2022 23:28:00 +0300","To":"Hans Verkuil ","Message-ID":"","References":"<20220817211824.20980-1-laurent.pinchart@ideasonboard.com>\n\t<8227eee1-074e-6c60-c24f-8d75ad5a3a74@xs4all.nl>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<8227eee1-074e-6c60-c24f-8d75ad5a3a74@xs4all.nl>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24725,"web_url":"https://patchwork.libcamera.org/comment/24725/","msgid":"<56677d58-4bf3-2cc4-a885-5dd86ba99350@ideasonboard.com>","date":"2022-08-22T15:22:42","subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOne more comment\n\nOn 8/18/22 7:31 PM, Umang Jain via libcamera-devel wrote:\n> Hi Laurent\n>\n> Thank you for the patch and detailed commit message.\n>\n> On 8/18/22 02:48, Laurent Pinchart wrote:\n>> The JPEG color space is badly name, as the JPEG specification (ITU-T\n>> T.81) doesn't define any particular color space:\n>>\n>>      The interchange format does not specify a complete coded image\n>>      representation. Application-dependent information, e.g. colour\n>>      space, is outside the scope of this Specification.\n>>\n>> The JFIF specification (ITU-T T.871) is clearer as it requires ITU-R\n>> BT.601 YCbCr encoding and a full quantization range:\n>>\n>>    The interpretations of Y, CB, and CR are derived from the E'Y, E'Cb,\n>>    and E'Cr signals defined in the 625-line specification of Rec. ITU-R\n>>    BT.601, but these signals are normalized so as to permit the usage of\n>>    the full range of 256 levels of the 8-bit binary encoding of the Y\n>>    component.\n>>\n>> It however doesn't specify color primaries or a transfer function\n>> explicitly. It only mentions the latter when describing the conversion\n>> from YCbCr to RGB:\n>>\n>>    The inverse relationship for computing full scale 8-bit per colour\n>>    channel gamma pre-corrected RGB values (following Rec. ITU-R BT.601\n>>    gamma pre-correction and colour primary specifications) from YCbCr\n>>    colours (with 256 levels per component) can be computed as follows:\n>>    [...]\n>>\n>> Given that ITU-R BT.601-5 (1995) didn't specify color primaries or a\n>> transfer function, and that the later ITU-R BT.601-7 (2011) version\n>> specifies color primaries for the 625-line variant that do not match\n>> sRGB, the JPEG color space in libcamera is badly named.\n>>\n>> Rename the color space to sYCC, as its definition matches the sYCC\n>> standard, and indicate that it is typically used to encode JPEG images.\n>>\n>> Signed-off-by: Laurent Pinchart \n>\n>\n> Reviewed-by: Umang Jain \n>\n> CC'ing David from RPi especially (for heads-up) because this patch \n> will affects libcamera-apps.\n>\n>> ---\n>>\n>> Hans, following our discussion on IRC, I double-check the JFIF standard,\n>> and my findings are described in the commit message above. It seems that\n>> naming the color space \"JPEG\" in V4L2 is actually incorrect, as JFIF\n>> expects the 626-line ITU-R BT.601 chromaticities for the primary colors,\n>> which are slightly different than sRGB (the 525-line chromaticities\n>> match SMPTE 170M and SMPTE 240M, but not sRGB either). It seems that we\n>> are missing a V4L2_COLORSPACE_* value to describe the 625-line ITU-R\n>> BT.601 chromaticities, is that an oversight ?\n>>\n>> ---\n>>   include/libcamera/color_space.h               |  2 +-\n>>   src/libcamera/color_space.cpp                 | 28 +++++++++----------\n>>   .../pipeline/raspberrypi/raspberrypi.cpp      | 10 +++----\n>>   src/libcamera/v4l2_device.cpp                 |  4 +--\n>>   src/py/libcamera/py_main.cpp                  |  2 +-\n>>   5 files changed, 23 insertions(+), 23 deletions(-)\n>>\n>> diff --git a/include/libcamera/color_space.h \n>> b/include/libcamera/color_space.h\n>> index 0d39fbc02220..8030a264c66f 100644\n>> --- a/include/libcamera/color_space.h\n>> +++ b/include/libcamera/color_space.h\n>> @@ -46,8 +46,8 @@ public:\n>>       }\n>>         static const ColorSpace Raw;\n>> -    static const ColorSpace Jpeg;\n>>       static const ColorSpace Srgb;\n>> +    static const ColorSpace Sycc;\n>>       static const ColorSpace Smpte170m;\n>>       static const ColorSpace Rec709;\n>>       static const ColorSpace Rec2020;\n>> diff --git a/src/libcamera/color_space.cpp \n>> b/src/libcamera/color_space.cpp\n>> index caf397607b10..6ace40dcb0fb 100644\n>> --- a/src/libcamera/color_space.cpp\n>> +++ b/src/libcamera/color_space.cpp\n>> @@ -121,8 +121,8 @@ namespace libcamera {\n>>    * \\brief Assemble and return a readable string representation of the\n>>    * ColorSpace\n>>    *\n>> - * If the color space matches a standard ColorSpace (such as \n>> ColorSpace::Jpeg)\n>> - * then the short name of the color space (\"JPEG\") is returned. \n>> Otherwise\n>> + * If the color space matches a standard ColorSpace (such as \n>> ColorSpace::Sycc)\n>> + * then the short name of the color space (\"sYCC\") is returned. \n>> Otherwise\n>>    * the four constituent parts of the ColorSpace are assembled into \n>> a longer\n>>    * string.\n\nBelow this paragraph, we have\n  * - JPEG\nas well.\n\nWe will probably in tricky situation here. Do we want to drop the JPEG \nlink entirely, or change to\n\n  * - sYCC\n\nCan you add your thoughts here and update the patch (no need to re-send \nI think, but if you decide to push it...)\n\n(It will be more tricky for sRGB too, as we will not be going with \nkernel's definition)\n\n>>    *\n>> @@ -134,8 +134,8 @@ std::string ColorSpace::toString() const\n>>         static const std::array, \n>> 6> colorSpaceNames = { {\n>>           { ColorSpace::Raw, \"RAW\" },\n>> -        { ColorSpace::Jpeg, \"JPEG\" },\n>>           { ColorSpace::Srgb, \"sRGB\" },\n>> +        { ColorSpace::Sycc, \"sYCC\" },\n>>           { ColorSpace::Smpte170m, \"SMPTE170M\" },\n>>           { ColorSpace::Rec709, \"Rec709\" },\n>>           { ColorSpace::Rec2020, \"Rec2020\" },\n>> @@ -242,17 +242,6 @@ const ColorSpace ColorSpace::Raw = {\n>>       Range::Full\n>>   };\n>>   -/**\n>> - * \\brief A constant representing the JPEG color space used for\n>> - * encoding JPEG images\n>> - */\n>> -const ColorSpace ColorSpace::Jpeg = {\n>> -    Primaries::Rec709,\n>> -    TransferFunction::Srgb,\n>> -    YcbcrEncoding::Rec601,\n>> -    Range::Full\n>> -};\n>> -\n>>   /**\n>>    * \\brief A constant representing the sRGB color space\n>>    *\n>> @@ -266,6 +255,17 @@ const ColorSpace ColorSpace::Srgb = {\n>>       Range::Limited\n>>   };\n>>   +/**\n>> + * \\brief A constant representing the sYCC color space, typically \n>> used for\n>> + * encoding JPEG images\n>> + */\n>> +const ColorSpace ColorSpace::Sycc = {\n>> +    Primaries::Rec709,\n>> +    TransferFunction::Srgb,\n>> +    YcbcrEncoding::Rec601,\n>> +    Range::Full\n>> +};\n>> +\n>>   /**\n>>    * \\brief A constant representing the SMPTE170M color space\n>>    */\n>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp \n>> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> index e895584d4fbc..b4094898ca6c 100644\n>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> @@ -593,11 +593,11 @@ CameraConfiguration \n>> *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n>>               fmts = data->isp_[Isp::Output0].dev()->formats();\n>>               pixelFormat = formats::NV12;\n>>               /*\n>> -             * Still image codecs usually expect the JPEG color space.\n>> +             * Still image codecs usually expect the sYCC color space.\n>>                * Even RGB codecs will be fine as the RGB we get with the\n>> -             * JPEG color space is the same as sRGB.\n>> +             * sYCC color space is the same as sRGB.\n>>                */\n>> -            colorSpace = ColorSpace::Jpeg;\n>> +            colorSpace = ColorSpace::Sycc;\n>>               /* Return the largest sensor resolution. */\n>>               size = sensorSize;\n>>               bufferCount = 1;\n>> @@ -628,7 +628,7 @@ CameraConfiguration \n>> *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n>>           case StreamRole::Viewfinder:\n>>               fmts = data->isp_[Isp::Output0].dev()->formats();\n>>               pixelFormat = formats::ARGB8888;\n>> -            colorSpace = ColorSpace::Jpeg;\n>> +            colorSpace = ColorSpace::Sycc;\n>>               size = { 800, 600 };\n>>               bufferCount = 4;\n>>               outCount++;\n>> @@ -835,7 +835,7 @@ int PipelineHandlerRPi::configure(Camera *camera, \n>> CameraConfiguration *config)\n>>           format.size = maxSize;\n>>           format.fourcc = dev->toV4L2PixelFormat(formats::YUV420);\n>>           /* No one asked for output, so the color space doesn't \n>> matter. */\n>> -        format.colorSpace = ColorSpace::Jpeg;\n>> +        format.colorSpace = ColorSpace::Sycc;\n>>           ret = dev->setFormat(&format);\n>>           if (ret) {\n>>               LOG(RPI, Error)\n>> diff --git a/src/libcamera/v4l2_device.cpp \n>> b/src/libcamera/v4l2_device.cpp\n>> index 3fc8438f6579..b22a981ff1a6 100644\n>> --- a/src/libcamera/v4l2_device.cpp\n>> +++ b/src/libcamera/v4l2_device.cpp\n>> @@ -745,8 +745,8 @@ void V4L2Device::eventAvailable()\n>>     static const std::map v4l2ToColorSpace = {\n>>       { V4L2_COLORSPACE_RAW, ColorSpace::Raw },\n>> -    { V4L2_COLORSPACE_JPEG, ColorSpace::Jpeg },\n>>       { V4L2_COLORSPACE_SRGB, ColorSpace::Srgb },\n>> +    { V4L2_COLORSPACE_JPEG, ColorSpace::Sycc },\n>>       { V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },\n>>       { V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },\n>>       { V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },\n>> @@ -771,8 +771,8 @@ static const std::map> ColorSpace::Range> v4l2ToRange = {\n>>     static const std::vector> \n>> colorSpaceToV4l2 = {\n>>       { ColorSpace::Raw, V4L2_COLORSPACE_RAW },\n>> -    { ColorSpace::Jpeg, V4L2_COLORSPACE_JPEG },\n>>       { ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },\n>> +    { ColorSpace::Sycc, V4L2_COLORSPACE_JPEG },\n>>       { ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },\n>>       { ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },\n>>       { ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },\n>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n>> index 505cc3dc1a0d..2a58e34a7cdb 100644\n>> --- a/src/py/libcamera/py_main.cpp\n>> +++ b/src/py/libcamera/py_main.cpp\n>> @@ -602,8 +602,8 @@ PYBIND11_MODULE(_libcamera, m)\n>>           .def_readwrite(\"ycbcrEncoding\", &ColorSpace::ycbcrEncoding)\n>>           .def_readwrite(\"range\", &ColorSpace::range)\n>>           .def_static(\"Raw\", []() { return ColorSpace::Raw; })\n>> -        .def_static(\"Jpeg\", []() { return ColorSpace::Jpeg; })\n>>           .def_static(\"Srgb\", []() { return ColorSpace::Srgb; })\n>> +        .def_static(\"Sycc\", []() { return ColorSpace::Sycc; })\n>>           .def_static(\"Smpte170m\", []() { return \n>> ColorSpace::Smpte170m; })\n>>           .def_static(\"Rec709\", []() { return ColorSpace::Rec709; })\n>>           .def_static(\"Rec2020\", []() { return ColorSpace::Rec2020; });\n>>","headers":{"Return-Path":"","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 BE975BE173\n\tfor ;\n\tMon, 22 Aug 2022 15:22:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE11D61FBE;\n\tMon, 22 Aug 2022 17:22:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AB05C61FA1\n\tfor ;\n\tMon, 22 Aug 2022 17:22:48 +0200 (CEST)","from [IPV6:2401:4900:1f3f:94e3:8381:d17e:f7b0:48ce] (unknown\n\t[IPv6:2401:4900:1f3f:94e3:8381:d17e:f7b0:48ce])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B676C2B3;\n\tMon, 22 Aug 2022 17:22:47 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661181769;\n\tbh=QTTnDGWmjglZAmb8OpLnhkDspZNPDAf/ucC54lZaNKo=;\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=JUG9cpG/YvoWfydBQgo0asBxy15+kh1uTUE/HiX1s3AYUrVdnvcb8WlmV/TL+5BDw\n\tsGy/tg/J2jMFYPCnnIF7nTDspgZRezgXy+/KtkqDG0Daa3UZJq37pXZKq77r0bsQa7\n\t+banOAnvOPdXOWNRrevNJ+xNbOuyPNCLK3076+HvsXdb7uywBnUUUWKl6qQZWsBTMe\n\tTghB5w0Q7c1WpKEYrDJ/n+4DgRetok7JqCxMpe07ntvJzTc8g6KnknqsI7iUhFC+Wh\n\tH5Emv7eUYb67/VzdIvIhUchGHNqUROgc3Ctw3s3Fr3vvII/m/Dnv+u4lCMYHknWApI\n\t0PIilaHehLtvQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661181768;\n\tbh=QTTnDGWmjglZAmb8OpLnhkDspZNPDAf/ucC54lZaNKo=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=Imet2mQtABa2W/GHPQbCpPxISMpvtNiFnVULuCAlF7BhUFiZpEdAHroWqxDt7Mqw3\n\tugo83PM5eRqWwLvtlMAlJAkF/wLlv0uFmEh8UI9C8MWcF+hR9xQLvEg2aiql7DPemd\n\tyRotxYZiEScliW4lPN83yoJYvtQKQVaUPPnT5Phw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Imet2mQt\"; dkim-atps=neutral","Message-ID":"<56677d58-4bf3-2cc4-a885-5dd86ba99350@ideasonboard.com>","Date":"Mon, 22 Aug 2022 20:52:42 +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 ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220817211824.20980-1-laurent.pinchart@ideasonboard.com>\n\t<8f9776bf-3c81-cc14-1048-10d877790c97@ideasonboard.com>","In-Reply-To":"<8f9776bf-3c81-cc14-1048-10d877790c97@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24726,"web_url":"https://patchwork.libcamera.org/comment/24726/","msgid":"","date":"2022-08-22T15:33:49","subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Mon, Aug 22, 2022 at 08:52:42PM +0530, Umang Jain wrote:\n> On 8/18/22 7:31 PM, Umang Jain via libcamera-devel wrote:\n> > Hi Laurent\n> >\n> > Thank you for the patch and detailed commit message.\n> >\n> > On 8/18/22 02:48, Laurent Pinchart wrote:\n> >> The JPEG color space is badly name, as the JPEG specification (ITU-T\n> >> T.81) doesn't define any particular color space:\n> >>\n> >>      The interchange format does not specify a complete coded image\n> >>      representation. Application-dependent information, e.g. colour\n> >>      space, is outside the scope of this Specification.\n> >>\n> >> The JFIF specification (ITU-T T.871) is clearer as it requires ITU-R\n> >> BT.601 YCbCr encoding and a full quantization range:\n> >>\n> >>    The interpretations of Y, CB, and CR are derived from the E'Y, E'Cb,\n> >>    and E'Cr signals defined in the 625-line specification of Rec. ITU-R\n> >>    BT.601, but these signals are normalized so as to permit the usage of\n> >>    the full range of 256 levels of the 8-bit binary encoding of the Y\n> >>    component.\n> >>\n> >> It however doesn't specify color primaries or a transfer function\n> >> explicitly. It only mentions the latter when describing the conversion\n> >> from YCbCr to RGB:\n> >>\n> >>    The inverse relationship for computing full scale 8-bit per colour\n> >>    channel gamma pre-corrected RGB values (following Rec. ITU-R BT.601\n> >>    gamma pre-correction and colour primary specifications) from YCbCr\n> >>    colours (with 256 levels per component) can be computed as follows:\n> >>    [...]\n> >>\n> >> Given that ITU-R BT.601-5 (1995) didn't specify color primaries or a\n> >> transfer function, and that the later ITU-R BT.601-7 (2011) version\n> >> specifies color primaries for the 625-line variant that do not match\n> >> sRGB, the JPEG color space in libcamera is badly named.\n> >>\n> >> Rename the color space to sYCC, as its definition matches the sYCC\n> >> standard, and indicate that it is typically used to encode JPEG images.\n> >>\n> >> Signed-off-by: Laurent Pinchart \n> >\n> >\n> > Reviewed-by: Umang Jain \n> >\n> > CC'ing David from RPi especially (for heads-up) because this patch \n> > will affects libcamera-apps.\n> >\n> >> ---\n> >>\n> >> Hans, following our discussion on IRC, I double-check the JFIF standard,\n> >> and my findings are described in the commit message above. It seems that\n> >> naming the color space \"JPEG\" in V4L2 is actually incorrect, as JFIF\n> >> expects the 626-line ITU-R BT.601 chromaticities for the primary colors,\n> >> which are slightly different than sRGB (the 525-line chromaticities\n> >> match SMPTE 170M and SMPTE 240M, but not sRGB either). It seems that we\n> >> are missing a V4L2_COLORSPACE_* value to describe the 625-line ITU-R\n> >> BT.601 chromaticities, is that an oversight ?\n> >>\n> >> ---\n> >>   include/libcamera/color_space.h               |  2 +-\n> >>   src/libcamera/color_space.cpp                 | 28 +++++++++----------\n> >>   .../pipeline/raspberrypi/raspberrypi.cpp      | 10 +++----\n> >>   src/libcamera/v4l2_device.cpp                 |  4 +--\n> >>   src/py/libcamera/py_main.cpp                  |  2 +-\n> >>   5 files changed, 23 insertions(+), 23 deletions(-)\n> >>\n> >> diff --git a/include/libcamera/color_space.h \n> >> b/include/libcamera/color_space.h\n> >> index 0d39fbc02220..8030a264c66f 100644\n> >> --- a/include/libcamera/color_space.h\n> >> +++ b/include/libcamera/color_space.h\n> >> @@ -46,8 +46,8 @@ public:\n> >>       }\n> >>         static const ColorSpace Raw;\n> >> -    static const ColorSpace Jpeg;\n> >>       static const ColorSpace Srgb;\n> >> +    static const ColorSpace Sycc;\n> >>       static const ColorSpace Smpte170m;\n> >>       static const ColorSpace Rec709;\n> >>       static const ColorSpace Rec2020;\n> >> diff --git a/src/libcamera/color_space.cpp \n> >> b/src/libcamera/color_space.cpp\n> >> index caf397607b10..6ace40dcb0fb 100644\n> >> --- a/src/libcamera/color_space.cpp\n> >> +++ b/src/libcamera/color_space.cpp\n> >> @@ -121,8 +121,8 @@ namespace libcamera {\n> >>    * \\brief Assemble and return a readable string representation of the\n> >>    * ColorSpace\n> >>    *\n> >> - * If the color space matches a standard ColorSpace (such as ColorSpace::Jpeg)\n> >> - * then the short name of the color space (\"JPEG\") is returned. Otherwise\n> >> + * If the color space matches a standard ColorSpace (such as ColorSpace::Sycc)\n> >> + * then the short name of the color space (\"sYCC\") is returned. Otherwise\n> >>    * the four constituent parts of the ColorSpace are assembled into a longer\n> >>    * string.\n> \n> Below this paragraph, we have\n\nI assume you mean above.\n\n>  * - JPEG\n> as well.\n> \n> We will probably in tricky situation here. Do we want to drop the JPEG \n> link entirely, or change to\n> \n>  * - sYCC\n> \n> Can you add your thoughts here and update the patch (no need to re-send \n> I think, but if you decide to push it...)\n> \n> (It will be more tricky for sRGB too, as we will not be going with \n> kernel's definition)\n\nGood points. There are also two other occurrence of \"JPEG\". I'll apply\nthe following changes for v2.\n\ndiff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp\nindex 6ace40dcb0fb..877c03848db0 100644\n--- a/src/libcamera/color_space.cpp\n+++ b/src/libcamera/color_space.cpp\n@@ -29,21 +29,28 @@ namespace libcamera {\n * (sometimes also referred to as the quantisation) of the color space.\n *\n * Certain combinations of these fields form well-known standard color\n- * spaces such as \"JPEG\" or \"REC709\".\n+ * spaces such as \"sRGB\" or \"Rec709\".\n *\n * In the strictest sense a \"color space\" formally only refers to the\n * color primaries and white point. Here, however, the ColorSpace class\n * adopts the common broader usage that includes the transfer function,\n * Y'CbCr encoding method and quantisation.\n *\n- * For more information on the specific color spaces described here, please\n- * see:\n+ * More information on color spaces is available in the V4L2 documentation, see\n+ * in particular\n *\n * - sRGB\n * - JPEG\n * - SMPTE 170M\n * - Rec.709\n * - Rec.2020\n+ *\n+ * Note that there is no guarantee of a 1:1 mapping between color space names\n+ * and definitions in libcamera and V4L2. A notable difference is that the sYCC\n+ * libcamera color space is called JPEG in V4L2 due to historical reasons.\n+ *\n+ * \\todo Define the color space fully in the libcamera API to avoid referencing\n+ * V4L2\n */\n \n /**\n@@ -245,7 +252,7 @@ const ColorSpace ColorSpace::Raw = {\n /**\n * \\brief A constant representing the sRGB color space\n *\n- * This is identical to the JPEG color space except that the Y'CbCr\n+ * This is identical to the sYCC color space except that the Y'CbCr\n * range is limited rather than full.\n */\n const ColorSpace ColorSpace::Srgb = {\n\n\nThe \"Note that ...\" part should be expanded in the patch that redefines\nsRGB to list that difference as well.\n\n> >>    *\n> >> @@ -134,8 +134,8 @@ std::string ColorSpace::toString() const\n> >>         static const std::array, \n> >> 6> colorSpaceNames = { {\n> >>           { ColorSpace::Raw, \"RAW\" },\n> >> -        { ColorSpace::Jpeg, \"JPEG\" },\n> >>           { ColorSpace::Srgb, \"sRGB\" },\n> >> +        { ColorSpace::Sycc, \"sYCC\" },\n> >>           { ColorSpace::Smpte170m, \"SMPTE170M\" },\n> >>           { ColorSpace::Rec709, \"Rec709\" },\n> >>           { ColorSpace::Rec2020, \"Rec2020\" },\n> >> @@ -242,17 +242,6 @@ const ColorSpace ColorSpace::Raw = {\n> >>       Range::Full\n> >>   };\n> >>   -/**\n> >> - * \\brief A constant representing the JPEG color space used for\n> >> - * encoding JPEG images\n> >> - */\n> >> -const ColorSpace ColorSpace::Jpeg = {\n> >> -    Primaries::Rec709,\n> >> -    TransferFunction::Srgb,\n> >> -    YcbcrEncoding::Rec601,\n> >> -    Range::Full\n> >> -};\n> >> -\n> >>   /**\n> >>    * \\brief A constant representing the sRGB color space\n> >>    *\n> >> @@ -266,6 +255,17 @@ const ColorSpace ColorSpace::Srgb = {\n> >>       Range::Limited\n> >>   };\n> >>   +/**\n> >> + * \\brief A constant representing the sYCC color space, typically \n> >> used for\n> >> + * encoding JPEG images\n> >> + */\n> >> +const ColorSpace ColorSpace::Sycc = {\n> >> +    Primaries::Rec709,\n> >> +    TransferFunction::Srgb,\n> >> +    YcbcrEncoding::Rec601,\n> >> +    Range::Full\n> >> +};\n> >> +\n> >>   /**\n> >>    * \\brief A constant representing the SMPTE170M color space\n> >>    */\n> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp \n> >> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> index e895584d4fbc..b4094898ca6c 100644\n> >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> @@ -593,11 +593,11 @@ CameraConfiguration \n> >> *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> >>               fmts = data->isp_[Isp::Output0].dev()->formats();\n> >>               pixelFormat = formats::NV12;\n> >>               /*\n> >> -             * Still image codecs usually expect the JPEG color space.\n> >> +             * Still image codecs usually expect the sYCC color space.\n> >>                * Even RGB codecs will be fine as the RGB we get with the\n> >> -             * JPEG color space is the same as sRGB.\n> >> +             * sYCC color space is the same as sRGB.\n> >>                */\n> >> -            colorSpace = ColorSpace::Jpeg;\n> >> +            colorSpace = ColorSpace::Sycc;\n> >>               /* Return the largest sensor resolution. */\n> >>               size = sensorSize;\n> >>               bufferCount = 1;\n> >> @@ -628,7 +628,7 @@ CameraConfiguration \n> >> *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> >>           case StreamRole::Viewfinder:\n> >>               fmts = data->isp_[Isp::Output0].dev()->formats();\n> >>               pixelFormat = formats::ARGB8888;\n> >> -            colorSpace = ColorSpace::Jpeg;\n> >> +            colorSpace = ColorSpace::Sycc;\n> >>               size = { 800, 600 };\n> >>               bufferCount = 4;\n> >>               outCount++;\n> >> @@ -835,7 +835,7 @@ int PipelineHandlerRPi::configure(Camera *camera, \n> >> CameraConfiguration *config)\n> >>           format.size = maxSize;\n> >>           format.fourcc = dev->toV4L2PixelFormat(formats::YUV420);\n> >>           /* No one asked for output, so the color space doesn't \n> >> matter. */\n> >> -        format.colorSpace = ColorSpace::Jpeg;\n> >> +        format.colorSpace = ColorSpace::Sycc;\n> >>           ret = dev->setFormat(&format);\n> >>           if (ret) {\n> >>               LOG(RPI, Error)\n> >> diff --git a/src/libcamera/v4l2_device.cpp \n> >> b/src/libcamera/v4l2_device.cpp\n> >> index 3fc8438f6579..b22a981ff1a6 100644\n> >> --- a/src/libcamera/v4l2_device.cpp\n> >> +++ b/src/libcamera/v4l2_device.cpp\n> >> @@ -745,8 +745,8 @@ void V4L2Device::eventAvailable()\n> >>     static const std::map v4l2ToColorSpace = {\n> >>       { V4L2_COLORSPACE_RAW, ColorSpace::Raw },\n> >> -    { V4L2_COLORSPACE_JPEG, ColorSpace::Jpeg },\n> >>       { V4L2_COLORSPACE_SRGB, ColorSpace::Srgb },\n> >> +    { V4L2_COLORSPACE_JPEG, ColorSpace::Sycc },\n> >>       { V4L2_COLORSPACE_SMPTE170M, ColorSpace::Smpte170m },\n> >>       { V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },\n> >>       { V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },\n> >> @@ -771,8 +771,8 @@ static const std::map >> ColorSpace::Range> v4l2ToRange = {\n> >>     static const std::vector> \n> >> colorSpaceToV4l2 = {\n> >>       { ColorSpace::Raw, V4L2_COLORSPACE_RAW },\n> >> -    { ColorSpace::Jpeg, V4L2_COLORSPACE_JPEG },\n> >>       { ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },\n> >> +    { ColorSpace::Sycc, V4L2_COLORSPACE_JPEG },\n> >>       { ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },\n> >>       { ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },\n> >>       { ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },\n> >> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> >> index 505cc3dc1a0d..2a58e34a7cdb 100644\n> >> --- a/src/py/libcamera/py_main.cpp\n> >> +++ b/src/py/libcamera/py_main.cpp\n> >> @@ -602,8 +602,8 @@ PYBIND11_MODULE(_libcamera, m)\n> >>           .def_readwrite(\"ycbcrEncoding\", &ColorSpace::ycbcrEncoding)\n> >>           .def_readwrite(\"range\", &ColorSpace::range)\n> >>           .def_static(\"Raw\", []() { return ColorSpace::Raw; })\n> >> -        .def_static(\"Jpeg\", []() { return ColorSpace::Jpeg; })\n> >>           .def_static(\"Srgb\", []() { return ColorSpace::Srgb; })\n> >> +        .def_static(\"Sycc\", []() { return ColorSpace::Sycc; })\n> >>           .def_static(\"Smpte170m\", []() { return \n> >> ColorSpace::Smpte170m; })\n> >>           .def_static(\"Rec709\", []() { return ColorSpace::Rec709; })\n> >>           .def_static(\"Rec2020\", []() { return ColorSpace::Rec2020; });\n> >>","headers":{"Return-Path":"","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 6BB3BC3272\n\tfor ;\n\tMon, 22 Aug 2022 15:33:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BD02961FBE;\n\tMon, 22 Aug 2022 17:33:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D483861FA1\n\tfor ;\n\tMon, 22 Aug 2022 17:33:53 +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 391A02B3;\n\tMon, 22 Aug 2022 17:33:53 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661182435;\n\tbh=z4zXjg9QLh25hdy2hf+ugWFjb1iGCHdqpQpI5m1tA5I=;\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=U1vER2qnxp9rFfC2y/e43/g6pb28XhmWBlkC3OBQ4lwt9ofZN6rXGtNNNnYXMHm50\n\tKVpgyB3pn2suOUGBq7kNeL2obK0yujp/FDRxAjbmrMh26s5ZXOcjbersLH5uM/VXNb\n\tDV5fSrfacJfOlKoCnLrsUigZhxM2S2SrrFryhLezuR8T9ydGT4J+y5Xohf5Y6iBEns\n\tf/3j9qiU+fbEsIirT/kkNC1i03+JKyTjvusZRpc3kPm58SoMQo2DwdZPRybM5IYdsq\n\tugkvKYdi7whc9W8QqvZqb5Zpmjv0PPdP9r+2SALD9Kwll2Q5wZ6PQ4nl+psFE7Mxtp\n\tNG0ix1L4+4z9w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661182433;\n\tbh=z4zXjg9QLh25hdy2hf+ugWFjb1iGCHdqpQpI5m1tA5I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JlQP9N6kIbIIDlPdRtJfQX4CZbf9You8xWXQYotTP0e9x/rhPPGf1xw9HHDPmNj17\n\tbu+ES+NS/C5bRnpIJq8mlzb8Z75xMf76wpVQLnm8xB+wiwfgJV3szt+igpcAXEPV0k\n\t4qfTHgXyNXkY/QND5ULXArO/zBCwavJVvJzT/xEA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JlQP9N6k\"; dkim-atps=neutral","Date":"Mon, 22 Aug 2022 18:33:49 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220817211824.20980-1-laurent.pinchart@ideasonboard.com>\n\t<8f9776bf-3c81-cc14-1048-10d877790c97@ideasonboard.com>\n\t<56677d58-4bf3-2cc4-a885-5dd86ba99350@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<56677d58-4bf3-2cc4-a885-5dd86ba99350@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: color_space: Rename Jpeg\n\tto Sycc","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]