From: Hans de Goede <hdegoede@redhat.com>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Hans Verkuil <hansverk@cisco.com>,
Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: Some comments on the new autocluster patches
Date: Tue, 26 Jul 2011 15:51:58 +0200 [thread overview]
Message-ID: <4E2EC67E.6010300@redhat.com> (raw)
In-Reply-To: <201107261126.22285.hverkuil@xs4all.nl>
Hi,
On 07/26/2011 11:26 AM, Hans Verkuil wrote:
> OK, I'm back to work after my vacation, so it's time to go through the
> backlog...
>
Welcome back :)
> On Tuesday, July 12, 2011 15:25:02 Hans de Goede wrote:
>> Hi,
>>
>> On 07/04/2011 11:43 AM, Hans Verkuil wrote:
<snip snip>
>>> It is relevant. Take an application that saves the current state of all
>>> controls and restores it the next time it is started. If you report the
>>> device's autogain value instead of the manual gain, then that manual gain
>>> value is lost. I consider this a major drawback.
>>
>> If autogain is on, then the gain is RO, so it should not be saved. Let alone
>> restored.
>
> Marking gain as inactive is fine, but marking it as read-only is not so clear.
> Currently the RO flag is static. This allows control panels to use e.g. a text
> field instead of an input field to show the value.
>
> I would like to keep that functionality. If we make the RO flag dynamic, then
> GUIs won't know whether to show it as a disabled input field or as a text field.
>
> Whereas with the inactive flag they will know that it has to be a disabled
> input field.
>
Agreed, where I wrote read only I meant inactive, which does make it less clear
that the control should not be saved / restored by a save / restore app.
> When the inactive flag is set, it is still allowed to set the value. However,
> if we add a volatile flag as well, then we may want to have the combination
> 'inactive and volatile' return an error when an application attempts to set the
> value.
I think that is a good solution to indicate dynamic-readonly ness (more or less),
and thus to indicate that the control should not be written (and thus not saved/
restored).
> Or is this too complex and should we just discard the value in a case like that?
I would prefer returning an error, so that things don't silently fail, also
unless we actually return an error many apps are likely to get this wrong.
<snip snip>
>> I still believe that everything boils down to 2 possible scenarios,
>> and the rest follows from that. With the 2 scenarios being:
>>
>> 1) There is a manual setting which is constant until explicitly
>> changed, when (ie) gain switches from auto mode to manual mode
>> then the actual used gain is reset to this manual setting
>>
>> 2) There is a single gain setting / register, which is r/w when the
>> control is in manual mode and ro when in auto mode. When auto mode
>> gets switched off, the gain stays at the last value set by auto mode.
>>
>> 2) Is what most webcam sensors (and the pwc firmware) implement at
>> the hardware level, and what to me also makes the most sense for webcams.
>>
>> To me this whole discussion centers around these 2 scenarios, with you
>> being a proponent of 1), and I guess that for video capture boards 1 makes
>> a lot of sense, and me being a proponent of 2.
>>
>> Proposal: lets agree that these 2 methods of handling autofoo controls
>> both exist and both have merits in certain cases, this means letting
>> it be up to the driver to choose which method to implement.
>
> OK.
>
>> If we can agree on this, then the next step would be to document both
>> methods, as well as how the controls should behave in either scenario.
>> I'm willing to write up a first draft for this.
>
> I can do that as well, see below.
Ah great, you just saved me some work I always like it when people
save me work :)
<snip snip>
>> I think we need to agree that we disagree :)
>
> Actually, I agree with much of what you wrote :-)
>
Good :)
> OK, so we have two scenarios:
>
> 1) There is a manual setting which is constant until explicitly changed, when e.g.
> gain switches from auto mode to manual mode then the actual used gain is reset to
> this manual setting.
>
> In this case the e.g. gain control is *not* marked volatile, but just inactive.
> If the hardware can return the gain as set by the autogain circuit, then that has
> to be exported as a separate read-only control (e.g. 'Current Gain').
>
>
> 2) There is a single gain setting / register, which is active when the control is in
> manual mode and inactive and volatile when in auto mode. When auto mode gets switched
> off, the gain stays at the last value set by auto mode.
>
> This scenario is only possible, of course, if you can obtain the gain value as set
> by the autogain circuitry.
>
I fully agree with the above, +1
> An open question is whether writing to an inactive and volatile control should return
> an error or not.
I would prefer an error return.
> Webcams should follow scenario 2 (if possible).
>
> It is less obvious what to recommend for video capture devices. I'd leave it up to
> the driver for now.
Sounds good to me.
Regards,
Hans
next prev parent reply other threads:[~2011-07-26 13:50 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-01 15:06 Some comments on the new autocluster patches Hans de Goede
2011-07-01 16:21 ` Hans Verkuil
2011-07-02 10:28 ` Hans de Goede
2011-07-02 11:10 ` Hans Verkuil
2011-07-02 14:31 ` Hans de Goede
2011-07-04 9:43 ` Hans Verkuil
2011-07-12 13:25 ` Hans de Goede
2011-07-26 9:26 ` Hans Verkuil
2011-07-26 13:51 ` Hans de Goede [this message]
2011-07-26 14:19 ` Hans Verkuil
2011-07-26 14:38 ` Hans de Goede
2011-07-26 14:39 ` Hans Verkuil
2011-07-02 0:55 ` Mauro Carvalho Chehab
2011-07-02 9:36 ` Hans Verkuil
2011-07-02 18:33 ` Mauro Carvalho Chehab
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=4E2EC67E.6010300@redhat.com \
--to=hdegoede@redhat.com \
--cc=hansverk@cisco.com \
--cc=hverkuil@xs4all.nl \
--cc=linux-media@vger.kernel.org \
/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