[PATCH] LED control

[PATCH] LED control

[Jean-Francois Moine]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

Some webcams have one or many lights (LED). This patch makes them
controlable by the applications.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

[-- Attachment #2: led.pat --]
[-- Type: application/octet-stream, Size: 1252 bytes --]

diff -r 1248509d8bed linux/include/linux/videodev2.h
--- a/linux/include/linux/videodev2.h	Sat Mar 14 08:44:42 2009 +0100
+++ b/linux/include/linux/videodev2.h	Sat Mar 14 12:53:00 2009 +0100
@@ -887,9 +887,10 @@
 	V4L2_COLORFX_BW		= 1,
 	V4L2_COLORFX_SEPIA	= 2,
 };
+#define V4L2_CID_LEDS				(V4L2_CID_BASE+32)
 
 /* last CID + 1 */
-#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+32)
+#define V4L2_CID_LASTP1				(V4L2_CID_BASE+33)
 
 /*  MPEG-class control IDs defined by V4L2 */
 #define V4L2_CID_MPEG_BASE 			(V4L2_CTRL_CLASS_MPEG | 0x900)
diff -r 1248509d8bed v4l2-spec/controls.sgml
--- a/v4l2-spec/controls.sgml	Sat Mar 14 08:44:42 2009 +0100
+++ b/v4l2-spec/controls.sgml	Sat Mar 14 12:53:00 2009 +0100
@@ -281,6 +281,13 @@
 <constant>V4L2_COLORFX_SEPIA</constant> (2).</entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_CID_LEDS</constant></entry>
+	    <entry>integer</entry>
+	    <entry>Switch on or off the LEDs or illuminators of the device.
+In the control value, each LED may be coded in one bit (0: off, 1: on) or in
+many bits (light intensity).</entry>
+	  </row>
+	  <row>
 	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
 	    <entry></entry>
 	    <entry>End of the predefined control IDs (currently

Re: [PATCH] LED control

[Mauro Carvalho Chehab]

On Sat, 14 Mar 2009 12:59:23 +0100
Jean-Francois Moine <moinejf@free.fr> wrote:

> +	    <entry><constant>V4L2_CID_LEDS</constant></entry>
> +	    <entry>integer</entry>
> +	    <entry>Switch on or off the LEDs or illuminators of the device.
> +In the control value, each LED may be coded in one bit (0: off, 1: on) or in
> +many bits (light intensity).</entry>
> +	  </row>
> +	  <row>

The idea of having some sort of control over the LEDs is interesting, but we
should have a better way of controlling it. If the LED may have more than one
bit, maybe the better would be to create more than one CID entry. Something like:

V4L2_CID_LED_POWER	- for showing that the camera is being used
V4L2_CID_LED_LIGHT	- for normal white light
V4L2_CID_LED_INFRARED	- for dark light, using infrared
...

This way a driver can enumberate what kind of leds are available, and get the
power intensity range for each individual one.

Cheers,
Mauro

Re: [PATCH] LED control

[Jean-Francois Moine]

On Sat, 14 Mar 2009 09:17:47 -0300
Mauro Carvalho Chehab <mchehab@infradead.org> wrote:

> On Sat, 14 Mar 2009 12:59:23 +0100
> Jean-Francois Moine <moinejf@free.fr> wrote:
> 
> > +	    <entry><constant>V4L2_CID_LEDS</constant></entry>
> > +	    <entry>integer</entry>
> > +	    <entry>Switch on or off the LEDs or illuminators of
> > the device. +In the control value, each LED may be coded in one bit
> > (0: off, 1: on) or in +many bits (light intensity).</entry>
> > +	  </row>
> > +	  <row>
> 
> The idea of having some sort of control over the LEDs is interesting,
> but we should have a better way of controlling it. If the LED may
> have more than one bit, maybe the better would be to create more than
> one CID entry. Something like:
> 
> V4L2_CID_LED_POWER	- for showing that the camera is being used
> V4L2_CID_LED_LIGHT	- for normal white light
> V4L2_CID_LED_INFRARED	- for dark light, using infrared
> ...
> 
> This way a driver can enumberate what kind of leds are available, and
> get the power intensity range for each individual one.

OK for V4L2_CID_LED_INFRARED: I already know a webcam type which needs
such a control, but I don't know if it is associated to a LED or to a
sensor capability.

I heard of some webcams with many LEDs (up to 6) and some of these last
ones may be independently switched on or off. But, actually, I don't
heard about individual or global LED power.

So, as I understand your controls, V4L2_CID_LED_LIGHT would contain a
bit array, one bit for each LED, and V4L2_CID_LED_POWER would give the
light intensity for all LEDs. This last control might be added later if
needed.

Am I right?

Cheers.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH] LED control

[Andy Walls]

