[libcamera-devel] [PATCH 1/1] libcamera: controls: Add StartupFrame control

[Naushir Patuck]

Hi Kieran,

On Mon, 12 Jun 2023 at 23:33, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting David Plowman via libcamera-devel (2023-06-12 10:43:52)
> > Hi again
> >
> > Thanks for the discussion on this! Mostly I probably want to reiterate
> > what Naush has said.
> >
> > On Mon, 5 Jun 2023 at 10:32, Naushir Patuck <naush at raspberrypi.com> wrote:
> > >
> > > Hi Laurent,
> > >
> > > David is away this week so his reply will be delayed.
> > >
> > > On Mon, 5 Jun 2023 at 09:17, Laurent Pinchart
> > > <laurent.pinchart at ideasonboard.com> wrote:
> > > >
> > > > Hello,
> > > >
> > > > On Wed, May 31, 2023 at 01:57:16PM +0100, Naushir Patuck via libcamera-devel wrote:
> > > > > Hi David,
> > > > >
> > > > > Thank you for this patch.  Indeed removing the drop frame logic the pipeline
> > > > > handler would simplify things!
> > > >
> > > > That's a change I would welcome :-)
> > > >
> > > > > On Wed, 31 May 2023 at 13:50, David Plowman via libcamera-devel wrote:
> > > > > >
> > > > > > This control is passed back in a frame as metadata to indicate whether
> > > > > > the camera system is still in a startup phase, and the application is
> > > > > > advised to avoid using the frame.
> > > > > >
> > > > > > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > > > > > ---
> > > > > >  src/libcamera/control_ids.yaml | 15 +++++++++++++++
> > > > > >  1 file changed, 15 insertions(+)
> > > > > >
> > > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > > > > index adea5f90..4742d907 100644
> > > > > > --- a/src/libcamera/control_ids.yaml
> > > > > > +++ b/src/libcamera/control_ids.yaml
> > > > > > @@ -694,6 +694,21 @@ controls:
> > > > > >              Continuous AF is paused. No further state changes or lens movements
> > > > > >              will occur until the AfPauseResume control is sent.
> > > > > >
> > > > > > +  - StartupFrame:
> > > > > > +      type: bool
> > > > > > +      description: |
> > > > > > +        The value true indicates that the camera system is still in a startup
> > > > > > +        phase where the output images may not be reliable, or that certain of
> > > > > > +        the control algorithms (such as AEC/AGC, AWB, and possibly others) may
> > > > > > +        still be changing quite rapidly.
> > > >
> > > > I think we need to decide with a bit more details what constitute a
> > > > "startup frame" and what doesn't, or we will have all kinds of
> > > > inconsistent behaviour.
> > > >
> > > > I read it that we have multiple criteria on which we base the decision:
> > > >
> > > > - output images not being "reliable"
> > > > - 3A algorithms convergence
> > > > - "possibly others"
> > > >
> > > > The second criteria is fairly clear, but I'm thinking we should possibly
> > > > exclude it from the startup frames and report convergence of the 3A
> > > > algorithms instead.
> > >
> > > The 3A "startup" convergence is included as a criteria here because during
> > > startup, we drive the AE/AWB/ALSC as aggressively (i.e. no filtering) as
> > > possible to achieve convergence as fast as we possibly can.  This means the
> > > image output can oscillate quite badly - hence applications should avoid
> > > displaying or consuming them.
> > >
> > > I feel that this startup phase needs to be treated differently compared to a
> > > normal "converging" phase where the algorithms are filtering the outputs and the
> > > transitions our smooth, and conversely the application can (and probably should)
> > > display/consume these frames.
> >
> > Basically, yes. The startup phase is different. For some algorithms we
> > indicate "converged" already, but that doesn't mean you shouldn't
> > display "unconverged" frames - of course we do. But during startup we
> > can expect algorithms possibly to overshoot and the recommendation is
> > simply not to use them. I suppose it doesn't mean an application
> > couldn't do something different - but we want to make the recommended
> > behaviour easy.
>
> I'm pleased to see this metadata introduction. I already see visible
> 'flashes' on the RkISP pipeline handler in my tests as the frames aren't
> dropped there, nor are they reported to the applications as being
> not-yet-ready for consumption.
>
> I know in the RPi pipeline handler it's well supported to capture a
> single frame with pre-set manual controls. I guess we would expect
> everything to be reported as converged in that instance, so it wouldn't
> get marked as a StartupFrame? (Just trying to see if I can find odd
> corner cases, but I expect this would already be fine).

