2023-04-10 06:45:32

by Max Staudt

[permalink] [raw]
Subject: [PATCH v1] media: vivid: Add webcam parameter for (un)limited bandwidth

This adds an option for higher frame rates from a simulated webcam.

Currently, vivid emulates (amongst other things) a webcam with somewhat
limited bandwidth - higher resolutions deliver fewer frames per second:

$ yavta --enum-formats -c /dev/video0
Device /dev/video0 opened.
Device `vivid' on `platform:vivid-000' (driver 'vivid') supports video, capture, without mplanes.
- Available formats:
Format 0: YUYV (56595559)
Type: Video capture (1)
Name: YUYV 4:2:2
Frame size: 320x180 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
Frame size: 640x360 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40)
Frame size: 640x480 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25)
Frame size: 1280x720 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25)
Frame size: 1920x1080 (1/1, 1/2, 1/4, 1/5)
Frame size: 3840x2160 (1/1, 1/2)

In some test cases, it is useful to allow for higher frame rates, as
configurations such as 720p@30 FPS have become commonplace now.

With `webcam_bandwidth_limit=0` we get more options:

$ yavta --enum-formats -c /dev/video0
Device /dev/video0 opened.
Device `vivid' on `platform:vivid-000' (driver 'vivid') supports video, capture, without mplanes.
- Available formats:
Format 0: YUYV (56595559)
Type: Video capture (1)
Name: YUYV 4:2:2
Frame size: 320x180 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
Frame size: 640x360 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
Frame size: 640x480 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
Frame size: 1280x720 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
Frame size: 1920x1080 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
Frame size: 3840x2160 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)

Passes v4l2-compliance 1.25.0-5039 from v4l-utils git ccc08732823f

Signed-off-by: Max Staudt <[email protected]>
---
Documentation/admin-guide/media/vivid.rst | 25 +++++++++++++++++++
drivers/media/test-drivers/vivid/vivid-core.c | 8 ++++++
drivers/media/test-drivers/vivid/vivid-core.h | 1 +
.../media/test-drivers/vivid/vivid-vid-cap.c | 18 ++++++++++---
4 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/media/vivid.rst b/Documentation/admin-guide/media/vivid.rst
index 58ac25b2c385..e65067550efc 100644
--- a/Documentation/admin-guide/media/vivid.rst
+++ b/Documentation/admin-guide/media/vivid.rst
@@ -110,6 +110,28 @@ all configurable using the following module options:

num_inputs=8 input_types=0xffa9

+- webcam_bandwidth_limit:
+
+ whether a simulated webcam offers fewer frames per second for higher
+ resolutions. This only affects webcam inputs as selected in input_types
+ and is ignored for all other inputs. It affects all webcam inputs of
+ a vivid instance.
+
+ - 0: All predefined frame intervals available for all
+ predefined resolutions
+ - 1: Simulate limited bandwidth by removing two FPS rates
+ for each step up in resolution
+
+ The default is for all webcams to cap their FPS at high resolutions.
+ This maintains the behaviour known from earlier versions of vivid.
+
+ To enable all frame rates across all resolutions on webcam inputs, load
+ vivid with this option set to 0:
+
+ .. code-block:: none
+
+ webcam_bandwidth_limit=0
+
- num_outputs:

the number of outputs, one for each instance. By default 2 outputs
@@ -336,6 +358,9 @@ supports frames per second settings of 10, 15, 25, 30, 50 and 60 fps. Which ones
are available depends on the chosen framesize: the larger the framesize, the
lower the maximum frames per second.

+The FPS limit for higher resolutions can be disabled by passing the
+`webcam_bandwidth_limit=0` parameter.
+
The initially selected colorspace when you switch to the webcam input will be
sRGB.

diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
index f28440e6c9f8..720ffe470709 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.c
+++ b/drivers/media/test-drivers/vivid/vivid-core.c
@@ -143,6 +143,11 @@ MODULE_PARM_DESC(input_types, " input types, default is 0xe4. Two bits per input
"\t\t bits 0-1 == input 0, bits 31-30 == input 15.\n"
"\t\t Type 0 == webcam, 1 == TV, 2 == S-Video, 3 == HDMI");