On Sat, 2009-03-14 at 09:17 -0300, Mauro Carvalho Chehab wrote:
> On Sat, 14 Mar 2009 12:59:23 +0100
> Jean-Francois Moine <moinejf@free.fr> wrote:
> 
> > +	    <entry><constant>V4L2_CID_LEDS</constant></entry>
> > +	    <entry>integer</entry>
> > +	    <entry>Switch on or off the LEDs or illuminators of the device.
> > +In the control value, each LED may be coded in one bit (0: off, 1: on) or in
> > +many bits (light intensity).</entry>
> > +	  </row>
> > +	  <row>
> 
> The idea of having some sort of control over the LEDs is interesting, but we
> should have a better way of controlling it. If the LED may have more than one
> bit, maybe the better would be to create more than one CID entry. Something like:
> 
> V4L2_CID_LED_POWER	- for showing that the camera is being used
> V4L2_CID_LED_LIGHT	- for normal white light
> V4L2_CID_LED_INFRARED	- for dark light, using infrared
> ...
> 
> This way a driver can enumberate what kind of leds are available, and get the
> power intensity range for each individual one.

Just some random thought of mine of the subject of LED control:


I have an EVK board that has 4 LED's on it.  

They are red and of the on/off type. (Although I suppose someday someone
might install a dual color LED unit (red & green) or tri-color LED unit
(red yellow green) on a piece of equipment.)

They are controlled by GPIOs.

Right now the cx18 driver implicitly controls them when switching
between audio inputs, tuner, and radio.  However, I was going to
internally make an "LED Controller" v4l2_subdevice to handle them.


So I agree that *if* giving control of LEDs to the user application
supports valid use cases, then the API spec needs some more thought.

There are at least these uses for LED's in devices:

1. Simple indicator/display
2. Subject matter illumination for video/image capture (I'm guessing)
3. Area illimunation to provide a reference level to an on board sensor
(i.e. IR motion detection?)
4. Transmission of data (e.g IR remotes)


The first questions to answer in all of these cases

1. "Should this be controllable from userspace?"

2. "Should it be a user control?"
(When the whole sensor orientation feedback thing came up and Hans spoke
of a good "fit" in the API for sensor feedback, I concluded that there
isn't a terribly good place in the V4L2 API for sensor feedback,
miscellaneous input, or indicator status.  Nor is there a good way to
enumerate them.  Maybe I'm missing something.)

For a subcase of a display indicator to indicate device "power on", if
the driver can control that at all (they should be tied to the output of
a voltage regulator really) why would you want a use app to "lie" to the
user and say power is off?

OK.  That's the end of my random thoughts.

Regards,
Andy


> Cheers,
> Mauro

Re: [PATCH] LED control

[Trent Piepho]

On Sat, 14 Mar 2009, Mauro Carvalho Chehab wrote:
> On Sat, 14 Mar 2009 12:59:23 +0100
> Jean-Francois Moine <moinejf@free.fr> wrote:
>
> > +	    <entry><constant>V4L2_CID_LEDS</constant></entry>
> > +	    <entry>integer</entry>
> > +	    <entry>Switch on or off the LEDs or illuminators of the device.
> > +In the control value, each LED may be coded in one bit (0: off, 1: on) or in
> > +many bits (light intensity).</entry>
> > +	  </row>
> > +	  <row>
>
> The idea of having some sort of control over the LEDs is interesting, but we
> should have a better way of controlling it. If the LED may have more than one
> bit, maybe the better would be to create more than one CID entry. Something like:
>
> V4L2_CID_LED_POWER	- for showing that the camera is being used
> V4L2_CID_LED_LIGHT	- for normal white light
> V4L2_CID_LED_INFRARED	- for dark light, using infrared
> ...
>
> This way a driver can enumberate what kind of leds are available, and get the
> power intensity range for each individual one.

There is already a sysfs led interface, you could just have the driver
export the leds to the led subsystem and use that.

Re: [PATCH] LED control

[Jean-Francois Moine]

On Sat, 14 Mar 2009 13:16:11 -0700 (PDT)
Trent Piepho <xyzzy@speakeasy.org> wrote:

> There is already a sysfs led interface, you could just have the driver
> export the leds to the led subsystem and use that.

Yes, but:
- this asks to have a kernel generated with CONFIG_NEW_LEDS,
- the user must use some new program to access /sys/class/leds/<device>,
- he must know how the LEDs of his webcam are named in the /sys tree.

While, when the LEDs are handled by a simple control, the user may
quickly change all webcam parameters from a single program as v4l2-ctl.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH] LED control

[Laurent Pinchart]

On Sunday 15 March 2009 10:50:37 Jean-Francois Moine wrote:
> On Sat, 14 Mar 2009 13:16:11 -0700 (PDT)
>
> Trent Piepho <xyzzy@speakeasy.org> wrote:
> > There is already a sysfs led interface, you could just have the driver
> > export the leds to the led subsystem and use that.
>
> Yes, but:
> - this asks to have a kernel generated with CONFIG_NEW_LEDS,
> - the user must use some new program to access /sys/class/leds/<device>,
> - he must know how the LEDs of his webcam are named in the /sys tree.
>
> While, when the LEDs are handled by a simple control, the user may
> quickly change all webcam parameters from a single program as v4l2-ctl.

This would work if devices only exported a few simple LED controls. If we 
throw multi-color blinking LEDs in the mix we will need a complete LED API 
anyway.

Laurent Pinchart

Re: [PATCH] LED control

[Trent Piepho]

On Sun, 15 Mar 2009, Jean-Francois Moine wrote:
> On Sat, 14 Mar 2009 13:16:11 -0700 (PDT)
> Trent Piepho <xyzzy@speakeasy.org> wrote:
>
> > There is already a sysfs led interface, you could just have the driver
> > export the leds to the led subsystem and use that.
>
> Yes, but:
> - this asks to have a kernel generated with CONFIG_NEW_LEDS,

So?

> - the user must use some new program to access /sys/class/leds/<device>,

echo, cat?

> - he must know how the LEDs of his webcam are named in the /sys tree.

Just give them a name like video0:power and it will be easy enough to
associate them with the device.  I think links in sysfs would do it to,
/sys/class/video4linux/video0/device/<ledname> or something like that.

The advantage of using the led class is that you get support for triggers
and automatic blink functions, etc.

Re: [PATCH] LED control

[Jean-Francois Moine]

On Sun, 15 Mar 2009 08:14:56 -0700 (PDT)
Trent Piepho <xyzzy@speakeasy.org> wrote:

> > - this asks to have a kernel generated with CONFIG_NEW_LEDS,  
> 
> So?
>
> > - the user must use some new program to
> > access /sys/class/leds/<device>,  
> 
> echo, cat?
> 
> > - he must know how the LEDs of his webcam are named in the /sys
> > tree.  
> 
> Just give them a name like video0:power and it will be easy enough to
> associate them with the device.  I think links in sysfs would do it
> to, /sys/class/video4linux/video0/device/<ledname> or something like
> that.
> 
> The advantage of using the led class is that you get support for
> triggers and automatic blink functions, etc.

Sorry for I don't see the usage of blinking the LEDs for a webcam.

Again, using the led class makes me wonder about:

1) The end users.

