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

[Naushir Patuck]

Hi Jacopo,


On Wed, 23 Jun 2021 at 11:32, Jacopo Mondi <jacopo at jmondi.org> wrote:

> 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 ?
>

That all sounds good to me, but I would like David to have his say as
well.  It will probably be the case that our IPA will treat controls::AeLock
the same as our "Paused" state.


> 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.
>

We currently track master, so will have to update our apps separately when
this
change goes through.  Eventually, we will freeze versions in our
distribution to
avoid breakages, but we are not there yet.

Regards,
Naush


> >
> >  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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210623/536b2067/attachment.htm>