[PATCH 0/2] uvcvideo: *** Support Intel SR300 Depth camera formats ***

[PATCH 0/2] uvcvideo: *** Support Intel SR300 Depth camera formats ***

[evgeni.raikhel]

From: eraikhel <evgeni.raikhel@intel.com>

*** Enable Intel RealSense™ SR300 Depth camera pixel formats to be recognized correctly by uvc module  ***

Aviv Greenberg (1):
  UVC: Add support for Intel SR300 depth camera

Evgeni Raikhel (1):
  Document Intel SR300 Depth camera INZI format

 Documentation/media/uapi/v4l/pixfmt-inzi.rst | 40 ++++++++++++++++++++++++++++
 drivers/media/usb/uvc/uvc_driver.c           | 15 +++++++++++
 drivers/media/usb/uvc/uvcvideo.h             |  9 +++++++
 include/uapi/linux/videodev2.h               |  1 +
 4 files changed, 65 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-inzi.rst

-- 
2.7.4

[PATCH 1/2] uvcvideo: Add support for Intel SR300 depth camera

[evgeni.raikhel]

From: Aviv Greenberg <aviv.d.greenberg@intel.com>

Add support for Intel SR300 depth camera in uvc driver.
This includes adding three uvc GUIDs for the required pixel formats,
adding a new V4L pixel format definition to user api headers,
and updating the uvc driver GUID-to-4cc tables with the new formats.

Signed-off-by: Aviv Greenberg <aviv.d.greenberg@intel.com>
Signed-off-by: Evgeni Raikhel <evgeni.raikhel@intel.com>
---
 drivers/media/usb/uvc/uvc_driver.c | 15 +++++++++++++++
 drivers/media/usb/uvc/uvcvideo.h   |  9 +++++++++
 include/uapi/linux/videodev2.h     |  1 +
 3 files changed, 25 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 11744f92097b..5b96a89f29ae 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -168,6 +168,21 @@ static struct uvc_format_desc uvc_fmts[] = {
 		.guid		= UVC_GUID_FORMAT_RW10,
 		.fcc		= V4L2_PIX_FMT_SRGGB10P,
 	},
+	{
+		.name		= "Depth data 16-bit (Z16)",
+		.guid		= UVC_GUID_FORMAT_INVZ,
+		.fcc		= V4L2_PIX_FMT_Z16,
+	},
+	{
+		.name		= "IR:Depth 26-bit (INZI)",
+		.guid		= UVC_GUID_FORMAT_INZI,
+		.fcc		= V4L2_PIX_FMT_INZI,
+	},
+	{
+		.name		= "Greyscale 10-bit (Y10 )",
+		.guid		= UVC_GUID_FORMAT_INVI,
+		.fcc		= V4L2_PIX_FMT_Y10,
+	},
 };
 
 /* ------------------------------------------------------------------------
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 7e4d3eea371b..460b99ca99b7 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -131,6 +131,15 @@
 #define UVC_GUID_FORMAT_RW10 \
 	{ 'R',  'W',  '1',  '0', 0x00, 0x00, 0x10, 0x00, \
 	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
+#define UVC_GUID_FORMAT_INVZ \
+	{ 'I',  'N',  'V',  'Z', 0x90, 0x2d, 0x58, 0x4a, \
+	 0x92, 0x0b, 0x77, 0x3f, 0x1f, 0x2c, 0x55, 0x6b}
+#define UVC_GUID_FORMAT_INZI \
+	{ 'I',  'N',  'Z',  'I', 0x66, 0x1a, 0x42, 0xa2, \
+	 0x90, 0x65, 0xd0, 0x18, 0x14, 0xa8, 0xef, 0x8a}
+#define UVC_GUID_FORMAT_INVI \
+	{ 'I',  'N',  'V',  'I', 0xdb, 0x57, 0x49, 0x5e, \
+	 0x8e, 0x3f, 0xf4, 0x79, 0x53, 0x2b, 0x94, 0x6f}
 
 /* ------------------------------------------------------------------------
  * Driver specific constants.
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index d3f613e2c54a..4ab995bbec5b 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -659,6 +659,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_Y12I     v4l2_fourcc('Y', '1', '2', 'I') /* Greyscale 12-bit L/R interleaved */
 #define V4L2_PIX_FMT_Z16      v4l2_fourcc('Z', '1', '6', ' ') /* Depth data 16-bit */
 #define V4L2_PIX_FMT_MT21C    v4l2_fourcc('M', 'T', '2', '1') /* Mediatek compressed block mode  */