Many Linux users don't know the kernel internal, nor which of the too
many applications they should use to make their devices work. If no
developper creates a tool to handle the webcams, the users have to
search again and again. Even if there is a full documentation, they
will try anything and eventually ask the kernel developpers why they
cannot have their LEDs switched on.

Actually, there are a few programs that can handle the webcam
parameters. In fact I know only 'v4l2-ctl': I did not succeeded to
compile qv4l2; v4l2ucp has been removed from Debian; and, anyway, these
programs don't handle the VIDIOC_G_JPEGCOMP and VIDIOC_S_JPEGCOMP
ioctls.

2) The memory overhead.

Using the led class adds more kernel code and asks the webcam drivers
to create a new device. Also, the function called for changing the LED
brighness cannot sleep, so the use a workqueue is required.

On contrary, with a webcam control, only one byte (for 8 LEDs) is added
to the webcam structure and the change is immediatly done in the ioctl.

3) The development time.

I already spent 2 hours to look how the led class was working. I am not
sure to develop a full LED mechanism in less than 5 to 8 hours, and, as
I cannot test it by myself, I will have to wait for testers to know if
the various protections (USB access, USB disconnection) work in all
cases.

Otherwise, adding a new control in a driver may be done in less than
half an hour, and, sure, it will work well at the first go!

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

[PATCH] LED control

[Jean-Francois Moine]

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]

Some media devices may have one or many lights (LEDs, illuminators,
lamps..). This patch makes them controlable by the applications.

Signed-off-by: Jean-Francois Moine <moinejf@free.fr>

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

