linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@redhat.com>
To: Andy Walls <awalls@md.metrocast.net>
Cc: Hans Verkuil <hverkuil@xs4all.nl>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Peter Korsgaard <jacmet@sunsite.dk>,
	Jean-Francois Moine <moinejf@free.fr>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	eduardo.valentin@nokia.com,
	ext Eino-Ville Talvala <talvala@stanford.edu>
Subject: Re: [PATCH] Illuminators and status LED controls
Date: Mon, 13 Sep 2010 11:38:03 -0300	[thread overview]
Message-ID: <4C8E374B.4050103@redhat.com> (raw)
In-Reply-To: <1284385792.2031.87.camel@morgan.silverblock.net>

Em 13-09-2010 10:49, Andy Walls escreveu:
> On Mon, 2010-09-13 at 08:45 -0300, Mauro Carvalho Chehab wrote:
>> Em 13-09-2010 05:06, Hans Verkuil escreveu:
>>> On Monday, September 13, 2010 09:04:18 Laurent Pinchart wrote:
>>>> Hi Hans,
>>>>
>>>> On Thursday 09 September 2010 13:48:58 Hans de Goede wrote:
>>>>> On 09/09/2010 03:29 PM, Hans Verkuil wrote:
>>>>>>> On 09/09/2010 08:55 AM, Peter Korsgaard wrote:
>>>>>>>> "Hans" == Hans Verkuil<hverkuil@xs4all.nl>   writes:
>>>>>>>>
>>>>>>>> I originally was in favor of controlling these through v4l as well, but
>>>>>>>> people made some good arguments against that. The main one being: why
>>>>>>>> would you want to show these as a control? What is the end user supposed
>>>>>>>> to do with them? It makes little sense.
>>>>
>>>> Status LEDs reflect in glasses, making annoying color dots on webcam pictures. 
>>>> That's why Logitech allows to turn the status LED off on its webcams.
>>>
>>> That's a really good argument. I didn't think of that one.
>>
>> There's one difference between illuminators and leds and anything else that we use
>> currently via CTRL interface: all other controls affects just an internal hardware
>> capability that are not visible to the user, nor can cause any kind of damage or 
>> annoyance.
>>
>> On the other hand, a LED and an illuminator that an application may forget to turn
>> off could be very annoying, and may eventually reduce the lifecycle or a device (in
>> the case of non-LED illuminators, for example).
> 
> Yes, I can appreciate that.  On driver unload and suspend that should
> certainly be the case for illuminators.
> 
> However, I don't think that's a good idea for final close on a file
> descriptor though.  That's a departure from normal V4L2 behavior.

This doesn't seem to be a good reason. Keeping a LED after its usage is annoying to 
the user, can cause damage on devices (reduce lifetime) and can draw lots of power 
from the batteries (on battery-powered devices).

> For a USB connected device, turning off the illuminator after the fact
> is simple, if the user has no other recourse: unplug the device. :)

Try to unplug the flash led on your cell phone ;)

>> So, a special treatment seems to be required for both cases: if the application that
>> changed the LED or illuminator to ON dies or closes, the LED/illuminator should be
>> turned off by the driver.
> 
> That will break cases like these:
> 
> $ v4l2-ctl -d /dev/video0 -c illuminator_2=1
> $ (command to run app that doesn't present all controls, e.g. cheese)

True, but it may have an alternative syntax for it, like:

v4l2-ctl -d /dev/video0 -c illuminator_2=1 --run cheese [<args>]

This way, if cheese or v4l2-ctl abends, the illuminator will be turned off.

Of course, we'll likely need to have a flag visible on userspace, to say that such
control resets to an "off" state when the application dies, to avoid someone to use it
like:
	v4l2-ctl -d /dev/video0 -c illuminator_2=1

Cheers,
Mauro

  reply	other threads:[~2010-09-13 14:38 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-07 20:33 [PATCH] Illuminators and status LED controls Andy Walls
2010-09-08  2:16 ` Eino-Ville Talvala
2010-09-08  7:59   ` Eduardo Valentin
2010-09-08 16:37     ` Andy Walls
2010-09-08 18:58       ` Peter Korsgaard
2010-09-08 19:27         ` Alex Deucher
2010-09-09  4:07           ` Andy Walls
2010-09-13  7:00           ` Laurent Pinchart
2010-09-09  6:07         ` Jean-Francois Moine
2010-09-09  6:25           ` Hans Verkuil
2010-09-09  6:55             ` Peter Korsgaard
2010-09-09 11:17               ` Hans de Goede
2010-09-09 13:29                 ` Hans Verkuil
2010-09-09 11:48                   ` Hans de Goede
2010-09-13  7:04                     ` Laurent Pinchart
2010-09-13  8:06                       ` Hans Verkuil
2010-09-13 11:45                         ` Mauro Carvalho Chehab
2010-09-13 13:49                           ` Andy Walls
2010-09-13 14:38                             ` Mauro Carvalho Chehab [this message]
2010-09-16 10:09                               ` Laurent Pinchart
2010-09-10 13:40           ` Andy Walls
  -- strict thread matches above, loose matches on Subject: below --
2010-09-09 14:41 Andy Walls
2010-09-09 13:17 ` Hans de Goede
2010-09-09 21:37   ` Andy Walls
2010-09-09 14:14 Andy Walls
2010-09-09 13:16 ` Hans de Goede
2010-09-09 14:01 Andy Walls
2010-09-09 14:17 ` Hans Verkuil
2010-09-09 19:26   ` Peter Korsgaard
2010-09-10  0:49   ` Andy Walls
2010-09-10  7:19     ` Peter Korsgaard
2010-09-10 13:30       ` Andy Walls
2010-09-07 16:35 Andy Walls
2010-09-06 18:11 Jean-Francois Moine
2010-09-07  7:16 ` Hans de Goede
2010-09-07  7:30 ` Hans Verkuil
2010-09-07  9:42   ` Hans de Goede
2010-09-07  9:44     ` Hans de Goede
2010-09-07  9:47       ` Hans Verkuil
2010-09-07 11:59         ` Hans de Goede
2010-09-07 14:50           ` Hans Verkuil
2010-09-07 13:04             ` Hans de Goede
2010-09-07 15:30               ` Hans Verkuil
2010-09-07 17:57                 ` Jean-Francois Moine
2010-09-07 18:42                   ` Hans Verkuil
2010-09-07 21:21                     ` Hans Verkuil
2010-09-07 22:29                       ` Theodore Kilgore
2010-09-08  5:17                         ` Hans Verkuil
2010-09-07 21:14                 ` Hans de Goede
2010-09-09  6:55                   ` Hans Verkuil
2010-09-09 11:15                     ` Hans de Goede
2010-09-09 13:38                       ` Hans Verkuil
2010-09-13  6:53                   ` Laurent Pinchart
2010-09-13  6:47         ` Laurent Pinchart
2010-09-13  6:59           ` Hans Verkuil
2010-09-07 19:12 ` Eduardo Valentin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C8E374B.4050103@redhat.com \
    --to=mchehab@redhat.com \
    --cc=awalls@md.metrocast.net \
    --cc=eduardo.valentin@nokia.com \
    --cc=hdegoede@redhat.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jacmet@sunsite.dk \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=moinejf@free.fr \
    --cc=talvala@stanford.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).