* [PATCH] media: vivid: Support 480p for webcam capture @ 2018-10-03 7:06 Keiichi Watanabe 2018-10-03 7:08 ` Keiichi Watanabe 2018-10-03 11:14 ` Kieran Bingham 0 siblings, 2 replies; 17+ messages in thread From: Keiichi Watanabe @ 2018-10-03 7:06 UTC (permalink / raw) To: linux-media Cc: Hans Verkuil, Mauro Carvalho Chehab, linux-kernel, tfiga, jcliang, shik, keiichiw 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-03 7:06 [PATCH] media: vivid: Support 480p for webcam capture Keiichi Watanabe @ 2018-10-03 7:08 ` Keiichi Watanabe 2018-10-03 11:16 ` Kieran Bingham 2018-10-05 9:18 ` Hans Verkuil 2018-10-03 11:14 ` Kieran Bingham 1 sibling, 2 replies; 17+ messages in thread From: Keiichi Watanabe @ 2018-10-03 7:08 UTC (permalink / raw) To: Linux Media Mailing List Cc: Hans Verkuil, Mauro Carvalho Chehab, Linux Kernel Mailing List, Tomasz Figa, Ricky Liang, Shik Chen, Keiichi Watanabe 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. 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 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-03 7:08 ` Keiichi Watanabe @ 2018-10-03 11:16 ` Kieran Bingham 2018-10-05 9:18 ` Hans Verkuil 1 sibling, 0 replies; 17+ messages in thread From: Kieran Bingham @ 2018-10-03 11:16 UTC (permalink / raw) To: Keiichi Watanabe, Linux Media Mailing List Cc: Hans Verkuil, Mauro Carvalho Chehab, Linux Kernel Mailing List, Tomasz Figa, Ricky Liang, Shik Chen Hi Kei, On 03/10/18 08: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. Aha - yes I did think the duplicates were a bit odd. Anyway, replied inline there :) > We might want to add a more flexible/extensible way to specify frame sizes. > e.g. passing frame sizes and intervals as module parameters I agree here, having recently extended this table my self with some local patches it would be great to be able to request some more arbitrary sizes and rates from VIVID in a more flexible manner. Regards Kieran > > 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 >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-03 7:08 ` Keiichi Watanabe 2018-10-03 11:16 ` Kieran Bingham @ 2018-10-05 9:18 ` Hans Verkuil 2018-10-06 10:29 ` Keiichi Watanabe 1 sibling, 1 reply; 17+ messages in thread From: Hans Verkuil @ 2018-10-05 9:18 UTC (permalink / raw) To: Keiichi Watanabe, Linux Media Mailing List Cc: Mauro Carvalho Chehab, Linux Kernel Mailing List, Tomasz Figa, Ricky Liang, Shik Chen 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. 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). 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 >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-05 9:18 ` Hans Verkuil @ 2018-10-06 10:29 ` Keiichi Watanabe 2018-10-08 8:35 ` Kieran Bingham 0 siblings, 1 reply; 17+ messages in thread From: Keiichi Watanabe @ 2018-10-06 10:29 UTC (permalink / raw) To: Hans Verkuil Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Linux Kernel Mailing List, Tomasz Figa, Ricky Liang, Shik Chen 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:) > > 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) 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. 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 >>> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-06 10:29 ` Keiichi Watanabe @ 2018-10-08 8:35 ` Kieran Bingham 2018-10-08 9:00 ` Hans Verkuil 0 siblings, 1 reply; 17+ messages in thread From: Kieran Bingham @ 2018-10-08 8:35 UTC (permalink / raw) To: Keiichi Watanabe, Hans Verkuil Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Linux Kernel Mailing List, Tomasz Figa, Ricky Liang, Shik Chen 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-08 8:35 ` Kieran Bingham @ 2018-10-08 9:00 ` Hans Verkuil 2018-10-08 9:47 ` Kieran Bingham 0 siblings, 1 reply; 17+ messages in thread From: Hans Verkuil @ 2018-10-08 9:00 UTC (permalink / raw) To: kieran.bingham, Keiichi Watanabe Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Linux Kernel Mailing List, Tomasz Figa, Ricky Liang, Shik Chen On 10/08/2018 10:35 AM, Kieran Bingham wrote: > 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 ? No, just override. It's hard otherwise since you would have to keep the lists sorted by resolution/fps. If you want to set it yourself, then just do 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... Stick to @, so: vivid.webcam=640x480@15@25@30@90@120 This looks like a good proposal to me. BTW, I still would like to add 640x480 added to the default list: it's a common resolution and it makes sense to add it. So I plan to accept the patch regardless. Regards, Hans > > >> >> 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 >>>>> >>> > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-08 9:00 ` Hans Verkuil @ 2018-10-08 9:47 ` Kieran Bingham 0 siblings, 0 replies; 17+ messages in thread From: Kieran Bingham @ 2018-10-08 9:47 UTC (permalink / raw) To: Hans Verkuil, Keiichi Watanabe Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Linux Kernel Mailing List, Tomasz Figa, Ricky Liang, Shik Chen On 08/10/18 10:00, Hans Verkuil wrote: > On 10/08/2018 10:35 AM, Kieran Bingham wrote: >> 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 ? > > No, just override. It's hard otherwise since you would have to keep the > lists sorted by resolution/fps. > > If you want to set it yourself, then just do a complete override. OK. That's reasonable for me. @@ -9,3 +9,0 @@ suggestion - magic-marker >> 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... > > Stick to @, so: vivid.webcam=640x480@15@25@30@90@120 Yes, of course that makes a lot more sense now I see it :-) -- Kieran > This looks like a good proposal to me. > > BTW, I still would like to add 640x480 added to the default list: > it's a common resolution and it makes sense to add it. > > So I plan to accept the patch regardless. > > Regards, > > Hans > >> >> >>> >>> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-03 7:06 [PATCH] media: vivid: Support 480p for webcam capture Keiichi Watanabe 2018-10-03 7:08 ` Keiichi Watanabe @ 2018-10-03 11:14 ` Kieran Bingham 2018-10-08 17:03 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 17+ messages in thread From: Kieran Bingham @ 2018-10-03 11:14 UTC (permalink / raw) To: Keiichi Watanabe, linux-media Cc: Hans Verkuil, Mauro Carvalho Chehab, linux-kernel, tfiga, jcliang, shik Hi Keiichi, On 03/10/18 08:06, Keiichi Watanabe 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 }, I agree this is a useful frame size to add... (having got local patches which also update these tables) > { 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 }, But won't this add duplicates of 25 and 15 FPS to all the frame sizes smaller than 1280,720 ? Or are they filtered out? Now the difficulty is adding smaller frame rates (like 1,1, 1,2) would effect/reduce the output rates of the larger frame sizes, so how about adding some high rate support (any two from 1/{60,75,90,100,120}) instead? Regards Kieran Bingham > { 1, 25 }, > { 1, 30 }, > { 1, 50 }, > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-03 11:14 ` Kieran Bingham @ 2018-10-08 17:03 ` Mauro Carvalho Chehab 2018-10-08 17:53 ` Hans Verkuil 0 siblings, 1 reply; 17+ messages in thread From: Mauro Carvalho Chehab @ 2018-10-08 17:03 UTC (permalink / raw) To: Kieran Bingham Cc: Keiichi Watanabe, linux-media, Hans Verkuil, Mauro Carvalho Chehab, linux-kernel, tfiga, jcliang, shik Em Wed, 3 Oct 2018 12:14:22 +0100 Kieran Bingham <kieran.bingham@ideasonboard.com> escreveu: > > @@ -75,6 +76,8 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { > > { 1, 5 }, > > { 1, 10 }, > > { 1, 15 }, > > + { 1, 15 }, > > + { 1, 25 }, As the code requires that VIVID_WEBCAM_IVALS would be twice the number of resolutions, I understand why you're doing that. > But won't this add duplicates of 25 and 15 FPS to all the frame sizes > smaller than 1280,720 ? Or are they filtered out? However, I agree with Kieran: looking at the code, it sounds to me that it will indeed duplicate 1/15 and 1/25 intervals. I suggest add two other intervals there, like: 12.5 fps and 29.995 fps, e. g.: static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { { 1, 1 }, { 1, 2 }, { 1, 4 }, { 1, 5 }, { 1, 10 }, { 1, 15 }, { 2, 50 }, { 1, 25 }, { 1, 30 }, { 1, 40 }, { 1, 50 }, { 1001, 30000 }, { 1, 60 }, }; Provided, of course, that vivid would support producing images at fractional rate. I didn't check. If not, then simply add 1/20 and 1/40. > Now the difficulty is adding smaller frame rates (like 1,1, 1,2) would > effect/reduce the output rates of the larger frame sizes, so how about > adding some high rate support (any two from 1/{60,75,90,100,120}) instead? Last week, I got a crash with vivid running at 30 fps, while running an event's race code, on a i7core (there, the code was switching all video controls while subscribing/unsubscribing events). The same code worked with lower fps. While I didn't have time to debug it yet, I suspect that it has to do with the time spent to produce a frame on vivid. So, while it would be nice to have high rate support, I'm not sure if this is doable. It may, but perhaps we need to disable some possible video output formats, as some types may consume more time to build frames. Thanks, Mauro ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-08 17:03 ` Mauro Carvalho Chehab @ 2018-10-08 17:53 ` Hans Verkuil 2018-10-08 18:23 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 17+ messages in thread From: Hans Verkuil @ 2018-10-08 17:53 UTC (permalink / raw) To: Mauro Carvalho Chehab, Kieran Bingham Cc: Keiichi Watanabe, linux-media, Mauro Carvalho Chehab, linux-kernel, tfiga, jcliang, shik On 10/08/2018 07:03 PM, Mauro Carvalho Chehab wrote: > Em Wed, 3 Oct 2018 12:14:22 +0100 > Kieran Bingham <kieran.bingham@ideasonboard.com> escreveu: > >>> @@ -75,6 +76,8 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { >>> { 1, 5 }, >>> { 1, 10 }, >>> { 1, 15 }, >>> + { 1, 15 }, >>> + { 1, 25 }, > > As the code requires that VIVID_WEBCAM_IVALS would be twice the number > of resolutions, I understand why you're doing that. > >> But won't this add duplicates of 25 and 15 FPS to all the frame sizes >> smaller than 1280,720 ? Or are they filtered out? > > However, I agree with Kieran: looking at the code, it sounds to me that > it will indeed duplicate 1/15 and 1/25 intervals. Oops, I missed this comment. Yes, you'll get duplicates which should be avoided. > > I suggest add two other intervals there, like: > 12.5 fps and 29.995 fps, e. g.: 29.995 is never used by webcams. > > static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { > { 1, 1 }, > { 1, 2 }, > { 1, 4 }, > { 1, 5 }, > { 1, 10 }, > { 1, 15 }, > { 2, 50 }, > { 1, 25 }, > { 1, 30 }, > { 1, 40 }, > { 1, 50 }, > { 1001, 30000 }, > { 1, 60 }, > }; > > Provided, of course, that vivid would support producing images > at fractional rate. I didn't check. If not, then simply add > 1/20 and 1/40. vivid can do fractional rates (it does support this for the TV input), but 29.995 makes no sense for a webcam. So 1/20 and 1/40 seems the right approach. > >> Now the difficulty is adding smaller frame rates (like 1,1, 1,2) would >> effect/reduce the output rates of the larger frame sizes, so how about >> adding some high rate support (any two from 1/{60,75,90,100,120}) instead? > > Last week, I got a crash with vivid running at 30 fps, while running an > event's race code, on a i7core (there, the code was switching all video > controls while subscribing/unsubscribing events). The same code worked > with lower fps. If you have a stack trace, then let me know. > While I didn't have time to debug it yet, I suspect that it has to do > with the time spent to produce a frame on vivid. So, while it would be > nice to have high rate support, I'm not sure if this is doable. It may, > but perhaps we need to disable some possible video output formats, as some > types may consume more time to build frames. In the end that depends on the CPU and what else is running. You'll know quickly enough if the CPU isn't fast enough to support a format. Although it shouldn't crash, of course. Regards, Hans ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-08 17:53 ` Hans Verkuil @ 2018-10-08 18:23 ` Mauro Carvalho Chehab 2018-10-08 18:28 ` Mauro Carvalho Chehab 2018-10-08 18:31 ` Hans Verkuil 0 siblings, 2 replies; 17+ messages in thread From: Mauro Carvalho Chehab @ 2018-10-08 18:23 UTC (permalink / raw) To: Hans Verkuil Cc: Kieran Bingham, Keiichi Watanabe, linux-media, Mauro Carvalho Chehab, linux-kernel, tfiga, jcliang, shik Em Mon, 8 Oct 2018 19:53:38 +0200 Hans Verkuil <hverkuil@xs4all.nl> escreveu: > On 10/08/2018 07:03 PM, Mauro Carvalho Chehab wrote: > > Em Wed, 3 Oct 2018 12:14:22 +0100 > > Kieran Bingham <kieran.bingham@ideasonboard.com> escreveu: > > > >>> @@ -75,6 +76,8 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { > >>> { 1, 5 }, > >>> { 1, 10 }, > >>> { 1, 15 }, > >>> + { 1, 15 }, > >>> + { 1, 25 }, > > > > As the code requires that VIVID_WEBCAM_IVALS would be twice the number > > of resolutions, I understand why you're doing that. > > > >> But won't this add duplicates of 25 and 15 FPS to all the frame sizes > >> smaller than 1280,720 ? Or are they filtered out? > > > > However, I agree with Kieran: looking at the code, it sounds to me that > > it will indeed duplicate 1/15 and 1/25 intervals. > > Oops, I missed this comment. Yes, you'll get duplicates which should be > avoided. > > > > > I suggest add two other intervals there, like: > > 12.5 fps and 29.995 fps, e. g.: > > 29.995 is never used by webcams. > > > > > static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { > > { 1, 1 }, > > { 1, 2 }, > > { 1, 4 }, > > { 1, 5 }, > > { 1, 10 }, > > { 1, 15 }, > > { 2, 50 }, > > { 1, 25 }, > > { 1, 30 }, > > { 1, 40 }, > > { 1, 50 }, > > { 1001, 30000 }, > > { 1, 60 }, > > }; > > > > Provided, of course, that vivid would support producing images > > at fractional rate. I didn't check. If not, then simply add > > 1/20 and 1/40. > > vivid can do fractional rates (it does support this for the TV input), > but 29.995 makes no sense for a webcam. Yes, I know. > So 1/20 and 1/40 seems the > right approach. I would have 1/12.5 at least. I have some webcams here whose seem to use things like that under bad light, and it sounds interesting to have at least one fraction that doesn't start with "1", in order to be sure that camera apps are doing the right thing. > >> Now the difficulty is adding smaller frame rates (like 1,1, 1,2) would > >> effect/reduce the output rates of the larger frame sizes, so how about > >> adding some high rate support (any two from 1/{60,75,90,100,120}) instead? > > > > Last week, I got a crash with vivid running at 30 fps, while running an > > event's race code, on a i7core (there, the code was switching all video > > controls while subscribing/unsubscribing events). The same code worked > > with lower fps. > > If you have a stack trace, then let me know. See at the end. I intend to do further tests when I have some time. > > > While I didn't have time to debug it yet, I suspect that it has to do > > with the time spent to produce a frame on vivid. So, while it would be > > nice to have high rate support, I'm not sure if this is doable. It may, > > but perhaps we need to disable some possible video output formats, as some > > types may consume more time to build frames. > > In the end that depends on the CPU and what else is running. You'll know quickly > enough if the CPU isn't fast enough to support a format. Although it shouldn't > crash, of course. Yes, but on this case, it caused an OOPS (with KASAN enabled). I was running the stress test on one VT while using qv4l2 to stream. When I changed the resolution, it caused the OOPS. This is one example. I ran the race test first. It placed all controls at some random state (only issued VIDIOC_EXT_CTRLS - kept everything else on default). when asked qv4l2 to start streaming (5fps), got this: [348569.866967] BUG: unable to handle kernel paging request at ffffc90303ff73b5 [348569.867070] PGD 406ee8067 P4D 406ee8067 PUD 0 [348569.867081] Oops: 0002 [#1] SMP KASAN [348569.867089] CPU: 2 PID: 4365 Comm: vivid-000-vid-c Tainted: G B 4.19.0-rc1+ #3 [348569.867098] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949 05/11/2017 [348569.867113] RIP: 0010:tpg_print_str_6+0x241/0x960 [v4l2_tpg] [348569.867122] Code: 24 18 e9 a5 01 00 00 84 c9 0f 84 48 03 00 00 40 84 ed 48 8d 7b 15 be 02 00 00 00 0f 88 cb 06 00 00 e8 a3 cd 2f ec 48 8d 7b 17 <66> 44 89 73 15 e8 b5 c7 2f ec 44 88 6b 17 40 f6 c5 40 48 8d 7b 12 [348569.867136] RSP: 0018:ffff88024b6cf8c8 EFLAGS: 00010246 [348569.867144] RAX: fffff520607fee77 RBX: ffffc90303ff73a0 RCX: ffffffffc10c22cd [348569.867152] RDX: 0000000000000001 RSI: 0000000000000002 RDI: ffffc90303ff73b7 [348569.867162] RBP: 0000000000000000 R08: fffff520607fee77 R09: fffff520607fee77 [348569.867170] R10: 0000000000000001 R11: fffff520607fee76 R12: ffff88024b6cfcf1 [348569.867179] R13: 0000000000000010 R14: 0000000000001010 R15: 00000000000000ea [348569.867189] FS: 0000000000000000(0000) GS:ffff880407300000(0000) knlGS:0000000000000000 [348569.867198] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [348569.867206] CR2: ffffc90303ff73b5 CR3: 0000000346c0d004 CR4: 00000000003606e0 [348569.867214] Call Trace: [348569.867227] tpg_gen_text+0x258/0x2b0 [v4l2_tpg] [348569.867249] vivid_fillbuff+0x1e9b/0x30b0 [vivid] [348569.867261] ? __list_add_valid+0x29/0x90 [348569.867271] ? __switch_to+0x345/0x700 [348569.867281] ? osq_unlock+0x6b/0xf0 [348569.867302] ? vivid_grab_controls+0x60/0x60 [vivid] [348569.867312] ? del_timer_sync+0x3e/0x50 [348569.867320] ? schedule_timeout+0x234/0x4e0 [348569.867330] ? mutex_lock+0xbd/0xc0 [348569.867337] ? mutex_lock+0xbd/0xc0 [348569.867345] ? __mutex_lock_slowpath+0x10/0x10 [348569.867365] ? vivid_thread_vid_cap+0x5b6/0xf20 [vivid] [348569.867385] vivid_thread_vid_cap+0x5b6/0xf20 [vivid] [348569.867396] ? __sched_text_start+0x8/0x8 [348569.867404] ? __wake_up_common+0x9c/0x230 [348569.867413] ? __kthread_parkme+0x77/0x90 [348569.867432] ? vivid_fillbuff+0x30b0/0x30b0 [vivid] [348569.867440] kthread+0x1ac/0x1d0 [348569.867448] ? kthread_create_worker_on_cpu+0xc0/0xc0 [348569.867458] ret_from_fork+0x1f/0x30 [348569.867465] Modules linked in: vivid videobuf2_dma_contig v4l2_tpg v4l2_dv_timings videobuf2_v4l2 videobuf2_vmalloc videobuf2_memops videobuf2_common v4l2_common videode v media xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c tun bridge stp llc ebtable _filter ebtables ip6table_filter ip6_tables bluetooth rfkill ecdh_generic snd_hda_codec_hdmi i915 snd_hda_intel snd_usb_audio snd_hda_codec intel_rapl snd_usbmidi_lib x86_pk g_temp_thermal snd_hda_core i2c_algo_bit snd_hwdep intel_powerclamp coretemp snd_pcm snd_seq_midi drm_kms_helper crct10dif_pclmul snd_seq_midi_event crc32_pclmul snd_rawmidi ghash_clmulni_intel intel_cstate snd_seq intel_uncore drm intel_rapl_perf snd_seq_device snd_timer e1000e snd ptp mei_me [348569.867574] video mei soundcore pps_core lpc_ich fuse binfmt_misc kvm_intel kvm irqbypass crc32c_intel [last unloaded: videobuf2_memops] [348569.867597] CR2: ffffc90303ff73b5 [348569.867604] ---[ end trace b85f80398f88914d ]--- [348569.867615] RIP: 0010:tpg_print_str_6+0x241/0x960 [v4l2_tpg] [348569.867624] Code: 24 18 e9 a5 01 00 00 84 c9 0f 84 48 03 00 00 40 84 ed 48 8d 7b 15 be 02 00 00 00 0f 88 cb 06 00 00 e8 a3 cd 2f ec 48 8d 7b 17 <66> 44 89 73 15 e8 b5 c7 2f ec 44 88 6b 17 40 f6 c5 40 48 8d 7b 12 [348569.867639] RSP: 0018:ffff88024b6cf8c8 EFLAGS: 00010246 [348569.867647] RAX: fffff520607fee77 RBX: ffffc90303ff73a0 RCX: ffffffffc10c22cd [348569.867656] RDX: 0000000000000001 RSI: 0000000000000002 RDI: ffffc90303ff73b7 [348569.867665] RBP: 0000000000000000 R08: fffff520607fee77 R09: fffff520607fee77 [348569.867674] R10: 0000000000000001 R11: fffff520607fee76 R12: ffff88024b6cfcf1 [348569.867682] R13: 0000000000000010 R14: 0000000000001010 R15: 00000000000000ea [348569.867692] FS: 0000000000000000(0000) GS:ffff880407300000(0000) knlGS:0000000000000000 [348569.867701] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [348569.867709] CR2: ffffc90303ff73b5 CR3: 0000000346c0d004 CR4: 00000000003606e0 (gdb) list *vivid_fillbuff+0x1e9b 0x1936b is in vivid_fillbuff (drivers/media/platform/vivid/vivid-kthread-cap.c:495). 490 ms % 1000, 491 buf->vb.sequence, 492 (dev->field_cap == V4L2_FIELD_ALTERNATE) ? 493 (buf->vb.field == V4L2_FIELD_TOP ? 494 " top" : " bottom") : ""); 495 tpg_gen_text(tpg, basep, line++ * line_height, 16, str); 496 } 497 if (dev->osd_mode == 0) { 498 snprintf(str, sizeof(str), " %dx%d, input %d ", 499 dev->src_rect.width, dev->src_rect.height, dev->input); Thanks, Mauro ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-08 18:23 ` Mauro Carvalho Chehab @ 2018-10-08 18:28 ` Mauro Carvalho Chehab 2018-10-08 18:31 ` Hans Verkuil 1 sibling, 0 replies; 17+ messages in thread From: Mauro Carvalho Chehab @ 2018-10-08 18:28 UTC (permalink / raw) To: Hans Verkuil Cc: Kieran Bingham, Keiichi Watanabe, linux-media, Mauro Carvalho Chehab, linux-kernel, tfiga, jcliang, shik [-- Attachment #1: Type: text/plain, Size: 9122 bytes --] Em Mon, 8 Oct 2018 15:23:48 -0300 Mauro Carvalho Chehab <mchehab+samsung@kernel.org> escreveu: > Em Mon, 8 Oct 2018 19:53:38 +0200 > Hans Verkuil <hverkuil@xs4all.nl> escreveu: > > > On 10/08/2018 07:03 PM, Mauro Carvalho Chehab wrote: > > > Em Wed, 3 Oct 2018 12:14:22 +0100 > > > Kieran Bingham <kieran.bingham@ideasonboard.com> escreveu: > > > > > >>> @@ -75,6 +76,8 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { > > >>> { 1, 5 }, > > >>> { 1, 10 }, > > >>> { 1, 15 }, > > >>> + { 1, 15 }, > > >>> + { 1, 25 }, > > > > > > As the code requires that VIVID_WEBCAM_IVALS would be twice the number > > > of resolutions, I understand why you're doing that. > > > > > >> But won't this add duplicates of 25 and 15 FPS to all the frame sizes > > >> smaller than 1280,720 ? Or are they filtered out? > > > > > > However, I agree with Kieran: looking at the code, it sounds to me that > > > it will indeed duplicate 1/15 and 1/25 intervals. > > > > Oops, I missed this comment. Yes, you'll get duplicates which should be > > avoided. > > > > > > > > I suggest add two other intervals there, like: > > > 12.5 fps and 29.995 fps, e. g.: > > > > 29.995 is never used by webcams. > > > > > > > > static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { > > > { 1, 1 }, > > > { 1, 2 }, > > > { 1, 4 }, > > > { 1, 5 }, > > > { 1, 10 }, > > > { 1, 15 }, > > > { 2, 50 }, > > > { 1, 25 }, > > > { 1, 30 }, > > > { 1, 40 }, > > > { 1, 50 }, > > > { 1001, 30000 }, > > > { 1, 60 }, > > > }; > > > > > > Provided, of course, that vivid would support producing images > > > at fractional rate. I didn't check. If not, then simply add > > > 1/20 and 1/40. > > > > vivid can do fractional rates (it does support this for the TV input), > > but 29.995 makes no sense for a webcam. > > Yes, I know. > > > So 1/20 and 1/40 seems the > > right approach. > > I would have 1/12.5 at least. I have some webcams here whose seem to > use things like that under bad light, and it sounds interesting to > have at least one fraction that doesn't start with "1", in order to > be sure that camera apps are doing the right thing. > > > >> Now the difficulty is adding smaller frame rates (like 1,1, 1,2) would > > >> effect/reduce the output rates of the larger frame sizes, so how about > > >> adding some high rate support (any two from 1/{60,75,90,100,120}) instead? > > > > > > Last week, I got a crash with vivid running at 30 fps, while running an > > > event's race code, on a i7core (there, the code was switching all video > > > controls while subscribing/unsubscribing events). The same code worked > > > with lower fps. > > > > If you have a stack trace, then let me know. > > See at the end. > > I intend to do further tests when I have some time. > > > > > > While I didn't have time to debug it yet, I suspect that it has to do > > > with the time spent to produce a frame on vivid. So, while it would be > > > nice to have high rate support, I'm not sure if this is doable. It may, > > > but perhaps we need to disable some possible video output formats, as some > > > types may consume more time to build frames. > > > > In the end that depends on the CPU and what else is running. You'll know quickly > > enough if the CPU isn't fast enough to support a format. Although it shouldn't > > crash, of course. > > Yes, but on this case, it caused an OOPS (with KASAN enabled). > > I was running the stress test on one VT while using qv4l2 to stream. > > When I changed the resolution, it caused the OOPS. > > This is one example. > > I ran the race test first. It placed all controls at some random state > (only issued VIDIOC_EXT_CTRLS - kept everything else on default). > > when asked qv4l2 to start streaming (5fps), got this: > > > [348569.866967] BUG: unable to handle kernel paging request at ffffc90303ff73b5 > [348569.867070] PGD 406ee8067 P4D 406ee8067 PUD 0 > [348569.867081] Oops: 0002 [#1] SMP KASAN > [348569.867089] CPU: 2 PID: 4365 Comm: vivid-000-vid-c Tainted: G B 4.19.0-rc1+ #3 > [348569.867098] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949 05/11/2017 > [348569.867113] RIP: 0010:tpg_print_str_6+0x241/0x960 [v4l2_tpg] > [348569.867122] Code: 24 18 e9 a5 01 00 00 84 c9 0f 84 48 03 00 00 40 84 ed 48 8d 7b 15 be 02 00 00 00 0f 88 cb 06 00 00 e8 a3 cd 2f ec 48 8d 7b 17 <66> 44 89 73 15 e8 b5 c7 > 2f ec 44 88 6b 17 40 f6 c5 40 48 8d 7b 12 > [348569.867136] RSP: 0018:ffff88024b6cf8c8 EFLAGS: 00010246 > [348569.867144] RAX: fffff520607fee77 RBX: ffffc90303ff73a0 RCX: ffffffffc10c22cd > [348569.867152] RDX: 0000000000000001 RSI: 0000000000000002 RDI: ffffc90303ff73b7 > [348569.867162] RBP: 0000000000000000 R08: fffff520607fee77 R09: fffff520607fee77 > [348569.867170] R10: 0000000000000001 R11: fffff520607fee76 R12: ffff88024b6cfcf1 > [348569.867179] R13: 0000000000000010 R14: 0000000000001010 R15: 00000000000000ea > [348569.867189] FS: 0000000000000000(0000) GS:ffff880407300000(0000) knlGS:0000000000000000 > [348569.867198] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [348569.867206] CR2: ffffc90303ff73b5 CR3: 0000000346c0d004 CR4: 00000000003606e0 > [348569.867214] Call Trace: > [348569.867227] tpg_gen_text+0x258/0x2b0 [v4l2_tpg] > [348569.867249] vivid_fillbuff+0x1e9b/0x30b0 [vivid] > [348569.867261] ? __list_add_valid+0x29/0x90 > [348569.867271] ? __switch_to+0x345/0x700 > [348569.867281] ? osq_unlock+0x6b/0xf0 > [348569.867302] ? vivid_grab_controls+0x60/0x60 [vivid] > [348569.867312] ? del_timer_sync+0x3e/0x50 > [348569.867320] ? schedule_timeout+0x234/0x4e0 > [348569.867330] ? mutex_lock+0xbd/0xc0 > [348569.867337] ? mutex_lock+0xbd/0xc0 > [348569.867345] ? __mutex_lock_slowpath+0x10/0x10 > [348569.867365] ? vivid_thread_vid_cap+0x5b6/0xf20 [vivid] > [348569.867385] vivid_thread_vid_cap+0x5b6/0xf20 [vivid] > [348569.867396] ? __sched_text_start+0x8/0x8 > [348569.867404] ? __wake_up_common+0x9c/0x230 > [348569.867413] ? __kthread_parkme+0x77/0x90 > [348569.867432] ? vivid_fillbuff+0x30b0/0x30b0 [vivid] > [348569.867440] kthread+0x1ac/0x1d0 > [348569.867448] ? kthread_create_worker_on_cpu+0xc0/0xc0 > [348569.867458] ret_from_fork+0x1f/0x30 > [348569.867465] Modules linked in: vivid videobuf2_dma_contig v4l2_tpg v4l2_dv_timings videobuf2_v4l2 videobuf2_vmalloc videobuf2_memops videobuf2_common v4l2_common videode > v media xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c tun bridge stp llc ebtable > _filter ebtables ip6table_filter ip6_tables bluetooth rfkill ecdh_generic snd_hda_codec_hdmi i915 snd_hda_intel snd_usb_audio snd_hda_codec intel_rapl snd_usbmidi_lib x86_pk > g_temp_thermal snd_hda_core i2c_algo_bit snd_hwdep intel_powerclamp coretemp snd_pcm snd_seq_midi drm_kms_helper crct10dif_pclmul snd_seq_midi_event crc32_pclmul snd_rawmidi > ghash_clmulni_intel intel_cstate snd_seq intel_uncore drm intel_rapl_perf snd_seq_device snd_timer e1000e snd ptp mei_me > [348569.867574] video mei soundcore pps_core lpc_ich fuse binfmt_misc kvm_intel kvm irqbypass crc32c_intel [last unloaded: videobuf2_memops] > [348569.867597] CR2: ffffc90303ff73b5 > [348569.867604] ---[ end trace b85f80398f88914d ]--- > [348569.867615] RIP: 0010:tpg_print_str_6+0x241/0x960 [v4l2_tpg] > [348569.867624] Code: 24 18 e9 a5 01 00 00 84 c9 0f 84 48 03 00 00 40 84 ed 48 8d 7b 15 be 02 00 00 00 0f 88 cb 06 00 00 e8 a3 cd 2f ec 48 8d 7b 17 <66> 44 89 73 15 e8 b5 c7 > 2f ec 44 88 6b 17 40 f6 c5 40 48 8d 7b 12 > [348569.867639] RSP: 0018:ffff88024b6cf8c8 EFLAGS: 00010246 > [348569.867647] RAX: fffff520607fee77 RBX: ffffc90303ff73a0 RCX: ffffffffc10c22cd > [348569.867656] RDX: 0000000000000001 RSI: 0000000000000002 RDI: ffffc90303ff73b7 > [348569.867665] RBP: 0000000000000000 R08: fffff520607fee77 R09: fffff520607fee77 > [348569.867674] R10: 0000000000000001 R11: fffff520607fee76 R12: ffff88024b6cfcf1 > [348569.867682] R13: 0000000000000010 R14: 0000000000001010 R15: 00000000000000ea > [348569.867692] FS: 0000000000000000(0000) GS:ffff880407300000(0000) knlGS:0000000000000000 > [348569.867701] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [348569.867709] CR2: ffffc90303ff73b5 CR3: 0000000346c0d004 CR4: 00000000003606e0 > > > (gdb) list *vivid_fillbuff+0x1e9b > 0x1936b is in vivid_fillbuff (drivers/media/platform/vivid/vivid-kthread-cap.c:495). > 490 ms % 1000, > 491 buf->vb.sequence, > 492 (dev->field_cap == V4L2_FIELD_ALTERNATE) ? > 493 (buf->vb.field == V4L2_FIELD_TOP ? > 494 " top" : " bottom") : ""); > 495 tpg_gen_text(tpg, basep, line++ * line_height, 16, str); > 496 } > 497 if (dev->osd_mode == 0) { > 498 snprintf(str, sizeof(str), " %dx%d, input %d ", > 499 dev->src_rect.width, dev->src_rect.height, dev->input); Not sure if VGER will accept, but I attached an image of qv4l2. It is unresponsive. Thanks, Mauro [-- Attachment #2: qv4l-1.png --] [-- Type: image/png, Size: 123343 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-08 18:23 ` Mauro Carvalho Chehab 2018-10-08 18:28 ` Mauro Carvalho Chehab @ 2018-10-08 18:31 ` Hans Verkuil 2018-10-08 19:00 ` Mauro Carvalho Chehab 1 sibling, 1 reply; 17+ messages in thread From: Hans Verkuil @ 2018-10-08 18:31 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Kieran Bingham, Keiichi Watanabe, linux-media, Mauro Carvalho Chehab, linux-kernel, tfiga, jcliang, shik On 10/08/2018 08:23 PM, Mauro Carvalho Chehab wrote: > Em Mon, 8 Oct 2018 19:53:38 +0200 > Hans Verkuil <hverkuil@xs4all.nl> escreveu: > >> On 10/08/2018 07:03 PM, Mauro Carvalho Chehab wrote: >>> Em Wed, 3 Oct 2018 12:14:22 +0100 >>> Kieran Bingham <kieran.bingham@ideasonboard.com> escreveu: >>> >>>>> @@ -75,6 +76,8 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { >>>>> { 1, 5 }, >>>>> { 1, 10 }, >>>>> { 1, 15 }, >>>>> + { 1, 15 }, >>>>> + { 1, 25 }, >>> >>> As the code requires that VIVID_WEBCAM_IVALS would be twice the number >>> of resolutions, I understand why you're doing that. >>> >>>> But won't this add duplicates of 25 and 15 FPS to all the frame sizes >>>> smaller than 1280,720 ? Or are they filtered out? >>> >>> However, I agree with Kieran: looking at the code, it sounds to me that >>> it will indeed duplicate 1/15 and 1/25 intervals. >> >> Oops, I missed this comment. Yes, you'll get duplicates which should be >> avoided. >> >>> >>> I suggest add two other intervals there, like: >>> 12.5 fps and 29.995 fps, e. g.: >> >> 29.995 is never used by webcams. >> >>> >>> static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = { >>> { 1, 1 }, >>> { 1, 2 }, >>> { 1, 4 }, >>> { 1, 5 }, >>> { 1, 10 }, >>> { 1, 15 }, >>> { 2, 50 }, >>> { 1, 25 }, >>> { 1, 30 }, >>> { 1, 40 }, >>> { 1, 50 }, >>> { 1001, 30000 }, >>> { 1, 60 }, >>> }; >>> >>> Provided, of course, that vivid would support producing images >>> at fractional rate. I didn't check. If not, then simply add >>> 1/20 and 1/40. >> >> vivid can do fractional rates (it does support this for the TV input), >> but 29.995 makes no sense for a webcam. > > Yes, I know. > >> So 1/20 and 1/40 seems the >> right approach. > > I would have 1/12.5 at least. I have some webcams here whose seem to > use things like that under bad light, and it sounds interesting to > have at least one fraction that doesn't start with "1", in order to > be sure that camera apps are doing the right thing. > >>>> Now the difficulty is adding smaller frame rates (like 1,1, 1,2) would >>>> effect/reduce the output rates of the larger frame sizes, so how about >>>> adding some high rate support (any two from 1/{60,75,90,100,120}) instead? >>> >>> Last week, I got a crash with vivid running at 30 fps, while running an >>> event's race code, on a i7core (there, the code was switching all video >>> controls while subscribing/unsubscribing events). The same code worked >>> with lower fps. >> >> If you have a stack trace, then let me know. > > See at the end. > > I intend to do further tests when I have some time. > >> >>> While I didn't have time to debug it yet, I suspect that it has to do >>> with the time spent to produce a frame on vivid. So, while it would be >>> nice to have high rate support, I'm not sure if this is doable. It may, >>> but perhaps we need to disable some possible video output formats, as some >>> types may consume more time to build frames. >> >> In the end that depends on the CPU and what else is running. You'll know quickly >> enough if the CPU isn't fast enough to support a format. Although it shouldn't >> crash, of course. > > Yes, but on this case, it caused an OOPS (with KASAN enabled). > > I was running the stress test on one VT while using qv4l2 to stream. > > When I changed the resolution, it caused the OOPS. > > This is one example. > > I ran the race test first. It placed all controls at some random state > (only issued VIDIOC_EXT_CTRLS - kept everything else on default). > > when asked qv4l2 to start streaming (5fps), got this: > > > [348569.866967] BUG: unable to handle kernel paging request at ffffc90303ff73b5 > [348569.867070] PGD 406ee8067 P4D 406ee8067 PUD 0 > [348569.867081] Oops: 0002 [#1] SMP KASAN > [348569.867089] CPU: 2 PID: 4365 Comm: vivid-000-vid-c Tainted: G B 4.19.0-rc1+ #3 > [348569.867098] Hardware name: /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949 05/11/2017 > [348569.867113] RIP: 0010:tpg_print_str_6+0x241/0x960 [v4l2_tpg] > [348569.867122] Code: 24 18 e9 a5 01 00 00 84 c9 0f 84 48 03 00 00 40 84 ed 48 8d 7b 15 be 02 00 00 00 0f 88 cb 06 00 00 e8 a3 cd 2f ec 48 8d 7b 17 <66> 44 89 73 15 e8 b5 c7 > 2f ec 44 88 6b 17 40 f6 c5 40 48 8d 7b 12 > [348569.867136] RSP: 0018:ffff88024b6cf8c8 EFLAGS: 00010246 > [348569.867144] RAX: fffff520607fee77 RBX: ffffc90303ff73a0 RCX: ffffffffc10c22cd > [348569.867152] RDX: 0000000000000001 RSI: 0000000000000002 RDI: ffffc90303ff73b7 > [348569.867162] RBP: 0000000000000000 R08: fffff520607fee77 R09: fffff520607fee77 > [348569.867170] R10: 0000000000000001 R11: fffff520607fee76 R12: ffff88024b6cfcf1 > [348569.867179] R13: 0000000000000010 R14: 0000000000001010 R15: 00000000000000ea > [348569.867189] FS: 0000000000000000(0000) GS:ffff880407300000(0000) knlGS:0000000000000000 > [348569.867198] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [348569.867206] CR2: ffffc90303ff73b5 CR3: 0000000346c0d004 CR4: 00000000003606e0 > [348569.867214] Call Trace: > [348569.867227] tpg_gen_text+0x258/0x2b0 [v4l2_tpg] > [348569.867249] vivid_fillbuff+0x1e9b/0x30b0 [vivid] > [348569.867261] ? __list_add_valid+0x29/0x90 > [348569.867271] ? __switch_to+0x345/0x700 > [348569.867281] ? osq_unlock+0x6b/0xf0 > [348569.867302] ? vivid_grab_controls+0x60/0x60 [vivid] > [348569.867312] ? del_timer_sync+0x3e/0x50 > [348569.867320] ? schedule_timeout+0x234/0x4e0 > [348569.867330] ? mutex_lock+0xbd/0xc0 > [348569.867337] ? mutex_lock+0xbd/0xc0 > [348569.867345] ? __mutex_lock_slowpath+0x10/0x10 > [348569.867365] ? vivid_thread_vid_cap+0x5b6/0xf20 [vivid] > [348569.867385] vivid_thread_vid_cap+0x5b6/0xf20 [vivid] > [348569.867396] ? __sched_text_start+0x8/0x8 > [348569.867404] ? __wake_up_common+0x9c/0x230 > [348569.867413] ? __kthread_parkme+0x77/0x90 > [348569.867432] ? vivid_fillbuff+0x30b0/0x30b0 [vivid] > [348569.867440] kthread+0x1ac/0x1d0 > [348569.867448] ? kthread_create_worker_on_cpu+0xc0/0xc0 > [348569.867458] ret_from_fork+0x1f/0x30 > [348569.867465] Modules linked in: vivid videobuf2_dma_contig v4l2_tpg v4l2_dv_timings videobuf2_v4l2 videobuf2_vmalloc videobuf2_memops videobuf2_common v4l2_common videode > v media xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c tun bridge stp llc ebtable > _filter ebtables ip6table_filter ip6_tables bluetooth rfkill ecdh_generic snd_hda_codec_hdmi i915 snd_hda_intel snd_usb_audio snd_hda_codec intel_rapl snd_usbmidi_lib x86_pk > g_temp_thermal snd_hda_core i2c_algo_bit snd_hwdep intel_powerclamp coretemp snd_pcm snd_seq_midi drm_kms_helper crct10dif_pclmul snd_seq_midi_event crc32_pclmul snd_rawmidi > ghash_clmulni_intel intel_cstate snd_seq intel_uncore drm intel_rapl_perf snd_seq_device snd_timer e1000e snd ptp mei_me > [348569.867574] video mei soundcore pps_core lpc_ich fuse binfmt_misc kvm_intel kvm irqbypass crc32c_intel [last unloaded: videobuf2_memops] > [348569.867597] CR2: ffffc90303ff73b5 > [348569.867604] ---[ end trace b85f80398f88914d ]--- > [348569.867615] RIP: 0010:tpg_print_str_6+0x241/0x960 [v4l2_tpg] > [348569.867624] Code: 24 18 e9 a5 01 00 00 84 c9 0f 84 48 03 00 00 40 84 ed 48 8d 7b 15 be 02 00 00 00 0f 88 cb 06 00 00 e8 a3 cd 2f ec 48 8d 7b 17 <66> 44 89 73 15 e8 b5 c7 > 2f ec 44 88 6b 17 40 f6 c5 40 48 8d 7b 12 > [348569.867639] RSP: 0018:ffff88024b6cf8c8 EFLAGS: 00010246 > [348569.867647] RAX: fffff520607fee77 RBX: ffffc90303ff73a0 RCX: ffffffffc10c22cd > [348569.867656] RDX: 0000000000000001 RSI: 0000000000000002 RDI: ffffc90303ff73b7 > [348569.867665] RBP: 0000000000000000 R08: fffff520607fee77 R09: fffff520607fee77 > [348569.867674] R10: 0000000000000001 R11: fffff520607fee76 R12: ffff88024b6cfcf1 > [348569.867682] R13: 0000000000000010 R14: 0000000000001010 R15: 00000000000000ea > [348569.867692] FS: 0000000000000000(0000) GS:ffff880407300000(0000) knlGS:0000000000000000 > [348569.867701] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [348569.867709] CR2: ffffc90303ff73b5 CR3: 0000000346c0d004 CR4: 00000000003606e0 > > > (gdb) list *vivid_fillbuff+0x1e9b > 0x1936b is in vivid_fillbuff (drivers/media/platform/vivid/vivid-kthread-cap.c:495). > 490 ms % 1000, > 491 buf->vb.sequence, > 492 (dev->field_cap == V4L2_FIELD_ALTERNATE) ? > 493 (buf->vb.field == V4L2_FIELD_TOP ? > 494 " top" : " bottom") : ""); > 495 tpg_gen_text(tpg, basep, line++ * line_height, 16, str); > 496 } > 497 if (dev->osd_mode == 0) { > 498 snprintf(str, sizeof(str), " %dx%d, input %d ", > 499 dev->src_rect.width, dev->src_rect.height, dev->input); > There is a bug with hflip handling in that function. Nothing to do with the resolution. I could reproduce it by just checking the hflip control. I'll investigate. Regards, Hans ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-08 18:31 ` Hans Verkuil @ 2018-10-08 19:00 ` Mauro Carvalho Chehab 2018-10-09 6:01 ` Keiichi Watanabe 0 siblings, 1 reply; 17+ messages in thread From: Mauro Carvalho Chehab @ 2018-10-08 19:00 UTC (permalink / raw) To: Hans Verkuil Cc: Kieran Bingham, Keiichi Watanabe, linux-media, Mauro Carvalho Chehab, linux-kernel, tfiga, jcliang, shik Em Mon, 8 Oct 2018 20:31:10 +0200 Hans Verkuil <hverkuil@xs4all.nl> escreveu: > > (gdb) list *vivid_fillbuff+0x1e9b > > 0x1936b is in vivid_fillbuff (drivers/media/platform/vivid/vivid-kthread-cap.c:495). > > 490 ms % 1000, > > 491 buf->vb.sequence, > > 492 (dev->field_cap == V4L2_FIELD_ALTERNATE) ? > > 493 (buf->vb.field == V4L2_FIELD_TOP ? > > 494 " top" : " bottom") : ""); > > 495 tpg_gen_text(tpg, basep, line++ * line_height, 16, str); > > 496 } > > 497 if (dev->osd_mode == 0) { > > 498 snprintf(str, sizeof(str), " %dx%d, input %d ", > > 499 dev->src_rect.width, dev->src_rect.height, dev->input); > > > > There is a bug with hflip handling in that function. Nothing to do with the > resolution. I could reproduce it by just checking the hflip control. > I'll investigate. Ah! Well, as I said, I got it only once last week while trying to use vivid for some event racing test. I didn't have time to actually seek. On that time, the bug only manifested when I changed the frame rate. Thanks, Mauro ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-08 19:00 ` Mauro Carvalho Chehab @ 2018-10-09 6:01 ` Keiichi Watanabe 2018-10-09 6:33 ` Hans Verkuil 0 siblings, 1 reply; 17+ messages in thread From: Keiichi Watanabe @ 2018-10-09 6:01 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Hans Verkuil, Kieran Bingham, Linux Media Mailing List, Mauro Carvalho Chehab, Linux Kernel Mailing List, Tomasz Figa, Ricky Liang, Shik Chen Let me back to the topic about interval. I'd like to send a nex version of this patch to avoid duplication of intervals. We need to choose two intervals from the three options: 1/20, 2/25 and 1/40. As Mauro said, we would want to have 2/25 for a fractional rate. Then, I think adding 2/25 and 1/40 will work well. If it's okay, I will send an updated version. Best regards, Kei On Tue, Oct 9, 2018 at 4:00 AM, Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote: > Em Mon, 8 Oct 2018 20:31:10 +0200 > Hans Verkuil <hverkuil@xs4all.nl> escreveu: > >> > (gdb) list *vivid_fillbuff+0x1e9b >> > 0x1936b is in vivid_fillbuff (drivers/media/platform/vivid/vivid-kthread-cap.c:495). >> > 490 ms % 1000, >> > 491 buf->vb.sequence, >> > 492 (dev->field_cap == V4L2_FIELD_ALTERNATE) ? >> > 493 (buf->vb.field == V4L2_FIELD_TOP ? >> > 494 " top" : " bottom") : ""); >> > 495 tpg_gen_text(tpg, basep, line++ * line_height, 16, str); >> > 496 } >> > 497 if (dev->osd_mode == 0) { >> > 498 snprintf(str, sizeof(str), " %dx%d, input %d ", >> > 499 dev->src_rect.width, dev->src_rect.height, dev->input); >> > >> >> There is a bug with hflip handling in that function. Nothing to do with the >> resolution. I could reproduce it by just checking the hflip control. >> I'll investigate. > > Ah! Well, as I said, I got it only once last week while trying to use > vivid for some event racing test. I didn't have time to actually > seek. On that time, the bug only manifested when I changed the frame > rate. > > Thanks, > Mauro ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] media: vivid: Support 480p for webcam capture 2018-10-09 6:01 ` Keiichi Watanabe @ 2018-10-09 6:33 ` Hans Verkuil 0 siblings, 0 replies; 17+ messages in thread From: Hans Verkuil @ 2018-10-09 6:33 UTC (permalink / raw) To: Keiichi Watanabe, Mauro Carvalho Chehab Cc: Kieran Bingham, Linux Media Mailing List, Mauro Carvalho Chehab, Linux Kernel Mailing List, Tomasz Figa, Ricky Liang, Shik Chen On 10/09/2018 08:01 AM, Keiichi Watanabe wrote: > Let me back to the topic about interval. > I'd like to send a nex version of this patch to avoid duplication of intervals. > > We need to choose two intervals from the three options: 1/20, 2/25 and 1/40. > As Mauro said, we would want to have 2/25 for a fractional rate. > Then, I think adding 2/25 and 1/40 will work well. > If it's okay, I will send an updated version. Fine by me! Hans > > Best regards, > Kei > > On Tue, Oct 9, 2018 at 4:00 AM, Mauro Carvalho Chehab > <mchehab+samsung@kernel.org> wrote: >> Em Mon, 8 Oct 2018 20:31:10 +0200 >> Hans Verkuil <hverkuil@xs4all.nl> escreveu: >> >>>> (gdb) list *vivid_fillbuff+0x1e9b >>>> 0x1936b is in vivid_fillbuff (drivers/media/platform/vivid/vivid-kthread-cap.c:495). >>>> 490 ms % 1000, >>>> 491 buf->vb.sequence, >>>> 492 (dev->field_cap == V4L2_FIELD_ALTERNATE) ? >>>> 493 (buf->vb.field == V4L2_FIELD_TOP ? >>>> 494 " top" : " bottom") : ""); >>>> 495 tpg_gen_text(tpg, basep, line++ * line_height, 16, str); >>>> 496 } >>>> 497 if (dev->osd_mode == 0) { >>>> 498 snprintf(str, sizeof(str), " %dx%d, input %d ", >>>> 499 dev->src_rect.width, dev->src_rect.height, dev->input); >>>> >>> >>> There is a bug with hflip handling in that function. Nothing to do with the >>> resolution. I could reproduce it by just checking the hflip control. >>> I'll investigate. >> >> Ah! Well, as I said, I got it only once last week while trying to use >> vivid for some event racing test. I didn't have time to actually >> seek. On that time, the bug only manifested when I changed the frame >> rate. >> >> Thanks, >> Mauro ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-10-09 6:34 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-03 7:06 [PATCH] media: vivid: Support 480p for webcam capture Keiichi Watanabe 2018-10-03 7:08 ` Keiichi Watanabe 2018-10-03 11:16 ` Kieran Bingham 2018-10-05 9:18 ` Hans Verkuil 2018-10-06 10:29 ` Keiichi Watanabe 2018-10-08 8:35 ` Kieran Bingham 2018-10-08 9:00 ` Hans Verkuil 2018-10-08 9:47 ` Kieran Bingham 2018-10-03 11:14 ` Kieran Bingham 2018-10-08 17:03 ` Mauro Carvalho Chehab 2018-10-08 17:53 ` Hans Verkuil 2018-10-08 18:23 ` Mauro Carvalho Chehab 2018-10-08 18:28 ` Mauro Carvalho Chehab 2018-10-08 18:31 ` Hans Verkuil 2018-10-08 19:00 ` Mauro Carvalho Chehab 2018-10-09 6:01 ` Keiichi Watanabe 2018-10-09 6:33 ` Hans Verkuil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).
![[-- Attachment #2: qv4l-1.png --] [-- Type: image/png, Size: 123343 bytes --]](../../20181008152800.52dff648@coco.lan/2-qv4l-1.png){kind=link}