[{"id":21763,"web_url":"https://patchwork.libcamera.org/comment/21763/","msgid":"","date":"2021-12-10T22:50:39","subject":"Re: [libcamera-devel] [PATCH v12 3/8] libcamera: video_device:\n\tConvert between ColorSpace 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\nOn Fri, Dec 10, 2021 at 02:44:19PM +0000, David Plowman wrote:\n> Add functions to the V4L2Device class to convert to and from\n> libcamera ColorSpace.\n> \n> These functions 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 functions 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> Reviewed-by: Jacopo Mondi \n> Reviewed-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/v4l2_device.h | 8 +\n> src/libcamera/v4l2_device.cpp | 210 +++++++++++++++++++++++\n> 2 files changed, 218 insertions(+)\n> \n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index 8886b750..a52a5f2c 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -9,6 +9,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -18,6 +19,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -56,6 +58,12 @@ protected:\n> \n> \tint fd() const { return fd_.get(); }\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> private:\n> \tstatic ControlType v4l2CtrlType(uint32_t ctrlType);\n> \tstatic std::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl &ctrl);\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 39f36009..25ba8c95 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -10,11 +10,15 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> #include \n> #include \n> +#include \n> +\n> +#include \n> \n> #include \n> #include \n> @@ -728,4 +732,210 @@ 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 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 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 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 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 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 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.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.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.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.xfer_func = V4L2_XFER_FUNC_DEFAULT;\n> +\tv4l2Format.ycbcr_enc = V4L2_YCBCR_ENC_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\nThe libcamera coding style is\n\n\tif (itPrimaries != primariesToV4l2.end()) {\n\t\tv4l2Format.colorspace = itPrimaries->second;\n\t} else {\n\nThere doesn't seem to be any clang-format option for this though :-(\n\nI'll fix it when applying.\n\n> +\t\tlibcamera::LOG(V4L2, Warning)\n> +\t\t\t<< \"Unrecognised primaries in \"\n> +\t\t\t<< ColorSpace::toString(colorSpace);\n> +\t\tret = -EINVAL;\n> +\t}\n> +\n> +\tauto itTransfer = transferFunctionToV4l2.find(colorSpace->transferFunction);\n> +\tif (itTransfer != transferFunctionToV4l2.end())\n> +\t\tv4l2Format.xfer_func = itTransfer->second;\n> +\telse {\n> +\t\tlibcamera::LOG(V4L2, Warning)\n> +\t\t\t<< \"Unrecognised transfer function in \"\n> +\t\t\t<< ColorSpace::toString(colorSpace);\n> +\t\tret = -EINVAL;\n> +\t}\n> +\n> +\tauto itYcbcrEncoding = ycbcrEncodingToV4l2.find(colorSpace->ycbcrEncoding);\n> +\tif (itYcbcrEncoding != ycbcrEncodingToV4l2.end())\n> +\t\tv4l2Format.ycbcr_enc = itYcbcrEncoding->second;\n> +\telse {\n> +\t\tlibcamera::LOG(V4L2, Warning)\n> +\t\t\t<< \"Unrecognised YCbCr encoding in \"\n> +\t\t\t<< ColorSpace::toString(colorSpace);\n> +\t\tret = -EINVAL;\n> +\t}\n> +\n> +\tauto itRange = rangeToV4l2.find(colorSpace->range);\n> +\tif (itRange != rangeToV4l2.end())\n> +\t\tv4l2Format.quantization = itRange->second;\n> +\telse {\n> +\t\tlibcamera::LOG(V4L2, Warning)\n> +\t\t\t<< \"Unrecognised quantization in \"\n> +\t\t\t<< ColorSpace::toString(colorSpace);\n> +\t\tret = -EINVAL;\n> +\t}\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 D1605BF415\n\tfor ;\n\tFri, 10 Dec 2021 22:51:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 171516087E;\n\tFri, 10 Dec 2021 23:51:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9767660868\n\tfor ;\n\tFri, 10 Dec 2021 23:51:09 +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 DEB85F84;\n\tFri, 10 Dec 2021 23:51:08 +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=\"JXxunVbY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1639176669;\n\tbh=vvOgpP07o7TMZJ8OcesxrY4W05ptUGmTa+4eO9ds2MI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JXxunVbYDeV2loZswAnUsmgBeAiNG7lcU1EmKP6AfKox3wxY0S6ivU8d3wSs4BSnL\n\tk8bLRI7PwyrSFx3RGL7ZYTUjeCD9j2PIF4OHfi/Xevp2j5d3ORN8ZpxGQIPRwNsU0t\n\tuUn2tRvH1FXSgcPAmvDDfe8zCkjItlAQQqGXE1eg=","Date":"Sat, 11 Dec 2021 00:50:39 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20211210144424.14747-1-david.plowman@raspberrypi.com>\n\t<20211210144424.14747-4-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211210144424.14747-4-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v12 3/8] libcamera: video_device:\n\tConvert between ColorSpace 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":"Tomasz Figa , libcamera-devel@lists.libcamera.org,\n\tHans Verkuil ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]