+#define V4L2_PIX_FMT_INZI     v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Infrared 10-bit linked with Depth 16-bit */
 
 /* SDR formats - used only for Software Defined Radio devices */
 #define V4L2_SDR_FMT_CU8          v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */
-- 
2.7.4

[PATCH 2/2] uvcvideo: Document Intel SR300 Depth camera INZI format

[evgeni.raikhel]

From: Evgeni Raikhel <evgeni.raikhel@intel.com>

Provide the frame structure and data layout of V4L2-PIX-FMT-INZI
format utilized by Intel SR300 Depth camera.

This is a complimentary patch for:
[PATCH] UVC: Add support for Intel SR300 depth camera

Signed-off-by: Evgeni Raikhel <evgeni.raikhel@intel.com>
---
 Documentation/media/uapi/v4l/pixfmt-inzi.rst | 40 ++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-inzi.rst

diff --git a/Documentation/media/uapi/v4l/pixfmt-inzi.rst b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
new file mode 100644
index 000000000000..cdfdeae4a664
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
@@ -0,0 +1,40 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2-PIX-FMT-INZI:
+
+**************************
+V4L2_PIX_FMT_INZI ('INZI')
+**************************
+
+Infrared 10-bit linked with Depth 16-bit images
+
+
+Description
+===========
+
+Custom multi-planar format used by Intel SR300 Depth cameras, comprise of Infrared image followed by Depth data.
+The pixel definition is 32-bpp, with the Depth and Infrared Data split into separate continuous planes of identical dimensions.
+
+The first plane - Infrared data - is stored in V4L2_PIX_FMT_Y10 (see :ref:`pixfmt-y10`) greyscale format. Each pixel is 16-bit cell, with actual data present in the 10 LSBs with values in range 0 to 1023. The six remaining MSBs are padded with zeros.
+
+The second plane provides 16-bit per-pixel Depth data in V4L2_PIX_FMT_Z16 (:ref:`pixfmt-z16`) format.
+
+
+**Frame Structure.**
+Each cell is a 16-bit word with the significant data byte is stored at lower memory address (little-endian).
+
++-----------------+-----------------+-----------------+-----------------+-----------------+-----------------+
+| Ir\ :sub:`0`    | Ir\ :sub:`1`    | Ir\ :sub:`2`    |       ...       |        ...      |       ...       |
++-----------------+-----------------+-----------------+-----------------+-----------------+-----------------+
+|      ...       ...       ...                                                                              |
+|                                 Infrared Data                                                             |
+|                                                 ...   ...   ...                                           |
++-----------------+-----------------+-----------------+-----------------+-----------------+-----------------+
+| Ir\ :sub:`n-3`  | Ir\ :sub:`n-2`  | Ir\ :sub:`n-1`  | Depth\ :sub:`0` | Depth\ :sub:`1` | Depth\ :sub:`2` |
++-----------------+-----------------+-----------------+-----------------+-----------------+-----------------+
+|      ...       ...       ...                                                                              |
+|                                 Depth Data                                                                |
+|                                                 ...   ...   ...                                           |
++-----------------+-----------------+-----------------+-----------------+-----------------+-----------------+
+|       ...       |       ...       |       ...       |Depth\ :sub:`n-3`|Depth\ :sub:`n-2`|Depth\ :sub:`n-1`|
++-----------------+-----------------+-----------------+-----------------+-----------------+-----------------+
-- 
2.7.4

Re: [PATCH 2/2] uvcvideo: Document Intel SR300 Depth camera INZI format

[Laurent Pinchart]

Hi Evgeni,

Thank you for the patch.

On Monday 05 Dec 2016 15:24:59 evgeni.raikhel@gmail.com wrote:
> From: Evgeni Raikhel <evgeni.raikhel@intel.com>
> 
> Provide the frame structure and data layout of V4L2-PIX-FMT-INZI
> format utilized by Intel SR300 Depth camera.
> 
> This is a complimentary patch for:
> [PATCH] UVC: Add support for Intel SR300 depth camera

