[PATCHv15,2/8] v4l2: video device: Add V4L2_CTRL_CLASS_FM_TX controls

Message ID 1249729833-24975-3-git-send-email-eduardo.valentin@nokia.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Eduardo Valentin Aug. 8, 2009, 11:10 a.m. UTC
  This patch adds a new class of extended controls. This class
is intended to support FM Radio Modulators properties such as:
rds, audio limiters, audio compression, pilot tone generation,
tuning power levels and preemphasis properties.

Signed-off-by: Eduardo Valentin <eduardo.valentin@nokia.com>
---
 linux/include/linux/videodev2.h |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)
  

Comments

Hans Verkuil Aug. 15, 2009, 11:57 a.m. UTC | #1
On Saturday 08 August 2009 13:10:27 Eduardo Valentin wrote:
> This patch adds a new class of extended controls. This class
> is intended to support FM Radio Modulators properties such as:
> rds, audio limiters, audio compression, pilot tone generation,
> tuning power levels and preemphasis properties.
> 
> Signed-off-by: Eduardo Valentin <eduardo.valentin@nokia.com>
> ---
>  linux/include/linux/videodev2.h |   34 ++++++++++++++++++++++++++++++++++
>  1 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/linux/include/linux/videodev2.h b/linux/include/linux/videodev2.h
> index b17898c..a86a575 100644
> --- a/linux/include/linux/videodev2.h
> +++ b/linux/include/linux/videodev2.h
> @@ -817,6 +817,7 @@ struct v4l2_ext_controls {
>  #define V4L2_CTRL_CLASS_USER 0x00980000	/* Old-style 'user' controls */
>  #define V4L2_CTRL_CLASS_MPEG 0x00990000	/* MPEG-compression controls */
>  #define V4L2_CTRL_CLASS_CAMERA 0x009a0000	/* Camera class controls */
> +#define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
>  
>  #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
>  #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
> @@ -1156,6 +1157,39 @@ enum  v4l2_exposure_auto_type {
>  
>  #define V4L2_CID_PRIVACY			(V4L2_CID_CAMERA_CLASS_BASE+16)
>  
> +/* FM Modulator class control IDs */
> +#define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
> +#define V4L2_CID_FM_TX_CLASS			(V4L2_CTRL_CLASS_FM_TX | 1)
> +
> +#define V4L2_CID_RDS_TX_DEVIATION		(V4L2_CID_FM_TX_CLASS_BASE + 1)
> +#define V4L2_CID_RDS_TX_PI			(V4L2_CID_FM_TX_CLASS_BASE + 2)
> +#define V4L2_CID_RDS_TX_PTY			(V4L2_CID_FM_TX_CLASS_BASE + 3)
> +#define V4L2_CID_RDS_TX_PS_NAME			(V4L2_CID_FM_TX_CLASS_BASE + 4)
> +#define V4L2_CID_RDS_TX_RADIO_TEXT		(V4L2_CID_FM_TX_CLASS_BASE + 5)
> +
> +#define V4L2_CID_AUDIO_LIMITER_ENABLED		(V4L2_CID_FM_TX_CLASS_BASE + 6)
> +#define V4L2_CID_AUDIO_LIMITER_RELEASE_TIME	(V4L2_CID_FM_TX_CLASS_BASE + 7)
> +#define V4L2_CID_AUDIO_LIMITER_DEVIATION	(V4L2_CID_FM_TX_CLASS_BASE + 8)
> +
> +#define V4L2_CID_AUDIO_COMPRESSION_ENABLED	(V4L2_CID_FM_TX_CLASS_BASE + 9)
> +#define V4L2_CID_AUDIO_COMPRESSION_GAIN		(V4L2_CID_FM_TX_CLASS_BASE + 10)
> +#define V4L2_CID_AUDIO_COMPRESSION_THRESHOLD	(V4L2_CID_FM_TX_CLASS_BASE + 11)
> +#define V4L2_CID_AUDIO_COMPRESSION_ATTACK_TIME	(V4L2_CID_FM_TX_CLASS_BASE + 12)
> +#define V4L2_CID_AUDIO_COMPRESSION_RELEASE_TIME	(V4L2_CID_FM_TX_CLASS_BASE + 13)
> +
> +#define V4L2_CID_PILOT_TONE_ENABLED		(V4L2_CID_FM_TX_CLASS_BASE + 14)
> +#define V4L2_CID_PILOT_TONE_DEVIATION		(V4L2_CID_FM_TX_CLASS_BASE + 15)
> +#define V4L2_CID_PILOT_TONE_FREQUENCY		(V4L2_CID_FM_TX_CLASS_BASE + 16)
> +
> +#define V4L2_CID_FM_TX_PREEMPHASIS		(V4L2_CID_FM_TX_CLASS_BASE + 17)
> +enum v4l2_preemphasis {
> +	V4L2_PREEMPHASIS_DISABLED	= 0,
> +	V4L2_PREEMPHASIS_50_uS		= 1,
> +	V4L2_PREEMPHASIS_75_uS		= 2,
> +};
> +#define V4L2_CID_TUNE_POWER_LEVEL		(V4L2_CID_FM_TX_CLASS_BASE + 18)
> +#define V4L2_CID_TUNE_ANTENNA_CAPACITOR		(V4L2_CID_FM_TX_CLASS_BASE + 19)
> +
>  /*
>   *	T U N I N G
>   */

Hi Eduardo,

I would like to make some small changes here: the control IDs do not have to
be consecutive so I think it is better to leave some gaps between the various
sections: e.g. let the AUDIO controls start at BASE + 64 (leaving more than
enough room for additional RDS controls). Between PTY and PS_NAME we should
skip one control since I'm sure at some time in the future a PTY_NAME will
appear. A comment like '/* placeholder for future PTY_NAME control */' would
be useful as well.

The AUDIO_COMPRESSION controls can start at BASE + 80, the PILOT controls at
BASE + 96 and the last set of controls at BASE + 112.

BTW, wouldn't it be slightly more consistent if V4L2_CID_FM_TX_PREEMPHASIS is
renamed to V4L2_CID_TUNE_PREEMPHASIS? It's a bit of an odd one out at the
moment.

Note that I've verified my compat32 implementation for strings: it's working
correctly on my 64-bit machine (I hacked a string control into vivi.c to do
the testing).

If you agree with these proposed changes, then I can modify the patch myself,
no need for a new patch series.

Regards,

	Hans
  
Eduardo Valentin Aug. 16, 2009, 10:16 a.m. UTC | #2
Hello Hans,

<snip>

>
> Hi Eduardo,
>
> I would like to make some small changes here: the control IDs do not have to
> be consecutive so I think it is better to leave some gaps between the various
> sections: e.g. let the AUDIO controls start at BASE + 64 (leaving more than
> enough room for additional RDS controls). Between PTY and PS_NAME we should
> skip one control since I'm sure at some time in the future a PTY_NAME will
> appear. A comment like '/* placeholder for future PTY_NAME control */' would
> be useful as well.

I think leaving room for following controls would be good. No problem to leave
some space left already in between these ID's.

About the comment for PTY_NAME, I also agree with you.

>
> The AUDIO_COMPRESSION controls can start at BASE + 80, the PILOT controls at
> BASE + 96 and the last set of controls at BASE + 112.

Right.

>
> BTW, wouldn't it be slightly more consistent if V4L2_CID_FM_TX_PREEMPHASIS is
> renamed to V4L2_CID_TUNE_PREEMPHASIS? It's a bit of an odd one out at the
> moment.

Right. Good. Better to aggregate it into the *_TUNE_* controls. Which
makes sense for me.

>
> Note that I've verified my compat32 implementation for strings: it's working
> correctly on my 64-bit machine (I hacked a string control into vivi.c to do
> the testing).

Right.

>
> If you agree with these proposed changes, then I can modify the patch myself,
> no need for a new patch series.

Ok then. You can proceed with these changes. No problem from my side.

>
> Regards,
>
>        Hans


BR,
  
m7aalton Oct. 23, 2009, 7:45 a.m. UTC | #3
Hi.

I have written a driver for the TI WL1273 FM Radio but it's not yet
quite ready for up-streaming because of its interface. Now I've started
to change the interface to v4l2 and I'm following Eduardo Valentin's
Si4713 TX driver as an example. However, WL1273 radio has RX and TX so
there are things that Eduardo's driver doesn't cover. For example: the
driver needs a mode switch for switching between TX and RX. Should that
be implemented as an extended control or should there be a new IOCTL
added to the v4l2 API? etc...

Also I've added some things to the ivtv-radio tool. Should I try to
"up-stream" those as well?

Cheers, Matti A.



--
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
  
Hans Verkuil Nov. 5, 2009, 1:18 p.m. UTC | #4
On Friday 23 October 2009 09:45:53 m7aalton wrote:
> Hi.
> 
> I have written a driver for the TI WL1273 FM Radio but it's not yet
> quite ready for up-streaming because of its interface. Now I've started
> to change the interface to v4l2 and I'm following Eduardo Valentin's
> Si4713 TX driver as an example. However, WL1273 radio has RX and TX so
> there are things that Eduardo's driver doesn't cover. For example: the
> driver needs a mode switch for switching between TX and RX. Should that
> be implemented as an extended control or should there be a new IOCTL
> added to the v4l2 API? etc...

So if I understand this correctly, then this device can only transmit or
receive, but not both at the same time?

If that's the case, then I wonder if it isn't enough to let it depend on
whether VIDIOC_S_AUDOUT or VIDIOC_S_AUDIO was called last.

> Also I've added some things to the ivtv-radio tool. Should I try to
> "up-stream" those as well?

Perhaps we should take the opportunity to merge this tool as v4l2-radio
into the v4l2-apps/util directory of the master v4l-dvb repository.

It would be really nice if it can be used to test the transmitter
features and RDS receiver as well, i.e. extending and improving this tool.

I like having such fairly low-level and easy to debug utilities. They are
a great tool to verify the proper functioning of hardware like this.

Regards,

	Hans
  
m7aalton Nov. 5, 2009, 2:05 p.m. UTC | #5
On Thu, 2009-11-05 at 14:18 +0100, ext Hans Verkuil wrote:
> On Friday 23 October 2009 09:45:53 m7aalton wrote:
> > Hi.
> > 
> > I have written a driver for the TI WL1273 FM Radio but it's not yet
> > quite ready for up-streaming because of its interface. Now I've started
> > to change the interface to v4l2 and I'm following Eduardo Valentin's
> > Si4713 TX driver as an example. However, WL1273 radio has RX and TX so
> > there are things that Eduardo's driver doesn't cover. For example: the
> > driver needs a mode switch for switching between TX and RX. Should that
> > be implemented as an extended control or should there be a new IOCTL
> > added to the v4l2 API? etc...
> 
> So if I understand this correctly, then this device can only transmit or
> receive, but not both at the same time?

Yes, it cannot receive and transmit at the same time.

> If that's the case, then I wonder if it isn't enough to let it depend on
> whether VIDIOC_S_AUDOUT or VIDIOC_S_AUDIO was called last.

OK. That sounds fine.

> > Also I've added some things to the ivtv-radio tool. Should I try to
> > "up-stream" those as well?
> 
> Perhaps we should take the opportunity to merge this tool as v4l2-radio
> into the v4l2-apps/util directory of the master v4l-dvb repository.
> 
> It would be really nice if it can be used to test the transmitter
> features and RDS receiver as well, i.e. extending and improving this tool.

I have some code for testing rds reception, I'll include that or at
least a part of it to v4l2-radio...

> I like having such fairly low-level and easy to debug utilities. They are
> a great tool to verify the proper functioning of hardware like this.
> 
> Regards,
> 
> 	Hans

Thanks for comments,
Matti


--
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
  

Patch

diff --git a/linux/include/linux/videodev2.h b/linux/include/linux/videodev2.h
index b17898c..a86a575 100644
--- a/linux/include/linux/videodev2.h
+++ b/linux/include/linux/videodev2.h
@@ -817,6 +817,7 @@  struct v4l2_ext_controls {
 #define V4L2_CTRL_CLASS_USER 0x00980000	/* Old-style 'user' controls */
 #define V4L2_CTRL_CLASS_MPEG 0x00990000	/* MPEG-compression controls */
 #define V4L2_CTRL_CLASS_CAMERA 0x009a0000	/* Camera class controls */
+#define V4L2_CTRL_CLASS_FM_TX 0x009b0000	/* FM Modulator control class */
 
 #define V4L2_CTRL_ID_MASK      	  (0x0fffffff)
 #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
@@ -1156,6 +1157,39 @@  enum  v4l2_exposure_auto_type {
 
 #define V4L2_CID_PRIVACY			(V4L2_CID_CAMERA_CLASS_BASE+16)
 
+/* FM Modulator class control IDs */
+#define V4L2_CID_FM_TX_CLASS_BASE		(V4L2_CTRL_CLASS_FM_TX | 0x900)
+#define V4L2_CID_FM_TX_CLASS			(V4L2_CTRL_CLASS_FM_TX | 1)
+
+#define V4L2_CID_RDS_TX_DEVIATION		(V4L2_CID_FM_TX_CLASS_BASE + 1)
+#define V4L2_CID_RDS_TX_PI			(V4L2_CID_FM_TX_CLASS_BASE + 2)
+#define V4L2_CID_RDS_TX_PTY			(V4L2_CID_FM_TX_CLASS_BASE + 3)
+#define V4L2_CID_RDS_TX_PS_NAME			(V4L2_CID_FM_TX_CLASS_BASE + 4)
+#define V4L2_CID_RDS_TX_RADIO_TEXT		(V4L2_CID_FM_TX_CLASS_BASE + 5)
+
+#define V4L2_CID_AUDIO_LIMITER_ENABLED		(V4L2_CID_FM_TX_CLASS_BASE + 6)
+#define V4L2_CID_AUDIO_LIMITER_RELEASE_TIME	(V4L2_CID_FM_TX_CLASS_BASE + 7)
+#define V4L2_CID_AUDIO_LIMITER_DEVIATION	(V4L2_CID_FM_TX_CLASS_BASE + 8)
+
+#define V4L2_CID_AUDIO_COMPRESSION_ENABLED	(V4L2_CID_FM_TX_CLASS_BASE + 9)
+#define V4L2_CID_AUDIO_COMPRESSION_GAIN		(V4L2_CID_FM_TX_CLASS_BASE + 10)
+#define V4L2_CID_AUDIO_COMPRESSION_THRESHOLD	(V4L2_CID_FM_TX_CLASS_BASE + 11)
+#define V4L2_CID_AUDIO_COMPRESSION_ATTACK_TIME	(V4L2_CID_FM_TX_CLASS_BASE + 12)
+#define V4L2_CID_AUDIO_COMPRESSION_RELEASE_TIME	(V4L2_CID_FM_TX_CLASS_BASE + 13)
+
+#define V4L2_CID_PILOT_TONE_ENABLED		(V4L2_CID_FM_TX_CLASS_BASE + 14)
+#define V4L2_CID_PILOT_TONE_DEVIATION		(V4L2_CID_FM_TX_CLASS_BASE + 15)
+#define V4L2_CID_PILOT_TONE_FREQUENCY		(V4L2_CID_FM_TX_CLASS_BASE + 16)
+
+#define V4L2_CID_FM_TX_PREEMPHASIS		(V4L2_CID_FM_TX_CLASS_BASE + 17)
+enum v4l2_preemphasis {
+	V4L2_PREEMPHASIS_DISABLED	= 0,
+	V4L2_PREEMPHASIS_50_uS		= 1,
+	V4L2_PREEMPHASIS_75_uS		= 2,
+};
+#define V4L2_CID_TUNE_POWER_LEVEL		(V4L2_CID_FM_TX_CLASS_BASE + 18)
+#define V4L2_CID_TUNE_ANTENNA_CAPACITOR		(V4L2_CID_FM_TX_CLASS_BASE + 19)
+
 /*
  *	T U N I N G
  */