+/* 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);
+MODULE_PARM_DESC(webcam_bandwidth_limit, " for webcam inputs, cap FPS at higher frame sizes (default: true).");
+
/* Default: 2 outputs */
static unsigned num_outputs[VIVID_MAX_DEVS] = { [0 ... (VIVID_MAX_DEVS - 1)] = 2 };
module_param_array(num_outputs, uint, NULL, 0444);
@@ -940,6 +945,9 @@ static int vivid_detect_feature_set(struct vivid_dev *dev, int inst,
v4l2_info(&dev->v4l2_dev, "using %splanar format API\n",
dev->multiplanar ? "multi" : "single ");

+ /* Are "webcam" type inputs of this instance rate limited? */
+ dev->webcam_bandwidth_limit = webcam_bandwidth_limit[inst];
+
/* how many inputs do we have and of what type? */
dev->num_inputs = num_inputs[inst];
if (node_type & 0x20007) {
diff --git a/drivers/media/test-drivers/vivid/vivid-core.h b/drivers/media/test-drivers/vivid/vivid-core.h
index 473f3598db5a..aa38988384e4 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.h
+++ b/drivers/media/test-drivers/vivid/vivid-core.h
@@ -186,6 +186,7 @@ struct vivid_dev {
unsigned int num_hdmi_outputs;
u8 output_type[MAX_OUTPUTS];
u8 output_name_counter[MAX_OUTPUTS];
+ bool webcam_bandwidth_limit;
bool has_audio_inputs;
bool has_audio_outputs;
bool has_vid_cap;
diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
index c0999581c599..347c51f36386 100644
--- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
@@ -79,6 +79,14 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = {
{ 1, 60 },
};

+static inline unsigned webcam_ival_count(const struct vivid_dev *dev,
+ unsigned frmsize_idx)
+{
+ return dev->webcam_bandwidth_limit ?
+ 2 * (VIVID_WEBCAM_SIZES - frmsize_idx) :
+ 2 * (VIVID_WEBCAM_SIZES);
+}
+
static int vid_cap_queue_setup(struct vb2_queue *vq,
unsigned *nbuffers, unsigned *nplanes,
unsigned sizes[], struct device *alloc_devs[])
@@ -773,14 +781,16 @@ int vivid_s_fmt_vid_cap(struct file *file, void *priv,
compose->height /= factor;
}
} else if (vivid_is_webcam(dev)) {
+ unsigned ival_sz = webcam_ival_count(dev, dev->webcam_size_idx);
+
/* Guaranteed to be a match */
for (i = 0; i < ARRAY_SIZE(webcam_sizes); i++)
if (webcam_sizes[i].width == mp->width &&
webcam_sizes[i].height == mp->height)
break;
dev->webcam_size_idx = i;
- if (dev->webcam_ival_idx >= 2 * (VIVID_WEBCAM_SIZES - i))
- dev->webcam_ival_idx = 2 * (VIVID_WEBCAM_SIZES - i) - 1;
+ if (dev->webcam_ival_idx >= ival_sz)
+ dev->webcam_ival_idx = ival_sz - 1;
vivid_update_format_cap(dev, false);
} else {
struct v4l2_rect r = { 0, 0, mp->width, mp->height };
@@ -1908,7 +1918,7 @@ int vidioc_enum_frameintervals(struct file *file, void *priv,
break;
if (i == ARRAY_SIZE(webcam_sizes))
return -EINVAL;
- if (fival->index >= 2 * (VIVID_WEBCAM_SIZES - i))
+ if (fival->index >= webcam_ival_count(dev, i))
return -EINVAL;
fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
fival->discrete = webcam_intervals[fival->index];
@@ -1935,7 +1945,7 @@ int vivid_vid_cap_s_parm(struct file *file, void *priv,
struct v4l2_streamparm *parm)
{
struct vivid_dev *dev = video_drvdata(file);
- unsigned ival_sz = 2 * (VIVID_WEBCAM_SIZES - dev->webcam_size_idx);
+ unsigned ival_sz = webcam_ival_count(dev, dev->webcam_size_idx);
struct v4l2_fract tpf;
unsigned i;

--
2.40.0.577.gac1e443424-goog


2023-04-10 09:27:06

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: Re: [PATCH v1] media: vivid: Add webcam parameter for (un)limited bandwidth

Em Mon, 10 Apr 2023 15:33:56 +0900
Max Staudt <[email protected]> escreveu:

> This adds an option for higher frame rates from a simulated webcam.
>
> Currently, vivid emulates (amongst other things) a webcam with somewhat
> limited bandwidth - higher resolutions deliver fewer frames per second:
>
> $ yavta --enum-formats -c /dev/video0
> Device /dev/video0 opened.
> Device `vivid' on `platform:vivid-000' (driver 'vivid') supports video, capture, without mplanes.
> - Available formats:
> Format 0: YUYV (56595559)
> Type: Video capture (1)
> Name: YUYV 4:2:2
> Frame size: 320x180 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
> Frame size: 640x360 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40)
> Frame size: 640x480 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25)
> Frame size: 1280x720 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25)
> Frame size: 1920x1080 (1/1, 1/2, 1/4, 1/5)
> Frame size: 3840x2160 (1/1, 1/2)
>
> In some test cases, it is useful to allow for higher frame rates, as
> configurations such as 720p@30 FPS have become commonplace now.
>
> With `webcam_bandwidth_limit=0` we get more options:
>
> $ yavta --enum-formats -c /dev/video0
> Device /dev/video0 opened.
> Device `vivid' on `platform:vivid-000' (driver 'vivid') supports video, capture, without mplanes.
> - Available formats:
> Format 0: YUYV (56595559)
> Type: Video capture (1)
> Name: YUYV 4:2:2
> Frame size: 320x180 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
> Frame size: 640x360 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
> Frame size: 640x480 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
> Frame size: 1280x720 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
> Frame size: 1920x1080 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
> Frame size: 3840x2160 (1/1, 1/2, 1/4, 1/5, 1/10, 2/25, 1/15, 1/25, 1/30, 1/40, 1/50, 1/60)
>
> Passes v4l2-compliance 1.25.0-5039 from v4l-utils git ccc08732823f
>
> Signed-off-by: Max Staudt <[email protected]>
> ---
> Documentation/admin-guide/media/vivid.rst | 25 +++++++++++++++++++
> drivers/media/test-drivers/vivid/vivid-core.c | 8 ++++++
> drivers/media/test-drivers/vivid/vivid-core.h | 1 +
> .../media/test-drivers/vivid/vivid-vid-cap.c | 18 ++++++++++---
> 4 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/media/vivid.rst b/Documentation/admin-guide/media/vivid.rst
> index 58ac25b2c385..e65067550efc 100644
> --- a/Documentation/admin-guide/media/vivid.rst
> +++ b/Documentation/admin-guide/media/vivid.rst
> @@ -110,6 +110,28 @@ all configurable using the following module options:
>
> num_inputs=8 input_types=0xffa9
>
> +- webcam_bandwidth_limit:
> +
> + whether a simulated webcam offers fewer frames per second for higher
> + resolutions. This only affects webcam inputs as selected in input_types
> + and is ignored for all other inputs. It affects all webcam inputs of
> + a vivid instance.
> +
> + - 0: All predefined frame intervals available for all
> + predefined resolutions
> + - 1: Simulate limited bandwidth by removing two FPS rates
> + for each step up in resolution

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.

Regards,
Mauro

> +
> + The default is for all webcams to cap their FPS at high resolutions.
> + This maintains the behaviour known from earlier versions of vivid.
> +
> + To enable all frame rates across all resolutions on webcam inputs, load
> + vivid with this option set to 0:
> +
> + .. code-block:: none
> +
> + webcam_bandwidth_limit=0
> +
> - num_outputs:
>
> the number of outputs, one for each instance. By default 2 outputs
> @@ -336,6 +358,9 @@ supports frames per second settings of 10, 15, 25, 30, 50 and 60 fps. Which ones
> are available depends on the chosen framesize: the larger the framesize, the
> lower the maximum frames per second.
>
> +The FPS limit for higher resolutions can be disabled by passing the
> +`webcam_bandwidth_limit=0` parameter.
> +
> The initially selected colorspace when you switch to the webcam input will be
> sRGB.
>
> diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
> index f28440e6c9f8..720ffe470709 100644
> --- a/drivers/media/test-drivers/vivid/vivid-core.c
> +++ b/drivers/media/test-drivers/vivid/vivid-core.c
> @@ -143,6 +143,11 @@ MODULE_PARM_DESC(input_types, " input types, default is 0xe4. Two bits per input
> "\t\t bits 0-1 == input 0, bits 31-30 == input 15.\n"
> "\t\t Type 0 == webcam, 1 == TV, 2 == S-Video, 3 == HDMI");
>
> +/* 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.

> +MODULE_PARM_DESC(webcam_bandwidth_limit, " for webcam inputs, cap FPS at higher frame sizes (default: true).");
> +
> /* Default: 2 outputs */
> static unsigned num_outputs[VIVID_MAX_DEVS] = { [0 ... (VIVID_MAX_DEVS - 1)] = 2 };
> module_param_array(num_outputs, uint, NULL, 0444);
> @@ -940,6 +945,9 @@ static int vivid_detect_feature_set(struct vivid_dev *dev, int inst,
> v4l2_info(&dev->v4l2_dev, "using %splanar format API\n",
> dev->multiplanar ? "multi" : "single ");
>
> + /* Are "webcam" type inputs of this instance rate limited? */
> + dev->webcam_bandwidth_limit = webcam_bandwidth_limit[inst];
> +
> /* how many inputs do we have and of what type? */
> dev->num_inputs = num_inputs[inst];
> if (node_type & 0x20007) {
> diff --git a/drivers/media/test-drivers/vivid/vivid-core.h b/drivers/media/test-drivers/vivid/vivid-core.h
> index 473f3598db5a..aa38988384e4 100644
> --- a/drivers/media/test-drivers/vivid/vivid-core.h
> +++ b/drivers/media/test-drivers/vivid/vivid-core.h
> @@ -186,6 +186,7 @@ struct vivid_dev {
> unsigned int num_hdmi_outputs;
> u8 output_type[MAX_OUTPUTS];
> u8 output_name_counter[MAX_OUTPUTS];
> + bool webcam_bandwidth_limit;
> bool has_audio_inputs;
> bool has_audio_outputs;
> bool has_vid_cap;
> diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> index c0999581c599..347c51f36386 100644
> --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
> @@ -79,6 +79,14 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = {
> { 1, 60 },
> };
>
> +static inline unsigned webcam_ival_count(const struct vivid_dev *dev,
> + unsigned frmsize_idx)
> +{
> + return dev->webcam_bandwidth_limit ?
> + 2 * (VIVID_WEBCAM_SIZES - frmsize_idx) :
> + 2 * (VIVID_WEBCAM_SIZES);
> +}
> +
> static int vid_cap_queue_setup(struct vb2_queue *vq,
> unsigned *nbuffers, unsigned *nplanes,
> unsigned sizes[], struct device *alloc_devs[])
> @@ -773,14 +781,16 @@ int vivid_s_fmt_vid_cap(struct file *file, void *priv,
> compose->height /= factor;
> }
> } else if (vivid_is_webcam(dev)) {
> + unsigned ival_sz = webcam_ival_count(dev, dev->webcam_size_idx);
> +
> /* Guaranteed to be a match */
> for (i = 0; i < ARRAY_SIZE(webcam_sizes); i++)
> if (webcam_sizes[i].width == mp->width &&
> webcam_sizes[i].height == mp->height)
> break;
> dev->webcam_size_idx = i;
> - if (dev->webcam_ival_idx >= 2 * (VIVID_WEBCAM_SIZES - i))
> - dev->webcam_ival_idx = 2 * (VIVID_WEBCAM_SIZES - i) - 1;
> + if (dev->webcam_ival_idx >= ival_sz)
> + dev->webcam_ival_idx = ival_sz - 1;
> vivid_update_format_cap(dev, false);
> } else {
> struct v4l2_rect r = { 0, 0, mp->width, mp->height };
> @@ -1908,7 +1918,7 @@ int vidioc_enum_frameintervals(struct file *file, void *priv,
> break;
> if (i == ARRAY_SIZE(webcam_sizes))
> return -EINVAL;
> - if (fival->index >= 2 * (VIVID_WEBCAM_SIZES - i))
> + if (fival->index >= webcam_ival_count(dev, i))
> return -EINVAL;
> fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> fival->discrete = webcam_intervals[fival->index];
> @@ -1935,7 +1945,7 @@ int vivid_vid_cap_s_parm(struct file *file, void *priv,
> struct v4l2_streamparm *parm)
> {
> struct vivid_dev *dev = video_drvdata(file);
> - unsigned ival_sz = 2 * (VIVID_WEBCAM_SIZES - dev->webcam_size_idx);
> + unsigned ival_sz = webcam_ival_count(dev, dev->webcam_size_idx);
> struct v4l2_fract tpf;
> unsigned i;
>

2023-04-11 06:55:03

by Max Staudt

[permalink] [raw]
Subject: Re: [PATCH v1] media: vivid: Add webcam parameter for (un)limited bandwidth

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...



Max

2023-04-11 07:30:04

by Hans Verkuil

[permalink] [raw]
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

2023-04-11 07:32:54

by Max Staudt

[permalink] [raw]
Subject: Re: [PATCH v1] media: vivid: Add webcam parameter for (un)limited bandwidth

On 4/11/23 16:26, Hans Verkuil wrote:
> 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.

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?

I'm happy to prepare an alternative patch for that, too.



Max

2023-04-11 07:45:48

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v1] media: vivid: Add webcam parameter for (un)limited bandwidth

On 11/04/2023 09:31, Max Staudt wrote:
> On 4/11/23 16:26, Hans Verkuil wrote:
>> 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.
>
> 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.

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).

Regards,

Hans

>
> I'm happy to prepare an alternative patch for that, too.
>
>
>
> Max
>

2023-04-11 11:36:12

by Max Staudt

[permalink] [raw]
Subject: Re: [PATCH v1] media: vivid: Add webcam parameter for (un)limited bandwidth

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