Re: [PATCH 1/3] add feedback LED control
Jean-Francois Moine wrote: > On Sun, 28 Feb 2010 20:38:00 +0100 > Németh Márton wrote: > >> With a bitfield on and off state can be specified. What about the >> "auto" mode? Should two bits grouped together to have auto, on and >> off state? Is there already a similar control? >> >> Is the brightness of the background light LEDs adjustable or are they >> just on/off? If yes, then maybe the feedback LEDs and the background >> light LEDs should be treated as different kind. > > OK. My idea about switching the LEDs by v4l2 controls was not good. So, > forget about it. > > Instead, some job of the led class may be done in the gspca main, > especially register/unregister. > > I propose to add a LED description in the gspca structure (level > 'struct cam'). There would be 'nleds' for the number of LEDS and > 'leds', a pointer to an array of: > > struct gspca_led { > struct led_classdev led_cdev; > char led_name[32]; > struct led_trigger led_trigger; > char trigger_name[32]; > }; > > (this array should be in the subdriver structure - I don't show the > #ifdef's) > > Then, this would work as: > > - on probe, in the 'config' function of the subdriver, this one > initializes the led and trigger fields. The 'led_cdev.name' and > 'led_trigger.name' should point to a sprintf format with one > argument: the video device number (ex: "video%d::toplight"). > > - then, at the end of gspca_dev_probe(), the gspca main creates the real > names of the leds and triggers, and does the register job. > > - all led/trigger events are treated by the subdriver, normally by a > workqueue. This one must not be the system workqueue. > > - on disconnection, the gspca main unregisters the leds and triggers > without calling the subdriver. In the workqueue, the disconnection > can be simply handled testing the flag 'present' after each subsystem > call. > > Cheers. > The feedback LED patch series doesn't apply anymore. patching file Documentation/DocBook/v4l/controls.xml Hunk #1 succeeded at 324 (offset 13 lines). patching file Documentation/DocBook/v4l/videodev2.h.xml Hunk #1 succeeded at 1031 (offset 7 lines). patching file drivers/media/video/v4l2-common.c Hunk #1 succeeded at 358 (offset 9 lines). Hunk #3 FAILED at 442. Hunk #4 succeeded at 598 (offset 14 lines). 1 out of 4 hunks FAILED -- saving rejects to file drivers/media/video/v4l2-common.c.rej patching file include/linux/videodev2.h Hunk #1 FAILED at 1030. 1 out of 1 hunk FAILED -- saving rejects to file include/linux/videodev2.h.rej >>> Patch patches/lmml_82773_1_3_add_feedback_led_control.patch doesn't apply As the original approach weren't accepted, I'm dropping those patches from my queue. Please re-submit after having some agreement about that. -- Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] add feedback LED control
On Sun, 28 Feb 2010 20:38:00 +0100 Németh Márton wrote: > With a bitfield on and off state can be specified. What about the > "auto" mode? Should two bits grouped together to have auto, on and > off state? Is there already a similar control? > > Is the brightness of the background light LEDs adjustable or are they > just on/off? If yes, then maybe the feedback LEDs and the background > light LEDs should be treated as different kind. OK. My idea about switching the LEDs by v4l2 controls was not good. So, forget about it. Instead, some job of the led class may be done in the gspca main, especially register/unregister. I propose to add a LED description in the gspca structure (level 'struct cam'). There would be 'nleds' for the number of LEDS and 'leds', a pointer to an array of: struct gspca_led { struct led_classdev led_cdev; char led_name[32]; struct led_trigger led_trigger; char trigger_name[32]; }; (this array should be in the subdriver structure - I don't show the #ifdef's) Then, this would work as: - on probe, in the 'config' function of the subdriver, this one initializes the led and trigger fields. The 'led_cdev.name' and 'led_trigger.name' should point to a sprintf format with one argument: the video device number (ex: "video%d::toplight"). - then, at the end of gspca_dev_probe(), the gspca main creates the real names of the leds and triggers, and does the register job. - all led/trigger events are treated by the subdriver, normally by a workqueue. This one must not be the system workqueue. - on disconnection, the gspca main unregisters the leds and triggers without calling the subdriver. In the workqueue, the disconnection can be simply handled testing the flag 'present' after each subsystem call. Cheers. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] add feedback LED control
On Sunday 28 February 2010 20:38:00 Németh Márton wrote: > Jean-Francois Moine wrote: > > On Sun, 28 Feb 2010 08:55:04 +0100 > > > > Németh Márton wrote: > >> On some webcams a feedback LED can be found. This LED usually shows > >> the state of streaming mode: this is the "Auto" mode. The LED can > >> be programmed to constantly switched off state (e.g. for power saving > >> reasons, preview mode) or on state (e.g. the application shows motion > >> detection or "on-air"). > > > > Hi, > > > > There may be many LEDs on the webcams. One LED may be used for > > the streaming state, Some other ones may be used to give more light in > > dark rooms. One webcam, the microscope 093a:050f, has a top and a bottom > > lights/illuminators; an other one, the MSI StarCam 370i 0c45:60c0, has > > an infra-red light. > > > > That's why I proposed to have bit fields in the control value to switch > > on/off each LED. > > With a bitfield on and off state can be specified. What about the "auto" > mode? Should two bits grouped together to have auto, on and off state? Is > there already a similar control? I don't like the bitfield either. As stated in my previous mail, we can have more than 3 states, so using 2 bits per LED will simply not scale. > Is the brightness of the background light LEDs adjustable or are they just > on/off? If yes, then maybe the feedback LEDs and the background light LEDs > should be treated as different kind. I think there should indeed be a different control for the background LEDs. Still, there could be more than one feedback LED. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] add feedback LED control
Hi Márton, On Sunday 28 February 2010 08:55:04 Németh Márton wrote: > From: Márton Németh > > On some webcams a feedback LED can be found. This LED usually shows > the state of streaming mode: this is the "Auto" mode. The LED can > be programmed to constantly switched off state (e.g. for power saving > reasons, preview mode) or on state (e.g. the application shows motion > detection or "on-air"). > > Signed-off-by: Márton Németh > --- > diff -r d8fafa7d88dc linux/Documentation/DocBook/v4l/controls.xml > --- a/linux/Documentation/DocBook/v4l/controls.xmlThu Feb 18 19:02:51 2010 > +0100 +++ b/linux/Documentation/DocBook/v4l/controls.xml Sun Feb 28 > 08:41:17 2010 +0100 @@ -311,6 +311,17 @@ > Applications depending on particular custom controls should check the > driver name and version, see . > > + > + V4L2_CID_LED > + enum > + Controls the feedback LED on the camera. In auto mode > +the LED is on when the streaming is active. The LED is off when not > streaming. +The LED can be also turned on and off independent from > streaming. +Possible values for enum v4l2_led are: > +V4L2_CID_LED_AUTO (0), > +V4L2_CID_LED_ON (1) and > +V4L2_CID_LED_OFF (2). > + There's more than just auto, on and off. On Logitech webcams, LEDs can be configured to blink as well. > > > > diff -r d8fafa7d88dc linux/Documentation/DocBook/v4l/videodev2.h.xml > --- a/linux/Documentation/DocBook/v4l/videodev2.h.xml Thu Feb 18 19:02:51 > 2010 +0100 +++ b/linux/Documentation/DocBook/v4l/videodev2.h.xml Sun Feb > 28 08:41:17 2010 +0100 @@ -1024,8 +1024,14 @@ > > #define V4L2_CID_ROTATE (V4L2_CID_BASE+34) > #define V4L2_CID_BG_COLOR (V4L2_CID_BASE+35) > +#define V4L2_CID_LED(V4L2_CID_BASE+36) > +enum v4l2_led { > +V4L2_LED_AUTO = 0, > +V4L2_LED_ON = 1, > +V4L2_LED_OFF= 2, > +}; > /* last CID + 1 */ > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+36) > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+37) > > /* MPEG-class control IDs defined by V4L2 */ > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | > 0x900) diff -r d8fafa7d88dc linux/drivers/media/video/v4l2-common.c > --- a/linux/drivers/media/video/v4l2-common.c Thu Feb 18 19:02:51 2010 > +0100 +++ b/linux/drivers/media/video/v4l2-common.c Sun Feb 28 08:41:17 > 2010 +0100 @@ -349,6 +349,12 @@ > "75 useconds", > NULL, > }; > + static const char *led[] = { > + "Auto", > + "On", > + "Off", > + NULL, > + }; > > switch (id) { > case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: > @@ -389,6 +395,8 @@ > return colorfx; > case V4L2_CID_TUNE_PREEMPHASIS: > return tune_preemphasis; > + case V4L2_CID_LED: > + return led; > default: > return NULL; > } > @@ -434,6 +442,7 @@ > case V4L2_CID_COLORFX: return "Color Effects"; > case V4L2_CID_ROTATE: return "Rotate"; > case V4L2_CID_BG_COLOR: return "Background color"; > + case V4L2_CID_LED: return "Feedback LED"; > > /* MPEG controls */ > case V4L2_CID_MPEG_CLASS: return "MPEG Encoder Controls"; > @@ -575,6 +584,7 @@ > case V4L2_CID_EXPOSURE_AUTO: > case V4L2_CID_COLORFX: > case V4L2_CID_TUNE_PREEMPHASIS: > + case V4L2_CID_LED: > qctrl->type = V4L2_CTRL_TYPE_MENU; > step = 1; > break; > diff -r d8fafa7d88dc linux/include/linux/videodev2.h > --- a/linux/include/linux/videodev2.h Thu Feb 18 19:02:51 2010 +0100 > +++ b/linux/include/linux/videodev2.h Sun Feb 28 08:41:17 2010 +0100 > @@ -1030,8 +1030,14 @@ > > #define V4L2_CID_ROTATE (V4L2_CID_BASE+34) > #define V4L2_CID_BG_COLOR(V4L2_CID_BASE+35) > +#define V4L2_CID_LED (V4L2_CID_BASE+36) > +enum v4l2_led { > + V4L2_LED_AUTO = 0, > + V4L2_LED_ON = 1, > + V4L2_LED_OFF= 2, > +}; enums shouldn't be used in kernelspace <-> userspace APIs, as their size is not stable across ARM ABIs. In this case it won't matter much, the enum not being part of any structure. If someone creates a new ioctl that uses enum v4l2_led as one field of its structure argument, things will break. > /* last CID + 1 */ > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+36) > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+37) > > /* MPEG-class control IDs defined by V4L2 */ > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) -- Regards, Laurent Pinchart -- To
Re: [PATCH 1/3] add feedback LED control
Jean-Francois Moine wrote: > On Sun, 28 Feb 2010 08:55:04 +0100 > Németh Márton wrote: > >> On some webcams a feedback LED can be found. This LED usually shows >> the state of streaming mode: this is the "Auto" mode. The LED can >> be programmed to constantly switched off state (e.g. for power saving >> reasons, preview mode) or on state (e.g. the application shows motion >> detection or "on-air"). > > Hi, > > There may be many LEDs on the webcams. One LED may be used for > the streaming state, Some other ones may be used to give more light in > dark rooms. One webcam, the microscope 093a:050f, has a top and a bottom > lights/illuminators; an other one, the MSI StarCam 370i 0c45:60c0, has > an infra-red light. > > That's why I proposed to have bit fields in the control value to switch > on/off each LED. With a bitfield on and off state can be specified. What about the "auto" mode? Should two bits grouped together to have auto, on and off state? Is there already a similar control? Is the brightness of the background light LEDs adjustable or are they just on/off? If yes, then maybe the feedback LEDs and the background light LEDs should be treated as different kind. Regards, Márton Németh -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] add feedback LED control
On Sun, 28 Feb 2010 08:55:04 +0100 Németh Márton wrote: > On some webcams a feedback LED can be found. This LED usually shows > the state of streaming mode: this is the "Auto" mode. The LED can > be programmed to constantly switched off state (e.g. for power saving > reasons, preview mode) or on state (e.g. the application shows motion > detection or "on-air"). Hi, There may be many LEDs on the webcams. One LED may be used for the streaming state, Some other ones may be used to give more light in dark rooms. One webcam, the microscope 093a:050f, has a top and a bottom lights/illuminators; an other one, the MSI StarCam 370i 0c45:60c0, has an infra-red light. That's why I proposed to have bit fields in the control value to switch on/off each LED. Cheers. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] add feedback LED control
From: Márton Németh On some webcams a feedback LED can be found. This LED usually shows the state of streaming mode: this is the "Auto" mode. The LED can be programmed to constantly switched off state (e.g. for power saving reasons, preview mode) or on state (e.g. the application shows motion detection or "on-air"). Signed-off-by: Márton Németh --- diff -r d8fafa7d88dc linux/Documentation/DocBook/v4l/controls.xml --- a/linux/Documentation/DocBook/v4l/controls.xml Thu Feb 18 19:02:51 2010 +0100 +++ b/linux/Documentation/DocBook/v4l/controls.xml Sun Feb 28 08:41:17 2010 +0100 @@ -311,6 +311,17 @@ Applications depending on particular custom controls should check the driver name and version, see . + + V4L2_CID_LED + enum + Controls the feedback LED on the camera. In auto mode +the LED is on when the streaming is active. The LED is off when not streaming. +The LED can be also turned on and off independent from streaming. +Possible values for enum v4l2_led are: +V4L2_CID_LED_AUTO (0), +V4L2_CID_LED_ON (1) and +V4L2_CID_LED_OFF (2). + diff -r d8fafa7d88dc linux/Documentation/DocBook/v4l/videodev2.h.xml --- a/linux/Documentation/DocBook/v4l/videodev2.h.xml Thu Feb 18 19:02:51 2010 +0100 +++ b/linux/Documentation/DocBook/v4l/videodev2.h.xml Sun Feb 28 08:41:17 2010 +0100 @@ -1024,8 +1024,14 @@ #define V4L2_CID_ROTATE (V4L2_CID_BASE+34) #define V4L2_CID_BG_COLOR (V4L2_CID_BASE+35) +#define V4L2_CID_LED(V4L2_CID_BASE+36) +enum v4l2_led { +V4L2_LED_AUTO = 0, +V4L2_LED_ON = 1, +V4L2_LED_OFF= 2, +}; /* last CID + 1 */ -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+36) +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+37) /* MPEG-class control IDs defined by V4L2 */ #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) diff -r d8fafa7d88dc linux/drivers/media/video/v4l2-common.c --- a/linux/drivers/media/video/v4l2-common.c Thu Feb 18 19:02:51 2010 +0100 +++ b/linux/drivers/media/video/v4l2-common.c Sun Feb 28 08:41:17 2010 +0100 @@ -349,6 +349,12 @@ "75 useconds", NULL, }; + static const char *led[] = { + "Auto", + "On", + "Off", + NULL, + }; switch (id) { case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: @@ -389,6 +395,8 @@ return colorfx; case V4L2_CID_TUNE_PREEMPHASIS: return tune_preemphasis; + case V4L2_CID_LED: + return led; default: return NULL; } @@ -434,6 +442,7 @@ case V4L2_CID_COLORFX: return "Color Effects"; case V4L2_CID_ROTATE: return "Rotate"; case V4L2_CID_BG_COLOR: return "Background color"; + case V4L2_CID_LED: return "Feedback LED"; /* MPEG controls */ case V4L2_CID_MPEG_CLASS: return "MPEG Encoder Controls"; @@ -575,6 +584,7 @@ case V4L2_CID_EXPOSURE_AUTO: case V4L2_CID_COLORFX: case V4L2_CID_TUNE_PREEMPHASIS: + case V4L2_CID_LED: qctrl->type = V4L2_CTRL_TYPE_MENU; step = 1; break; diff -r d8fafa7d88dc linux/include/linux/videodev2.h --- a/linux/include/linux/videodev2.h Thu Feb 18 19:02:51 2010 +0100 +++ b/linux/include/linux/videodev2.h Sun Feb 28 08:41:17 2010 +0100 @@ -1030,8 +1030,14 @@ #define V4L2_CID_ROTATE(V4L2_CID_BASE+34) #define V4L2_CID_BG_COLOR (V4L2_CID_BASE+35) +#define V4L2_CID_LED (V4L2_CID_BASE+36) +enum v4l2_led { + V4L2_LED_AUTO = 0, + V4L2_LED_ON = 1, + V4L2_LED_OFF= 2, +}; /* last CID + 1 */ -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+36) +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+37) /* MPEG-class control IDs defined by V4L2 */ #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html