linux-kernel - Re: [PATCH v1] media: vivid: Add webcam parameter for (un)limited bandwidth

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: <6ee01cf1-5a8b-081f-e218-8c7da39343bc@xs4all.nl>
Date:   Tue, 11 Apr 2023 09:26:48 +0200
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Max Staudt <mstaudt@...omium.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Ricardo Ribalda <ribalda@...omium.org>,
        Yunke Cao <yunkec@...omium.org>,
        Tomasz Figa <tfiga@...omium.org>
Subject: Re: [PATCH v1] media: vivid: Add webcam parameter for (un)limited
 bandwidth

On 11/04/2023 08:51, Max Staudt wrote:
> Thank you Mauro for having a first look!
> 
> Questions below.
> 
> 
> On 4/10/23 18:23, Mauro Carvalho Chehab wrote:
>> IMO, instead of a parameter that just enables/disables the bandwidth
>> limit, the best would be to have a parameter specifying the bandwidth
>> (with 0 meaning unlimited).
>>
>> If not used, vivid would initialize it to dev->webcam_bandwidth_limit,
>> so a read operation will show the current limit.
> Up until now, the bandwidth limit is a rather arbitrary reduction of two interval sizes per frame size.
> 
> How would you prefer to define a limited bandwidth in this parameter? How would it affect the simulated camera, do you have a suggestion for a formula from bandwidth to frame/interval sizes offered?
> 
> 
>>> +/* Default: limited webcam bandwidth */
>>> +static bool webcam_bandwidth_limit[VIVID_MAX_DEVS] = { [0 ... (VIVID_MAX_DEVS - 1)] = true };
>>> +module_param_array(webcam_bandwidth_limit, bool, NULL, 0444);
>>
>> I would also use 0666, to allow changing this on runtime.
> 
> I guess that's possible, though it would add complexity.
> 
> Currently we can ask for two instances, each with a different setting:
> 
>   n_devs=2 webcam_bandwidth_limit=1,0
> 
> This creates /dev/video0 which is limited, and /dev/video4 which is unlimited.
> 
> Maybe this already sufficiently covers the case you are looking for, and we can keep the complexity low? A real webcam won't suddenly offer new frame rates either...

I think we either use this bandwidth option and calculate the max fps based on
that (basically the bandwidth divided by (image_size + some blanking factor)),
or we keep it simple and instead of going down two steps in fps we allow up to
60 fps up to 720p, then 30 fps for 1080p and 15 fps for 4k.

The fps values currently used are a bit outdated w.r.t. modern webcams, so
upgrading it wouldn't hurt. And this is a lot simpler than doing bandwidth
calculations.

Regards,

	Hans