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

[Kieran Bingham]

Quoting Naushir Patuck (2023-10-11 12:42:48)
> 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

But it 'is' reporting *not convergence* state. Therefore the lack of
this flag could be implied to mean Iq 'stable' (converged).


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

perhaps it sounds like some of that should be captured in the control
documentation?


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

I'm still stuck at the logic of:

 '(!IqUnstable) == Stable image quality == Converged Algorithms'

> Hope that makes sense!
> 
> Naush
> 
> 
> >
> >
> >
> > > >    # ----------------------------------------------------------------------------
> > > >    # Draft controls section
> > > >
> > > > --
> > > > 2.30.2
> > > >