[{"id":24330,"web_url":"https://patchwork.libcamera.org/comment/24330/","msgid":"","date":"2022-08-03T11:06:37","subject":"Re: [libcamera-devel] [PATCH v5 6/7] libcamera: v4l2_videodevice:\n\tMatch formats supported by the device","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Aug 03, 2022 at 12:38:48PM +0200, Jacopo Mondi wrote:\n> Now that V4L2PixelFormat::fromPixelFormat() returns a list of formats\n> to chose from, select the one supported by the video device by matching\n> against the list of supported pixel formats.\n> \n> The first format found to match one of the device supported ones is\n> returned.\n> \n> As the list of pixel formats supported by the video device does not\n> change at run-time, cache it at device open() time. To maximize the\n> lookup efficiency store the list of supported V4L2PixelFormat in an\n> std::unordered_set<> which requires a specialization of\n> std::hash to be injected in the std namespace.\n\nWould you mind including the patch I sent to add\nstd::hash in this series and rebasing this on top ?\nIt's a separate function change, and if we later want to hash\nPixelFormat too, it will show how to do so (with a commit message\ntemplate) without unrelated changes.\n\nThe commit message here should then drop \" ... which requires \" unwards,\nand you can also drop my SoB line.\n\n> Signed-off-by: Jacopo Mondi \n> Signed-off-by: Laurent Pinchart \n> Reviewed-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/v4l2_pixelformat.h | 13 +++++\n> include/libcamera/internal/v4l2_videodevice.h | 4 ++\n> src/libcamera/v4l2_videodevice.cpp | 57 +++++++++++++++----\n> 3 files changed, 62 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h\n> index d5400f90a67e..34d283db44f4 100644\n> --- a/include/libcamera/internal/v4l2_pixelformat.h\n> +++ b/include/libcamera/internal/v4l2_pixelformat.h\n> @@ -8,6 +8,7 @@\n> \n> #pragma once\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -55,3 +56,15 @@ private:\n> std::ostream &operator<<(std::ostream &out, const V4L2PixelFormat &f);\n> \n> } /* namespace libcamera */\n> +\n> +namespace std {\n> +\n> +template<>\n> +struct hash {\n> +\tsize_t operator()(libcamera::V4L2PixelFormat const &format) const noexcept\n> +\t{\n> +\t\treturn format.fourcc();\n> +\t}\n> +};\n> +\n> +} /* namespace std */\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 29fa0bbaf670..ed98a284de16 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -242,6 +243,8 @@ private:\n> \t\tStopped,\n> \t};\n> \n> +\tint initFormats();\n> +\n> \tint getFormatMeta(V4L2DeviceFormat *format);\n> \tint trySetFormatMeta(V4L2DeviceFormat *format, bool set);\n> \n> @@ -268,6 +271,7 @@ private:\n> \tV4L2Capability caps_;\n> \tV4L2DeviceFormat format_;\n> \tconst PixelFormatInfo *formatInfo_;\n> +\tstd::unordered_set pixelFormats_;\n> \n> \tenum v4l2_buf_type bufferType_;\n> \tenum v4l2_memory memoryType_;\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 2ca22f485d45..b80ee1cdbcca 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -633,13 +633,9 @@ int V4L2VideoDevice::open()\n> \t\t<< \"Opened device \" << caps_.bus_info() << \": \"\n> \t\t<< caps_.driver() << \": \" << caps_.card();\n> \n> -\tret = getFormat(&format_);\n> -\tif (ret) {\n> -\t\tLOG(V4L2, Error) << \"Failed to get format\";\n> +\tret = initFormats();\n> +\tif (ret)\n> \t\treturn ret;\n> -\t}\n> -\n> -\tformatInfo_ = &PixelFormatInfo::info(format_.fourcc);\n> \n> \treturn 0;\n> }\n> @@ -726,7 +722,24 @@ int V4L2VideoDevice::open(SharedFD handle, enum v4l2_buf_type type)\n> \t\t<< \"Opened device \" << caps_.bus_info() << \": \"\n> \t\t<< caps_.driver() << \": \" << caps_.card();\n> \n> -\tret = getFormat(&format_);\n> +\tret = initFormats();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int V4L2VideoDevice::initFormats()\n> +{\n> +\tconst std::vector &deviceFormats = enumPixelformats(0);\n> +\tif (deviceFormats.empty()) {\n> +\t\tLOG(V4L2, Error) << \"Failed to initialize device formats\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tpixelFormats_ = { deviceFormats.begin(), deviceFormats.end() };\n> +\n> +\tint ret = getFormat(&format_);\n> \tif (ret) {\n> \t\tLOG(V4L2, Error) << \"Failed to get format\";\n> \t\treturn ret;\n> @@ -1990,17 +2003,37 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media,\n> }\n> \n> /**\n> - * \\brief Convert \\a PixelFormat to its corresponding V4L2 FourCC\n> + * \\brief Convert \\a PixelFormat to a V4L2PixelFormat supported by the device\n> * \\param[in] pixelFormat The PixelFormat to convert\n> *\n> - * The V4L2 format variant the function returns the contiguous version\n> - * unconditionally.\n> + * Convert \\a pixelformat to a V4L2 FourCC that is known to be supported by\n> + * the video device.\n> *\n> - * \\return The V4L2_PIX_FMT_* pixel format code corresponding to \\a pixelFormat\n> + * A V4L2VideoDevice may support different V4L2 pixel formats that map the same\n> + * PixelFormat. This is the case of the contiguous and non-contiguous variants\n> + * of multiplanar formats, and with the V4L2 MJPEG and JPEG pixel formats.\n> + * Converting a PixelFormat to a V4L2PixelFormat may thus have multiple answers.\n> + *\n> + * This function converts the \\a pixelFormat using the list of V4L2 pixel\n> + * formats that the V4L2VideoDevice supports. This guarantees that the returned\n> + * V4L2PixelFormat will be valid for the device. If multiple matches are still\n> + * possible, contiguous variants are preferred. If the \\a pixelFormat is not\n> + * supported by the device, the function returns an invalid V4L2PixelFormat.\n> + *\n> + * \\return The V4L2PixelFormat corresponding to \\a pixelFormat if supported by\n> + * the device, or an invalid V4L2PixelFormat otherwise\n> */\n> V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat) const\n> {\n> -\treturn V4L2PixelFormat::fromPixelFormat(pixelFormat)[0];\n> +\tconst std::vector &v4l2PixelFormats =\n> +\t\tV4L2PixelFormat::fromPixelFormat(pixelFormat);\n> +\n> +\tfor (const V4L2PixelFormat &v4l2Format : v4l2PixelFormats) {\n> +\t\tif (pixelFormats_.count(v4l2Format))\n> +\t\t\treturn v4l2Format;\n> +\t}\n> +\n> +\treturn {};\n> }\n> \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 A5AA8BE173\n\tfor ;\n\tWed, 3 Aug 2022 11:06:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2A90B61FAE;\n\tWed, 3 Aug 2022 13:06:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 19C9B603E6\n\tfor ;\n\tWed, 3 Aug 2022 13:06:44 +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 94CB38B;\n\tWed, 3 Aug 2022 13:06:43 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659524806;\n\tbh=iDh2xvFO3x0sIcJlxRNersq/l/08AC83I8QhDgJrsKg=;\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=DYjqWgh3YUoKOU2rHCo8FCnoQUrfMKz927OqqUsXAAMnTKeg91xRam6CXuj5gIpcy\n\t3jCzWwYJra7u9JnIw/uSfz1r0uA5KEbZQBMdeM3ZPtA6xp/P6wmwnhdXKx05YOVBe1\n\tmRawVKYCqqIvjV9enSdLMUaLm4EyQDfSvDC1upXTSlrZpSqtouVjT3k14GoCBRQESN\n\tv4ze9UkWuJSw++ldI0fdrZg8NSgidops50YnrpNgDs3ps9rFCs6hKxGlxAXp5Er6ls\n\tBm0I/jZLlma9qMbSMUMFDXlDZSSsvbpboeqLUul1BEAJzBunMT77P9zemd/K/xMPJT\n\tQq+BdEB1rpJDg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659524803;\n\tbh=iDh2xvFO3x0sIcJlxRNersq/l/08AC83I8QhDgJrsKg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FlZPXj9tkegoT4FRTItNgZuOv4EHhApuZdwgVLCQB5J0MSSdZBf3PnuUnUmIO83bT\n\tlc1xI/VMyNSZBc+utpmNzZbfmfE+C9zeEwvVu+Ljoa/fIWxqu6FAuWLwn1c5gXSokT\n\tQrhGavrCJapzD7Jng8QhhUV3oTsFo8j4LBvNOha4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"FlZPXj9t\"; dkim-atps=neutral","Date":"Wed, 3 Aug 2022 14:06:37 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220803103849.26144-1-jacopo@jmondi.org>\n\t<20220803103849.26144-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220803103849.26144-7-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v5 6/7] libcamera: v4l2_videodevice:\n\tMatch formats supported by the device","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\" "}}]