* [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions
@ 2017-12-19 8:17 Philipp Zabel
2018-01-04 21:42 ` Laurent Pinchart
0 siblings, 1 reply; 5+ messages in thread
From: Philipp Zabel @ 2017-12-19 8:17 UTC (permalink / raw)
To: linux-media; +Cc: Laurent Pinchart, Nicolas Dufresne, Philipp Zabel
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.
Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
drivers/media/usb/uvc/uvc_v4l2.c | 66 ++++++++++++++++++++++++++++++----------
1 file changed, 50 insertions(+), 16 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3e7e283a44a8..7d5bf8d56a99 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -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;
--
2.15.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions
2017-12-19 8:17 [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions Philipp Zabel
@ 2018-01-04 21:42 ` Laurent Pinchart
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2018-01-04 21:42 UTC (permalink / raw)
To: Philipp Zabel; +Cc: linux-media, Nicolas Dufresne
Hi Philipp,
Thank you for the patch.
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 ? :-(
> 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.
>
> Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
> ---
> drivers/media/usb/uvc/uvc_v4l2.c | 66 +++++++++++++++++++++++++++----------
> 1 file changed, 50 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index 3e7e283a44a8..7d5bf8d56a99 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -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.
> int ret;
> + int i;
i can be unsigned.
> 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++) {
As a small optimization you can also stop when maxd == 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. */
> @@ -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).
> /* 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;
Same comments here.
> /* 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;
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 :)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions
@ 2018-01-04 22:51 Philipp Zabel
2018-01-05 13:30 ` Laurent Pinchart
0 siblings, 1 reply; 5+ messages in thread
From: Philipp Zabel @ 2018-01-04 22:51 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, Philipp Zabel
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.
Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
Changes since v1 [1]:
- Break out of frame size loop if maxd == 0 in uvc_v4l2_set_streamparm.
- Moved d and tmp variables in uvc_v4l2_set_streamparm into loop,
renamed tmp variable to tmp_ival.
- Changed i loop variables to unsigned int.
- Changed index variables to unsigned int.
- One line per variable declaration.
[1] https://patchwork.linuxtv.org/patch/46109/
---
drivers/media/usb/uvc/uvc_v4l2.c | 71 +++++++++++++++++++++++++++++++---------
1 file changed, 55 insertions(+), 16 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index f5ab8164bca5..d9ee400bf47c 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -373,7 +373,10 @@ 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, maxd;
+ unsigned int i;
int ret;
if (parm->type != stream->type)
@@ -396,9 +399,33 @@ 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 && maxd != 0; i++) {
+ __u32 d, tmp_ival;
+
+ 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_ival = uvc_try_frame_interval(&format->frame[i], interval);
+ d = abs((__s32)tmp_ival - interval);
+ if (d >= maxd)
+ continue;
+
+ frame = &format->frame[i];
+ probe.bFrameIndex = frame->bFrameIndex;
+ probe.dwFrameInterval = tmp_ival;
+ maxd = d;
+ }
/* Probe the device with the new settings. */
ret = uvc_probe_video(stream, &probe);
@@ -408,6 +435,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. */
@@ -1209,7 +1237,8 @@ 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;
+ unsigned int index;
+ unsigned int i;
/* Look for the given pixel format */
for (i = 0; i < stream->nformats; i++) {
@@ -1221,10 +1250,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;
@@ -1238,7 +1277,9 @@ 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;
+ unsigned int nintervals;
+ unsigned int index;
+ unsigned int i;
/* Look for the given pixel format and frame size */
for (i = 0; i < stream->nformats; i++) {
@@ -1250,30 +1291,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;
--
2.15.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions
2018-01-04 22:51 Philipp Zabel
@ 2018-01-05 13:30 ` Laurent Pinchart
2018-01-05 16:35 ` Philipp Zabel
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2018-01-05 13:30 UTC (permalink / raw)
To: Philipp Zabel; +Cc: linux-media
Hi Philipp,
Thank you for the patch.
On Friday, 5 January 2018 00:51:29 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:
>
> 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.
>
> Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
> ---
> Changes since v1 [1]:
> - Break out of frame size loop if maxd == 0 in uvc_v4l2_set_streamparm.
> - Moved d and tmp variables in uvc_v4l2_set_streamparm into loop,
> renamed tmp variable to tmp_ival.
> - Changed i loop variables to unsigned int.
> - Changed index variables to unsigned int.
> - One line per variable declaration.
>
> [1] https://patchwork.linuxtv.org/patch/46109/
> ---
> drivers/media/usb/uvc/uvc_v4l2.c | 71
> +++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+),
> 16 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index f5ab8164bca5..d9ee400bf47c 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -373,7 +373,10 @@ 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, maxd;
> + unsigned int i;
> int ret;
>
> if (parm->type != stream->type)
> @@ -396,9 +399,33 @@ 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 && maxd != 0; i++) {
> + __u32 d, tmp_ival;
How about ival instead of tmp_ival ?
Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
If you're fine with the change there's no need to resubmit.
> +
> + 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_ival = uvc_try_frame_interval(&format->frame[i], interval);
> + d = abs((__s32)tmp_ival - interval);
> + if (d >= maxd)
> + continue;
> +
> + frame = &format->frame[i];
> + probe.bFrameIndex = frame->bFrameIndex;
> + probe.dwFrameInterval = tmp_ival;
> + maxd = d;
> + }
>
> /* Probe the device with the new settings. */
> ret = uvc_probe_video(stream, &probe);
> @@ -408,6 +435,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. */
> @@ -1209,7 +1237,8 @@ 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;
> + unsigned int index;
> + unsigned int i;
>
> /* Look for the given pixel format */
> for (i = 0; i < stream->nformats; i++) {
> @@ -1221,10 +1250,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;
> @@ -1238,7 +1277,9 @@ 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;
> + unsigned int nintervals;
> + unsigned int index;
> + unsigned int i;
>
> /* Look for the given pixel format and frame size */
> for (i = 0; i < stream->nformats; i++) {
> @@ -1250,30 +1291,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;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions
2018-01-05 13:30 ` Laurent Pinchart
@ 2018-01-05 16:35 ` Philipp Zabel
0 siblings, 0 replies; 5+ messages in thread
From: Philipp Zabel @ 2018-01-05 16:35 UTC (permalink / raw)
To: Laurent Pinchart, Philipp Zabel; +Cc: linux-media
Hi Laurent,
On Fri, 2018-01-05 at 15:30 +0200, Laurent Pinchart wrote:
> Hi Philipp,
>
> Thank you for the patch.
>
> On Friday, 5 January 2018 00:51:29 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:
> >
> > 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.
> >
> > Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
> > ---
> > Changes since v1 [1]:
> > - Break out of frame size loop if maxd == 0 in uvc_v4l2_set_streamparm.
> > - Moved d and tmp variables in uvc_v4l2_set_streamparm into loop,
> > renamed tmp variable to tmp_ival.
> > - Changed i loop variables to unsigned int.
> > - Changed index variables to unsigned int.
> > - One line per variable declaration.
> >
> > [1] https://patchwork.linuxtv.org/patch/46109/
> > ---
> > drivers/media/usb/uvc/uvc_v4l2.c | 71
> > +++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+),
> > 16 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> > b/drivers/media/usb/uvc/uvc_v4l2.c index f5ab8164bca5..d9ee400bf47c 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -373,7 +373,10 @@ 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, maxd;
> > + unsigned int i;
> > int ret;
> >
> > if (parm->type != stream->type)
> > @@ -396,9 +399,33 @@ 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 && maxd != 0; i++) {
> > + __u32 d, tmp_ival;
>
> How about ival instead of tmp_ival ?
>
> Apart from that,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> If you're fine with the change there's no need to resubmit.
Absolutely, thanks for the review!
regards
Philipp
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-05 16:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-19 8:17 [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions Philipp Zabel
2018-01-04 21:42 ` Laurent Pinchart
-- strict thread matches above, loose matches on Subject: below --
2018-01-04 22:51 Philipp Zabel
2018-01-05 13:30 ` Laurent Pinchart
2018-01-05 16:35 ` Philipp Zabel
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).