On 05/09/2012 12:11 PM, Sylwester Nawrocki wrote:
> On 05/06/2012 08:46 PM, Sakari Ailus wrote:
>> Sylwester Nawrocki wrote:
>>> Add following auto focus controls:
>>>
>>> - V4L2_CID_AUTO_FOCUS_START - single-shot auto focus start
>>> - V4L2_CID_AUTO_FOCUS_STOP - single-shot auto focus stop
>>> - V4L2_CID_AUTO_FOCUS_STATUS - automatic focus status
>>> - V4L2_CID_AUTO_FOCUS_AREA - automatic focus area selection
>>> - V4L2_CID_AUTO_FOCUS_DISTANCE - automatic focus scan range selection
>>>
>>> Signed-off-by: Sylwester Nawrocki<s.nawrocki@xxxxxxxxxxx>
>>> Signed-off-by: Kyungmin Park<kyungmin.park@xxxxxxxxxxx>
>>> ---
>>> Documentation/DocBook/media/v4l/controls.xml | 147 +++++++++++++++++++++++++-
>>> drivers/media/video/v4l2-ctrls.c | 31 +++++-
>>> include/linux/videodev2.h | 25 +++++
>>> 3 files changed, 200 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
>>> index 4a463d3..d8ef71e 100644
>>> --- a/Documentation/DocBook/media/v4l/controls.xml
>>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>>> @@ -2902,13 +2902,156 @@ negative values towards infinity. This is a write-only control.</entry>
>>> <row>
>>> <entry spanname="id"><constant>V4L2_CID_FOCUS_AUTO</constant> </entry>
>>> <entry>boolean</entry>
>>> - </row><row><entry spanname="descr">Enables automatic focus
>>> -adjustments. The effect of manual focus adjustments while this feature
>>> + </row><row><entry spanname="descr">Enables continuous automatic
>>> +focus adjustments. The effect of manual focus adjustments while this feature
>>> is enabled is undefined, drivers should ignore such requests.</entry>
>>> </row>
>>> <row><entry></entry></row>
>>>
>>> <row>
>>> + <entry spanname="id"><constant>V4L2_CID_AUTO_FOCUS_START</constant> </entry>
>>> + <entry>button</entry>
>>> + </row><row><entry spanname="descr">Starts single auto focus process.
>>> +The effect of setting this control when<constant>V4L2_CID_FOCUS_AUTO</constant>
>>> +is set to<constant>TRUE</constant> (1) is undefined, drivers should ignore
>>> +such requests.</entry>
>>> + </row>
>>> + <row><entry></entry></row>
>>> +
>>> + <row>
>>> + <entry spanname="id"><constant>V4L2_CID_AUTO_FOCUS_STOP</constant> </entry>
>>> + <entry>button</entry>
>>> + </row><row><entry spanname="descr">Aborts automatic focusing
>>> +started with<constant>V4L2_CID_AUTO_FOCUS_START</constant> control. It is
>>> +effective only when the continuous autofocus is disabled, that is when
>>> +<constant>V4L2_CID_FOCUS_AUTO</constant> control is set to<constant>FALSE
>>> +</constant> (0).</entry>
>>> + </row>
>>> + <row><entry></entry></row>
>>> +
>>> + <row id="v4l2-auto-focus-status">
>>> + <entry spanname="id">
>>> + <constant>V4L2_CID_AUTO_FOCUS_STATUS</constant> </entry>
>>> + <entry>bitmask</entry>
>>> + </row>
>>> + <row><entry spanname="descr">The automatic focus status. This is a read-only
>>> + control.</entry>
>>> + </row>
>>> + <row>
>>> + <entrytbl spanname="descr" cols="2">
>>> + <tbody valign="top">
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_STATUS_IDLE</constant> </entry>
>>> + <entry>Automatic focus is not active.</entry>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_STATUS_BUSY</constant> </entry>
>>> + <entry>Automatic focusing is in progress.</entry>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_STATUS_REACHED</constant> </entry>
>>> + <entry>Focus has been reached.</entry>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_STATUS_LOST</constant> </entry>
>>> + <entry>Focus has been lost.</entry>
>>
>> When does this happen?
>
> Hmm, good question. I intended this one for continuous auto focus, for the moments
> when the focus is lost. I felt the control is incomplete without such status bit.
>
> Thinking about it a bit more, it is just a negation of V4L2_AUTO_FOCUS_STATUS_FOCUSED.
^^^^^^^^^
Sorry, that should be V4L2_AUTO_FOCUS_STATUS_REACHED.
> The focus lost notifications could be well provided to user space be clearing this bit.
>
> So I would just get rid of V4L2_AUTO_FOCUS_STATUS_LOST, I don't really use it in any
> driver, it was supposed to be just for completeness.
>
> What do you think ?
>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_STATUS_FAILED</constant> </entry>
>>> + <entry>Automatic focus has failed, the driver will not
>>> + transition from this state until another action is
>>> + performed by an application.</entry>
>>
>> Which of these are valid for regular autofocus and which ones for
>> continuous autofocus? I'm a little bit confused with the above descriptions.
>
> All, except V4L2_AUTO_FOCUS_STATUS_LOST are valid for both auto focus modes.
> But I'm going to remove V4L2_AUTO_FOCUS_STATUS_LOST bit, as indicated above.
>
>> I might as well say that temporary conditions such as failed and reached
>> would return to idle after being read from user space. This is how the
>> flash faults behave, too.
>
> I'm not sure if it would be possible to fulfil such assumption in drivers
> in all cases. The status often comes from hardware and the driver might
> not be able to change it at will. I'm not sure if it is safe to change
> state just by reading it from user-space. This probably wouldn't work well
> with multiple processes accessing the camera.
>
> For instance, from state V4L2_AUTO_FOCUS_STATUS_FAILED a driver would have
> transitioned to something else after user-space sets V4L2_CID_AUTO_FOCUS_START
> control. Also I would prefer having V4L2_AUTO_FOCUS_STATUS_REACHED bit set
> as long as the camera stays in this state, regardless of how many status
> readers there are.
>
>>
>> How does this interact with the 3A lock control?
>
> Setting V4L2_LOCK_FOCUS lock would just stop updates to the status control
> value. The 3A lock control is just another one that influences the auto
> focus status, among V4L2_CID_AUTO_FOCUS_START and V4L2_CID_AUTO_FOCUS_STOP.
>
> Nevertheless, I see your point, that it's not clear from the Spec.
> How about adding something like this to the AF status control description:
>
> "Setting V4L2_LOCK_FOCUS lock may stop updates of the V4L2_CID_AUTO_FOCUS_STOP
^^^^^
And this - V4L2_CID_AUTO_FOCUS_STATUS. I guess I need more sleeping, and not on
the computer keyboard... :-)
The updated patch series is also available here:
http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/v4l-camera-controls
> control value."
>
> ?
>
> I'm not sure how much detailed the documentation should be, I wouldn't like
> to add something that would be hard to implement in drivers, for sake of
> the applications' simplicity... :)
>
>>> + </row>
>>> + </tbody>
>>> + </entrytbl>
>>> + </row>
>>> + <row><entry></entry></row>
>>> +
>>> + <row id="v4l2-auto-focus-range">
>>> + <entry spanname="id">
>>> + <constant>V4L2_CID_AUTO_FOCUS_RANGE</constant> </entry>
>>> + <entry>enum v4l2_auto_focus_range</entry>
>>> + </row>
>>> + <row><entry spanname="descr">Determines auto focus distance range
>>> +for which lens may be adjusted.</entry>
>>> + </row>
>>> + <row>
>>> + <entrytbl spanname="descr" cols="2">
>>> + <tbody valign="top">
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_RANGE_AUTO</constant> </entry>
>>> + <entry>The camera automatically selects the focus range.</entry>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_RANGE_NORMAL</constant> </entry>
>>> + <entry>The auto focus normal distance range. It is limited
>>> +for best auto focus algorithm performance.</entry>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_RANGE_MACRO</constant> </entry>
>>> + <entry>Macro (close-up) auto focus. The camera will
>>> +use minimum possible distance that it is capable of for auto focus.</entry>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_RANGE_INFINITY</constant> </entry>
>>> + <entry>The focus at an object at infinite distance.</entry>
>>> + </row>
>>> + </tbody>
>>> + </entrytbl>
>>> + </row>
>>> + <row><entry></entry></row>
>>> +
>>> + <row id="v4l2-auto-focus-area">
>>> + <entry spanname="id">
>>> + <constant>V4L2_CID_AUTO_FOCUS_AREA</constant> </entry>
>>> + <entry>enum v4l2_auto_focus_area</entry>
>>> + </row>
>>> + <row><entry spanname="descr">Determines the area of the frame that
>>> +the camera uses for automatic focus. The corresponding coordinates of the
>>> +focusing spot or rectangle can be specified and queried using the selection API.
>>> +To change the auto focus region of interest applications first select required
>>> +mode of this control and then set the rectangle or spot coordinates by means
>>> +of the&VIDIOC-SUBDEV-S-SELECTION; or&VIDIOC-S-SELECTION; ioctl. In order to
>>> +trigger again an auto focus process with same coordinates applications should
>>> +use the<constant>V4L2_CID_AUTO_FOCUS_START</constant> control. Or alternatively
>>> +invoke a&VIDIOC-SUBDEV-S-SELECTION; or a&VIDIOC-S-SELECTION; ioctl again.
>>> +In the latter case the new pixel coordinates are applied to hardware only when
>>> +the focus area control is set to a value other than
>>> +<constant>V4L2_AUTO_FOCUS_AREA_ALL</constant>.</entry>
>>> + </row>
>>> + <row>
>>> + <entrytbl spanname="descr" cols="2">
>>> + <tbody valign="top">
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_AREA_ALL</constant> </entry>
>>> + <entry>Normal auto focus, the focusing area extends over the
>>> +entire frame.</entry>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_AREA_SPOT</constant> </entry>
>>> + <entry>Automatic focus on a spot within the frame at position
>>> +specified by the<constant>V4L2_SEL_TGT_AUTO_FOCUS_ACTUAL</constant> or
>>> +<constant>V4L2_SUBDEV_SEL_TGT_AUTO_FOCUS_ACTUAL</constant> selection. When these
>>> +selections are not supported by driver the default spot's position is center of
>>> +the frame.</entry>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_AREA_RECTANGLE</constant> </entry>
>>> + <entry>The auto focus area is determined by the<constant>
>>> +V4L2_SEL_TGT_AUTO_FOCUS_ACTUAL</constant> or<constant>
>>> +V4L2_SUBDEV_SEL_TGT_AUTO_FOCUS_ACTUAL</constant> selection rectangle.</entry>
>>> + </row>
>>> + <row>
>>> + <entry><constant>V4L2_AUTO_FOCUS_AREA_FACE_DETECTION</constant> </entry>
>>> + <entry>The camera automatically focuses on a detected face
>>> +area.</entry>
>>
>> I assume there could be one or more faces to focus to, right?
>
> Indeed, I presume we're going to need another set of controls for Face Detection
> features.
>
> That's true, there can be more faces. I wasn't good enough at expressing this
> here :(
>
> Maybe something like:
>
> "The camera automatically focuses on the face detection regions."
>
> would be better ?
--
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
