media: uvcvideo: support multiple frame descriptors with the same dimensions
Subsystems (2)
PATCH 1/1 media: uvcvideo: support multiple frame descriptors with the same dimensions
The Microsoft HoloLens Sensors device has two separate frame descriptors with the same dimensions, each with a single different frame interval:
VideoStreaming Interface Descriptor:
bLength 30
bDescriptorType 36
bDescriptorSubtype 5 (FRAME_UNCOMPRESSED)
bFrameIndex 1
bmCapabilities 0x00
Still image unsupported
wWidth 1280
wHeight 481
dwMinBitRate 147763200
dwMaxBitRate 147763200
dwMaxVideoFrameBufferSize 615680
dwDefaultFrameInterval 333333
bFrameIntervalType 1
dwFrameInterval( 0) 333333
VideoStreaming Interface Descriptor:
bLength 30
bDescriptorType 36
bDescriptorSubtype 5 (FRAME_UNCOMPRESSED)
bFrameIndex 2
bmCapabilities 0x00
Still image unsupported
wWidth 1280
wHeight 481
dwMinBitRate 443289600
dwMaxBitRate 443289600
dwMaxVideoFrameBufferSize 615680
dwDefaultFrameInterval 111111
bFrameIntervalType 1
dwFrameInterval( 0) 111111
Skip duplicate dimensions in enum_framesizes, let enum_frameintervals list the intervals from both frame descriptors. Change set_streamparm to switch to the correct frame index when changing the interval. This enables 90 fps capture on a Lenovo Explorer Windows Mixed Reality headset.
--- drivers/media/usb/uvc/uvc_v4l2.c | 66 ++++++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 16 deletions(-)
drivers/media/usb/uvc/uvc_v4l2.c +50 -16
@@ -373,8 +373,11 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream, {struct uvc_streaming_control probe;struct v4l2_fract timeperframe; -uint32_t interval; +struct uvc_format *format; +struct uvc_frame *frame; +__u32 interval, tmp, d, maxd;int ret; +int i;if (parm->type != stream->type)return -EINVAL ;
@@ -396,9 +399,31 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,return -EBUSY ; } + format = stream->cur_format; + frame = stream->cur_frame; probe = stream->ctrl; - probe.dwFrameInterval = -uvc_try_frame_interval (stream->cur_frame, interval); + probe.dwFrameInterval =uvc_try_frame_interval (frame, interval); + maxd =abs ((__s32 )probe.dwFrameInterval - interval); + +/* Try frames with matching size to find the best frame interval. */ +for (i =0 ; i < format->nframes; i++) { +if (&format->frame[i] == stream->cur_frame) +continue ; + +if (format->frame[i].wWidth != stream->cur_frame->wWidth || + format->frame[i].wHeight != stream->cur_frame->wHeight) +continue ; + + tmp =uvc_try_frame_interval (&format->frame[i], interval); + d =abs ((__s32 )tmp - interval); +if (d >= maxd) +continue ; + + frame = &format->frame[i]; + probe.bFrameIndex = frame->bFrameIndex; + probe.dwFrameInterval = tmp; + maxd = d; + }/* Probe the device with the new settings. */ ret =uvc_probe_video (stream, &probe);
@@ -408,6 +433,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream, } stream->ctrl = probe; + stream->cur_frame = frame;mutex_unlock (&stream->mutex);/* Return the actual frame period. */
@@ -1150,7 +1176,7 @@ static int uvc_ioctl_enum_framesizes(struct file *file, void *fh,struct uvc_streaming *stream = handle->stream;struct uvc_format *format =NULL ;struct uvc_frame *frame; -int i; +int i, index;/* Look for the given pixel format */ for (i =0 ; i < stream->nformats; i++) {
@@ -1162,10 +1188,20 @@ static int uvc_ioctl_enum_framesizes(struct file *file, void *fh,if (format ==NULL )return -EINVAL ; -if (fsize->index >= format->nframes) +/* Skip duplicate frame sizes */ +for (i =0 , index =0 ; i < format->nframes; i++) { +if (i && frame->wWidth == format->frame[i].wWidth && + frame->wHeight == format->frame[i].wHeight) +continue ; + frame = &format->frame[i]; +if (index == fsize->index) +break ; + index++; + } + +if (i == format->nframes)return -EINVAL ; - frame = &format->frame[fsize->index]; fsize->type =V4L2_FRMSIZE_TYPE_DISCRETE ; fsize->discrete.width = frame->wWidth; fsize->discrete.height = frame->wHeight;
@@ -1179,7 +1215,7 @@ static int uvc_ioctl_enum_frameintervals(struct file *file, void *fh,struct uvc_streaming *stream = handle->stream;struct uvc_format *format =NULL ;struct uvc_frame *frame =NULL ; -int i; +int i, index, nintervals;/* Look for the given pixel format and frame size */ for (i =0 ; i < stream->nformats; i++) {
@@ -1191,30 +1227,28 @@ static int uvc_ioctl_enum_frameintervals(struct file *file, void *fh,if (format ==NULL )return -EINVAL ; + index = fival->index;for (i =0 ; i < format->nframes; i++) {if (format->frame[i].wWidth == fival->width && format->frame[i].wHeight == fival->height) { frame = &format->frame[i]; -break ; + nintervals = frame->bFrameIntervalType ?:1 ; +if (index < nintervals) +break ; + index -= nintervals; } } -if (frame ==NULL ) +if (i == format->nframes)return -EINVAL ;if (frame->bFrameIntervalType) { -if (fival->index >= frame->bFrameIntervalType) -return -EINVAL ; - fival->type =V4L2_FRMIVAL_TYPE_DISCRETE ; fival->discrete.numerator = - frame->dwFrameInterval[fival->index]; + frame->dwFrameInterval[index]; fival->discrete.denominator =10000000 ;uvc_simplify_fraction (&fival->discrete.numerator, &fival->discrete.denominator,8 ,333 ); }else { -if (fival->index) -return -EINVAL ; - fival->type =V4L2_FRMIVAL_TYPE_STEPWISE ; fival->stepwise.min.numerator = frame->dwFrameInterval[0 ]; fival->stepwise.min.denominator =10000000 ;
Signature · 1 line
2.15.1
Laurent Pinchart
maintainerHi Philipp,
Thank you for the patch.
Quoted 2 lines · On Tuesday, 19 December 2017 10:17:35 EET Philipp Zabel wrote:
The Microsoft HoloLens Sensors device has two separate frame descriptors with the same dimensions, each with a single different frame interval:
Why, oh why ? :-(
Quoted 46 lines
VideoStreaming Interface Descriptor:
bLength 30
bDescriptorType 36
bDescriptorSubtype 5 (FRAME_UNCOMPRESSED)
bFrameIndex 1
bmCapabilities0x00
Still image unsupported
wWidth 1280
wHeight 481
dwMinBitRate 147763200
dwMaxBitRate 147763200
dwMaxVideoFrameBufferSize 615680
dwDefaultFrameInterval 333333
bFrameIntervalType 1
dwFrameInterval( 0) 333333
VideoStreaming Interface Descriptor:
bLength 30
bDescriptorType 36
bDescriptorSubtype 5 (FRAME_UNCOMPRESSED)
bFrameIndex 2
bmCapabilities0x00
Still image unsupported
wWidth 1280
wHeight 481
dwMinBitRate 443289600
dwMaxBitRate 443289600
dwMaxVideoFrameBufferSize 615680
dwDefaultFrameInterval 111111
bFrameIntervalType 1
dwFrameInterval( 0) 111111Skip duplicate dimensions in
enum_framesizes, letenum_frameintervalslist the intervals from both frame descriptors. Changeset_streamparmto switch to the correct frame index when changing the interval. This enables 90 fps capture on a Lenovo Explorer Windows Mixed Reality headset.Signed-off-by Philipp Zabel <[email protected]>--- drivers/media/usb/uvc/uvc_v4l2.c | 66 +++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 16 deletions(-)(quoted diff) +0
-0
b/drivers/media/usb/uvc/uvc_v4l2.cindex3e7e283a44a8..7d5bf8d56a99100644
drivers/media/usb/uvc/uvc_v4l2.c static int uvc_v4l2_set_streamparm(struct uvc_streaming · line 378
Quoted 7 lines
drivers/media/usb/uvc/uvc_v4l2.c +3
-1@@ -373,8 +373,11 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream, {struct uvc_streaming_control probe;struct v4l2_fract timeperframe; -uint32_t interval; +struct uvc_format *format; +struct uvc_frame *frame; +__u32 interval, tmp, d, maxd;
You can declare tmp and d inside the loop, it should simplify the code slightly. Please also avoid naming the variable tmp.
drivers/media/usb/uvc/uvc_v4l2.c static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream, · line 380
Quoted 2 lines
drivers/media/usb/uvc/uvc_v4l2.c +1
-0int ret; +int i;
i can be unsigned.
Quoted 6 lines
drivers/media/usb/uvc/uvc_v4l2.c +0
-0if (parm->type != stream->type)return -EINVAL ;@@ -396,9 +399,31 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream, return -EBUSY ; }
drivers/media/usb/uvc/uvc_v4l2.c static int uvc_v4l2_set_streamparm(struct uvc_streaming · line 409
Quoted 10 lines
drivers/media/usb/uvc/uvc_v4l2.c +7
-2+ format = stream->cur_format; + frame = stream->cur_frame; probe = stream->ctrl; - probe.dwFrameInterval = -uvc_try_frame_interval (stream->cur_frame, interval); + probe.dwFrameInterval =uvc_try_frame_interval (frame, interval); + maxd =abs ((__s32 )probe.dwFrameInterval - interval); + +/* Try frames with matching size to find the best frame interval. */ +for (i =0 ; i < format->nframes; i++) {
As a small optimization you can also stop when maxd == 0.
Quoted 29 lines
drivers/media/usb/uvc/uvc_v4l2.c +18
-0+if (&format->frame[i] == stream->cur_frame) +continue ; + +if (format->frame[i].wWidth != stream->cur_frame->wWidth || + format->frame[i].wHeight != stream->cur_frame->wHeight) +continue ; + + tmp =uvc_try_frame_interval (&format->frame[i], interval); + d =abs ((__s32 )tmp - interval); +if (d >= maxd) +continue ; + + frame = &format->frame[i]; + probe.bFrameIndex = frame->bFrameIndex; + probe.dwFrameInterval = tmp; + maxd = d; + }/* Probe the device with the new settings. */ ret =uvc_probe_video (stream, &probe);@@ -408,6 +433,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream, } stream->ctrl = probe; + stream->cur_frame = frame;mutex_unlock (&stream->mutex);/* Return the actual frame period. */
drivers/media/usb/uvc/uvc_v4l2.c static int uvc_ioctl_enum_framesizes(struct file · line 1179
Quoted 5 lines
drivers/media/usb/uvc/uvc_v4l2.c +1
-1@@ -1150,7 +1176,7 @@ static int uvc_ioctl_enum_framesizes(struct file *file, void *fh, struct uvc_streaming *stream = handle->stream;struct uvc_format *format =NULL ;struct uvc_frame *frame; -int i; +int i, index;
Could you declare unrelated variables on separate lines ?
I think index can be unsigned (and it seems i can be as well for that matter).
Quoted 26 lines
drivers/media/usb/uvc/uvc_v4l2.c +12
-2/* Look for the given pixel format */ for (i =0 ; i < stream->nformats; i++) {@@ -1162,10 +1188,20 @@ static int uvc_ioctl_enum_framesizes(struct file *file, void *fh, if (format == NULL) return -EINVAL; -if (fsize->index >= format->nframes) +/* Skip duplicate frame sizes */ +for (i =0 , index =0 ; i < format->nframes; i++) { +if (i && frame->wWidth == format->frame[i].wWidth && + frame->wHeight == format->frame[i].wHeight) +continue ; + frame = &format->frame[i]; +if (index == fsize->index) +break ; + index++; + } + +if (i == format->nframes)return -EINVAL ; - frame = &format->frame[fsize->index]; fsize->type =V4L2_FRMSIZE_TYPE_DISCRETE ; fsize->discrete.width = frame->wWidth; fsize->discrete.height = frame->wHeight;
drivers/media/usb/uvc/uvc_v4l2.c static int uvc_ioctl_enum_frameintervals(struct file · line 1218
Quoted 5 lines
drivers/media/usb/uvc/uvc_v4l2.c +1
-1@@ -1179,7 +1215,7 @@ static int uvc_ioctl_enum_frameintervals(struct file *file, void *fh, struct uvc_streaming *stream = handle->stream;struct uvc_format *format =NULL ;struct uvc_frame *frame =NULL ; -int i; +int i, index, nintervals;
Same comments here.
Quoted 30 lines
drivers/media/usb/uvc/uvc_v4l2.c +7
-6/* Look for the given pixel format and frame size */ for (i =0 ; i < stream->nformats; i++) {@@ -1191,30 +1227,28 @@ static int uvc_ioctl_enum_frameintervals(struct file *file, void *fh, if (format == NULL) return -EINVAL; + index = fival->index;for (i =0 ; i < format->nframes; i++) {if (format->frame[i].wWidth == fival->width && format->frame[i].wHeight == fival->height) { frame = &format->frame[i]; -break ; + nintervals = frame->bFrameIntervalType ?:1 ; +if (index < nintervals) +break ; + index -= nintervals; } } -if (frame ==NULL ) +if (i == format->nframes)return -EINVAL ;if (frame->bFrameIntervalType) { -if (fival->index >= frame->bFrameIntervalType) -return -EINVAL ; - fival->type =V4L2_FRMIVAL_TYPE_DISCRETE ; fival->discrete.numerator = - frame->dwFrameInterval[fival->index]; + frame->dwFrameInterval[index];
drivers/media/usb/uvc/uvc_v4l2.c static int uvc_ioctl_enum_frameintervals(struct file · line 1254
Quoted 10 lines
drivers/media/usb/uvc/uvc_v4l2.c +0
-3fival->discrete.denominator =10000000 ;uvc_simplify_fraction (&fival->discrete.numerator, &fival->discrete.denominator,8 ,333 ); }else { -if (fival->index) -return -EINVAL ; - fival->type =V4L2_FRMIVAL_TYPE_STEPWISE ; fival->stepwise.min.numerator = frame->dwFrameInterval[0 ]; fival->stepwise.min.denominator =10000000 ;
I have to say I dislike the complexity added by this patch, but it's not your fault. The code itself is probably as clean as it can be :)
Signature · 3 lines
Regards, Laurent Pinchart