<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 16, 2022 at 12:34 AM David Plowman <<a href="mailto:david.plowman@raspberrypi.com">david.plowman@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Hanlin, Jean-Michel, everyone<br>
<br>
On Mon, 14 Mar 2022 at 11:06, Hanlin Chen <<a href="mailto:hanlinchen@chromium.org" target="_blank">hanlinchen@chromium.org</a>> wrote:<br>
><br>
> Hi David, Thanks for the great patch!<br>
><br>
> On Thu, Mar 10, 2022 at 10:53 PM Jean-Michel Hautbois <<a href="mailto:jeanmichel.hautbois@ideasonboard.com" target="_blank">jeanmichel.hautbois@ideasonboard.com</a>> wrote:<br>
>><br>
>> Hi David,<br>
>><br>
>> Thanks for the patch !<br>
>><br>
>> On 10/03/2022 13:05, David Plowman via libcamera-devel wrote:<br>
>> > This patch describes a series of controls that allow applications to<br>
>> > drive AF algorithms:<br>
>> ><br>
>> > AfMode - manual, auto or continuous<br>
>> > AfRange - full, macro or normal<br>
>> > AfSpeed - fast or slow<br>
>> > AfWindow - AF window locations<br>
>> > AfTrigger - start (trigger an AF scan) or cancel<br>
>> > AfPause - pause continuous AF<br>
>> > LensPosition - position of lens from lens driver<br>
>> > AfState - reset, scanning, focused or failed<br>
>> > ---<br>
>> > src/libcamera/control_ids.yaml | 271 +++++++++++++++++++++++++--------<br>
>> > 1 file changed, 211 insertions(+), 60 deletions(-)<br>
>> ><br>
>> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml<br>
>> > index 9d4638ae..89636d82 100644<br>
>> > --- a/src/libcamera/control_ids.yaml<br>
>> > +++ b/src/libcamera/control_ids.yaml<br>
>> > @@ -406,27 +406,6 @@ controls:<br>
>> > The camera will cancel any active or completed metering sequence.<br>
>> > The AE algorithm is reset to its initial state.<br>
>> ><br>
>> > - - AfTrigger:<br>
>> > - type: int32_t<br>
>> > - draft: true<br>
>> > - description: |<br>
>> > - Control for AF trigger. Currently identical to<br>
>> > - ANDROID_CONTROL_AF_TRIGGER.<br>
>> > -<br>
>> > - Whether the camera device will trigger autofocus for this request.<br>
>> > - enum:<br>
>> > - - name: AfTriggerIdle<br>
>> > - value: 0<br>
>> > - description: The trigger is idle.<br>
>> > - - name: AfTriggerStart<br>
>> > - value: 1<br>
>> > - description: The AF routine is started by the camera.<br>
>> > - - name: AfTriggerCancel<br>
>> > - value: 2<br>
>> > - description: |<br>
>> > - The camera will cancel any active trigger and the AF routine is<br>
>> > - reset to its initial state.<br>
>> > -<br>
>> > - NoiseReductionMode:<br>
>> > type: int32_t<br>
>> > draft: true<br>
>> > @@ -507,45 +486,6 @@ controls:<br>
>> > The AE algorithm has started a pre-capture metering session.<br>
>> > \sa AePrecaptureTrigger<br>
>> ><br>
>> > - - AfState:<br>
>> > - type: int32_t<br>
>> > - draft: true<br>
>> > - description: |<br>
>> > - Control to report the current AF algorithm state. Currently identical to<br>
>> > - ANDROID_CONTROL_AF_STATE.<br>
>> > -<br>
>> > - Current state of the AF algorithm.<br>
>> > - enum:<br>
>> > - - name: AfStateInactive<br>
>> > - value: 0<br>
>> > - description: The AF algorithm is inactive.<br>
>> > - - name: AfStatePassiveScan<br>
>> > - value: 1<br>
>> > - description: |<br>
>> > - AF is performing a passive scan of the scene in continuous<br>
>> > - auto-focus mode.<br>
>> > - - name: AfStatePassiveFocused<br>
>> > - value: 2<br>
>> > - description: |<br>
>> > - AF believes the scene is in focus, but might restart scanning.<br>
>> > - - name: AfStateActiveScan<br>
>> > - value: 3<br>
>> > - description: |<br>
>> > - AF is performing a scan triggered by an AF trigger request.<br>
>> > - \sa AfTrigger<br>
>> > - - name: AfStateFocusedLock<br>
>> > - value: 4<br>
>> > - description: |<br>
>> > - AF believes has focused correctly and has locked focus.<br>
>> > - - name: AfStateNotFocusedLock<br>
>> > - value: 5<br>
>> > - description: |<br>
>> > - AF has not been able to focus and has locked.<br>
>> > - - name: AfStatePassiveUnfocused<br>
>> > - value: 6<br>
>> > - description: |<br>
>> > - AF has completed a passive scan without finding focus.<br>
>> > -<br>
>> > - AwbState:<br>
>> > type: int32_t<br>
>> > draft: true<br>
>> > @@ -690,4 +630,215 @@ controls:<br>
>> > value. All of the custom test patterns will be static (that is the<br>
>> > raw image must not vary from frame to frame).<br>
>> ><br>
>> > + - AfMode:<br>
>> > + type: int32_t<br>
>><br>
>> Same open question for almost all: why int32_t and not uint8_t ?<br>
>> We won't have negative values ? Maybe is it yaml ?<br>
<br>
The file only seems to contain int32_ts so that's what I copied. Am I<br>
allowed other things?<br>
<br>
>><br>
>> > + draft: true<br>
>> > + description: |<br>
>> > + Control to set the mode of the AF (autofocus) algorithm. Applications<br>
>> > + are allowed to set a new mode, and to send additional controls for<br>
>> > + that new mode, in the same request.<br>
>> > + enum:<br>
>> > + - name: AfModeManual<br>
>> > + value: 0<br>
>> > + description: |<br>
>> > + The AF algorithm is in manual mode. In this mode it will never<br>
>> > + perform any action nor move the lens of its own accord. The only<br>
>> > + autofocus controls that have an immediate effect are AfMode (to<br>
>> > + switch out of manual mode) and LensPosition (so that the lens can<br>
>> > + be moved "manually").<br>
>><br>
>> Alignment changed in the paragraph. I wonder if the part "The only<br>
>> autofocus [...] "manually")." should be moved in the top description ?<br>
<br>
Thanks, I'll fix this up and all the ones below...<br>
<br>
>><br>
>> > +<br>
>> > + In this mode the AfState will always report AfStateReset.<br>
>> > + - name: AfModeAuto<br>
>> > + value: 1<br>
>> > + description: |<br>
>> > + The AF algorithm is in auto mode. This means that the algorithm<br>
>> > + will never move the lens or change state unless the AfTrigger<br>
>> > + control is used. The AfTrigger control can be used to initiate a<br>
>> > + focus scan, the results of which will also be reported by AfState.<br>
>> > +<br>
>> > + If the autofocus algorithm is moved from AfModeAuto to another<br>
>> > + mode while a scan is in progress, the scan is cancelled<br>
>> > + immediately, without waiting for the scan to finish.<br>
>> > +<br>
>> > + When first entering this mode the AfState will report<br>
>><br>
>> Alignment broken ?<br>
>><br>
>> > + AfStateReset. When a trigger control is sent, AfState will<br>
>> > + report AfStateScanning for a period before spontaneously<br>
>> > + changing to AfStateFocused or AfStateFailed, depending on the<br>
>> > + outcome of the scan. It will remain in this state until another<br>
>> > + scan is initiated by the AfTrigger control. If a scan is<br>
>> > + cancelled (without changing to another mode), AfState will return<br>
>> > + to AfStateReset.<br>
>> > + - name: AfModeContinuous<br>
>> > + value: 2<br>
>> > + description: |<br>
>> > + The AF algorithm is in continuous mode. This means that the lens<br>
>> > + can re-start a scan spontaneously at any moment, without any user<br>
>> > + intervention. The AfState still reports whether the algorithm is<br>
>> > + currently scanning or not, though the application has no ability<br>
>> > + to initiate or cancel scans, nor move the lens for itself.<br>
>> > +<br>
>><br>
>> Alignement broken ?<br>
>><br>
>> > + When set to AfModeContinuous, the system will immediately initiate<br>
>> > + a scan so AfState will report AfStateScanning, and will settle on<br>
>> > + one of AfStateFocused or AfStateFailed, depending on the scan<br>
>> > + result.<br>
>> > +<br>
>> > + The continuous autofocus behaviour can be paused with the<br>
>> > + AfPause control. Pausing the algorithm does not change the value<br>
>> > + reported by AfState, so that applications can determine the<br>
>> > + state of the algorithm when the pause control took effect. Once<br>
>> > + un-paused ("resumed"), the algorithm starts again from exactly<br>
>> > + where it left off when it paused.<br>
><br>
> An open question:<br>
> Should we make Macro and Continuous mandatory? Or they can be optional depending on the algorithm implementation?<br>
<br>
I'd certainly like CAF (Continuous AF) to be optional. It's a feature<br>
you could choose not to implement as it's quite a lot of work, and<br>
hard to make work well (especially without PDAF).<br>
<br>
Something like the "Macro" range... don't mind. We could make it<br>
optional, or define it to be the same as "Normal". What do people<br>
think?<br>
<br>
>><br>
>><br>
>> I find this difficult to follow without having a state machine in some<br>
>> drawn way... Is it something we could include in the documentation ?<br>
>> Seeing which Control changes to which one, which states are then<br>
>> triggered, etc. ?<br>
>><br>
>> > +<br>
>> > + - AfRange:<br>
>> > + type: int32_t<br>
>> > + draft: true<br>
>> > + description: |<br>
>> > + Control to set the range of focus distances that is scanned.<br>
>> > + enum:<br>
>> > + - name: AfRangeNormal<br>
>> > + value: 0<br>
>> > + description: |<br>
>> > + A wide range of focus distances is scanned, all the way from<br>
>> > + infinity down to close distances, though depending on the<br>
>> > + implementation, possibly not including the very closest macro<br>
>> > + positions.<br>
>><br>
>> Alignment broken ?<br>
>><br>
>> > + - name: AfRangeMacro<br>
>> > + value: 1<br>
>> > + description: Only close distances are scanned.<br>
>> > + - name: AfRangeFull<br>
>> > + value: 2<br>
>> > + description: |<br>
>> > + The full range of focus distances is scanned just as with<br>
>> > + AfRangeNormal but this time including the very closest macro<br>
>> > + positions.<br>
>> > +<br>
>> > + - AfSpeed:<br>
>> > + type: int32_t<br>
>> > + draft: true<br>
>> > + description: |<br>
>> > + Control that determines whether the AF algorithm is to move the lens<br>
>> > + as quickly as possible or more steadily. For example, during video<br>
>> > + recording it may be desirable not to move the lens too abruptly, but<br>
>> > + when in a preview mode (waiting for a still capture) it may be<br>
>> > + helpful to move the lens as quickly as is reasonably possible.<br>
>> > + enum:<br>
>> > + - name: AfSpeedNormal<br>
>> > + value: 0<br>
>> > + description: Move the lens at its usual speed.<br>
>> > + - name: AfSpeedFast<br>
>> > + value: 1<br>
>> > + description: Move the lens more quickly.<br>
>> > +<br>
>><br>
>> Can we somehow configure this speed ?<br>
<br>
Good question. You could imagine a continuous value here, though that<br>
does start to force the developer to do more work, and testing gets<br>
harder. So I feel slightly inclined to leave this as a binary choice,<br>
but am certainly open to persuasion.<br>
<br>
>><br>
>> > + - AfWindow:<br>
>> > + type: Rectangle<br>
>> > + draft: true<br>
>> > + description: |<br>
>> > + Sets the focus windows used by the AF algorithm. The units used express<br>
>> s/windows/window ?<br>
>> > + a proportion of the ScalerCrop control (or if unavailable, of the entire<br>
>> > + image), as u0.16 format numbers.<br>
>> > +<br>
>> > + In order to be activated, a rectangle must be programmed with non-zero<br>
>> > + width and height. If no rectangles are programmed in this way, then the<br>
>> > + system will choose its own single default window in the centre of the<br>
>> > + image.<br>
>> > +<br>
>> > + The details of how the windows are used are platform dependent. We note<br>
>> > + that when there is more than one AF window, a typical implementation<br>
>> > + might find the optimal focus position for each one and finally select<br>
>> > + the window closest to the camera.<br>
>><br>
>> Alignment broken ?<br>
>><br>
>> > +<br>
>> > + size: [platform dependent]<br>
>> > +<br>
>> > + - AfTrigger:<br>
>> > + type: int32_t<br>
>> > + draft: true<br>
>> > + description: |<br>
>> > + This control starts an autofocus scan when AfMode is set to AfModeAuto,<br>
>> > + and can also be used to terminate a scan early.<br>
>> > +<br>
>> > + It is ignored if AfMode is set to AfModeContinuous.<br>
>> > +<br>
>> > + enum:<br>
>> > + - name: AfTriggerStart<br>
>> > + value: 0<br>
>> > + description: Start an AF scan. Ignored if a scan is in progress.<br>
>> > + - name: AfTriggerCancel<br>
>> > + value: 1<br>
>> > + description: Cancel an AF scan. Ingored if no scan is in progress.<br>
>><br>
>> Does cancelling a scan make the lens stay at its current position ? Or<br>
>> should it be reset ?<br>
<br>
Stay at the current position. It's your (or the application's) choice<br>
whether to reset the lens. I'll make that clearer.<br>
<br>
>><br>
>> > +<br>
>> > + - AfPause:<br>
>> > + type: int32_t<br>
>> > + draft: true<br>
>> > + description: |<br>
>> > + This control has no effect except when in continuous autofocus mode<br>
>> > + (AfModeContinuous). It can be used to pause any lens movements while<br>
>> > + (for example) images are captured. The algorithm remains inactive<br>
>> > + until it is instructed to resume.<br>
>> > +<br>
>> > + enum:<br>
>> > + - name: AfPauseImmediate<br>
>> > + value: 0<br>
>> > + description: |<br>
>> > + Pause the continuous autofocus algorithm immediately, whether or<br>
>> > + not any kind of scan is underway. The AfState will continue to<br>
>> > + report whatever value it had when the control was enacted.<br>
>> > + - name AfPauseDeferred<br>
>> > + value: 1<br>
>> > + description: |<br>
>> > + Pause the continuous autofocus algorithm as soon as it is no longer<br>
>> > + scanning. The AfState will report AfStateFocused or AfStateFailed,<br>
>> > + depending on whether the final scan succeeds or not. If no scan is<br>
>> > + in currently progress, the algorithm will pause immediately.<br>
>><br>
> Just to confirm. Since we removed the AftriggerIdle, I guess the two controls AfTriggerStart and AfPauseDeferred should be continuously set until the state becomes focused or failed? If so, is it better to mention it in the description?<br>
<br>
I imagined that these would be "one-shot" things. So you only need to<br>
send them once and they happen.<br>
<br>
I imagine there might also be a slight delay between something being<br>
in focus and that feeding back to the application, so you wouldn't<br>
want the algorithm to think "oh, I'm being told to scan again even<br>
though I've only just finished!"<br>
<br>
> For AfPauseImmediate, should it only report the state as failed when it's scanning and reset?<br>
<br>
So I think if it was "scanning" and gets "AfPauseImmediate", the state<br>
remains "scanning". If it was "focused" when it gets<br>
"AfPauseImmediate", then it stays "focused", and so on. The idea is<br>
that this means you can be sure what state it was in when you sent<br>
"AfPauseImmediate". If we were to change the state you couldn't be<br>
sure whether it was focused or not. This could give an application the<br>
chance to put up a "picture might be blurry" message, for example.<br>
<br>
><br>
>><br>
>> Alignment broken ?<br>
>><br>
>> > + - name: AfPauseResume<br>
>> > + value: 2<br>
>> > + description: |<br>
>><br>
>> Alignment broken ?<br>
>><br>
>> > + Resume continous autofocus operation. The algorithm starts again<br>
>> > + from exactly where it left off, with AfState unchanged (one of<br>
>> > + AfStateFocused, AfStateFailed or following AfPauseImmediate it<br>
>> > + might also have been in the AfStateScanning state).<br>
>> > +<br>
>> > + - LensPosition:<br>
>> > + type: int32_t<br>
>> > + draft: true<br>
>> > + description: |<br>
>> > + Acts as a control to instruct the lens to move to a particular position<br>
>> > + and also reports back the position of the lens for each frame.<br>
>> > +<br>
>> > + The units are determined by the lens driver.<br>
>> > +<br>
>> > + The LensPosition control is ignored unless the AfMode is set to<br>
>> > + AfModeManual.<br>
>> > +<br>
>> > + - AfState:<br>
>> > + type: int32_t<br>
>> > + draft: true<br>
>> > + description: |<br>
>> > + Reports the current state of the AF algorithm.<br>
>> > + enum:<br>
>> > + - name: AfStateReset<br>
>> > + value: 0<br>
>> > + description: |<br>
>> > + The AF algorithm reports this state when:<br>
>> > + * It is in manual mode (AfModeManual).<br>
>> > + * The system has entered auto mode (AfModeAuto) but no scan<br>
>> > + has yet been initiated.<br>
>> > + * The system is in auto mode (AfModeAuto) and a scan has been<br>
>> > + cancelled.<br>
>> > + - name: AfStateScanning<br>
>> > + value: 1<br>
>> > + description: |<br>
>> > + AF is performing a scan. This state can be entered spontaneously<br>
>> > + if AfMode is set to AfModeContinuous, otherwise it requires the<br>
>> > + application to use the AfTrigger control to start the scan.<br>
>> > + - name: AfStateFocused<br>
>> > + value: 2<br>
>> > + description: |<br>
>> > + An AF scan has been performed and the algorithm believes the<br>
>> > + scene is in focus.<br>
>> > + - name: AfStateFailed<br>
>> > + value: 3<br>
>> > + description: |<br>
>> > + An AF scan has been performed but the algorithm has not been able<br>
>> > + to find the best focus position.<br>
>><br>
>> I confirm my wish of having a graphical state machine :-).<br>
><br>
> Vote +1 :-).<br>
<br>
I'll look into doing that. Do we have a preferred package? Do I have<br>
to turn it into Ascii Art? :)<br></blockquote><div>How about a state transition table? We could prove its completeness and it may be easier for rst documentation.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks for all the feedback!<br>
<br>
David<br>
<br>
>><br>
>><br>
>> Great work, thanks for it !<br>
>> JM<br>
>><br>
>> > +<br>
>> > ...<br>
</blockquote></div></div>