If the user requests fully manual controls (i.e. shutter speed, analogue gain,
manual red/blue gains), then we don't signal any drop frames for algorithm
convergence.  However, we would still signal drop frames if the sensor is
known to produce N garbage frames on startup.

Regards,
Naush


>
>
>
> >
> > >
> > > >
> > > > The first and third criteria are quite vague. If I recall correctly, the
> > > > first one includes bad frames from the sensor (as in completely
> > > > corrupted frames, such as frames that are all black or made of random
> > > > data). Those are completely unusable for applications, is there a value
> > > > in still making them available instead of dropping them correctly ?
> > >
> > > This is what we do now (plus dropping the startup frames that may be good from
> > > the sensor but still in the 3A startup phase), but I think a key goals of this
> > > change is be consistent (i.e. let the application handle all frames always), and
> > > also help with avoiding some framebuffer allocations to help with startup
> > > convergence performance.
> > >
> > > Regards,
> > > Naush
> > >
> > > >
> > > > What are the other reasons a frame would be a "startup frame" ?
> >
> > There is a genuine discussion to have here, I think. There are some
> > very clear reasons for telling an application not to use a frame - if
> > it's complete garbage, for example. And there are some more "advisory"
> > ones, which is what we can have for several frames. Categorising the
> > "usability" of a frame is certainly an idea, though I don't know if we
> > want to do that without a clear reason.
>
> I expect minimising latency to getting the first 'usable' frame to be a
> priority on occasions here - so if applications can know some quantative
> level of detail about 'how' usable the frame is, that could potetnially
> reduce the number of frames an appliction might discard in some use
> cases.
>
> I'm not sure how easy 'quantifying' the usability of the frame will be
> though.
>
>
>
> >
> > But it's also very hard to predict the behaviour of all pipeline
> > handlers in this respect. Perhaps someone has a pipeline handler where
> > the first frame after every mode switch comes out badly, perhaps it
> > needs to consume a frame first for its IPAs to sort themselves out.
> > But maybe another PH doesn't. So the idea is to have a portable way to
> > indicate this kind of thing so that applications don't have to start
> > guessing the behaviour underneath.
> >
> > Note that these "behaviours" can be quite complex. For us, it's
> > completely different if you request fixed exposure and gain, and
> > colour gains. But only slightly different, IIRC, if you don't give the
> > colour gains. Forcing this kind of stuff into applications,
> > particularly ones that expect to use different PHs, feels quite
> > undesirable.
> >
> > Don't know if I've really added anything, but I hope it makes a bit of sense!
> >
> > Thanks
> > David
> >
> > > >
> > > > > > +
> > > > > > +        Applications are advised to avoid using these frames. Mostly, they will
> > > > > > +        occur when the camera system starts for the first time, although,
> > > > > > +        depending on the sensor and the implementation, they could occur at
> > > > > > +        other times.
> > > > > > +
> > > > > > +        The value false indicates that this is a normal frame.
>
> Presumably an application can also 'assume' that a lack of this metadata
> would also indicate 'false' ...
>
> > > > >
> > > > > Just throwing it out there, but would it be useful if this control was an
> > > > > integer with the count of startup frames left to handle? A value of 0, or the
> > > > > absence of the control would indicate this is a "valid" frame.
>
> Ah yes - that's what I mean ;-)
>
>
>
> > > > >
> > > > > > +
> > > > > >    # ----------------------------------------------------------------------------
> > > > > >    # Draft controls section
> > > > > >
> > > >
> > > > --
> > > > Regards,
> > > >
> > > > Laurent Pinchart