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

[Naushir Patuck]

On Wed, 11 Oct 2023 at 13:02, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> 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'

iqunstable != unconverged

You can have iqunstable == false and converged == true or false.
But of course, if iqunstable == true, it implies converged == false
based on the definition above.

So...

'(!IqUnstable) == Stable algorithm state but does not necessarily ==
Converged Algorithms'

The full truth table of states is thus as follows:

iqunstable | converged | App behaviour
0          | 0         | Display viewfinder but don't capture JPEG
0          | 1         | Display viewfinder and capture JPEG
1          | 0         | Do not use frame at all

That probably does not clear things up... :)


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