[-- Attachment #2: led.patch --]
[-- Type: text/x-patch, Size: 1421 bytes --]

diff --git a/Documentation/DocBook/v4l/controls.xml b/Documentation/DocBook/v4l/controls.xml
index 8408caa..c9b8ca5 100644
--- a/Documentation/DocBook/v4l/controls.xml
+++ b/Documentation/DocBook/v4l/controls.xml
@@ -312,6 +312,13 @@ minimum value disables backlight compensation.</entry>
 	    information and bits 24-31 must be zero.</entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_CID_LEDS</constant></entry>
+	    <entry>integer</entry>
+	    <entry>Switch on or off the LED(s) or illuminator(s) of the device.
+	    The control type and values depend on the driver and may be either
+	    a single boolean (0: off, 1:on) or the index in a menu type.</entry>
+	  </row>
+	  <row>
 	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
 	    <entry></entry>
 	    <entry>End of the predefined control IDs (currently
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 61490c6..3807492 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1045,8 +1045,10 @@ enum v4l2_colorfx {
 
 #define V4L2_CID_CHROMA_GAIN                    (V4L2_CID_BASE+36)
 
+#define V4L2_CID_LEDS				(V4L2_CID_BASE+37)
+
 /* last CID + 1 */
-#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+37)
+#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+38)
 
 /*  MPEG-class control IDs defined by V4L2 */
 #define V4L2_CID_MPEG_BASE 			(V4L2_CTRL_CLASS_MPEG | 0x900)

Re: [PATCH] LED control

[Andy Walls]

On Sat, 2010-09-04 at 13:10 +0200, Jean-Francois Moine wrote:
> Some media devices may have one or many lights (LEDs, illuminators,
> lamps..). This patch makes them controlable by the applications.
> 
> Signed-off-by: Jean-Francois Moine <moinejf@free.fr>

Jean-Francois,

Thanks for the proposal.  It looks OK to me.

Acked-by: Andy Walls <awalls@md.metrocast.net>

Regards,
Andy

> diff --git a/Documentation/DocBook/v4l/controls.xml b/Documentation/DocBook/v4l/controls.xml
> index 8408caa..c9b8ca5 100644
> --- a/Documentation/DocBook/v4l/controls.xml
> +++ b/Documentation/DocBook/v4l/controls.xml
> @@ -312,6 +312,13 @@ minimum value disables backlight compensation.</entry>
>             information and bits 24-31 must be zero.</entry>
>           </row>
>           <row>
> +           <entry><constant>V4L2_CID_LEDS</constant></entry>
> +           <entry>integer</entry>
> +           <entry>Switch on or off the LED(s) or illuminator(s) of the device.
> +           The control type and values depend on the driver and may be either
> +           a single boolean (0: off, 1:on) or the index in a menu type.</entry>
> +         </row>
> +         <row>
>             <entry><constant>V4L2_CID_LASTP1</constant></entry>
>             <entry></entry>
>             <entry>End of the predefined control IDs (currently
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 61490c6..3807492 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1045,8 +1045,10 @@ enum v4l2_colorfx {
>  
>  #define V4L2_CID_CHROMA_GAIN                    (V4L2_CID_BASE+36)
>  
> +#define V4L2_CID_LEDS                          (V4L2_CID_BASE+37)
> +
>  /* last CID + 1 */
> -#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+37)
> +#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+38)
>  
>  /*  MPEG-class control IDs defined by V4L2 */
>  #define V4L2_CID_MPEG_BASE                     (V4L2_CTRL_CLASS_MPEG | 0x900)
> 

Re: [PATCH] LED control

[Hans de Goede]

Hi all,

On 09/04/2010 01:10 PM, Jean-Francois Moine wrote:
> Some media devices may have one or many lights (LEDs, illuminators,
> lamps..). This patch makes them controlable by the applications.
>
> Signed-off-by: Jean-Francois Moine<moinejf@free.fr>
>
> -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/
>
>
> led.patch
>
>
> diff --git a/Documentation/DocBook/v4l/controls.xml b/Documentation/DocBook/v4l/controls.xml
> index 8408caa..c9b8ca5 100644
> --- a/Documentation/DocBook/v4l/controls.xml
> +++ b/Documentation/DocBook/v4l/controls.xml
> @@ -312,6 +312,13 @@ minimum value disables backlight compensation.</entry>
>   	information and bits 24-31 must be zero.</entry>
>   	</row>
>   	<row>
> +	<entry><constant>V4L2_CID_LEDS</constant></entry>
> +	<entry>integer</entry>
> +	<entry>Switch on or off the LED(s) or illuminator(s) of the device.
> +	    The control type and values depend on the driver and may be either
> +	    a single boolean (0: off, 1:on) or the index in a menu type.</entry>
> +	</row>

I think that using one control for both status leds (which is what we are usually
talking about) and illuminator(s) is a bad idea. I'm fine with standardizing these,
but can we please have 2 CID's one for status lights and one for the led. Esp, as I
can easily see us supporting a microscope in the future where the microscope itself
or other devices with the same bridge will have a status led, so then we will need
2 separate controls anyways.

Regards,

Hans

Re: [PATCH] LED control

[Peter Korsgaard]

>>>>> "Hans" == Hans de Goede <hdegoede@redhat.com> writes:

Hi,

 >> +	<entry><constant>V4L2_CID_LEDS</constant></entry>
 >> +	<entry>integer</entry>
 >> +	<entry>Switch on or off the LED(s) or illuminator(s) of the device.
 >> +	    The control type and values depend on the driver and may be either
 >> +	    a single boolean (0: off, 1:on) or the index in a menu type.</entry>
 >> +	</row>

 Hans> I think that using one control for both status leds (which is
 Hans> what we are usually talking about) and illuminator(s) is a bad
 Hans> idea. I'm fine with standardizing these, but can we please have 2
 Hans> CID's one for status lights and one for the led. Esp, as I can
 Hans> easily see us supporting a microscope in the future where the
 Hans> microscope itself or other devices with the same bridge will have
 Hans> a status led, so then we will need 2 separate controls anyways.

Why does this need to go through the v4l2 api and not just use the
standard LED (sysfs) api in the first place?

-- 
Bye, Peter Korsgaard

Re: [PATCH] LED control

[Hans de Goede]

Hi,

On 09/05/2010 10:04 AM, Peter Korsgaard wrote:
>>>>>> "Hans" == Hans de Goede<hdegoede@redhat.com>  writes:
>
> Hi,
>
>   >>  +	<entry><constant>V4L2_CID_LEDS</constant></entry>
>   >>  +	<entry>integer</entry>
>   >>  +	<entry>Switch on or off the LED(s) or illuminator(s) of the device.
>   >>  +	    The control type and values depend on the driver and may be either
>   >>  +	    a single boolean (0: off, 1:on) or the index in a menu type.</entry>
>   >>  +	</row>
>
>   Hans>  I think that using one control for both status leds (which is
>   Hans>  what we are usually talking about) and illuminator(s) is a bad
>   Hans>  idea. I'm fine with standardizing these, but can we please have 2
>   Hans>  CID's one for status lights and one for the led. Esp, as I can
>   Hans>  easily see us supporting a microscope in the future where the
>   Hans>  microscope itself or other devices with the same bridge will have
>   Hans>  a status led, so then we will need 2 separate controls anyways.
>
> Why does this need to go through the v4l2 api and not just use the
> standard LED (sysfs) api in the first place?
>

Quoting from the reply by Jean-Francois Moine to a patch adding illuminator control
support to the cpia1 driver where this proposal is a result of:

###

As many gspca users are waiting for a light/LED/illuminator/lamp
control, I tried to define a standard one in March 2009:
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/3095

A second, but more restrictive, attempt was done by Németh Márton in
February 2010:
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/16705

The main objection to that proposals was that the sysfs LED interface
should be used instead:
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/3114

A patch in this way was done by Németh Márton in February 2010:
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/16670

but it was rather complex, and there was no consensus
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/17111

###

So using the sysfs interface for this is non trivial. Most cameras don't offer
any hardware dimming / blinking features, but we do want to do an auto setting
where the led goes on when streaming and goes off again when not streaming.
So the sysfs interface is not a good match to what we need.

A more important argument IMHO however is that the LED control is just one element
of many things one can control on  (some) webcams, things like focus, pan and tilt
for the more fancy ones also come into play. Not to mention contrast, brightness
etc. settings. Currently we have one central API for this which is the v4l2 ctrl
API, and we have several apps which dynamically build up a UI for this depending
on the ctrls advertised by the device. Adding a LED ctrl to the v4l2 API will
make this automatically show up in these apps and give the user one central place
to control everything related to the camera. Where as using the led sysfs API would
mean that the led control will basically stay invisible to the end user unless we
start patching all apps to also use support this API, requiring all v4l2 apps
to grow code to support a whole new api just to turn on / off a led does not
seem like a good idea to me.

Regards,

Hans

Re: [PATCH] LED control

[Andy Walls]

On Sun, 2010-09-05 at 10:04 +0200, Peter Korsgaard wrote:
> >>>>> "Hans" == Hans de Goede <hdegoede@redhat.com> writes:
> 
> Hi,
> 
>  >> +	<entry><constant>V4L2_CID_LEDS</constant></entry>
>  >> +	<entry>integer</entry>
>  >> +	<entry>Switch on or off the LED(s) or illuminator(s) of the device.
>  >> +	    The control type and values depend on the driver and may be either
>  >> +	    a single boolean (0: off, 1:on) or the index in a menu type.</entry>
>  >> +	</row>
> 
>  Hans> I think that using one control for both status leds (which is
>  Hans> what we are usually talking about) and illuminator(s) is a bad
>  Hans> idea. I'm fine with standardizing these, but can we please have 2
>  Hans> CID's one for status lights and one for the led. Esp, as I can
>  Hans> easily see us supporting a microscope in the future where the
>  Hans> microscope itself or other devices with the same bridge will have
>  Hans> a status led, so then we will need 2 separate controls anyways.
> 
> Why does this need to go through the v4l2 api and not just use the
> standard LED (sysfs) api in the first place?

It puts a larger burden on the application programmer.

Video capture applications are already programmed to provide controls to
the user using the V4L2 API for manipulating all aspects of the incoming
video image.  Using something different for turning on subject
illumination would be an exceptional case. 

The V4L2 control API also allows applications to be written such that
they need no apriori knowledge about a video driver's controls, and yet
can still present all driver supported controls to the user for
manipulation.  The VIDIOC_QUERYCTL interface allows applications to
discover controls and metadata to build a UI.  Two applications that
illustrate the point are 'qv4l2' (a Qt control GUI) and 'v4l2-ctl' (a
CLI control UI) found here:

http://git.linuxtv.org/v4l-utils.git?a=tree;f=utils;h=8d37309cc3b896d11fc77d9f275f82f02ee9c03d;hb=HEAD
 

As an example, here is the output of v4l2-ctl -L for my QX3 microscope:

$ v4l2-ctl -d /dev/video1 -L
                     brightness (int)  : min=0 max=100 step=1 default=50 value=50
                       contrast (int)  : min=0 max=96 step=8 default=48 value=48
                     saturation (int)  : min=0 max=100 step=1 default=50 value=50
         light_frequency_filter (menu) : min=0 max=2 default=1 value=1
				0: NoFliker
				1: 50 Hz
				2: 60 Hz
             compression_target (menu) : min=0 max=1 default=0 value=0
				0: Quality
				1: Framerate
                          lamps (menu) : min=0 max=3 default=0 value=0
				0: Off
				1: Bottom
				2: Top
				3: Both

And here is the output for my PVR-350 TV capture card:

$ v4l2-ctl -d /dev/video2 -L

User Controls

                     brightness (int)  : min=0 max=255 step=1 default=128 value=128 flags=slider
                       contrast (int)  : min=0 max=127 step=1 default=64 value=64 flags=slider
                     saturation (int)  : min=0 max=127 step=1 default=64 value=64 flags=slider
                            hue (int)  : min=-128 max=127 step=1 default=0 value=0 flags=slider
                         volume (int)  : min=0 max=65535 step=655 default=58880 value=58880 flags=slider
                           mute (bool) : default=0 value=0

MPEG Encoder Controls

                    stream_type (menu) : min=0 max=5 default=0 value=0 flags=update
				0: MPEG-2 Program Stream
				2: MPEG-1 System Stream
				3: MPEG-2 DVD-compatible Stream
				4: MPEG-1 VCD-compatible Stream
				5: MPEG-2 SVCD-compatible Stream
              stream_vbi_format (menu) : min=0 max=1 default=0 value=0
				0: No VBI
				1: Private packet, IVTV format
       audio_sampling_frequency (menu) : min=0 max=2 default=1 value=1
				0: 44.1 kHz
				1: 48 kHz
				2: 32 kHz
                 audio_encoding (menu) : min=1 max=1 default=1 value=1 flags=update
				1: MPEG-1/2 Layer II
         audio_layer_ii_bitrate (menu) : min=9 max=13 default=10 value=10
				9: 192 kbps
				10: 224 kbps
				11: 256 kbps
				12: 320 kbps
				13: 384 kbps
              audio_stereo_mode (menu) : min=0 max=3 default=0 value=0 flags=update
				0: Stereo
				1: Joint Stereo
				2: Dual
				3: Mono
    audio_stereo_mode_extension (menu) : min=0 max=3 default=0 value=0 flags=inactive
				0: Bound 4
				1: Bound 8
				2: Bound 12
				3: Bound 16
                 audio_emphasis (menu) : min=0 max=2 default=0 value=0
				0: No Emphasis
				1: 50/15 us
				2: CCITT J17
                      audio_crc (menu) : min=0 max=1 default=0 value=0
				0: No CRC
				1: 16-bit CRC
                     audio_mute (bool) : default=0 value=0
                 video_encoding (menu) : min=0 max=1 default=1 value=1 flags=readonly
				0: MPEG-1
				1: MPEG-2
                   video_aspect (menu) : min=0 max=3 default=1 value=1
				0: 1x1
				1: 4x3
				2: 16x9
				3: 2.21x1
                 video_b_frames (int)  : min=0 max=33 step=1 default=2 value=2 flags=update
                 video_gop_size (int)  : min=1 max=34 step=1 default=15 value=15
              video_gop_closure (bool) : default=1 value=1
             video_bitrate_mode (menu) : min=0 max=1 default=0 value=0 flags=update
				0: Variable Bitrate
				1: Constant Bitrate
                  video_bitrate (int)  : min=0 max=27000000 step=1 default=6000000 value=6000000
             video_peak_bitrate (int)  : min=0 max=27000000 step=1 default=8000000 value=8000000
      video_temporal_decimation (int)  : min=0 max=255 step=1 default=0 value=0
                     video_mute (bool) : default=0 value=0
                 video_mute_yuv (int)  : min=0 max=16777215 step=1 default=32896 value=32896
            spatial_filter_mode (menu) : min=0 max=1 default=0 value=0 flags=update
				0: Manual
				1: Auto
                 spatial_filter (int)  : min=0 max=15 step=1 default=0 value=0 flags=slider
       spatial_luma_filter_type (menu) : min=0 max=4 default=1 value=1
				0: Off
				1: 1D Horizontal
				2: 1D Vertical
				3: 2D H/V Separable
				4: 2D Symmetric non-separable
     spatial_chroma_filter_type (menu) : min=0 max=1 default=1 value=1
				0: Off
				1: 1D Horizontal
           temporal_filter_mode (menu) : min=0 max=1 default=0 value=0 flags=update
				0: Manual
				1: Auto
                temporal_filter (int)  : min=0 max=31 step=1 default=8 value=8 flags=slider
             median_filter_type (menu) : min=0 max=4 default=0 value=0 flags=update
				0: Off
				1: Horizontal
				2: Vertical
				3: Horizontal/Vertical
				4: Diagonal
     median_luma_filter_minimum (int)  : min=0 max=255 step=1 default=0 value=0 flags=inactive slider
     median_luma_filter_maximum (int)  : min=0 max=255 step=1 default=255 value=255 flags=inactive slider
   median_chroma_filter_minimum (int)  : min=0 max=255 step=1 default=0 value=0 flags=inactive slider
   median_chroma_filter_maximum (int)  : min=0 max=255 step=1 default=255 value=255 flags=inactive slider
      insert_navigation_packets (bool) : default=0 value=0



v4l2-ctl needed no hardcoded knowledge of what the respective drivers
(gspca_cpia1 and ivtv) supported to build a UI.  Nor did the v4l2-ctl
need to go rummaging about in sysfs to find which control applied to
which video device node, because the driver simply tell the app upon
request: "here are your options for controlling this device".  No
guesswork on where the control for this device may be located in sysfs,
and no exceptional application code to be written just to toggle an
illuminator on and off.

Regards,
Andy

Re: [PATCH] LED control

[Jean-Francois Moine]

On Sun, 05 Sep 2010 09:56:54 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> I think that using one control for both status leds (which is what we
> are usually talking about) and illuminator(s) is a bad idea. I'm fine
> with standardizing these, but can we please have 2 CID's one for
> status lights and one for the led. Esp, as I can easily see us
> supporting a microscope in the future where the microscope itself or
> other devices with the same bridge will have a status led, so then we
> will need 2 separate controls anyways.

Hi Hans,

I was not thinking about the status light (I do not see any other usage
for it), but well about illuminators which I saw only in microscopes.

So, which is the better name? V4L2_CID_LAMPS? V4L2_CID_ILLUMINATORS?

Cheers.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

Re: [PATCH] LED control

[Hans de Goede]

Hi,

On 09/05/2010 10:56 AM, Jean-Francois Moine wrote:
> On Sun, 05 Sep 2010 09:56:54 +0200
> Hans de Goede<hdegoede@redhat.com>  wrote:
>
>> I think that using one control for both status leds (which is what we
>> are usually talking about) and illuminator(s) is a bad idea. I'm fine
>> with standardizing these, but can we please have 2 CID's one for
>> status lights and one for the led. Esp, as I can easily see us
>> supporting a microscope in the future where the microscope itself or
>> other devices with the same bridge will have a status led, so then we
>> will need 2 separate controls anyways.
>
> Hi Hans,
>
> I was not thinking about the status light (I do not see any other usage
> for it), but well about illuminators which I saw only in microscopes.
>

Ah, ok thanks for clarifying. For some more on this see p.s. below.

> So, which is the better name? V4L2_CID_LAMPS? V4L2_CID_ILLUMINATORS?

I think that V4L2_CID_ILLUMINATORS together with a comment in the .h
and explanation in the spec that this specifically applies to microscopes
would be good.

Regards,

Hans

p.s.

I think it would be good to have a V4L2_CID_STATUS_LED too. In many drivers
we are explicitly controlling the led by register writes. Some people may very
well prefer the led to always be off. I know that uvc logitech cameras have
controls for the status led through the extended uvc controls. Once we have
a standardized LED control, we can move the logitech uvc cams over from
using their own private one to this one.

Once this is in place I would like to build some framework in to gspca
for supporting this control in gspca (the control would be handled by the core,
and sub drivers would have an sd_set_led function).

While at it could you write a proposal / patch for adding this control to the
spec as well ?

Re: [PATCH] LED control

[Andy Walls]

On Sun, 2010-09-05 at 15:54 +0200, Hans de Goede wrote:
> Hi,
> 
> On 09/05/2010 10:56 AM, Jean-Francois Moine wrote:
> > On Sun, 05 Sep 2010 09:56:54 +0200
> > Hans de Goede<hdegoede@redhat.com>  wrote:
> >
> >> I think that using one control for both status leds (which is what we
> >> are usually talking about) and illuminator(s) is a bad idea. I'm fine
> >> with standardizing these, but can we please have 2 CID's one for
> >> status lights and one for the led. Esp, as I can easily see us
> >> supporting a microscope in the future where the microscope itself or
> >> other devices with the same bridge will have a status led, so then we
> >> will need 2 separate controls anyways.
> >
> > Hi Hans,
> >
> > I was not thinking about the status light (I do not see any other usage
> > for it), but well about illuminators which I saw only in microscopes.
> >
> 
> Ah, ok thanks for clarifying. For some more on this see p.s. below.
> 
> > So, which is the better name? V4L2_CID_LAMPS? V4L2_CID_ILLUMINATORS?
> 
> I think that V4L2_CID_ILLUMINATORS together with a comment in the .h
> and explanation in the spec that this specifically applies to microscopes
> would be good.

I concur with ILLUMINATORS.  The word makes it very clear the control is
about actively putting light on a subject.  A quick Goggle search shows
that the term 'illuminator" appears to apply to photography and IR
cameras as well.


> Regards,
> 
> Hans
> 
> p.s.
> 
> I think it would be good to have a V4L2_CID_STATUS_LED too. In many drivers
> we are explicitly controlling the led by register writes. Some people may very
> well prefer the led to always be off. I know that uvc logitech cameras have
> controls for the status led through the extended uvc controls. Once we have
> a standardized LED control, we can move the logitech uvc cams over from
> using their own private one to this one.

I saw two use cases mentioned for status LEDs:

1. always off
2. driver automatically controls the LEDs.

Can't that choice be handled with a module option, is there a case where
one needs more control?

Regards,
Andy

> Once this is in place I would like to build some framework in to gspca
> for supporting this control in gspca (the control would be handled by the core,
> and sub drivers would have an sd_set_led function).
> 
> While at it could you write a proposal / patch for adding this control to the
> spec as well ?

Re: [PATCH] LED control

[Hans de Goede]

Hi,

On 09/05/2010 08:43 PM, Andy Walls wrote:
> On Sun, 2010-09-05 at 15:54 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 09/05/2010 10:56 AM, Jean-Francois Moine wrote:
>>> On Sun, 05 Sep 2010 09:56:54 +0200
>>> Hans de Goede<hdegoede@redhat.com>   wrote:
>>>
>>>> I think that using one control for both status leds (which is what we
>>>> are usually talking about) and illuminator(s) is a bad idea. I'm fine
>>>> with standardizing these, but can we please have 2 CID's one for
>>>> status lights and one for the led. Esp, as I can easily see us
>>>> supporting a microscope in the future where the microscope itself or
>>>> other devices with the same bridge will have a status led, so then we
>>>> will need 2 separate controls anyways.
>>>
>>> Hi Hans,
>>>
>>> I was not thinking about the status light (I do not see any other usage
>>> for it), but well about illuminators which I saw only in microscopes.
>>>
>>
>> Ah, ok thanks for clarifying. For some more on this see p.s. below.
>>
>>> So, which is the better name? V4L2_CID_LAMPS? V4L2_CID_ILLUMINATORS?
>>
>> I think that V4L2_CID_ILLUMINATORS together with a comment in the .h
>> and explanation in the spec that this specifically applies to microscopes
>> would be good.
>
> I concur with ILLUMINATORS.  The word makes it very clear the control is
> about actively putting light on a subject.  A quick Goggle search shows
> that the term 'illuminator" appears to apply to photography and IR
> cameras as well.
>
>
>> Regards,
>>
>> Hans
>>
>> p.s.
>>
>> I think it would be good to have a V4L2_CID_STATUS_LED too. In many drivers
>> we are explicitly controlling the led by register writes. Some people may very
>> well prefer the led to always be off. I know that uvc logitech cameras have
>> controls for the status led through the extended uvc controls. Once we have
>> a standardized LED control, we can move the logitech uvc cams over from
>> using their own private one to this one.
>
> I saw two use cases mentioned for status LEDs:
>
> 1. always off
> 2. driver automatically controls the LEDs.
>
> Can't that choice be handled with a module option

Sure, just like all other v4l2 controls could be a module option, that is
not very userfriendly though.

, is there a case where
> one needs more control?

Not really.

Regards,

Hans

Re: [PATCH] LED control

[Laurent Pinchart]

Hi Andy,

On Sunday 05 September 2010 20:43:27 Andy Walls wrote:
> On Sun, 2010-09-05 at 15:54 +0200, Hans de Goede wrote:
> > On 09/05/2010 10:56 AM, Jean-Francois Moine wrote:
> > > On Sun, 05 Sep 2010 09:56:54 +0200 Hans de Goede wrote:
> > >> I think that using one control for both status leds (which is what we
> > >> are usually talking about) and illuminator(s) is a bad idea. I'm fine
> > >> with standardizing these, but can we please have 2 CID's one for
> > >> status lights and one for the led. Esp, as I can easily see us
> > >> supporting a microscope in the future where the microscope itself or
> > >> other devices with the same bridge will have a status led, so then we
> > >> will need 2 separate controls anyways.
> > > 
> > > Hi Hans,
> > > 
> > > I was not thinking about the status light (I do not see any other usage
> > > for it), but well about illuminators which I saw only in microscopes.
> > 
> > Ah, ok thanks for clarifying. For some more on this see p.s. below.
> > 
> > > So, which is the better name? V4L2_CID_LAMPS? V4L2_CID_ILLUMINATORS?
> > 
> > I think that V4L2_CID_ILLUMINATORS together with a comment in the .h
> > and explanation in the spec that this specifically applies to microscopes
> > would be good.
> 
> I concur with ILLUMINATORS.  The word makes it very clear the control is
> about actively putting light on a subject.  A quick Goggle search shows
> that the term 'illuminator" appears to apply to photography and IR
> cameras as well.
> 
> > Regards,
> > 
> > Hans
> > 
> > p.s.
> > 
> > I think it would be good to have a V4L2_CID_STATUS_LED too. In many
> > drivers we are explicitly controlling the led by register writes. Some
> > people may very well prefer the led to always be off. I know that uvc
> > logitech cameras have controls for the status led through the extended
> > uvc controls. Once we have a standardized LED control, we can move the
> > logitech uvc cams over from using their own private one to this one.
> 
> I saw two use cases mentioned for status LEDs:
> 
> 1. always off
> 2. driver automatically controls the LEDs.
> 
> Can't that choice be handled with a module option, is there a case where
> one needs more control?

On Logitech UVC cameras, the status led can be set to

- always on
- always off
- controlled by the camera
- blinking (with a configurable frequency)

-- 
Regards,

Laurent Pinchart