Re: [PATCH v4,2/2] media: mediatek: vcodec: Using MM21 as the default capture format

["Yunfei Dong (董云飞)" <Yunfei.Dong@mediatek.com>]

Hi Nicolas,

Thanks for your suggestion and test result.
On Fri, 2023-03-17 at 12:16 -0400, Nícolas F. R. A. Prado wrote:
> Hi Yunfei,
> 
> thanks for the patch.
> 
> The commit title should be in imperative, so I suggest:
> 
> 	media: mediatek: vcodec: Make MM21 the default capture format
> 
Accepted in next patch v4.
> On Fri, Mar 17, 2023 at 11:08:33AM +0800, Yunfei Dong wrote:
> > For the capture queue only support MM21 format with LibYuv, need to
> > set MM21 as the
> > default format.
> 
> Again, I think this commit message could be improved a bit. Here's a
> suggestion:
> 
> 	Given that only the MM21 capture format is supported by
> userspace tools (like
> 	gstreamer and libyuv), make it the default capture format.
> 
> 	This allows us to force the MM21 format even when a MM21 and
> MT21C capable
> 	firmware is available (which is needed while dynamic format
> switching isn't
> 	implemented in the driver), without causing the following
> regressions on
> 	v4l2-compliance:
> 
> 				fail: v4l2-test-formats.cpp(478):
> pixelformat 3132544d (MT21) for buftype 9 not reported by ENUM_FMT
> 			test VIDIOC_G_FMT: FAIL
> 				fail: v4l2-test-formats.cpp(478):
> pixelformat 3132544d (MT21) for buftype 9 not reported by ENUM_FMT
> 			test VIDIOC_TRY_FMT: FAIL
> 				fail: v4l2-test-formats.cpp(478):
> pixelformat 3132544d (MT21) for buftype 9 not reported by ENUM_FMT
> 			test VIDIOC_S_FMT: FAIL
> 
> Also, I think it would be slightly better if this was the first patch
> in the
> series, so that v4l2-compliance doesn't fail in between the patches.
> 
Accepted in next patch v4.

Best Regards,
Yunfei Dong
> > 
> > Fixes: 7501edef6b1f ("media: mediatek: vcodec: Different codec
> > using different capture format")
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> 
> With this change I've confirmed that all v4l2-compliance tests are
> passing again:
> 
> Total for mtk-vcodec-dec device /dev/video2: 46, Succeeded: 46,
> Failed: 0, Warnings: 0
> 
> So, after the above comments are addressed,
> 
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> Thanks,
> Nícolas