Re: [PATCH v2] [media] uvcvideo: Add quirk to force the Oculus DK2 IR tracker to grayscale

2014-10-06 Thread Laurent Pinchart
Hi Philipp,

On Monday 29 September 2014 21:38:39 Philipp Zabel wrote:
 On Wed, Aug 6, 2014 at 10:50 PM, Philipp Zabel wrote:
  This patch adds a quirk to force Y8 pixel format even if the camera
  reports half-width YUYV.
  
  Signed-off-by: Philipp Zabel philipp.za...@gmail.com
 
 do you have any further comments on this patch?

Sorry for the late reply. Please see below for a couple of small comments.

  ---
  
   drivers/media/usb/uvc/uvc_driver.c | 29 -
   drivers/media/usb/uvc/uvcvideo.h   |  1 +
   2 files changed, 29 insertions(+), 1 deletion(-)
  
  diff --git a/drivers/media/usb/uvc/uvc_driver.c
  b/drivers/media/usb/uvc/uvc_driver.c index c3bb250..90a8f10 100644
  --- a/drivers/media/usb/uvc/uvc_driver.c
  +++ b/drivers/media/usb/uvc/uvc_driver.c
  @@ -311,6 +311,7 @@ static int uvc_parse_format(struct uvc_device *dev,
  struct uvc_format_desc *fmtdesc;
  struct uvc_frame *frame;
  const unsigned char *start = buffer;
  +   bool force_yuy2_to_y8 = false;

To keep things a bit more generic, how about an unsigned int width_multiplier 
initialized to 1 and set to 2 when the quirk applies ?

  unsigned int interval;
  unsigned int i, n;
  __u8 ftype;
  @@ -333,6 +334,22 @@ static int uvc_parse_format(struct uvc_device *dev,
  
  /* Find the format descriptor from its GUID. */
  fmtdesc = uvc_format_by_guid(buffer[5]);
  +   format-bpp = buffer[21];
  +
  +   if (dev-quirks  UVC_QUIRK_FORCE_Y8) {
  +   if (fmtdesc  fmtdesc-fcc == V4L2_PIX_FMT_YUYV
  
  +   format-bpp == 16) {

I wonder if that check is really needed, all YUYV formats should have 16bpp.

  +   force_yuy2_to_y8 = true;
  +   fmtdesc = uvc_fmts[9];

The hardcoded index here is hair-raising :-) How about something like the 
following instead ?

}
 
format-bpp = buffer[21];
+
+   /* Some devices report a format that doesn't match what they
+* really send.
+*/
+   if (dev-quirks  UVC_QUIRK_FORCE_Y8) {
+   if (format-fcc == V4L2_PIX_FMT_YUYV) {
+   strlcpy(format-name, Greyscale 8-bit (Y8  ),
+   sizeof(format-name));
+   format-fcc = V4L2_PIX_FMT_GREY;
+   format-bpp = 8;
+   width_multiplier = 2;
+   }
+   }
+
if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
ftype = UVC_VS_FRAME_UNCOMPRESSED;
} else {

I know it duplicates the format string, but as we're trying to move them to 
the V4L2 core anyway, I don't see that as being a big problem.


  +   format-bpp = 8;
  +   } else {
  +   uvc_printk(KERN_WARNING,
  +   Forcing %d-bit %s to %s not
  supported,
  +   format-bpp, fmtdesc-name,
  +   uvc_fmts[9].name);
  +   }
  +   }
  +
  if (fmtdesc != NULL) {
  strlcpy(format-name, fmtdesc-name,
  sizeof format-name);
  @@ -345,7 +362,6 @@ static int uvc_parse_format(struct uvc_device *dev,
  format-fcc = 0;
  }
  
  -   format-bpp = buffer[21];
  if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
  ftype = UVC_VS_FRAME_UNCOMPRESSED;
  } else {
  @@ -455,6 +471,8 @@ static int uvc_parse_format(struct uvc_device *dev,
  frame-bFrameIndex = buffer[3];
  frame-bmCapabilities = buffer[4];
  frame-wWidth = get_unaligned_le16(buffer[5]);
  +   if (force_yuy2_to_y8)
  +   frame-wWidth *= 2;

This would become

+   frame-wWidth = get_unaligned_le16(buffer[5])
+ * width_multiplier;

If you're fine with that there's no need to resubmit, I'll modify the patch 
when applying it to my tree.

  frame-wHeight = get_unaligned_le16(buffer[7]);
  frame-dwMinBitRate = get_unaligned_le32(buffer[9]);
  frame-dwMaxBitRate = get_unaligned_le32(buffer[13]);
  @@ -2467,6 +2485,15 @@ static struct usb_device_id uvc_ids[] = {
.bInterfaceProtocol   = 0,
.driver_info  = UVC_QUIRK_PROBE_MINMAX
  | UVC_QUIRK_IGNORE_SELECTOR_UNIT },
  +   /* Oculus VR Positional Tracker DK2 */
  +   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
  +   | 

Re: [PATCH v2] [media] uvcvideo: Add quirk to force the Oculus DK2 IR tracker to grayscale

2014-10-06 Thread Philipp Zabel
Hi Laurent,

On Mon, Oct 6, 2014 at 4:34 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
  @@ -311,6 +311,7 @@ static int uvc_parse_format(struct uvc_device *dev,
  struct uvc_format_desc *fmtdesc;
  struct uvc_frame *frame;
  const unsigned char *start = buffer;
  +   bool force_yuy2_to_y8 = false;

 To keep things a bit more generic, how about an unsigned int width_multiplier
 initialized to 1 and set to 2 when the quirk applies ?
[...]
  @@ -333,6 +334,22 @@ static int uvc_parse_format(struct uvc_device *dev,
 
  /* Find the format descriptor from its GUID. */
  fmtdesc = uvc_format_by_guid(buffer[5]);
  +   format-bpp = buffer[21];
  +
  +   if (dev-quirks  UVC_QUIRK_FORCE_Y8) {
  +   if (fmtdesc  fmtdesc-fcc == V4L2_PIX_FMT_YUYV
  
  +   format-bpp == 16) {

 I wonder if that check is really needed, all YUYV formats should have 16bpp.

  +   force_yuy2_to_y8 = true;
  +   fmtdesc = uvc_fmts[9];

 The hardcoded index here is hair-raising :-) How about something like the
 following instead ?

 }

 format-bpp = buffer[21];
 +
 +   /* Some devices report a format that doesn't match what they
 +* really send.
 +*/
 +   if (dev-quirks  UVC_QUIRK_FORCE_Y8) {
 +   if (format-fcc == V4L2_PIX_FMT_YUYV) {
 +   strlcpy(format-name, Greyscale 8-bit (Y8  
 ),
 +   sizeof(format-name));
 +   format-fcc = V4L2_PIX_FMT_GREY;
 +   format-bpp = 8;
 +   width_multiplier = 2;
 +   }
 +   }
 +
 if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
 ftype = UVC_VS_FRAME_UNCOMPRESSED;
 } else {

 I know it duplicates the format string, but as we're trying to move them to
 the V4L2 core anyway, I don't see that as being a big problem.
[...]
  @@ -455,6 +471,8 @@ static int uvc_parse_format(struct uvc_device *dev,
  frame-bFrameIndex = buffer[3];
  frame-bmCapabilities = buffer[4];
  frame-wWidth = get_unaligned_le16(buffer[5]);
  +   if (force_yuy2_to_y8)
  +   frame-wWidth *= 2;

 This would become

 +   frame-wWidth = get_unaligned_le16(buffer[5])
 + * width_multiplier;

 If you're fine with that there's no need to resubmit, I'll modify the patch
 when applying it to my tree.

Thank you, I'm fine with your suggested changes.
Especially the format setting part looks a lot more civilized now.

regards
Philipp
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] [media] uvcvideo: Add quirk to force the Oculus DK2 IR tracker to grayscale

2014-10-06 Thread Laurent Pinchart
Hi Philipp,

On Monday 06 October 2014 23:45:54 Philipp Zabel wrote:
 On Mon, Oct 6, 2014 at 4:34 PM, Laurent Pinchart wrote:
   @@ -311,6 +311,7 @@ static int uvc_parse_format(struct uvc_device *dev,
   struct uvc_format_desc *fmtdesc;
   struct uvc_frame *frame;
   const unsigned char *start = buffer;
   +   bool force_yuy2_to_y8 = false;
  
  To keep things a bit more generic, how about an unsigned int
  width_multiplier initialized to 1 and set to 2 when the quirk applies ?
 
 [...]
 
   @@ -333,6 +334,22 @@ static int uvc_parse_format(struct uvc_device
   *dev,
   
   /* Find the format descriptor from its GUID. */
   fmtdesc = uvc_format_by_guid(buffer[5]);
   
   +   format-bpp = buffer[21];
   +
   +   if (dev-quirks  UVC_QUIRK_FORCE_Y8) {
   +   if (fmtdesc  fmtdesc-fcc ==
   V4L2_PIX_FMT_YUYV
   
   +   format-bpp == 16) {
  
  I wonder if that check is really needed, all YUYV formats should have
  16bpp.
 
   +   force_yuy2_to_y8 = true;
   +   fmtdesc = uvc_fmts[9];
  
  The hardcoded index here is hair-raising :-) How about something like the
  following instead ?
  
  }
  
  format-bpp = buffer[21];
  
  +
  +   /* Some devices report a format that doesn't match what
  they
  +* really send.
  +*/
  +   if (dev-quirks  UVC_QUIRK_FORCE_Y8) {
  +   if (format-fcc == V4L2_PIX_FMT_YUYV) {
  +   strlcpy(format-name, Greyscale 8-bit (Y8
   ),
   +   sizeof(format-name));
  +   format-fcc = V4L2_PIX_FMT_GREY;
  +   format-bpp = 8;
  +   width_multiplier = 2;
  +   }
  +   }
  +
  
  if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
  ftype = UVC_VS_FRAME_UNCOMPRESSED;
  } else {
  
  I know it duplicates the format string, but as we're trying to move them
  to the V4L2 core anyway, I don't see that as being a big problem.
 
 [...]
 
   @@ -455,6 +471,8 @@ static int uvc_parse_format(struct uvc_device *dev,
   
   frame-bFrameIndex = buffer[3];
   frame-bmCapabilities = buffer[4];
   frame-wWidth = get_unaligned_le16(buffer[5]);
   
   +   if (force_yuy2_to_y8)
   +   frame-wWidth *= 2;
  
  This would become
  
  +   frame-wWidth = get_unaligned_le16(buffer[5])
  + * width_multiplier;
  
  If you're fine with that there's no need to resubmit, I'll modify the
  patch when applying it to my tree.
 
 Thank you, I'm fine with your suggested changes.
 Especially the format setting part looks a lot more civilized now.

Great. I'll add the patch to my next pull request. Thank you.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] [media] uvcvideo: Add quirk to force the Oculus DK2 IR tracker to grayscale

2014-09-29 Thread Philipp Zabel
Hi Laurent,

On Wed, Aug 6, 2014 at 10:50 PM, Philipp Zabel philipp.za...@gmail.com wrote:
 This patch adds a quirk to force Y8 pixel format even if the camera reports
 half-width YUYV.

 Signed-off-by: Philipp Zabel philipp.za...@gmail.com

do you have any further comments on this patch?

regards
Philipp

 ---
  drivers/media/usb/uvc/uvc_driver.c | 29 -
  drivers/media/usb/uvc/uvcvideo.h   |  1 +
  2 files changed, 29 insertions(+), 1 deletion(-)

 diff --git a/drivers/media/usb/uvc/uvc_driver.c 
 b/drivers/media/usb/uvc/uvc_driver.c
 index c3bb250..90a8f10 100644
 --- a/drivers/media/usb/uvc/uvc_driver.c
 +++ b/drivers/media/usb/uvc/uvc_driver.c
 @@ -311,6 +311,7 @@ static int uvc_parse_format(struct uvc_device *dev,
 struct uvc_format_desc *fmtdesc;
 struct uvc_frame *frame;
 const unsigned char *start = buffer;
 +   bool force_yuy2_to_y8 = false;
 unsigned int interval;
 unsigned int i, n;
 __u8 ftype;
 @@ -333,6 +334,22 @@ static int uvc_parse_format(struct uvc_device *dev,
 /* Find the format descriptor from its GUID. */
 fmtdesc = uvc_format_by_guid(buffer[5]);

 +   format-bpp = buffer[21];
 +
 +   if (dev-quirks  UVC_QUIRK_FORCE_Y8) {
 +   if (fmtdesc  fmtdesc-fcc == V4L2_PIX_FMT_YUYV 
 +   format-bpp == 16) {
 +   force_yuy2_to_y8 = true;
 +   fmtdesc = uvc_fmts[9];
 +   format-bpp = 8;
 +   } else {
 +   uvc_printk(KERN_WARNING,
 +   Forcing %d-bit %s to %s not 
 supported,
 +   format-bpp, fmtdesc-name,
 +   uvc_fmts[9].name);
 +   }
 +   }
 +
 if (fmtdesc != NULL) {
 strlcpy(format-name, fmtdesc-name,
 sizeof format-name);
 @@ -345,7 +362,6 @@ static int uvc_parse_format(struct uvc_device *dev,
 format-fcc = 0;
 }

 -   format-bpp = buffer[21];
 if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
 ftype = UVC_VS_FRAME_UNCOMPRESSED;
 } else {
 @@ -455,6 +471,8 @@ static int uvc_parse_format(struct uvc_device *dev,
 frame-bFrameIndex = buffer[3];
 frame-bmCapabilities = buffer[4];
 frame-wWidth = get_unaligned_le16(buffer[5]);
 +   if (force_yuy2_to_y8)
 +   frame-wWidth *= 2;
 frame-wHeight = get_unaligned_le16(buffer[7]);
 frame-dwMinBitRate = get_unaligned_le32(buffer[9]);
 frame-dwMaxBitRate = get_unaligned_le32(buffer[13]);
 @@ -2467,6 +2485,15 @@ static struct usb_device_id uvc_ids[] = {
   .bInterfaceProtocol   = 0,
   .driver_info  = UVC_QUIRK_PROBE_MINMAX
 | UVC_QUIRK_IGNORE_SELECTOR_UNIT },
 +   /* Oculus VR Positional Tracker DK2 */
 +   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
 +   | USB_DEVICE_ID_MATCH_INT_INFO,
 + .idVendor = 0x2833,
 + .idProduct= 0x0201,
 + .bInterfaceClass  = USB_CLASS_VIDEO,
 + .bInterfaceSubClass   = 1,
 + .bInterfaceProtocol   = 0,
 + .driver_info  = UVC_QUIRK_FORCE_Y8 },
 /* Generic USB Video Class */
 { USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, 0) },
 {}
 diff --git a/drivers/media/usb/uvc/uvcvideo.h 
 b/drivers/media/usb/uvc/uvcvideo.h
 index 9e35982..1dd78c0 100644
 --- a/drivers/media/usb/uvc/uvcvideo.h
 +++ b/drivers/media/usb/uvc/uvcvideo.h
 @@ -137,6 +137,7 @@
  #define UVC_QUIRK_FIX_BANDWIDTH0x0080
  #define UVC_QUIRK_PROBE_DEF0x0100
  #define UVC_QUIRK_RESTRICT_FRAME_RATE  0x0200
 +#define UVC_QUIRK_FORCE_Y8 0x0400

  /* Format flags */
  #define UVC_FMT_FLAG_COMPRESSED0x0001
 --
 2.0.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] [media] uvcvideo: Add quirk to force the Oculus DK2 IR tracker to grayscale

2014-08-06 Thread Philipp Zabel
This patch adds a quirk to force Y8 pixel format even if the camera reports
half-width YUYV.

Signed-off-by: Philipp Zabel philipp.za...@gmail.com
---
 drivers/media/usb/uvc/uvc_driver.c | 29 -
 drivers/media/usb/uvc/uvcvideo.h   |  1 +
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c 
b/drivers/media/usb/uvc/uvc_driver.c
index c3bb250..90a8f10 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -311,6 +311,7 @@ static int uvc_parse_format(struct uvc_device *dev,
struct uvc_format_desc *fmtdesc;
struct uvc_frame *frame;
const unsigned char *start = buffer;
+   bool force_yuy2_to_y8 = false;
unsigned int interval;
unsigned int i, n;
__u8 ftype;
@@ -333,6 +334,22 @@ static int uvc_parse_format(struct uvc_device *dev,
/* Find the format descriptor from its GUID. */
fmtdesc = uvc_format_by_guid(buffer[5]);
 
+   format-bpp = buffer[21];
+
+   if (dev-quirks  UVC_QUIRK_FORCE_Y8) {
+   if (fmtdesc  fmtdesc-fcc == V4L2_PIX_FMT_YUYV 
+   format-bpp == 16) {
+   force_yuy2_to_y8 = true;
+   fmtdesc = uvc_fmts[9];
+   format-bpp = 8;
+   } else {
+   uvc_printk(KERN_WARNING,
+   Forcing %d-bit %s to %s not supported,
+   format-bpp, fmtdesc-name,
+   uvc_fmts[9].name);
+   }
+   }
+
if (fmtdesc != NULL) {
strlcpy(format-name, fmtdesc-name,
sizeof format-name);
@@ -345,7 +362,6 @@ static int uvc_parse_format(struct uvc_device *dev,
format-fcc = 0;
}
 
-   format-bpp = buffer[21];
if (buffer[2] == UVC_VS_FORMAT_UNCOMPRESSED) {
ftype = UVC_VS_FRAME_UNCOMPRESSED;
} else {
@@ -455,6 +471,8 @@ static int uvc_parse_format(struct uvc_device *dev,
frame-bFrameIndex = buffer[3];
frame-bmCapabilities = buffer[4];
frame-wWidth = get_unaligned_le16(buffer[5]);
+   if (force_yuy2_to_y8)
+   frame-wWidth *= 2;
frame-wHeight = get_unaligned_le16(buffer[7]);
frame-dwMinBitRate = get_unaligned_le32(buffer[9]);
frame-dwMaxBitRate = get_unaligned_le32(buffer[13]);
@@ -2467,6 +2485,15 @@ static struct usb_device_id uvc_ids[] = {
  .bInterfaceProtocol   = 0,
  .driver_info  = UVC_QUIRK_PROBE_MINMAX
| UVC_QUIRK_IGNORE_SELECTOR_UNIT },
+   /* Oculus VR Positional Tracker DK2 */
+   { .match_flags  = USB_DEVICE_ID_MATCH_DEVICE
+   | USB_DEVICE_ID_MATCH_INT_INFO,
+ .idVendor = 0x2833,
+ .idProduct= 0x0201,
+ .bInterfaceClass  = USB_CLASS_VIDEO,
+ .bInterfaceSubClass   = 1,
+ .bInterfaceProtocol   = 0,
+ .driver_info  = UVC_QUIRK_FORCE_Y8 },
/* Generic USB Video Class */
{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, 0) },
{}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 9e35982..1dd78c0 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -137,6 +137,7 @@
 #define UVC_QUIRK_FIX_BANDWIDTH0x0080
 #define UVC_QUIRK_PROBE_DEF0x0100
 #define UVC_QUIRK_RESTRICT_FRAME_RATE  0x0200
+#define UVC_QUIRK_FORCE_Y8 0x0400
 
 /* Format flags */
 #define UVC_FMT_FLAG_COMPRESSED0x0001
-- 
2.0.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html