| Message ID | 20240403-uvc_request_length_by_interval-v2-2-12690f7a2eff@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: gadget: uvc: allocate requests based on frame interval length and buffersize | expand |
On 6/22/24 4:48 PM, Michael Grzeschik wrote: > The uvc gadget driver is lacking the information which frame interval > was set by the host. We add this information by implementing the g_parm > and s_parm callbacks. This change requires the userspace application (uvc-gagdet equivalent) to call s/g_parm when the FPS negotiations are finished. This is fine, but we should document that in the commit message here so implementers know that something needs to be done to take advantage of the change. On a similar note, the reference uvc-gadget should also be updated to call the added functions (and apologies if you've already put up a patch for it, I was unable find one). > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > > --- > v1 -> v2: - > --- > drivers/usb/gadget/function/uvc.h | 1 + > drivers/usb/gadget/function/uvc_v4l2.c | 52 ++++++++++++++++++++++++++++++++++ > 2 files changed, 53 insertions(+) > > diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h > index cb35687b11e7e..d153bd9e35e31 100644 > --- a/drivers/usb/gadget/function/uvc.h > +++ b/drivers/usb/gadget/function/uvc.h > @@ -97,6 +97,7 @@ struct uvc_video { > unsigned int width; > unsigned int height; > unsigned int imagesize; > + unsigned int interval; > struct mutex mutex; /* protects frame parameters */ > > unsigned int uvc_num_requests; > diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c > index a024aecb76dc3..5b579ec1f5040 100644 > --- a/drivers/usb/gadget/function/uvc_v4l2.c > +++ b/drivers/usb/gadget/function/uvc_v4l2.c > @@ -307,6 +307,56 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt) > return ret; > } > > +static int uvc_v4l2_g_parm(struct file *file, void *fh, > + struct v4l2_streamparm *parm) > +{ > + struct video_device *vdev = video_devdata(file); > + struct uvc_device *uvc = video_get_drvdata(vdev); > + struct uvc_video *video = &uvc->video; > + struct v4l2_fract timeperframe; > + > + if (parm->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > + return -EINVAL; > + > + /* Return the actual frame period. */ > + timeperframe.numerator = video->interval; > + timeperframe.denominator = 10000000; > + v4l2_simplify_fraction(&timeperframe.numerator, > + &timeperframe.denominator, 8, 333); > + > + uvcg_dbg(&uvc->func, "Getting frame interval of %u/%u (%u)\n", > + timeperframe.numerator, timeperframe.denominator, > + video->interval); > + > + parm->parm.output.timeperframe = timeperframe; > + parm->parm.output.capability = V4L2_CAP_TIMEPERFRAME; > + > + return 0; > +} > + > +static int uvc_v4l2_s_parm(struct file *file, void *fh, > + struct v4l2_streamparm *parm) > +{ > + struct video_device *vdev = video_devdata(file); > + struct uvc_device *uvc = video_get_drvdata(vdev); > + struct uvc_video *video = &uvc->video; > + struct v4l2_fract timeperframe; > + > + if (parm->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) > + return -EINVAL; > + > + timeperframe = parm->parm.output.timeperframe; > + > + video->interval = v4l2_fraction_to_interval(timeperframe.numerator, > + timeperframe.denominator); > + > + uvcg_dbg(&uvc->func, "Setting frame interval to %u/%u (%u)\n", > + timeperframe.numerator, timeperframe.denominator, > + video->interval); > + > + return 0; > +} > + > static int > uvc_v4l2_enum_frameintervals(struct file *file, void *fh, > struct v4l2_frmivalenum *fival) > @@ -577,6 +627,8 @@ const struct v4l2_ioctl_ops uvc_v4l2_ioctl_ops = { > .vidioc_dqbuf = uvc_v4l2_dqbuf, > .vidioc_streamon = uvc_v4l2_streamon, > .vidioc_streamoff = uvc_v4l2_streamoff, > + .vidioc_s_parm = uvc_v4l2_s_parm, > + .vidioc_g_parm = uvc_v4l2_g_parm, > .vidioc_subscribe_event = uvc_v4l2_subscribe_event, > .vidioc_unsubscribe_event = uvc_v4l2_unsubscribe_event, > .vidioc_default = uvc_v4l2_ioctl_default, >
Hi Avichal, On Wed, Jun 26, 2024 at 11:30:58AM -0700, Avichal Rakesh wrote: >On 6/22/24 4:48 PM, Michael Grzeschik wrote: >> The uvc gadget driver is lacking the information which frame interval >> was set by the host. We add this information by implementing the g_parm >> and s_parm callbacks. > >This change requires the userspace application (uvc-gagdet equivalent) >to call s/g_parm when the FPS negotiations are finished. This is fine, >but we should document that in the commit message here so implementers >know that something needs to be done to take advantage of the change. Fair point! I will do that for v3. >On a similar note, the reference uvc-gadget should also be updated to >call the added functions (and apologies if you've already put up a >patch for it, I was unable find one). Since I am only working with gstreamer with the uvcsink nowadays I missed that. The internal v4l2sink does already do everything right. But you are absolutely right. I will send an patch for uvc-gadget. Regards, Michael >> >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> >> >> --- >> v1 -> v2: - >> --- >> drivers/usb/gadget/function/uvc.h | 1 + >> drivers/usb/gadget/function/uvc_v4l2.c | 52 ++++++++++++++++++++++++++++++++++ >> 2 files changed, 53 insertions(+) >> >> diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h >> index cb35687b11e7e..d153bd9e35e31 100644 >> --- a/drivers/usb/gadget/function/uvc.h >> +++ b/drivers/usb/gadget/function/uvc.h >> @@ -97,6 +97,7 @@ struct uvc_video { >> unsigned int width; >> unsigned int height; >> unsigned int imagesize; >> + unsigned int interval; >> struct mutex mutex; /* protects frame parameters */ >> >> unsigned int uvc_num_requests; >> diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c >> index a024aecb76dc3..5b579ec1f5040 100644 >> --- a/drivers/usb/gadget/function/uvc_v4l2.c >> +++ b/drivers/usb/gadget/function/uvc_v4l2.c >> @@ -307,6 +307,56 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt) >> return ret; >> } >> >> +static int uvc_v4l2_g_parm(struct file *file, void *fh, >> + struct v4l2_streamparm *parm) >> +{ >> + struct video_device *vdev = video_devdata(file); >> + struct uvc_device *uvc = video_get_drvdata(vdev); >> + struct uvc_video *video = &uvc->video; >> + struct v4l2_fract timeperframe; >> + >> + if (parm->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) >> + return -EINVAL; >> + >> + /* Return the actual frame period. */ >> + timeperframe.numerator = video->interval; >> + timeperframe.denominator = 10000000; >> + v4l2_simplify_fraction(&timeperframe.numerator, >> + &timeperframe.denominator, 8, 333); >> + >> + uvcg_dbg(&uvc->func, "Getting frame interval of %u/%u (%u)\n", >> + timeperframe.numerator, timeperframe.denominator, >> + video->interval); >> + >> + parm->parm.output.timeperframe = timeperframe; >> + parm->parm.output.capability = V4L2_CAP_TIMEPERFRAME; >> + >> + return 0; >> +} >> + >> +static int uvc_v4l2_s_parm(struct file *file, void *fh, >> + struct v4l2_streamparm *parm) >> +{ >> + struct video_device *vdev = video_devdata(file); >> + struct uvc_device *uvc = video_get_drvdata(vdev); >> + struct uvc_video *video = &uvc->video; >> + struct v4l2_fract timeperframe; >> + >> + if (parm->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) >> + return -EINVAL; >> + >> + timeperframe = parm->parm.output.timeperframe; >> + >> + video->interval = v4l2_fraction_to_interval(timeperframe.numerator, >> + timeperframe.denominator); >> + >> + uvcg_dbg(&uvc->func, "Setting frame interval to %u/%u (%u)\n", >> + timeperframe.numerator, timeperframe.denominator, >> + video->interval); >> + >> + return 0; >> +} >> + >> static int >> uvc_v4l2_enum_frameintervals(struct file *file, void *fh, >> struct v4l2_frmivalenum *fival) >> @@ -577,6 +627,8 @@ const struct v4l2_ioctl_ops uvc_v4l2_ioctl_ops = { >> .vidioc_dqbuf = uvc_v4l2_dqbuf, >> .vidioc_streamon = uvc_v4l2_streamon, >> .vidioc_streamoff = uvc_v4l2_streamoff, >> + .vidioc_s_parm = uvc_v4l2_s_parm, >> + .vidioc_g_parm = uvc_v4l2_g_parm, >> .vidioc_subscribe_event = uvc_v4l2_subscribe_event, >> .vidioc_unsubscribe_event = uvc_v4l2_unsubscribe_event, >> .vidioc_default = uvc_v4l2_ioctl_default, >> >
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index cb35687b11e7e..d153bd9e35e31 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -97,6 +97,7 @@ struct uvc_video { unsigned int width; unsigned int height; unsigned int imagesize; + unsigned int interval; struct mutex mutex; /* protects frame parameters */ unsigned int uvc_num_requests; diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index a024aecb76dc3..5b579ec1f5040 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -307,6 +307,56 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt) return ret; } +static int uvc_v4l2_g_parm(struct file *file, void *fh, + struct v4l2_streamparm *parm) +{ + struct video_device *vdev = video_devdata(file); + struct uvc_device *uvc = video_get_drvdata(vdev); + struct uvc_video *video = &uvc->video; + struct v4l2_fract timeperframe; + + if (parm->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + return -EINVAL; + + /* Return the actual frame period. */ + timeperframe.numerator = video->interval; + timeperframe.denominator = 10000000; + v4l2_simplify_fraction(&timeperframe.numerator, + &timeperframe.denominator, 8, 333); + + uvcg_dbg(&uvc->func, "Getting frame interval of %u/%u (%u)\n", + timeperframe.numerator, timeperframe.denominator, + video->interval); + + parm->parm.output.timeperframe = timeperframe; + parm->parm.output.capability = V4L2_CAP_TIMEPERFRAME; + + return 0; +} + +static int uvc_v4l2_s_parm(struct file *file, void *fh, + struct v4l2_streamparm *parm) +{ + struct video_device *vdev = video_devdata(file); + struct uvc_device *uvc = video_get_drvdata(vdev); + struct uvc_video *video = &uvc->video; + struct v4l2_fract timeperframe; + + if (parm->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) + return -EINVAL; + + timeperframe = parm->parm.output.timeperframe; + + video->interval = v4l2_fraction_to_interval(timeperframe.numerator, + timeperframe.denominator); + + uvcg_dbg(&uvc->func, "Setting frame interval to %u/%u (%u)\n", + timeperframe.numerator, timeperframe.denominator, + video->interval); + + return 0; +} + static int uvc_v4l2_enum_frameintervals(struct file *file, void *fh, struct v4l2_frmivalenum *fival) @@ -577,6 +627,8 @@ const struct v4l2_ioctl_ops uvc_v4l2_ioctl_ops = { .vidioc_dqbuf = uvc_v4l2_dqbuf, .vidioc_streamon = uvc_v4l2_streamon, .vidioc_streamoff = uvc_v4l2_streamoff, + .vidioc_s_parm = uvc_v4l2_s_parm, + .vidioc_g_parm = uvc_v4l2_g_parm, .vidioc_subscribe_event = uvc_v4l2_subscribe_event, .vidioc_unsubscribe_event = uvc_v4l2_unsubscribe_event, .vidioc_default = uvc_v4l2_ioctl_default,
The uvc gadget driver is lacking the information which frame interval was set by the host. We add this information by implementing the g_parm and s_parm callbacks. Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> --- v1 -> v2: - --- drivers/usb/gadget/function/uvc.h | 1 + drivers/usb/gadget/function/uvc_v4l2.c | 52 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+)