media: Introduce pixel format helpers
Subsystems (3)
cover media: Introduce pixel format helpers
Following the "Add MPEG-2 decoding to Rockchip VPU"[1], and given we want to start using the pixel format helpers rather soon, I've decided to submit them separatedly.
Aside from documenting the block_w and block_h, as suggested by Emil Velikov, these two patches are unmodified compared to v2 [1].
This series contains the minimum patches to introduce the pixel format helpers, it currently will have no users, but we expect users to start popping-up soon.
very shortly [2].
To avoid confusing the patches here with those on v2 [1], I'm marking them as v3.
Thanks!
[1] https://patchwork.kernel.org/cover/10838327/
[2] https://patchwork.kernel.org/patch/10851639/
Ezequiel Garcia (2):
rockchip/vpu: Rename pixel format helpers
media: Introduce helpers to fill pixel format structs
drivers/media/v4l2-core/v4l2-common.c | 164 ++++++++++++++++++ .../media/rockchip/vpu/rockchip_vpu_enc.c | 12 +- include/media/v4l2-common.h | 33 ++++ 3 files changed, 203 insertions(+), 6 deletions(-)
Signature · 1 line
2.20.1
PATCH v3 1/2 rockchip/vpu: Rename pixel format helpers
The rockchip VPU driver uses generic names for its pixel format helpers. We want to use the same names for generic versions of these helpers, so rename the rockchip ones.
The driver will be switched to the generic helpers later.
--- .../staging/media/rockchip/vpu/rockchip_vpu_enc.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c +6 -6
@@ -41,7 +41,7 @@* @is_compressed: Is it a compressed format? * @multiplanar: Is it a multiplanar variant format? (e.g. NV12M) */ -struct v4l2_format_info { +struct rockchip_vpu_v4l2_format_info {u32 format;u32 header_size;u8 num_planes;
@@ -52,10 +52,10 @@ struct v4l2_format_info {u8 multiplanar; }; -static const struct v4l2_format_info * -v4l2_format_info (u32 format) +static const struct rockchip_vpu_v4l2_format_info * +rockchip_vpu_v4l2_format_info (u32 format) { -static const struct v4l2_format_info formats[] = { +static const struct rockchip_vpu_v4l2_format_info formats[] = { { .format =V4L2_PIX_FMT_YUV420M , .num_planes =3 , .cpp = {1 ,1 ,1 }, .hsub =2 , .vsub =2 , .multiplanar =1 }, { .format =V4L2_PIX_FMT_NV12M , .num_planes =2 , .cpp = {1 ,2 ,0 }, .hsub =2 , .vsub =2 , .multiplanar =1 }, { .format =V4L2_PIX_FMT_YUYV , .num_planes =1 , .cpp = {2 ,0 ,0 }, .hsub =2 , .vsub =1 },
@@ -76,11 +76,11 @@ static void fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, int pixelformat, int width, int height) { - const struct v4l2_format_info *info; + const struct rockchip_vpu_v4l2_format_info *info;struct v4l2_plane_pix_format *plane;int i; - info =v4l2_format_info (pixelformat); + info =rockchip_vpu_v4l2_format_info (pixelformat);if (!info)return ;
Signature · 1 line
2.20.1
PATCH v3 2/2 media: Introduce helpers to fill pixel format structs
In reply to Ezequiel GarciaAdd two new API helpers, v4l2_fill_pixfmt and v4l2_fill_pixfmt_mp, to be used by drivers to calculate plane sizes and bytes per lines.
Note that driver-specific padding and alignment are not taken into account, and must be done by drivers using this API.
--- drivers/media/v4l2-core/v4l2-common.c | 164 ++++++++++++++++++++++++++ include/media/v4l2-common.h | 33 ++++++ 2 files changed, 197 insertions(+)
drivers/media/v4l2-core/v4l2-common.c +164 -0
@@ -445,3 +445,167 @@ int v4l2_s_parm_cap(struct video_device *vdev,return ret; }EXPORT_SYMBOL_GPL (v4l2_s_parm_cap); + +const struct v4l2_format_info *v4l2_format_info (u32 format) +{ +static const struct v4l2_format_info formats[] = { +/* RGB formats */ + { .format =V4L2_PIX_FMT_BGR24 , .mem_planes =1 , .comp_planes =1 , .bpp = {3 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_RGB24 , .mem_planes =1 , .comp_planes =1 , .bpp = {3 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_HSV24 , .mem_planes =1 , .comp_planes =1 , .bpp = {3 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_BGR32 , .mem_planes =1 , .comp_planes =1 , .bpp = {4 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_XBGR32 , .mem_planes =1 , .comp_planes =1 , .bpp = {4 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_RGB32 , .mem_planes =1 , .comp_planes =1 , .bpp = {4 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_XRGB32 , .mem_planes =1 , .comp_planes =1 , .bpp = {4 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_HSV32 , .mem_planes =1 , .comp_planes =1 , .bpp = {4 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_ARGB32 , .mem_planes =1 , .comp_planes =1 , .bpp = {4 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_ABGR32 , .mem_planes =1 , .comp_planes =1 , .bpp = {4 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_GREY , .mem_planes =1 , .comp_planes =1 , .bpp = {1 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + +/* YUV packed formats */ + { .format =V4L2_PIX_FMT_YUYV , .mem_planes =1 , .comp_planes =1 , .bpp = {2 ,0 ,0 ,0 }, .hdiv =2 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_YVYU , .mem_planes =1 , .comp_planes =1 , .bpp = {2 ,0 ,0 ,0 }, .hdiv =2 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_UYVY , .mem_planes =1 , .comp_planes =1 , .bpp = {2 ,0 ,0 ,0 }, .hdiv =2 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_VYUY , .mem_planes =1 , .comp_planes =1 , .bpp = {2 ,0 ,0 ,0 }, .hdiv =2 , .vdiv =1 }, + +/* YUV planar formats */ + { .format =V4L2_PIX_FMT_NV12 , .mem_planes =1 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =2 , .vdiv =2 }, + { .format =V4L2_PIX_FMT_NV21 , .mem_planes =1 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =2 , .vdiv =2 }, + { .format =V4L2_PIX_FMT_NV16 , .mem_planes =1 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =2 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_NV61 , .mem_planes =1 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =2 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_NV24 , .mem_planes =1 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_NV42 , .mem_planes =1 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =1 , .vdiv =1 }, + + { .format =V4L2_PIX_FMT_YUV410 , .mem_planes =1 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =4 , .vdiv =4 }, + { .format =V4L2_PIX_FMT_YVU410 , .mem_planes =1 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =4 , .vdiv =4 }, + { .format =V4L2_PIX_FMT_YUV411P , .mem_planes =1 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =4 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_YUV420 , .mem_planes =1 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =2 , .vdiv =2 }, + { .format =V4L2_PIX_FMT_YVU420 , .mem_planes =1 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =2 , .vdiv =2 }, + { .format =V4L2_PIX_FMT_YUV422P , .mem_planes =1 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =2 , .vdiv =1 }, + +/* YUV planar formats, non contiguous variant */ + { .format =V4L2_PIX_FMT_YUV420M , .mem_planes =3 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =2 , .vdiv =2 }, + { .format =V4L2_PIX_FMT_YVU420M , .mem_planes =3 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =2 , .vdiv =2 }, + { .format =V4L2_PIX_FMT_YUV422M , .mem_planes =3 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =2 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_YVU422M , .mem_planes =3 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =2 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_YUV444M , .mem_planes =3 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =1 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_YVU444M , .mem_planes =3 , .comp_planes =3 , .bpp = {1 ,1 ,1 ,0 }, .hdiv =1 , .vdiv =1 }, + + { .format =V4L2_PIX_FMT_NV12M , .mem_planes =2 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =2 , .vdiv =2 }, + { .format =V4L2_PIX_FMT_NV21M , .mem_planes =2 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =2 , .vdiv =2 }, + { .format =V4L2_PIX_FMT_NV16M , .mem_planes =2 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =2 , .vdiv =1 }, + { .format =V4L2_PIX_FMT_NV61M , .mem_planes =2 , .comp_planes =2 , .bpp = {1 ,2 ,0 ,0 }, .hdiv =2 , .vdiv =1 }, + }; +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (formats); ++i) +if (formats[i].format == format) +return &formats[i]; +return NULL ; +} +EXPORT_SYMBOL (v4l2_format_info); + +static inline unsigned int v4l2_format_block_width (const struct v4l2_format_info *info,int plane) +{ +if (!info->block_w[plane]) +return 1 ; +return info->block_w[plane]; +} + +static inline unsigned int v4l2_format_block_height (const struct v4l2_format_info *info,int plane) +{ +if (!info->block_h[plane]) +return 1 ; +return info->block_h[plane]; +} + +int v4l2_fill_pixfmt_mp (struct v4l2_pix_format_mplane *pixfmt, +int pixelformat,int width,int height) +{ +const struct v4l2_format_info *info; +struct v4l2_plane_pix_format *plane; +int i; + + info =v4l2_format_info (pixelformat); +if (!info) +return -EINVAL ; + + pixfmt->width = width; + pixfmt->height = height; + pixfmt->pixelformat = pixelformat; + pixfmt->num_planes = info->mem_planes; + +if (info->mem_planes ==1 ) { + plane = &pixfmt->plane_fmt[0 ]; + plane->bytesperline =ALIGN (width,v4l2_format_block_width (info,0 )) * info->bpp[0 ]; + plane->sizeimage =0 ; + +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height; + + aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + plane->sizeimage += info->bpp[i] * +DIV_ROUND_UP (aligned_width, hdiv) * +DIV_ROUND_UP (aligned_height, vdiv); + } + }else { +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height; + + aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + plane = &pixfmt->plane_fmt[i]; + plane->bytesperline = + info->bpp[i] *DIV_ROUND_UP (aligned_width, hdiv); + plane->sizeimage = + plane->bytesperline *DIV_ROUND_UP (aligned_height, vdiv); + } + } +return 0 ; +} +EXPORT_SYMBOL_GPL (v4l2_fill_pixfmt_mp); + +int v4l2_fill_pixfmt (struct v4l2_pix_format *pixfmt,int pixelformat,int width,int height) +{ +const struct v4l2_format_info *info; +int i; + + info =v4l2_format_info (pixelformat); +if (!info) +return -EINVAL ; + +/* Single planar API cannot be used for multi plane formats. */ +if (info->mem_planes >1 ) +return -EINVAL ; + + pixfmt->width = width; + pixfmt->height = height; + pixfmt->pixelformat = pixelformat; + pixfmt->bytesperline =ALIGN (width,v4l2_format_block_width (info,0 )) * info->bpp[0 ]; + pixfmt->sizeimage =0 ; + +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height; + + aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + pixfmt->sizeimage += info->bpp[i] * +DIV_ROUND_UP (aligned_width, hdiv) * +DIV_ROUND_UP (aligned_height, vdiv); + } +return 0 ; +} +EXPORT_SYMBOL_GPL (v4l2_fill_pixfmt);
include/media/v4l2-common.h +33 -0
@@ -392,4 +392,37 @@ int v4l2_s_parm_cap(struct video_device *vdev, ((u64)(a).numerator * (b).denominator OP \ (u64)(b).numerator * (a).denominator) +/* ------------------------------------------------------------------------- */ + +/* Pixel format and FourCC helpers */ + +/** +* struct v4l2_format_info - information about a V4L2 format +* @format: 4CC format identifier (V4L2_PIX_FMT_*) +* @mem_planes: Number of memory planes, which includes the alpha plane (1 to 4). +* @comp_planes: Number of component planes, which includes the alpha plane (1 to 4). +* @bpp: Array of per-plane bytes per pixel +* @hdiv: Horizontal chroma subsampling factor +* @vdiv: Vertical chroma subsampling factor +* @block_w: Per-plane macroblock pixel width (optional) +* @block_h: Per-plane macroblock pixel height (optional) + */ +struct v4l2_format_info { +u32 format; +u8 mem_planes; +u8 comp_planes; +u8 bpp[4 ]; +u8 hdiv; +u8 vdiv; +u8 block_w[4 ]; +u8 block_h[4 ]; +}; + +const struct v4l2_format_info *v4l2_format_info (u32 format); + +int v4l2_fill_pixfmt (struct v4l2_pix_format *pixfmt,int pixelformat, +int width,int height); +int v4l2_fill_pixfmt_mp (struct v4l2_pix_format_mplane *pixfmt,int pixelformat, +int width,int height); +#endif /* V4L2_COMMON_H_ */
Signature · 1 line
2.20.1
Re: [PATCH v3 2/2] media: Introduce helpers to fill pixel format structs
Hi Ezequiel,
this is very nice, thank you!
Quoted 17 lines · Ezequiel Garcia
Add two new API helpers,
v4l2_fill_pixfmtandv4l2_fill_pixfmt_mp, to be used by drivers to calculate plane sizes and bytes per lines.Note that driver-specific padding and alignment are not taken into account, and must be done by drivers using this API.
Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/media/v4l2-core/v4l2-common.c | 164 ++++++++++++++++++++++++++ include/media/v4l2-common.h | 33 ++++++ 2 files changed, 197 insertions(+)drivers/media/v4l2-core/v4l2-common.c +0
-0
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 453
Quoted 9 lines
drivers/media/v4l2-core/v4l2-common.c +6
-0@@ -445,3 +445,167 @@ int v4l2_s_parm_cap(struct video_device *vdev,return ret; }EXPORT_SYMBOL_GPL (v4l2_s_parm_cap); + +const struct v4l2_format_info *v4l2_format_info (u32 format) +{ +static const struct v4l2_format_info formats[] = { +/* RGB formats */ + { .format =V4L2_PIX_FMT_BGR24 , .mem_planes =1 , .comp_planes =1 , .bpp = {3 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 },
How would you feel using macros here, this would help sticking to 80-or-so columns, as otherwise this table is pretty wide.
I've sketched out something quick:
#define V4L2_FMT_INFO_BPP(p1_, p2_, p3_, p4_) { p1_, p2_, p3_, p4_ }
#define V4L2_FMT_INFO(pf_, mpl_, cpl_, bpp_, hd_, vd_) \
{ \
.format = V4L2_PIX_FMT_##pf_, \
.mem_planes = mpl_, \
.comp_planes = cpl_, \
.bpp = bpp_, \
.hdiv = hd_, \
.vdiv = vd_ \
}const struct v4l2_format_info *v4l2_format_info(u32 format)
{
static const struct v4l2_format_info formats[] = {
/* RGB formats */
V4L2_FMT_INFO(BGR24, 1, 1, V4L2_FMT_INFO_BPP(3, 0, 0, 0), 1, 1),
...
}Not sure you might like it or not.
Quoted 35 lines
drivers/media/v4l2-core/v4l2-common.c +35
-0+ { .format = V4L2_PIX_FMT_RGB24, .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_HSV24, .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_BGR32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_XBGR32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_RGB32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_XRGB32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_HSV32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_ARGB32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_ABGR32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_GREY, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + + /* YUV packed formats */ + { .format = V4L2_PIX_FMT_YUYV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YVYU, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_UYVY, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_VYUY, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + + /* YUV planar formats */ + { .format = V4L2_PIX_FMT_NV12, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV21, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV16, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV61, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV24, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV42, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + + { .format = V4L2_PIX_FMT_YUV410, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 4 }, + { .format = V4L2_PIX_FMT_YVU410, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 4 }, + { .format = V4L2_PIX_FMT_YUV411P, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YUV420, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YVU420, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YUV422P, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 }, + + /* YUV planar formats, non contiguous variant */ + { .format = V4L2_PIX_FMT_YUV420M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YVU420M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 },
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 498
Quoted 10 lines
drivers/media/v4l2-core/v4l2-common.c +10
-0+ { .format = V4L2_PIX_FMT_YUV422M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YVU422M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YUV444M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YVU444M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 1, .vdiv = 1 }, + + { .format = V4L2_PIX_FMT_NV12M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV21M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV16M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV61M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + };
Can I ask why you define the array inside this function and not as a static array with file scope? Is this something trivial I am missing? It will be placed in the same data segment anyhow, is this to reduce the probability of an unlikely name clash within the file? Just curious..
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 503
Quoted 5 lines
drivers/media/v4l2-core/v4l2-common.c +5
-0+unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (formats); ++i) +if (formats[i].format == format) +return &formats[i];
Nit: empty line maybe
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 511
Quoted 8 lines
drivers/media/v4l2-core/v4l2-common.c +8
-0+return NULL ; +} +EXPORT_SYMBOL (v4l2_format_info); + +static inline unsigned int v4l2_format_block_width (const struct v4l2_format_info *info,int plane) +{ +if (!info->block_w[plane]) +return 1 ;
Nit: empty line, or just
return info->block_w[plane] ? info->block_w[plane] : 1;
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 515
Quoted 6 lines
drivers/media/v4l2-core/v4l2-common.c +4
-0+return info->block_w[plane]; +} + +static inline unsigned int v4l2_format_block_height (const struct v4l2_format_info *info,int plane)
You don't like 80 columns that much, right :p ?
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 518
Quoted 3 lines
drivers/media/v4l2-core/v4l2-common.c +3
-0+{ +if (!info->block_h[plane]) +return 1 ;
Same here
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 523
Quoted 5 lines
drivers/media/v4l2-core/v4l2-common.c +5
-0+return info->block_h[plane]; +} + +int v4l2_fill_pixfmt_mp (struct v4l2_pix_format_mplane *pixfmt, +int pixelformat,int width,int height)
Bad alignment to open parenthesis
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 527
Quoted 4 lines
drivers/media/v4l2-core/v4l2-common.c +4
-0+{ +const struct v4l2_format_info *info; +struct v4l2_plane_pix_format *plane; +int i;
unsigned int i; ?
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 540
Quoted 13 lines
drivers/media/v4l2-core/v4l2-common.c +13
-0+ + info =v4l2_format_info (pixelformat); +if (!info) +return -EINVAL ; + + pixfmt->width = width; + pixfmt->height = height; + pixfmt->pixelformat = pixelformat; + pixfmt->num_planes = info->mem_planes; + +if (info->mem_planes ==1 ) { + plane = &pixfmt->plane_fmt[0 ]; + plane->bytesperline =ALIGN (width,v4l2_format_block_width (info,0 )) * info->bpp[0 ];
80 cols :'(
Quoted 5 lines
drivers/media/v4l2-core/v4l2-common.c +5
-0+ plane->sizeimage =0 ; + +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv;
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 555
Quoted 10 lines
drivers/media/v4l2-core/v4l2-common.c +10
-0+unsigned int aligned_width; +unsigned int aligned_height; + + aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + plane->sizeimage += info->bpp[i] * +DIV_ROUND_UP (aligned_width, hdiv) * +DIV_ROUND_UP (aligned_height, vdiv); + }
I was initially puzzled by some of your definitions, say NV12, where you mark the hdiv factor for the second plane to 2, while it is actually only vertically subsampled. But now I see what you've done for NV12 and this works for all the other formats I have verified (NV12, NV16, YUYV packed, and YVV4xx). Nice :)
Nit: you can save the below else branch indentation if you return here.
Quoted 7 lines
drivers/media/v4l2-core/v4l2-common.c +7
-0+ }else { +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height; +
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 572
Quoted 10 lines
drivers/media/v4l2-core/v4l2-common.c +10
-0+ aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + plane = &pixfmt->plane_fmt[i]; + plane->bytesperline = + info->bpp[i] *DIV_ROUND_UP (aligned_width, hdiv); + plane->sizeimage = + plane->bytesperline *DIV_ROUND_UP (aligned_height, vdiv); + } + }
Empty line maybe?
Quoted 28 lines
drivers/media/v4l2-core/v4l2-common.c +28
-0+return 0 ; +} +EXPORT_SYMBOL_GPL (v4l2_fill_pixfmt_mp); + +int v4l2_fill_pixfmt (struct v4l2_pix_format *pixfmt,int pixelformat,int width,int height) +{ +const struct v4l2_format_info *info; +int i; + + info =v4l2_format_info (pixelformat); +if (!info) +return -EINVAL ; + +/* Single planar API cannot be used for multi plane formats. */ +if (info->mem_planes >1 ) +return -EINVAL ; + + pixfmt->width = width; + pixfmt->height = height; + pixfmt->pixelformat = pixelformat; + pixfmt->bytesperline =ALIGN (width,v4l2_format_block_width (info,0 )) * info->bpp[0 ]; + pixfmt->sizeimage =0 ; + +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height;
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 610
Quoted 10 lines
drivers/media/v4l2-core/v4l2-common.c +10
-0+ + aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + pixfmt->sizeimage += info->bpp[i] * +DIV_ROUND_UP (aligned_width, hdiv) * +DIV_ROUND_UP (aligned_height, vdiv); + } +return 0 ; +}
Same comments as per the multi planar version.
Quoted 8 lines
drivers/media/v4l2-core/v4l2-common.c +1
-0+EXPORT_SYMBOL_GPL (v4l2_fill_pixfmt);include/media/v4l2-common.h +0
-0@@ -392,4 +392,37 @@ int v4l2_s_parm_cap(struct video_device *vdev, ((u64)(a).numerator * (b).denominator OP \ (u64)(b).numerator * (a).denominator)
include/media/v4l2-common.h int v4l2_s_parm_cap(struct video_device *vdev, · line 403
Quoted 10 lines
include/media/v4l2-common.h +9
-0+/* ------------------------------------------------------------------------- */ + +/* Pixel format and FourCC helpers */ + +/** +* struct v4l2_format_info - information about a V4L2 format +* @format: 4CC format identifier (V4L2_PIX_FMT_*) +* @mem_planes: Number of memory planes, which includes the alpha plane (1 to 4). +* @comp_planes: Number of component planes, which includes the alpha plane (1 to 4).
Is alpha channel present in other formats that are not packed RGB? Because if that's not the case, they are packed, and all the ones you have defined here have 1 memory and 1 component plane only. What formats would you expect to be multi planar and have an alpha channel?
Quoted 5 lines
include/media/v4l2-common.h +5
-0+* @bpp: Array of per-plane bytes per pixel +* @hdiv: Horizontal chroma subsampling factor +* @vdiv: Vertical chroma subsampling factor +* @block_w: Per-plane macroblock pixel width (optional) +* @block_h: Per-plane macroblock pixel height (optional)
include/media/v4l2-common.h int v4l2_s_parm_cap(struct video_device *vdev, · line 418
Quoted 10 lines
include/media/v4l2-common.h +10
-0+*/ +struct v4l2_format_info { +u32 format; +u8 mem_planes; +u8 comp_planes; +u8 bpp[4 ]; +u8 hdiv; +u8 vdiv; +u8 block_w[4 ]; +u8 block_h[4 ];
I might have missed how a user of these fill functions is supposed to set block_w and block_h. If these are alignment requirements that might be platform dependent, shouldn't they come as parameters to v4l2_fill_pixfmt() and v4l2_fill_pixfmt_mp()? Or assume drivers already provide aligned sizes maybe...
Thank you, sorry for the many questions.
j
Quoted 13 lines
include/media/v4l2-common.h +9
-1+}; + +const struct v4l2_format_info *v4l2_format_info (u32 format); + +int v4l2_fill_pixfmt (struct v4l2_pix_format *pixfmt,int pixelformat, +int width,int height); +int v4l2_fill_pixfmt_mp (struct v4l2_pix_format_mplane *pixfmt,int pixelformat, +int width,int height); +#endif /* V4L2_COMMON_H_ */ --2.20.1
Re: [PATCH v3 2/2] media: Introduce helpers to fill pixel format structs
Quoted 3 lines · Jacopo Mondi
Hi Ezequiel,
this is very nice, thank you!
Hi Jacopo,
Quoted 45 lines · Ezequiel Garcia
Quoted 17 lines
Add two new API helpers,
v4l2_fill_pixfmtandv4l2_fill_pixfmt_mp, to be used by drivers to calculate plane sizes and bytes per lines.Note that driver-specific padding and alignment are not taken into account, and must be done by drivers using this API.
Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/media/v4l2-core/v4l2-common.c | 164 ++++++++++++++++++++++++++ include/media/v4l2-common.h | 33 ++++++ 2 files changed, 197 insertions(+)drivers/media/v4l2-core/v4l2-common.c +0
-0drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 453
Quoted 9 lines
drivers/media/v4l2-core/v4l2-common.c +6
-0@@ -445,3 +445,167 @@ int v4l2_s_parm_cap(struct video_device *vdev,return ret; }EXPORT_SYMBOL_GPL (v4l2_s_parm_cap); + +const struct v4l2_format_info *v4l2_format_info (u32 format) +{ +static const struct v4l2_format_info formats[] = { +/* RGB formats */ + { .format =V4L2_PIX_FMT_BGR24 , .mem_planes =1 , .comp_planes =1 , .bpp = {3 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 },How would you feel using macros here, this would help sticking to 80-or-so columns, as otherwise this table is pretty wide.
I've sketched out something quick:
#define
V4L2_FMT_INFO_BPP(p1_, p2_, p3_, p4_){p1_,p2_,p3_,p4_}#define V4L2_FMT_INFO(pf_, mpl_, cpl_, bpp_, hd_, vd_) \ { \ .format = V4L2_PIX_FMT_##pf_, \ .mem_planes = mpl_, \ .comp_planes = cpl_, \ .bpp = bpp_, \ .hdiv = hd_, \ .vdiv = vd_ \ }const struct
v4l2_format_info*v4l2_format_info(u32 format)
Quoted 9 lines
{ static const struct v4l2_format_info formats[] = { /* RGB formats */ V4L2_FMT_INFO(BGR24, 1, 1, V4L2_FMT_INFO_BPP(3, 0, 0, 0), 1, 1), ... }Not sure you might like it or not.
I'm not entirely sure this improves readability. Hans has just sent a pull-request for this. But see below, my replies to some of your questions.
Quoted 42 lines
Quoted 42 lines
drivers/media/v4l2-core/v4l2-common.c +42
-0+ { .format = V4L2_PIX_FMT_RGB24, .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_HSV24, .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_BGR32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_XBGR32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_RGB32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_XRGB32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_HSV32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_ARGB32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_ABGR32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_GREY, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + + /* YUV packed formats */ + { .format = V4L2_PIX_FMT_YUYV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YVYU, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_UYVY, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_VYUY, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + + /* YUV planar formats */ + { .format = V4L2_PIX_FMT_NV12, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV21, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV16, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV61, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV24, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV42, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + + { .format = V4L2_PIX_FMT_YUV410, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 4 }, + { .format = V4L2_PIX_FMT_YVU410, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 4 }, + { .format = V4L2_PIX_FMT_YUV411P, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YUV420, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YVU420, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YUV422P, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 }, + + /* YUV planar formats, non contiguous variant */ + { .format = V4L2_PIX_FMT_YUV420M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YVU420M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YUV422M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YVU422M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YUV444M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YVU444M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 1, .vdiv = 1 }, + + { .format = V4L2_PIX_FMT_NV12M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV21M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
Quoted 9 lines
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 498
Quoted 3 lines
drivers/media/v4l2-core/v4l2-common.c +3
-0+ { .format = V4L2_PIX_FMT_NV16M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV61M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + };Can I ask why you define the array inside this function and not as a static array with file scope? Is this something trivial I am missing? It will be placed in the same data segment anyhow, is this to reduce the probability of an unlikely name clash within the file? Just curious..
I followed drm-fourcc.c style. I like this pattern because it keeps the data and the function together, and because it forces all users to get the pixel format via v4l2_format_info().
Quoted 18 lines
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 503
Quoted 5 lines
drivers/media/v4l2-core/v4l2-common.c +5
-0+unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (formats); ++i) +if (formats[i].format == format) +return &formats[i];Nit: empty line maybe
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 511
Quoted 8 lines
drivers/media/v4l2-core/v4l2-common.c +8
-0+return NULL ; +} +EXPORT_SYMBOL (v4l2_format_info); + +static inline unsigned int v4l2_format_block_width (const struct v4l2_format_info *info,int plane) +{ +if (!info->block_w[plane]) +return 1 ;Nit: empty line, or just
Quoted 9 lines
return info->block_w[plane] ? info->block_w[plane] : 1;drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 515
Quoted 4 lines
drivers/media/v4l2-core/v4l2-common.c +4
-0+return info->block_w[plane]; +} + +static inline unsigned int v4l2_format_block_height (const struct v4l2_format_info *info,int plane)You don't like 80 columns that much, right :p ?
Guess I'm not a fan :-)
Quoted 50 lines
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 518
Quoted 3 lines
drivers/media/v4l2-core/v4l2-common.c +3
-0+{ +if (!info->block_h[plane]) +return 1 ;Same here
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 523
Quoted 5 lines
drivers/media/v4l2-core/v4l2-common.c +5
-0+return info->block_h[plane]; +} + +int v4l2_fill_pixfmt_mp (struct v4l2_pix_format_mplane *pixfmt, +int pixelformat,int width,int height)Bad alignment to open parenthesis
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 527
Quoted 4 lines
drivers/media/v4l2-core/v4l2-common.c +4
-0+{ +const struct v4l2_format_info *info; +struct v4l2_plane_pix_format *plane; +int i;unsigned int i; ?
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 540
Quoted 13 lines
drivers/media/v4l2-core/v4l2-common.c +13
-0+ + info =v4l2_format_info (pixelformat); +if (!info) +return -EINVAL ; + + pixfmt->width = width; + pixfmt->height = height; + pixfmt->pixelformat = pixelformat; + pixfmt->num_planes = info->mem_planes; + +if (info->mem_planes ==1 ) { + plane = &pixfmt->plane_fmt[0 ]; + plane->bytesperline =ALIGN (width,v4l2_format_block_width (info,0 )) * info->bpp[0 ];80 cols :'(
Quoted 13 lines
drivers/media/v4l2-core/v4l2-common.c +13
-0+ plane->sizeimage =0 ; + +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height; + + aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + plane->sizeimage += info->bpp[i] * + DIV_ROUND_UP(aligned_width, hdiv) *
Quoted 9 lines
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 555
Quoted 2 lines
drivers/media/v4l2-core/v4l2-common.c +2
-0+DIV_ROUND_UP (aligned_height, vdiv); + }I was initially puzzled by some of your definitions, say NV12, where you mark the hdiv factor for the second plane to 2, while it is actually only vertically subsampled. But now I see what you've done for NV12 and this works for all the other formats I have verified (NV12, NV16, YUYV packed, and YVV4xx). Nice :)
NV12 is 2x2 subsampled, that is why the format is:
{ .format = V4L2_PIX_FMT_NV12, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },
It has two planes: one luma plane and one plane with the two chroma components interleaved (hence bytes-per-pixel being 2).
The chroma components are subsampled in 2x2, and that is why hdiv and vdiv is 2.
Quoted 73 lines
Nit: you can save the below else branch indentation if you return here.
Quoted 7 lines
drivers/media/v4l2-core/v4l2-common.c +7
-0+ }else { +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height; +drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 572
Quoted 10 lines
drivers/media/v4l2-core/v4l2-common.c +10
-0+ aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + plane = &pixfmt->plane_fmt[i]; + plane->bytesperline = + info->bpp[i] *DIV_ROUND_UP (aligned_width, hdiv); + plane->sizeimage = + plane->bytesperline *DIV_ROUND_UP (aligned_height, vdiv); + } + }Empty line maybe?
Quoted 28 lines
drivers/media/v4l2-core/v4l2-common.c +28
-0+return 0 ; +} +EXPORT_SYMBOL_GPL (v4l2_fill_pixfmt_mp); + +int v4l2_fill_pixfmt (struct v4l2_pix_format *pixfmt,int pixelformat,int width,int height) +{ +const struct v4l2_format_info *info; +int i; + + info =v4l2_format_info (pixelformat); +if (!info) +return -EINVAL ; + +/* Single planar API cannot be used for multi plane formats. */ +if (info->mem_planes >1 ) +return -EINVAL ; + + pixfmt->width = width; + pixfmt->height = height; + pixfmt->pixelformat = pixelformat; + pixfmt->bytesperline =ALIGN (width,v4l2_format_block_width (info,0 )) * info->bpp[0 ]; + pixfmt->sizeimage =0 ; + +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height;drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 610
Quoted 10 lines
drivers/media/v4l2-core/v4l2-common.c +10
-0+ + aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + pixfmt->sizeimage += info->bpp[i] * +DIV_ROUND_UP (aligned_width, hdiv) * +DIV_ROUND_UP (aligned_height, vdiv); + } +return 0 ; +}Same comments as per the multi planar version.
Quoted 9 lines
drivers/media/v4l2-core/v4l2-common.c +1
-0+EXPORT_SYMBOL_GPL (v4l2_fill_pixfmt);include/media/v4l2-common.h +0
-0@@ -392,4 +392,37 @@ int v4l2_s_parm_cap(struct video_device *vdev, ((u64)(a).numerator * (b).denominator OP \ (u64)(b).numerator * (a).denominator)
Quoted 15 lines
include/media/v4l2-common.h int v4l2_s_parm_cap(struct video_device *vdev, · line 403
Quoted 9 lines
include/media/v4l2-common.h +9
-0+/* ------------------------------------------------------------------------- */ + +/* Pixel format and FourCC helpers */ + +/** +* struct v4l2_format_info - information about a V4L2 format +* @format: 4CC format identifier (V4L2_PIX_FMT_*) +* @mem_planes: Number of memory planes, which includes the alpha plane (1 to 4). +* @comp_planes: Number of component planes, which includes the alpha plane (1 to 4). Is alpha channel present in other formats that are not packed RGB? Because if that's not the case, they are packed, and all the ones you have defined here have 1 memory and 1 component plane only. What formats would you expect to be multi planar and have an alpha channel?
There's a format called AYUV https://www.fourcc.org/pixel-format/yuv-ayuv/.
It's not currently supported by the media subsystem, but our intention is to try to get this API as generic as possible.
Quoted 13 lines
Quoted 13 lines
include/media/v4l2-common.h +13
-0+* @bpp: Array of per-plane bytes per pixel +* @hdiv: Horizontal chroma subsampling factor +* @vdiv: Vertical chroma subsampling factor +* @block_w: Per-plane macroblock pixel width (optional) +* @block_h: Per-plane macroblock pixel height (optional) +*/ +struct v4l2_format_info { +u32 format; +u8 mem_planes; +u8 comp_planes; +u8 bpp[4 ]; +u8 hdiv; +u8 vdiv;
Quoted 9 lines
include/media/v4l2-common.h int v4l2_s_parm_cap(struct video_device *vdev, · line 418
Quoted 2 lines
include/media/v4l2-common.h +2
-0+u8 block_w[4 ]; +u8 block_h[4 ];I might have missed how a user of these fill functions is supposed to set
block_wandblock_h. If these are alignment requirements that might be platform dependent, shouldn't they come as parameters tov4l2_fill_pixfmt()andv4l2_fill_pixfmt_mp()? Or assume drivers already provide aligned sizes maybe...
These will need a contributor to introduce a pixel format. They are meant for tiled pixel formats. I left the introduction of them to the developers of the respective drivers.
Quoted 2 lines
Thank you, sorry for the many questions.
No problem.
Thanks!
Eze
Re: [PATCH v3 2/2] media: Introduce helpers to fill pixel format structs
Quoted 338 lines · Ezequiel Garcia
Quoted 3 lines · Jacopo Mondi
Hi Ezequiel,
this is very nice, thank you!Hi Jacopo,
Quoted 45 lines · Ezequiel Garcia
Quoted 17 lines
Add two new API helpers,
v4l2_fill_pixfmtandv4l2_fill_pixfmt_mp, to be used by drivers to calculate plane sizes and bytes per lines.Note that driver-specific padding and alignment are not taken into account, and must be done by drivers using this API.
Signed-off-by Ezequiel Garcia <[email protected]>--- drivers/media/v4l2-core/v4l2-common.c | 164 ++++++++++++++++++++++++++ include/media/v4l2-common.h | 33 ++++++ 2 files changed, 197 insertions(+)drivers/media/v4l2-core/v4l2-common.c +0
-0drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 453
Quoted 9 lines
drivers/media/v4l2-core/v4l2-common.c +6
-0@@ -445,3 +445,167 @@ int v4l2_s_parm_cap(struct video_device *vdev,return ret; }EXPORT_SYMBOL_GPL (v4l2_s_parm_cap); + +const struct v4l2_format_info *v4l2_format_info (u32 format) +{ +static const struct v4l2_format_info formats[] = { +/* RGB formats */ + { .format =V4L2_PIX_FMT_BGR24 , .mem_planes =1 , .comp_planes =1 , .bpp = {3 ,0 ,0 ,0 }, .hdiv =1 , .vdiv =1 },How would you feel using macros here, this would help sticking to 80-or-so columns, as otherwise this table is pretty wide.
I've sketched out something quick:
#define
V4L2_FMT_INFO_BPP(p1_, p2_, p3_, p4_){p1_,p2_,p3_,p4_}#define V4L2_FMT_INFO(pf_, mpl_, cpl_, bpp_, hd_, vd_) \ { \ .format = V4L2_PIX_FMT_##pf_, \ .mem_planes = mpl_, \ .comp_planes = cpl_, \ .bpp = bpp_, \ .hdiv = hd_, \ .vdiv = vd_ \ }const struct
v4l2_format_info*v4l2_format_info(u32 format)Quoted 9 lines
{ static const struct v4l2_format_info formats[] = { /* RGB formats */ V4L2_FMT_INFO(BGR24, 1, 1, V4L2_FMT_INFO_BPP(3, 0, 0, 0), 1, 1), ... }Not sure you might like it or not.
I'm not entirely sure this improves readability. Hans has just sent a pull-request for this. But see below, my replies to some of your questions.
Quoted 42 lines
Quoted 42 lines
drivers/media/v4l2-core/v4l2-common.c +42
-0+ { .format = V4L2_PIX_FMT_RGB24, .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_HSV24, .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_BGR32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_XBGR32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_RGB32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_XRGB32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_HSV32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_ARGB32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_ABGR32, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_GREY, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + + /* YUV packed formats */ + { .format = V4L2_PIX_FMT_YUYV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YVYU, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_UYVY, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_VYUY, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + + /* YUV planar formats */ + { .format = V4L2_PIX_FMT_NV12, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV21, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV16, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV61, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV24, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV42, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 1, .vdiv = 1 }, + + { .format = V4L2_PIX_FMT_YUV410, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 4 }, + { .format = V4L2_PIX_FMT_YVU410, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 4 }, + { .format = V4L2_PIX_FMT_YUV411P, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 4, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YUV420, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YVU420, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YUV422P, .mem_planes = 1, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 }, + + /* YUV planar formats, non contiguous variant */ + { .format = V4L2_PIX_FMT_YUV420M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YVU420M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_YUV422M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YVU422M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YUV444M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 1, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_YVU444M, .mem_planes = 3, .comp_planes = 3, .bpp = { 1, 1, 1, 0 }, .hdiv = 1, .vdiv = 1 }, + + { .format = V4L2_PIX_FMT_NV12M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 }, + { .format = V4L2_PIX_FMT_NV21M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },Quoted 9 lines
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 498
Quoted 3 lines
drivers/media/v4l2-core/v4l2-common.c +3
-0+ { .format = V4L2_PIX_FMT_NV16M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + { .format = V4L2_PIX_FMT_NV61M, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 1 }, + };Can I ask why you define the array inside this function and not as a static array with file scope? Is this something trivial I am missing? It will be placed in the same data segment anyhow, is this to reduce the probability of an unlikely name clash within the file? Just curious..
I followed
drm-fourcc.cstyle. I like this pattern because it keeps the data and the function together, and because it forces all users to get the pixel format viav4l2_format_info().Quoted 18 lines
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 503
Quoted 5 lines
drivers/media/v4l2-core/v4l2-common.c +5
-0+unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (formats); ++i) +if (formats[i].format == format) +return &formats[i];Nit: empty line maybe
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 511
Quoted 8 lines
drivers/media/v4l2-core/v4l2-common.c +8
-0+return NULL ; +} +EXPORT_SYMBOL (v4l2_format_info); + +static inline unsigned int v4l2_format_block_width (const struct v4l2_format_info *info,int plane) +{ +if (!info->block_w[plane]) +return 1 ;Nit: empty line, or just
Quoted 9 lines
return info->block_w[plane] ? info->block_w[plane] : 1;drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 515
Quoted 4 lines
drivers/media/v4l2-core/v4l2-common.c +4
-0+return info->block_w[plane]; +} + +static inline unsigned int v4l2_format_block_height (const struct v4l2_format_info *info,int plane)You don't like 80 columns that much, right :p ?
Guess I'm not a fan :-)
Quoted 50 lines
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 518
Quoted 3 lines
drivers/media/v4l2-core/v4l2-common.c +3
-0+{ +if (!info->block_h[plane]) +return 1 ;Same here
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 523
Quoted 5 lines
drivers/media/v4l2-core/v4l2-common.c +5
-0+return info->block_h[plane]; +} + +int v4l2_fill_pixfmt_mp (struct v4l2_pix_format_mplane *pixfmt, +int pixelformat,int width,int height)Bad alignment to open parenthesis
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 527
Quoted 4 lines
drivers/media/v4l2-core/v4l2-common.c +4
-0+{ +const struct v4l2_format_info *info; +struct v4l2_plane_pix_format *plane; +int i;unsigned int i; ?
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 540
Quoted 13 lines
drivers/media/v4l2-core/v4l2-common.c +13
-0+ + info =v4l2_format_info (pixelformat); +if (!info) +return -EINVAL ; + + pixfmt->width = width; + pixfmt->height = height; + pixfmt->pixelformat = pixelformat; + pixfmt->num_planes = info->mem_planes; + +if (info->mem_planes ==1 ) { + plane = &pixfmt->plane_fmt[0 ]; + plane->bytesperline =ALIGN (width,v4l2_format_block_width (info,0 )) * info->bpp[0 ];80 cols :'(
Quoted 13 lines
drivers/media/v4l2-core/v4l2-common.c +13
-0+ plane->sizeimage =0 ; + +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height; + + aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + plane->sizeimage += info->bpp[i] * + DIV_ROUND_UP(aligned_width, hdiv) *Quoted 9 lines
drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 555
Quoted 2 lines
drivers/media/v4l2-core/v4l2-common.c +2
-0+DIV_ROUND_UP (aligned_height, vdiv); + }I was initially puzzled by some of your definitions, say NV12, where you mark the hdiv factor for the second plane to 2, while it is actually only vertically subsampled. But now I see what you've done for NV12 and this works for all the other formats I have verified (NV12, NV16, YUYV packed, and YVV4xx). Nice :)
NV12 is 2x2 subsampled, that is why the format is:
{ .format =
V4L2_PIX_FMT_NV12, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .hdiv = 2, .vdiv = 2 },It has two planes: one luma plane and one plane with the two chroma components interleaved (hence bytes-per-pixel being 2).
The chroma components are subsampled in 2x2, and that is why hdiv and vdiv is 2.
Quoted 73 lines
Nit: you can save the below else branch indentation if you return here.
Quoted 7 lines
drivers/media/v4l2-core/v4l2-common.c +7
-0+ }else { +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height; +drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 572
Quoted 10 lines
drivers/media/v4l2-core/v4l2-common.c +10
-0+ aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + plane = &pixfmt->plane_fmt[i]; + plane->bytesperline = + info->bpp[i] *DIV_ROUND_UP (aligned_width, hdiv); + plane->sizeimage = + plane->bytesperline *DIV_ROUND_UP (aligned_height, vdiv); + } + }Empty line maybe?
Quoted 28 lines
drivers/media/v4l2-core/v4l2-common.c +28
-0+return 0 ; +} +EXPORT_SYMBOL_GPL (v4l2_fill_pixfmt_mp); + +int v4l2_fill_pixfmt (struct v4l2_pix_format *pixfmt,int pixelformat,int width,int height) +{ +const struct v4l2_format_info *info; +int i; + + info =v4l2_format_info (pixelformat); +if (!info) +return -EINVAL ; + +/* Single planar API cannot be used for multi plane formats. */ +if (info->mem_planes >1 ) +return -EINVAL ; + + pixfmt->width = width; + pixfmt->height = height; + pixfmt->pixelformat = pixelformat; + pixfmt->bytesperline =ALIGN (width,v4l2_format_block_width (info,0 )) * info->bpp[0 ]; + pixfmt->sizeimage =0 ; + +for (i =0 ; i < info->comp_planes; i++) { +unsigned int hdiv = (i ==0 ) ?1 : info->hdiv; +unsigned int vdiv = (i ==0 ) ?1 : info->vdiv; +unsigned int aligned_width; +unsigned int aligned_height;drivers/media/v4l2-core/v4l2-common.c int v4l2_s_parm_cap(struct video_device *vdev, · line 610
Quoted 10 lines
drivers/media/v4l2-core/v4l2-common.c +10
-0+ + aligned_width =ALIGN (width,v4l2_format_block_width (info, i)); + aligned_height =ALIGN (height,v4l2_format_block_height (info, i)); + + pixfmt->sizeimage += info->bpp[i] * +DIV_ROUND_UP (aligned_width, hdiv) * +DIV_ROUND_UP (aligned_height, vdiv); + } +return 0 ; +}Same comments as per the multi planar version.
Quoted 9 lines
drivers/media/v4l2-core/v4l2-common.c +1
-0+EXPORT_SYMBOL_GPL (v4l2_fill_pixfmt);include/media/v4l2-common.h +0
-0@@ -392,4 +392,37 @@ int v4l2_s_parm_cap(struct video_device *vdev, ((u64)(a).numerator * (b).denominator OP \ (u64)(b).numerator * (a).denominator)Quoted 15 lines
include/media/v4l2-common.h int v4l2_s_parm_cap(struct video_device *vdev, · line 403
Quoted 9 lines
include/media/v4l2-common.h +9
-0+/* ------------------------------------------------------------------------- */ + +/* Pixel format and FourCC helpers */ + +/** +* struct v4l2_format_info - information about a V4L2 format +* @format: 4CC format identifier (V4L2_PIX_FMT_*) +* @mem_planes: Number of memory planes, which includes the alpha plane (1 to 4). +* @comp_planes: Number of component planes, which includes the alpha plane (1 to 4). Is alpha channel present in other formats that are not packed RGB? Because if that's not the case, they are packed, and all the ones you have defined here have 1 memory and 1 component plane only. What formats would you expect to be multi planar and have an alpha channel?
There's a format called AYUV https://www.fourcc.org/pixel-format/yuv-ayuv/.
It's not currently supported by the media subsystem, but our intention is to try to get this API as generic as possible.
Quoted 13 lines
Quoted 13 lines
include/media/v4l2-common.h +13
-0+* @bpp: Array of per-plane bytes per pixel +* @hdiv: Horizontal chroma subsampling factor +* @vdiv: Vertical chroma subsampling factor +* @block_w: Per-plane macroblock pixel width (optional) +* @block_h: Per-plane macroblock pixel height (optional) +*/ +struct v4l2_format_info { +u32 format; +u8 mem_planes; +u8 comp_planes; +u8 bpp[4 ]; +u8 hdiv; +u8 vdiv;Quoted 9 lines
include/media/v4l2-common.h int v4l2_s_parm_cap(struct video_device *vdev, · line 418
Quoted 2 lines
include/media/v4l2-common.h +2
-0+u8 block_w[4 ]; +u8 block_h[4 ];I might have missed how a user of these fill functions is supposed to set
block_wandblock_h. If these are alignment requirements that might be platform dependent, shouldn't they come as parameters tov4l2_fill_pixfmt()andv4l2_fill_pixfmt_mp()? Or assume drivers already provide aligned sizes maybe...
Quoted 3 lines
These will need a contributor to introduce a pixel format. They are meant for tiled pixel formats. I left the introduction of them to the developers of the respective drivers.
I suspect we already have a format, see:
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-nv12mt.html
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-nv12m.html
(the 16x16 variant)
They were respectively added for Exynos 4 and Exynos 5 support. So for the second, I suspsect block_w/h is 16. But for NV12MT I'm not very clear if it would be block_w of 64 (the actual tile width) or 128 which is the required width alignment.
Quoted 10 lines
Quoted 4 lines
Thank you, sorry for the many questions.
No problem.
Thanks!
Eze
Quoted 11 lines · Ezequiel Garcia
Following the "Add MPEG-2 decoding to Rockchip VPU"[1], and given we want to start using the pixel format helpers rather soon, I've decided to submit them separatedly.
Aside from documenting the
block_wandblock_h, as suggested by Emil Velikov, these two patches are unmodified compared to v2 [1].This series contains the minimum patches to introduce the pixel format helpers, it currently will have no users, but we expect users to start popping-up soon.
very shortly [2].
Is there anything that prevents you from moving the Rockchip drivers to using these right after patch 2/2? IIRC that's what your MPEG-2 series did initially, and that would introduce a user immediately.
Hi Ezequiel,
Quoted 6 lines · Ezequiel Garcia
Following the "Add MPEG-2 decoding to Rockchip VPU"[1], and given we want to start using the pixel format helpers rather soon, I've decided to submit them separatedly.
Aside from documenting the
block_wandblock_h, as suggested by Emil Velikov, these two patches are unmodified compared to v2 [1].
Quoted 6 lines
This series contains the minimum patches to introduce the pixel format helpers, it currently will have no users, but we expect users to start popping-up soon.
very shortly [2].To avoid confusing the patches here with those on v2 [1], I'm marking them as v3.
You might have missed it, but have you looked at the RFC "drm: Split out the formats API and move it to a common place" I sent a couple of weeks ago? It seems to cover the same use cases.
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com