linux-kernel - Re: [PATCH] media: vivid: Support 480p for webcam capture

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: <00b0a8af-b7a5-cb49-0684-0fd0efefa196@xs4all.nl>
Date:   Mon, 8 Oct 2018 19:53:38 +0200
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
        Kieran Bingham <kieran.bingham@...asonboard.com>
Cc:     Keiichi Watanabe <keiichiw@...omium.org>,
        linux-media@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-kernel@...r.kernel.org, tfiga@...omium.org,
        jcliang@...omium.org, shik@...omium.org
Subject: Re: [PATCH] media: vivid: Support 480p for webcam capture

On 10/08/2018 07:03 PM, Mauro Carvalho Chehab wrote:
> Em Wed, 3 Oct 2018 12:14:22 +0100
> Kieran Bingham <kieran.bingham@...asonboard.com> escreveu:
> 
>>> @@ -75,6 +76,8 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = {
>>>  	{  1, 5 },
>>>  	{  1, 10 },
>>>  	{  1, 15 },
>>> +	{  1, 15 },
>>> +	{  1, 25 },  
> 
> As the code requires that VIVID_WEBCAM_IVALS would be twice the number
> of resolutions, I understand why you're doing that.
> 
>> But won't this add duplicates of 25 and 15 FPS to all the frame sizes
>> smaller than 1280,720 ? Or are they filtered out?
> 
> However, I agree with Kieran: looking at the code, it sounds to me that
> it will indeed duplicate 1/15 and 1/25 intervals.

Oops, I missed this comment. Yes, you'll get duplicates which should be
avoided.

> 
> I suggest add two other intervals there, like:
> 	12.5 fps and 29.995 fps, e. g.:

29.995 is never used by webcams.

> 
> static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = {
>         {  1, 1 },
>         {  1, 2 },
>         {  1, 4 },
>         {  1, 5 },
>         {  1, 10 },
>         {  1, 15 },
> 	{  2, 50 },
>         {  1, 25 },
>         {  1, 30 },
>         {  1, 40 },
>         {  1, 50 },
> 	{  1001, 30000 },
>         {  1, 60 },
> };
> 
> Provided, of course, that vivid would support producing images
> at fractional rate. I didn't check. If not, then simply add
> 1/20 and 1/40.

vivid can do fractional rates (it does support this for the TV input),
but 29.995 makes no sense for a webcam. So 1/20 and 1/40 seems the
right approach.

> 
>> Now the difficulty is adding smaller frame rates (like 1,1, 1,2) would
>> effect/reduce the output rates of the larger frame sizes, so how about
>> adding some high rate support (any two from 1/{60,75,90,100,120}) instead?
> 
> Last week, I got a crash with vivid running at 30 fps, while running an 
> event's race code, on a i7core (there, the code was switching all video
> controls while subscribing/unsubscribing events). The same code worked
> with lower fps.

If you have a stack trace, then let me know.

> While I didn't have time to debug it yet, I suspect that it has to do
> with the time spent to produce a frame on vivid. So, while it would be
> nice to have high rate support, I'm not sure if this is doable. It may,
> but perhaps we need to disable some possible video output formats, as some
> types may consume more time to build frames.

In the end that depends on the CPU and what else is running. You'll know quickly
enough if the CPU isn't fast enough to support a format. Although it shouldn't
crash, of course.

Regards,

	Hans