[{"id":11294,"web_url":"https://patchwork.libcamera.org/comment/11294/","msgid":"<20200709155157.GB5868@pendragon.ideasonboard.com>","date":"2020-07-09T15:51:57","subject":"Re: [libcamera-devel] [PATCH v5 06/23] 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 Thu, Jul 09, 2020 at 10:28:18PM +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> Reviewed-by: Laurent Pinchart \n> \n> ---\n> Changes in v5:\n> - changed default align parameter for stride() to 1\n> - added align parameter to frameSize()\n> \n> Changes in v4:\n> - add overloaded frameSize() that takes array of strides\n> - add optional parameter to stride() for byte alignment\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 | 6 +++\n> src/libcamera/formats.cpp | 80 ++++++++++++++++++++++++++++\n> 2 files changed, 86 insertions(+)\n> \n> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h\n> index 211da4a..cad41ad 100644\n> --- a/include/libcamera/internal/formats.h\n> +++ b/include/libcamera/internal/formats.h\n> @@ -53,6 +53,12 @@ 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,\n> +\t\t\t unsigned int align = 1) const;\n> +\tunsigned int frameSize(const Size &size, unsigned int align = 1) const;\n> +\tunsigned int frameSize(const Size &size,\n> +\t\t\t const std::array &strides) const;\n> +\n> \t/* \\todo Add support for non-contiguous memory planes */\n> \tconst char *name;\n> \tPixelFormat format;\n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index 6d96055..0f71061 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -732,4 +732,84 @@ 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> + * \\param[in] align The number of bytes to align to (1 for default alignment)\n> + * \\return The number of bytes necessary to store a line, or 0 if the\n> + * PixelFormatInfo instance or the \\a plane is not valid\n> + */\n> +unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane,\n> +\t\t\t\t unsigned int align) const\n> +{\n> +\tif (!isValid()) {\n> +\t\tLOG(Formats, Warning) << \"Invalid pixel format, stride is zero\";\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tif (plane > planes.size() || !planes[plane].bytesPerGroup) {\n> +\t\tLOG(Formats, Warning) << \"Invalid plane index, stride is zero\";\n> +\t\treturn 0;\n> +\t}\n> +\n> +\t/* ceil(width / pixelsPerGroup) * bytesPerGroup */\n> +\tunsigned int stride = (width + pixelsPerGroup - 1) / pixelsPerGroup\n> +\t\t\t * planes[plane].bytesPerGroup;\n> +\n> +\t/* ceil(stride / align) * align */\n> +\treturn (stride + align - 1) / align * align;\n> +}\n> +\n> +/**\n> + * \\brief Compute the number of bytes necessary to store a frame\n> + * \\param[in] size The size of the frame, in pixels\n> + * \\param[in] align The number of bytes to align to (1 for default alignment)\n\nFor the stride() function this is quite clear, but here it could be\nmisinterpreted as a frame alignment instead of a stride alignment.\n\n * \\param[in] align The stride alignment, in bytes\n *\n * The frame is computed by adding the product of the line stride and the frame\n * height for all planes, taking subsampling and other format characteristics\n * into account. Additional stride alignment constraints may be specified\n * through the \\a align parameter, and will apply to all planes. For more\n * complex stride constraints, use the frameSize() overloaded version that takes\n * an array of stride values.\n *\n * \\sa stride()\n *\n\nAnd maybe the documentation of stride should also be expanded ?\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 * \\param[in] align The stride alignment, in bytes\n *\n * The stride is the number of bytes necessary to store a full line of a frame,\n * including padding at the end of the line. This function takes into account\n * the alignment constraints intrinsic to the format (for instance, the\n * SGRBG12_CSI2P format stores two 12-bit pixels in 3 bytes, and thus has a\n * required stride alignment of 3 bytes). Additional alignment constraints may\n * be specified through the \\a align parameter, which will cause the stride to\n * be rounded up to the next multiple of \\a align.\n *\n * For multi-planar formats, different planes may have different stride values.\n * The \\a plane parameter selects which plane to compute the stride for.\n *\n * \\return The number of bytes necessary to store a line, or 0 if the\n * PixelFormatInfo instance or the \\a plane is not valid\n */\n\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, unsigned int align) const\n> +{\n> +\t/* stride * ceil(height / verticalSubSampling) */\n> +\tunsigned int sum = 0;\n> +\tfor (unsigned int i = 0; i < 3; i++) {\n> +\t\tunsigned int vertSubSample = planes[i].verticalSubSampling;\n> +\t\tif (!vertSubSample)\n> +\t\t\tcontinue;\n> +\t\tsum += stride(size.width, i, align)\n> +\t\t * ((size.height + vertSubSample - 1) / vertSubSample);\n> +\t}\n> +\n> +\treturn sum;\n> +}\n> +\n> +/**\n> + * \\brief Compute the number of bytes necessary to store a frame\n> + * \\param[in] size The size of the frame, in pixels\n> + * \\param[in] strides The strides to use for each plane\n> + *\n> + * This function is an overloaded version that takes custom strides for each\n> + * plane, to be used when the device has custom alignment constraints that\n> + * can't be described by just an alignment value.\n> + *\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\n> +PixelFormatInfo::frameSize(const Size &size,\n> +\t\t\t const std::array &strides) const\n> +{\n> +\t/* stride * ceil(height / verticalSubSampling) */\n> +\tunsigned int sum = 0;\n> +\tfor (unsigned int i = 0; i < 3; i++) {\n> +\t\tunsigned int vertSubSample = planes[i].verticalSubSampling;\n> +\t\tif (!vertSubSample)\n> +\t\t\tcontinue;\n> +\t\tsum += strides[i]\n> +\t\t * ((size.height + vertSubSample - 1) / vertSubSample);\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 AE7DBBD792\n\tfor ;\n\tThu, 9 Jul 2020 15:52:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F7E86134E;\n\tThu, 9 Jul 2020 17:52:06 +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 E76FC6123A\n\tfor ;\n\tThu, 9 Jul 2020 17:52:04 +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 54882525;\n\tThu, 9 Jul 2020 17:52:04 +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=\"YtVnbh9/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594309924;\n\tbh=P7HxacGVuVV4YfTbVWfT3WDBkHpG7oWem+gw0Zb7OpU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YtVnbh9/ESQqsFHVSmUSFexaNy+TM5b/55gZJFAUG8CIMvH3SYFITxhO4oaQryAy6\n\tAXb6Gryx9fl1JbfViiQ+9iuyfYH6AsYkTmGktuWv9e0osG/ryeKTHBUwBmaHvziWBe\n\tD8+yWjCL7ebBK6L260O5mgjcrr0ZOaT+NEvcVQ1M=","Date":"Thu, 9 Jul 2020 18:51:57 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20200709155157.GB5868@pendragon.ideasonboard.com>","References":"<20200709132835.112593-1-paul.elder@ideasonboard.com>\n\t<20200709132835.112593-7-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200709132835.112593-7-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 06/23] 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\" "}}]