Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <8abf9651-5ce1-7b73-6d14-04f42081a743@chromium.org>
Date:   Tue, 11 Apr 2023 20:34:29 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v1] media: vivid: Add webcam parameter for (un)limited
 bandwidth
Content-Language: en-US
To:     Hans Verkuil <hverkuil@xs4all.nl>,
        Mauro Carvalho Chehab <mchehab@kernel.org>
Cc:     linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
        Ricardo Ribalda <ribalda@chromium.org>,
        Yunke Cao <yunkec@chromium.org>,
        Tomasz Figa <tfiga@chromium.org>
References: <20230410063356.3894767-1-mstaudt@chromium.org>
 <20230410102350.382f7d02@sal.lan>
 <6aafad18-13a2-ef45-48a1-1f094554af31@chromium.org>
 <6ee01cf1-5a8b-081f-e218-8c7da39343bc@xs4all.nl>
 <c6d5be4c-42c9-b8fa-fbd7-108c5da694bc@chromium.org>
 <5649adcd-3afe-e413-2eac-a92c78427dc9@xs4all.nl>
From:   Max Staudt <mstaudt@chromium.org>
In-Reply-To: <5649adcd-3afe-e413-2eac-a92c78427dc9@xs4all.nl>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/11/23 16:45, Hans Verkuil wrote:
>> Do I understand you correctly, are you suggesting to simply update the FPS limits to a new fixed schema, and not have an option at all?
> 
> Correct.
> 
> The ideal solution is indeed proper bandwidth calculations, since this would
> be a proper emulation of actual webcam hardware. If you have time and are
> interested in doing the work, then that would be great, of course.

My patch suggestion is motivated by a test which expects higher FPS 
rates at large frame sizes than vivid currently provides.

If I have a choice, then let's keep this patch simple ;)


> But if you just want to increase the fps limits to be more in line with
> modern webcams, then that's much quicker and should be fine.
> 
> It might also be interesting to perhaps allow for 120 fps for the low
> resolutions (below 720p).

Coincidentally, this would solve the problem we have at hand, but only 
until someone wants to see even higher frame rates, and then we'd 
revisit today's thread. Hence the idea for a parameter to simply unlock 
all rates, depending on whether a test needs a vivid webcam with low or 
high "performance".


My rationale behind the module parameter is twofold:

1. To allow for higher FPS without touching the kernel code again.
2. To stay backward compatible to previous behaviour of vivid. Changing 
the FPS formula would break this.


Actually I have a new idea: How about renaming my suggestion to 
"webcam_limit_enable" - this way, we can keep the current design with 
the boolean value: Not setting this value would default to "enabled" - 
i.e. vivid behaves as before. Disabling the limit unlocks all FPS at all 
sizes.

And later, should the need for a more precise simulation arise, we can 
add a second parameter "webcam_limit_value".

I've removed the "bandwidth" word from the new suggestions, so if a 
numeric limit is introduced, it can be anything, even an arbitrary 
number like the current "remove 2 FPS rates per size".


Please let me know what you think.

Max