linux-kernel - RE: [EXT] Re: [PATCH] media: amphion: ensure the buffer count is not less than min

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: <AM6PR04MB6341A433AC17C5A6C773245FE7FA9@AM6PR04MB6341.eurprd04.prod.outlook.com>
Date:   Wed, 27 Apr 2022 09:31:29 +0000
From:   Ming Qian <ming.qian@....com>
To:     Hans Verkuil <hverkuil-cisco@...all.nl>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>
CC:     "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        Aisheng Dong <aisheng.dong@....com>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [EXT] Re: [PATCH] media: amphion: ensure the buffer count is not
 less than min_buffer

> From: Hans Verkuil [mailto:hverkuil-cisco@...all.nl]
> Sent: Wednesday, April 27, 2022 3:25 PM
> To: Ming Qian <ming.qian@....com>; mchehab@...nel.org;
> shawnguo@...nel.org
> Cc: robh+dt@...nel.org; s.hauer@...gutronix.de; kernel@...gutronix.de;
> festevam@...il.com; dl-linux-imx <linux-imx@....com>; Aisheng Dong
> <aisheng.dong@....com>; linux-media@...r.kernel.org;
> linux-kernel@...r.kernel.org; linux-arm-kernel@...ts.infradead.org
> Subject: Re: [EXT] Re: [PATCH] media: amphion: ensure the buffer count is not
> less than min_buffer
> 
> Caution: EXT Email
> 
> On 27/04/2022 09:01, Ming Qian wrote:
> >> From: Hans Verkuil [mailto:hverkuil-cisco@...all.nl]
> >> Sent: Wednesday, April 27, 2022 2:38 PM
> >> To: Ming Qian <ming.qian@....com>; mchehab@...nel.org;
> >> shawnguo@...nel.org
> >> Cc: robh+dt@...nel.org; s.hauer@...gutronix.de;
> >> kernel@...gutronix.de; festevam@...il.com; dl-linux-imx
> >> <linux-imx@....com>; Aisheng Dong <aisheng.dong@....com>;
> >> linux-media@...r.kernel.org; linux-kernel@...r.kernel.org;
> >> linux-arm-kernel@...ts.infradead.org
> >> Subject: [EXT] Re: [PATCH] media: amphion: ensure the buffer count is
> >> not less than min_buffer
> >>
> >> Caution: EXT Email
> >>
> >> Hi Ming Qian,
> >>
> >> On 22/03/2022 09:28, Ming Qian wrote:
> >>> the output buffer count should >= min_buffer_out the capture buffer
> >>> count should >= min_buffer_cap
> >>>
> >>> Signed-off-by: Ming Qian <ming.qian@....com>
> >>> ---
> >>>  drivers/media/platform/amphion/vpu_v4l2.c | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/drivers/media/platform/amphion/vpu_v4l2.c
> >>> b/drivers/media/platform/amphion/vpu_v4l2.c
> >>> index cbf3315605a9..72a0544f4da3 100644
> >>> --- a/drivers/media/platform/amphion/vpu_v4l2.c
> >>> +++ b/drivers/media/platform/amphion/vpu_v4l2.c
> >>> @@ -355,6 +355,10 @@ static int vpu_vb2_queue_setup(struct
> vb2_queue
> >> *vq,
> >>>               return 0;
> >>>       }
> >>>
> >>> +     if (V4L2_TYPE_IS_OUTPUT(vq->type))
> >>> +             *buf_count = max_t(unsigned int, *buf_count,
> >> inst->min_buffer_out);
> >>> +     else
> >>> +             *buf_count = max_t(unsigned int, *buf_count,
> >>> + inst->min_buffer_cap);
> >>
> >> I noticed that min_buffer_out/cap is set to 2, but min_buffers_needed
> >> is set to 1. Wouldn't it make more sense to set min_buffers_needed to
> >> 2 as well?
> >>
> >> If you do that, then the vb2 core will already take care of ensuring
> >> that the buf_count is adjusted.
> >>
> >> If you *do* have to do this manually, then you need to place the
> >> whole if-else under 'if (!*num_planes) {', otherwise it will mess up
> >> the VIDIOC_CREATE_BUFS ioctl. See the queue_setup in
> >> include/media/videobuf2-core.h documentation for the sordid details.
> >>
> >> Regards,
> >>
> >>         Hans
> >>
> >
> > Hi Hans,
> >     I want to make the vpu start when 1 frames is queued, so I set the
> min_buffers_needed to 1.
> > And the min_buffer_cap may be changed when a source change event is
> triggered. So in most case, it will be larger than 2.
> 
> Ah, I only grepped for min_buffer_out, not _cap, so I missed that that one isn't
> constant.
> 
> >     I'll make a v2 patch that place the whole if-else under 'if (!*num_planes)
> {'
>
Hi Hans,
    I send a v2 patch. 
	But I think the v1 is OK, as the full code has already guaranteed the condition ` if (!*num_planes)`,

	if (*plane_count) {
		... ...
		return 0;
    }
 	if (V4L2_TYPE_IS_OUTPUT(vq->type))
		*buf_count = max_t(unsigned int, *buf_count, inst->min_buffer_out);
	else
		*buf_count = max_t(unsigned int, *buf_count, inst->min_buffer_cap);

Ming

> Great, thank you!
> 
>         Hans
> 
> >     Thanks for your reminder
> >
> > Ming
> >
> >>>       *plane_count = cur_fmt->num_planes;
> >>>       for (i = 0; i < cur_fmt->num_planes; i++)
> >>>               psize[i] = cur_fmt->sizeimage[i];
> >