[libcamera-devel] [RFC PATCH 03/14] controls: Replace AeLocked with AeState, and add AeLock

[Jacopo Mondi]

Hi Naush,

On Wed, Jun 23, 2021 at 10:06:00AM +0100, Naushir Patuck wrote:
> Hi Jacopo and David,
>
> On Wed, 23 Jun 2021 at 09:12, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> <snip>
>
>
> > >
> > > Are we saying that AeDisable means "stop adjusting the AEC/AGC
> > > immediately, whether it's converged or not", but AeLock means "wait
> > > until it's converged and then immediately stop adjusting the AEC/AGC"?
> >
> > If AeLock is set on a Converged state then yes, the values are not
> > changed until the lock is lifted.
> >
> > Does the transition table repoted here helps ?
> >
> > https://developer.android.com/reference/android/hardware/camera2/CaptureResult#CONTROL_AE_STATE
> >
> > Thanks
> >    j
> >
>
> Having a brief look at the above document, I think
> Jacopo's description makes
> sense now.  Enabling AeLock locks the current AE computed values, whether
> the algorithm has converged or not.  However, with AeLock set to on, other
> controls are effectively ignored by the AE (e.g. aePrecaptureTrigger, maybe
> other things as well).  Not sure why this is the case, as the application
> surely
> knows what state it wants the AE to be in...?

So my previous example is slightly wrong, as the AeLock was meant to
be set only once the AE has converged, not in request #0.

>
> We don't deal with pre-capture sequences in RPi AE, so for us AeLock and
> AeDisable are actually the same thing.  Thinking about it, even with a
> pre-capture
> sequence, we could probably say the same.  This may also be the case for
> other vendor AE algorithms, but Android makes these states explicit.

I think that's fine.

That's more of a philosophical question, but I don't think we should
try to impose all the IPA implementation to behave exactly the same,
as that would impede the implementation of the most advanced features.
The means the most performant application won't behave exactly the
same between different platform, but I think at least in the controls
definition we should try to get to the minimum common denominator that
satisfies most needs. After all I think Android does the same, the
definition is there for all platforms, but each implementation is
slightly different and indeed it's usually the phone manufacturer
provided camera app that actually makes use of the most advanced
features, while a generic camera app is usually limited to the
simplest use cases.

For this specific use case (the AECG lock/state/enable), let me try to
summarize how we could move forward:

- Replace controls::AeLocked with a more descriptive AeState where the
  current 'locked' is represented by the 'Converged' state.
- Introduce controls::AeLock (which for RPi won't be supported)
- We have controls::AeEnable which more or less maps to
  android.AeMode. AeMode has more states which include control of the
  flash. We do not strictly have to support it now afaict but moving
  controls::AeMode from being a boolean on/off to a list of states
  might help that transition to happen in future.

Do you agree with my understanding here ?

The controls::AeLocked -> controls::AeState[Converged] will require us
and the corresponding RPi camera app update to happen synchronously
even if I admit I don't know how you distribute libcamera. IOW, are the
camera apps compiled against the most recent master or against a
frozen version? So that master can freely advance and the app updates can
follow later once you update the version of libcamera you ship.

>
>  Or it could be that I've just completely misunderstood that document :-)

The fact we all keep asking that to ourselves makes me wonder if we're
all a bunch of idiots or if we're actually exploring rather new
territories in the open camera support landscape. I think time will
tell :)

Thanks
   j

PS As you might have noticed we're actually in the process of
addressing the most advanced controls that android requires for
high-end devices, so expect this kind of discussions to happen again
in future :)

>
> Naush