linux-kernel - Re: [PATCH v2] media: mediatek: vcodec: Handle VP9 superframe bitstream with 8 sub-frames

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] [day] [month] [year] [list]

Message-ID: <14c9dea999efc16f2f1753a9524f558cf066a244.camel@mediatek.com>
Date: Mon, 4 Mar 2024 02:00:49 +0000
From: Irui Wang (王瑞) <Irui.Wang@...iatek.com>
To: "sebastian.fricke@...labora.com" <sebastian.fricke@...labora.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Longfei Wang (王龙飞) <Longfei.Wang@...iatek.com>,
	Tiffany Lin (林慧珊) <tiffany.lin@...iatek.com>,
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	Maoguang Meng (孟毛广)
	<Maoguang.Meng@...iatek.com>, "mchehab@...nel.org" <mchehab@...nel.org>,
	"nicolas.dufresne@...labora.com" <nicolas.dufresne@...labora.com>,
	Yunfei Dong (董云飞) <Yunfei.Dong@...iatek.com>,
	Project_Global_Chrome_Upstream_Group
	<Project_Global_Chrome_Upstream_Group@...iatek.com>,
	"hverkuil-cisco@...all.nl" <hverkuil-cisco@...all.nl>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, "angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH v2] media: mediatek: vcodec: Handle VP9 superframe
 bitstream with 8 sub-frames

Dear Sebastian Sir,

Thanks for your reviewing.

I will reword the commit message according to your suggestion, and send
a new patch v3. Thank you very much.

Thanks
Best Regards

On Fri, 2024-03-01 at 16:13 +0100, Sebastian Fricke wrote:
> Hey Irui,
> 
> On 29.02.2024 11:02, Irui Wang wrote:
> > The VP9 bitstream has 8 sub-frames into one superframe, the
> > superframe
> > index validate failed when reach 8, modify the index checking so
> > that the
> > last sub-frame can be decoded normally with stateful vp9 decoder.
> 
> I find this commit message a bit confusing, you say that you couldn't
> index the last superframe, but then you say that you modify the index
> checking so that you can access the last sub-frame.
> 
> I would reword this section, here is my suggestion:
> 
> The VP9 bitstream uses superframes, which each contain 8 sub-frames,
> enable accessing the last superframe by increasing the range of the
> index validation as the maximum number of superframes is 8 and not 7.
> 
> The rest looks good as already mentioned by Nicolas.
> 
> Greetings,
> Sebastian
> 
> > 
> > Signed-off-by: Irui Wang <irui.wang@...iatek.com>
> > ---
> > changed with v1:
> > - add a new define 'VP9_MAX_SUPER_FRAMES_NUM' for superframes.
> > ---
> > .../mediatek/vcodec/decoder/vdec/vdec_vp9_if.c        | 11 ++++++
> > -----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > index 55355fa70090..039082f600c8 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > @@ -16,6 +16,7 @@
> > #include "../vdec_drv_base.h"
> > #include "../vdec_vpu_if.h"
> > 
> > +#define VP9_MAX_SUPER_FRAMES_NUM 8
> > #define VP9_SUPER_FRAME_BS_SZ 64
> > #define MAX_VP9_DPB_SIZE	9
> > 
> > @@ -133,11 +134,11 @@ struct vp9_sf_ref_fb {
> >  */
> > struct vdec_vp9_vsi {
> > 	unsigned char sf_bs_buf[VP9_SUPER_FRAME_BS_SZ];
> > -	struct vp9_sf_ref_fb sf_ref_fb[VP9_MAX_FRM_BUF_NUM-1];
> > +	struct vp9_sf_ref_fb sf_ref_fb[VP9_MAX_SUPER_FRAMES_NUM];
> > 	int sf_next_ref_fb_idx;
> > 	unsigned int sf_frm_cnt;
> > -	unsigned int sf_frm_offset[VP9_MAX_FRM_BUF_NUM-1];
> > -	unsigned int sf_frm_sz[VP9_MAX_FRM_BUF_NUM-1];
> > +	unsigned int sf_frm_offset[VP9_MAX_SUPER_FRAMES_NUM];
> > +	unsigned int sf_frm_sz[VP9_MAX_SUPER_FRAMES_NUM];
> > 	unsigned int sf_frm_idx;
> > 	unsigned int sf_init;
> > 	struct vdec_fb fb;
> > @@ -526,7 +527,7 @@ static void vp9_swap_frm_bufs(struct
> > vdec_vp9_inst *inst)
> > 	/* if this super frame and it is not last sub-frame, get next
> > fb for
> > 	 * sub-frame decode
> > 	 */
> > -	if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt -
> > 1)
> > +	if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt)
> > 		vsi->sf_next_ref_fb_idx = vp9_get_sf_ref_fb(inst);
> > }
> > 
> > @@ -735,7 +736,7 @@ static void get_free_fb(struct vdec_vp9_inst
> > *inst, struct vdec_fb **out_fb)
> > 
> > static int validate_vsi_array_indexes(struct vdec_vp9_inst *inst,
> > 		struct vdec_vp9_vsi *vsi) {
> > -	if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM - 1) {
> > +	if (vsi->sf_frm_idx > VP9_MAX_SUPER_FRAMES_NUM) {
> > 		mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.",
> > vsi->sf_frm_idx);
> > 		return -EIO;
> > 	}
> > -- 
> > 2.18.0
> > 
> >