[{"id":24250,"web_url":"https://patchwork.libcamera.org/comment/24250/","msgid":"","date":"2022-07-29T17:41:04","subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: v4l2_pixelformat:\n\tReturn the list of V4L2 formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Fri, Jul 29, 2022 at 06:00:12PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> Multiple V4L2 formats can be associated with a single PixelFormat.\n> Now that users of V4L2PixelFormat::fromPixelFormat() have been converted\n> to use V4L2VideoDevice::toV4L2PixelFormat(), return the full list of\n> V4L2 formats in order to prepare to match them against the ones\n> supported by the video device.\n> \n> The V4L2 compatibility layer, not having any video device to interact\n> with, are converted to use the first returned format by default.\n\ns/are converted/is converted/\ns/by default/unconditionally/\n\n> Signed-off-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/v4l2_pixelformat.h | 4 +++-\n> src/libcamera/v4l2_pixelformat.cpp | 14 +++++++-------\n> src/libcamera/v4l2_videodevice.cpp | 2 +-\n> src/v4l2/v4l2_camera_proxy.cpp | 6 +++---\n> 4 files changed, 14 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h\n> index ed94baf92795..a8aec29552e4 100644\n> --- a/include/libcamera/internal/v4l2_pixelformat.h\n> +++ b/include/libcamera/internal/v4l2_pixelformat.h\n> @@ -11,6 +11,7 @@\n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> \n> @@ -44,7 +45,8 @@ public:\n> \tconst char *description() const;\n> \n> \tPixelFormat toPixelFormat() const;\n> -\tstatic V4L2PixelFormat fromPixelFormat(const PixelFormat &pixelFormat);\n> +\tstatic const std::vector &\n> +\t\tfromPixelFormat(const PixelFormat &pixelFormat);\n\nI think we usually wrap lines as\n\n\tstatic const std::vector &\n\tfromPixelFormat(const PixelFormat &pixelFormat);\n\nThe function name isn't great anymore, as static from*() functions are\nexpected to act as constructors and return an instance of the class. I\ncan't think of a much better name right now though.\n\n> \n> private:\n> \tuint32_t fourcc_;\n> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> index 91a882baa75b..bca97d1e3b4f 100644\n> --- a/src/libcamera/v4l2_pixelformat.cpp\n> +++ b/src/libcamera/v4l2_pixelformat.cpp\n> @@ -302,23 +302,23 @@ PixelFormat V4L2PixelFormat::toPixelFormat() const\n> }\n> \n> /**\n> - * \\brief Convert \\a pixelFormat to its corresponding V4L2PixelFormat\n> + * \\brief Retrieve the list of V4L2PixelFormat associated with \\a pixelFormat\n> * \\param[in] pixelFormat The PixelFormat to convert\n> *\n> * Multiple V4L2 formats may exist for one PixelFormat as V4L2 defines separate\n> * 4CCs for contiguous and non-contiguous versions of the same image format.\n> *\n> - * Return the contiguous planes format version by default.\n> - *\n> - * \\return The V4L2PixelFormat corresponding to \\a pixelFormat\n> + * \\return The list of V4L2PixelFormat corresponding to \\a pixelFormat\n> */\n> -V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat)\n> +const std::vector &V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat)\n\nYou could wrap this line too.\n\nReviewed-by: Laurent Pinchart \n\n> {\n> +\tstatic const std::vector empty;\n> +\n> \tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> \tif (!info.isValid())\n> -\t\treturn V4L2PixelFormat();\n> +\t\treturn empty;\n> \n> -\treturn info.v4l2Formats[0];\n> +\treturn info.v4l2Formats;\n> }\n> \n> /**\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index f41afa06f460..2ca22f485d45 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -2000,7 +2000,7 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media,\n> */\n> V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat) const\n> {\n> -\treturn V4L2PixelFormat::fromPixelFormat(pixelFormat);\n> +\treturn V4L2PixelFormat::fromPixelFormat(pixelFormat)[0];\n> }\n> \n> /**\n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 26a227da6db2..55ff62cdb430 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -182,7 +182,7 @@ void V4L2CameraProxy::setFmtFromConfig(const StreamConfiguration &streamConfig)\n> \n> \tv4l2PixFormat_.width = size.width;\n> \tv4l2PixFormat_.height = size.height;\n> -\tv4l2PixFormat_.pixelformat = V4L2PixelFormat::fromPixelFormat(streamConfig.pixelFormat);\n> +\tv4l2PixFormat_.pixelformat = V4L2PixelFormat::fromPixelFormat(streamConfig.pixelFormat)[0];\n> \tv4l2PixFormat_.field = V4L2_FIELD_NONE;\n> \tv4l2PixFormat_.bytesperline = streamConfig.stride;\n> \tv4l2PixFormat_.sizeimage = streamConfig.frameSize;\n> @@ -290,7 +290,7 @@ int V4L2CameraProxy::vidioc_enum_fmt(V4L2CameraFile *file, struct v4l2_fmtdesc *\n> \t\treturn -EINVAL;\n> \n> \tPixelFormat format = streamConfig_.formats().pixelformats()[arg->index];\n> -\tV4L2PixelFormat v4l2Format = V4L2PixelFormat::fromPixelFormat(format);\n> +\tV4L2PixelFormat v4l2Format = V4L2PixelFormat::fromPixelFormat(format)[0];\n> \n> \targ->flags = format == formats::MJPEG ? V4L2_FMT_FLAG_COMPRESSED : 0;\n> \tutils::strlcpy(reinterpret_cast(arg->description),\n> @@ -333,7 +333,7 @@ int V4L2CameraProxy::tryFormat(struct v4l2_format *arg)\n> \n> \targ->fmt.pix.width = config.size.width;\n> \targ->fmt.pix.height = config.size.height;\n> -\targ->fmt.pix.pixelformat = V4L2PixelFormat::fromPixelFormat(config.pixelFormat);\n> +\targ->fmt.pix.pixelformat = V4L2PixelFormat::fromPixelFormat(config.pixelFormat)[0];\n> \targ->fmt.pix.field = V4L2_FIELD_NONE;\n> \targ->fmt.pix.bytesperline = config.stride;\n> \targ->fmt.pix.sizeimage = config.frameSize;","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 C5851C3275\n\tfor ;\n\tFri, 29 Jul 2022 17:41:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40D7B63312;\n\tFri, 29 Jul 2022 19:41:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9A1ED603EC\n\tfor ;\n\tFri, 29 Jul 2022 19:41:07 +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 05DED6D4;\n\tFri, 29 Jul 2022 19:41:06 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659116469;\n\tbh=bfe0TrJYd5DoEmZToee5YlyoTdCaf7elKkTaJ41qBB8=;\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=iNcRrZjf+OBo/rXoluTEyHCp9CjnLYrn919m78Ma4LyHtiDOiTAp/Oh9KcBFs+NJG\n\t9Jk5mrBWgg+J0ZOzZCOAAjlZVKen0SJwbr6dH+iK6LgYd3nNdb6Pi1wGFvXd+bVrVb\n\tSsNNAx6iMrj9so99Sz64vKxJnaDm/jUvsuOE1aWb7rjPYcBU5dPBIxhLJ+N8TuCr+M\n\t0VajPoMSy6TJF9X7dqrZeWpGupiAcY6+8DLY3nLvhMDMabVDRWqwP+e2ItQiPH/gki\n\tlOL723ya0Jve9FW6wEpIgZkDajXi8rqyFp7uW+JbmBKQ3iWNwW2rPrmPH2z0agOifp\n\tsowtzQ3PAsBUQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659116467;\n\tbh=bfe0TrJYd5DoEmZToee5YlyoTdCaf7elKkTaJ41qBB8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=q18RE9BnJPn663DKua0Wvg3uH34h+MvctfuqypWp3nGK3nim0u4VoCJCUeuLLv70C\n\tikNFDhH0RVda/7DG7UbvzestFuFQj7/m2xZ+PztN2rfUQI+BBzQ1bJ5BOkPISWFEHQ\n\tj8HWrK8d/c7TsRiqfeeYQ9N5vipv8ftNR2h/WgZo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"q18RE9Bn\"; dkim-atps=neutral","Date":"Fri, 29 Jul 2022 20:41:04 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220729160014.101503-1-jacopo@jmondi.org>\n\t<20220729160014.101503-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220729160014.101503-5-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] libcamera: v4l2_pixelformat:\n\tReturn the list of 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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, jozef@mlich.cz,\n\tPavel Machek ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]