Hi Laurent,
Laurent Pinchart wrote:
Hi Sakari,
On Wednesday 04 January 2012 21:39:34 Sakari Ailus wrote:
On Sun, Jan 01, 2012 at 04:38:21PM +0100, Sylwester Nawrocki wrote:
On 12/30/2011 09:41 PM, Sakari Ailus wrote:
On Fri, Dec 30, 2011 at 11:14:39AM +0100, Sylwester Nawrocki wrote:
On 12/30/2011 12:34 AM, Sakari Ailus wrote:
On Wed, Dec 28, 2011 at 02:51:38PM +0100, Laurent Pinchart wrote:
On Wednesday 28 December 2011 14:35:00 Sylwester Nawrocki wrote:
On 12/28/2011 07:23 AM, HeungJun, Kim wrote:
It adds the new CID for setting White Balance Preset. This CID is
provided as menu type using the following items:
0 - V4L2_WHITE_BALANCE_INCANDESCENT,
1 - V4L2_WHITE_BALANCE_FLUORESCENT,
2 - V4L2_WHITE_BALANCE_DAYLIGHT,
3 - V4L2_WHITE_BALANCE_CLOUDY,
4 - V4L2_WHITE_BALANCE_SHADE,
I have been also investigating those white balance presets recently
and noticed they're also needed for the pwc driver. Looking at
drivers/media/video/pwc/pwc-v4l2.c there is something like:
const char * const pwc_auto_whitebal_qmenu[] = {
"Indoor (Incandescant Lighting) Mode",
"Outdoor (Sunlight) Mode",
"Indoor (Fluorescent Lighting) Mode",
"Manual Mode",
"Auto Mode",
NULL
};
static const struct v4l2_ctrl_config pwc_auto_white_balance_cfg = {
.ops =&pwc_ctrl_ops,
.id = V4L2_CID_AUTO_WHITE_BALANCE,
.type = V4L2_CTRL_TYPE_MENU,
.max = awb_auto,
.qmenu = pwc_auto_whitebal_qmenu,
};
...
cfg = pwc_auto_white_balance_cfg;
cfg.name = v4l2_ctrl_get_name(cfg.id);
cfg.def = def;
pdev->auto_white_balance = v4l2_ctrl_new_custom(hdl,&cfg, NULL);
So this driver re-defines V4L2_CID_AUTO_WHITE_BALANCE as a menu
control with custom entries. That's interesting... However it
works in practice and applications have access to what's provided
by hardware. Perhaps V4L2_CID_AUTO_WHITE_BALANCE_TEMPERATURE would
be a better fit for that :)
Nevertheless, redefining standard controls in particular drivers
sounds a little dubious. I wonder if this is a generally agreed
approach ?
No agreed with me at least :-)
Then, how does your V4L2_CID_PRESET_WHITE_BALANCE control interact
with V4L2_CID_AUTO_WHITE_BALANCE control ? Does
V4L2_CID_AUTO_WHITE_BALANCE need to be set to false for
V4L2_CID_PRESET_WHITE_BALANCE to be effective ?
Is the preset a fixed white balance setting, or is it an auto white
balance with the algorithm tuned for a particular configuration ?
In the first case, does it correspond to a fixed white balance
temperature value ?
While I'm waiting for a final answer to this, I guess it's the
second. There are three things involved here:
- V4L2_CID_WHITE_BALANCE_TEMPERATURE: relatively low level control
telling
the colour temperature of the light source. Setting a value for
this essentially means using manual white balance.
- V4L2_CID_AUTO_WHITE_BALANCE: automatic white balance enabled or
disabled.
Was the third thing the V4L2_CID_DO_WHITE_BALANCE control that you
wanted to say ? It's also quite essential functionality, to be able
to fix white balance after pointing camera to a white object. And I
would expect
V4L2_CID_WHITE_BALANCE_PRESET control's documentation to state how an
interaction with V4L2_CID_DO_WHITE_BALANCE looks like.
I expected the new control to be the third thing as configuration for
the awb algorithm, which it turned out not to be.
I don't quite understand the purpose of the do_white_balance; the
automatic white balance algorithm is operational until it's disabled,
and after disabling it the white balance shouldn't change. What is the
extra functionality that the do_white_balance control implements?
Maybe DO_WHITE_BALANCE was inspired by some hardware's behaviour, I don't
know. I have nothing against this control. It allows you to perform
one-shot white balance in a given moment in time. Simple and clear.
Well, yes, if you have an automatic white balance algorithm which supports
"one-shot" mode. Typically it's rather a feedback loop. I guess this means
"just run one iteration".
Something like this should possibly be used to get the white balance
correct by pointing the camera to an object of known colour (white
typically, I think). But this isn't it, at least based on the description
in the spec.
Then either the spec is incorrect, or I'm mistaken. My understanding of the
DO_WHITE_BALANCE control is exactly what you described.
This is what the spec says:
"This is an action control. When set (the value is ignored), the device
will do a white balance and then hold the current setting. Contrast this
with the boolean V4L2_CID_AUTO_WHITE_BALANCE, which, when activated,
keeps adjusting the white balance."
I wonder if that should be then changed --- or is it just me who got a
different idea from the above description?
My understanding is that the operation for getting the white balance
information from a white object is by far simpler than getting the white
balance correct without that.
These seem to be only two references to this control in drivers and both
drivers are grossly misusing it. On one of them the description is
"white balance background: blue" and on the other it's "night mode".
That makes me wonder in what kind of circumstances this control was
originally introduced. Whatever it was, it seems to have taken place
before 16th April in 2005. :-)
I think we could change the description to something more suitable or
just remove this one...
Regards,
--
Sakari Ailus
sakari.ailus@xxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