Once the patches will be committed this reference will be harder to use, as it 
would require searching for the other commit through the whole git history. 
Here you should instead group related changes in a single patch. The first 
patch in this series should add the new INZI format to the V4L2 API, with the 
new format definition in include/uapi/linux/videodev2.h (currently part of 
patch 1/2) and this documentation. The second patch should then add support 
for that format in the uvcvideo driver, with the changes to 
drivers/media/usb/uvc/* from patch 1/2.

Could you please split the patches that way and resubmit ? And please see 
below for additional comments.

> Signed-off-by: Evgeni Raikhel <evgeni.raikhel@intel.com>
> ---
>  Documentation/media/uapi/v4l/pixfmt-inzi.rst | 40 +++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-inzi.rst
> 
> diff --git a/Documentation/media/uapi/v4l/pixfmt-inzi.rst
> b/Documentation/media/uapi/v4l/pixfmt-inzi.rst new file mode 100644
> index 000000000000..cdfdeae4a664
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
> @@ -0,0 +1,40 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2-PIX-FMT-INZI:
> +
> +**************************
> +V4L2_PIX_FMT_INZI ('INZI')
> +**************************
> +
> +Infrared 10-bit linked with Depth 16-bit images
> +
> +
> +Description
> +===========
> +
> +Custom multi-planar format used by Intel SR300 Depth cameras, comprise of
> Infrared image followed by Depth data. +The pixel definition is 32-bpp,
> with the Depth and Infrared Data split into separate continuous planes of
> identical dimensions. +
> +The first plane - Infrared data - is stored in V4L2_PIX_FMT_Y10 (see
> :ref:`pixfmt-y10`) greyscale format. Each pixel is 16-bit cell, with actual
> data present in the 10 LSBs with values in range 0 to 1023. The six
> remaining MSBs are padded with zeros. +
> +The second plane provides 16-bit per-pixel Depth data in V4L2_PIX_FMT_Z16
> (:ref:`pixfmt-z16`) format. +

The documentation, like source code, should be limited to 80 characters per 
column. Could you please reformat the file ?

> +**Frame Structure.**
> +Each cell is a 16-bit word with the significant data byte is stored at
> lower memory address (little-endian). +
> ++-----------------+-----------------+-----------------+-----------------+--
> ---------------+-----------------+ +| Ir\ :sub:`0`    | Ir\ :sub:`1`    |
> Ir\ :sub:`2`    |       ...       |        ...      |       ...       |
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+ +|      ...       ...       ...         
>                                                                     | +|   
>                              Infrared Data                                 
>                            | +|                                            
>     ...   ...   ...                                           |
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+ +| Ir\ :sub:`n-3`  | Ir\ :sub:`n-2`  |
> Ir\ :sub:`n-1`  | Depth\ :sub:`0` | Depth\ :sub:`1` | Depth\ :sub:`2` |
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+ +|      ...       ...       ...         
>                                                                     | +|   
>                              Depth Data                                    
>                            | +|                                            
>     ...   ...   ...                                           |
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+ +|       ...       |       ...       |  
>     ...       |Depth\ :sub:`n-3`|Depth\ :sub:`n-2`|Depth\ :sub:`n-1`|
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+

This table is harder to wrap to 80 columns, but if you use the same format 
description style as the rest of the media documentation the limit shouldn't 
be an issue.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] uvcvideo: Document Intel SR300 Depth camera INZI format

[Laurent Pinchart]

Hi Evgeni,

On Monday 05 Dec 2016 15:24:59 evgeni.raikhel@gmail.com wrote:
> From: Evgeni Raikhel <evgeni.raikhel@intel.com>
> 
> Provide the frame structure and data layout of V4L2-PIX-FMT-INZI
> format utilized by Intel SR300 Depth camera.
> 
> This is a complimentary patch for:
> [PATCH] UVC: Add support for Intel SR300 depth camera
> 
> Signed-off-by: Evgeni Raikhel <evgeni.raikhel@intel.com>
> ---
>  Documentation/media/uapi/v4l/pixfmt-inzi.rst | 40 +++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-inzi.rst
> 
> diff --git a/Documentation/media/uapi/v4l/pixfmt-inzi.rst
> b/Documentation/media/uapi/v4l/pixfmt-inzi.rst new file mode 100644
> index 000000000000..cdfdeae4a664
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-inzi.rst
> @@ -0,0 +1,40 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _V4L2-PIX-FMT-INZI:
> +
> +**************************
> +V4L2_PIX_FMT_INZI ('INZI')
> +**************************
> +
> +Infrared 10-bit linked with Depth 16-bit images
> +
> +
> +Description
> +===========
> +
> +Custom multi-planar format used by Intel SR300 Depth cameras, comprise of
> Infrared image followed by Depth data. +The pixel definition is 32-bpp,
> with the Depth and Infrared Data split into separate continuous planes of
> identical dimensions. +
> +The first plane - Infrared data - is stored in V4L2_PIX_FMT_Y10 (see
> :ref:`pixfmt-y10`) greyscale format. Each pixel is 16-bit cell, with actual
> data present in the 10 LSBs with values in range 0 to 1023. The six
> remaining MSBs are padded with zeros. +
> +The second plane provides 16-bit per-pixel Depth data in V4L2_PIX_FMT_Z16
> (:ref:`pixfmt-z16`) format. +

In addition to my previous comments, wouldn't it make more sense to create a 
multiplanar format for this instead of bundling the two separate images into a 
single plane ?

> +**Frame Structure.**
> +Each cell is a 16-bit word with the significant data byte is stored at
> lower memory address (little-endian). +
> ++-----------------+-----------------+-----------------+-----------------+--
> ---------------+-----------------+ +| Ir\ :sub:`0`    | Ir\ :sub:`1`    |
> Ir\ :sub:`2`    |       ...       |        ...      |       ...       |
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+ +|      ...       ...       ...         
>                                                                     | +|   
>                              Infrared Data                                 
>                            | +|                                            
>     ...   ...   ...                                           |
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+ +| Ir\ :sub:`n-3`  | Ir\ :sub:`n-2`  |
> Ir\ :sub:`n-1`  | Depth\ :sub:`0` | Depth\ :sub:`1` | Depth\ :sub:`2` |
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+ +|      ...       ...       ...         
>                                                                     | +|   
>                              Depth Data                                    
>                            | +|                                            
>     ...   ...   ...                                           |
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+ +|       ...       |       ...       |  
>     ...       |Depth\ :sub:`n-3`|Depth\ :sub:`n-2`|Depth\ :sub:`n-1`|
> ++-----------------+-----------------+-----------------+-----------------+-
> ----------------+-----------------+

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/2] uvcvideo: Document Intel SR300 Depth camera INZI format

[Daniel Johnson]

> In addition to my previous comments, wouldn't it make more sense to create a
> multiplanar format for this instead of bundling the two separate images into a
> single plane ?

Unfortunately that would break userspace at this point as multiple
libraries are already depending on a patch that implements the INZI
format in this way. I first released a work in progress patch in March
of 2015. It was integrated into a Robot Operating System module
shortly after that, and Intel has included it in librealsense since
January, and also in the firmware for their new Joule module. Since
people using this have mostly had to patch their own kernel to use it
that might not be a deal breaker. I had initially held back on
upstreaming it because I was trying to work out some details of the
image formats. Two depth formats seemed the same, and I was trying to
figure out what was different enough about them to justify having two
formats. I've never had access to any intel documentation beyond what
is public on their website, and many details are missing.

For reference I got an early RealSense camera when only windows was
supported, and figured out some rudimentary support for Linux over a
year before intel released their own support. A manager hiring for
Intel's open source library told me over the phone that they were in
fact using my blog posts to help them develop it so it wasn't
surprising that the patch they distributed with the library included
my comments. It was surprising that they didn't mention me as the
author of the patch.

I could rebase my original patch on the current development kernel and
submit it if that helps. I can reformat the useful bits from my blog
posts as documentation on how 3d cameras work. I wrote a C hotplug
utility to let the kernel know about non standard camera controls. I
also have a partially finished kernel driver for the SR300, and F200
cameras for things like retrieving the calibration to turn depth
images into point clouds, putting the camera into firmware update
mode, etc that intel's library does with libusb. I think there should
be a standard v4l2 api for 3d cameras because as it is now userspace
programs have to be written differently for each vendor. Really all
they need are some calibration matrices, and distortion coefficients.
Precise time synchronization between /dev/videoX nodes would also be
really helpful. These two things would be helpful for other cameras as
well for things like stitching 360 degree video.

Here are links to my blog series.
http://solsticlipse.com/2015/01/09/intel-real-sense-camera-on-linux.html
http://solsticlipse.com/2015/02/10/intel-real-sense-on-linux-part-2-3d-camera-controls.html
http://solsticlipse.com/2015/03/31/intel-real-sense-3d-on-linux-macos.html
http://solsticlipse.com/2016/09/26/long-road-to-ubiquitous-3d-cameras.html