Re: [PATCH v2 2/3] usb: gadget: uvc: add g_parm and s_parm for frame interval

Michael Grzeschik
2024-07-17T22:22:08Z

[-- Attachment #1: Type: text/plain, Size: 4748 bytes --]

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

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]