[{"id":21605,"web_url":"https://patchwork.libcamera.org/comment/21605/","msgid":"<20211206112129.khwcip3ohes5pxpf@uno.localdomain>","date":"2021-12-06T11:21:29","subject":"Re: [libcamera-devel] [PATCH v9 3/8] libcamera: Convert between\n\tColorSpace class and V4L2 formats","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David\n\nOn Mon, Dec 06, 2021 at 10:50:26AM +0000, David Plowman wrote:\n> Add functions to the V4L2Device class to convert to and from\n> libcamera ColorSpace.\n>\n> These methods are added to the base V4L2Device class so that they can\n> be shared both by the video device class and subdevices.\n>\n> With the ColorSpace class, the color space and related other fields\n> are stored together, corresponding to a number of fields in the\n> various different V4L2 format structures. Template methods are\n> therefore a convenient implementation, and we must explicitly\n> instantiate the templates that will be needed.\n>\n> Note that unset color spaces are converted to requests for the\n> device's \"default\" color space.\n>\n> Signed-off-by: David Plowman \n\nThanks\n\nReviewed-by: Jacopo Mondi \n\n\n> ---\n> include/libcamera/internal/v4l2_device.h | 7 +\n> src/libcamera/v4l2_device.cpp | 192 +++++++++++++++++++++++\n> 2 files changed, 199 insertions(+)\n>\n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index 8886b750..38cf93a6 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -18,6 +18,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n>\n> namespace libcamera {\n> @@ -45,6 +46,12 @@ public:\n>\n> \tvoid updateControlInfo();\n>\n> +\ttemplate\n> +\tstatic std::optional toColorSpace(const T &v4l2Format);\n> +\n> +\ttemplate\n> +\tstatic int fromColorSpace(const std::optional &colorSpace, T &v4l2Format);\n> +\n> protected:\n> \tV4L2Device(const std::string &deviceNode);\n> \t~V4L2Device();\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 39f36009..cd8a5b1b 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -16,6 +16,8 @@\n> #include \n> #include \n>\n> +#include \n> +\n> #include \n> #include \n> #include \n> @@ -728,4 +730,194 @@ void V4L2Device::eventAvailable()\n> \tframeStart.emit(event.u.frame_sync.frame_sequence);\n> }\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_SMPTE170M, ColorSpace::Smpte170m },\n> +\t{ V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },\n> +\t{ V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },\n> +};\n> +\n> +static const std::map v4l2ToYcbcrEncoding = {\n> +\t{ V4L2_YCBCR_ENC_601, ColorSpace::YcbcrEncoding::Rec601 },\n> +\t{ V4L2_YCBCR_ENC_709, ColorSpace::YcbcrEncoding::Rec709 },\n> +\t{ V4L2_YCBCR_ENC_BT2020, ColorSpace::YcbcrEncoding::Rec2020 },\n> +};\n> +\n> +static const std::map v4l2ToTransferFunction = {\n> +\t{ V4L2_XFER_FUNC_NONE, ColorSpace::TransferFunction::Linear },\n> +\t{ V4L2_XFER_FUNC_SRGB, ColorSpace::TransferFunction::Srgb },\n> +\t{ V4L2_XFER_FUNC_709, ColorSpace::TransferFunction::Rec709 },\n> +};\n> +\n> +static const std::map v4l2ToRange = {\n> +\t{ V4L2_QUANTIZATION_FULL_RANGE, ColorSpace::Range::Full },\n> +\t{ V4L2_QUANTIZATION_LIM_RANGE, ColorSpace::Range::Limited },\n> +};\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::Smpte170m, V4L2_COLORSPACE_SMPTE170M },\n> +\t{ ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },\n> +\t{ ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },\n> +};\n> +\n> +static const std::map primariesToV4l2 = {\n> +\t{ ColorSpace::Primaries::Raw, V4L2_COLORSPACE_RAW },\n> +\t{ ColorSpace::Primaries::Smpte170m, V4L2_COLORSPACE_SMPTE170M },\n> +\t{ ColorSpace::Primaries::Rec709, V4L2_COLORSPACE_REC709 },\n> +\t{ ColorSpace::Primaries::Rec2020, V4L2_COLORSPACE_BT2020 },\n> +};\n> +\n> +static const std::map ycbcrEncodingToV4l2 = {\n> +\t{ ColorSpace::YcbcrEncoding::Rec601, V4L2_YCBCR_ENC_601 },\n> +\t{ ColorSpace::YcbcrEncoding::Rec709, V4L2_YCBCR_ENC_709 },\n> +\t{ ColorSpace::YcbcrEncoding::Rec2020, V4L2_YCBCR_ENC_BT2020 },\n> +};\n> +\n> +static const std::map transferFunctionToV4l2 = {\n> +\t{ ColorSpace::TransferFunction::Linear, V4L2_XFER_FUNC_NONE },\n> +\t{ ColorSpace::TransferFunction::Srgb, V4L2_XFER_FUNC_SRGB },\n> +\t{ ColorSpace::TransferFunction::Rec709, V4L2_XFER_FUNC_709 },\n> +};\n> +\n> +static const std::map rangeToV4l2 = {\n> +\t{ ColorSpace::Range::Full, V4L2_QUANTIZATION_FULL_RANGE },\n> +\t{ ColorSpace::Range::Limited, V4L2_QUANTIZATION_LIM_RANGE },\n> +};\n> +\n> +/**\n> + * \\brief Convert the color space fields in a V4L2 format to a ColorSpace\n> + * \\param[in] v4l2Format A V4L2 format containing color space information\n> + *\n> + * The colorspace, ycbcr_enc, xfer_func and quantization fields within a\n> + * V4L2 format structure are converted to a corresponding ColorSpace.\n> + *\n> + * If any V4L2 fields are not recognised then we return an \"unset\"\n> + * color space.\n> + *\n> + * \\return The ColorSpace corresponding to the input V4L2 format\n> + * \\retval std::nullopt One or more V4L2 color space fields were not recognised\n> + */\n> +template\n> +std::optional V4L2Device::toColorSpace(const T &v4l2Format)\n> +{\n> +\tauto itColor = v4l2ToColorSpace.find(v4l2Format.colorspace);\n> +\tif (itColor == v4l2ToColorSpace.end())\n> +\t\treturn std::nullopt;\n> +\n> +\t/* This sets all the color space fields to the correct \"default\" values. */\n> +\tColorSpace colorSpace = itColor->second;\n> +\n> +\tif (v4l2Format.ycbcr_enc != V4L2_YCBCR_ENC_DEFAULT) {\n> +\t\tauto itYcbcrEncoding = v4l2ToYcbcrEncoding.find(v4l2Format.ycbcr_enc);\n> +\t\tif (itYcbcrEncoding == v4l2ToYcbcrEncoding.end())\n> +\t\t\treturn std::nullopt;\n> +\n> +\t\tcolorSpace.ycbcrEncoding = itYcbcrEncoding->second;\n> +\t}\n> +\n> +\tif (v4l2Format.xfer_func != V4L2_XFER_FUNC_DEFAULT) {\n> +\t\tauto itTransfer = v4l2ToTransferFunction.find(v4l2Format.xfer_func);\n> +\t\tif (itTransfer == v4l2ToTransferFunction.end())\n> +\t\t\treturn std::nullopt;\n> +\n> +\t\tcolorSpace.transferFunction = itTransfer->second;\n> +\t}\n> +\n> +\tif (v4l2Format.quantization != V4L2_QUANTIZATION_DEFAULT) {\n> +\t\tauto itRange = v4l2ToRange.find(v4l2Format.quantization);\n> +\t\tif (itRange == v4l2ToRange.end())\n> +\t\t\treturn std::nullopt;\n> +\n> +\t\tcolorSpace.range = itRange->second;\n> +\t}\n> +\n> +\treturn colorSpace;\n> +}\n> +\n> +template std::optional V4L2Device::toColorSpace(const struct v4l2_pix_format &);\n> +template std::optional V4L2Device::toColorSpace(const struct v4l2_pix_format_mplane &);\n> +template std::optional V4L2Device::toColorSpace(const struct v4l2_mbus_framefmt &);\n> +\n> +/**\n> + * \\brief Fill in the color space fields of a V4L2 format from a ColorSpace\n> + * \\param[in] colorSpace The ColorSpace to be converted\n> + * \\param[out] v4l2Format A V4L2 format containing color space information\n> + *\n> + * The colorspace, ycbcr_enc, xfer_func and quantization fields within a\n> + * V4L2 format structure are filled in from a corresponding ColorSpace.\n> + *\n> + * An error is returned if any of the V4L2 fields do not support the\n> + * value given in the ColorSpace. Such fields are set to the V4L2\n> + * \"default\" values, but all other fields are still filled in where\n> + * possible.\n> + *\n> + * If the color space is completely unset, \"default\" V4L2 values are used\n> + * everywhere, so a driver would then choose its preferred color space.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + * \\retval -EINVAL The ColorSpace does not have a representation using V4L2 enums\n> + */\n> +template\n> +int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v4l2Format)\n> +{\n> +\tv4l2Format.colorspace = V4L2_COLORSPACE_DEFAULT;\n> +\tv4l2Format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;\n> +\tv4l2Format.xfer_func = V4L2_XFER_FUNC_DEFAULT;\n> +\tv4l2Format.quantization = V4L2_QUANTIZATION_DEFAULT;\n> +\n> +\tif (!colorSpace)\n> +\t\treturn 0;\n> +\n> +\tauto itColor = std::find_if(colorSpaceToV4l2.begin(), colorSpaceToV4l2.end(),\n> +\t\t\t\t [&colorSpace](const auto &item) {\n> +\t\t\t\t\t return colorSpace == item.first;\n> +\t\t\t\t });\n> +\tif (itColor != colorSpaceToV4l2.end()) {\n> +\t\tv4l2Format.colorspace = itColor->second;\n> +\t\t/* Leaving all the other fields as \"default\" should be fine. */\n> +\t\treturn 0;\n> +\t}\n> +\n> +\t/*\n> +\t * If the colorSpace doesn't precisely match a standard color space,\n> +\t * then we must choose a V4L2 colorspace with matching primaries.\n> +\t */\n> +\tint ret = 0;\n> +\n> +\tauto itPrimaries = primariesToV4l2.find(colorSpace->primaries);\n> +\tif (itPrimaries != primariesToV4l2.end())\n> +\t\tv4l2Format.colorspace = itPrimaries->second;\n> +\telse\n> +\t\tret = -EINVAL;\n> +\n> +\tauto itYcbcrEncoding = ycbcrEncodingToV4l2.find(colorSpace->ycbcrEncoding);\n> +\tif (itYcbcrEncoding != ycbcrEncodingToV4l2.end())\n> +\t\tv4l2Format.ycbcr_enc = itYcbcrEncoding->second;\n> +\telse\n> +\t\tret = -EINVAL;\n> +\n> +\tauto itTransfer = transferFunctionToV4l2.find(colorSpace->transferFunction);\n> +\tif (itTransfer != transferFunctionToV4l2.end())\n> +\t\tv4l2Format.xfer_func = itTransfer->second;\n> +\telse\n> +\t\tret = -EINVAL;\n> +\n> +\tauto itRange = rangeToV4l2.find(colorSpace->range);\n> +\tif (itRange != rangeToV4l2.end())\n> +\t\tv4l2Format.quantization = itRange->second;\n> +\telse\n> +\t\tret = -EINVAL;\n> +\n> +\treturn ret;\n> +}\n> +\n> +template int V4L2Device::fromColorSpace(const std::optional &, struct v4l2_pix_format &);\n> +template int V4L2Device::fromColorSpace(const std::optional &, struct v4l2_pix_format_mplane &);\n> +template int V4L2Device::fromColorSpace(const std::optional &, struct v4l2_mbus_framefmt &);\n> +\n> } /* namespace libcamera */\n> --\n> 2.20.1\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 17CD1BF415\n\tfor ;\n\tMon, 6 Dec 2021 11:20:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB66A60876;\n\tMon, 6 Dec 2021 12:20:39 +0100 (CET)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0F1F260725\n\tfor ;\n\tMon, 6 Dec 2021 12:20:39 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 9B200100005;\n\tMon, 6 Dec 2021 11:20:36 +0000 (UTC)"],"Date":"Mon, 6 Dec 2021 12:21:29 +0100","From":"Jacopo Mondi ","To":"David Plowman ","Message-ID":"<20211206112129.khwcip3ohes5pxpf@uno.localdomain>","References":"<20211206105032.13876-1-david.plowman@raspberrypi.com>\n\t<20211206105032.13876-4-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211206105032.13876-4-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v9 3/8] libcamera: Convert between\n\tColorSpace class and V4L2 formats","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","Cc":"tfiga@google.com, libcamera-devel@lists.libcamera.org,\n\thverkuil-cisco@xs4all.nl","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21628,"web_url":"https://patchwork.libcamera.org/comment/21628/","msgid":"","date":"2021-12-07T13:24:01","subject":"Re: [libcamera-devel] [PATCH v9 3/8] libcamera: Convert between\n\tColorSpace class and V4L2 formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nThe subject line should read\n\nlibcamera: video_device: Convert between ColorSpace class and V4L2 formats\n\nOn Mon, Dec 06, 2021 at 10:50:26AM +0000, David Plowman wrote:\n> Add functions to the V4L2Device class to convert to and from\n> libcamera ColorSpace.\n> \n> These methods are added to the base V4L2Device class so that they can\n\ns/methods/functions/ (same below)\n\n> be shared both by the video device class and subdevices.\n> \n> With the ColorSpace class, the color space and related other fields\n> are stored together, corresponding to a number of fields in the\n> various different V4L2 format structures. Template methods are\n> therefore a convenient implementation, and we must explicitly\n> instantiate the templates that will be needed.\n> \n> Note that unset color spaces are converted to requests for the\n> device's \"default\" color space.\n> \n> Signed-off-by: David Plowman \n> ---\n> include/libcamera/internal/v4l2_device.h | 7 +\n> src/libcamera/v4l2_device.cpp | 192 +++++++++++++++++++++++\n> 2 files changed, 199 insertions(+)\n> \n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index 8886b750..38cf93a6 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -18,6 +18,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -45,6 +46,12 @@ public:\n> \n> \tvoid updateControlInfo();\n> \n> +\ttemplate\n> +\tstatic std::optional toColorSpace(const T &v4l2Format);\n> +\n> +\ttemplate\n> +\tstatic int fromColorSpace(const std::optional &colorSpace, T &v4l2Format);\n> +\n> protected:\n> \tV4L2Device(const std::string &deviceNode);\n> \t~V4L2Device();\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 39f36009..cd8a5b1b 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -16,6 +16,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \n> #include \n> #include \n> @@ -728,4 +730,194 @@ void V4L2Device::eventAvailable()\n> \tframeStart.emit(event.u.frame_sync.frame_sequence);\n> }\n> \n> +static const std::map v4l2ToColorSpace = {\n\nYou should include .\n\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_SMPTE170M, ColorSpace::Smpte170m },\n> +\t{ V4L2_COLORSPACE_REC709, ColorSpace::Rec709 },\n> +\t{ V4L2_COLORSPACE_BT2020, ColorSpace::Rec2020 },\n> +};\n> +\n> +static const std::map v4l2ToYcbcrEncoding = {\n> +\t{ V4L2_YCBCR_ENC_601, ColorSpace::YcbcrEncoding::Rec601 },\n> +\t{ V4L2_YCBCR_ENC_709, ColorSpace::YcbcrEncoding::Rec709 },\n> +\t{ V4L2_YCBCR_ENC_BT2020, ColorSpace::YcbcrEncoding::Rec2020 },\n> +};\n> +\n> +static const std::map v4l2ToTransferFunction = {\n> +\t{ V4L2_XFER_FUNC_NONE, ColorSpace::TransferFunction::Linear },\n> +\t{ V4L2_XFER_FUNC_SRGB, ColorSpace::TransferFunction::Srgb },\n> +\t{ V4L2_XFER_FUNC_709, ColorSpace::TransferFunction::Rec709 },\n> +};\n> +\n> +static const std::map v4l2ToRange = {\n> +\t{ V4L2_QUANTIZATION_FULL_RANGE, ColorSpace::Range::Full },\n> +\t{ V4L2_QUANTIZATION_LIM_RANGE, ColorSpace::Range::Limited },\n> +};\n> +\n> +static const std::vector> colorSpaceToV4l2 = {\n\nAnd should include too.\n\nColorSpace has a constexpr constructor, could this be a map ? If it\ncan't, no big deal.\n\n> +\t{ ColorSpace::Raw, V4L2_COLORSPACE_RAW },\n> +\t{ ColorSpace::Jpeg, V4L2_COLORSPACE_JPEG },\n> +\t{ ColorSpace::Srgb, V4L2_COLORSPACE_SRGB },\n> +\t{ ColorSpace::Smpte170m, V4L2_COLORSPACE_SMPTE170M },\n> +\t{ ColorSpace::Rec709, V4L2_COLORSPACE_REC709 },\n> +\t{ ColorSpace::Rec2020, V4L2_COLORSPACE_BT2020 },\n> +};\n> +\n> +static const std::map primariesToV4l2 = {\n> +\t{ ColorSpace::Primaries::Raw, V4L2_COLORSPACE_RAW },\n> +\t{ ColorSpace::Primaries::Smpte170m, V4L2_COLORSPACE_SMPTE170M },\n> +\t{ ColorSpace::Primaries::Rec709, V4L2_COLORSPACE_REC709 },\n> +\t{ ColorSpace::Primaries::Rec2020, V4L2_COLORSPACE_BT2020 },\n> +};\n> +\n> +static const std::map ycbcrEncodingToV4l2 = {\n> +\t{ ColorSpace::YcbcrEncoding::Rec601, V4L2_YCBCR_ENC_601 },\n> +\t{ ColorSpace::YcbcrEncoding::Rec709, V4L2_YCBCR_ENC_709 },\n> +\t{ ColorSpace::YcbcrEncoding::Rec2020, V4L2_YCBCR_ENC_BT2020 },\n> +};\n> +\n> +static const std::map transferFunctionToV4l2 = {\n> +\t{ ColorSpace::TransferFunction::Linear, V4L2_XFER_FUNC_NONE },\n> +\t{ ColorSpace::TransferFunction::Srgb, V4L2_XFER_FUNC_SRGB },\n> +\t{ ColorSpace::TransferFunction::Rec709, V4L2_XFER_FUNC_709 },\n> +};\n> +\n> +static const std::map rangeToV4l2 = {\n> +\t{ ColorSpace::Range::Full, V4L2_QUANTIZATION_FULL_RANGE },\n> +\t{ ColorSpace::Range::Limited, V4L2_QUANTIZATION_LIM_RANGE },\n> +};\n\nA std::bidir_map<> could be nice :-)\n\n> +\n> +/**\n> + * \\brief Convert the color space fields in a V4L2 format to a ColorSpace\n> + * \\param[in] v4l2Format A V4L2 format containing color space information\n> + *\n> + * The colorspace, ycbcr_enc, xfer_func and quantization fields within a\n> + * V4L2 format structure are converted to a corresponding ColorSpace.\n> + *\n> + * If any V4L2 fields are not recognised then we return an \"unset\"\n> + * color space.\n> + *\n> + * \\return The ColorSpace corresponding to the input V4L2 format\n> + * \\retval std::nullopt One or more V4L2 color space fields were not recognised\n> + */\n> +template\n> +std::optional V4L2Device::toColorSpace(const T &v4l2Format)\n> +{\n> +\tauto itColor = v4l2ToColorSpace.find(v4l2Format.colorspace);\n> +\tif (itColor == v4l2ToColorSpace.end())\n> +\t\treturn std::nullopt;\n> +\n> +\t/* This sets all the color space fields to the correct \"default\" values. */\n> +\tColorSpace colorSpace = itColor->second;\n> +\n> +\tif (v4l2Format.ycbcr_enc != V4L2_YCBCR_ENC_DEFAULT) {\n> +\t\tauto itYcbcrEncoding = v4l2ToYcbcrEncoding.find(v4l2Format.ycbcr_enc);\n> +\t\tif (itYcbcrEncoding == v4l2ToYcbcrEncoding.end())\n> +\t\t\treturn std::nullopt;\n> +\n> +\t\tcolorSpace.ycbcrEncoding = itYcbcrEncoding->second;\n> +\t}\n> +\n> +\tif (v4l2Format.xfer_func != V4L2_XFER_FUNC_DEFAULT) {\n> +\t\tauto itTransfer = v4l2ToTransferFunction.find(v4l2Format.xfer_func);\n> +\t\tif (itTransfer == v4l2ToTransferFunction.end())\n> +\t\t\treturn std::nullopt;\n> +\n> +\t\tcolorSpace.transferFunction = itTransfer->second;\n> +\t}\n> +\n> +\tif (v4l2Format.quantization != V4L2_QUANTIZATION_DEFAULT) {\n> +\t\tauto itRange = v4l2ToRange.find(v4l2Format.quantization);\n> +\t\tif (itRange == v4l2ToRange.end())\n> +\t\t\treturn std::nullopt;\n> +\n> +\t\tcolorSpace.range = itRange->second;\n> +\t}\n> +\n> +\treturn colorSpace;\n> +}\n> +\n> +template std::optional V4L2Device::toColorSpace(const struct v4l2_pix_format &);\n> +template std::optional V4L2Device::toColorSpace(const struct v4l2_pix_format_mplane &);\n> +template std::optional V4L2Device::toColorSpace(const struct v4l2_mbus_framefmt &);\n\nIt would have been nice if V4L2 had grouped the colour space fields in a\nstructure. Oh well :-)\n\n> +\n> +/**\n> + * \\brief Fill in the color space fields of a V4L2 format from a ColorSpace\n> + * \\param[in] colorSpace The ColorSpace to be converted\n> + * \\param[out] v4l2Format A V4L2 format containing color space information\n> + *\n> + * The colorspace, ycbcr_enc, xfer_func and quantization fields within a\n> + * V4L2 format structure are filled in from a corresponding ColorSpace.\n> + *\n> + * An error is returned if any of the V4L2 fields do not support the\n> + * value given in the ColorSpace. Such fields are set to the V4L2\n> + * \"default\" values, but all other fields are still filled in where\n> + * possible.\n> + *\n> + * If the color space is completely unset, \"default\" V4L2 values are used\n> + * everywhere, so a driver would then choose its preferred color space.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + * \\retval -EINVAL The ColorSpace does not have a representation using V4L2 enums\n\nThis can never happen, at least for now, until we add support for colour\nspaces not supported in V4L2, right ? I don't really mind keeping the\nerror checking, but if it can indeed never happen, and if there's no\nforeseen extension to colour spaces support in libcamera that wouldn't\nhave V4L2 equivalents, we could also drop it.\n\n> + */\n> +template\n> +int V4L2Device::fromColorSpace(const std::optional &colorSpace, T &v4l2Format)\n\nStill not sure about what the optional is for, I'll see in the remaining\npatches.\n\nReviewed-by: Laurent Pinchart \n\n> +{\n> +\tv4l2Format.colorspace = V4L2_COLORSPACE_DEFAULT;\n> +\tv4l2Format.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;\n> +\tv4l2Format.xfer_func = V4L2_XFER_FUNC_DEFAULT;\n> +\tv4l2Format.quantization = V4L2_QUANTIZATION_DEFAULT;\n> +\n> +\tif (!colorSpace)\n> +\t\treturn 0;\n> +\n> +\tauto itColor = std::find_if(colorSpaceToV4l2.begin(), colorSpaceToV4l2.end(),\n> +\t\t\t\t [&colorSpace](const auto &item) {\n> +\t\t\t\t\t return colorSpace == item.first;\n> +\t\t\t\t });\n> +\tif (itColor != colorSpaceToV4l2.end()) {\n> +\t\tv4l2Format.colorspace = itColor->second;\n> +\t\t/* Leaving all the other fields as \"default\" should be fine. */\n> +\t\treturn 0;\n> +\t}\n> +\n> +\t/*\n> +\t * If the colorSpace doesn't precisely match a standard color space,\n> +\t * then we must choose a V4L2 colorspace with matching primaries.\n> +\t */\n> +\tint ret = 0;\n> +\n> +\tauto itPrimaries = primariesToV4l2.find(colorSpace->primaries);\n> +\tif (itPrimaries != primariesToV4l2.end())\n> +\t\tv4l2Format.colorspace = itPrimaries->second;\n> +\telse\n> +\t\tret = -EINVAL;\n> +\n> +\tauto itYcbcrEncoding = ycbcrEncodingToV4l2.find(colorSpace->ycbcrEncoding);\n> +\tif (itYcbcrEncoding != ycbcrEncodingToV4l2.end())\n> +\t\tv4l2Format.ycbcr_enc = itYcbcrEncoding->second;\n> +\telse\n> +\t\tret = -EINVAL;\n> +\n> +\tauto itTransfer = transferFunctionToV4l2.find(colorSpace->transferFunction);\n> +\tif (itTransfer != transferFunctionToV4l2.end())\n> +\t\tv4l2Format.xfer_func = itTransfer->second;\n> +\telse\n> +\t\tret = -EINVAL;\n> +\n> +\tauto itRange = rangeToV4l2.find(colorSpace->range);\n> +\tif (itRange != rangeToV4l2.end())\n> +\t\tv4l2Format.quantization = itRange->second;\n> +\telse\n> +\t\tret = -EINVAL;\n> +\n> +\treturn ret;\n> +}\n> +\n> +template int V4L2Device::fromColorSpace(const std::optional &, struct v4l2_pix_format &);\n> +template int V4L2Device::fromColorSpace(const std::optional &, struct v4l2_pix_format_mplane &);\n> +template int V4L2Device::fromColorSpace(const std::optional &, struct v4l2_mbus_framefmt &);\n> +\n> } /* namespace libcamera */","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 40722BDB13\n\tfor ;\n\tTue, 7 Dec 2021 13:24:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 75031605C4;\n\tTue, 7 Dec 2021 14:24:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9188760592\n\tfor ;\n\tTue, 7 Dec 2021 14:24:30 +0100 (CET)","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 F0FCB556;\n\tTue, 7 Dec 2021 14:24:29 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"J75fg3k9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638883470;\n\tbh=8fE4LmPa7P9F+kgsS33igv2spbH6Ouyj5KJ/l/x4XZM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=J75fg3k9slBHRnUZTSzOYxjh/lwfeZZ2vaclsFjxkQ+pMfcwkar2lfcInP1kut3c6\n\tb1Ij986RO6RfUqOxGfO77ZQv+JxPn6fcl/wY7lrS2YTuRp83EVxfeQTzBtvX59QvWP\n\t6OQYVDjf14gLUJsGQ1YgEPb4Hg8HxMiqn77dl5XA=","Date":"Tue, 7 Dec 2021 15:24:01 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20211206105032.13876-1-david.plowman@raspberrypi.com>\n\t<20211206105032.13876-4-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211206105032.13876-4-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v9 3/8] libcamera: Convert between\n\tColorSpace class and V4L2 formats","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","Cc":"tfiga@google.com, libcamera-devel@lists.libcamera.org,\n\thverkuil-cisco@xs4all.nl","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]