All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.