<div dir="ltr">Hi again everyone<div><br></div><div>Thanks for the latest replies and all the discussions. I think the best thing is probably if I get on and produce a v2 of this patch. I think the main differences will be: a "pause" control for CAF, and I might add an "off" state in which the lens can be moved. Then we can resume the discussion once more!</div><div><br></div><div>Best regards</div><div><br></div><div>David</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 17 Jan 2022 at 10:13, Hanlin Chen <<a href="mailto:hanlinchen@chromium.org">hanlinchen@chromium.org</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">The mail is re-sent by another account to avoid DMARC rejection. Sorry<br>
for the inconvenience.<br>
<br>
Hi David,<br>
Thanks for the comments.<br>
<br>
On Fri, Jan 14, 2022 at 1:54 AM David Plowman<br>
<<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>> wrote:<br>
><br>
> Hi Hanlin<br>
><br>
> Sorry for taking some time to get back to this!<br>
No worries. We all work asynchronously :)<br>
<br>
> From one of your other emails, and checking the Android documentation,<br>
> I see that "Trigger" does mean something in CAF mode though, as you<br>
> say, a "pause" control captures the meaning a bit better. I think<br>
> Android uses "trigger start" to mean:<br>
><br>
> - In "video" CAF mode: pause immediately, whatever is happening.<br>
> - In "picture" CAF mode: pause when any current (passive) scan is finished.<br>
><br>
> Perhaps a pause control would have 3 values: "pause immediately",<br>
> "pause when ready" and "resume".<br>
I moved the comment up, so I can comment on it earlier.<br>
Yes, I think it is more clear, although the algorithm may need to<br>
implement it for both video and picture CAF.<br>
I suppose it's fine?<br>
><br>
> On Tue, 21 Dec 2021 at 10:21, Hanlin Chen <<a href="mailto:hanlinchen@chromium.org" target="_blank">hanlinchen@chromium.org</a>> wrote:<br>
> ><br>
> > Hi David, thanks for the comments.<br>
> ><br>
> > On Fri, Dec 17, 2021 at 4:51 PM David Plowman<br>
> > <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>> wrote:<br>
> > ><br>
> > > Hi again<br>
> > ><br>
> > > Thanks for the comments!<br>
> > ><br>
> > > On Thu, 16 Dec 2021 at 11:18, Hanlin Chen <<a href="mailto:hanlinchen@chromium.org" target="_blank">hanlinchen@chromium.org</a>> wrote:<br>
> > > ><br>
> > > > Hi David, thanks for this work!<br>
> > > ><br>
> > > > On Mon, Dec 13, 2021 at 11:22 PM David Plowman<br>
> > > > <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>> wrote:<br>
> > > > ><br>
> > > > > This patch describes a series of controls that allow applications to<br>
> > > > > drive AF algorithms:<br>
> > > > ><br>
> > > > > AfMode - auto or continuous<br>
> > > > > AfRange - full, macro or normal<br>
> > > > > AfSpeed - fast or slow<br>
> > > > > AfMethod - single or multi-spot<br>
> > > > > AfWindow - AF window locations<br>
> > > > > AfTrigger - start (trigger an AF scan) or cancel<br>
> > > > > LensPosition - position of lens from lens driver<br>
> > > > > AfState - reset, scanning, focused or failed<br>
> > > > > ---<br>
> > > > > src/libcamera/control_ids.yaml | 227 ++++++++++++++++++++++++---------<br>
> > > > > 1 file changed, 167 insertions(+), 60 deletions(-)<br>
> > > > ><br>
> > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml<br>
> > > > > index 9d4638ae..e579f7b7 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,171 @@ 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>
> > > > > + draft: true<br>
> > > > > + description: |<br>
> > > > > + Control to set the mode of the AF (autofocus) algorithm.<br>
> > > > > + enum:<br>
> > > > > + - name: AfModeAuto<br>
> > > > > + value: 0<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 be reported by the AfState.<br>
> > > > > +<br>
> > > > > + In this mode, an application can move the lens for itself using<br>
> > > > > + the LensPosition control. If the algorithm was scanning when the<br>
> > > > > + lens is moved in this way, then the scan is implicitly cancelled.<br>
> > > ><br>
> > > > I would suggest splitting the Auto mode into AfModeOnDemand and<br>
> > > > AfModeManual (The naming could be re-considered).<br>
> > > > The reason is that the sub-controls for each AfModes could be distinct.<br>
> > > > 1. AfModeManual:<br>
> > > > LensPosition is valid only for the mode, and AfState is always AfStateReset.<br>
> > > > 2. AfModeOnDemand<br>
> > > > AfTrigger and AfRange are valid only for the mode, and AfState reports<br>
> > > > accordingly. The scan is implicitly canceled when changing from the<br>
> > > > mode.<br>
> > ><br>
> > > I'm a little bit nervous about having a mode where the lens position<br>
> > > control doesn't work. In my earlier email I described performing an AF<br>
> > > scan, capturing an image, then moving the lens back to hyperfocal. It<br>
> > > seems inconvenient to have to put the AF algorithm into a separate<br>
> > > mode in order to move the lens, only to put it back in the mode where<br>
> > > a scan is once again allowed.<br>
> > ><br>
> > > I don't think anything is lost by doing it this way - if an<br>
> > > application doesn't want to perform an AF scan it should simply not<br>
> > > request one.<br>
> > I agreed that it does cover the controls from Android, i.e., the<br>
> > Android request can be translated to the API.<br>
> > My concern is from another direction: translation from libcamera to<br>
> > Android Camera3.<br>
> > From the Android to libcamera:<br>
> > ANDROID_CONTROL_AF_MODE_OFF + ANDROID_LENS_FOCUS_DISTANCE ==><br>
> > AfModeAuto + lensPosition<br>
> > ANDROID_CONTROL_AF_MODE_CONTINUOUS_VIDEO +<br>
> > ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_START ==> AfModeAuto +<br>
> > AfTrigger<br>
><br>
> I don't quite understand how an AE_PRECAPTURE_TRIGGER_START affects<br>
> the AF state machine, isn't that something we'd send to the AE<br>
> algorithm (if it had such a control)? And then we'd wait for the AE<br>
> state to say it was finished?<br>
<br>
Ah... It's a copy error, I simply mean AF_TRIGGER here.<br>
><br>
> > From libcamera to Android:<br>
> > AfModeAuto + lensPostion (Assuming current position is reported by<br>
> > libcamera) ==> ??<br>
> > It could be CAF, AUTO or OFF mode.<br>
><br>
> As I understand it, Android only lets you set the lens position when<br>
> AF mode is "OFF". So in this case we'd have to set the Android AF to<br>
> "OFF" along with the lens position, right?<br>
><br>
> If the next thing libcamera wants to do is something other than move<br>
> the lens, then we'll have to set Android back to the state<br>
> corresponding to libcamera's. Does that work?<br>
><br>
The context here is that I seem to assume that if we don't have<br>
"pause" in CAF, we need to mimic it by changing Auto with<br>
lensPosition.<br>
Thus, when translating back to Android, it's not clear that it's Auto<br>
or CAF with mimicking "pause".<br>
Maybe my assumption is wrong?<br>
<br>
However, if we have "pause", then the problem disappears:<br>
with lensPosition == OFF, without == AUTO.<br>
<br>
> > States from libcamera Android:<br>
> > AfModeAuto + AfTrigger + focused ==><br>
> > ANDROID_CONTROL_AF_STATE_FOCUSED_LOCKED? or<br>
> > ANDROID_CONTROL_AF_STATE_PASSIVE_FOCUSED?<br>
><br>
> If there was a trigger, then I believe the "focused" state needs to<br>
> correspond to AF_STATE_FOCUSED_LOCKED. In fact, is it the case that in<br>
> auto mode, Android can never report AF_STATE_PASSIVE_FOCUSED?<br>
Same above.<br>
Yes, if it's AF_MODE_AUTO, it never report AF_STATE_PASSIVE_FOCUSED.<br>
><br>
> ><br>
> > For the Android adapter layer, above might be workarounded by caching<br>
> > the requests and do reference and find out current context.<br>
> > A more subtle case might arise when integrating other platforms.<br>
> > For example, Qualcomm closed 3A lib follows the modes and states<br>
> > according to Android (Partly true for Intel and MTK).<br>
> > In the case, the translation flow will like:<br>
> > Android API -> libcamera API -> Android like API for 3A lib -><br>
> > libcamera API -> Android API<br>
> > <Android> <libcamera> <IPA><br>
> > <libcamera> <Android><br>
> > It's hard to predict how to work around all corner cases.<br>
> > This makes me believe that if we could have a clear one to one mapping<br>
> > between Android and libcamera API, it could be beneficial later.<br>
> > However, it will make the API more verbose.<br>
> ><br>
> > Another perspective is that Android defines required state machine transition:<br>
> > <a href="https://source.android.com/devices/camera/camera3_3Amodes" rel="noreferrer" target="_blank">https://source.android.com/devices/camera/camera3_3Amodes</a><br>
> ><br>
> > For IPA to implement the state transition satisfying Android's<br>
> > requirement, the IPA should know exactly how the libcamera states<br>
> > mapping back to the Android states.<br>
> > Above is not satisfying because we will be somehow restricted by<br>
> > Android's definition ;-|, However, I think it could be beneficial if<br>
> > the API cam map libcamera states/modes to all Android states/modes<br>
> > directly without extra context/call order information.<br>
><br>
> I'm really in two minds about the question of an "off" mode in<br>
> libcamera. Either we make libcamera application code handle the<br>
> transition to "off" and back to "auto", or a libcamera-to-Android<br>
> translation layer would have to do it. Does anyone else out there feel<br>
> strongly?<br>
<br>
I suppose we could align it with AE (or let AE align with AF :P). AE<br>
has a similar case for AnalogueGainMode and ExposureTimeMode.<br>
<a href="https://patchwork.libcamera.org/patch/15179/" rel="noreferrer" target="_blank">https://patchwork.libcamera.org/patch/15179/</a><br>
Where Paul seems to use "Auto" and "Disabled" for the case.<br>
cc'ed Paul.<br>
<br>
><br>
> ><br>
> > ><br>
> > > I think AfRange applies to CAF as well. I see no reason why you might<br>
> > > not want to restrict the range of searches for just the same reasons<br>
> > > as you might do with "regular" AF.<br>
> > Agreed.<br>
> > ><br>
> > > > 3. AfModeContinuous<br>
> > > > AfSpeed and "pause" are valid only for the mode, and AfState reports<br>
> > > > accordingly. The scan is implicitly canceled when changing from the<br>
> > > > mode.<br>
> > ><br>
> > > I'm not sure AfSpeed is *necessarily* only for CAF. I think we've done<br>
> > > AF systems in the past where even regular AF has a "I really want to<br>
> > > take the picture quickly" mode. But it could be a platform choice?<br>
> > Agreed.<br>
> > ><br>
> > > ><br>
> > > > > + - name: AfModeContinuous<br>
> > > > > + value: 1<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>
> > > > > + The mode can be set to AfModeAuto which has the effect of<br>
> > > > > + "pausing" any continuous AF activity, after which it could then<br>
> > > > > + be moved back to AfModeContinuous to resume operation.<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: AfRangeFull<br>
> > > > > + value: 0<br>
> > > > > + description: The full range of focus distances is scanned.<br>
> > > > > + - name: AfRangeMacro<br>
> > > > > + value: 1<br>
> > > > > + description: Only close distances are scanned.<br>
> > > > > + - name: AfRangeNormal<br>
> > > > > + value: 2<br>
> > > > > + description: |<br>
> > > > > + The full range of focus distances is scanned except for some of<br>
> > > > > + the closest macro 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: AfSpeedFast<br>
> > > > > + value: 0<br>
> > > > > + description: Move the lens quickly.<br>
> > > > > + - name: AfSpeedSlow<br>
> > > > > + value: 0<br>
> > > > > + description: Move the lens more steadily.<br>
> > > > How about<br>
> > > > s/Fast/Aggressively<br>
> > > > s/Slow/Steadily<br>
> > ><br>
> > > I agree the choice is tricky here. I think I would regard what I<br>
> > > called "Slow" to be the "usual" choice, so perhaps "AfSpeedNormal" is<br>
> > > better (and maybe make that value 0). I still prefer "AfModeFast" to<br>
> > > "AfModeAggressive" - "aggressive" sounds just a bit too... aggressive?<br>
> > > :)<br>
> > LGTM.<br>
> > ><br>
> > > But thanks for all the discussion. Glad we're making progress on all this!<br>
> > ><br>
> > > Best regards<br>
> > > David<br>
> > ><br>
> > > > > +<br>
> > > > > + - AfMethod:<br>
> > > > > + type: int32_t<br>
> > > > > + draft: true<br>
> > > > > + description: |<br>
> > > > > + Control whether the AF algorithm uses a single window in the image to<br>
> > > > > + determine the best focus position, or multiple windows simultaneously.<br>
> > > > > + enum:<br>
> > > > > + - name: AfMethodSingle<br>
> > > > > + value: 0<br>
> > > > > + description: |<br>
> > > > > + A single window within the image, defaulting to the centre, is used<br>
> > > > > + to select the best focus distance.<br>
> > > > > + - name: AfMethodMultiSpot<br>
> > > > > + value: 0<br>
> > > > > + description: |<br>
> > > > > + Multiple windows within the image are used to select the best focus<br>
> > > > > + distance. The best focus distance is found for each one of the<br>
> > > > > + windows, and then the distance that is closest to the camera is<br>
> > > > > + selected.<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>
> > > > > + 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>
> > > > > + If AfMethod is set to AfMethodSingle, then only the first Rectangle in<br>
> > > > > + this list is used (or the system default one if it is unprogrammed).<br>
> > > > > +<br>
> > > > > + If AfMethod is set to AfMethodMultiSpot then all the valid Rectangles in<br>
> > > > > + this list are used. The size of the control indicates how many such<br>
> > > > > + windows can be programmed and will vary between different platforms.<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>
> From one of your other emails, and checking the Android documentation,<br>
> I see that "Trigger" does mean something in CAF mode though, as you<br>
> say, a "pause" control captures the meaning a bit better. I think<br>
> Android uses "trigger start" to mean:<br>
><br>
> - In "video" CAF mode: pause immediately, whatever is happening.<br>
> - In "picture" CAF mode: pause when any current (passive) scan is finished.<br>
><br>
> Perhaps a pause control would have 3 values: "pause immediately",<br>
> "pause when ready" and "resume".<br>
><br>
> Thanks and best regards<br>
><br>
> David<br>
><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>
> > > > > + If the LensPosition control is used while an AF scan is in progress,<br>
> > > > > + the scan is implicitly cancelled and the lens is then moved to the<br>
> > > > > + desired location. It is ignored if AfMode is set to AfModeContinuous.<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>
> > > > > + * The camera system has just been started.<br>
> > > > > + * A scan has been cancelled.<br>
> > > > > + * The lens has been moved by the LensPosition control.<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>
> > > > > ...<br>
> > > > > --<br>
> > > > > 2.30.2<br>
> > > > ><br>
</blockquote></div>