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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240823124023.uhypfcixfsget26q@basti-XPS-13-9310>
Date: Fri, 23 Aug 2024 14:40:23 +0200
From: Sebastian Fricke <sebastian.fricke@...labora.com>
To: Yunfei Dong <yunfei.dong@...iatek.com>
Cc: Nícolas F . R . A . Prado <nfraprado@...labora.com>,
	Nicolas Dufresne <nicolas.dufresne@...labora.com>,
	Hans Verkuil <hverkuil-cisco@...all.nl>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Benjamin Gaignard <benjamin.gaignard@...labora.com>,
	Nathan Hebert <nhebert@...omium.org>,
	Daniel Almeida <daniel.almeida@...labora.com>,
	Hsin-Yi Wang <hsinyi@...omium.org>,
	Fritz Koenig <frkoenig@...omium.org>,
	Daniel Vetter <daniel@...ll.ch>, Steve Cho <stevecho@...omium.org>,
	linux-media@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org,
	Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v4 4/7] media: mediatek: vcodec: using input information
 to get vb2 buffer

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@...iatek.com>
>---
> .../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
>
>