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

[Michael Grzeschik <mgr@pengutronix.de>]

[-- 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 --]