linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: "HeungJun, Kim" <riverful.kim@samsung.com>
Cc: "'Sakari Ailus'" <sakari.ailus@iki.fi>,
	"'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 09:41:57 +0100	[thread overview]
Message-ID: <4EFD7955.8070603@redhat.com> (raw)
In-Reply-To: <000801ccc6bd$4b844520$e28ccf60$%kim@samsung.com>

Hi,

On 12/30/2011 07:35 AM, HeungJun, Kim wrote:
> Hi Sakari,
>
> Thanks for the comments!
>
> Your comments help me to order my thoughts and re-send RFC.
>

<snip>

>> The value of the new control would have an effect as long as automatic white
>> balance is enabled.
> No, it's a kind of Manual White Balance, not Auto. It's the same level of
> V4L2_CID_WHITE_BALANCE_TEMPERATURE. So, only when V4L2_CID_AUTO_WHITE_BALANCE is
>
> disabled, this control is enabled.
>
> The relationship between each white balance controls by my understanding is
> here.
>
> Auto White Balance
>    - V4L2_CID_AUTO_WHITE_BALANCE(Boolean)
>      : enable/disable Auto white balance.
>      : Enable means current mode is Auto, and disable means current mode is
> Manual
>
> Manual White Balance
>    - V4L2_CID_WHITE_BALANCE_TEMPERATURE(integer)
>      : Setting the temperature of Manual
>      : Only when the V4L2_CID_AUTO_WHITE_BALANCE is disabled, and current mode
> Manual.
>
> - V4L2_CID_WHITE_BALANCE_PRESET(menu) - I suggested
>      : Setting the specific temperature value(but, the value is not fetched by
> user) of Manual
>      : Only when the V4L2_CID_AUTO_WHITE_BALANCE is disabled, and current mode
> Manual.
>
> The "input" is right. And, this "input" just triggers the ISP(sensor) set the
> specific
> manual white balance value embedded in the ISP.
> I think this control does not affect the Auto White Balance.

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

Notice that we also need to add some standardized controls for the 3c case, but that
is a different discussion.

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.

Regards,

Hans

  reply	other threads:[~2011-12-30  8:41 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 [this message]
2011-12-30 18:42             ` 'Sakari Ailus'
2011-12-30 18:56               ` Hans de Goede
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=4EFD7955.8070603@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).