[libcamera-devel] [PATCH 1/1] libcamera: controls: Add a control for IQ stability

[Naushir Patuck]

Hi Kieran,

On Wed, 11 Oct 2023 at 12:00, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting Naushir Patuck via libcamera-devel (2023-09-18 08:53:57)
> > Hi David,
> >
> > Thank you for this work.
> >
> > On Tue, 12 Sept 2023 at 15:23, David Plowman via libcamera-devel
> > <libcamera-devel at lists.libcamera.org> wrote:
> > >
> > > The IqUnstable metadata can be used by IPAs to indicate to an
> > > application that they have not settled sufficiently to produce
> > > reliable image quality. Applications would be advised to avoid using
> > > frames flagged in this way.
> > >
> > > One example would be when the camera starts, when the AEC/AGC might
> > > oscillate for a few frames.
> > >
> > > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> >
> > I do like this term better than the more generic startup frames we used before.
> > FYI, I have implemented this control in the RPi pipeline handler that
> > folks can peek at here:
> > https://github.com/naushir/libcamera/tree/iq_stability
> >
> > Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
> >
> > > ---
> > >  src/libcamera/control_ids.yaml | 20 ++++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > >
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > index f2e542f4..b96e1272 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -774,6 +774,26 @@ controls:
> > >              Continuous AF is paused. No further state changes or lens movements
> > >              will occur until the AfPauseResume control is sent.
> > >
> > > +  - IqUnstable:
> > > +      type: bool
> > > +      description: |
> > > +        The value true indicates that the camera algorithms have not settled
> > > +        sufficiently to generate images of reliable quality. The application
> > > +        receiving this frame is advised to drop it and wait for a frame where
> > > +        this metadata reports false (or is absent).
> > > +
> > > +        One example of this would be when the camera system starts. It may be
> > > +        trying to adapt very quickly to the ambient conditions, resulting in a
> > > +        few frames where the image brightness may be subject to unusually
> > > +        extreme oscillations.
> > > +
> > > +        The control may report true at other times, for example when an HDR mode
> > > +        is enabled. Here too there may be a few frames of unpredictable exposure
> > > +        until the algorithms have settled.
> > > +
> > > +        The value false (or absence of the control) indicates that this is a
> > > +        normal frame.
> > > +
>
> It sounds like this is a flag that tells the application to look at
> other aspects too. (Or just ignore or drop the frame).
>
> I.e. the setting of this IqUnstable flag might be because any of the
> AWB/AEGC/AF/HDR state machines are not reporting a converged state.

I've mentioned this before, but this control is not meant to be used
for reporting AWB/AE convergence state.  We have separate controls
(AeLocked and AwbLocked) for that.  There is a subtle but important
distinction between IqStability and the *Locked controls.  The former
informs the application that the output frame should not be consumed
because the IQ could be entirely wrong and/or very unstable because
the algorithms are in an undetermined state (e.g. on first startup or
a transition from non-hdr to hdr mode).  The latter controls are used
when the algorithms are running in a stable state (i.e. the frames are
perfectly valid to consume), but the AE/AWB has not reached its final
target due to filtering or a scene change (so, for example, best to
avoid taking a still capture just yet).

>
> Do the controls for those algorithms already have a way to report their
> state?

Yes, see above.

>
> I do think a global 'this flag means you need to check things' could
> make sense in this instance, but I'm keen to clarify the relationship of
> what an application should do here.
>
> It also feels like this could be set 'automatically' by a common layer
> that processes metadata before it gets back to the application and would
> set it for all pipeline handlers if one of AWB/AEGC/AF/HDR reports an
> incorrect state?

Given my above understanding of what this control is trying to convey
to the application, it is not related in any way to the other
algorithms locked state.  So I don't think we can set this in a common
layer based on the state of other controls.

Hope that makes sense!

Naush


>
>
>
> > >    # ----------------------------------------------------------------------------
> > >    # Draft controls section
> > >
> > > --
> > > 2.30.2
> > >