All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
@ 2015-12-15 16:46 Guennadi Liakhovetski
  2016-01-09 10:27 ` Guennadi Liakhovetski
  2016-01-13 10:24 ` Sakari Ailus
  0 siblings, 2 replies; 12+ messages in thread
From: Guennadi Liakhovetski @ 2015-12-15 16:46 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Aviv Greenberg, Hans Verkuil

Add documentation for 3 formats, used by RealSense cameras like R200.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 Documentation/DocBook/media/v4l/pixfmt-y12i.xml | 49 +++++++++++++++
 Documentation/DocBook/media/v4l/pixfmt-y8i.xml  | 80 +++++++++++++++++++++++++
 Documentation/DocBook/media/v4l/pixfmt-z16.xml  | 79 ++++++++++++++++++++++++
 Documentation/DocBook/media/v4l/pixfmt.xml      | 10 ++++
 4 files changed, 218 insertions(+)
 create mode 100644 Documentation/DocBook/media/v4l/pixfmt-y12i.xml
 create mode 100644 Documentation/DocBook/media/v4l/pixfmt-y8i.xml
 create mode 100644 Documentation/DocBook/media/v4l/pixfmt-z16.xml

diff --git a/Documentation/DocBook/media/v4l/pixfmt-y12i.xml b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
new file mode 100644
index 0000000..443598d
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
@@ -0,0 +1,49 @@
+<refentry id="V4L2-PIX-FMT-Y12I">
+  <refmeta>
+    <refentrytitle>V4L2_PIX_FMT_Y12I ('Y12I ')</refentrytitle>
+    &manvol;
+  </refmeta>
+  <refnamediv>
+    <refname><constant>V4L2_PIX_FMT_Y12I</constant></refname>
+    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
+  </refnamediv>
+  <refsect1>
+    <title>Description</title>
+
+    <para>This is a grey-scale image with a depth of 12 bits per pixel, but with
+pixels from 2 sources interleaved and bit-packed. Each pixel is stored in a
+24-bit word. E.g. data, stored by a R200 RealSense camera on a little-endian
+machine can be deinterlaced using</para>
+
+<para>
+<programlisting>
+__u8 *buf;
+left0 = 0xfff &amp; *(__u16 *)buf;
+rirhgt0 = *(__u16 *)(buf + 1) >> 4;
+</programlisting>
+</para>
+
+    <example>
+      <title><constant>V4L2_PIX_FMT_Y12I</constant> 2 pixel data stream taking 3 bytes</title>
+
+      <formalpara>
+	<title>Bit-packed representation</title>
+	<para>pixels cross the byte boundary and have a ratio of 3 bytes for each
+          interleaved pixel.
+	  <informaltable frame="all">
+	    <tgroup cols="3" align="center">
+	      <colspec align="left" colwidth="2*" />
+	      <tbody valign="top">
+		<row>
+		  <entry>Y'<subscript>0left[7:0]</subscript></entry>
+		  <entry>Y'<subscript>0right[3:0]</subscript>Y'<subscript>0left[11:8]</subscript></entry>
+		  <entry>Y'<subscript>0right[11:4]</subscript></entry>
+		</row>
+	      </tbody>
+	    </tgroup>
+	  </informaltable>
+	</para>
+      </formalpara>
+    </example>
+  </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/media/v4l/pixfmt-y8i.xml b/Documentation/DocBook/media/v4l/pixfmt-y8i.xml
new file mode 100644
index 0000000..99f389d
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/pixfmt-y8i.xml
@@ -0,0 +1,80 @@
+<refentry id="V4L2-PIX-FMT-Y8I">
+  <refmeta>
+    <refentrytitle>V4L2_PIX_FMT_Y8I ('Y8I ')</refentrytitle>
+    &manvol;
+  </refmeta>
+  <refnamediv>
+    <refname><constant>V4L2_PIX_FMT_Y8I</constant></refname>
+    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
+  </refnamediv>
+  <refsect1>
+    <title>Description</title>
+
+    <para>This is a grey-scale image with a depth of 8 bits per pixel, but with
+pixels from 2 sources interleaved. Each pixel is stored in a 16-bit word. E.g.
+the R200 RealSense camera stores pixel from the left sensor in lower and from
+the right sensor in the higher 8 bits.</para>
+
+    <example>
+      <title><constant>V4L2_PIX_FMT_Y8I</constant> 4 &times; 4
+pixel image</title>
+
+      <formalpara>
+	<title>Byte Order.</title>
+	<para>Each cell is one byte.
+	  <informaltable frame="none">
+	    <tgroup cols="9" align="center">
+	      <colspec align="left" colwidth="2*" />
+	      <tbody valign="top">
+		<row>
+		  <entry>start&nbsp;+&nbsp;0:</entry>
+		  <entry>Y'<subscript>00left</subscript></entry>
+		  <entry>Y'<subscript>00right</subscript></entry>
+		  <entry>Y'<subscript>01left</subscript></entry>
+		  <entry>Y'<subscript>01right</subscript></entry>
+		  <entry>Y'<subscript>02left</subscript></entry>
+		  <entry>Y'<subscript>02right</subscript></entry>
+		  <entry>Y'<subscript>03left</subscript></entry>
+		  <entry>Y'<subscript>03right</subscript></entry>
+		</row>
+		<row>
+		  <entry>start&nbsp;+&nbsp;8:</entry>
+		  <entry>Y'<subscript>10left</subscript></entry>
+		  <entry>Y'<subscript>10right</subscript></entry>
+		  <entry>Y'<subscript>11left</subscript></entry>
+		  <entry>Y'<subscript>11right</subscript></entry>
+		  <entry>Y'<subscript>12left</subscript></entry>
+		  <entry>Y'<subscript>12right</subscript></entry>
+		  <entry>Y'<subscript>13left</subscript></entry>
+		  <entry>Y'<subscript>13right</subscript></entry>
+		</row>
+		<row>
+		  <entry>start&nbsp;+&nbsp;16:</entry>
+		  <entry>Y'<subscript>20left</subscript></entry>
+		  <entry>Y'<subscript>20right</subscript></entry>
+		  <entry>Y'<subscript>21left</subscript></entry>
+		  <entry>Y'<subscript>21right</subscript></entry>
+		  <entry>Y'<subscript>22left</subscript></entry>
+		  <entry>Y'<subscript>22right</subscript></entry>
+		  <entry>Y'<subscript>23left</subscript></entry>
+		  <entry>Y'<subscript>23right</subscript></entry>
+		</row>
+		<row>
+		  <entry>start&nbsp;+&nbsp;24:</entry>
+		  <entry>Y'<subscript>30left</subscript></entry>
+		  <entry>Y'<subscript>30right</subscript></entry>
+		  <entry>Y'<subscript>31left</subscript></entry>
+		  <entry>Y'<subscript>31right</subscript></entry>
+		  <entry>Y'<subscript>32left</subscript></entry>
+		  <entry>Y'<subscript>32right</subscript></entry>
+		  <entry>Y'<subscript>33left</subscript></entry>
+		  <entry>Y'<subscript>33right</subscript></entry>
+		</row>
+	      </tbody>
+	    </tgroup>
+	  </informaltable>
+	</para>
+      </formalpara>
+    </example>
+  </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/media/v4l/pixfmt-z16.xml b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
new file mode 100644
index 0000000..fac3c68
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
@@ -0,0 +1,79 @@
+<refentry id="V4L2-PIX-FMT-Z16">
+  <refmeta>
+    <refentrytitle>V4L2_PIX_FMT_Z16 ('Z16 ')</refentrytitle>
+    &manvol;
+  </refmeta>
+  <refnamediv>
+    <refname><constant>V4L2_PIX_FMT_Z16</constant></refname>
+    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
+  </refnamediv>
+  <refsect1>
+    <title>Description</title>
+
+    <para>This is a 16-bit format, representing depth data. Each pixel is a
+distance in mm to the respective point in the image coordinates. Each pixel is
+stored in a 16-bit word in the little endian byte order.</para>
+
+    <example>
+      <title><constant>V4L2_PIX_FMT_Z16</constant> 4 &times; 4
+pixel image</title>
+
+      <formalpara>
+	<title>Byte Order.</title>
+	<para>Each cell is one byte.
+	  <informaltable frame="none">
+	    <tgroup cols="9" align="center">
+	      <colspec align="left" colwidth="2*" />
+	      <tbody valign="top">
+		<row>
+		  <entry>start&nbsp;+&nbsp;0:</entry>
+		  <entry>Z<subscript>00low</subscript></entry>
+		  <entry>Z<subscript>00high</subscript></entry>
+		  <entry>Z<subscript>01low</subscript></entry>
+		  <entry>Z<subscript>01high</subscript></entry>
+		  <entry>Z<subscript>02low</subscript></entry>
+		  <entry>Z<subscript>02high</subscript></entry>
+		  <entry>Z<subscript>03low</subscript></entry>
+		  <entry>Z<subscript>03high</subscript></entry>
+		</row>
+		<row>
+		  <entry>start&nbsp;+&nbsp;8:</entry>
+		  <entry>Z<subscript>10low</subscript></entry>
+		  <entry>Z<subscript>10high</subscript></entry>
+		  <entry>Z<subscript>11low</subscript></entry>
+		  <entry>Z<subscript>11high</subscript></entry>
+		  <entry>Z<subscript>12low</subscript></entry>
+		  <entry>Z<subscript>12high</subscript></entry>
+		  <entry>Z<subscript>13low</subscript></entry>
+		  <entry>Z<subscript>13high</subscript></entry>
+		</row>
+		<row>
+		  <entry>start&nbsp;+&nbsp;16:</entry>
+		  <entry>Z<subscript>20low</subscript></entry>
+		  <entry>Z<subscript>20high</subscript></entry>
+		  <entry>Z<subscript>21low</subscript></entry>
+		  <entry>Z<subscript>21high</subscript></entry>
+		  <entry>Z<subscript>22low</subscript></entry>
+		  <entry>Z<subscript>22high</subscript></entry>
+		  <entry>Z<subscript>23low</subscript></entry>
+		  <entry>Z<subscript>23high</subscript></entry>
+		</row>
+		<row>
+		  <entry>start&nbsp;+&nbsp;24:</entry>
+		  <entry>Z<subscript>30low</subscript></entry>
+		  <entry>Z<subscript>30high</subscript></entry>
+		  <entry>Z<subscript>31low</subscript></entry>
+		  <entry>Z<subscript>31high</subscript></entry>
+		  <entry>Z<subscript>32low</subscript></entry>
+		  <entry>Z<subscript>32high</subscript></entry>
+		  <entry>Z<subscript>33low</subscript></entry>
+		  <entry>Z<subscript>33high</subscript></entry>
+		</row>
+	      </tbody>
+	    </tgroup>
+	  </informaltable>
+	</para>
+      </formalpara>
+    </example>
+  </refsect1>
+</refentry>
diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml b/Documentation/DocBook/media/v4l/pixfmt.xml
index d871245..9924732 100644
--- a/Documentation/DocBook/media/v4l/pixfmt.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt.xml
@@ -1620,6 +1620,8 @@ information.</para>
     &sub-y10b;
     &sub-y16;
     &sub-y16-be;
