[{"id":11168,"web_url":"https://patchwork.libcamera.org/comment/11168/","msgid":"<20200704182947.GB6018@pendragon.ideasonboard.com>","date":"2020-07-04T18:29:47","subject":"Re: [libcamera-devel] [PATCH v3 02/22] libcamera: formats: Add\n\tfields to info to ease calculating stride","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:20PM +0900, Paul Elder wrote:\n> Packed formats make it difficult to calculate stride as well as\n> frame size with the fields that PixelFormatInfo currently has.\n> bitsPerPixel is defined as the average number of bits per pixel, and\n> only counts effective bits, so it is not useful for calculating\n> stride and frame size.\n> \n> To fix this, we introduce a concept of a \"pixel group\". The size of this\n> group is defined as the minimum number of pixels (including padding)\n> necessary in a row when the image has only one column of effective\n> pixels. The pixel group has one more attribute, that is the \"bytes per\n> group\". This determines how many bytes one pixel group consumes. These\n> are the fields pixelsPerGroup and bytesPerGroup that are defined in this\n> patch. Defining these two values makes it really simple to calculate\n> bytes-per-line, as ceil(width / pixelsPerGroup) * bytesPerGroup, where\n> width is measured in number of pixels. The ceiling accounts for padding.\n> \n> Clearly, pixelsPerGroup must be constant for all planes in the format.\n> The bytesPerGroup then, must be a per-plane attribute. There is one more\n> field, verticalSubSampling, that is per-plane. This is simply a divider,\n> to divide the number of rows of pixels by the sub-sampling value, to\n> obtain the number of rows of pixels for the subsampled plane.\n> \n> For example, for something simple like BGR888, it is self-explanatory:\n> the pixel group size is 1, and the bytes necessary is 3, and there is\n> only one plane with no (= 1) vertical subsampling. For YUYV, the\n> CbCr pair is shared between two pixels, so even if you have only one\n> pixel, you would still need a padded second Y, therefore the pixel\n> group size is 2, and bytes necessary is 4 (as opposed to 1 and 2). YUYV\n> also has no vertical subsampling. NV12 has a pixel group size of 2\n> pixels, due to the CbCr plane. The bytes per group then, for both\n> planes, is 2. The first plane has no vertical subsampling, but the\n> second plane is subsampled by a factor of 2.\n> The IPU3 formats are also self-explanatory, as they are single-planar,\n> and have a pixel group size of 25, consuming 32 bytes. Although a\n> comment in the driver suggests that it should be 50 and 64,\n> respectively, this is an attribute of the driver, and not the format, so\n> this shall be set by the ipu3 pipeline handler.\n> \n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v3:\n> - add planes\n> - redefine the parameters for the formats\n> - pixelsPerGroup is for whole format\n> - add verticalSubSampling per plane\n> \n> Changes in v2:\n> - add documentation for bytesPerGroup pixelsPerGroup\n> - fix wording in commit message\n> - bytes-per-line -> stride\n> - buffer size -> frame size\n> - changed MJPEG todo to allowing pipeline handlers to set parameters of\n> format infos\n> ---\n> include/libcamera/internal/formats.h | 11 ++-\n> src/libcamera/formats.cpp | 128 ++++++++++++++++++++++++++-\n> 2 files changed, 137 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h\n> index f59ac8f..ebd256c 100644\n> --- a/include/libcamera/internal/formats.h\n> +++ b/include/libcamera/internal/formats.h\n> @@ -32,6 +32,12 @@ private:\n> \tstd::map> data_;\n> };\n> \n> +struct PixelFormatPlane\n\nMaybe PixelFormatPlaneInfo ?\n\n> +{\n> +\tunsigned int bytesPerGroup;\n> +\tunsigned int verticalSubSampling;\n> +};\n> +\n> class PixelFormatInfo\n> {\n> public:\n> @@ -45,13 +51,16 @@ public:\n> \n> \tstatic const PixelFormatInfo &info(const PixelFormat &format);\n> \n> -\t/* \\todo Add support for non-contiguous memory planes */\n\nI think you still need to keep this, as V4L2 uses different 4CCs for\ncontiguous and non-contiguous formats (yeah, I know, ...) and we don't\nsupport this yet.\n\n> \tconst char *name;\n> \tPixelFormat format;\n> \tV4L2PixelFormat v4l2Format;\n> \tunsigned int bitsPerPixel;\n> \tenum ColourEncoding colourEncoding;\n> \tbool packed;\n> +\n> +\tunsigned int pixelsPerGroup;\n> +\n> +\tstd::array planes;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index d3b722c..6bdd28d 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -110,6 +110,22 @@ const std::map> &ImageFormats::data() const\n> \treturn data_;\n> }\n> \n> +/**\n> + * \\class PixelFormatPlane\n> + * \\brief Information about a single plane of a pixel format\n> + *\n> + * \\var PixelFormatPlane::bytesPerGroup\n> + * \\brief The number of bytes that a pixel group consumes\n> + *\n> + * \\sa PixelFormatInfo::pixelsPerGroup\n> + *\n> + * \\var PixelFormatPlane::verticalSubSampling\n> + * \\brief Vertical subsampling multiplier\n> + *\n> + * This value is the ratio between the number of rows of pixels in the frame\n> + * to the number of rows of pixels in the plane.\n> + */\n> +\n> /**\n> * \\class PixelFormatInfo\n> * \\brief Information about pixel formats\n> @@ -152,6 +168,26 @@ const std::map> &ImageFormats::data() const\n> * bytes. For instance, 12-bit Bayer data with two pixels stored in three bytes\n> * is packed, while the same data stored with 4 bits of padding in two bytes\n> * per pixel is not packed.\n> + *\n> + * \\var PixelFormatInfo::pixelsPerGroup\n> + * \\brief The number of pixels in a pixel group\n> + *\n> + * The minimum number of pixels (including padding) necessary in a row\n> + * when the frame has only one column of effective pixels\n\ns/$/./\n\n> + *\n> + * A pixel group is defined as the minimum number of pixels (including padding)\n> + * necessary in a row when the image has only one column of effective pixels.\n> + * pixelsPerGroup refers to this value. bytesPerGroup, then, refers to the\n\ns/bytesPerGroup/PixelFormatPlane::bytesPerGroup/\n\n> + * number of bytes that a pixel group consumes. This definition of a pixel\n> + * group allows simple calculation of stride, as\n> + * ceil(width / pixelsPerGroup) * bytesPerGroup. These values are determined\n> + * only in terms of a row, and include bytes that are used in all planes (for\n> + * multiplanar formats).\n\nWhat do you mean by \"include bytes that are used in all planes\" ?\n\n> + *\n> + * \\var PixelFormatInfo::planes\n> + * \\brief Information about pixels for each plane\n> + *\n> + * \\sa PixelFormatPlane\n> */\n> \n> /**\n> @@ -179,6 +215,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 24,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::RGB888, {\n> \t\t.name = \"RGB888\",\n> @@ -187,6 +225,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 24,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::ABGR8888, {\n> \t\t.name = \"ABGR8888\",\n> @@ -195,6 +235,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 32,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::ARGB8888, {\n> \t\t.name = \"ARGB8888\",\n> @@ -203,6 +245,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 32,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::BGRA8888, {\n> \t\t.name = \"BGRA8888\",\n> @@ -211,6 +255,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 32,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::RGBA8888, {\n> \t\t.name = \"RGBA8888\",\n> @@ -219,6 +265,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 32,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \n> \t/* YUV packed formats. */\n> @@ -229,6 +277,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::YVYU, {\n> \t\t.name = \"YVYU\",\n> @@ -237,6 +287,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::UYVY, {\n> \t\t.name = \"UYVY\",\n> @@ -245,6 +297,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::VYUY, {\n> \t\t.name = \"VYUY\",\n> @@ -253,6 +307,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \n> \t/* YUV planar formats. */\n> @@ -263,6 +319,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 2, 2 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV21, {\n> \t\t.name = \"NV21\",\n> @@ -271,6 +329,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 2, 2 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV16, {\n> \t\t.name = \"NV16\",\n> @@ -279,6 +339,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 2, 1 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV61, {\n> \t\t.name = \"NV61\",\n> @@ -287,6 +349,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 2, 1 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV24, {\n> \t\t.name = \"NV24\",\n> @@ -295,6 +359,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 24,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 1, 1 }, { 2, 1 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV42, {\n> \t\t.name = \"NV42\",\n> @@ -303,6 +369,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 24,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 1, 1 }, { 2, 1 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::YUV420, {\n> \t\t.name = \"YUV420\",\n> @@ -311,6 +379,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 1, 2 }, { 1, 2 } }},\n> \t} },\n> \t{ formats::YUV422, {\n> \t\t.name = \"YUV422\",\n> @@ -319,6 +389,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 1, 1 }, { 1, 1 } }},\n> \t} },\n> \n> \t/* Greyscale formats. */\n> @@ -329,6 +401,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \n> \t/* Bayer formats. */\n> @@ -339,6 +413,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG8, {\n> \t\t.name = \"SGBRG8\",\n> @@ -347,6 +423,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG8, {\n> \t\t.name = \"SGRBG8\",\n> @@ -355,6 +433,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB8, {\n> \t\t.name = \"SRGGB8\",\n> @@ -363,6 +443,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR10, {\n> \t\t.name = \"SBGGR10\",\n> @@ -371,6 +453,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG10, {\n> \t\t.name = \"SGBRG10\",\n> @@ -379,6 +463,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG10, {\n> \t\t.name = \"SGRBG10\",\n> @@ -387,6 +473,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB10, {\n> \t\t.name = \"SRGGB10\",\n> @@ -395,6 +483,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR10_CSI2P, {\n> \t\t.name = \"SBGGR10_CSI2P\",\n> @@ -403,6 +493,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 4,\n> +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG10_CSI2P, {\n> \t\t.name = \"SGBRG10_CSI2P\",\n> @@ -411,6 +503,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 4,\n> +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG10_CSI2P, {\n> \t\t.name = \"SGRBG10_CSI2P\",\n> @@ -419,6 +513,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 4,\n> +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB10_CSI2P, {\n> \t\t.name = \"SRGGB10_CSI2P\",\n> @@ -427,6 +523,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 4,\n> +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR12, {\n> \t\t.name = \"SBGGR12\",\n> @@ -435,6 +533,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG12, {\n> \t\t.name = \"SGBRG12\",\n> @@ -443,6 +543,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG12, {\n> \t\t.name = \"SGRBG12\",\n> @@ -451,6 +553,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB12, {\n> \t\t.name = \"SRGGB12\",\n> @@ -459,6 +563,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR12_CSI2P, {\n> \t\t.name = \"SBGGR12_CSI2P\",\n> @@ -467,6 +573,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG12_CSI2P, {\n> \t\t.name = \"SGBRG12_CSI2P\",\n> @@ -475,6 +583,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG12_CSI2P, {\n> \t\t.name = \"SGRBG12_CSI2P\",\n> @@ -483,6 +593,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB12_CSI2P, {\n> \t\t.name = \"SRGGB12_CSI2P\",\n> @@ -491,6 +603,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR10_IPU3, {\n> \t\t.name = \"SBGGR10_IPU3\",\n> @@ -499,6 +613,9 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t/* \\todo remember to double this in the ipu3 pipeline handler */\n> +\t\t.pixelsPerGroup = 25,\n> +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG10_IPU3, {\n> \t\t.name = \"SGBRG10_IPU3\",\n> @@ -507,6 +624,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 25,\n> +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG10_IPU3, {\n> \t\t.name = \"SGRBG10_IPU3\",\n> @@ -515,6 +634,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 25,\n> +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB10_IPU3, {\n> \t\t.name = \"SRGGB10_IPU3\",\n> @@ -523,16 +644,21 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 25,\n> +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \n> \t/* Compressed formats. */\n> +\t/* \\todo remember to fill this in from the pipeline handler. */\n\nI don't think this comment is needed, the information here won't be\nfilled by the pipeline handlers.\n\n> \t{ formats::MJPEG, {\n> \t\t.name = \"MJPEG\",\n> \t\t.format = formats::MJPEG,\n> \t\t.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> -\t\t.bitsPerPixel = 0,\n> +\t\t.bitsPerPixel = 16,\n\nI'd still set this to 0, it's not valid for MJPEG.\n\nReviewed-by: Laurent Pinchart \n\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\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 9C082BD792\n\tfor ;\n\tSat, 4 Jul 2020 18:29:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1FBC660E01;\n\tSat, 4 Jul 2020 20:29:53 +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 B9156609C7\n\tfor ;\n\tSat, 4 Jul 2020 20:29:51 +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 1C0F8296;\n\tSat, 4 Jul 2020 20:29:51 +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=\"kLCqP+RN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593887391;\n\tbh=mAs4caom+wiNHeiKH1hcUqWrYFUgjuVxj4Hrcd23zFE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kLCqP+RNALYQE0SC4bNdDB5T7xGZ4dvONvlsBHDhytOQRm8EPbgMGemivv6VDujJM\n\tBC19eM3lKANlTh2MRfl4OsT8SICTotv0pQO3BMTmkqyzLkXiuRzFIP5HMxOaZdVLcT\n\tn2IzlKf8YQwWXAnXwhXM4gTGVPd1tE+CXotR/k4I=","Date":"Sat, 4 Jul 2020 21:29:47 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20200704182947.GB6018@pendragon.ideasonboard.com>","References":"<20200704133140.1738660-1-paul.elder@ideasonboard.com>\n\t<20200704133140.1738660-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200704133140.1738660-3-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 02/22] libcamera: formats: Add\n\tfields to info to ease calculating stride","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\" "}},{"id":11218,"web_url":"https://patchwork.libcamera.org/comment/11218/","msgid":"<20200706141351.oxy7rzzblzw2qovo@uno.localdomain>","date":"2020-07-06T14:13:51","subject":"Re: [libcamera-devel] [PATCH v3 02/22] libcamera: formats: Add\n\tfields to info to ease calculating stride","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Sat, Jul 04, 2020 at 10:31:20PM +0900, Paul Elder wrote:\n> Packed formats make it difficult to calculate stride as well as\n> frame size with the fields that PixelFormatInfo currently has.\n> bitsPerPixel is defined as the average number of bits per pixel, and\n> only counts effective bits, so it is not useful for calculating\n> stride and frame size.\n>\n> To fix this, we introduce a concept of a \"pixel group\". The size of this\n> group is defined as the minimum number of pixels (including padding)\n> necessary in a row when the image has only one column of effective\n> pixels. The pixel group has one more attribute, that is the \"bytes per\n> group\". This determines how many bytes one pixel group consumes. These\n> are the fields pixelsPerGroup and bytesPerGroup that are defined in this\n> patch. Defining these two values makes it really simple to calculate\n> bytes-per-line, as ceil(width / pixelsPerGroup) * bytesPerGroup, where\n> width is measured in number of pixels. The ceiling accounts for padding.\n\nHow does ceiling accounts for padding ? this depends on the format\ndefined byte alignement. There's no guarantee 'the next biggest\ninteger' is enough for that, am I wrong ?\n\n>\n> Clearly, pixelsPerGroup must be constant for all planes in the format.\n> The bytesPerGroup then, must be a per-plane attribute. There is one more\n> field, verticalSubSampling, that is per-plane. This is simply a divider,\n> to divide the number of rows of pixels by the sub-sampling value, to\n> obtain the number of rows of pixels for the subsampled plane.\n>\n> For example, for something simple like BGR888, it is self-explanatory:\n> the pixel group size is 1, and the bytes necessary is 3, and there is\n> only one plane with no (= 1) vertical subsampling. For YUYV, the\n> CbCr pair is shared between two pixels, so even if you have only one\n> pixel, you would still need a padded second Y, therefore the pixel\n\ns/padded second Y/second sample ?\n\n> group size is 2, and bytes necessary is 4 (as opposed to 1 and 2). YUYV\n\nwhat do you mean with \"as opposed to\" ?\n\n> also has no vertical subsampling. NV12 has a pixel group size of 2\n\nAssuming I got this right, YUYV has a single plane, hence no\nsub-sampling. Is it true that sub-sampling applies only to the\nadditional planes, and formats with a single plane always have 1 as\nsub-sampling value ?\n\n> pixels, due to the CbCr plane. The bytes per group then, for both\n> planes, is 2. The first plane has no vertical subsampling, but the\n> second plane is subsampled by a factor of 2.\n> The IPU3 formats are also self-explanatory, as they are single-planar,\n> and have a pixel group size of 25, consuming 32 bytes. Although a\n\nThe IPU3 formats pack pixels in 32 bytes as you said, but how does\nthis play with the definition of\n\n\"The size of [a pixel group] is defined as the minimum number of pixels\n(including padding) necessary in a row when the image has only one column\nof effective pixels\" ?\n\nI'm mostly missing what you mean by \"necessary\" here.\n\nI understand how tricky it is to find a way to express what you're\nhere describing, and I don't have much better wording for that.\n\nFor other formats is clear: a pixel group is the minium number of\nsamples that are required to re-construct one pixel. Sticking to your\nexamples, for RGB888 a pixel group is 1 sample, of 24bits each.\nFor YUYV you need two samples to reconstruct a pixel, and each sample\nis 16 bits in size. For formats like the IPU3 to reconstruct 1 pixel\nyou actually need a single sample of 30 bits each (those formats are\nequivalent to S[GGBR]10 in sample size and sample ordering). The\nrequirement to pack 25 pixels in 32bytes is the due to how they pack\none 'tile' of pixels in a byte-aligned memory area.\n\nI know 'tile' has a precise meaing for, say, display formats which do\nnot apply in the context of video formats (for my understanding) but\nwould this be a better term to use in place of \"pixel group\" ?\nI would leave out any attempt to try to define the number of samples\nper tile in terms of re-constructed pixels, as for the IPU3 format,\nthat does not apply well.\n\nIn example:\n\n-------------------------------------------------------------------------------\nTo fix this, we introduce a concept of a \"memory tile\". A tile is\ncomposed by a number of samples (which are combined to re-construct\npixels color informations) defined by the image format, and a\ntile size in bytes which includes any necessay padding bits to align\nthe effective color information samples to a boundary defined\nby the format.\n\nThe number of samples per tile is reported by the samplesPerTile\nattribute, while the overall tile size by the bytesPerTile one.\n\nBy using those two format-defined values, calculating the number of\nbytes per line becomes trival as in:\n\n bytesPerLine = (width / samplesPerTile) * bytesPerTile\n\nFor RAW formats such as RAW888 the number of samples is 3, with a size\nof 1 bytes each. Each tile is then 3 bytes in size, and the number of\nbytes per line is simply the number of pixels\n\n bytesPerLine = (width / 3) * 3 = width\n\nIn RAW10 formats each tile is composed of 3 samples (one per colour\ninformation) and each sample is composed by 10-bits of effective color\ninformation and 6 padding bits. A tile has a bytes size of 6 and the\nline size is calculated as\n\n bytesPerLine = (width / 3) * 6 = 2width\n\nFor YUYV formats, each tile has a size of two samples, with each\nsample being 2 bytes in size. The tile size is then 4 bytes and the\nline width is calculated as\n\n bytesPerLine = (width / 2) * 4 = 2width\n\nOther formats, mostly the platform specific ones, define the number of\nsamples per tile in order to maximize the format space efficiency or\nto align samples to boundaries defined by the platform image\nprocessing components.\n\nFor example, the IPU3 platform defines custom RAW10 format where 25\nsamples are packed in a tile of 32 bytes in size, to maximize the\nformat space efficiency and to align tiles to the size requirements of\nits ISP image processing components. The full line width in bytes\ncan be calculated as well with\n\n bytesPerLine = width * (32/25)\n-------------------------------------------------------------------------------\n\nI'm not sure if this could be a commit message or part of the\ndocumentation. It tries to express the same concepts that you\nexpressed but removes the pixel re-construction process from the\ndefinition, as the size of a 'group' (or 'tile') does not only depends\non that.\n\n\n> comment in the driver suggests that it should be 50 and 64,\n> respectively, this is an attribute of the driver, and not the format, so\n> this shall be set by the ipu3 pipeline handler.\n>\n> Signed-off-by: Paul Elder \n>\n> ---\n> Changes in v3:\n> - add planes\n> - redefine the parameters for the formats\n> - pixelsPerGroup is for whole format\n> - add verticalSubSampling per plane\n>\n> Changes in v2:\n> - add documentation for bytesPerGroup pixelsPerGroup\n> - fix wording in commit message\n> - bytes-per-line -> stride\n> - buffer size -> frame size\n> - changed MJPEG todo to allowing pipeline handlers to set parameters of\n> format infos\n> ---\n> include/libcamera/internal/formats.h | 11 ++-\n> src/libcamera/formats.cpp | 128 ++++++++++++++++++++++++++-\n> 2 files changed, 137 insertions(+), 2 deletions(-)\n>\n> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h\n> index f59ac8f..ebd256c 100644\n> --- a/include/libcamera/internal/formats.h\n> +++ b/include/libcamera/internal/formats.h\n> @@ -32,6 +32,12 @@ private:\n> \tstd::map> data_;\n> };\n>\n> +struct PixelFormatPlane\n> +{\n> +\tunsigned int bytesPerGroup;\n> +\tunsigned int verticalSubSampling;\n> +};\n> +\n> class PixelFormatInfo\n> {\n> public:\n> @@ -45,13 +51,16 @@ public:\n>\n> \tstatic const PixelFormatInfo &info(const PixelFormat &format);\n>\n> -\t/* \\todo Add support for non-contiguous memory planes */\n\nI wish we could remove this :)\n\n> \tconst char *name;\n> \tPixelFormat format;\n> \tV4L2PixelFormat v4l2Format;\n> \tunsigned int bitsPerPixel;\n> \tenum ColourEncoding colourEncoding;\n> \tbool packed;\n> +\n> +\tunsigned int pixelsPerGroup;\n> +\n> +\tstd::array planes;\n> };\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index d3b722c..6bdd28d 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -110,6 +110,22 @@ const std::map> &ImageFormats::data() const\n> \treturn data_;\n> }\n>\n> +/**\n> + * \\class PixelFormatPlane\n> + * \\brief Information about a single plane of a pixel format\n> + *\n> + * \\var PixelFormatPlane::bytesPerGroup\n> + * \\brief The number of bytes that a pixel group consumes\n> + *\n> + * \\sa PixelFormatInfo::pixelsPerGroup\n> + *\n> + * \\var PixelFormatPlane::verticalSubSampling\n> + * \\brief Vertical subsampling multiplier\n> + *\n> + * This value is the ratio between the number of rows of pixels in the frame\n> + * to the number of rows of pixels in the plane.\n> + */\n> +\n> /**\n> * \\class PixelFormatInfo\n> * \\brief Information about pixel formats\n> @@ -152,6 +168,26 @@ const std::map> &ImageFormats::data() const\n> * bytes. For instance, 12-bit Bayer data with two pixels stored in three bytes\n> * is packed, while the same data stored with 4 bits of padding in two bytes\n> * per pixel is not packed.\n> + *\n> + * \\var PixelFormatInfo::pixelsPerGroup\n> + * \\brief The number of pixels in a pixel group\n> + *\n> + * The minimum number of pixels (including padding) necessary in a row\n> + * when the frame has only one column of effective pixels\n> + *\n> + * A pixel group is defined as the minimum number of pixels (including padding)\n> + * necessary in a row when the image has only one column of effective pixels.\n> + * pixelsPerGroup refers to this value. bytesPerGroup, then, refers to the\n> + * number of bytes that a pixel group consumes. This definition of a pixel\n> + * group allows simple calculation of stride, as\n> + * ceil(width / pixelsPerGroup) * bytesPerGroup. These values are determined\n> + * only in terms of a row, and include bytes that are used in all planes (for\n> + * multiplanar formats).\n> + *\n> + * \\var PixelFormatInfo::planes\n> + * \\brief Information about pixels for each plane\n> + *\n> + * \\sa PixelFormatPlane\n> */\n>\n> /**\n> @@ -179,6 +215,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 24,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::RGB888, {\n> \t\t.name = \"RGB888\",\n> @@ -187,6 +225,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 24,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::ABGR8888, {\n> \t\t.name = \"ABGR8888\",\n> @@ -195,6 +235,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 32,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::ARGB8888, {\n> \t\t.name = \"ARGB8888\",\n> @@ -203,6 +245,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 32,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::BGRA8888, {\n> \t\t.name = \"BGRA8888\",\n> @@ -211,6 +255,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 32,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::RGBA8888, {\n> \t\t.name = \"RGBA8888\",\n> @@ -219,6 +265,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 32,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n>\n> \t/* YUV packed formats. */\n> @@ -229,6 +277,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::YVYU, {\n> \t\t.name = \"YVYU\",\n> @@ -237,6 +287,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::UYVY, {\n> \t\t.name = \"UYVY\",\n> @@ -245,6 +297,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::VYUY, {\n> \t\t.name = \"VYUY\",\n> @@ -253,6 +307,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n>\n> \t/* YUV planar formats. */\n> @@ -263,6 +319,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 2, 2 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV21, {\n> \t\t.name = \"NV21\",\n> @@ -271,6 +329,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 2, 2 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV16, {\n> \t\t.name = \"NV16\",\n> @@ -279,6 +339,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 2, 1 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV61, {\n> \t\t.name = \"NV61\",\n> @@ -287,6 +349,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 2, 1 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV24, {\n> \t\t.name = \"NV24\",\n> @@ -295,6 +359,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 24,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 1, 1 }, { 2, 1 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::NV42, {\n> \t\t.name = \"NV42\",\n> @@ -303,6 +369,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 24,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 1, 1 }, { 2, 1 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::YUV420, {\n> \t\t.name = \"YUV420\",\n> @@ -311,6 +379,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 1, 2 }, { 1, 2 } }},\n> \t} },\n> \t{ formats::YUV422, {\n> \t\t.name = \"YUV422\",\n> @@ -319,6 +389,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 1, 1 }, { 1, 1 } }},\n> \t} },\n>\n> \t/* Greyscale formats. */\n> @@ -329,6 +401,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n>\n> \t/* Bayer formats. */\n> @@ -339,6 +413,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG8, {\n> \t\t.name = \"SGBRG8\",\n> @@ -347,6 +423,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG8, {\n> \t\t.name = \"SGRBG8\",\n> @@ -355,6 +433,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB8, {\n> \t\t.name = \"SRGGB8\",\n> @@ -363,6 +443,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 8,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR10, {\n> \t\t.name = \"SBGGR10\",\n> @@ -371,6 +453,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG10, {\n> \t\t.name = \"SGBRG10\",\n> @@ -379,6 +463,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG10, {\n> \t\t.name = \"SGRBG10\",\n> @@ -387,6 +473,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB10, {\n> \t\t.name = \"SRGGB10\",\n> @@ -395,6 +483,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR10_CSI2P, {\n> \t\t.name = \"SBGGR10_CSI2P\",\n> @@ -403,6 +493,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 4,\n> +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG10_CSI2P, {\n> \t\t.name = \"SGBRG10_CSI2P\",\n> @@ -411,6 +503,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 4,\n> +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG10_CSI2P, {\n> \t\t.name = \"SGRBG10_CSI2P\",\n> @@ -419,6 +513,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 4,\n> +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB10_CSI2P, {\n> \t\t.name = \"SRGGB10_CSI2P\",\n> @@ -427,6 +523,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 4,\n> +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR12, {\n> \t\t.name = \"SBGGR12\",\n> @@ -435,6 +533,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG12, {\n> \t\t.name = \"SGBRG12\",\n> @@ -443,6 +543,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG12, {\n> \t\t.name = \"SGRBG12\",\n> @@ -451,6 +553,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB12, {\n> \t\t.name = \"SRGGB12\",\n> @@ -459,6 +563,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR12_CSI2P, {\n> \t\t.name = \"SBGGR12_CSI2P\",\n> @@ -467,6 +573,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG12_CSI2P, {\n> \t\t.name = \"SGBRG12_CSI2P\",\n> @@ -475,6 +583,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG12_CSI2P, {\n> \t\t.name = \"SGRBG12_CSI2P\",\n> @@ -483,6 +593,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB12_CSI2P, {\n> \t\t.name = \"SRGGB12_CSI2P\",\n> @@ -491,6 +603,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 12,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SBGGR10_IPU3, {\n> \t\t.name = \"SBGGR10_IPU3\",\n> @@ -499,6 +613,9 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t/* \\todo remember to double this in the ipu3 pipeline handler */\n> +\t\t.pixelsPerGroup = 25,\n> +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGBRG10_IPU3, {\n> \t\t.name = \"SGBRG10_IPU3\",\n> @@ -507,6 +624,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 25,\n> +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SGRBG10_IPU3, {\n> \t\t.name = \"SGRBG10_IPU3\",\n> @@ -515,6 +634,8 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 25,\n> +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> \t{ formats::SRGGB10_IPU3, {\n> \t\t.name = \"SRGGB10_IPU3\",\n> @@ -523,16 +644,21 @@ const std::map pixelFormatInfo{\n> \t\t.bitsPerPixel = 10,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> \t\t.packed = true,\n> +\t\t.pixelsPerGroup = 25,\n> +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n>\n> \t/* Compressed formats. */\n> +\t/* \\todo remember to fill this in from the pipeline handler. */\n> \t{ formats::MJPEG, {\n> \t\t.name = \"MJPEG\",\n> \t\t.format = formats::MJPEG,\n> \t\t.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> -\t\t.bitsPerPixel = 0,\n> +\t\t.bitsPerPixel = 16,\n> \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> \t\t.packed = false,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> };\n>\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 784FBBD792\n\tfor ;\n\tMon, 6 Jul 2020 14:10:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EBBC960E09;\n\tMon, 6 Jul 2020 16:10:19 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5560A603AA\n\tfor ;\n\tMon, 6 Jul 2020 16:10:19 +0200 (CEST)","from uno.localdomain (host-79-34-235-173.business.telecomitalia.it\n\t[79.34.235.173]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id D7029C0009;\n\tMon, 6 Jul 2020 14:10:17 +0000 (UTC)"],"X-Originating-IP":"79.34.235.173","Date":"Mon, 6 Jul 2020 16:13:51 +0200","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20200706141351.oxy7rzzblzw2qovo@uno.localdomain>","References":"<20200704133140.1738660-1-paul.elder@ideasonboard.com>\n\t<20200704133140.1738660-3-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200704133140.1738660-3-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 02/22] libcamera: formats: Add\n\tfields to info to ease calculating stride","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\" "}},{"id":11221,"web_url":"https://patchwork.libcamera.org/comment/11221/","msgid":"<20200706144627.GC19803@pendragon.ideasonboard.com>","date":"2020-07-06T14:46:27","subject":"Re: [libcamera-devel] [PATCH v3 02/22] libcamera: formats: Add\n\tfields to info to ease calculating stride","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Jul 06, 2020 at 04:13:51PM +0200, Jacopo Mondi wrote:\n> On Sat, Jul 04, 2020 at 10:31:20PM +0900, Paul Elder wrote:\n> > Packed formats make it difficult to calculate stride as well as\n> > frame size with the fields that PixelFormatInfo currently has.\n> > bitsPerPixel is defined as the average number of bits per pixel, and\n> > only counts effective bits, so it is not useful for calculating\n> > stride and frame size.\n> >\n> > To fix this, we introduce a concept of a \"pixel group\". The size of this\n> > group is defined as the minimum number of pixels (including padding)\n> > necessary in a row when the image has only one column of effective\n> > pixels. The pixel group has one more attribute, that is the \"bytes per\n> > group\". This determines how many bytes one pixel group consumes. These\n> > are the fields pixelsPerGroup and bytesPerGroup that are defined in this\n> > patch. Defining these two values makes it really simple to calculate\n> > bytes-per-line, as ceil(width / pixelsPerGroup) * bytesPerGroup, where\n> > width is measured in number of pixels. The ceiling accounts for padding.\n> \n> How does ceiling accounts for padding ? this depends on the format\n> defined byte alignement. There's no guarantee 'the next biggest\n> integer' is enough for that, am I wrong ?\n\nYes and no :-) There's an intrinsic alignment that this patch takes into\naccount. For instance SGRBG10_CSI2P has an intrinsic alignment of 5\nbytes, as it stores 4 pixels in 5 bytes. On top of that, there's a\ndevice-specific alignment, usually related to the implementation of the\nDMA engine. Some devices won't have additional alignment constraints,\nsome will. I've pointed this out to Paul before, we agreed that pipeline\nhandlers need to be ultimately responsible for aligning the stride.\nStill, PixelFormatInfo can help, computing the intrinsic alignment, and\nletting the pipeline handler apply further constraints. Paul told me he\nwas considering adding an alignment parameter to the\nPixelFormatInfo::stride() function (it should have a default value of\n1). In the vast majority of cases this should be enough for pipeline\nhandlers, and if they need to take more exotic constraints into account,\nthey can always do so (either on top of the value returned by stride(),\nor by computing the stride manually).\n\nWould that make sense for you ?\n\n> > Clearly, pixelsPerGroup must be constant for all planes in the format.\n> > The bytesPerGroup then, must be a per-plane attribute. There is one more\n> > field, verticalSubSampling, that is per-plane. This is simply a divider,\n> > to divide the number of rows of pixels by the sub-sampling value, to\n> > obtain the number of rows of pixels for the subsampled plane.\n> >\n> > For example, for something simple like BGR888, it is self-explanatory:\n> > the pixel group size is 1, and the bytes necessary is 3, and there is\n> > only one plane with no (= 1) vertical subsampling. For YUYV, the\n> > CbCr pair is shared between two pixels, so even if you have only one\n> > pixel, you would still need a padded second Y, therefore the pixel\n> \n> s/padded second Y/second sample ?\n> \n> > group size is 2, and bytes necessary is 4 (as opposed to 1 and 2). YUYV\n> \n> what do you mean with \"as opposed to\" ?\n> \n> > also has no vertical subsampling. NV12 has a pixel group size of 2\n> \n> Assuming I got this right, YUYV has a single plane, hence no\n> sub-sampling. Is it true that sub-sampling applies only to the\n> additional planes, and formats with a single plane always have 1 as\n> sub-sampling value ?\n\nI think this makes sense. We can of course change this later if there's\na need to, when we discover new formats that can't fit nicely into the\nformat info scheme, or just if we figure out we need more information\nfor existing formats than what is being proposed here.\n\n> > pixels, due to the CbCr plane. The bytes per group then, for both\n> > planes, is 2. The first plane has no vertical subsampling, but the\n> > second plane is subsampled by a factor of 2.\n> > The IPU3 formats are also self-explanatory, as they are single-planar,\n> > and have a pixel group size of 25, consuming 32 bytes. Although a\n> \n> The IPU3 formats pack pixels in 32 bytes as you said, but how does\n> this play with the definition of\n> \n> \"The size of [a pixel group] is defined as the minimum number of pixels\n> (including padding) necessary in a row when the image has only one column\n> of effective pixels\" ?\n> \n> I'm mostly missing what you mean by \"necessary\" here.\n\nI also had trouble with this. The idea if that if you have an image of\n1xn pixels (a single column of effective pixels), the IPU3 packed format\nwill still write it as 25 pixels in 32 bytes, with the other 24 pixels\nhaving unknown values (maybe they're set to 0x00, 0xff or a random\nvalue, I don't know what the hardware does). The group size is thus 25\npixels and 32 bytes. After understanding this and re-reading the\ndocumentation I found it quite clear, and wasn't sure how to express it\nin a better way, but given that both of us failed to get it in the first\nplace, an additional effort may be required.\n\n> I understand how tricky it is to find a way to express what you're\n> here describing, and I don't have much better wording for that.\n> \n> For other formats is clear: a pixel group is the minium number of\n> samples that are required to re-construct one pixel. Sticking to your\n> examples, for RGB888 a pixel group is 1 sample, of 24bits each.\n> For YUYV you need two samples to reconstruct a pixel, and each sample\n> is 16 bits in size. For formats like the IPU3 to reconstruct 1 pixel\n> you actually need a single sample of 30 bits each (those formats are\n> equivalent to S[GGBR]10 in sample size and sample ordering). The\n> requirement to pack 25 pixels in 32bytes is the due to how they pack\n> one 'tile' of pixels in a byte-aligned memory area.\n> \n> I know 'tile' has a precise meaing for, say, display formats which do\n> not apply in the context of video formats (for my understanding) but\n> would this be a better term to use in place of \"pixel group\" ?\n\nI'd rather avoid the word tile, exactly because of its meaning in\ndisplay contexts.\n\nI initially thought we could define the pixel group as the minimum\nnumber of pixels that can be stored in an integer number of bytes, but\nthat's not true for the IPU3 packed formats, as it stores 4 pixels in 5\nbytes but adds an additional pixel stored in 2 bytes after 24 pixels.\nMaybe we could somehow define the pixel group based on a minimal number\nof pixels stored in an integer number of bytes with a repeating pattern\n? A better wording is needed, but repeating pattern / periodicity may be\nthe key.\n\n> I would leave out any attempt to try to define the number of samples\n> per tile in terms of re-constructed pixels, as for the IPU3 format,\n> that does not apply well.\n> \n> In example:\n> \n> -------------------------------------------------------------------------------\n> To fix this, we introduce a concept of a \"memory tile\". A tile is\n> composed by a number of samples (which are combined to re-construct\n> pixels color informations) defined by the image format, and a\n> tile size in bytes which includes any necessay padding bits to align\n> the effective color information samples to a boundary defined\n> by the format.\n> \n> The number of samples per tile is reported by the samplesPerTile\n> attribute, while the overall tile size by the bytesPerTile one.\n> \n> By using those two format-defined values, calculating the number of\n> bytes per line becomes trival as in:\n> \n> bytesPerLine = (width / samplesPerTile) * bytesPerTile\n> \n> For RAW formats such as RAW888 the number of samples is 3, with a size\n> of 1 bytes each. Each tile is then 3 bytes in size, and the number of\n> bytes per line is simply the number of pixels\n> \n> bytesPerLine = (width / 3) * 3 = width\n> \n> In RAW10 formats each tile is composed of 3 samples (one per colour\n> information) and each sample is composed by 10-bits of effective color\n> information and 6 padding bits. A tile has a bytes size of 6 and the\n> line size is calculated as\n> \n> bytesPerLine = (width / 3) * 6 = 2width\n> \n> For YUYV formats, each tile has a size of two samples, with each\n> sample being 2 bytes in size. The tile size is then 4 bytes and the\n> line width is calculated as\n> \n> bytesPerLine = (width / 2) * 4 = 2width\n> \n> Other formats, mostly the platform specific ones, define the number of\n> samples per tile in order to maximize the format space efficiency or\n> to align samples to boundaries defined by the platform image\n> processing components.\n> \n> For example, the IPU3 platform defines custom RAW10 format where 25\n> samples are packed in a tile of 32 bytes in size, to maximize the\n> format space efficiency and to align tiles to the size requirements of\n> its ISP image processing components. The full line width in bytes\n> can be calculated as well with\n> \n> bytesPerLine = width * (32/25)\n> -------------------------------------------------------------------------------\n> \n> I'm not sure if this could be a commit message or part of the\n> documentation. It tries to express the same concepts that you\n> expressed but removes the pixel re-construction process from the\n> definition, as the size of a 'group' (or 'tile') does not only depends\n> on that.\n\nA couple of examples in the documentation may be useful (especially to\nexplain the weird IPU3 constraints, I think that's the only format we\nhave where the periodicity isn't identical to the smallest number of\npixels stored in an integer number of bytes).\n\n> > comment in the driver suggests that it should be 50 and 64,\n> > respectively, this is an attribute of the driver, and not the format, so\n> > this shall be set by the ipu3 pipeline handler.\n> >\n> > Signed-off-by: Paul Elder \n> >\n> > ---\n> > Changes in v3:\n> > - add planes\n> > - redefine the parameters for the formats\n> > - pixelsPerGroup is for whole format\n> > - add verticalSubSampling per plane\n> >\n> > Changes in v2:\n> > - add documentation for bytesPerGroup pixelsPerGroup\n> > - fix wording in commit message\n> > - bytes-per-line -> stride\n> > - buffer size -> frame size\n> > - changed MJPEG todo to allowing pipeline handlers to set parameters of\n> > format infos\n> > ---\n> > include/libcamera/internal/formats.h | 11 ++-\n> > src/libcamera/formats.cpp | 128 ++++++++++++++++++++++++++-\n> > 2 files changed, 137 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h\n> > index f59ac8f..ebd256c 100644\n> > --- a/include/libcamera/internal/formats.h\n> > +++ b/include/libcamera/internal/formats.h\n> > @@ -32,6 +32,12 @@ private:\n> > \tstd::map> data_;\n> > };\n> >\n> > +struct PixelFormatPlane\n> > +{\n> > +\tunsigned int bytesPerGroup;\n> > +\tunsigned int verticalSubSampling;\n> > +};\n> > +\n> > class PixelFormatInfo\n> > {\n> > public:\n> > @@ -45,13 +51,16 @@ public:\n> >\n> > \tstatic const PixelFormatInfo &info(const PixelFormat &format);\n> >\n> > -\t/* \\todo Add support for non-contiguous memory planes */\n> \n> I wish we could remove this :)\n\nWe will, we will :-)\n\n> > \tconst char *name;\n> > \tPixelFormat format;\n> > \tV4L2PixelFormat v4l2Format;\n> > \tunsigned int bitsPerPixel;\n> > \tenum ColourEncoding colourEncoding;\n> > \tbool packed;\n> > +\n> > +\tunsigned int pixelsPerGroup;\n> > +\n> > +\tstd::array planes;\n> > };\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > index d3b722c..6bdd28d 100644\n> > --- a/src/libcamera/formats.cpp\n> > +++ b/src/libcamera/formats.cpp\n> > @@ -110,6 +110,22 @@ const std::map> &ImageFormats::data() const\n> > \treturn data_;\n> > }\n> >\n> > +/**\n> > + * \\class PixelFormatPlane\n> > + * \\brief Information about a single plane of a pixel format\n> > + *\n> > + * \\var PixelFormatPlane::bytesPerGroup\n> > + * \\brief The number of bytes that a pixel group consumes\n> > + *\n> > + * \\sa PixelFormatInfo::pixelsPerGroup\n> > + *\n> > + * \\var PixelFormatPlane::verticalSubSampling\n> > + * \\brief Vertical subsampling multiplier\n> > + *\n> > + * This value is the ratio between the number of rows of pixels in the frame\n> > + * to the number of rows of pixels in the plane.\n> > + */\n> > +\n> > /**\n> > * \\class PixelFormatInfo\n> > * \\brief Information about pixel formats\n> > @@ -152,6 +168,26 @@ const std::map> &ImageFormats::data() const\n> > * bytes. For instance, 12-bit Bayer data with two pixels stored in three bytes\n> > * is packed, while the same data stored with 4 bits of padding in two bytes\n> > * per pixel is not packed.\n> > + *\n> > + * \\var PixelFormatInfo::pixelsPerGroup\n> > + * \\brief The number of pixels in a pixel group\n> > + *\n> > + * The minimum number of pixels (including padding) necessary in a row\n> > + * when the frame has only one column of effective pixels\n> > + *\n> > + * A pixel group is defined as the minimum number of pixels (including padding)\n> > + * necessary in a row when the image has only one column of effective pixels.\n> > + * pixelsPerGroup refers to this value. bytesPerGroup, then, refers to the\n> > + * number of bytes that a pixel group consumes. This definition of a pixel\n> > + * group allows simple calculation of stride, as\n> > + * ceil(width / pixelsPerGroup) * bytesPerGroup. These values are determined\n> > + * only in terms of a row, and include bytes that are used in all planes (for\n> > + * multiplanar formats).\n> > + *\n> > + * \\var PixelFormatInfo::planes\n> > + * \\brief Information about pixels for each plane\n> > + *\n> > + * \\sa PixelFormatPlane\n> > */\n> >\n> > /**\n> > @@ -179,6 +215,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 24,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::RGB888, {\n> > \t\t.name = \"RGB888\",\n> > @@ -187,6 +225,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 24,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::ABGR8888, {\n> > \t\t.name = \"ABGR8888\",\n> > @@ -195,6 +235,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 32,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::ARGB8888, {\n> > \t\t.name = \"ARGB8888\",\n> > @@ -203,6 +245,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 32,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::BGRA8888, {\n> > \t\t.name = \"BGRA8888\",\n> > @@ -211,6 +255,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 32,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::RGBA8888, {\n> > \t\t.name = \"RGBA8888\",\n> > @@ -219,6 +265,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 32,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRGB,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> >\n> > \t/* YUV packed formats. */\n> > @@ -229,6 +277,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 16,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::YVYU, {\n> > \t\t.name = \"YVYU\",\n> > @@ -237,6 +287,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 16,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::UYVY, {\n> > \t\t.name = \"UYVY\",\n> > @@ -245,6 +297,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 16,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::VYUY, {\n> > \t\t.name = \"VYUY\",\n> > @@ -253,6 +307,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 16,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> >\n> > \t/* YUV planar formats. */\n> > @@ -263,6 +319,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 2, 2 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::NV21, {\n> > \t\t.name = \"NV21\",\n> > @@ -271,6 +329,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 2, 2 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::NV16, {\n> > \t\t.name = \"NV16\",\n> > @@ -279,6 +339,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 16,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 2, 1 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::NV61, {\n> > \t\t.name = \"NV61\",\n> > @@ -287,6 +349,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 16,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 2, 1 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::NV24, {\n> > \t\t.name = \"NV24\",\n> > @@ -295,6 +359,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 24,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 1, 1 }, { 2, 1 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::NV42, {\n> > \t\t.name = \"NV42\",\n> > @@ -303,6 +369,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 24,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 1, 1 }, { 2, 1 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::YUV420, {\n> > \t\t.name = \"YUV420\",\n> > @@ -311,6 +379,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 1, 2 }, { 1, 2 } }},\n> > \t} },\n> > \t{ formats::YUV422, {\n> > \t\t.name = \"YUV422\",\n> > @@ -319,6 +389,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 16,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 1, 1 }, { 1, 1 } }},\n> > \t} },\n> >\n> > \t/* Greyscale formats. */\n> > @@ -329,6 +401,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 8,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> >\n> > \t/* Bayer formats. */\n> > @@ -339,6 +413,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 8,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGBRG8, {\n> > \t\t.name = \"SGBRG8\",\n> > @@ -347,6 +423,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 8,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGRBG8, {\n> > \t\t.name = \"SGRBG8\",\n> > @@ -355,6 +433,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 8,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SRGGB8, {\n> > \t\t.name = \"SRGGB8\",\n> > @@ -363,6 +443,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 8,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SBGGR10, {\n> > \t\t.name = \"SBGGR10\",\n> > @@ -371,6 +453,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGBRG10, {\n> > \t\t.name = \"SGBRG10\",\n> > @@ -379,6 +463,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGRBG10, {\n> > \t\t.name = \"SGRBG10\",\n> > @@ -387,6 +473,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SRGGB10, {\n> > \t\t.name = \"SRGGB10\",\n> > @@ -395,6 +483,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SBGGR10_CSI2P, {\n> > \t\t.name = \"SBGGR10_CSI2P\",\n> > @@ -403,6 +493,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 4,\n> > +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGBRG10_CSI2P, {\n> > \t\t.name = \"SGBRG10_CSI2P\",\n> > @@ -411,6 +503,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 4,\n> > +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGRBG10_CSI2P, {\n> > \t\t.name = \"SGRBG10_CSI2P\",\n> > @@ -419,6 +513,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 4,\n> > +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SRGGB10_CSI2P, {\n> > \t\t.name = \"SRGGB10_CSI2P\",\n> > @@ -427,6 +523,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 4,\n> > +\t\t.planes = {{ { 5, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SBGGR12, {\n> > \t\t.name = \"SBGGR12\",\n> > @@ -435,6 +533,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGBRG12, {\n> > \t\t.name = \"SGBRG12\",\n> > @@ -443,6 +543,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGRBG12, {\n> > \t\t.name = \"SGRBG12\",\n> > @@ -451,6 +553,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SRGGB12, {\n> > \t\t.name = \"SRGGB12\",\n> > @@ -459,6 +563,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SBGGR12_CSI2P, {\n> > \t\t.name = \"SBGGR12_CSI2P\",\n> > @@ -467,6 +573,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGBRG12_CSI2P, {\n> > \t\t.name = \"SGBRG12_CSI2P\",\n> > @@ -475,6 +583,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGRBG12_CSI2P, {\n> > \t\t.name = \"SGRBG12_CSI2P\",\n> > @@ -483,6 +593,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SRGGB12_CSI2P, {\n> > \t\t.name = \"SRGGB12_CSI2P\",\n> > @@ -491,6 +603,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 12,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 3, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SBGGR10_IPU3, {\n> > \t\t.name = \"SBGGR10_IPU3\",\n> > @@ -499,6 +613,9 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t/* \\todo remember to double this in the ipu3 pipeline handler */\n> > +\t\t.pixelsPerGroup = 25,\n> > +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGBRG10_IPU3, {\n> > \t\t.name = \"SGBRG10_IPU3\",\n> > @@ -507,6 +624,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 25,\n> > +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SGRBG10_IPU3, {\n> > \t\t.name = \"SGRBG10_IPU3\",\n> > @@ -515,6 +634,8 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 25,\n> > +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > \t{ formats::SRGGB10_IPU3, {\n> > \t\t.name = \"SRGGB10_IPU3\",\n> > @@ -523,16 +644,21 @@ const std::map pixelFormatInfo{\n> > \t\t.bitsPerPixel = 10,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > \t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 25,\n> > +\t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> >\n> > \t/* Compressed formats. */\n> > +\t/* \\todo remember to fill this in from the pipeline handler. */\n> > \t{ formats::MJPEG, {\n> > \t\t.name = \"MJPEG\",\n> > \t\t.format = formats::MJPEG,\n> > \t\t.v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > -\t\t.bitsPerPixel = 0,\n> > +\t\t.bitsPerPixel = 16,\n> > \t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > \t\t.packed = false,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 1, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\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 0255ABD792\n\tfor ;\n\tMon, 6 Jul 2020 14:46:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E5CD60E09;\n\tMon, 6 Jul 2020 16:46:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 026BC603AA\n\tfor ;\n\tMon, 6 Jul 2020 16:46:31 +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 56E11A16;\n\tMon, 6 Jul 2020 16:46:31 +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=\"dWhbfAk1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594046791;\n\tbh=MXgOaehHEoZEpPFpGWh0w43rE9jjofLw9HtkCgqn+vE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dWhbfAk1xNk1QBvzg9SE5uE22mHShY8c7o6aSqguV1WcV6GTsTmmZds+zeA/4N1LH\n\tqO4CjOskonZ63knElgvtyTD5TBoljnH+dsDjpcp1/f358jXpe7ckeyYr8T9eZzGVSe\n\tymtyomaeEE4H8wSpX+cxzu4MD7gyjb7Ej5e1HBxQ=","Date":"Mon, 6 Jul 2020 17:46:27 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200706144627.GC19803@pendragon.ideasonboard.com>","References":"<20200704133140.1738660-1-paul.elder@ideasonboard.com>\n\t<20200704133140.1738660-3-paul.elder@ideasonboard.com>\n\t<20200706141351.oxy7rzzblzw2qovo@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200706141351.oxy7rzzblzw2qovo@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 02/22] libcamera: formats: Add\n\tfields to info to ease calculating stride","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\" "}}]