[libcamera-devel] How to deal with "atomic sequences" of AF controls?

[Nick Hollinghurst]

Hi Jacopo,

On Fri, 25 Nov 2022 at 11:43, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi Nick,
>
> On Fri, Nov 25, 2022 at 11:17:36AM +0000, Nick Hollinghurst via libcamera-devel wrote:
> > Hi Umang,
> >
> > On Fri, 25 Nov 2022 at 10:47, Umang Jain <umang.jain at ideasonboard.com> wrote:
> > >
> > > Hi Nick,
> > >
> > > On 11/25/22 4:01 PM, Nick Hollinghurst via libcamera-devel wrote:
> > > > Hi all,
> > > >
> > > > I'm having some difficulty with the definitions of Autofocus controls
> > > > (perhaps the question can be generalized to other groups of controls).
> > > > Consider these example sequences of control writes:
> > > >
> > > >   1. set LensPosition
> > > >   2. switch to Continuous mode
> > > >
> > > >   1. switch to Auto mode
> > > >   2. trigger a scan
> > > >
> > > >   1. switch to Manual mode
> > > >   2. set LensPosition
> > > >
> > > > For each pair, the order is important -- it would not work if
> > > > reversed. It's entirely reasonable that an application might want to
> > > > send such a sequence quickly, ideally atomically. And that's a problem
> > > > because the order of controls in a ControlList is not well defined.
> > >
> > > Can you explain a bit more on 'order of controls in a ControlList'?
> > >
> > > Does it mean sending controls with sequential queuing to requests? For .e.g
> > >
> > > Request 1 queued with - ControlList A
> > > Request 2 queued with - ControlList B
> > > Request 3 queued with - ControlList C
> > >
> > > So A -> B -> C is the order of Controls sent to the camera?
> >
> > I'm referring to the case where more than one AF control is set in the
> > same request (all in Request 1 - ControlList A). In our IPA, controls
> > are enumerated in raspberrypi.cpp IPARPi::queueRequest(). The order in
> > which each individual control is visited by the loop is undefined. It
> > means that the same ControlList could have different effects in
> > different builds, or change randomly over time. This appears to be
> > unchanged in the patched version.
> >
>
> If I understand the issue correctly, you're looking at a precedence
> rules for controls being part of the same request ?
>
> You are correct control lists are not ordered, but that doesn't seem
> an issue to me as it's the algorithm that evaluates the controls in a
> request in the order it deems correct.
>
> Getting back to your examples:
>
>    1. set LensPosition
>    2. switch to Continuous mode
>
> This would result in a Request with the following ControlList
> attacched
>
>         { { AfMode: AfModeContinuous }, { LensPosition: 1.2 } }
>
> The definition of LensMode says:
>
>         The LensPosition control is ignored unless the AfMode is set to
>         AfModeManual, though the value is reported back unconditionally in all
>         modes.
>
> Hence your algorithm implementation should go as (pseudo-code)
>
>         queueRequest()
>         {
>
>                 auto mode = controls.get(AfMode);
>                 auto lensPosition = controls.get(LensPosition);
>
>                 if (lensPosition && mode == AfModeManual) {
>                         /*
>                          * Store *lensPosition and use it to move the
>                          * lens
>                          */
>                 }
>         }
>
> To make a similar example using a different algorithm for which we
> have several implemenations, the above use case is exactly the same as
> sending a manual exposure time in a request where the AEGC mode is set
> to auto. As a reference, you can see how that's handled, in example,
> in the RkISP1 IPA:
>
> Request queueing: Inspect the mode and store the exposure value
> control eventually
> https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa/rkisp1/algorithms/agc.cpp#n120
>
> Prepare: decide if auto or manual values have to be used based on the
> current algorithm mode (auto vs manual)
> https://git.libcamera.org/libcamera/libcamera.git/tree/src/ipa/rkisp1/algorithms/agc.cpp#n165
>
> Does this help ?
>
> Thanks
>   j
>

Yes it does -- I see in all those examples the order is deterministic;
in that pseudo-code the mode change takes priority, which is a very
reasonable interpretation of the specification (where the words
"AfMode is set" can refer to both past and concurrent events). I think
that is the clarification I needed. It means that this particular
"atomic sequence" is not admissible in the order that I would have
liked, but the others are OK.

It would bother me if it was left to a particular IPA to deem what was
the correct semantics, rather than it be specified for all
implementations of AF (so that an applications could be portable). So
perhaps spelling this out more clearly in the spec would not go amiss?

 Nick

>
> > > >
> > > > An implementation could (and probably should) use heuristics to ensure
> > > > such combinations of controls are interpreted in the "most useful"
> > > > order. But in the absence of guidance from the specification, it seems
> > > > an application could never rely on this working!
> > >
> > > I was looking at one of the autofocus implementation just yesterday.  If
> > > I understand correctly, you are askng about AF states?
> > >
> > > There is a out-of-tree AF algorithm series from our (IoB) side which
> > > seems to implement/handle AF states [1].
> > >
> > > The state machine can be validated using a capture script as included in
> > > [10/10]. So I believe that's what are you trying to do i.e. simulating a
> > > use case where the state / controls queued doesn't behave as expected.
> > >
> > > [1] https://patchwork.libcamera.org/project/libcamera/list/?series=3174
> >
> > I agree that putting each control in a separate request would fix
> > their order. It would also slow down the application, as operations
> > (like the examples above) that are intended to be atomic, would have
> > to be spaced out over multiple frame-intervals. A specified ordering
> > for multiple controls within the same ControlList, could help here.
> >
> > > >
> > > > Is this something that needs addressing either in documentation, or by
> > > > some kind of change to the API?
> > >
> > > My opinion is that, if something is not logical and is not valid by the
> > > state machine - I think they should be documented. I haven't looked deep
> > > in AF controls specifically, so let's wait for others to pitch in on this!
> > > >
> > > > Regards,
> > > >
> > > >   Nick
> > >