[{"id":11170,"web_url":"https://patchwork.libcamera.org/comment/11170/","msgid":"<20200704183917.GD6018@pendragon.ideasonboard.com>","date":"2020-07-04T18:39:17","subject":"Re: [libcamera-devel] [PATCH v3 06/22] libcamera: PixelFormatInfo:\n\tAdd functions stride and frameSize","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Sat, Jul 04, 2020 at 10:31:24PM +0900, Paul Elder wrote:\n> Add member functions to PixelFormatInfo for calculating stride and frame\n> size. This will simplify existing code that calculates these things.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v3:\n> - rename functions to stride and frameSize, from bytesPerLine and\n> imageSize, respectively\n> \n> Changes in v2:\n> - make these functions const\n> - add documentation\n> - inline DIV_ROUND_UP\n> ---\n> include/libcamera/internal/formats.h | 3 +++\n> src/libcamera/formats.cpp | 40 ++++++++++++++++++++++++++++\n> 2 files changed, 43 insertions(+)\n> \n> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h\n> index cf00814..8957059 100644\n> --- a/include/libcamera/internal/formats.h\n> +++ b/include/libcamera/internal/formats.h\n> @@ -52,6 +52,9 @@ public:\n> \tstatic const PixelFormatInfo &info(const PixelFormat &format);\n> \tstatic const PixelFormatInfo &info(const V4L2PixelFormat &format);\n> \n> +\tunsigned int stride(unsigned int width, unsigned int plane) const;\n> +\tunsigned int frameSize(const Size &size) const;\n> +\n> \tconst char *name;\n> \tPixelFormat format;\n> \tV4L2PixelFormat v4l2Format;\n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index f96bf1f..6d558f2 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -710,4 +710,44 @@ const PixelFormatInfo &PixelFormatInfo::info(const V4L2PixelFormat &format)\n> \treturn info->second;\n> }\n> \n> +/**\n> + * \\brief Compute the stride\n> + * \\param[in] width The width of the line, in pixels\n> + * \\param[in] plane The index of the plane whose stride is to be computed\n> + * \\return The number of bytes necessary to store a line, or 0 if the\n> + * PixelFormatInfo instance not valid\n\ns/not valid/or the \\a plane is not valid/\n\n> + */\n> +unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane) const\n> +{\n> +\tif (!isValid())\n> +\t\treturn 0;\n> +\n> +\tif (plane > planes.size() || !planes[plane].bytesPerGroup)\n> +\t\treturn 0;\n> +\n> +\t/* ceil(width / pixelsPerGroup) * bytesPerGroup */\n> +\treturn ((width + pixelsPerGroup - 1) / pixelsPerGroup) * planes[plane].bytesPerGroup;\n\nYou can remove the outer parentheses, and let's add a line break to keep\nthis shorter.\n\n> +}\n> +\n> +/**\n> + * \\brief Compute the bytes necessary to store the frame\n> + * \\param[in] size The size of the frame, in pixels\n> + * \\return The number of bytes necessary to store the frame, or 0 if the\n> + * PixelFormatInfo instance is not valid\n> + */\n> +unsigned int PixelFormatInfo::frameSize(const Size &size) const\n> +{\n> +\t/* stride * ceil(height / verticalSubSampling) */\n> +\tunsigned int sum = 0;\n\ns/sum/size/ ?\n\n> +\tfor (int i = 0; i < 3; i++) {\n\nunsigned int as i is always positive, but even better,\n\n\tfor (const PixelFormatPlaneInfo &plane : planes) {\n\n> +\t\tunsigned int vertSubSample = planes[i].verticalSubSampling;\n> +\t\tif (!vertSubSample)\n> +\t\t\tcontinue;\n> +\t\tsum += stride(size.width, i)\n> +\t\t * ((size.height + vertSubSample - 1) / vertSubSample);\n\nThis is an issue. The stride may be aligned by the pipeline handler, so\nyou can't calculate it here (the IPU3 pipeline handler will align it to\n64 bytes for RAW formats for instance). I'm afraid you need to pass the\nwidth and an array of strides to this function (and the range-based for\nloop will then not be possible anymore). Or maybe find a better API than\nwhat I'm proposing :-) I haven't looked at how you use this in the\npipeline handlers yet.\n\n> +\t}\n> +\n> +\treturn sum;\n> +}\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 0105BBD790\n\tfor ;\n\tSat, 4 Jul 2020 18:39:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8B7A260E01;\n\tSat, 4 Jul 2020 20:39:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F534609C7\n\tfor ;\n\tSat, 4 Jul 2020 20:39:21 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F1A79296;\n\tSat, 4 Jul 2020 20:39:20 +0200 (CEST)"],"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=\"UKyZ86ra\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593887961;\n\tbh=dYewoun8flD6Nr+obDACfuQ00+Vcm9dF0GLwNO98Seg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UKyZ86rah4NBM58UyjbjTvlINdrYJskEZH5D+/nb7D3XTSHK0A8sgPA1Utety7ca0\n\tPq7gsgjUYnN9sVdsQRmE3jCTmwl05xs1wGN++SsswxO1vWRjOAs15TRmxBsrYjSFUu\n\txYF7Cas6lUyl28E6T+r5245RpOyUVm0gYuLT4MNs=","Date":"Sat, 4 Jul 2020 21:39:17 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20200704183917.GD6018@pendragon.ideasonboard.com>","References":"<20200704133140.1738660-1-paul.elder@ideasonboard.com>\n\t<20200704133140.1738660-7-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200704133140.1738660-7-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 06/22] libcamera: PixelFormatInfo:\n\tAdd functions stride and frameSize","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":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]