| Message ID | 20240409064431.16909-1-yunfei.dong@mediatek.com |
|---|---|
| Headers | show |
| Series | media: change the calculation method of sizeimage | expand |
Le mardi 09 avril 2024 à 14:44 +0800, Yunfei Dong a écrit : > Changing the bpp and hdiv values to make sure the bytesperline > of plane[0] and plane[1] are the same. The width and height are > 64 align. > > Fixes: 6afcc2b0aebf ("media: mediatek: vcodec: Add capture format to support 10bit tile mode") > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > drivers/media/v4l2-core/v4l2-common.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > index d34d210908d9..8587cd14741c 100644 > --- a/drivers/media/v4l2-core/v4l2-common.c > +++ b/drivers/media/v4l2-core/v4l2-common.c > @@ -265,8 +265,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format) > { .format = V4L2_PIX_FMT_VYUY, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 }, > { .format = V4L2_PIX_FMT_Y212, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 }, > { .format = V4L2_PIX_FMT_YUV48_12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 6, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 1 }, > - { .format = V4L2_PIX_FMT_MT2110T, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2, > - .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, > + { .format = V4L2_PIX_FMT_MT2110T, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 5, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 1, .vdiv = 2, > + .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }}, The code that calculate the info you are fixing: plane->bytesperline = info->bpp[i] * DIV_ROUND_UP(aligned_width, hdiv) / info->bpp_div[i]; plane->sizeimage = plane->bytesperline * DIV_ROUND_UP(aligned_height, vdiv); hdiv is 1 for the first plane, and 2 for second plane. Let say we have 1920x1080 video, bytesperline for that first planes would be 5 * (1920 / 1) / 4 = 2400. But for the second plane with your change we have 5 * (1920 / 2) / 4 = 1200. For me, you failed to achieve your goal, or there is a bug elsewhere. The second change should have its own commit, but I think it should be dropped. The driver is hacked to be compatible with the Mali GPUs (64x64 textures), but does not need this alignment. And in any case, the block should represent the pixel format required alignment, so I'd suggest to leave it like this, and simply align width/height before calling the function to calculate stride/sizeimage. The 64x64 alignment can be applied using v4l2_apply_frmsize_constraints() during try fmt (or v4l_bound_align_image(), but first is preferred). Same comments apply to the following patch, which should be squashed. > { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2, > .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }}, >
Hi, sorry for the long delay. Le mardi 09 avril 2024 à 14:44 +0800, Yunfei Dong a écrit : > The bytesperline and sizeimage of each plan are different for 8bit > and 10bit bitstreams. Using v4l2 common interface to calculate them > independently. > > --- > compare with v1: > - add patch 1/2/3/4 > - change the calculation method for sizeimage for patch 5 > --- > Yunfei Dong (5): > media: mediatek: vcodec: fix incorrect MT2T format information > media: mediatek: vcodec: fix incorrect MT2R format information As per recommandation of the "Submitting Patches": > On the other hand, if you make a single change to numerous files, group those > changes into a single patch. Thus a single logical change is contained within > a single patch. In general, when two patches have an identical description its a sign they should be one patch. Would be nice to squash these two. > media: mediatek: vcodec: add MM21 format definition > media: mediatek: vcodec: add MT21 format definition Same. > media: mediatek: vcodec: fix incorrect sizeimage for 10bit bitstream > > .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 28 ++++++------------- > drivers/media/v4l2-core/v4l2-common.c | 12 +++++--- > 2 files changed, 16 insertions(+), 24 deletions(-) >
Le mardi 09 avril 2024 à 14:44 +0800, Yunfei Dong a écrit : > Adding the definition of MM21 format to calculate bytesperline > and sizeimage of plane[0] and plane[1]. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > drivers/media/v4l2-core/v4l2-common.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > index cef1492dba22..0d5de132e07f 100644 > --- a/drivers/media/v4l2-core/v4l2-common.c > +++ b/drivers/media/v4l2-core/v4l2-common.c > @@ -269,6 +269,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format) > .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }}, > { .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 5, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 1, .vdiv = 2, > .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }}, > + { .format = V4L2_PIX_FMT_MM21, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 1, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 2, > + .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }}, MM21 has the same subsampling as NV12M, so hdiv should be 2 for correctness. The 64x64 block is incorrect. The only difference between this and NV12M should be with block. > > /* YUV planar formats */ > { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },
Le mardi 09 avril 2024 à 14:44 +0800, Yunfei Dong a écrit : > Adding the definition of MT21 format to calculate bytesperline MT21C in the subject and description. > and sizeimage of plane[0] and plane[1]. > > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com> > --- > drivers/media/v4l2-core/v4l2-common.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c > index 0d5de132e07f..6dba989c2291 100644 > --- a/drivers/media/v4l2-core/v4l2-common.c > +++ b/drivers/media/v4l2-core/v4l2-common.c > @@ -271,6 +271,8 @@ const struct v4l2_format_info *v4l2_format_info(u32 format) > .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }}, > { .format = V4L2_PIX_FMT_MM21, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 1, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 2, > .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }}, > + { .format = V4L2_PIX_FMT_MT21C, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 1, 1, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 2, > + .block_w = { 64, 64, 0, 0 }, .block_h = { 64, 64, 0, 0 }}, As you may notice, we have no support for compressed formats yet in this helper. I believe for these formats we need some extra code to add the header size calculation. One way could be to add a ops in that table for when a header is needed. That ops would implement the format specific extra space calculation which would be added to sizeimage of each planes (in in one of the planes depending on the specifics of the format). > > /* YUV planar formats */ > { .format = V4L2_PIX_FMT_NV12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 2, .bpp = { 1, 2, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 2 },