[{"id":24076,"web_url":"https://patchwork.libcamera.org/comment/24076/","msgid":"<Ytw0CMA9f09hq5CR@pendragon.ideasonboard.com>","date":"2022-07-23T17:46:48","subject":"Re: [libcamera-devel] [PATCH v2 6/8] libcamera: v4l2_videodevice:\n\tAdd multiplanar argument to toV4L2PixelFormat","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 Sat, Jul 23, 2022 at 11:53:28AM +0200, Jacopo Mondi via libcamera-devel wrote:\n> The toV4L2PixelFormat() function selects which V4L2 format variant to\n> use (contiguous planes vs non-contiguous version) by using video device\n> multiplanar API from the capabilities.\n> \n> The isMultiplanar() function however verifies if the video device uses\n> the singleplanar or the multiplanar V4L2 API, something which is\n> unrelated to the format variant to use.\n> \n> Add a 'multiplanar' flag, which defaults to false, to the\n> toV4L2PixelFormat() function to allow explicit selection of the format\n> variant.\n\nNone of the callers set multiplanar to true in this series, so I'd drop\nthis patch (see the review of the previous patch for more details).\n\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  include/libcamera/internal/v4l2_videodevice.h |  3 ++-\n>  src/libcamera/v4l2_videodevice.cpp            | 13 +++++++------\n>  2 files changed, 9 insertions(+), 7 deletions(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 6d8850c99afd..64296c5849f2 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -228,7 +228,8 @@ public:\n>  \tstatic std::unique_ptr<V4L2VideoDevice>\n>  \tfromEntityName(const MediaDevice *media, const std::string &entity);\n>  \n> -\tV4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat) const;\n> +\tV4L2PixelFormat toV4L2PixelFormat(const PixelFormat &pixelFormat,\n> +\t\t\t\t\t  bool multiplanar = false) const;\n>  \n>  protected:\n>  \tstd::string logPrefix() const override;\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index a3242ba755c0..767ab2361ef5 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -1992,23 +1992,24 @@ V4L2VideoDevice::fromEntityName(const MediaDevice *media,\n>  /**\n>   * \\brief Convert \\a PixelFormat to one of the device supported V4L2 FourCC\n>   * \\param[in] pixelFormat The PixelFormat to convert\n> + * \\param[in] multiplanar Use the multiplanar format version, default to false\n>   *\n>   * Convert a\\ pixelformat to a V4L2 FourCC that is known to be supported by\n>   * the video device.\n>   *\n> - * For multiplanar formats, the V4L2 format variant (contiguous or\n> - * non-contiguous planes) is selected automatically based on the capabilities\n> - * of the video device. If the video device supports the V4L2 multiplanar API,\n> - * non-contiguous formats are preferred.\n> + * V4L2 defines different format variants for the same format when using\n> + * contiguous or non-contiguous planes. The \\a multiplanar parameter allows\n> + * to select which variant to use.\n>   *\n>   * \\return The V4L2PixelFormat corresponding to \\a pixelFormat or an invalid\n>   * PixelFormat if \\a pixelFormat is not supported by the video device\n>   */\n> -V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat) const\n> +V4L2PixelFormat V4L2VideoDevice::toV4L2PixelFormat(const PixelFormat &pixelFormat,\n> +\t\t\t\t\t\t   bool multiplanar) const\n>  {\n>  \tstd::vector<V4L2PixelFormat> deviceFormats = enumPixelformats(0);\n>  \tstd::vector<V4L2PixelFormat> v4l2PixelFormats =\n> -\t\tV4L2PixelFormat::fromPixelFormat(pixelFormat, caps_.isMultiplanar());\n> +\t\tV4L2PixelFormat::fromPixelFormat(pixelFormat, multiplanar);\n>  \n>  \tfor (const V4L2PixelFormat &v4l2Format : v4l2PixelFormats) {\n>  \t\tauto it = std::find_if(deviceFormats.begin(), deviceFormats.end(),","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 EB14EC3275\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 23 Jul 2022 17:46:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A423763312;\n\tSat, 23 Jul 2022 19:46:52 +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 B1391603F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 23 Jul 2022 19:46:51 +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 4AD419F7;\n\tSat, 23 Jul 2022 19:46:51 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1658598412;\n\tbh=m6HfG5lKwGIbDWaeBD/KVQHjbfbvspk0agmGSlZf8xU=;\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=Jhsleo/u2nmiW/BUUvAuszEO9dGbP5bHz84kN0+RCxYmci5uggk2WNZJcnh2YNRfC\n\tmwDtif10RQsggPb2AWiEe5TaJop8FSl3C/6XPkxUQspPPzcmB1zf/rk/PX0ybADTGt\n\tgxXqhKNaFcHPFnrms8x+w39w4gUmLLWydSaJIx0qBSqQivQIF6JsJzhLdgtdXiRynn\n\tDGrK5+Emq0mmTT9arTqlrfgu+FzL/Q6MHTgqNKfTtIkjeNNUCCdqQV3vnorRWEZhTV\n\t7Wz4ZNL+PHg23tKSCejFMaur9fr8AwjMfk0FT23FiFA04pfVsD7D2QwTj9pDAZ9kTh\n\tIq4YziFDPrhnA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1658598411;\n\tbh=m6HfG5lKwGIbDWaeBD/KVQHjbfbvspk0agmGSlZf8xU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sIwYgLdn+AmAZiNPH6xrb4Kp9mW/bAPEcu1SvLObXk6Bm0emF5ziQwzY0hvToZV4f\n\tnDyqR+jp9fGGs+lvntY4V+5jzbgjQKJjE0rCZ+/Dajkq7mamUvpLGqfjBz2SPU9MlL\n\tHr2vyQjVJYCbwqVRVowj9bz80+D0gCa9TYZUz/i4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sIwYgLdn\"; dkim-atps=neutral","Date":"Sat, 23 Jul 2022 20:46:48 +0300","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<Ytw0CMA9f09hq5CR@pendragon.ideasonboard.com>","References":"<20220723095330.43542-1-jacopo@jmondi.org>\n\t<20220723095330.43542-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220723095330.43542-7-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 6/8] libcamera: v4l2_videodevice:\n\tAdd multiplanar argument to toV4L2PixelFormat","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, jozef@mlich.cz,\n\tPavel Machek <pavel@ucw.cz>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]