On 06/10/18 11:29, Keiichi Watanabe wrote: > Hi all, > > On Fri, Oct 5, 2018 at 6:18 PM, Hans Verkuil <hverkuil@xxxxxxxxx> 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@xxxxxxxxxxxx> wrote: >>>> Support 640x480 as a frame size for video input devices of vivid. >>>> >>>> Signed-off-by: Keiichi Watanabe <keiichiw@xxxxxxxxxxxx> >>>> --- >>>> 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