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

Kieran Bingham
2018-10-08T08:35:10Z

On 06/10/18 11:29, Keiichi Watanabe wrote:
> Hi all,
>
> On Fri, Oct 5, 2018 at 6:18 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> On 10/03/18 09:08, Keiichi Watanabe wrote:
>>> I think 480p is a common frame size and it's worth supporting in vivid.
>>> But, my patch might be ad-hoc. Actually, I'm not sure which values are
>>> suitable for the intervals.
>>
>> I can apply this ad-hoc patch as-is.
>>
>> Or do you want to postpone this and work on a more generic solution?
>> Although I am not sure what that would look like.
>
> I prefer providing a more flexible way rather than this ad-hoc patch.
> It would be helpful if there is a way of changing supported frame sizes easily.
> Perhaps, Kieran and me would use it, at least:)
>

o/ <raising hand in agreement>

>>
>> Proposals are welcome!
>>
>> The main purpose of this code is to have something that kind of acts like
>> a real webcam that has various resolutions, and a slower framerate for
>> higher resolutions (as you would expect).
>
> This sounds reasonable, so I guess we can keep this way as default and
> provide a way for specifying extra frame sizes as an option.
>
> For example, how about a module option like this?
> "webcam_sizes=640x480@15,320x240@30"
>
> If this parameter is passed to vivid, vivid works like a webcam that
> supports the following 7 pairs of frame size and fps:
> - 5 pairs of frame sizes and fps, as with the current implementation
> - 640x480 (15fps)
> - 320x240 (30fps)

I like the concept of being able to specify as a module parameter.

Perhaps we could have a magic marker on the string to define if the
existing frame sizes should be kept - or if this is just a complete
override ?

vivid.webcam=640x480@15,320x240@30 # Specify sizes explicitly
vivid.webcam=+640x480@15,320x240@30 # Append to existing frames
^ Magic Marker

We might of course want multiple rates per frame,

vivid.webcam=640x480@15-25-30-90-120, # Separator to be defined...

>
> If this parameter is not passed, vivid's behavior won't change from
> the current one.
>
> How do you think?
>
> We might want to stop using the default framesizes, i.e. vivid only
> supports framesize/fps that passed as the option.
> But, if we do so, the parameter will become mandatory and it would be annoying.

I think mandatory would be annoying yes.

Thus my suggestion for a magic marker above :)

--
Kieran

> So, I personally like to keep the default framesizes.
>
> Best regards,
> Kei
>
>>
>> Regards,
>>
>> Hans
>>
>>>
>>> We might want to add a more flexible/extensible way to specify frame sizes.
>>> e.g. passing frame sizes and intervals as module parameters
>>>
>>> Kei
>>>
>>> On Wed, Oct 3, 2018 at 4:06 PM, Keiichi Watanabe <keiichiw@chromium.org> wrote:
>>>> Support 640x480 as a frame size for video input devices of vivid.
>>>>
>>>> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
>>>> ---
>>>> drivers/media/platform/vivid/vivid-vid-cap.c | 5 ++++-
>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
>>>> index 58e14dd1dcd3..da80bf4bc365 100644
>>>> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
>>>> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
>>>> @@ -51,7 +51,7 @@ static const struct vivid_fmt formats_ovl[] = {
>>>> };
>>>>
>>>> /* The number of discrete webcam framesizes */
>>>> -#define VIVID_WEBCAM_SIZES 5
>>>> +#define VIVID_WEBCAM_SIZES 6
>>>> /* The number of discrete webcam frameintervals */
>>>> #define VIVID_WEBCAM_IVALS (VIVID_WEBCAM_SIZES * 2)
>>>>
>>>> @@ -59,6 +59,7 @@ static const struct vivid_fmt formats_ovl[] = {
>>>> static const struct v4l2_frmsize_discrete webcam_sizes[VIVID_WEBCAM_SIZES] = {
>>>> { 320, 180 },
>>>> { 640, 360 },
>>>> + { 640, 480 },
>>>> { 1280, 720 },
>>>> { 1920, 1080 },
>>>> { 3840, 2160 },
>>>> @@ -75,6 +76,8 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = {
>>>> { 1, 5 },
>>>> { 1, 10 },
>>>> { 1, 15 },
>>>> + { 1, 15 },
>>>> + { 1, 25 },
>>>> { 1, 25 },
>>>> { 1, 30 },
>>>> { 1, 50 },
>>>> --
>>>> 2.19.0.605.g01d371f741-goog
>>>>
>>

--
Regards
--
Kieran