[PATCH 0/3] Add per-device-node capabilities

[PATCH 0/3] Add per-device-node capabilities

[Hans Verkuil]

Hi Mauro,

This patch series adds support for per-device capabilities.

All comments from the previous RFC PATCH series have been incorporated and
the documentation has been updated.

As far as I am concerned it is ready to be merged for v3.3.

The git request-pull output is below:

The following changes since commit 6fd7dba026f17076ac4bd63a3590f993c1f5c2c6:

[media] get_dvb_firmware: add support for HVR-930C firmware (2011-11-21 10:38:39 -0200)

are available in the git repository at:
git://linuxtv.org/hverkuil/media_tree.git capsv2

Hans Verkuil (3):
	V4L2: Add per-device-node capabilities
	vivi: set device_caps.
	ivtv: setup per-device caps.

	Documentation/DocBook/media/v4l/compat.xml         |    9 ++++++
	Documentation/DocBook/media/v4l/v4l2.xml           |    9 +++++-
	.../DocBook/media/v4l/vidioc-querycap.xml          |   29 +++++++++++++++++--
	drivers/media/video/cx231xx/cx231xx-417.c          |    1 -
	drivers/media/video/ivtv/ivtv-driver.h             |    1 +
	drivers/media/video/ivtv/ivtv-ioctl.c              |    7 +++-
	drivers/media/video/ivtv/ivtv-streams.c            |   14 +++++++++
	drivers/media/video/pvrusb2/pvrusb2-v4l2.c         |    1 -
	drivers/media/video/v4l2-ioctl.c                   |    6 +++-
	drivers/media/video/vivi.c                         |    5 ++-
	include/linux/videodev2.h                          |   29 +++++++++++++------
11 files changed, 90 insertions(+), 21 deletions(-)

[PATCH 1/3] V4L2: Add per-device-node capabilities

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

If V4L2_CAP_DEVICE_CAPS is set, then the new device_caps field is filled with
the capabilities of the opened device node.

The capabilities field traditionally contains the capabilities of the whole
device. E.g., if you open video0, then if it contains VBI caps then that means
that there is a corresponding vbi node as well. And the capabilities field of
both the video and vbi node should contain identical caps.

However, it would be very useful to also have a capabilities field that contains
just the caps for the currently open device, hence the new CAP bit and field.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/DocBook/media/v4l/compat.xml         |    9 ++++++
 Documentation/DocBook/media/v4l/v4l2.xml           |    9 +++++-
 .../DocBook/media/v4l/vidioc-querycap.xml          |   29 +++++++++++++++++--
 drivers/media/video/cx231xx/cx231xx-417.c          |    1 -
 drivers/media/video/pvrusb2/pvrusb2-v4l2.c         |    1 -
 drivers/media/video/v4l2-ioctl.c                   |    6 +++-
 include/linux/videodev2.h                          |   29 +++++++++++++------
 7 files changed, 67 insertions(+), 17 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
index b68698f..88ea4fc 100644
--- a/Documentation/DocBook/media/v4l/compat.xml
+++ b/Documentation/DocBook/media/v4l/compat.xml
@@ -2378,6 +2378,15 @@ that used it. It was originally scheduled for removal in 2.6.35.
         </listitem>
       </orderedlist>
     </section>
+    <section>
+      <title>V4L2 in Linux 3.3</title>
+      <orderedlist>
+        <listitem>
+	  <para>Added the device_caps field to struct v4l2_capabilities and added the new
+	  V4L2_CAP_DEVICE_CAPS capability.</para>
+        </listitem>
+      </orderedlist>
+    </section>
 
     <section id="other">
       <title>Relation of V4L2 to other Linux multimedia APIs</title>
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 2ab365c..6b6e584 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -128,6 +128,13 @@ structs, ioctls) must be noted in more detail in the history chapter
 applications. -->
 
       <revision>
+	<revnumber>3.3</revnumber>
+	<date>2011-11-22</date>
+	<authorinitials>hv</authorinitials>
+	<revremark>Added device_caps field to struct v4l2_capabilities.</revremark>
+      </revision>
+
+      <revision>
 	<revnumber>3.2</revnumber>
 	<date>2011-08-26</date>
 	<authorinitials>hv</authorinitials>
@@ -417,7 +424,7 @@ and discussions on the V4L mailing list.</revremark>
 </partinfo>
 
 <title>Video for Linux Two API Specification</title>
- <subtitle>Revision 3.2</subtitle>
+ <subtitle>Revision 3.3</subtitle>
 
   <chapter id="common">
     &sub-common;
diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
index e3664d6..632ad13 100644
--- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
@@ -124,12 +124,29 @@ printf ("Version: %u.%u.%u\n",
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>capabilities</structfield></entry>
-	    <entry>Device capabilities, see <xref
-		linkend="device-capabilities" />.</entry>
+	    <entry>Device capabilities of the physical device as a whole, see <xref
+		linkend="device-capabilities" />. The same physical device can export
+		multiple devices in /dev (e.g. /dev/videoX, /dev/vbiY and /dev/radioZ).
+		For all those devices the capabilities field returns the same set of
+		capabilities. This allows applications to open just the video device
+		and discover whether vbi or radio is also supported.
+	    </entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>reserved</structfield>[4]</entry>
+	    <entry><structfield>device_caps</structfield></entry>
+	    <entry>Device capabilities of the open device, see <xref
+		linkend="device-capabilities" />. This is the set of capabilities
+		of just the open device. So <structfield>device_caps</structfield>
+		of a radio device will only contain radio related capabilities and
+		no video capabilities. This field is only set if the <structfield>capabilities</structfield>
+		field contains the <constant>V4L2_CAP_DEVICE_CAPS</constant>
+		capability.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[3]</entry>
 	    <entry>Reserved for future extensions. Drivers must set
 this array to zero.</entry>
 	  </row>
@@ -276,6 +293,12 @@ linkend="async">asynchronous</link> I/O methods.</entry>
 	    <entry>The device supports the <link
 linkend="mmap">streaming</link> I/O method.</entry>
 	  </row>
+	  <row>
+	    <entry><constant>V4L2_CAP_DEVICE_CAPS</constant></entry>
+	    <entry>0x80000000</entry>
+	    <entry>The driver fills the <structfield>device_caps</structfield>
+	    field.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/drivers/media/video/cx231xx/cx231xx-417.c b/drivers/media/video/cx231xx/cx231xx-417.c
index f8f0e59..d4327da 100644
--- a/drivers/media/video/cx231xx/cx231xx-417.c
+++ b/drivers/media/video/cx231xx/cx231xx-417.c
@@ -1686,7 +1686,6 @@ static struct v4l2_capability pvr_capability = {
 	.capabilities   = (V4L2_CAP_VIDEO_CAPTURE |
 			   V4L2_CAP_TUNER | V4L2_CAP_AUDIO | V4L2_CAP_RADIO |
 			 V4L2_CAP_STREAMING | V4L2_CAP_READWRITE),
-	.reserved       = {0, 0, 0, 0}
 };
 static int vidioc_querycap(struct file *file, void  *priv,
 				struct v4l2_capability *cap)
diff --git a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
index ce7ac45..e98b414 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
@@ -95,7 +95,6 @@ static struct v4l2_capability pvr_capability ={
 	.capabilities   = (V4L2_CAP_VIDEO_CAPTURE |
 			   V4L2_CAP_TUNER | V4L2_CAP_AUDIO | V4L2_CAP_RADIO |
 			   V4L2_CAP_READWRITE),
-	.reserved       = {0,0,0,0}
 };
 
 static struct v4l2_fmtdesc pvr_fmtdesc [] = {
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index e1da8fc..ae716c2 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -538,10 +538,12 @@ static long __video_do_ioctl(struct file *file,
 		if (!ret)
 			dbgarg(cmd, "driver=%s, card=%s, bus=%s, "
 					"version=0x%08x, "
-					"capabilities=0x%08x\n",
+					"capabilities=0x%08x, "
+					"device_caps=0x%08x\n",
 					cap->driver, cap->card, cap->bus_info,
 					cap->version,
-					cap->capabilities);
+					cap->capabilities,
+					cap->device_caps);
 		break;
 	}
 
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 4b752d5..c185707 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -235,16 +235,25 @@ struct v4l2_fract {
 	__u32   denominator;
 };
 
-/*
- *	D R I V E R   C A P A B I L I T I E S
- */
+/**
+  * struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
+  *
+  * @driver:	   name of the driver module (e.g. "bttv")
+  * @card:	   name of the card (e.g. "Hauppauge WinTV")
+  * @bus_info:	   name of the bus (e.g. "PCI:" + pci_name(pci_dev) )
+  * @version:	   KERNEL_VERSION
+  * @capabilities: capabilities of the physical device as a whole
+  * @device_caps:  capabilities accessed via this particular device (node)
+  * @reserved:	   reserved fields for future extensions
+  */
 struct v4l2_capability {
-	__u8	driver[16];	/* i.e. "bttv" */
-	__u8	card[32];	/* i.e. "Hauppauge WinTV" */
-	__u8	bus_info[32];	/* "PCI:" + pci_name(pci_dev) */
-	__u32   version;        /* should use KERNEL_VERSION() */
-	__u32	capabilities;	/* Device capabilities */
-	__u32	reserved[4];
+	__u8	driver[16];
+	__u8	card[32];
+	__u8	bus_info[32];
+	__u32   version;
+	__u32	capabilities;
+	__u32	device_caps;
+	__u32	reserved[3];
 };
 
 /* Values for 'capabilities' field */
@@ -274,6 +283,8 @@ struct v4l2_capability {
 #define V4L2_CAP_ASYNCIO                0x02000000  /* async I/O */
 #define V4L2_CAP_STREAMING              0x04000000  /* streaming I/O ioctls */
 
+#define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
+
 /*
  *	V I D E O   I M A G E   F O R M A T
  */
-- 
1.7.7.3

[PATCH 2/3] vivi: set device_caps.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/vivi.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
index 7d754fb..84ea88d 100644
--- a/drivers/media/video/vivi.c
+++ b/drivers/media/video/vivi.c
@@ -819,8 +819,9 @@ static int vidioc_querycap(struct file *file, void  *priv,
 	strcpy(cap->driver, "vivi");
 	strcpy(cap->card, "vivi");
 	strlcpy(cap->bus_info, dev->v4l2_dev.name, sizeof(cap->bus_info));
-	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING | \
-			    V4L2_CAP_READWRITE;
+	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
+			    V4L2_CAP_READWRITE | V4L2_CAP_DEVICE_CAPS;
+	cap->device_caps = cap->capabilities;
 	return 0;
 }
 
-- 
1.7.7.3

[PATCH 3/3] ivtv: setup per-device caps.

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/video/ivtv/ivtv-driver.h  |    1 +
 drivers/media/video/ivtv/ivtv-ioctl.c   |    7 +++++--
 drivers/media/video/ivtv/ivtv-streams.c |   14 ++++++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/ivtv/ivtv-driver.h b/drivers/media/video/ivtv/ivtv-driver.h
index 8f9cc17..06b9efd 100644
--- a/drivers/media/video/ivtv/ivtv-driver.h
+++ b/drivers/media/video/ivtv/ivtv-driver.h
@@ -331,6 +331,7 @@ struct ivtv_stream {
 	struct ivtv *itv; 		/* for ease of use */
 	const char *name;		/* name of the stream */
 	int type;			/* stream type */
+	u32 caps;			/* V4L2 capabilities */
 
 	u32 id;
 	spinlock_t qlock; 		/* locks access to the queues */
diff --git a/drivers/media/video/ivtv/ivtv-ioctl.c b/drivers/media/video/ivtv/ivtv-ioctl.c
index ecafa69..6be63e9 100644
--- a/drivers/media/video/ivtv/ivtv-ioctl.c
+++ b/drivers/media/video/ivtv/ivtv-ioctl.c
@@ -752,12 +752,15 @@ static int ivtv_s_register(struct file *file, void *fh, struct v4l2_dbg_register
 
 static int ivtv_querycap(struct file *file, void *fh, struct v4l2_capability *vcap)
 {
-	struct ivtv *itv = fh2id(fh)->itv;
+	struct ivtv_open_id *id = fh2id(file->private_data);
+	struct ivtv *itv = id->itv;
+	struct ivtv_stream *s = &itv->streams[id->type];
 
 	strlcpy(vcap->driver, IVTV_DRIVER_NAME, sizeof(vcap->driver));
 	strlcpy(vcap->card, itv->card_name, sizeof(vcap->card));
 	snprintf(vcap->bus_info, sizeof(vcap->bus_info), "PCI:%s", pci_name(itv->pdev));
-	vcap->capabilities = itv->v4l2_cap; 	    /* capabilities */
+	vcap->capabilities = itv->v4l2_cap | V4L2_CAP_DEVICE_CAPS;
+	vcap->device_caps = s->caps;
 	return 0;
 }
 
diff --git a/drivers/media/video/ivtv/ivtv-streams.c b/drivers/media/video/ivtv/ivtv-streams.c
index e7794dc..4d4ae6e 100644
--- a/drivers/media/video/ivtv/ivtv-streams.c
+++ b/drivers/media/video/ivtv/ivtv-streams.c
@@ -78,60 +78,73 @@ static struct {
 	int num_offset;
 	int dma, pio;
 	enum v4l2_buf_type buf_type;
+	u32 v4l2_caps;
 	const struct v4l2_file_operations *fops;
 } ivtv_stream_info[] = {
 	{	/* IVTV_ENC_STREAM_TYPE_MPG */
 		"encoder MPG",
 		VFL_TYPE_GRABBER, 0,
 		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VIDEO_CAPTURE,
+		V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TUNER |
+			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
 		&ivtv_v4l2_enc_fops
 	},
 	{	/* IVTV_ENC_STREAM_TYPE_YUV */
 		"encoder YUV",
 		VFL_TYPE_GRABBER, IVTV_V4L2_ENC_YUV_OFFSET,
 		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VIDEO_CAPTURE,
+		V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TUNER |
+			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
 		&ivtv_v4l2_enc_fops
 	},
 	{	/* IVTV_ENC_STREAM_TYPE_VBI */
 		"encoder VBI",
 		VFL_TYPE_VBI, 0,
 		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VBI_CAPTURE,
+		V4L2_CAP_VBI_CAPTURE | V4L2_CAP_SLICED_VBI_CAPTURE | V4L2_CAP_TUNER |
+			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
 		&ivtv_v4l2_enc_fops
 	},
 	{	/* IVTV_ENC_STREAM_TYPE_PCM */
 		"encoder PCM",
 		VFL_TYPE_GRABBER, IVTV_V4L2_ENC_PCM_OFFSET,
 		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_PRIVATE,
+		V4L2_CAP_TUNER | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
 		&ivtv_v4l2_enc_fops
 	},
 	{	/* IVTV_ENC_STREAM_TYPE_RAD */
 		"encoder radio",
 		VFL_TYPE_RADIO, 0,
 		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_PRIVATE,
+		V4L2_CAP_RADIO | V4L2_CAP_TUNER,
 		&ivtv_v4l2_enc_fops
 	},
 	{	/* IVTV_DEC_STREAM_TYPE_MPG */
 		"decoder MPG",
 		VFL_TYPE_GRABBER, IVTV_V4L2_DEC_MPG_OFFSET,
 		PCI_DMA_TODEVICE, 0, V4L2_BUF_TYPE_VIDEO_OUTPUT,
+		V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
 		&ivtv_v4l2_dec_fops
 	},
 	{	/* IVTV_DEC_STREAM_TYPE_VBI */
 		"decoder VBI",
 		VFL_TYPE_VBI, IVTV_V4L2_DEC_VBI_OFFSET,
 		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_VBI_CAPTURE,
+		V4L2_CAP_SLICED_VBI_CAPTURE | V4L2_CAP_READWRITE,
 		&ivtv_v4l2_enc_fops
 	},
 	{	/* IVTV_DEC_STREAM_TYPE_VOUT */
 		"decoder VOUT",
 		VFL_TYPE_VBI, IVTV_V4L2_DEC_VOUT_OFFSET,
 		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_VBI_OUTPUT,
+		V4L2_CAP_SLICED_VBI_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
 		&ivtv_v4l2_dec_fops
 	},
 	{	/* IVTV_DEC_STREAM_TYPE_YUV */
 		"decoder YUV",
 		VFL_TYPE_GRABBER, IVTV_V4L2_DEC_YUV_OFFSET,
 		PCI_DMA_TODEVICE, 0, V4L2_BUF_TYPE_VIDEO_OUTPUT,
+		V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
 		&ivtv_v4l2_dec_fops
 	}
 };
@@ -149,6 +162,7 @@ static void ivtv_stream_init(struct ivtv *itv, int type)
 	s->itv = itv;
 	s->type = type;
 	s->name = ivtv_stream_info[type].name;
+	s->caps = ivtv_stream_info[type].v4l2_caps;
 
 	if (ivtv_stream_info[type].pio)
 		s->dma = PCI_DMA_NONE;
-- 
1.7.7.3

Re: [PATCH 1/3] V4L2: Add per-device-node capabilities

[Mauro Carvalho Chehab]

Hi Hans,

I have a few notes about the wording. See bellow.

Em 22-11-2011 08:05, Hans Verkuil escreveu:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> If V4L2_CAP_DEVICE_CAPS is set, then the new device_caps field is filled with
> the capabilities of the opened device node.
> 
> The capabilities field traditionally contains the capabilities of the whole
> device. 

Confusing. /dev/video is a "whole device", from kernel POV. 
It should be, instead, "physical device".

Maybe it could be written as:

	"The capabilities field traditionally contains the capabilities of the physical
	 device, being a superset of all capabilities available at the several device nodes."

> E.g., if you open video0, then if it contains VBI caps then that means
> that there is a corresponding vbi node as well. And the capabilities field of
> both the video and vbi node should contain identical caps.
> 
> However, it would be very useful to also have a capabilities field that contains
> just the caps for the currently open device, hence the new CAP bit and field.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  Documentation/DocBook/media/v4l/compat.xml         |    9 ++++++
>  Documentation/DocBook/media/v4l/v4l2.xml           |    9 +++++-
>  .../DocBook/media/v4l/vidioc-querycap.xml          |   29 +++++++++++++++++--
>  drivers/media/video/cx231xx/cx231xx-417.c          |    1 -
>  drivers/media/video/pvrusb2/pvrusb2-v4l2.c         |    1 -
>  drivers/media/video/v4l2-ioctl.c                   |    6 +++-
>  include/linux/videodev2.h                          |   29 +++++++++++++------
>  7 files changed, 67 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
> index b68698f..88ea4fc 100644
> --- a/Documentation/DocBook/media/v4l/compat.xml
> +++ b/Documentation/DocBook/media/v4l/compat.xml
> @@ -2378,6 +2378,15 @@ that used it. It was originally scheduled for removal in 2.6.35.
>          </listitem>
>        </orderedlist>
>      </section>
> +    <section>
> +      <title>V4L2 in Linux 3.3</title>
> +      <orderedlist>
> +        <listitem>
> +	  <para>Added the device_caps field to struct v4l2_capabilities and added the new
> +	  V4L2_CAP_DEVICE_CAPS capability.</para>
> +        </listitem>
> +      </orderedlist>
> +    </section>
>  
>      <section id="other">
>        <title>Relation of V4L2 to other Linux multimedia APIs</title>
> diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
> index 2ab365c..6b6e584 100644
> --- a/Documentation/DocBook/media/v4l/v4l2.xml
> +++ b/Documentation/DocBook/media/v4l/v4l2.xml
> @@ -128,6 +128,13 @@ structs, ioctls) must be noted in more detail in the history chapter
>  applications. -->
>  
>        <revision>
> +	<revnumber>3.3</revnumber>
> +	<date>2011-11-22</date>
> +	<authorinitials>hv</authorinitials>
> +	<revremark>Added device_caps field to struct v4l2_capabilities.</revremark>
> +      </revision>
> +
> +      <revision>
>  	<revnumber>3.2</revnumber>
>  	<date>2011-08-26</date>
>  	<authorinitials>hv</authorinitials>
> @@ -417,7 +424,7 @@ and discussions on the V4L mailing list.</revremark>
>  </partinfo>
>  
>  <title>Video for Linux Two API Specification</title>
> - <subtitle>Revision 3.2</subtitle>
> + <subtitle>Revision 3.3</subtitle>
>  
>    <chapter id="common">
>      &sub-common;
> diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> index e3664d6..632ad13 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> @@ -124,12 +124,29 @@ printf ("Version: %u.%u.%u\n",
>  	  <row>
>  	    <entry>__u32</entry>
>  	    <entry><structfield>capabilities</structfield></entry>
> -	    <entry>Device capabilities, see <xref
> -		linkend="device-capabilities" />.</entry>
> +	    <entry>Device capabilities of the physical device as a whole, see <xref
> +		linkend="device-capabilities" />. The same physical device can export
> +		multiple devices in /dev (e.g. /dev/videoX, /dev/vbiY and /dev/radioZ).
> +		For all those devices the capabilities field returns the same set of
> +		capabilities. This allows applications to open just the video device
> +		and discover whether vbi or radio is also supported.

I would write it as:

	    <entry>Available capabilities of the physical device as a whole, see <xref
		linkend="device-capabilities" />. The same physical device can export
		multiple devices in /dev (e.g. /dev/videoX, /dev/vbiY and /dev/radioZ).
		The capabilities field should contain an union of all capabilities available
		around the several V4L2 devices exported to userspace.
		For all those devices the capabilities field returns the same set of
		capabilities. This allows applications to open just one of the devices
		(typically the video device) and discover whether video, vbi and/or radio are
		also supported.


> +	    </entry>
>  	  </row>
>  	  <row>
>  	    <entry>__u32</entry>
> -	    <entry><structfield>reserved</structfield>[4]</entry>
> +	    <entry><structfield>device_caps</structfield></entry>
> +	    <entry>Device capabilities of the open device, see <xref
> +		linkend="device-capabilities" />. This is the set of capabilities
> +		of just the open device. So <structfield>device_caps</structfield>
> +		of a radio device will only contain radio related capabilities and
> +		no video capabilities. This field is only set if the <structfield>capabilities</structfield>
> +		field contains the <constant>V4L2_CAP_DEVICE_CAPS</constant>
> +		capability.

I would write it as:

	    <entry>Device capabilities of the opened device, see <xref
		linkend="device-capabilities" />. Should contain the available
		capabilities via that specific device node. So, for example,
		<structfield>device_caps</structfield> of a radio device will only 
		contain radio related capabilities and 	no video or vbi capabilities. 
	        This field is only set if the <structfield>capabilities</structfield>
		field contains the <constant>V4L2_CAP_DEVICE_CAPS</constant>
		capability.

> +	    </entry>
> +	  </row>
> +	  <row>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>reserved</structfield>[3]</entry>
>  	    <entry>Reserved for future extensions. Drivers must set
>  this array to zero.</entry>
>  	  </row>
> @@ -276,6 +293,12 @@ linkend="async">asynchronous</link> I/O methods.</entry>
>  	    <entry>The device supports the <link
>  linkend="mmap">streaming</link> I/O method.</entry>
>  	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_CAP_DEVICE_CAPS</constant></entry>
> +	    <entry>0x80000000</entry>
> +	    <entry>The driver fills the <structfield>device_caps</structfield>
> +	    field.</entry>
> +	  </row>
>  	</tbody>
>        </tgroup>
>      </table>
> diff --git a/drivers/media/video/cx231xx/cx231xx-417.c b/drivers/media/video/cx231xx/cx231xx-417.c
> index f8f0e59..d4327da 100644
> --- a/drivers/media/video/cx231xx/cx231xx-417.c
> +++ b/drivers/media/video/cx231xx/cx231xx-417.c
> @@ -1686,7 +1686,6 @@ static struct v4l2_capability pvr_capability = {
>  	.capabilities   = (V4L2_CAP_VIDEO_CAPTURE |
>  			   V4L2_CAP_TUNER | V4L2_CAP_AUDIO | V4L2_CAP_RADIO |
>  			 V4L2_CAP_STREAMING | V4L2_CAP_READWRITE),
> -	.reserved       = {0, 0, 0, 0}
>  };
>  static int vidioc_querycap(struct file *file, void  *priv,
>  				struct v4l2_capability *cap)
> diff --git a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
> index ce7ac45..e98b414 100644
> --- a/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
> +++ b/drivers/media/video/pvrusb2/pvrusb2-v4l2.c
> @@ -95,7 +95,6 @@ static struct v4l2_capability pvr_capability ={
>  	.capabilities   = (V4L2_CAP_VIDEO_CAPTURE |
>  			   V4L2_CAP_TUNER | V4L2_CAP_AUDIO | V4L2_CAP_RADIO |
>  			   V4L2_CAP_READWRITE),
> -	.reserved       = {0,0,0,0}
>  };
>  
>  static struct v4l2_fmtdesc pvr_fmtdesc [] = {
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index e1da8fc..ae716c2 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -538,10 +538,12 @@ static long __video_do_ioctl(struct file *file,
>  		if (!ret)
>  			dbgarg(cmd, "driver=%s, card=%s, bus=%s, "
>  					"version=0x%08x, "
> -					"capabilities=0x%08x\n",
> +					"capabilities=0x%08x, "
> +					"device_caps=0x%08x\n",
>  					cap->driver, cap->card, cap->bus_info,
>  					cap->version,
> -					cap->capabilities);
> +					cap->capabilities,
> +					cap->device_caps);
>  		break;
>  	}
>  
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 4b752d5..c185707 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -235,16 +235,25 @@ struct v4l2_fract {
>  	__u32   denominator;
>  };
>  
> -/*
> - *	D R I V E R   C A P A B I L I T I E S
> - */
> +/**
> +  * struct v4l2_capability - Describes V4L2 device caps returned by VIDIOC_QUERYCAP
> +  *
> +  * @driver:	   name of the driver module (e.g. "bttv")
> +  * @card:	   name of the card (e.g. "Hauppauge WinTV")
> +  * @bus_info:	   name of the bus (e.g. "PCI:" + pci_name(pci_dev) )
> +  * @version:	   KERNEL_VERSION
> +  * @capabilities: capabilities of the physical device as a whole
> +  * @device_caps:  capabilities accessed via this particular device (node)
> +  * @reserved:	   reserved fields for future extensions
> +  */
>  struct v4l2_capability {
> -	__u8	driver[16];	/* i.e. "bttv" */
> -	__u8	card[32];	/* i.e. "Hauppauge WinTV" */
> -	__u8	bus_info[32];	/* "PCI:" + pci_name(pci_dev) */
> -	__u32   version;        /* should use KERNEL_VERSION() */
> -	__u32	capabilities;	/* Device capabilities */
> -	__u32	reserved[4];
> +	__u8	driver[16];
> +	__u8	card[32];
> +	__u8	bus_info[32];
> +	__u32   version;
> +	__u32	capabilities;
> +	__u32	device_caps;
> +	__u32	reserved[3];
>  };
>  
>  /* Values for 'capabilities' field */
> @@ -274,6 +283,8 @@ struct v4l2_capability {
>  #define V4L2_CAP_ASYNCIO                0x02000000  /* async I/O */
>  #define V4L2_CAP_STREAMING              0x04000000  /* streaming I/O ioctls */
>  
> +#define V4L2_CAP_DEVICE_CAPS            0x80000000  /* sets device capabilities field */
> +
>  /*
>   *	V I D E O   I M A G E   F O R M A T
>   */

Re: [PATCH 2/3] vivi: set device_caps.

[Mauro Carvalho Chehab]

Em 22-11-2011 08:05, Hans Verkuil escreveu:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/video/vivi.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
> index 7d754fb..84ea88d 100644
> --- a/drivers/media/video/vivi.c
> +++ b/drivers/media/video/vivi.c
> @@ -819,8 +819,9 @@ static int vidioc_querycap(struct file *file, void  *priv,
>  	strcpy(cap->driver, "vivi");
>  	strcpy(cap->card, "vivi");
>  	strlcpy(cap->bus_info, dev->v4l2_dev.name, sizeof(cap->bus_info));
> -	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING | \
> -			    V4L2_CAP_READWRITE;
> +	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> +			    V4L2_CAP_READWRITE | V4L2_CAP_DEVICE_CAPS;
> +	cap->device_caps = cap->capabilities;

Hmm... should V4L2_CAP_DEVICE_CAPS be present at both device_caps and capabilities?

IMHO, the better would be to do:

	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
			    V4L2_CAP_READWRITE | V4L2_CAP_DEVICE_CAPS;
	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;

Btw, this ambiguity should also be solved at the V4L2 spec.

Regards,
Mauro

Re: [PATCH 3/3] ivtv: setup per-device caps.

[Mauro Carvalho Chehab]

Em 22-11-2011 08:05, Hans Verkuil escreveu:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/video/ivtv/ivtv-driver.h  |    1 +
>  drivers/media/video/ivtv/ivtv-ioctl.c   |    7 +++++--
>  drivers/media/video/ivtv/ivtv-streams.c |   14 ++++++++++++++
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/video/ivtv/ivtv-driver.h b/drivers/media/video/ivtv/ivtv-driver.h
> index 8f9cc17..06b9efd 100644
> --- a/drivers/media/video/ivtv/ivtv-driver.h
> +++ b/drivers/media/video/ivtv/ivtv-driver.h
> @@ -331,6 +331,7 @@ struct ivtv_stream {
>  	struct ivtv *itv; 		/* for ease of use */
>  	const char *name;		/* name of the stream */
>  	int type;			/* stream type */
> +	u32 caps;			/* V4L2 capabilities */
>  
>  	u32 id;
>  	spinlock_t qlock; 		/* locks access to the queues */
> diff --git a/drivers/media/video/ivtv/ivtv-ioctl.c b/drivers/media/video/ivtv/ivtv-ioctl.c
> index ecafa69..6be63e9 100644
> --- a/drivers/media/video/ivtv/ivtv-ioctl.c
> +++ b/drivers/media/video/ivtv/ivtv-ioctl.c
> @@ -752,12 +752,15 @@ static int ivtv_s_register(struct file *file, void *fh, struct v4l2_dbg_register
>  
>  static int ivtv_querycap(struct file *file, void *fh, struct v4l2_capability *vcap)
>  {
> -	struct ivtv *itv = fh2id(fh)->itv;
> +	struct ivtv_open_id *id = fh2id(file->private_data);
> +	struct ivtv *itv = id->itv;
> +	struct ivtv_stream *s = &itv->streams[id->type];
>  
>  	strlcpy(vcap->driver, IVTV_DRIVER_NAME, sizeof(vcap->driver));
>  	strlcpy(vcap->card, itv->card_name, sizeof(vcap->card));
>  	snprintf(vcap->bus_info, sizeof(vcap->bus_info), "PCI:%s", pci_name(itv->pdev));
> -	vcap->capabilities = itv->v4l2_cap; 	    /* capabilities */
> +	vcap->capabilities = itv->v4l2_cap | V4L2_CAP_DEVICE_CAPS;

IMO, the right thing to do here would be:

	vcap->capabilities = V4L2_CAP_DEVICE_CAPS;
	for (i = 0; i < ARRAY_SIZE(ivtv_stream_info); i++)
		vcap->capabilities |= ivtv_stream_info[v4l2_caps];

This avoids the risk of future patches adding new device_caps at the devices, but
forgetting to update the physical device capabilities.

Also, as the initial patches will be used as implementation reference by others,
such implementation will be more effective than a "magic" set of features that
may or may not match the union of all device capabilities.

> +	vcap->device_caps = s->caps;
>  	return 0;
>  }
>  
> diff --git a/drivers/media/video/ivtv/ivtv-streams.c b/drivers/media/video/ivtv/ivtv-streams.c
> index e7794dc..4d4ae6e 100644
> --- a/drivers/media/video/ivtv/ivtv-streams.c
> +++ b/drivers/media/video/ivtv/ivtv-streams.c
> @@ -78,60 +78,73 @@ static struct {
>  	int num_offset;
>  	int dma, pio;
>  	enum v4l2_buf_type buf_type;
> +	u32 v4l2_caps;
>  	const struct v4l2_file_operations *fops;
>  } ivtv_stream_info[] = {
>  	{	/* IVTV_ENC_STREAM_TYPE_MPG */
>  		"encoder MPG",
>  		VFL_TYPE_GRABBER, 0,
>  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VIDEO_CAPTURE,
> +		V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TUNER |
> +			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>  		&ivtv_v4l2_enc_fops
>  	},
>  	{	/* IVTV_ENC_STREAM_TYPE_YUV */
>  		"encoder YUV",
>  		VFL_TYPE_GRABBER, IVTV_V4L2_ENC_YUV_OFFSET,
>  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VIDEO_CAPTURE,
> +		V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TUNER |
> +			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>  		&ivtv_v4l2_enc_fops
>  	},
>  	{	/* IVTV_ENC_STREAM_TYPE_VBI */
>  		"encoder VBI",
>  		VFL_TYPE_VBI, 0,
>  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VBI_CAPTURE,
> +		V4L2_CAP_VBI_CAPTURE | V4L2_CAP_SLICED_VBI_CAPTURE | V4L2_CAP_TUNER |
> +			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>  		&ivtv_v4l2_enc_fops
>  	},
>  	{	/* IVTV_ENC_STREAM_TYPE_PCM */
>  		"encoder PCM",
>  		VFL_TYPE_GRABBER, IVTV_V4L2_ENC_PCM_OFFSET,
>  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_PRIVATE,
> +		V4L2_CAP_TUNER | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>  		&ivtv_v4l2_enc_fops
>  	},
>  	{	/* IVTV_ENC_STREAM_TYPE_RAD */
>  		"encoder radio",
>  		VFL_TYPE_RADIO, 0,
>  		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_PRIVATE,
> +		V4L2_CAP_RADIO | V4L2_CAP_TUNER,
>  		&ivtv_v4l2_enc_fops
>  	},
>  	{	/* IVTV_DEC_STREAM_TYPE_MPG */
>  		"decoder MPG",
>  		VFL_TYPE_GRABBER, IVTV_V4L2_DEC_MPG_OFFSET,
>  		PCI_DMA_TODEVICE, 0, V4L2_BUF_TYPE_VIDEO_OUTPUT,
> +		V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>  		&ivtv_v4l2_dec_fops
>  	},
>  	{	/* IVTV_DEC_STREAM_TYPE_VBI */
>  		"decoder VBI",
>  		VFL_TYPE_VBI, IVTV_V4L2_DEC_VBI_OFFSET,
>  		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_VBI_CAPTURE,
> +		V4L2_CAP_SLICED_VBI_CAPTURE | V4L2_CAP_READWRITE,
>  		&ivtv_v4l2_enc_fops
>  	},
>  	{	/* IVTV_DEC_STREAM_TYPE_VOUT */
>  		"decoder VOUT",
>  		VFL_TYPE_VBI, IVTV_V4L2_DEC_VOUT_OFFSET,
>  		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_VBI_OUTPUT,
> +		V4L2_CAP_SLICED_VBI_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>  		&ivtv_v4l2_dec_fops
>  	},
>  	{	/* IVTV_DEC_STREAM_TYPE_YUV */
>  		"decoder YUV",
>  		VFL_TYPE_GRABBER, IVTV_V4L2_DEC_YUV_OFFSET,
>  		PCI_DMA_TODEVICE, 0, V4L2_BUF_TYPE_VIDEO_OUTPUT,
> +		V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>  		&ivtv_v4l2_dec_fops
>  	}
>  };
> @@ -149,6 +162,7 @@ static void ivtv_stream_init(struct ivtv *itv, int type)
>  	s->itv = itv;
>  	s->type = type;
>  	s->name = ivtv_stream_info[type].name;
> +	s->caps = ivtv_stream_info[type].v4l2_caps;
>  
>  	if (ivtv_stream_info[type].pio)
>  		s->dma = PCI_DMA_NONE;

Re: [PATCH 1/3] V4L2: Add per-device-node capabilities

[Hans Verkuil]

On Friday, November 25, 2011 11:51:20 Mauro Carvalho Chehab wrote:
> Hi Hans,
> 
> I have a few notes about the wording. See bellow.
> 
> Em 22-11-2011 08:05, Hans Verkuil escreveu:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > If V4L2_CAP_DEVICE_CAPS is set, then the new device_caps field is filled with
> > the capabilities of the opened device node.
> > 
> > The capabilities field traditionally contains the capabilities of the whole
> > device. 
> 
> Confusing. /dev/video is a "whole device", from kernel POV. 
> It should be, instead, "physical device".
> 
> Maybe it could be written as:
> 
> 	"The capabilities field traditionally contains the capabilities of the physical
> 	 device, being a superset of all capabilities available at the several device nodes."

Nice. I'll use your phrasing.

> 
> > E.g., if you open video0, then if it contains VBI caps then that means
> > that there is a corresponding vbi node as well. And the capabilities field of
> > both the video and vbi node should contain identical caps.
> > 
> > However, it would be very useful to also have a capabilities field that contains
> > just the caps for the currently open device, hence the new CAP bit and field.
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > ---
> >  Documentation/DocBook/media/v4l/compat.xml         |    9 ++++++
> >  Documentation/DocBook/media/v4l/v4l2.xml           |    9 +++++-
> >  .../DocBook/media/v4l/vidioc-querycap.xml          |   29 +++++++++++++++++--
> >  drivers/media/video/cx231xx/cx231xx-417.c          |    1 -
> >  drivers/media/video/pvrusb2/pvrusb2-v4l2.c         |    1 -
> >  drivers/media/video/v4l2-ioctl.c                   |    6 +++-
> >  include/linux/videodev2.h                          |   29 +++++++++++++------
> >  7 files changed, 67 insertions(+), 17 deletions(-)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
> > index b68698f..88ea4fc 100644
> > --- a/Documentation/DocBook/media/v4l/compat.xml
> > +++ b/Documentation/DocBook/media/v4l/compat.xml
> > @@ -2378,6 +2378,15 @@ that used it. It was originally scheduled for removal in 2.6.35.
> >          </listitem>
> >        </orderedlist>
> >      </section>
> > +    <section>
> > +      <title>V4L2 in Linux 3.3</title>
> > +      <orderedlist>
> > +        <listitem>
> > +	  <para>Added the device_caps field to struct v4l2_capabilities and added the new
> > +	  V4L2_CAP_DEVICE_CAPS capability.</para>
> > +        </listitem>
> > +      </orderedlist>
> > +    </section>
> >  
> >      <section id="other">
> >        <title>Relation of V4L2 to other Linux multimedia APIs</title>
> > diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
> > index 2ab365c..6b6e584 100644
> > --- a/Documentation/DocBook/media/v4l/v4l2.xml
> > +++ b/Documentation/DocBook/media/v4l/v4l2.xml
> > @@ -128,6 +128,13 @@ structs, ioctls) must be noted in more detail in the history chapter
> >  applications. -->
> >  
> >        <revision>
> > +	<revnumber>3.3</revnumber>
> > +	<date>2011-11-22</date>
> > +	<authorinitials>hv</authorinitials>
> > +	<revremark>Added device_caps field to struct v4l2_capabilities.</revremark>
> > +      </revision>
> > +
> > +      <revision>
> >  	<revnumber>3.2</revnumber>
> >  	<date>2011-08-26</date>
> >  	<authorinitials>hv</authorinitials>
> > @@ -417,7 +424,7 @@ and discussions on the V4L mailing list.</revremark>
> >  </partinfo>
> >  
> >  <title>Video for Linux Two API Specification</title>
> > - <subtitle>Revision 3.2</subtitle>
> > + <subtitle>Revision 3.3</subtitle>
> >  
> >    <chapter id="common">
> >      &sub-common;
> > diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> > index e3664d6..632ad13 100644
> > --- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> > +++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml
> > @@ -124,12 +124,29 @@ printf ("Version: %u.%u.%u\n",
> >  	  <row>
> >  	    <entry>__u32</entry>
> >  	    <entry><structfield>capabilities</structfield></entry>
> > -	    <entry>Device capabilities, see <xref
> > -		linkend="device-capabilities" />.</entry>
> > +	    <entry>Device capabilities of the physical device as a whole, see <xref
> > +		linkend="device-capabilities" />. The same physical device can export
> > +		multiple devices in /dev (e.g. /dev/videoX, /dev/vbiY and /dev/radioZ).
> > +		For all those devices the capabilities field returns the same set of
> > +		capabilities. This allows applications to open just the video device
> > +		and discover whether vbi or radio is also supported.
> 
> I would write it as:
> 
> 	    <entry>Available capabilities of the physical device as a whole, see <xref
> 		linkend="device-capabilities" />. The same physical device can export
> 		multiple devices in /dev (e.g. /dev/videoX, /dev/vbiY and /dev/radioZ).
> 		The capabilities field should contain an union of all capabilities available
> 		around the several V4L2 devices exported to userspace.
> 		For all those devices the capabilities field returns the same set of
> 		capabilities. This allows applications to open just one of the devices
> 		(typically the video device) and discover whether video, vbi and/or radio are
> 		also supported.

Ditto.

> 
> 
> > +	    </entry>
> >  	  </row>
> >  	  <row>
> >  	    <entry>__u32</entry>
> > -	    <entry><structfield>reserved</structfield>[4]</entry>
> > +	    <entry><structfield>device_caps</structfield></entry>
> > +	    <entry>Device capabilities of the open device, see <xref
> > +		linkend="device-capabilities" />. This is the set of capabilities
> > +		of just the open device. So <structfield>device_caps</structfield>
> > +		of a radio device will only contain radio related capabilities and
> > +		no video capabilities. This field is only set if the <structfield>capabilities</structfield>
> > +		field contains the <constant>V4L2_CAP_DEVICE_CAPS</constant>
> > +		capability.
> 
> I would write it as:
> 
> 	    <entry>Device capabilities of the opened device, see <xref
> 		linkend="device-capabilities" />. Should contain the available
> 		capabilities via that specific device node. So, for example,
> 		<structfield>device_caps</structfield> of a radio device will only 
> 		contain radio related capabilities and 	no video or vbi capabilities. 
> 	        This field is only set if the <structfield>capabilities</structfield>
> 		field contains the <constant>V4L2_CAP_DEVICE_CAPS</constant>
> 		capability.

Ditto.

Good suggestions, I should be able to prepare a new version today.

Regards,

	Hans

Re: [PATCH 2/3] vivi: set device_caps.

[Hans Verkuil]

On Friday, November 25, 2011 11:53:52 Mauro Carvalho Chehab wrote:
> Em 22-11-2011 08:05, Hans Verkuil escreveu:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > ---
> >  drivers/media/video/vivi.c |    5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
> > index 7d754fb..84ea88d 100644
> > --- a/drivers/media/video/vivi.c
> > +++ b/drivers/media/video/vivi.c
> > @@ -819,8 +819,9 @@ static int vidioc_querycap(struct file *file, void  *priv,
> >  	strcpy(cap->driver, "vivi");
> >  	strcpy(cap->card, "vivi");
> >  	strlcpy(cap->bus_info, dev->v4l2_dev.name, sizeof(cap->bus_info));
> > -	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING | \
> > -			    V4L2_CAP_READWRITE;
> > +	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> > +			    V4L2_CAP_READWRITE | V4L2_CAP_DEVICE_CAPS;
> > +	cap->device_caps = cap->capabilities;
> 
> Hmm... should V4L2_CAP_DEVICE_CAPS be present at both device_caps and capabilities?

Good question. It doesn't feel right to me to add it to device_caps: it
really doesn't mean anything there.

The whole point is to have device_caps only show the capabilities
of that device node. V4L2_CAP_DEVICE_CAPS is a capability of the whole
physical device, so I don't believe it should be set in device_caps.

> 
> IMHO, the better would be to do:
> 
> 	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> 			    V4L2_CAP_READWRITE | V4L2_CAP_DEVICE_CAPS;
> 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> 
> Btw, this ambiguity should also be solved at the V4L2 spec.

I will update the docs, once we agree on this.

Regards,

	Hans

Re: [PATCH 3/3] ivtv: setup per-device caps.

[Hans Verkuil]

On Friday, November 25, 2011 12:00:31 Mauro Carvalho Chehab wrote:
> Em 22-11-2011 08:05, Hans Verkuil escreveu:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > ---
> >  drivers/media/video/ivtv/ivtv-driver.h  |    1 +
> >  drivers/media/video/ivtv/ivtv-ioctl.c   |    7 +++++--
> >  drivers/media/video/ivtv/ivtv-streams.c |   14 ++++++++++++++
> >  3 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/video/ivtv/ivtv-driver.h b/drivers/media/video/ivtv/ivtv-driver.h
> > index 8f9cc17..06b9efd 100644
> > --- a/drivers/media/video/ivtv/ivtv-driver.h
> > +++ b/drivers/media/video/ivtv/ivtv-driver.h
> > @@ -331,6 +331,7 @@ struct ivtv_stream {
> >  	struct ivtv *itv; 		/* for ease of use */
> >  	const char *name;		/* name of the stream */
> >  	int type;			/* stream type */
> > +	u32 caps;			/* V4L2 capabilities */
> >  
> >  	u32 id;
> >  	spinlock_t qlock; 		/* locks access to the queues */
> > diff --git a/drivers/media/video/ivtv/ivtv-ioctl.c b/drivers/media/video/ivtv/ivtv-ioctl.c
> > index ecafa69..6be63e9 100644
> > --- a/drivers/media/video/ivtv/ivtv-ioctl.c
> > +++ b/drivers/media/video/ivtv/ivtv-ioctl.c
> > @@ -752,12 +752,15 @@ static int ivtv_s_register(struct file *file, void *fh, struct v4l2_dbg_register
> >  
> >  static int ivtv_querycap(struct file *file, void *fh, struct v4l2_capability *vcap)
> >  {
> > -	struct ivtv *itv = fh2id(fh)->itv;
> > +	struct ivtv_open_id *id = fh2id(file->private_data);
> > +	struct ivtv *itv = id->itv;
> > +	struct ivtv_stream *s = &itv->streams[id->type];
> >  
> >  	strlcpy(vcap->driver, IVTV_DRIVER_NAME, sizeof(vcap->driver));
> >  	strlcpy(vcap->card, itv->card_name, sizeof(vcap->card));
> >  	snprintf(vcap->bus_info, sizeof(vcap->bus_info), "PCI:%s", pci_name(itv->pdev));
> > -	vcap->capabilities = itv->v4l2_cap; 	    /* capabilities */
> > +	vcap->capabilities = itv->v4l2_cap | V4L2_CAP_DEVICE_CAPS;
> 
> IMO, the right thing to do here would be:
> 
> 	vcap->capabilities = V4L2_CAP_DEVICE_CAPS;
> 	for (i = 0; i < ARRAY_SIZE(ivtv_stream_info); i++)
> 		vcap->capabilities |= ivtv_stream_info[v4l2_caps];

This won't work actually. Which devices are available depends on more things
than just that array. It's not something I think needs to change.

> This avoids the risk of future patches adding new device_caps at the devices, but
> forgetting to update the physical device capabilities.
>
> Also, as the initial patches will be used as implementation reference by others,
> such implementation will be more effective than a "magic" set of features that
> may or may not match the union of all device capabilities.

I wouldn't use ivtv as a reference implementation (other than for cx18). vivi however
is a very nice reference implementation these days.

As soon as this patch is in I'll also update v4l2-compliance.

Regards,

	Hans

> 
> > +	vcap->device_caps = s->caps;
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/media/video/ivtv/ivtv-streams.c b/drivers/media/video/ivtv/ivtv-streams.c
> > index e7794dc..4d4ae6e 100644
> > --- a/drivers/media/video/ivtv/ivtv-streams.c
> > +++ b/drivers/media/video/ivtv/ivtv-streams.c
> > @@ -78,60 +78,73 @@ static struct {
> >  	int num_offset;
> >  	int dma, pio;
> >  	enum v4l2_buf_type buf_type;
> > +	u32 v4l2_caps;
> >  	const struct v4l2_file_operations *fops;
> >  } ivtv_stream_info[] = {
> >  	{	/* IVTV_ENC_STREAM_TYPE_MPG */
> >  		"encoder MPG",
> >  		VFL_TYPE_GRABBER, 0,
> >  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VIDEO_CAPTURE,
> > +		V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TUNER |
> > +			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
> >  		&ivtv_v4l2_enc_fops
> >  	},
> >  	{	/* IVTV_ENC_STREAM_TYPE_YUV */
> >  		"encoder YUV",
> >  		VFL_TYPE_GRABBER, IVTV_V4L2_ENC_YUV_OFFSET,
> >  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VIDEO_CAPTURE,
> > +		V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TUNER |
> > +			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
> >  		&ivtv_v4l2_enc_fops
> >  	},
> >  	{	/* IVTV_ENC_STREAM_TYPE_VBI */
> >  		"encoder VBI",
> >  		VFL_TYPE_VBI, 0,
> >  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VBI_CAPTURE,
> > +		V4L2_CAP_VBI_CAPTURE | V4L2_CAP_SLICED_VBI_CAPTURE | V4L2_CAP_TUNER |
> > +			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
> >  		&ivtv_v4l2_enc_fops
> >  	},
> >  	{	/* IVTV_ENC_STREAM_TYPE_PCM */
> >  		"encoder PCM",
> >  		VFL_TYPE_GRABBER, IVTV_V4L2_ENC_PCM_OFFSET,
> >  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_PRIVATE,
> > +		V4L2_CAP_TUNER | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
> >  		&ivtv_v4l2_enc_fops
> >  	},
> >  	{	/* IVTV_ENC_STREAM_TYPE_RAD */
> >  		"encoder radio",
> >  		VFL_TYPE_RADIO, 0,
> >  		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_PRIVATE,
> > +		V4L2_CAP_RADIO | V4L2_CAP_TUNER,
> >  		&ivtv_v4l2_enc_fops
> >  	},
> >  	{	/* IVTV_DEC_STREAM_TYPE_MPG */
> >  		"decoder MPG",
> >  		VFL_TYPE_GRABBER, IVTV_V4L2_DEC_MPG_OFFSET,
> >  		PCI_DMA_TODEVICE, 0, V4L2_BUF_TYPE_VIDEO_OUTPUT,
> > +		V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
> >  		&ivtv_v4l2_dec_fops
> >  	},
> >  	{	/* IVTV_DEC_STREAM_TYPE_VBI */
> >  		"decoder VBI",
> >  		VFL_TYPE_VBI, IVTV_V4L2_DEC_VBI_OFFSET,
> >  		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_VBI_CAPTURE,
> > +		V4L2_CAP_SLICED_VBI_CAPTURE | V4L2_CAP_READWRITE,
> >  		&ivtv_v4l2_enc_fops
> >  	},
> >  	{	/* IVTV_DEC_STREAM_TYPE_VOUT */
> >  		"decoder VOUT",
> >  		VFL_TYPE_VBI, IVTV_V4L2_DEC_VOUT_OFFSET,
> >  		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_VBI_OUTPUT,
> > +		V4L2_CAP_SLICED_VBI_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
> >  		&ivtv_v4l2_dec_fops
> >  	},
> >  	{	/* IVTV_DEC_STREAM_TYPE_YUV */
> >  		"decoder YUV",
> >  		VFL_TYPE_GRABBER, IVTV_V4L2_DEC_YUV_OFFSET,
> >  		PCI_DMA_TODEVICE, 0, V4L2_BUF_TYPE_VIDEO_OUTPUT,
> > +		V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
> >  		&ivtv_v4l2_dec_fops
> >  	}
> >  };
> > @@ -149,6 +162,7 @@ static void ivtv_stream_init(struct ivtv *itv, int type)
> >  	s->itv = itv;
> >  	s->type = type;
> >  	s->name = ivtv_stream_info[type].name;
> > +	s->caps = ivtv_stream_info[type].v4l2_caps;
> >  
> >  	if (ivtv_stream_info[type].pio)
> >  		s->dma = PCI_DMA_NONE;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 3/3] ivtv: setup per-device caps.

[Mauro Carvalho Chehab]

Em 25-11-2011 09:29, Hans Verkuil escreveu:
> On Friday, November 25, 2011 12:00:31 Mauro Carvalho Chehab wrote:
>> Em 22-11-2011 08:05, Hans Verkuil escreveu:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> ---
>>>  drivers/media/video/ivtv/ivtv-driver.h  |    1 +
>>>  drivers/media/video/ivtv/ivtv-ioctl.c   |    7 +++++--
>>>  drivers/media/video/ivtv/ivtv-streams.c |   14 ++++++++++++++
>>>  3 files changed, 20 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/video/ivtv/ivtv-driver.h b/drivers/media/video/ivtv/ivtv-driver.h
>>> index 8f9cc17..06b9efd 100644
>>> --- a/drivers/media/video/ivtv/ivtv-driver.h
>>> +++ b/drivers/media/video/ivtv/ivtv-driver.h
>>> @@ -331,6 +331,7 @@ struct ivtv_stream {
>>>  	struct ivtv *itv; 		/* for ease of use */
>>>  	const char *name;		/* name of the stream */
>>>  	int type;			/* stream type */
>>> +	u32 caps;			/* V4L2 capabilities */
>>>  
>>>  	u32 id;
>>>  	spinlock_t qlock; 		/* locks access to the queues */
>>> diff --git a/drivers/media/video/ivtv/ivtv-ioctl.c b/drivers/media/video/ivtv/ivtv-ioctl.c
>>> index ecafa69..6be63e9 100644
>>> --- a/drivers/media/video/ivtv/ivtv-ioctl.c
>>> +++ b/drivers/media/video/ivtv/ivtv-ioctl.c
>>> @@ -752,12 +752,15 @@ static int ivtv_s_register(struct file *file, void *fh, struct v4l2_dbg_register
>>>  
>>>  static int ivtv_querycap(struct file *file, void *fh, struct v4l2_capability *vcap)
>>>  {
>>> -	struct ivtv *itv = fh2id(fh)->itv;
>>> +	struct ivtv_open_id *id = fh2id(file->private_data);
>>> +	struct ivtv *itv = id->itv;
>>> +	struct ivtv_stream *s = &itv->streams[id->type];
>>>  
>>>  	strlcpy(vcap->driver, IVTV_DRIVER_NAME, sizeof(vcap->driver));
>>>  	strlcpy(vcap->card, itv->card_name, sizeof(vcap->card));
>>>  	snprintf(vcap->bus_info, sizeof(vcap->bus_info), "PCI:%s", pci_name(itv->pdev));
>>> -	vcap->capabilities = itv->v4l2_cap; 	    /* capabilities */
>>> +	vcap->capabilities = itv->v4l2_cap | V4L2_CAP_DEVICE_CAPS;
>>
>> IMO, the right thing to do here would be:
>>
>> 	vcap->capabilities = V4L2_CAP_DEVICE_CAPS;
>> 	for (i = 0; i < ARRAY_SIZE(ivtv_stream_info); i++)
>> 		vcap->capabilities |= ivtv_stream_info[v4l2_caps];
> 
> This won't work actually. Which devices are available depends on more things
> than just that array. 

Yes, I suspect so. Yet, a loop like that would still work:

 	for (i = 0; i < ARRAY_SIZE(ivtv_stream_info); i++)
		if (check_if_device_applies(ivtv_stream_info[i]))
	 		vcap->capabilities |= ivtv_stream_info[v4l2_caps];

> It's not something I think needs to change.

I still think that something like the above is better than two separate magic sets.

OK, it is perhaps an overkill for ivtv, as there aren't any new features/boards/etc added 
there for some time.

> 
>> This avoids the risk of future patches adding new device_caps at the devices, but
>> forgetting to update the physical device capabilities.
>>
>> Also, as the initial patches will be used as implementation reference by others,
>> such implementation will be more effective than a "magic" set of features that
>> may or may not match the union of all device capabilities.
> 
> I wouldn't use ivtv as a reference implementation (other than for cx18). vivi however
> is a very nice reference implementation these days.

Yes, but vivi creates just one device per "physical device". So, the implementation there is trivial.

> 
> As soon as this patch is in I'll also update v4l2-compliance.
> 
> Regards,
> 
> 	Hans
> 
>>
>>> +	vcap->device_caps = s->caps;
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/drivers/media/video/ivtv/ivtv-streams.c b/drivers/media/video/ivtv/ivtv-streams.c
>>> index e7794dc..4d4ae6e 100644
>>> --- a/drivers/media/video/ivtv/ivtv-streams.c
>>> +++ b/drivers/media/video/ivtv/ivtv-streams.c
>>> @@ -78,60 +78,73 @@ static struct {
>>>  	int num_offset;
>>>  	int dma, pio;
>>>  	enum v4l2_buf_type buf_type;
>>> +	u32 v4l2_caps;
>>>  	const struct v4l2_file_operations *fops;
>>>  } ivtv_stream_info[] = {
>>>  	{	/* IVTV_ENC_STREAM_TYPE_MPG */
>>>  		"encoder MPG",
>>>  		VFL_TYPE_GRABBER, 0,
>>>  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VIDEO_CAPTURE,
>>> +		V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TUNER |
>>> +			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>>>  		&ivtv_v4l2_enc_fops
>>>  	},
>>>  	{	/* IVTV_ENC_STREAM_TYPE_YUV */
>>>  		"encoder YUV",
>>>  		VFL_TYPE_GRABBER, IVTV_V4L2_ENC_YUV_OFFSET,
>>>  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VIDEO_CAPTURE,
>>> +		V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TUNER |
>>> +			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>>>  		&ivtv_v4l2_enc_fops
>>>  	},
>>>  	{	/* IVTV_ENC_STREAM_TYPE_VBI */
>>>  		"encoder VBI",
>>>  		VFL_TYPE_VBI, 0,
>>>  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_VBI_CAPTURE,
>>> +		V4L2_CAP_VBI_CAPTURE | V4L2_CAP_SLICED_VBI_CAPTURE | V4L2_CAP_TUNER |
>>> +			V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>>>  		&ivtv_v4l2_enc_fops
>>>  	},
>>>  	{	/* IVTV_ENC_STREAM_TYPE_PCM */
>>>  		"encoder PCM",
>>>  		VFL_TYPE_GRABBER, IVTV_V4L2_ENC_PCM_OFFSET,
>>>  		PCI_DMA_FROMDEVICE, 0, V4L2_BUF_TYPE_PRIVATE,
>>> +		V4L2_CAP_TUNER | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>>>  		&ivtv_v4l2_enc_fops
>>>  	},
>>>  	{	/* IVTV_ENC_STREAM_TYPE_RAD */
>>>  		"encoder radio",
>>>  		VFL_TYPE_RADIO, 0,
>>>  		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_PRIVATE,
>>> +		V4L2_CAP_RADIO | V4L2_CAP_TUNER,
>>>  		&ivtv_v4l2_enc_fops
>>>  	},
>>>  	{	/* IVTV_DEC_STREAM_TYPE_MPG */
>>>  		"decoder MPG",
>>>  		VFL_TYPE_GRABBER, IVTV_V4L2_DEC_MPG_OFFSET,
>>>  		PCI_DMA_TODEVICE, 0, V4L2_BUF_TYPE_VIDEO_OUTPUT,
>>> +		V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>>>  		&ivtv_v4l2_dec_fops
>>>  	},
>>>  	{	/* IVTV_DEC_STREAM_TYPE_VBI */
>>>  		"decoder VBI",
>>>  		VFL_TYPE_VBI, IVTV_V4L2_DEC_VBI_OFFSET,
>>>  		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_VBI_CAPTURE,
>>> +		V4L2_CAP_SLICED_VBI_CAPTURE | V4L2_CAP_READWRITE,
>>>  		&ivtv_v4l2_enc_fops
>>>  	},
>>>  	{	/* IVTV_DEC_STREAM_TYPE_VOUT */
>>>  		"decoder VOUT",
>>>  		VFL_TYPE_VBI, IVTV_V4L2_DEC_VOUT_OFFSET,
>>>  		PCI_DMA_NONE, 1, V4L2_BUF_TYPE_VBI_OUTPUT,
>>> +		V4L2_CAP_SLICED_VBI_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>>>  		&ivtv_v4l2_dec_fops
>>>  	},
>>>  	{	/* IVTV_DEC_STREAM_TYPE_YUV */
>>>  		"decoder YUV",
>>>  		VFL_TYPE_GRABBER, IVTV_V4L2_DEC_YUV_OFFSET,
>>>  		PCI_DMA_TODEVICE, 0, V4L2_BUF_TYPE_VIDEO_OUTPUT,
>>> +		V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_AUDIO | V4L2_CAP_READWRITE,
>>>  		&ivtv_v4l2_dec_fops
>>>  	}
>>>  };
>>> @@ -149,6 +162,7 @@ static void ivtv_stream_init(struct ivtv *itv, int type)
>>>  	s->itv = itv;
>>>  	s->type = type;
>>>  	s->name = ivtv_stream_info[type].name;
>>> +	s->caps = ivtv_stream_info[type].v4l2_caps;
>>>  
>>>  	if (ivtv_stream_info[type].pio)
>>>  		s->dma = PCI_DMA_NONE;
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html