From: Hans de Goede <hdegoede@redhat.com>
To: "'Sakari Ailus'" <sakari.ailus@iki.fi>
Cc: "HeungJun, Kim" <riverful.kim@samsung.com>,
"'Laurent Pinchart'" <laurent.pinchart@ideasonboard.com>,
"'Sylwester Nawrocki'" <snjw23@gmail.com>,
linux-media@vger.kernel.org, mchehab@redhat.com,
hverkuil@xs4all.nl, kyungmin.park@samsung.com
Subject: Re: [RFC PATCH 1/4] v4l: Add V4L2_CID_PRESET_WHITE_BALANCE menu control
Date: Fri, 30 Dec 2011 19:56:39 +0100 [thread overview]
Message-ID: <4EFE0967.6020904@redhat.com> (raw)
In-Reply-To: <20111230184212.GW3677@valkosipuli.localdomain>
Hi,
On 12/30/2011 07:42 PM, 'Sakari Ailus' wrote:
> Hi Hans,
>
> On Fri, Dec 30, 2011 at 09:41:57AM +0100, Hans de Goede wrote:
> ...
>> Right, so the above is exactly why I ended up making the pwc whitebalance
>> control the way it is, the user can essentially choice between a number
>> of options:
>> 1) auto whitebal
>> 2) a number of preset whitebal values (seems your proposal has some more then the pwc
>> driver, which is fine)
>> 3) manual whitebal, at which point the user may set whitebal through one of:
>> a) a color temperature control
>> b) red and blue balance controls
>> c) red, green and blue gains
>
> d) red, green, blue and... another green. This is how some raw bayer sensors
> can be controlled.
>
Yes, but have you ever tried setting the 2 green gains to different values?
(Hint the result is not pretty) I strongly believe the 2 separate green gains are
only there as it is easier to do things that way (it keeps the sensor array symmetric) and
there is no value in controlling them separately.
>> Notice that we also need to add some standardized controls for the 3c case, but that
>> is a different discussion.
>
> I was planning to add the gain to low-level controls once the other sensor
> controls have been properly discussed, and hopefully approved, first.
>
>> Seeing how this discussion has evolved I believe that what I did in the pwc driver
>> is actually right from the user pov, the user gets one simple menu control which
>> allows the user to choice between auto / preset 1 - x / manual and since as
>> described above choosing one of the options excludes the other options from being
>> active I believe having this all in one control is the right thing to do.
>
> I still think automatic white balance should be separate from the rest, as
> it currently is (V4L2_CID_AUTO_WHITE_BALANCE). If such presets aren't
> available, the way the user would enable automatic white balance changes,
> which is bad.
Well we can just put in the spec that the control can be either a boolean or
a menu control depending on if it has presets. I really believe that we should
not make this 2 separate controls, it does not match from a user pov.
There are 2 things which the user wants to control here:
1) How the value of the white balance controls gets controlled, which is
one of: a) auto b) select values from preset 1 - # c) manaul
2) The white balance controls themselves (temperature, or color balances ...),
but only if 1) is set to manual
There is no reason to separate 1) in 2 separate controls, that will only serve
to confuse the user.
As for the theoretical automatic wb which takes some hints like the presets,
AFAIK no such device exists and it is unlikely one will show up in the near
future. IOW it is purely theoretical and thus not interesting.
Regards,
Hans
next prev parent reply other threads:[~2011-12-30 18:55 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-28 6:23 [RFC PATCH 0/4] Add some new camera controls HeungJun, Kim
2011-12-28 6:23 ` [RFC PATCH 1/4] v4l: Add V4L2_CID_PRESET_WHITE_BALANCE menu control HeungJun, Kim
2011-12-28 13:35 ` Sylwester Nawrocki
2011-12-28 13:51 ` Laurent Pinchart
2011-12-29 5:08 ` HeungJun, Kim
2011-12-29 23:58 ` Laurent Pinchart
2011-12-30 5:21 ` Kim, Heungjun
2011-12-30 10:30 ` Sylwester Nawrocki
2012-01-02 4:38 ` Kim, Heungjun
2012-01-02 21:50 ` Sylwester Nawrocki
2011-12-29 23:34 ` Sakari Ailus
2011-12-30 6:35 ` HeungJun, Kim
2011-12-30 8:41 ` Hans de Goede
2011-12-30 18:42 ` 'Sakari Ailus'
2011-12-30 18:56 ` Hans de Goede [this message]
2011-12-30 21:03 ` 'Sakari Ailus'
2011-12-30 18:17 ` 'Sakari Ailus'
2011-12-30 10:14 ` Sylwester Nawrocki
2011-12-30 20:41 ` Sakari Ailus
2012-01-01 15:38 ` Sylwester Nawrocki
2012-01-04 20:39 ` Sakari Ailus
2012-01-04 20:57 ` Laurent Pinchart
2012-01-04 21:24 ` Sakari Ailus
2012-01-04 22:06 ` Sylwester Nawrocki
2012-01-11 22:36 ` Sakari Ailus
2012-01-13 21:41 ` Sylwester Nawrocki
2011-12-29 4:06 ` HeungJun, Kim
2012-01-02 9:53 ` Sylwester Nawrocki
2011-12-30 11:23 ` Sylwester Nawrocki
2011-12-28 6:23 ` [RFC PATCH 2/4] v4l: Add V4L2_CID_SCENEMODE " HeungJun, Kim
2011-12-28 13:56 ` Laurent Pinchart
2011-12-29 5:40 ` HeungJun, Kim
2011-12-30 0:11 ` Laurent Pinchart
2011-12-30 5:31 ` HeungJun, Kim
2011-12-28 6:23 ` [RFC PATCH 3/4] v4l: Add V4L2_CID_WDR button control HeungJun, Kim
2011-12-28 13:56 ` Laurent Pinchart
2011-12-29 5:52 ` HeungJun, Kim
2011-12-30 0:13 ` Laurent Pinchart
2011-12-30 5:41 ` HeungJun, Kim
2011-12-30 21:10 ` Sakari Ailus
2011-12-28 6:23 ` [RFC PATCH 4/4] v4l: Add V4L2_CID_ANTISHAKE " HeungJun, Kim
2011-12-28 13:58 ` Laurent Pinchart
2011-12-29 5:57 ` HeungJun, Kim
2011-12-28 14:01 ` [RFC PATCH 0/4] Add some new camera controls Laurent Pinchart
2011-12-29 6:15 ` HeungJun, Kim
2011-12-30 0:16 ` Laurent Pinchart
2011-12-30 7:52 ` HeungJun, Kim
2011-12-30 11:18 ` Sylwester Nawrocki
2012-01-04 21:07 ` Sakari Ailus
2012-01-28 17:01 ` Sylwester Nawrocki
2012-01-30 22:25 ` Sakari Ailus
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=4EFE0967.6020904@redhat.com \
--to=hdegoede@redhat.com \
--cc=hverkuil@xs4all.nl \
--cc=kyungmin.park@samsung.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@redhat.com \
--cc=riverful.kim@samsung.com \
--cc=sakari.ailus@iki.fi \
--cc=snjw23@gmail.com \
/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).