[{"id":24779,"web_url":"https://patchwork.libcamera.org/comment/24779/","msgid":"<20220825200813.GQ109174@pyrite.rasen.tech>","date":"2022-08-25T20:08:13","subject":"Re: [libcamera-devel] [PATCH 5/6] libcamera: pipeline: rkisp1:\n\tImplement color space support","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Laurent,\n\nOn Tue, Aug 23, 2022 at 08:43:13PM +0300, Laurent Pinchart via libcamera-devel wrote:\n\nI think some changelog would be nice to have :)\n\n\nPaul\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 ++++++++++++++++++++----\n> 1 file changed, 33 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 03bbe6b467ae..25fbf9f1a0a9 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -18,6 +18,7 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -416,11 +417,13 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> {\n> \tconst CameraSensor *sensor = data_->sensor_.get();\n> \tunsigned int pathCount = data_->selfPath_ ? 2 : 1;\n> -\tStatus status = Valid;\n> +\tStatus status;\n> \n> \tif (config_.empty())\n> \t\treturn Invalid;\n> \n> +\tstatus = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace);\n> +\n> \tif (transform != Transform::Identity) {\n> \t\ttransform = Transform::Identity;\n> \t\tstatus = Adjusted;\n> @@ -547,21 +550,44 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> \tif (roles.empty())\n> \t\treturn config;\n> \n> +\t/*\n> +\t * As the ISP can't output different color spaces for the main and self\n> +\t * path, pick a sensible default color space based on the role of the\n> +\t * first stream and use it for all streams.\n> +\t */\n> +\tstd::optional colorSpace;\n> +\n> \tbool mainPathAvailable = true;\n> \tbool selfPathAvailable = data->selfPath_;\n> +\n> \tfor (const StreamRole role : roles) {\n> \t\tbool useMainPath;\n> \n> \t\tswitch (role) {\n> -\t\tcase StreamRole::StillCapture: {\n> +\t\tcase StreamRole::StillCapture:\n> \t\t\tuseMainPath = mainPathAvailable;\n> +\t\t\t/* JPEG encoders typically expect sYCC. */\n> +\t\t\tif (!colorSpace)\n> +\t\t\t\tcolorSpace = ColorSpace::Sycc;\n> \t\t\tbreak;\n> -\t\t}\n> +\n> \t\tcase StreamRole::Viewfinder:\n> -\t\tcase StreamRole::VideoRecording: {\n> \t\t\tuseMainPath = !selfPathAvailable;\n> +\t\t\t/*\n> +\t\t\t * sYCC is the YCbCr encoding of sRGB, which is commonly\n> +\t\t\t * used by displays.\n> +\t\t\t */\n> +\t\t\tif (!colorSpace)\n> +\t\t\t\tcolorSpace = ColorSpace::Sycc;\n> \t\t\tbreak;\n> -\t\t}\n> +\n> +\t\tcase StreamRole::VideoRecording:\n> +\t\t\tuseMainPath = !selfPathAvailable;\n> +\t\t\t/* Rec. 709 is a good default for HD video recording. */\n> +\t\t\tif (!colorSpace)\n> +\t\t\t\tcolorSpace = ColorSpace::Rec709;\n> +\t\t\tbreak;\n> +\n> \t\tdefault:\n> \t\t\tLOG(RkISP1, Warning)\n> \t\t\t\t<< \"Requested stream role not supported: \" << role;\n> @@ -580,6 +606,7 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> \t\t\tselfPathAvailable = false;\n> \t\t}\n> \n> +\t\tcfg.colorSpace = colorSpace;\n> \t\tconfig->addConfiguration(cfg);\n> \t}\n> \n> @@ -642,6 +669,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> +\tformat.colorSpace = config->at(0).colorSpace;\n> \tret = isp_->setFormat(2, &format);\n> \tif (ret < 0)\n> \t\treturn ret;\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 D9802C0DA4\n\tfor ;\n\tThu, 25 Aug 2022 20:08:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3443D61FBF;\n\tThu, 25 Aug 2022 22:08:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1B0FA61FA0\n\tfor ;\n\tThu, 25 Aug 2022 22:08:21 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id ACFD12B3;\n\tThu, 25 Aug 2022 22:08:19 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661458102;\n\tbh=P3W+vYVCrTSbiP7+sdtoamM4qpRtc6eZHAHcu863y/c=;\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=C6A9/2SaTFPLynL38rZh9H6GtlGhso3m3sKWKWv1hmVSnAn1SoU6Avj5l2iEGXwQN\n\tQhzkR6Jh3LjhOjjmVm2jtrRjjgLIzg7zJVPToSrSWApQpgwBBBkA7iYBmttvuh50A0\n\tF3v0YCWwOO0aNuP02e3U0LZR7fhetH7KcwOgmBFHBsCrPmQoHmRjoNxJPPFQYcFVTa\n\te41FWCZ/24tAp5uPVdbCAF3wVquqdh51G6jSvl/a1wDG4yV7DCbz86Q8NGakFKSsvj\n\tl2uZgZxOEvQPJF1AfQ+dV0ir6LU9a6MF5wTWOQuIr8rAl80i/cjpi0wiaJZl/0o2m4\n\tHbdfzfRUW9sZQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661458100;\n\tbh=P3W+vYVCrTSbiP7+sdtoamM4qpRtc6eZHAHcu863y/c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DU7Txt5t3tUp295hfiic2CqkpAPOO31zYCYu0JwBLYN/xtXWeAOfcxz4QYFZDdCsY\n\tNiIkuaVPaGBpYZVPrl1DSxU0IDIPOebHpgvXPXxB2BDuAwuh0503vpM9h+sPNKj7Ol\n\tIk4j6nuwWuVymjxxXo+TgfxMtFPg9xjEVl8iy5pU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"DU7Txt5t\"; dkim-atps=neutral","Date":"Thu, 25 Aug 2022 15:08:13 -0500","To":"Laurent Pinchart ","Message-ID":"<20220825200813.GQ109174@pyrite.rasen.tech>","References":"<20220823174314.14881-1-laurent.pinchart@ideasonboard.com>\n\t<20220823174314.14881-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220823174314.14881-6-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 5/6] libcamera: pipeline: rkisp1:\n\tImplement color space support","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":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24781,"web_url":"https://patchwork.libcamera.org/comment/24781/","msgid":"","date":"2022-08-25T20:17:11","subject":"Re: [libcamera-devel] [PATCH 5/6] libcamera: pipeline: rkisp1:\n\tImplement color space support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Thu, Aug 25, 2022 at 03:08:13PM -0500, paul.elder@ideasonboard.com wrote:\n> On Tue, Aug 23, 2022 at 08:43:13PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> \n> I think some changelog would be nice to have :)\n\nOops :-) I'll add\n\nImplement color space support in the rkisp1 pipeline handler, in the\nconfiguration generation, configuration validation and camera\nconfiguration. As all the processing related to the color space is\nperformed in the part of the pipeline shared by all streams, a single\ncolor space must cover all stream configurations. This is enforced\nmanually when generating the configuration, and with the\nvalidateColorSpaces() helper when validating it.\n\nOnly the Y'CbCr encoding and quantization range are currently taken into\naccount, and they are programmed through the V4L2 color space API. The\nprimary colors chromaticities and the transfer function need to be\nconfigured in the ISP parameters buffer, and thus conveyed to the IPA,\nbut the rkisp1 driver does not currently support tone mapping, so this\nwill be done later.\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 ++++++++++++++++++++----\n> > 1 file changed, 33 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 03bbe6b467ae..25fbf9f1a0a9 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -18,6 +18,7 @@\n> > #include \n> > \n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -416,11 +417,13 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > {\n> > \tconst CameraSensor *sensor = data_->sensor_.get();\n> > \tunsigned int pathCount = data_->selfPath_ ? 2 : 1;\n> > -\tStatus status = Valid;\n> > +\tStatus status;\n> > \n> > \tif (config_.empty())\n> > \t\treturn Invalid;\n> > \n> > +\tstatus = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace);\n> > +\n> > \tif (transform != Transform::Identity) {\n> > \t\ttransform = Transform::Identity;\n> > \t\tstatus = Adjusted;\n> > @@ -547,21 +550,44 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > \tif (roles.empty())\n> > \t\treturn config;\n> > \n> > +\t/*\n> > +\t * As the ISP can't output different color spaces for the main and self\n> > +\t * path, pick a sensible default color space based on the role of the\n> > +\t * first stream and use it for all streams.\n> > +\t */\n> > +\tstd::optional colorSpace;\n> > +\n> > \tbool mainPathAvailable = true;\n> > \tbool selfPathAvailable = data->selfPath_;\n> > +\n> > \tfor (const StreamRole role : roles) {\n> > \t\tbool useMainPath;\n> > \n> > \t\tswitch (role) {\n> > -\t\tcase StreamRole::StillCapture: {\n> > +\t\tcase StreamRole::StillCapture:\n> > \t\t\tuseMainPath = mainPathAvailable;\n> > +\t\t\t/* JPEG encoders typically expect sYCC. */\n> > +\t\t\tif (!colorSpace)\n> > +\t\t\t\tcolorSpace = ColorSpace::Sycc;\n> > \t\t\tbreak;\n> > -\t\t}\n> > +\n> > \t\tcase StreamRole::Viewfinder:\n> > -\t\tcase StreamRole::VideoRecording: {\n> > \t\t\tuseMainPath = !selfPathAvailable;\n> > +\t\t\t/*\n> > +\t\t\t * sYCC is the YCbCr encoding of sRGB, which is commonly\n> > +\t\t\t * used by displays.\n> > +\t\t\t */\n> > +\t\t\tif (!colorSpace)\n> > +\t\t\t\tcolorSpace = ColorSpace::Sycc;\n> > \t\t\tbreak;\n> > -\t\t}\n> > +\n> > +\t\tcase StreamRole::VideoRecording:\n> > +\t\t\tuseMainPath = !selfPathAvailable;\n> > +\t\t\t/* Rec. 709 is a good default for HD video recording. */\n> > +\t\t\tif (!colorSpace)\n> > +\t\t\t\tcolorSpace = ColorSpace::Rec709;\n> > +\t\t\tbreak;\n> > +\n> > \t\tdefault:\n> > \t\t\tLOG(RkISP1, Warning)\n> > \t\t\t\t<< \"Requested stream role not supported: \" << role;\n> > @@ -580,6 +606,7 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > \t\t\tselfPathAvailable = false;\n> > \t\t}\n> > \n> > +\t\tcfg.colorSpace = colorSpace;\n> > \t\tconfig->addConfiguration(cfg);\n> > \t}\n> > \n> > @@ -642,6 +669,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> > \tif (ret < 0)\n> > \t\treturn ret;\n> > \n> > +\tformat.colorSpace = config->at(0).colorSpace;\n> > \tret = isp_->setFormat(2, &format);\n> > \tif (ret < 0)\n> > \t\treturn ret;","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 B5285C0DA4\n\tfor ;\n\tThu, 25 Aug 2022 20:17:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1006A61FC0;\n\tThu, 25 Aug 2022 22:17:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B5F6B61FA0\n\tfor ;\n\tThu, 25 Aug 2022 22:17:18 +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 09E8B2B3;\n\tThu, 25 Aug 2022 22:17:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661458640;\n\tbh=QG8/72vEQ5HaLiDOi8vEiSi/H/gLNfwiMBnHBk/gzd4=;\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=IzdD95GUB9/we33iu1aHB9ocsYkDsedpPcq6n4WPm9k70/ugZoytd0cGoDAdrpXVC\n\tyMOvAAkVT29GfDVc39mvIFgvNZAsoucPSn+UXlrloJhd/usfF4dSOvKrYY4ogA1Iv0\n\tpZh1d/LETWYxOM7Nv7iavMusf5grQo9kdTKm4cSplUG+7lvO6c98dQ+HwAvUvbXIyA\n\tBxUQfJtgjWIep/Nl2h3HYD1JWgRPVkeYwi4+MtV+2K0ZgI7jlqkXUWLlBxWpqkRf5Y\n\tE6iN4Qu7uo4CB91MrHZoQr0RjbrR3D3i53CtXaCWXcbr4Ejm4GcR8Sl9QK7xV+ARJW\n\t4slV9huTE/VYg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661458638;\n\tbh=QG8/72vEQ5HaLiDOi8vEiSi/H/gLNfwiMBnHBk/gzd4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DEbDLRcS7Lh4uux5+Xk+Ce2rfh0DsjQ+HzD1yx5m+z0cb1YYnoZua7fgt7L5T00ev\n\tSDWUeppnLBgQobMKeIiOedAPbE72ovF3bEYDHzGFhZ4kNxkyCnvurFY/8XSwo2+KuL\n\t7Oc10ABd4NmWkb6XW4L1TuRjfR+5x138M54PukDs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"DEbDLRcS\"; dkim-atps=neutral","Date":"Thu, 25 Aug 2022 23:17:11 +0300","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"<20220823174314.14881-1-laurent.pinchart@ideasonboard.com>\n\t<20220823174314.14881-6-laurent.pinchart@ideasonboard.com>\n\t<20220825200813.GQ109174@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220825200813.GQ109174@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH 5/6] libcamera: pipeline: rkisp1:\n\tImplement color space support","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":24784,"web_url":"https://patchwork.libcamera.org/comment/24784/","msgid":"<20220825211706.GU109174@pyrite.rasen.tech>","date":"2022-08-25T21:17:06","subject":"Re: [libcamera-devel] [PATCH 5/6] libcamera: pipeline: rkisp1:\n\tImplement color space support","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Laurent,\n\nOn Thu, Aug 25, 2022 at 11:17:11PM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> On Thu, Aug 25, 2022 at 03:08:13PM -0500, paul.elder@ideasonboard.com wrote:\n> > On Tue, Aug 23, 2022 at 08:43:13PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > \n> > I think some changelog would be nice to have :)\n> \n> Oops :-) I'll add\n> \n> Implement color space support in the rkisp1 pipeline handler, in the\n> configuration generation, configuration validation and camera\n> configuration. As all the processing related to the color space is\n> performed in the part of the pipeline shared by all streams, a single\n> color space must cover all stream configurations. This is enforced\n> manually when generating the configuration, and with the\n> validateColorSpaces() helper when validating it.\n> \n> Only the Y'CbCr encoding and quantization range are currently taken into\n> account, and they are programmed through the V4L2 color space API. The\n> primary colors chromaticities and the transfer function need to be\n> configured in the ISP parameters buffer, and thus conveyed to the IPA,\n> but the rkisp1 driver does not currently support tone mapping, so this\n> will be done later.\n\nLooks good.\n\nReviewed-by: Paul Elder \n\n> \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 ++++++++++++++++++++----\n> > > 1 file changed, 33 insertions(+), 5 deletions(-)\n> > > \n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 03bbe6b467ae..25fbf9f1a0a9 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -18,6 +18,7 @@\n> > > #include \n> > > \n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -416,11 +417,13 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > {\n> > > \tconst CameraSensor *sensor = data_->sensor_.get();\n> > > \tunsigned int pathCount = data_->selfPath_ ? 2 : 1;\n> > > -\tStatus status = Valid;\n> > > +\tStatus status;\n> > > \n> > > \tif (config_.empty())\n> > > \t\treturn Invalid;\n> > > \n> > > +\tstatus = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace);\n> > > +\n> > > \tif (transform != Transform::Identity) {\n> > > \t\ttransform = Transform::Identity;\n> > > \t\tstatus = Adjusted;\n> > > @@ -547,21 +550,44 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > \tif (roles.empty())\n> > > \t\treturn config;\n> > > \n> > > +\t/*\n> > > +\t * As the ISP can't output different color spaces for the main and self\n> > > +\t * path, pick a sensible default color space based on the role of the\n> > > +\t * first stream and use it for all streams.\n> > > +\t */\n> > > +\tstd::optional colorSpace;\n> > > +\n> > > \tbool mainPathAvailable = true;\n> > > \tbool selfPathAvailable = data->selfPath_;\n> > > +\n> > > \tfor (const StreamRole role : roles) {\n> > > \t\tbool useMainPath;\n> > > \n> > > \t\tswitch (role) {\n> > > -\t\tcase StreamRole::StillCapture: {\n> > > +\t\tcase StreamRole::StillCapture:\n> > > \t\t\tuseMainPath = mainPathAvailable;\n> > > +\t\t\t/* JPEG encoders typically expect sYCC. */\n> > > +\t\t\tif (!colorSpace)\n> > > +\t\t\t\tcolorSpace = ColorSpace::Sycc;\n> > > \t\t\tbreak;\n> > > -\t\t}\n> > > +\n> > > \t\tcase StreamRole::Viewfinder:\n> > > -\t\tcase StreamRole::VideoRecording: {\n> > > \t\t\tuseMainPath = !selfPathAvailable;\n> > > +\t\t\t/*\n> > > +\t\t\t * sYCC is the YCbCr encoding of sRGB, which is commonly\n> > > +\t\t\t * used by displays.\n> > > +\t\t\t */\n> > > +\t\t\tif (!colorSpace)\n> > > +\t\t\t\tcolorSpace = ColorSpace::Sycc;\n> > > \t\t\tbreak;\n> > > -\t\t}\n> > > +\n> > > +\t\tcase StreamRole::VideoRecording:\n> > > +\t\t\tuseMainPath = !selfPathAvailable;\n> > > +\t\t\t/* Rec. 709 is a good default for HD video recording. */\n> > > +\t\t\tif (!colorSpace)\n> > > +\t\t\t\tcolorSpace = ColorSpace::Rec709;\n> > > +\t\t\tbreak;\n> > > +\n> > > \t\tdefault:\n> > > \t\t\tLOG(RkISP1, Warning)\n> > > \t\t\t\t<< \"Requested stream role not supported: \" << role;\n> > > @@ -580,6 +606,7 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > \t\t\tselfPathAvailable = false;\n> > > \t\t}\n> > > \n> > > +\t\tcfg.colorSpace = colorSpace;\n> > > \t\tconfig->addConfiguration(cfg);\n> > > \t}\n> > > \n> > > @@ -642,6 +669,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> > > \tif (ret < 0)\n> > > \t\treturn ret;\n> > > \n> > > +\tformat.colorSpace = config->at(0).colorSpace;\n> > > \tret = isp_->setFormat(2, &format);\n> > > \tif (ret < 0)\n> > > \t\treturn ret;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 33569C0DA4\n\tfor ;\n\tThu, 25 Aug 2022 21:17:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91E8B61FC0;\n\tThu, 25 Aug 2022 23:17:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A14A61FA0\n\tfor ;\n\tThu, 25 Aug 2022 23:17:13 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0C2202B3;\n\tThu, 25 Aug 2022 23:17:11 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661462234;\n\tbh=J5znx2Q0/guFSCFE2m5tWyuZ4RBKGJtBR1SCQElPdFQ=;\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=eo4AiBykwLZH2QM6d6nGk5nFPSMio4NykfQXoAGMFmzxffUizSPYOXn22/IUF/h27\n\tZPsiZv6eayTh1KYp5s8VHAfaH7SuEDlqbX7be+706fFCXmXgEDIN13phmjNz28LwPH\n\tMOUX8zeooWkmEHCcI0GPwoVtjN6U54t20AYFzFY61v2ZV9nJ5M739HcXXG8e3HZ0oM\n\t9Rmo+m1foJmIXATSRw51EOeM80VY1Y5JT1cR1tyfEQRF+9bUcQTuBtapaOIN+bJTdA\n\trCqlof3mcLz2z87yI9y/mygQEinky8BQBPJmL26ZqJ0VEf+17ttWwd7OTHuR917dzq\n\tnneuU736nKSxQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1661462232;\n\tbh=J5znx2Q0/guFSCFE2m5tWyuZ4RBKGJtBR1SCQElPdFQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AJqnQRh7UBXs3PZrJaij2/uEnnAM1HW5FC9TXLlXW8lBksU4CzZ8rRaCZWF6iGx5j\n\tJvDBaYclNNozz8A00nnN5b4BjAxrD+y36XA/DPeCAvIl+Sb1pb0fPJL5Vd/aukFPGC\n\tGOMHBG0cXK9AYiZRZEpw8jg+oTIYFTZ96aFSlt4k="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AJqnQRh7\"; dkim-atps=neutral","Date":"Thu, 25 Aug 2022 16:17:06 -0500","To":"Laurent Pinchart ","Message-ID":"<20220825211706.GU109174@pyrite.rasen.tech>","References":"<20220823174314.14881-1-laurent.pinchart@ideasonboard.com>\n\t<20220823174314.14881-6-laurent.pinchart@ideasonboard.com>\n\t<20220825200813.GQ109174@pyrite.rasen.tech>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 5/6] libcamera: pipeline: rkisp1:\n\tImplement color space support","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":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24828,"web_url":"https://patchwork.libcamera.org/comment/24828/","msgid":"","date":"2022-08-30T07:17:30","subject":"Re: [libcamera-devel] [PATCH 5/6] libcamera: pipeline: rkisp1:\n\tImplement color space support","submitter":{"id":123,"url":"https://patchwork.libcamera.org/api/people/123/","name":"Florian Sylvestre","email":"fsylvestre@baylibre.com"},"content":"Hi Laurent,\n\nReviewed-by: Florian Sylvestre \n\nOn Thu, 25 Aug 2022 at 23:17, Paul Elder via libcamera-devel\n wrote:\n>\n> Hi Laurent,\n>\n> On Thu, Aug 25, 2022 at 11:17:11PM +0300, Laurent Pinchart wrote:\n> > Hi Paul,\n> >\n> > On Thu, Aug 25, 2022 at 03:08:13PM -0500, paul.elder@ideasonboard.com wrote:\n> > > On Tue, Aug 23, 2022 at 08:43:13PM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > >\n> > > I think some changelog would be nice to have :)\n> >\n> > Oops :-) I'll add\n> >\n> > Implement color space support in the rkisp1 pipeline handler, in the\n> > configuration generation, configuration validation and camera\n> > configuration. As all the processing related to the color space is\n> > performed in the part of the pipeline shared by all streams, a single\n> > color space must cover all stream configurations. This is enforced\n> > manually when generating the configuration, and with the\n> > validateColorSpaces() helper when validating it.\n> >\n> > Only the Y'CbCr encoding and quantization range are currently taken into\n> > account, and they are programmed through the V4L2 color space API. The\n> > primary colors chromaticities and the transfer function need to be\n> > configured in the ISP parameters buffer, and thus conveyed to the IPA,\n> > but the rkisp1 driver does not currently support tone mapping, so this\n> > will be done later.\n>\n> Looks good.\n>\n> Reviewed-by: Paul Elder \n>\n> >\n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 ++++++++++++++++++++----\n> > > > 1 file changed, 33 insertions(+), 5 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > index 03bbe6b467ae..25fbf9f1a0a9 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > @@ -18,6 +18,7 @@\n> > > > #include \n> > > >\n> > > > #include \n> > > > +#include \n> > > > #include \n> > > > #include \n> > > > #include \n> > > > @@ -416,11 +417,13 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > > {\n> > > > const CameraSensor *sensor = data_->sensor_.get();\n> > > > unsigned int pathCount = data_->selfPath_ ? 2 : 1;\n> > > > - Status status = Valid;\n> > > > + Status status;\n> > > >\n> > > > if (config_.empty())\n> > > > return Invalid;\n> > > >\n> > > > + status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace);\n> > > > +\n> > > > if (transform != Transform::Identity) {\n> > > > transform = Transform::Identity;\n> > > > status = Adjusted;\n> > > > @@ -547,21 +550,44 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > > if (roles.empty())\n> > > > return config;\n> > > >\n> > > > + /*\n> > > > + * As the ISP can't output different color spaces for the main and self\n> > > > + * path, pick a sensible default color space based on the role of the\n> > > > + * first stream and use it for all streams.\n> > > > + */\n> > > > + std::optional colorSpace;\n> > > > +\n> > > > bool mainPathAvailable = true;\n> > > > bool selfPathAvailable = data->selfPath_;\n> > > > +\n> > > > for (const StreamRole role : roles) {\n> > > > bool useMainPath;\n> > > >\n> > > > switch (role) {\n> > > > - case StreamRole::StillCapture: {\n> > > > + case StreamRole::StillCapture:\n> > > > useMainPath = mainPathAvailable;\n> > > > + /* JPEG encoders typically expect sYCC. */\n> > > > + if (!colorSpace)\n> > > > + colorSpace = ColorSpace::Sycc;\n> > > > break;\n> > > > - }\n> > > > +\n> > > > case StreamRole::Viewfinder:\n> > > > - case StreamRole::VideoRecording: {\n> > > > useMainPath = !selfPathAvailable;\n> > > > + /*\n> > > > + * sYCC is the YCbCr encoding of sRGB, which is commonly\n> > > > + * used by displays.\n> > > > + */\n> > > > + if (!colorSpace)\n> > > > + colorSpace = ColorSpace::Sycc;\n> > > > break;\n> > > > - }\n> > > > +\n> > > > + case StreamRole::VideoRecording:\n> > > > + useMainPath = !selfPathAvailable;\n> > > > + /* Rec. 709 is a good default for HD video recording. */\n> > > > + if (!colorSpace)\n> > > > + colorSpace = ColorSpace::Rec709;\n> > > > + break;\n> > > > +\n> > > > default:\n> > > > LOG(RkISP1, Warning)\n> > > > << \"Requested stream role not supported: \" << role;\n> > > > @@ -580,6 +606,7 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> > > > selfPathAvailable = false;\n> > > > }\n> > > >\n> > > > + cfg.colorSpace = colorSpace;\n> > > > config->addConfiguration(cfg);\n> > > > }\n> > > >\n> > > > @@ -642,6 +669,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> > > > if (ret < 0)\n> > > > return ret;\n> > > >\n> > > > + format.colorSpace = config->at(0).colorSpace;\n> > > > ret = isp_->setFormat(2, &format);\n> > > > if (ret < 0)\n> > > > return ret;\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart","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 5D0FFC0DA4\n\tfor ;\n\tTue, 30 Aug 2022 07:17:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 795A761FC1;\n\tTue, 30 Aug 2022 09:17:44 +0200 (CEST)","from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com\n\t[IPv6:2a00:1450:4864:20::42e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0333B61FB9\n\tfor ;\n\tTue, 30 Aug 2022 09:17:41 +0200 (CEST)","by mail-wr1-x42e.google.com with SMTP id v16so10197984wrm.8\n\tfor ;\n\tTue, 30 Aug 2022 00:17:41 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1661843864;\n\tbh=QClkY1BmIq1LaHfLaVf5tc6Et1uNQ8j+F47d6so250c=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=GupUbXys7+lczk+GyODgZwqHmFHLjIQq80m+gcY1p88qaOVnmfBe5M4LTk17ve0jQ\n\tnIiADELHDNajbvZ2mR6glD9+zpZemSS3Rl0ZNTmkgW8bOvL/tp6NFDL4WyjfLqkYBh\n\tf9+dSWmeKbRtynmeln/YzPLmjOzlu0wilKsc8ekhXTZVxJyiSCqT2MUImyBRHEPgz6\n\tKZahs8C+n+EbkONtSAMnBiuFXdHXFtZOha9iR6Ecxwky9RvQmBfXjyl8O0ejrSglX0\n\tZNybKDfIZDYWyZZ8hAhI4Dewszcr+g99nkLVXaaPx7qCYhoSqtG9q+ehOLB73XUF0I\n\tvZ/MC8qQQ2sXw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=baylibre-com.20210112.gappssmtp.com; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc;\n\tbh=chZLMB26jryvJnzz+OjQftQfwoMWdHYbwsKVdVG/hFQ=;\n\tb=dR0sSlY440lfaWPh9dqnE8IGG4mC5bUv+1NWyWZRzsA09aBOcqCF8NGJUXK8wEiEhB\n\tKc/Nh2Q+ucpD8qWaXOiItACeE1XCTgVe7xoNDXiqAAbruifSdOamx5C+7rixVYFa/0yI\n\tStb5Gir49LUy9bI18mbJlHeeiJnrjveREeHRf+oMXHhzHZNN9ZjeMFCivVqi724MJiJa\n\tFSvFNQTHHCwxVj7UgyjNLf0ty+DksePYIyCYZZ6qmCm12mA+t0DcE/G3OLNF5Uqfl0Wk\n\tr92V8zHvfXZShsAl1FNEv97NsLJ5zJN7XlkqIXtSi0qZGzINGZ8WuLlu6r2UkBJWJseb\n\tshYQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=baylibre-com.20210112.gappssmtp.com\n\theader.i=@baylibre-com.20210112.gappssmtp.com header.b=\"dR0sSlY4\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc;\n\tbh=chZLMB26jryvJnzz+OjQftQfwoMWdHYbwsKVdVG/hFQ=;\n\tb=SGR259j9PN6mybV/gxw4Dy+9M6FRJfX2UrV0wMkz1b9MTLDSq4AA8H/w8K3si0V5Uk\n\taopc6sVSPH45Q7aF63ac0kZVI4TbdVC2lEOaixYXsAuzpWnRMDbNI6ZAFlZZo0ClTay5\n\t0F0qq48yGAflvtqdatYXus/ZQRBCw9ttDVUFA58HqWdCpQrle2Im2SR61/IVbTgW4dUj\n\tbjmdpxURmABJ3b+93IXxYSOHrJBqu0D0DCkwtctJwBNWtKzC5qzgeDSTJPaLuacBCFGd\n\tnbsdG6RiNDqlNJEcRwyaykNjkdGyvCLTPkxUVuwPSTCUqshKOvfNVt4VPkyEvPinfVVI\n\tDnxg==","X-Gm-Message-State":"ACgBeo3R8U/JNFX+1LDfV1JPH+8xPe91RXIYKoK6AiM9I373iMap4Ckw\n\tDwfuScwcVpO69N0bb69lAXZ7pjXEi2Fwjplp0AgCJTV1Dozv6Q==","X-Google-Smtp-Source":"AA6agR5x1/pglsMfzPflScvXEhIhrD543y9pzwsFPC5rQi47OHA8R9hHonNnlBz/Qm7hsX9XqseKE/cQA1RMUNAbrN0=","X-Received":"by 2002:a5d:5984:0:b0:226:de85:30ab with SMTP id\n\tn4-20020a5d5984000000b00226de8530abmr3174565wri.232.1661843861453;\n\tTue, 30 Aug 2022 00:17:41 -0700 (PDT)","MIME-Version":"1.0","References":"<20220823174314.14881-1-laurent.pinchart@ideasonboard.com>\n\t<20220823174314.14881-6-laurent.pinchart@ideasonboard.com>\n\t<20220825200813.GQ109174@pyrite.rasen.tech>\n\t\n\t<20220825211706.GU109174@pyrite.rasen.tech>","In-Reply-To":"<20220825211706.GU109174@pyrite.rasen.tech>","Date":"Tue, 30 Aug 2022 09:17:30 +0200","Message-ID":"","To":"paul.elder@ideasonboard.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 5/6] libcamera: pipeline: rkisp1:\n\tImplement color space support","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":"Florian Sylvestre via libcamera-devel\n\t","Reply-To":"Florian Sylvestre ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]