Re: [PATCH v4 4/7] media: mediatek: vcodec: using input information to get vb2 buffer

[Sebastian Fricke <sebastian.fricke@xxxxxxxxxxxxx>]

Hey Yunfei,

I would rename the title to something like this:

media: mediatek: vcodec: Get SRC buffer from bitstream instead of M2M

On 07.08.2024 16:24, Yunfei Dong wrote:
> vb2 buffer may be removed from ready list when lat try to get next
> src buffer, leading to vb2 buffer not the current one. Need to get
> vb2 buffer according to current input memory information.

And I would rewrite the commit log like this:

Getting the SRC buffer from the M2M buffer-queue risks picking a
different SRC buffer than the one used for the current decode operation.
Get the SRC buffer therefore from the bitstream data, which was set up
earlier during the decode.

Did I get that right?

Also could you explain why this change is required in this series?

Regards,
Sebastian Fricke

> Signed-off-by: Yunfei Dong <yunfei.dong@xxxxxxxxxxxx>
> ---
> .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c     | 13 +++++++------
> .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c     | 15 +++++++--------
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> index 90217cc8e242..a744740ba5f1 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
> @@ -1062,19 +1062,20 @@ static inline void vdec_av1_slice_vsi_to_remote(struct vdec_av1_slice_vsi *vsi,
>
> static int vdec_av1_slice_setup_lat_from_src_buf(struct vdec_av1_slice_instance *instance,
> 						 struct vdec_av1_slice_vsi *vsi,
> +						 struct mtk_vcodec_mem *bs,
> 						 struct vdec_lat_buf *lat_buf)
> {
> -	struct vb2_v4l2_buffer *src;
> +	struct mtk_video_dec_buf *src_buf_info;
> 	struct vb2_v4l2_buffer *dst;
>
> -	src = v4l2_m2m_next_src_buf(instance->ctx->m2m_ctx);
> -	if (!src)
> +	src_buf_info = container_of(bs, struct mtk_video_dec_buf, bs_buffer);
> +	if (!src_buf_info)
> 		return -EINVAL;
>
> -	lat_buf->vb2_v4l2_src = src;
> +	lat_buf->vb2_v4l2_src = &src_buf_info->m2m_buf.vb;
>
> 	dst = &lat_buf->ts_info;
> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> +	v4l2_m2m_buf_copy_metadata(lat_buf->vb2_v4l2_src, dst, true);
> 	vsi->frame.cur_ts = dst->vb2_buf.timestamp;
>
> 	return 0;
> @@ -1724,7 +1725,7 @@ static int vdec_av1_slice_setup_lat(struct vdec_av1_slice_instance *instance,
> 	struct vdec_av1_slice_vsi *vsi = &pfc->vsi;
> 	int ret;
>
> -	ret = vdec_av1_slice_setup_lat_from_src_buf(instance, vsi, lat_buf);
> +	ret = vdec_av1_slice_setup_lat_from_src_buf(instance, vsi, bs, lat_buf);
> 	if (ret)
> 		return ret;
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> index 3dceb668ba1c..c50a454ab4f7 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
> @@ -712,19 +712,18 @@ int vdec_vp9_slice_setup_single_from_src_to_dst(struct vdec_vp9_slice_instance *
> }
>
> static int vdec_vp9_slice_setup_lat_from_src_buf(struct vdec_vp9_slice_instance *instance,
> +						 struct mtk_vcodec_mem *bs,
> 						 struct vdec_lat_buf *lat_buf)
> {
> -	struct vb2_v4l2_buffer *src;
> -	struct vb2_v4l2_buffer *dst;
> +	struct mtk_video_dec_buf *src_buf_info;
>
> -	src = v4l2_m2m_next_src_buf(instance->ctx->m2m_ctx);
> -	if (!src)
> +	src_buf_info = container_of(bs, struct mtk_video_dec_buf, bs_buffer);
> +	if (!src_buf_info)
> 		return -EINVAL;
>
> -	lat_buf->vb2_v4l2_src = src;
> +	lat_buf->vb2_v4l2_src = &src_buf_info->m2m_buf.vb;
>
> -	dst = &lat_buf->ts_info;
> -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> +	v4l2_m2m_buf_copy_metadata(lat_buf->vb2_v4l2_src, &lat_buf->ts_info, true);
> 	return 0;
> }
>
> @@ -1154,7 +1153,7 @@ static int vdec_vp9_slice_setup_lat(struct vdec_vp9_slice_instance *instance,
> 	struct vdec_vp9_slice_vsi *vsi = &pfc->vsi;
> 	int ret;
>
> -	ret = vdec_vp9_slice_setup_lat_from_src_buf(instance, lat_buf);
> +	ret = vdec_vp9_slice_setup_lat_from_src_buf(instance, bs, lat_buf);
> 	if (ret)
> 		goto err;
>
> --
> 2.46.0