+    &sub-y8i;
+    &sub-y12i;
     &sub-uv8;
     &sub-yuyv;
     &sub-uyvy;
@@ -1641,6 +1643,14 @@ information.</para>
     &sub-m420;
   </section>
 
+  <section id="depth-formats">
+    <title>Depth Formats</title>
+    <para>Depth data provides distance to points, mapped onto the image plane
+    </para>
+
+    &sub-z16;
+  </section>
+
   <section>
     <title>Compressed Formats</title>
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2015-12-15 16:46 [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation Guennadi Liakhovetski
@ 2016-01-09 10:27 ` Guennadi Liakhovetski
  2016-01-11 20:22   ` Daniel Johnson
  2016-01-13 10:24 ` Sakari Ailus
  1 sibling, 1 reply; 12+ messages in thread
From: Guennadi Liakhovetski @ 2016-01-09 10:27 UTC (permalink / raw)
  To: Linux Media Mailing List; +Cc: Aviv Greenberg, Hans Verkuil

Hi Mauro,

Ping - what about this patch? If there are no comments - would you like me 
to push it via my tree?

Thanks
Guennadi

On Tue, 15 Dec 2015, Guennadi Liakhovetski wrote:

> Add documentation for 3 formats, used by RealSense cameras like R200.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  Documentation/DocBook/media/v4l/pixfmt-y12i.xml | 49 +++++++++++++++
>  Documentation/DocBook/media/v4l/pixfmt-y8i.xml  | 80 +++++++++++++++++++++++++
>  Documentation/DocBook/media/v4l/pixfmt-z16.xml  | 79 ++++++++++++++++++++++++
>  Documentation/DocBook/media/v4l/pixfmt.xml      | 10 ++++
>  4 files changed, 218 insertions(+)
>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-y12i.xml
>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-y8i.xml
>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-z16.xml
> 
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-y12i.xml b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
> new file mode 100644
> index 0000000..443598d
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
> @@ -0,0 +1,49 @@
> +<refentry id="V4L2-PIX-FMT-Y12I">
> +  <refmeta>
> +    <refentrytitle>V4L2_PIX_FMT_Y12I ('Y12I ')</refentrytitle>
> +    &manvol;
> +  </refmeta>
> +  <refnamediv>
> +    <refname><constant>V4L2_PIX_FMT_Y12I</constant></refname>
> +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> +  </refnamediv>
> +  <refsect1>
> +    <title>Description</title>
> +
> +    <para>This is a grey-scale image with a depth of 12 bits per pixel, but with
> +pixels from 2 sources interleaved and bit-packed. Each pixel is stored in a
> +24-bit word. E.g. data, stored by a R200 RealSense camera on a little-endian
> +machine can be deinterlaced using</para>
> +
> +<para>
> +<programlisting>
> +__u8 *buf;
> +left0 = 0xfff &amp; *(__u16 *)buf;
> +rirhgt0 = *(__u16 *)(buf + 1) >> 4;
> +</programlisting>
> +</para>
> +
> +    <example>
> +      <title><constant>V4L2_PIX_FMT_Y12I</constant> 2 pixel data stream taking 3 bytes</title>
> +
> +      <formalpara>
> +	<title>Bit-packed representation</title>
> +	<para>pixels cross the byte boundary and have a ratio of 3 bytes for each
> +          interleaved pixel.
> +	  <informaltable frame="all">
> +	    <tgroup cols="3" align="center">
> +	      <colspec align="left" colwidth="2*" />
> +	      <tbody valign="top">
> +		<row>
> +		  <entry>Y'<subscript>0left[7:0]</subscript></entry>
> +		  <entry>Y'<subscript>0right[3:0]</subscript>Y'<subscript>0left[11:8]</subscript></entry>
> +		  <entry>Y'<subscript>0right[11:4]</subscript></entry>
> +		</row>
> +	      </tbody>
> +	    </tgroup>
> +	  </informaltable>
> +	</para>
> +      </formalpara>
> +    </example>
> +  </refsect1>
> +</refentry>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-y8i.xml b/Documentation/DocBook/media/v4l/pixfmt-y8i.xml
> new file mode 100644
> index 0000000..99f389d
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/pixfmt-y8i.xml
> @@ -0,0 +1,80 @@
> +<refentry id="V4L2-PIX-FMT-Y8I">
> +  <refmeta>
> +    <refentrytitle>V4L2_PIX_FMT_Y8I ('Y8I ')</refentrytitle>
> +    &manvol;
> +  </refmeta>
> +  <refnamediv>
> +    <refname><constant>V4L2_PIX_FMT_Y8I</constant></refname>
> +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> +  </refnamediv>
> +  <refsect1>
> +    <title>Description</title>
> +
> +    <para>This is a grey-scale image with a depth of 8 bits per pixel, but with
> +pixels from 2 sources interleaved. Each pixel is stored in a 16-bit word. E.g.
> +the R200 RealSense camera stores pixel from the left sensor in lower and from
> +the right sensor in the higher 8 bits.</para>
> +
> +    <example>
> +      <title><constant>V4L2_PIX_FMT_Y8I</constant> 4 &times; 4
> +pixel image</title>
> +
> +      <formalpara>
> +	<title>Byte Order.</title>
> +	<para>Each cell is one byte.
> +	  <informaltable frame="none">
> +	    <tgroup cols="9" align="center">
> +	      <colspec align="left" colwidth="2*" />
> +	      <tbody valign="top">
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;0:</entry>
> +		  <entry>Y'<subscript>00left</subscript></entry>
> +		  <entry>Y'<subscript>00right</subscript></entry>
> +		  <entry>Y'<subscript>01left</subscript></entry>
> +		  <entry>Y'<subscript>01right</subscript></entry>
> +		  <entry>Y'<subscript>02left</subscript></entry>
> +		  <entry>Y'<subscript>02right</subscript></entry>
> +		  <entry>Y'<subscript>03left</subscript></entry>
> +		  <entry>Y'<subscript>03right</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;8:</entry>
> +		  <entry>Y'<subscript>10left</subscript></entry>
> +		  <entry>Y'<subscript>10right</subscript></entry>
> +		  <entry>Y'<subscript>11left</subscript></entry>
> +		  <entry>Y'<subscript>11right</subscript></entry>
> +		  <entry>Y'<subscript>12left</subscript></entry>
> +		  <entry>Y'<subscript>12right</subscript></entry>
> +		  <entry>Y'<subscript>13left</subscript></entry>
> +		  <entry>Y'<subscript>13right</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;16:</entry>
> +		  <entry>Y'<subscript>20left</subscript></entry>
> +		  <entry>Y'<subscript>20right</subscript></entry>
> +		  <entry>Y'<subscript>21left</subscript></entry>
> +		  <entry>Y'<subscript>21right</subscript></entry>
> +		  <entry>Y'<subscript>22left</subscript></entry>
> +		  <entry>Y'<subscript>22right</subscript></entry>
> +		  <entry>Y'<subscript>23left</subscript></entry>
> +		  <entry>Y'<subscript>23right</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;24:</entry>
> +		  <entry>Y'<subscript>30left</subscript></entry>
> +		  <entry>Y'<subscript>30right</subscript></entry>
> +		  <entry>Y'<subscript>31left</subscript></entry>
> +		  <entry>Y'<subscript>31right</subscript></entry>
> +		  <entry>Y'<subscript>32left</subscript></entry>
> +		  <entry>Y'<subscript>32right</subscript></entry>
> +		  <entry>Y'<subscript>33left</subscript></entry>
> +		  <entry>Y'<subscript>33right</subscript></entry>
> +		</row>
> +	      </tbody>
> +	    </tgroup>
> +	  </informaltable>
> +	</para>
> +      </formalpara>
> +    </example>
> +  </refsect1>
> +</refentry>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-z16.xml b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> new file mode 100644
> index 0000000..fac3c68
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> @@ -0,0 +1,79 @@
> +<refentry id="V4L2-PIX-FMT-Z16">
> +  <refmeta>
> +    <refentrytitle>V4L2_PIX_FMT_Z16 ('Z16 ')</refentrytitle>
> +    &manvol;
> +  </refmeta>
> +  <refnamediv>
> +    <refname><constant>V4L2_PIX_FMT_Z16</constant></refname>
> +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> +  </refnamediv>
> +  <refsect1>
> +    <title>Description</title>
> +
> +    <para>This is a 16-bit format, representing depth data. Each pixel is a
> +distance in mm to the respective point in the image coordinates. Each pixel is
> +stored in a 16-bit word in the little endian byte order.</para>
> +
> +    <example>
> +      <title><constant>V4L2_PIX_FMT_Z16</constant> 4 &times; 4
> +pixel image</title>
> +
> +      <formalpara>
> +	<title>Byte Order.</title>
> +	<para>Each cell is one byte.
> +	  <informaltable frame="none">
> +	    <tgroup cols="9" align="center">
> +	      <colspec align="left" colwidth="2*" />
> +	      <tbody valign="top">
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;0:</entry>
> +		  <entry>Z<subscript>00low</subscript></entry>
> +		  <entry>Z<subscript>00high</subscript></entry>
> +		  <entry>Z<subscript>01low</subscript></entry>
> +		  <entry>Z<subscript>01high</subscript></entry>
> +		  <entry>Z<subscript>02low</subscript></entry>
> +		  <entry>Z<subscript>02high</subscript></entry>
> +		  <entry>Z<subscript>03low</subscript></entry>
> +		  <entry>Z<subscript>03high</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;8:</entry>
> +		  <entry>Z<subscript>10low</subscript></entry>
> +		  <entry>Z<subscript>10high</subscript></entry>
> +		  <entry>Z<subscript>11low</subscript></entry>
> +		  <entry>Z<subscript>11high</subscript></entry>
> +		  <entry>Z<subscript>12low</subscript></entry>
> +		  <entry>Z<subscript>12high</subscript></entry>
> +		  <entry>Z<subscript>13low</subscript></entry>
> +		  <entry>Z<subscript>13high</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;16:</entry>
> +		  <entry>Z<subscript>20low</subscript></entry>
> +		  <entry>Z<subscript>20high</subscript></entry>
> +		  <entry>Z<subscript>21low</subscript></entry>
> +		  <entry>Z<subscript>21high</subscript></entry>
> +		  <entry>Z<subscript>22low</subscript></entry>
> +		  <entry>Z<subscript>22high</subscript></entry>
> +		  <entry>Z<subscript>23low</subscript></entry>
> +		  <entry>Z<subscript>23high</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;24:</entry>
> +		  <entry>Z<subscript>30low</subscript></entry>
> +		  <entry>Z<subscript>30high</subscript></entry>
> +		  <entry>Z<subscript>31low</subscript></entry>
> +		  <entry>Z<subscript>31high</subscript></entry>
> +		  <entry>Z<subscript>32low</subscript></entry>
> +		  <entry>Z<subscript>32high</subscript></entry>
> +		  <entry>Z<subscript>33low</subscript></entry>
> +		  <entry>Z<subscript>33high</subscript></entry>
> +		</row>
> +	      </tbody>
> +	    </tgroup>
> +	  </informaltable>
> +	</para>
> +      </formalpara>
> +    </example>
> +  </refsect1>
> +</refentry>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml b/Documentation/DocBook/media/v4l/pixfmt.xml
> index d871245..9924732 100644
> --- a/Documentation/DocBook/media/v4l/pixfmt.xml
> +++ b/Documentation/DocBook/media/v4l/pixfmt.xml
> @@ -1620,6 +1620,8 @@ information.</para>
>      &sub-y10b;
>      &sub-y16;
>      &sub-y16-be;
> +    &sub-y8i;
> +    &sub-y12i;
>      &sub-uv8;
>      &sub-yuyv;
>      &sub-uyvy;
> @@ -1641,6 +1643,14 @@ information.</para>
>      &sub-m420;
>    </section>
>  
> +  <section id="depth-formats">
> +    <title>Depth Formats</title>
> +    <para>Depth data provides distance to points, mapped onto the image plane
> +    </para>
> +
> +    &sub-z16;
> +  </section>
> +
>    <section>
>      <title>Compressed Formats</title>
>  
> -- 
> 1.9.3
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2016-01-09 10:27 ` Guennadi Liakhovetski
@ 2016-01-11 20:22   ` Daniel Johnson
  2016-01-12 16:12     ` Guennadi Liakhovetski
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Johnson @ 2016-01-11 20:22 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

On Sat, Jan 9, 2016 at 2:27 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Hi Mauro,
>
> Ping - what about this patch? If there are no comments - would you like me
> to push it via my tree?

In testing the V4L2_PIX_FMT_Z16 ('Z16 ') format documentation seems to
be incomplete.

uvc_xu_control_query unit=2 selector=4 seems to be a z scale factor.
Changing the value of that control greatly changes the value of
pixels. Millimeters seems to be correct for the default value of that
control. This control is on the /dev node for the infrared camera
rather than the node using the Z16 depth format.

The one thing that every depth camera I've ever used has in common is
factory calibration. Translating pixel values to Z is only 1/3 of what
is needed to translate a depth image into a point cloud. The
calibration is needed to translate X and Y pixel indexes into
positions in 3d space. Documentation on fetching, and parsing the
factory calibration would make the camera much more usable.

Documentation on the 21 UVC controls would be helpful, but less
critical than the calibration data.

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2016-01-11 20:22   ` Daniel Johnson
@ 2016-01-12 16:12     ` Guennadi Liakhovetski
  0 siblings, 0 replies; 12+ messages in thread
From: Guennadi Liakhovetski @ 2016-01-12 16:12 UTC (permalink / raw)
  To: Daniel Johnson; +Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

Hi Daniel,

On Mon, 11 Jan 2016, Daniel Johnson wrote:

> On Sat, Jan 9, 2016 at 2:27 AM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > Hi Mauro,
> >
> > Ping - what about this patch? If there are no comments - would you like me
> > to push it via my tree?
> 
> In testing the V4L2_PIX_FMT_Z16 ('Z16 ') format documentation seems to
> be incomplete.
> 
> uvc_xu_control_query unit=2 selector=4 seems to be a z scale factor.
> Changing the value of that control greatly changes the value of
> pixels. Millimeters seems to be correct for the default value of that
> control. This control is on the /dev node for the infrared camera
> rather than the node using the Z16 depth format.

Thanks for your comments. Let me point out though, that the purpose of 
those patches isn't a complete programmers guide of those RealSense 
cameras, rather giving minimum boot-up info. Providing more information 
would require significantly more work.

Thanks
Guennadi

> The one thing that every depth camera I've ever used has in common is
> factory calibration. Translating pixel values to Z is only 1/3 of what
> is needed to translate a depth image into a point cloud. The
> calibration is needed to translate X and Y pixel indexes into
> positions in 3d space. Documentation on fetching, and parsing the
> factory calibration would make the camera much more usable.
> 
> Documentation on the 21 UVC controls would be helpful, but less
> critical than the calibration data.
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2015-12-15 16:46 [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation Guennadi Liakhovetski
  2016-01-09 10:27 ` Guennadi Liakhovetski
@ 2016-01-13 10:24 ` Sakari Ailus
  2016-01-14 11:12   ` Guennadi Liakhovetski
  2016-01-18 12:14   ` Guennadi Liakhovetski
  1 sibling, 2 replies; 12+ messages in thread
From: Sakari Ailus @ 2016-01-13 10:24 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

Hi Guennadi,

A few comments below.

On Tue, Dec 15, 2015 at 05:46:08PM +0100, Guennadi Liakhovetski wrote:
> Add documentation for 3 formats, used by RealSense cameras like R200.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  Documentation/DocBook/media/v4l/pixfmt-y12i.xml | 49 +++++++++++++++
>  Documentation/DocBook/media/v4l/pixfmt-y8i.xml  | 80 +++++++++++++++++++++++++
>  Documentation/DocBook/media/v4l/pixfmt-z16.xml  | 79 ++++++++++++++++++++++++
>  Documentation/DocBook/media/v4l/pixfmt.xml      | 10 ++++
>  4 files changed, 218 insertions(+)
>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-y12i.xml
>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-y8i.xml
>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-z16.xml
> 
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-y12i.xml b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
> new file mode 100644
> index 0000000..443598d
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
> @@ -0,0 +1,49 @@
> +<refentry id="V4L2-PIX-FMT-Y12I">
> +  <refmeta>
> +    <refentrytitle>V4L2_PIX_FMT_Y12I ('Y12I ')</refentrytitle>

Extra space after 4cc.                        ^

> +    &manvol;
> +  </refmeta>
> +  <refnamediv>
> +    <refname><constant>V4L2_PIX_FMT_Y12I</constant></refname>
> +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> +  </refnamediv>
> +  <refsect1>
> +    <title>Description</title>
> +
> +    <para>This is a grey-scale image with a depth of 12 bits per pixel, but with
> +pixels from 2 sources interleaved and bit-packed. Each pixel is stored in a
> +24-bit word. E.g. data, stored by a R200 RealSense camera on a little-endian
> +machine can be deinterlaced using</para>

I think we should precisely define the format, either big or little. Is the
endianness of the format affected by the machine endianness? (I'd guess no,
but that's just a guess.)

I wonder if the format should convey the information which one is right and
which one is left, e.g. by adding "LR" to the name.

No need to mention RealSense specifically IMO.

> +
> +<para>
> +<programlisting>
> +__u8 *buf;
> +left0 = 0xfff &amp; *(__u16 *)buf;
> +rirhgt0 = *(__u16 *)(buf + 1) >> 4;

"right"

> +</programlisting>
> +</para>
> +
> +    <example>
> +      <title><constant>V4L2_PIX_FMT_Y12I</constant> 2 pixel data stream taking 3 bytes</title>
> +
> +      <formalpara>
> +	<title>Bit-packed representation</title>
> +	<para>pixels cross the byte boundary and have a ratio of 3 bytes for each
> +          interleaved pixel.
> +	  <informaltable frame="all">
> +	    <tgroup cols="3" align="center">
> +	      <colspec align="left" colwidth="2*" />
> +	      <tbody valign="top">
> +		<row>
> +		  <entry>Y'<subscript>0left[7:0]</subscript></entry>
> +		  <entry>Y'<subscript>0right[3:0]</subscript>Y'<subscript>0left[11:8]</subscript></entry>
> +		  <entry>Y'<subscript>0right[11:4]</subscript></entry>
> +		</row>
> +	      </tbody>
> +	    </tgroup>
> +	  </informaltable>
> +	</para>
> +      </formalpara>
> +    </example>
> +  </refsect1>
> +</refentry>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-y8i.xml b/Documentation/DocBook/media/v4l/pixfmt-y8i.xml
> new file mode 100644
> index 0000000..99f389d
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/pixfmt-y8i.xml
> @@ -0,0 +1,80 @@
> +<refentry id="V4L2-PIX-FMT-Y8I">
> +  <refmeta>
> +    <refentrytitle>V4L2_PIX_FMT_Y8I ('Y8I ')</refentrytitle>
> +    &manvol;
> +  </refmeta>
> +  <refnamediv>
> +    <refname><constant>V4L2_PIX_FMT_Y8I</constant></refname>
> +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> +  </refnamediv>
> +  <refsect1>
> +    <title>Description</title>
> +
> +    <para>This is a grey-scale image with a depth of 8 bits per pixel, but with
> +pixels from 2 sources interleaved. Each pixel is stored in a 16-bit word. E.g.
> +the R200 RealSense camera stores pixel from the left sensor in lower and from
> +the right sensor in the higher 8 bits.</para>
> +
> +    <example>
> +      <title><constant>V4L2_PIX_FMT_Y8I</constant> 4 &times; 4
> +pixel image</title>
> +
> +      <formalpara>
> +	<title>Byte Order.</title>
> +	<para>Each cell is one byte.
> +	  <informaltable frame="none">
> +	    <tgroup cols="9" align="center">
> +	      <colspec align="left" colwidth="2*" />
> +	      <tbody valign="top">
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;0:</entry>
> +		  <entry>Y'<subscript>00left</subscript></entry>
> +		  <entry>Y'<subscript>00right</subscript></entry>
> +		  <entry>Y'<subscript>01left</subscript></entry>
> +		  <entry>Y'<subscript>01right</subscript></entry>
> +		  <entry>Y'<subscript>02left</subscript></entry>
> +		  <entry>Y'<subscript>02right</subscript></entry>
> +		  <entry>Y'<subscript>03left</subscript></entry>
> +		  <entry>Y'<subscript>03right</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;8:</entry>
> +		  <entry>Y'<subscript>10left</subscript></entry>
> +		  <entry>Y'<subscript>10right</subscript></entry>
> +		  <entry>Y'<subscript>11left</subscript></entry>
> +		  <entry>Y'<subscript>11right</subscript></entry>
> +		  <entry>Y'<subscript>12left</subscript></entry>
> +		  <entry>Y'<subscript>12right</subscript></entry>
> +		  <entry>Y'<subscript>13left</subscript></entry>
> +		  <entry>Y'<subscript>13right</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;16:</entry>
> +		  <entry>Y'<subscript>20left</subscript></entry>
> +		  <entry>Y'<subscript>20right</subscript></entry>
> +		  <entry>Y'<subscript>21left</subscript></entry>
> +		  <entry>Y'<subscript>21right</subscript></entry>
> +		  <entry>Y'<subscript>22left</subscript></entry>
> +		  <entry>Y'<subscript>22right</subscript></entry>
> +		  <entry>Y'<subscript>23left</subscript></entry>
> +		  <entry>Y'<subscript>23right</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;24:</entry>
> +		  <entry>Y'<subscript>30left</subscript></entry>
> +		  <entry>Y'<subscript>30right</subscript></entry>
> +		  <entry>Y'<subscript>31left</subscript></entry>
> +		  <entry>Y'<subscript>31right</subscript></entry>
> +		  <entry>Y'<subscript>32left</subscript></entry>
> +		  <entry>Y'<subscript>32right</subscript></entry>
> +		  <entry>Y'<subscript>33left</subscript></entry>
> +		  <entry>Y'<subscript>33right</subscript></entry>
> +		</row>
> +	      </tbody>
> +	    </tgroup>
> +	  </informaltable>
> +	</para>
> +      </formalpara>
> +    </example>
> +  </refsect1>
> +</refentry>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-z16.xml b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> new file mode 100644
> index 0000000..fac3c68
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> @@ -0,0 +1,79 @@
> +<refentry id="V4L2-PIX-FMT-Z16">
> +  <refmeta>
> +    <refentrytitle>V4L2_PIX_FMT_Z16 ('Z16 ')</refentrytitle>
> +    &manvol;
> +  </refmeta>
> +  <refnamediv>
> +    <refname><constant>V4L2_PIX_FMT_Z16</constant></refname>
> +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> +  </refnamediv>
> +  <refsect1>
> +    <title>Description</title>
> +
> +    <para>This is a 16-bit format, representing depth data. Each pixel is a
> +distance in mm to the respective point in the image coordinates. Each pixel is
> +stored in a 16-bit word in the little endian byte order.</para>

The format itself looks quite generic but the unit might be specific to the
device. It'd sound silly to add a new format if just the unit is different.

How about re-purpose the colourspace field for depth formats and
add a flag telling the colour space field contains the unit and the unit
prefix. Not something to have in this patch nor patchset though: controls
should gain that as well.

> +
> +    <example>
> +      <title><constant>V4L2_PIX_FMT_Z16</constant> 4 &times; 4
> +pixel image</title>
> +
> +      <formalpara>

I'm not sure there are strict rules for indenting DocBook in kernel, but
this looks a bit funny. Two is being used elsewhere, this is -1.

> +	<title>Byte Order.</title>
> +	<para>Each cell is one byte.
> +	  <informaltable frame="none">
> +	    <tgroup cols="9" align="center">
> +	      <colspec align="left" colwidth="2*" />
> +	      <tbody valign="top">
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;0:</entry>
> +		  <entry>Z<subscript>00low</subscript></entry>
> +		  <entry>Z<subscript>00high</subscript></entry>
> +		  <entry>Z<subscript>01low</subscript></entry>
> +		  <entry>Z<subscript>01high</subscript></entry>
> +		  <entry>Z<subscript>02low</subscript></entry>
> +		  <entry>Z<subscript>02high</subscript></entry>
> +		  <entry>Z<subscript>03low</subscript></entry>
> +		  <entry>Z<subscript>03high</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;8:</entry>
> +		  <entry>Z<subscript>10low</subscript></entry>
> +		  <entry>Z<subscript>10high</subscript></entry>
> +		  <entry>Z<subscript>11low</subscript></entry>
> +		  <entry>Z<subscript>11high</subscript></entry>
> +		  <entry>Z<subscript>12low</subscript></entry>
> +		  <entry>Z<subscript>12high</subscript></entry>
> +		  <entry>Z<subscript>13low</subscript></entry>
> +		  <entry>Z<subscript>13high</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;16:</entry>
> +		  <entry>Z<subscript>20low</subscript></entry>
> +		  <entry>Z<subscript>20high</subscript></entry>
> +		  <entry>Z<subscript>21low</subscript></entry>
> +		  <entry>Z<subscript>21high</subscript></entry>
> +		  <entry>Z<subscript>22low</subscript></entry>
> +		  <entry>Z<subscript>22high</subscript></entry>
> +		  <entry>Z<subscript>23low</subscript></entry>
> +		  <entry>Z<subscript>23high</subscript></entry>
> +		</row>
> +		<row>
> +		  <entry>start&nbsp;+&nbsp;24:</entry>
> +		  <entry>Z<subscript>30low</subscript></entry>
> +		  <entry>Z<subscript>30high</subscript></entry>
> +		  <entry>Z<subscript>31low</subscript></entry>
> +		  <entry>Z<subscript>31high</subscript></entry>
> +		  <entry>Z<subscript>32low</subscript></entry>
> +		  <entry>Z<subscript>32high</subscript></entry>
> +		  <entry>Z<subscript>33low</subscript></entry>
> +		  <entry>Z<subscript>33high</subscript></entry>
> +		</row>
> +	      </tbody>
> +	    </tgroup>
> +	  </informaltable>
> +	</para>
> +      </formalpara>
> +    </example>
> +  </refsect1>
> +</refentry>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt.xml b/Documentation/DocBook/media/v4l/pixfmt.xml
> index d871245..9924732 100644
> --- a/Documentation/DocBook/media/v4l/pixfmt.xml
> +++ b/Documentation/DocBook/media/v4l/pixfmt.xml
> @@ -1620,6 +1620,8 @@ information.</para>
>      &sub-y10b;
>      &sub-y16;
>      &sub-y16-be;
> +    &sub-y8i;
> +    &sub-y12i;
>      &sub-uv8;
>      &sub-yuyv;
>      &sub-uyvy;
> @@ -1641,6 +1643,14 @@ information.</para>
>      &sub-m420;
>    </section>
>  
> +  <section id="depth-formats">
> +    <title>Depth Formats</title>
> +    <para>Depth data provides distance to points, mapped onto the image plane
> +    </para>
> +
> +    &sub-z16;
> +  </section>
> +
>    <section>
>      <title>Compressed Formats</title>
>  

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2016-01-13 10:24 ` Sakari Ailus
@ 2016-01-14 11:12   ` Guennadi Liakhovetski
  2016-01-14 11:29     ` Sakari Ailus
  2016-01-18 12:14   ` Guennadi Liakhovetski
  1 sibling, 1 reply; 12+ messages in thread
From: Guennadi Liakhovetski @ 2016-01-14 11:12 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

Hi Sakari,

Thanks for a review! I'll fix all the cosmetic issues, thanks. As for 
other comments:

On Wed, 13 Jan 2016, Sakari Ailus wrote:

[snip]

> > --- /dev/null
> > +++ b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
> > @@ -0,0 +1,49 @@
> > +<refentry id="V4L2-PIX-FMT-Y12I">
> > +  <refmeta>
> > +    <refentrytitle>V4L2_PIX_FMT_Y12I ('Y12I ')</refentrytitle>
> 
> Extra space after 4cc.                        ^
> 
> > +    &manvol;
> > +  </refmeta>
> > +  <refnamediv>
> > +    <refname><constant>V4L2_PIX_FMT_Y12I</constant></refname>
> > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > +  </refnamediv>
> > +  <refsect1>
> > +    <title>Description</title>
> > +
> > +    <para>This is a grey-scale image with a depth of 12 bits per pixel, but with
> > +pixels from 2 sources interleaved and bit-packed. Each pixel is stored in a
> > +24-bit word. E.g. data, stored by a R200 RealSense camera on a little-endian
> > +machine can be deinterlaced using</para>
> 
> I think we should precisely define the format, either big or little. Is the
> endianness of the format affected by the machine endianness? (I'd guess no,
> but that's just a guess.)

Ok, since this works on a LE machine:

left0 = 0xfff & *(__u16 *)buf;

I think we can call data LE in the buffer. But specifying left-right order 
cannot be done in terms of endianness, so, I provided that code snippet.

> I wonder if the format should convey the information which one is right and
> which one is left, e.g. by adding "LR" to the name.

You mean to distinguish between LR and RL? Can do in principle, yes.

> No need to mention RealSense specifically IMO.

Ok.

> > +
> > +<para>
> > +<programlisting>
> > +__u8 *buf;
> > +left0 = 0xfff &amp; *(__u16 *)buf;
> > +rirhgt0 = *(__u16 *)(buf + 1) >> 4;
> 
> "right"

[snip]

> > diff --git a/Documentation/DocBook/media/v4l/pixfmt-z16.xml b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > new file mode 100644
> > index 0000000..fac3c68
> > --- /dev/null
> > +++ b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > @@ -0,0 +1,79 @@
> > +<refentry id="V4L2-PIX-FMT-Z16">
> > +  <refmeta>
> > +    <refentrytitle>V4L2_PIX_FMT_Z16 ('Z16 ')</refentrytitle>
> > +    &manvol;
> > +  </refmeta>
> > +  <refnamediv>
> > +    <refname><constant>V4L2_PIX_FMT_Z16</constant></refname>
> > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > +  </refnamediv>
> > +  <refsect1>
> > +    <title>Description</title>
> > +
> > +    <para>This is a 16-bit format, representing depth data. Each pixel is a
> > +distance in mm to the respective point in the image coordinates. Each pixel is
> > +stored in a 16-bit word in the little endian byte order.</para>
> 
> The format itself looks quite generic but the unit might be specific to the
> device. It'd sound silly to add a new format if just the unit is different.

My understanding is, that each format must have a fixed meaning, i.e. a 
fixed depth unit too, although it would definitely help to be able to 
relax that requirement in this case.

> How about re-purpose the colourspace field for depth formats and
> add a flag telling the colour space field contains the unit and the unit
> prefix.

Hmmm... Not sure I find this a proper use of the .colorspace field...

> Not something to have in this patch nor patchset though: controls
> should gain that as well.

Sorry, didn't get this - how can a control tell you what units a specific 
format uses? What if your camera can output depth in multiple units?

Thanks
Guennadi

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2016-01-14 11:12   ` Guennadi Liakhovetski
@ 2016-01-14 11:29     ` Sakari Ailus
  2016-01-18 11:55       ` Guennadi Liakhovetski
  0 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2016-01-14 11:29 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

Hi Guennadi,

On Thu, Jan 14, 2016 at 12:12:08PM +0100, Guennadi Liakhovetski wrote:
> Hi Sakari,
> 
> Thanks for a review! I'll fix all the cosmetic issues, thanks. As for 
> other comments:
> 
> On Wed, 13 Jan 2016, Sakari Ailus wrote:
> 
> [snip]
> 
> > > --- /dev/null
> > > +++ b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
> > > @@ -0,0 +1,49 @@
> > > +<refentry id="V4L2-PIX-FMT-Y12I">
> > > +  <refmeta>
> > > +    <refentrytitle>V4L2_PIX_FMT_Y12I ('Y12I ')</refentrytitle>
> > 
> > Extra space after 4cc.                        ^
> > 
> > > +    &manvol;
> > > +  </refmeta>
> > > +  <refnamediv>
> > > +    <refname><constant>V4L2_PIX_FMT_Y12I</constant></refname>
> > > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > > +  </refnamediv>
> > > +  <refsect1>
> > > +    <title>Description</title>
> > > +
> > > +    <para>This is a grey-scale image with a depth of 12 bits per pixel, but with
> > > +pixels from 2 sources interleaved and bit-packed. Each pixel is stored in a
> > > +24-bit word. E.g. data, stored by a R200 RealSense camera on a little-endian
> > > +machine can be deinterlaced using</para>
> > 
> > I think we should precisely define the format, either big or little. Is the
> > endianness of the format affected by the machine endianness? (I'd guess no,
> > but that's just a guess.)
> 
> Ok, since this works on a LE machine:
> 
> left0 = 0xfff & *(__u16 *)buf;
> 
> I think we can call data LE in the buffer. But specifying left-right order 
> cannot be done in terms of endianness, so, I provided that code snippet.

I meant that the the format definition should clearly say which one is the
order.

> 
> > I wonder if the format should convey the information which one is right and
> > which one is left, e.g. by adding "LR" to the name.
> 
> You mean to distinguish between LR and RL? Can do in principle, yes.

If we want the format to have an exact definition, we should have this as
well.

I think the formats increasingly have little details such as this one which
require adding many format variants but I'm not sure if it's even a problem.

I'd postfix the name with "LR" or at least document that this is the pixel
order.

>
> > No need to mention RealSense specifically IMO.
> 
> Ok.
> 
> > > +
> > > +<para>
> > > +<programlisting>
> > > +__u8 *buf;
> > > +left0 = 0xfff &amp; *(__u16 *)buf;
> > > +rirhgt0 = *(__u16 *)(buf + 1) >> 4;
> > 
> > "right"
> 
> [snip]
> 
> > > diff --git a/Documentation/DocBook/media/v4l/pixfmt-z16.xml b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > new file mode 100644
> > > index 0000000..fac3c68
> > > --- /dev/null
> > > +++ b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > @@ -0,0 +1,79 @@
> > > +<refentry id="V4L2-PIX-FMT-Z16">
> > > +  <refmeta>
> > > +    <refentrytitle>V4L2_PIX_FMT_Z16 ('Z16 ')</refentrytitle>
> > > +    &manvol;
> > > +  </refmeta>
> > > +  <refnamediv>
> > > +    <refname><constant>V4L2_PIX_FMT_Z16</constant></refname>
> > > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > > +  </refnamediv>
> > > +  <refsect1>
> > > +    <title>Description</title>
> > > +
> > > +    <para>This is a 16-bit format, representing depth data. Each pixel is a
> > > +distance in mm to the respective point in the image coordinates. Each pixel is
> > > +stored in a 16-bit word in the little endian byte order.</para>
> > 
> > The format itself looks quite generic but the unit might be specific to the
> > device. It'd sound silly to add a new format if just the unit is different.
> 
> My understanding is, that each format must have a fixed meaning, i.e. a 
> fixed depth unit too, although it would definitely help to be able to 
> relax that requirement in this case.

Agreed.

> > How about re-purpose the colourspace field for depth formats and
> > add a flag telling the colour space field contains the unit and the unit
> > prefix.
> 
> Hmmm... Not sure I find this a proper use of the .colorspace field...

I think colour space doesn't make much sense in context of depth.

> 
> > Not something to have in this patch nor patchset though: controls
> > should gain that as well.
> 
> Sorry, didn't get this - how can a control tell you what units a specific 
> format uses? What if your camera can output depth in multiple units?

Controls do not have units at the moment. The specification often suggests a
unit but using the unit suggested by the spec isn't always possible, thus
it'd be useful to have this available through VIDIOC_QUERYCTRL.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2016-01-14 11:29     ` Sakari Ailus
@ 2016-01-18 11:55       ` Guennadi Liakhovetski
  2016-01-18 12:21         ` Sakari Ailus
  0 siblings, 1 reply; 12+ messages in thread
From: Guennadi Liakhovetski @ 2016-01-18 11:55 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

On Thu, 14 Jan 2016, Sakari Ailus wrote:

> Hi Guennadi,
> 
> On Thu, Jan 14, 2016 at 12:12:08PM +0100, Guennadi Liakhovetski wrote:
> > Hi Sakari,
> > 
> > Thanks for a review! I'll fix all the cosmetic issues, thanks. As for 
> > other comments:
> > 
> > On Wed, 13 Jan 2016, Sakari Ailus wrote:
> > 
> > [snip]
> > 
> > > > --- /dev/null
> > > > +++ b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
> > > > @@ -0,0 +1,49 @@
> > > > +<refentry id="V4L2-PIX-FMT-Y12I">
> > > > +  <refmeta>
> > > > +    <refentrytitle>V4L2_PIX_FMT_Y12I ('Y12I ')</refentrytitle>
> > > 
> > > Extra space after 4cc.                        ^
> > > 
> > > > +    &manvol;
> > > > +  </refmeta>
> > > > +  <refnamediv>
> > > > +    <refname><constant>V4L2_PIX_FMT_Y12I</constant></refname>
> > > > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > > > +  </refnamediv>
> > > > +  <refsect1>
> > > > +    <title>Description</title>
> > > > +
> > > > +    <para>This is a grey-scale image with a depth of 12 bits per pixel, but with
> > > > +pixels from 2 sources interleaved and bit-packed. Each pixel is stored in a
> > > > +24-bit word. E.g. data, stored by a R200 RealSense camera on a little-endian
> > > > +machine can be deinterlaced using</para>
> > > 
> > > I think we should precisely define the format, either big or little. Is the
> > > endianness of the format affected by the machine endianness? (I'd guess no,
> > > but that's just a guess.)
> > 
> > Ok, since this works on a LE machine:
> > 
> > left0 = 0xfff & *(__u16 *)buf;
> > 
> > I think we can call data LE in the buffer. But specifying left-right order 
> > cannot be done in terms of endianness, so, I provided that code snippet.
> 
> I meant that the the format definition should clearly say which one is the
> order.
> 
> > 
> > > I wonder if the format should convey the information which one is right and
> > > which one is left, e.g. by adding "LR" to the name.
> > 
> > You mean to distinguish between LR and RL? Can do in principle, yes.
> 
> If we want the format to have an exact definition, we should have this as
> well.
> 
> I think the formats increasingly have little details such as this one which
> require adding many format variants but I'm not sure if it's even a problem.
> 
> I'd postfix the name with "LR" or at least document that this is the pixel
> order.

Don't think that's a good option ATM since the format is already in 
videodev2.h

> > > No need to mention RealSense specifically IMO.
> > 
> > Ok.
> > 
> > > > +
> > > > +<para>
> > > > +<programlisting>
> > > > +__u8 *buf;
> > > > +left0 = 0xfff &amp; *(__u16 *)buf;
> > > > +rirhgt0 = *(__u16 *)(buf + 1) >> 4;
> > > 
> > > "right"
> > 
> > [snip]
> > 
> > > > diff --git a/Documentation/DocBook/media/v4l/pixfmt-z16.xml b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > > new file mode 100644
> > > > index 0000000..fac3c68
> > > > --- /dev/null
> > > > +++ b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > > @@ -0,0 +1,79 @@
> > > > +<refentry id="V4L2-PIX-FMT-Z16">
> > > > +  <refmeta>
> > > > +    <refentrytitle>V4L2_PIX_FMT_Z16 ('Z16 ')</refentrytitle>
> > > > +    &manvol;
> > > > +  </refmeta>
> > > > +  <refnamediv>
> > > > +    <refname><constant>V4L2_PIX_FMT_Z16</constant></refname>
> > > > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > > > +  </refnamediv>
> > > > +  <refsect1>
> > > > +    <title>Description</title>
> > > > +
> > > > +    <para>This is a 16-bit format, representing depth data. Each pixel is a
> > > > +distance in mm to the respective point in the image coordinates. Each pixel is
> > > > +stored in a 16-bit word in the little endian byte order.</para>
> > > 
> > > The format itself looks quite generic but the unit might be specific to the
> > > device. It'd sound silly to add a new format if just the unit is different.
> > 
> > My understanding is, that each format must have a fixed meaning, i.e. a 
> > fixed depth unit too, although it would definitely help to be able to 
> > relax that requirement in this case.
> 
> Agreed.
> 
> > > How about re-purpose the colourspace field for depth formats and
> > > add a flag telling the colour space field contains the unit and the unit
> > > prefix.
> > 
> > Hmmm... Not sure I find this a proper use of the .colorspace field...
> 
> I think colour space doesn't make much sense in context of depth.

Agree, still I don't think it is a good idea to abuse it for a different 
purpose. If it doesn't make sense it simply shouldn't be used.

Thanks
Guennadi

> > > Not something to have in this patch nor patchset though: controls
> > > should gain that as well.
> > 
> > Sorry, didn't get this - how can a control tell you what units a specific 
> > format uses? What if your camera can output depth in multiple units?
> 
> Controls do not have units at the moment. The specification often suggests a
> unit but using the unit suggested by the spec isn't always possible, thus
> it'd be useful to have this available through VIDIOC_QUERYCTRL.
> 
> -- 
> Kind regards,
> 
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2016-01-13 10:24 ` Sakari Ailus
  2016-01-14 11:12   ` Guennadi Liakhovetski
@ 2016-01-18 12:14   ` Guennadi Liakhovetski
  1 sibling, 0 replies; 12+ messages in thread
From: Guennadi Liakhovetski @ 2016-01-18 12:14 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

Hi Sakari,

to one of your earlier comments:

On Wed, 13 Jan 2016, Sakari Ailus wrote:

[snip]

> > +
> > +    <example>
> > +      <title><constant>V4L2_PIX_FMT_Z16</constant> 4 &times; 4
> > +pixel image</title>
> > +
> > +      <formalpara>
> 
> I'm not sure there are strict rules for indenting DocBook in kernel, but
> this looks a bit funny. Two is being used elsewhere, this is -1.

That's not two instead of -1. That's a TAB instead of 8 spaces, which is 
the same as in other pixel format DocBook files.

Thanks
Guennadi

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2016-01-18 11:55       ` Guennadi Liakhovetski
@ 2016-01-18 12:21         ` Sakari Ailus
  2016-01-18 12:36           ` Guennadi Liakhovetski
  0 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2016-01-18 12:21 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

Hi Guennadi,

On Mon, Jan 18, 2016 at 12:55:20PM +0100, Guennadi Liakhovetski wrote:
> On Thu, 14 Jan 2016, Sakari Ailus wrote:
> 
> > Hi Guennadi,
> > 
> > On Thu, Jan 14, 2016 at 12:12:08PM +0100, Guennadi Liakhovetski wrote:
> > > Hi Sakari,
> > > 
> > > Thanks for a review! I'll fix all the cosmetic issues, thanks. As for 
> > > other comments:
> > > 
> > > On Wed, 13 Jan 2016, Sakari Ailus wrote:
> > > 
> > > [snip]
> > > 
> > > > > --- /dev/null
> > > > > +++ b/Documentation/DocBook/media/v4l/pixfmt-y12i.xml
> > > > > @@ -0,0 +1,49 @@
> > > > > +<refentry id="V4L2-PIX-FMT-Y12I">
> > > > > +  <refmeta>
> > > > > +    <refentrytitle>V4L2_PIX_FMT_Y12I ('Y12I ')</refentrytitle>
> > > > 
> > > > Extra space after 4cc.                        ^
> > > > 
> > > > > +    &manvol;
> > > > > +  </refmeta>
> > > > > +  <refnamediv>
> > > > > +    <refname><constant>V4L2_PIX_FMT_Y12I</constant></refname>
> > > > > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > > > > +  </refnamediv>
> > > > > +  <refsect1>
> > > > > +    <title>Description</title>
> > > > > +
> > > > > +    <para>This is a grey-scale image with a depth of 12 bits per pixel, but with
> > > > > +pixels from 2 sources interleaved and bit-packed. Each pixel is stored in a
> > > > > +24-bit word. E.g. data, stored by a R200 RealSense camera on a little-endian
> > > > > +machine can be deinterlaced using</para>
> > > > 
> > > > I think we should precisely define the format, either big or little. Is the
> > > > endianness of the format affected by the machine endianness? (I'd guess no,
> > > > but that's just a guess.)
> > > 
> > > Ok, since this works on a LE machine:
> > > 
> > > left0 = 0xfff & *(__u16 *)buf;
> > > 
> > > I think we can call data LE in the buffer. But specifying left-right order 
> > > cannot be done in terms of endianness, so, I provided that code snippet.
> > 
> > I meant that the the format definition should clearly say which one is the
> > order.
> > 
> > > 
> > > > I wonder if the format should convey the information which one is right and
> > > > which one is left, e.g. by adding "LR" to the name.
> > > 
> > > You mean to distinguish between LR and RL? Can do in principle, yes.
> > 
> > If we want the format to have an exact definition, we should have this as
> > well.
> > 
> > I think the formats increasingly have little details such as this one which
> > require adding many format variants but I'm not sure if it's even a problem.
> > 
> > I'd postfix the name with "LR" or at least document that this is the pixel
> > order.
> 
> Don't think that's a good option ATM since the format is already in 
> videodev2.h

Is it? I can't see it in my tree at least.

14:16:48 vihersipuli sailus [~/scratch/git/linux]git grep -c V4L2_PIX_FMT_Y12I nclude/uapi/linux/videodev2.h
14:16:50 vihersipuli sailus [~/scratch/git/linux]

> 
> > > > No need to mention RealSense specifically IMO.
> > > 
> > > Ok.
> > > 
> > > > > +
> > > > > +<para>
> > > > > +<programlisting>
> > > > > +__u8 *buf;
> > > > > +left0 = 0xfff &amp; *(__u16 *)buf;
> > > > > +rirhgt0 = *(__u16 *)(buf + 1) >> 4;
> > > > 
> > > > "right"
> > > 
> > > [snip]
> > > 
> > > > > diff --git a/Documentation/DocBook/media/v4l/pixfmt-z16.xml b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > > > new file mode 100644
> > > > > index 0000000..fac3c68
> > > > > --- /dev/null
> > > > > +++ b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > > > @@ -0,0 +1,79 @@
> > > > > +<refentry id="V4L2-PIX-FMT-Z16">
> > > > > +  <refmeta>
> > > > > +    <refentrytitle>V4L2_PIX_FMT_Z16 ('Z16 ')</refentrytitle>
> > > > > +    &manvol;
> > > > > +  </refmeta>
> > > > > +  <refnamediv>
> > > > > +    <refname><constant>V4L2_PIX_FMT_Z16</constant></refname>
> > > > > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > > > > +  </refnamediv>
> > > > > +  <refsect1>
> > > > > +    <title>Description</title>
> > > > > +
> > > > > +    <para>This is a 16-bit format, representing depth data. Each pixel is a
> > > > > +distance in mm to the respective point in the image coordinates. Each pixel is
> > > > > +stored in a 16-bit word in the little endian byte order.</para>
> > > > 
> > > > The format itself looks quite generic but the unit might be specific to the
> > > > device. It'd sound silly to add a new format if just the unit is different.
> > > 
> > > My understanding is, that each format must have a fixed meaning, i.e. a 
> > > fixed depth unit too, although it would definitely help to be able to 
> > > relax that requirement in this case.
> > 
> > Agreed.
> > 
> > > > How about re-purpose the colourspace field for depth formats and
> > > > add a flag telling the colour space field contains the unit and the unit
> > > > prefix.
> > > 
> > > Hmmm... Not sure I find this a proper use of the .colorspace field...
> > 
> > I think colour space doesn't make much sense in context of depth.
> 
> Agree, still I don't think it is a good idea to abuse it for a different 
> purpose. If it doesn't make sense it simply shouldn't be used.

We are already using anonymous unions for this exact purpose already, albeit
their use was planned in most cases at least. I don't see anything wrong
with this, considering that existing applications dealing with the format
wouldn't know what to do about it anyway.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2016-01-18 12:21         ` Sakari Ailus
@ 2016-01-18 12:36           ` Guennadi Liakhovetski
  2016-01-18 14:37             ` Sakari Ailus
  0 siblings, 1 reply; 12+ messages in thread
From: Guennadi Liakhovetski @ 2016-01-18 12:36 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

On Mon, 18 Jan 2016, Sakari Ailus wrote:

[snip]

> > > > > I wonder if the format should convey the information which one is right and
> > > > > which one is left, e.g. by adding "LR" to the name.
> > > > 
> > > > You mean to distinguish between LR and RL? Can do in principle, yes.
> > > 
> > > If we want the format to have an exact definition, we should have this as
> > > well.
> > > 
> > > I think the formats increasingly have little details such as this one which
> > > require adding many format variants but I'm not sure if it's even a problem.
> > > 
> > > I'd postfix the name with "LR" or at least document that this is the pixel
> > > order.
> > 
> > Don't think that's a good option ATM since the format is already in 
> > videodev2.h
> 
> Is it? I can't see it in my tree at least.

It is, and you signed off under it and submitted it;-)

https://patchwork.linuxtv.org/patch/31690/

> 14:16:48 vihersipuli sailus [~/scratch/git/linux]git grep -c V4L2_PIX_FMT_Y12I nclude/uapi/linux/videodev2.h
> 14:16:50 vihersipuli sailus [~/scratch/git/linux]
> 
> > 
> > > > > No need to mention RealSense specifically IMO.
> > > > 
> > > > Ok.
> > > > 
> > > > > > +
> > > > > > +<para>
> > > > > > +<programlisting>
> > > > > > +__u8 *buf;
> > > > > > +left0 = 0xfff &amp; *(__u16 *)buf;
> > > > > > +rirhgt0 = *(__u16 *)(buf + 1) >> 4;
> > > > > 
> > > > > "right"
> > > > 
> > > > [snip]
> > > > 
> > > > > > diff --git a/Documentation/DocBook/media/v4l/pixfmt-z16.xml b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > > > > new file mode 100644
> > > > > > index 0000000..fac3c68
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > > > > @@ -0,0 +1,79 @@
> > > > > > +<refentry id="V4L2-PIX-FMT-Z16">
> > > > > > +  <refmeta>
> > > > > > +    <refentrytitle>V4L2_PIX_FMT_Z16 ('Z16 ')</refentrytitle>
> > > > > > +    &manvol;
> > > > > > +  </refmeta>
> > > > > > +  <refnamediv>
> > > > > > +    <refname><constant>V4L2_PIX_FMT_Z16</constant></refname>
> > > > > > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > > > > > +  </refnamediv>
> > > > > > +  <refsect1>
> > > > > > +    <title>Description</title>
> > > > > > +
> > > > > > +    <para>This is a 16-bit format, representing depth data. Each pixel is a
> > > > > > +distance in mm to the respective point in the image coordinates. Each pixel is
> > > > > > +stored in a 16-bit word in the little endian byte order.</para>
> > > > > 
> > > > > The format itself looks quite generic but the unit might be specific to the
> > > > > device. It'd sound silly to add a new format if just the unit is different.
> > > > 
> > > > My understanding is, that each format must have a fixed meaning, i.e. a 
> > > > fixed depth unit too, although it would definitely help to be able to 
> > > > relax that requirement in this case.
> > > 
> > > Agreed.
> > > 
> > > > > How about re-purpose the colourspace field for depth formats and
> > > > > add a flag telling the colour space field contains the unit and the unit
> > > > > prefix.
> > > > 
> > > > Hmmm... Not sure I find this a proper use of the .colorspace field...
> > > 
> > > I think colour space doesn't make much sense in context of depth.
> > 
> > Agree, still I don't think it is a good idea to abuse it for a different 
> > purpose. If it doesn't make sense it simply shouldn't be used.
> 
> We are already using anonymous unions for this exact purpose already, albeit
> their use was planned in most cases at least. I don't see anything wrong
> with this, considering that existing applications dealing with the format
> wouldn't know what to do about it anyway.

Sure, I understand that it can be done using an anonymous union. I just 
don't want to specify this in this patch , it should be a separate change, 
I think.

Thanks
Guennadi

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation
  2016-01-18 12:36           ` Guennadi Liakhovetski
@ 2016-01-18 14:37             ` Sakari Ailus
  0 siblings, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2016-01-18 14:37 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Aviv Greenberg, Hans Verkuil

Hi Guennadi,

On Mon, Jan 18, 2016 at 01:36:49PM +0100, Guennadi Liakhovetski wrote:
> On Mon, 18 Jan 2016, Sakari Ailus wrote:
> 
> [snip]
> 
> > > > > > I wonder if the format should convey the information which one is right and
> > > > > > which one is left, e.g. by adding "LR" to the name.
> > > > > 
> > > > > You mean to distinguish between LR and RL? Can do in principle, yes.
> > > > 
> > > > If we want the format to have an exact definition, we should have this as
> > > > well.
> > > > 
> > > > I think the formats increasingly have little details such as this one which
> > > > require adding many format variants but I'm not sure if it's even a problem.
> > > > 
> > > > I'd postfix the name with "LR" or at least document that this is the pixel
> > > > order.
> > > 
> > > Don't think that's a good option ATM since the format is already in 
> > > videodev2.h
> > 
> > Is it? I can't see it in my tree at least.
> 
> It is, and you signed off under it and submitted it;-)
> 
> https://patchwork.linuxtv.org/patch/31690/

I think you're missing this one:

commit 52d60eb7e6d6429a766ea1b8f67e01c3b2dcd3c5
Author: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Date:   Sat Dec 12 08:10:40 2015 -0200

    Revert "[media] UVC: Add support for ds4 depth camera"
    
    This reverts commit 120c41d3477a23c6941059401db63677736f1935.
    
    The patch doesn't add the corresponding documentation bits to the
    media infrastructure uAPI DocBook. Also, they're for 3D formats,
    with requre further discussions.
    
    Requested-by: Hans Verkuil <hverkuil@xs4all.nl>
    Requested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> 
> > 14:16:48 vihersipuli sailus [~/scratch/git/linux]git grep -c V4L2_PIX_FMT_Y12I nclude/uapi/linux/videodev2.h
> > 14:16:50 vihersipuli sailus [~/scratch/git/linux]
> > 
> > > 
> > > > > > No need to mention RealSense specifically IMO.
> > > > > 
> > > > > Ok.
> > > > > 
> > > > > > > +
> > > > > > > +<para>
> > > > > > > +<programlisting>
> > > > > > > +__u8 *buf;
> > > > > > > +left0 = 0xfff &amp; *(__u16 *)buf;
> > > > > > > +rirhgt0 = *(__u16 *)(buf + 1) >> 4;
> > > > > > 
> > > > > > "right"
> > > > > 
> > > > > [snip]
> > > > > 
> > > > > > > diff --git a/Documentation/DocBook/media/v4l/pixfmt-z16.xml b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > > > > > new file mode 100644
> > > > > > > index 0000000..fac3c68
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/DocBook/media/v4l/pixfmt-z16.xml
> > > > > > > @@ -0,0 +1,79 @@
> > > > > > > +<refentry id="V4L2-PIX-FMT-Z16">
> > > > > > > +  <refmeta>
> > > > > > > +    <refentrytitle>V4L2_PIX_FMT_Z16 ('Z16 ')</refentrytitle>
> > > > > > > +    &manvol;
> > > > > > > +  </refmeta>
> > > > > > > +  <refnamediv>
> > > > > > > +    <refname><constant>V4L2_PIX_FMT_Z16</constant></refname>
> > > > > > > +    <refpurpose>Interleaved grey-scale image, e.g. from a stereo-pair</refpurpose>
> > > > > > > +  </refnamediv>
> > > > > > > +  <refsect1>
> > > > > > > +    <title>Description</title>
> > > > > > > +
> > > > > > > +    <para>This is a 16-bit format, representing depth data. Each pixel is a
> > > > > > > +distance in mm to the respective point in the image coordinates. Each pixel is
> > > > > > > +stored in a 16-bit word in the little endian byte order.</para>
> > > > > > 
> > > > > > The format itself looks quite generic but the unit might be specific to the
> > > > > > device. It'd sound silly to add a new format if just the unit is different.
> > > > > 
> > > > > My understanding is, that each format must have a fixed meaning, i.e. a 
> > > > > fixed depth unit too, although it would definitely help to be able to 
> > > > > relax that requirement in this case.
> > > > 
> > > > Agreed.
> > > > 
> > > > > > How about re-purpose the colourspace field for depth formats and
> > > > > > add a flag telling the colour space field contains the unit and the unit
> > > > > > prefix.
> > > > > 
> > > > > Hmmm... Not sure I find this a proper use of the .colorspace field...
> > > > 
> > > > I think colour space doesn't make much sense in context of depth.
> > > 
> > > Agree, still I don't think it is a good idea to abuse it for a different 
> > > purpose. If it doesn't make sense it simply shouldn't be used.
> > 
> > We are already using anonymous unions for this exact purpose already, albeit
> > their use was planned in most cases at least. I don't see anything wrong
> > with this, considering that existing applications dealing with the format
> > wouldn't know what to do about it anyway.
> 
> Sure, I understand that it can be done using an anonymous union. I just 
> don't want to specify this in this patch , it should be a separate change, 
> I think.

Ack.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-01-18 14:37 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-15 16:46 [PATCH] V4L: add Y12I, Y8I and Z16 pixel format documentation Guennadi Liakhovetski
2016-01-09 10:27 ` Guennadi Liakhovetski
2016-01-11 20:22   ` Daniel Johnson
2016-01-12 16:12     ` Guennadi Liakhovetski
2016-01-13 10:24 ` Sakari Ailus
2016-01-14 11:12   ` Guennadi Liakhovetski
2016-01-14 11:29     ` Sakari Ailus
2016-01-18 11:55       ` Guennadi Liakhovetski
2016-01-18 12:21         ` Sakari Ailus
2016-01-18 12:36           ` Guennadi Liakhovetski
2016-01-18 14:37             ` Sakari Ailus
2016-01-18 12:14   ` Guennadi Liakhovetski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.