Hi Laurent,
On 12/06/2011 01:26 PM, Laurent Pinchart wrote:
> On Sunday 04 December 2011 16:16:13 Sylwester Nawrocki wrote:
>> From: Heungjun Kim <riverful.kim@xxxxxxxxxxx>
>>
>> The V4L2_CID_FOCUS_AUTO control has been converted from boolean type,
>> where control's value 0 and 1 were corresponding to manual and automatic
>> focus respectively, to menu type with following menu items:
>> 0 - V4L2_FOCUS_MANUAL,
>> 1 - V4L2_FOCUS_AUTO,
>> 2 - V4L2_FOCUS_AUTO_MACRO,
>> 3 - V4L2_FOCUS_AUTO_CONTINUOUS.
>>
>> According to this change the uvc control mappings are modified to retain
>> original sematics, where 0 corresponds to manual and 1 to auto focus.
>
> UVC auto-focus works in continuous mode, not single-shot mode. As the existing
Hmm, I suspected that.
> V4L2_CID_FOCUS_AUTO uses 0 to mean manual focus and 1 to mean continuous auto-
> focus, shouldn't this patch set define the following values instead ?
>
> 0 - V4L2_FOCUS_MANUAL
> 1 - V4L2_FOCUS_AUTO
> 2 - V4L2_FOCUS_AUTO_MACRO
> 3 - V4L2_FOCUS_AUTO_SINGLE_SHOT
It's not that bad, at least we would not have changed the existing ABI.
It depends how other focus modes are defined, e.g. V4L2_FOCUS_AUTO_MACRO.
There is also an auto focus driven by face detection module output.
The question would be whether we want to append _SINGLE_SHOT or _CONTINUOUS
to the names. And if most of the focus modes are single-shot we will
probably need a common "do auto-focus" control for them.
What do you think about such assignment:
0 - V4L2_FOCUS_MANUAL,
1 - V4L2_FOCUS_AUTO_CONTINUOUS,
2 - V4L2_FOCUS_AUTO,
3 - V4L2_FOCUS_AUTO_MACRO,
?
I'm not 100% sure right now, but the macro and "face detection"
are rather "single-shot". Single-shot focus might be more common. Perhaps
someone else can put more light on that :-)
Using a "single-shot notation" we might have something like:
0 - V4L2_FOCUS_MANUAL,
1 - V4L2_FOCUS_AUTO,
2 - V4L2_FOCUS_AUTO_SINGLE_SHOT,
3 - V4L2_FOCUS_AUTO_MACRO_SINGLE_SHOT,
3 - V4L2_FOCUS_AUTO_FACE_DETECTION_SINGLE_SHOT,
I'm not sure if this convention is the best one.
Alternatively we could define a "do-focus" control for each mode,
but then these controls have to be properly coordinated, for exclusive
operation.
>
> V4L2_FOCUS_AUTO_SINGLE_SHOT could also be named V4L2_FOCUS_SINGLE_SHOT.
>
>> Signed-off-by: Heungjun Kim <riverful.kim@xxxxxxxxxxx>
>> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
>>
>> ---
>> The V4L2_CID_FOCUS_AUTO control in V4L2_FOCUS_AUTO mode does only
>> a one-shot auto focus, when switched from V4L2_FOCUS_MANUAL.
>> It might be worth to implement also the V4L2_CID_DO_AUTO_FOCUS button
>> control in uvc, however I didn't take time yet to better understand
>> the driver and add this. I also don't have any uvc hardware to test
>> this patch so it's just compile tested.
>> ---
>> drivers/media/video/uvc/uvc_ctrl.c | 9 ++++++++-
>> 1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/video/uvc/uvc_ctrl.c
>> b/drivers/media/video/uvc/uvc_ctrl.c index 254d326..6860ca1 100644
>> --- a/drivers/media/video/uvc/uvc_ctrl.c
>> +++ b/drivers/media/video/uvc/uvc_ctrl.c
>> @@ -365,6 +365,11 @@ static struct uvc_menu_info exposure_auto_controls[] =
>> { { 8, "Aperture Priority Mode" },
>> };
>>
>> +static struct uvc_menu_info focus_auto_controls[] = {
>> + { 0, "Manual Mode" },
>> + { 1, "Auto Mode" },
Do you think it could be better to change this to:
+ { 0, "Manual Focus" },
+ { 1, "Continuous Auto Focus" },
?
--
Regards,
Sylwester
--
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
