subject:"\[PATCH\] media\: uvcvideo\: support multiple frame descriptors with the same dimensions"

Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

2018-01-05 Thread Philipp Zabel

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:
> > bLength30
> > bDescriptorType36
> > bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> > bFrameIndex 1
> > bmCapabilities   0x00
> >   Still image unsupported
> > wWidth   1280
> > wHeight   481
> > dwMinBitRate147763200
> > dwMaxBitRate147763200
> > dwMaxVideoFrameBufferSize  615680
> > dwDefaultFrameInterval 33
> > bFrameIntervalType  1
> > dwFrameInterval( 0)33
> >   VideoStreaming Interface Descriptor:
> > bLength30
> > bDescriptorType36
> > bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> > bFrameIndex 2
> > bmCapabilities   0x00
> >   Still image unsupported
> > wWidth   1280
> > wHeight   481
> > dwMinBitRate443289600
> > dwMaxBitRate443289600
> > dwMaxVideoFrameBufferSize  615680
> > dwDefaultFrameInterval 11
> > bFrameIntervalType  1
> > dwFrameInterval( 0)11
> > 
> > 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 
> > ---
> > 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 
> 
> If you're fine with the change there's no need to resubmit.

Absolutely, thanks for the review!

regards
Philipp

Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

2018-01-05 Thread Laurent Pinchart

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:
> bLength30
> bDescriptorType36
> bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> bFrameIndex 1
> bmCapabilities   0x00
>   Still image unsupported
> wWidth   1280
> wHeight   481
> dwMinBitRate147763200
> dwMaxBitRate147763200
> dwMaxVideoFrameBufferSize  615680
> dwDefaultFrameInterval 33
> bFrameIntervalType  1
> dwFrameInterval( 0)33
>   VideoStreaming Interface Descriptor:
> bLength30
> bDescriptorType36
> bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> bFrameIndex 2
> bmCapabilities   0x00
>   Still image unsupported
> wWidth   1280
> wHeight   481
> dwMinBitRate443289600
> dwMaxBitRate443289600
> dwMaxVideoFrameBufferSize  615680
> dwDefaultFrameInterval 11
> bFrameIntervalType  1
> dwFrameInterval( 0)11
> 
> 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 
> ---
> 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 

If you're fine with the change there's no need to resubmit.

> +
> + if (>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(>frame[i], interval);
> + d = abs((__s32)tmp_ival - interval);
> + if (d >= maxd)
> + continue;
> +
> + frame = >frame[i];
> + probe.bFrameIndex = frame->bFrameIndex;
> + probe.dwFrameInterval = tmp_ival;
> + maxd = d;
> + }
> 
>   /* Probe the device with the new settings. */
>   ret = uvc_probe_video(stream, );
> @@ -408,6 +435,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming
> *stream, }
> 
>   stream->ctrl = probe;
> + stream->cur_frame = frame;
>   mutex_unlock(>mutex);
> 
>   /* Return the actual frame period. */
>

[PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

2018-01-04 Thread 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:
bLength30
bDescriptorType36
bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
bFrameIndex 1
bmCapabilities   0x00
  Still image unsupported
wWidth   1280
wHeight   481
dwMinBitRate147763200
dwMaxBitRate147763200
dwMaxVideoFrameBufferSize  615680
dwDefaultFrameInterval 33
bFrameIntervalType  1
dwFrameInterval( 0)33
  VideoStreaming Interface Descriptor:
bLength30
bDescriptorType36
bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
bFrameIndex 2
bmCapabilities   0x00
  Still image unsupported
wWidth   1280
wHeight   481
dwMinBitRate443289600
dwMaxBitRate443289600
dwMaxVideoFrameBufferSize  615680
dwDefaultFrameInterval 11
bFrameIntervalType  1
dwFrameInterval( 0)11

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 
---
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 (>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(>frame[i], interval);
+   d = abs((__s32)tmp_ival - interval);
+   if (d >= maxd)
+   continue;
+
+   frame = >frame[i];
+   probe.bFrameIndex = frame->bFrameIndex;
+   probe.dwFrameInterval = tmp_ival;
+   maxd = d;
+   }
 
/* Probe the device with the new settings. */
ret = uvc_probe_video(stream, );
@@ -408,6 +435,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming 
*stream,
}
 
stream->ctrl = probe;
+   stream->cur_frame = frame;
mutex_unlock(>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

Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

2018-01-04 Thread Laurent Pinchart

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:
> bLength30
> bDescriptorType36
> bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> bFrameIndex 1
> bmCapabilities   0x00
>   Still image unsupported
> wWidth   1280
> wHeight   481
> dwMinBitRate147763200
> dwMaxBitRate147763200
> dwMaxVideoFrameBufferSize  615680
> dwDefaultFrameInterval 33
> bFrameIntervalType  1
> dwFrameInterval( 0)33
>   VideoStreaming Interface Descriptor:
> bLength30
> bDescriptorType36
> bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> bFrameIndex 2
> bmCapabilities   0x00
>   Still image unsupported
> wWidth   1280
> wHeight   481
> dwMinBitRate443289600
> dwMaxBitRate443289600
> dwMaxVideoFrameBufferSize  615680
> dwDefaultFrameInterval 11
> bFrameIntervalType  1
> dwFrameInterval( 0)11
> 
> 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 
> ---
>  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 (>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(>frame[i], interval);
> + d = abs((__s32)tmp - interval);
> + if (d >= maxd)
> + continue;
> +
> + frame = >frame[i];
> + probe.bFrameIndex = frame->bFrameIndex;
> + probe.dwFrameInterval = tmp;
> + maxd = d;
> + }
> 
>   /* Probe the device with the new settings. */
>   ret = uvc_probe_video(stream, );
> @@ -408,6 +433,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming
> *stream, }
> 
>   stream->ctrl = probe;
> + stream->cur_frame = frame;
>   mutex_unlock(>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).

>

[PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

2017-12-19 Thread 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:
bLength30
bDescriptorType36
bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
bFrameIndex 1
bmCapabilities   0x00
  Still image unsupported
wWidth   1280
wHeight   481
dwMinBitRate147763200
dwMaxBitRate147763200
dwMaxVideoFrameBufferSize  615680
dwDefaultFrameInterval 33
bFrameIntervalType  1
dwFrameInterval( 0)33
  VideoStreaming Interface Descriptor:
bLength30
bDescriptorType36
bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
bFrameIndex 2
bmCapabilities   0x00
  Still image unsupported
wWidth   1280
wHeight   481
dwMinBitRate443289600
dwMaxBitRate443289600
dwMaxVideoFrameBufferSize  615680
dwDefaultFrameInterval 11
bFrameIntervalType  1
dwFrameInterval( 0)11

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 
---
 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 (>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(>frame[i], interval);
+   d = abs((__s32)tmp - interval);
+   if (d >= maxd)
+   continue;
+
+   frame = >frame[i];
+   probe.bFrameIndex = frame->bFrameIndex;
+   probe.dwFrameInterval = tmp;
+   maxd = d;
+   }
 
/* Probe the device with the new settings. */
ret = uvc_probe_video(stream, );
@@ -408,6 +433,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming 
*stream,
}
 
stream->ctrl = probe;
+   stream->cur_frame = frame;
mutex_unlock(>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)
+

Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

[PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

[PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

5 matches

Site Navigation

Mail list logo

Footer information