[libcamera-devel] [PATCH v2 1/3] libcamera: controls: Add frame duration control

Naushir Patuck naush at raspberrypi.com
Tue Jun 2 11:38:41 CEST 2020 

Hi Jacopo,


On Mon, 1 Jun 2020 at 16:40, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi Naush,
>
> On Thu, May 28, 2020 at 09:52:51AM +0100, Naushir Patuck wrote:
> > Hi Laurent and Jacopo,
> >
> > I'll address the comments from both emails jointly below:
> >
> > On Thu, 28 May 2020 at 01:55, Laurent Pinchart
> > <laurent.pinchart at ideasonboard.com> wrote:
> > >
> > > Hi Jacopo,
> > >
> > > On Wed, May 27, 2020 at 11:47:34PM +0200, Jacopo Mondi wrote:
> > > > On Mon, May 25, 2020 at 01:23:02AM +0300, Laurent Pinchart wrote:
> > > > > On Wed, May 13, 2020 at 10:11:18AM +0100, Naushir Patuck wrote:
> > > > > > Add a float array control (controls::FrameDurations) to specify the
> > > > > > minimum and maximum (in that order) frame duration to be used by the
> > > > > > camera sensor.
> > > > > >
> > > > > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > > > > ---
> > > > > >  src/libcamera/control_ids.yaml | 14 ++++++++++++++
> > > > > >  1 file changed, 14 insertions(+)
> > > > > >
> > > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > > > > index 77ebc3f9..42694fc7 100644
> > > > > > --- a/src/libcamera/control_ids.yaml
> > > > > > +++ b/src/libcamera/control_ids.yaml
> > > > > > @@ -239,4 +239,18 @@ controls:
> > > > > >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels
> > > > > >          control can only be returned in metadata.
> > > > > >        size: [4]
> > > > > > +
> > > > > > +  - FrameDurations:
> > > > > > +      type: float
> > > > >
> > > > > Do we need sub-microsecond precision, or would a uint32_t control work ?
> > > > > I'd rather have a fixed precision if possible.
> > > > >
> > > > > > +      description: |
> > > > > > +        Specifies the minimum and maximum (in that order) allowable frame
> > > > > > +        duration, in micro-seconds, for the sensor to use. This could also limit
> > > > > > +        the largest exposure times the sensor can use. For example, if a maximum
> > > > > > +        frame duration of 33ms is requested (corresponding to 30 frames per
> > > > > > +        second), the sensor will not be able raise the exposure time above 33ms.
> > > >
> > > > Isn't it s/maximum frame duration/minimum frame duration/ ?
> > > >
> > > > If at least 30fps are requested, the exposure time cannot be longer
> > > > than 33msec ? Am I reading this wrong ?
> > >
> > > I may read it wrong, but I think Naush is correct here. If a *maximum*
> > > frame duration of 33ms is requested [that is, a *minimum* frame rate of
> > > 30fps], the sensor will not be able to *raise* the exposure time above
> > > 33ms.
> > >
> >
> > Yes, my wording in the description matches Laurent's interpretation.
> > I must admit, I do find it more difficult talking about frame
> > durations vs framerates :)
> >
> > > > > > +        Note that the sensor may not always be able to provide the requested
> > > > > > +        frame duration limits depending on its mode configuration.
> > > >
> > > > I've been really confused by this, until I realized I was having an
> > > > hard time trying to figure out how to use this for applications to
> > > > express a request for a precise duration and how should the library
> > > > have used it to report the actual frame durtion in metadata. Until I
> > > > realized this could probably be better interpreted not as a frame
> > > > duration request or result, but as an hint to the AE algorithm to
> > > > clamp its calculation into certain duration limtis.
> > > >
> > > > And I agree a duration range might be a good hint to drive the AE
> > > > routine and if I got the rest of the series right, that's
> > > > what happens in the IPA with these patches.
> > > >
> > > > If that's actually the intention, what about making of this an
> > > > "AeFrameDurationLimits" control, to make it clear it applies to AE and
> > > > use "FrameDuration" as an integer control to represent the requested
> > > > and precise frame duration in requests and metadata respectively.
> > >
> > > I think (Ae)FrameDurationLimits makes sense as a control (and as a
> > > property too, with the caveat that the minimum frame duration - highest
> > > frame rate - can be dependent on the stream resolution), and
> > > FrameDuration makes sense for metadata.
> >
> > Good points here.  When doing this change, I purposely did not want to
> > directly tie this control with AE for a few of reasons, even though
> > there is an interaction:
> >
> > 1) Even though the FrameDurationLimits does restrict what the AE can
> > ask for, in our IPA, they are two distinct and separate operations.
> > In fact, our AE does not even see the frameduration limits that the
> > user set.  The results of AE just get clipped by the
> > FrameDurationLimits.  This is possibly sub-optimal (see my last email
> > for the discussion), but still gives the desired exposure.  In future
> > we will probably optimise this, but given the complexity, will leave
> > it for another change.
>
> Not sure I got this: "AE just get clipped by the FrameDuration"
> doesn't mean that this is an AE-tuning parameter ?

Roughly speaking, the following sequence of events occur in the RPi
IPA (per frame) for AE calculations:

1) AE gets given the current frame shutter speed and gain.
2) If the current exposure does not have the required exposure for the
target brightness, it sets up a digital gain in the ISP to make up the
difference.
3) It also generates a new shutter speed and gain value to program the
sensor to achieve the target brightness without digital gain.  This is
done with no knowledge of what the maximum achievable exposure is
(based on the FrameDuration).
4) In the IPA, we look at this shutter speed and gain value requested
by the AE.  We clip the shutter speed based on what user requested
FrameDuration was provided (if any).

So even though AE results do get clipped by the FrameDuration setting,
AE has no knowledge about this.  In the case of manual exposures (see
below for a bit more context), the FrameDuration clipping will still
apply exactly like above.  Hence my choosing to call this control
FrameDuration to decouple it from what the AE does.  However, I am
happy to change things and rename FrameDuration -> AeFrameDuration (or
AeFrameDurationLimits?) if the consensus is there.

>
> Ideally, I think we should have both an "AE frame duration limits" and
> an "AE exposure limits" to allow applications (with the help of some
> pre-cooked profile possibily) to implement the canonical
> shutter-priority and exposure-priority modes..
>
> > 2) AE does have a way to clip requested shutter speeds using the
> > "Exposure Profile" tuning parameters which offers another degree of
> > freedom to control the interaction of shutter speed with analogue
> > gain.  FrameDurationLimits only controls shutter speed which may not
> > be desirable.
>
> Could this be addressed with an AE-exposure-priority control ?

You are right that we do need controls for both.  The RPi IPA does
have both controls.  For  "AE frame duration limits", we use
FrameDuration (or AeFrameDuration depending on what we decide).  For
"AE exposure limits", we use the control AeExposureMode for exactly
this.

>
> >
>
> I'm not sure, based on your last points, what's your view on this
> control future developments. Should this stay "frameDuration" or
> should this become AeFrameDurationLimit (or AeTargetDuration or
> whatever else appropriate) ?

See below.

>
> > >
> > > More about FrameDuration as a control below.
> > >
> > > > On "FrameDuration" itself: would it make sense to only considered it
> > > > when running with AE off ? In that case it would be applications
> > > > responsibility to opportunely calculate the exposure time and the frame
> > > > duration, but I think it's expected if you run in manual mode. And for
> > > > reference, that's what Android does as well :)
> > > >
> > > > It would also rule out the need to specify how the frameDurationLimits
> > > > would be used when running in manual mode and wanting to configure a
> > > > precise value there, as I guess you would have to give to min and max
> > > > the same value to express that.
> >
> > FrameDurationLimits should only really apply with AE enabled.  AE
> > varies the shutter speed all over the place, that could cause
> > framerate changes that we need to clip.  If AE is disabled and we have
> > a manual shutter speed set, that dictates the frame duration, and any
> > limits will not apply.  Perhaps I should add this in the description?
> >

I have to correct myself here.  FrameDurationLimits will also apply
with manual shutter speeds when AE is disabled.  We could, say, have a
manual shutter speed request of 25 ms, but still want to run at 30fps.
There is a question on what happens if they clash, e.g. request a
manual shutter speed of 66ms, and a FrameDuration of 30fps?  What gets
prioritised?  Currently, in our implementation, FrameDuration will
always be prioritised.

>
> It really depends what developments you foresee here, if the control
> becomes an AE-relatd one I think it's not necessary to specify that.
>
> All in all, to me it seems like "frameDuration" should be reserved for
> manual duration control and to report the actual duration of the
> captured frames.

So circling back to your earlier question:
> Should this stay "frameDuration" or
> should this become AeFrameDurationLimit (or AeTargetDuration or
> whatever else appropriate) ?

Given that I think FrameDurationLimits might apply in both AE and
non-AE operations equally, I think we should keep the name as-is.
However, again, if the consensus is to change, I am more than happy to
make the change.

>
> As I'm working on reporting the camera frame duration limits, synching
> on naming and semantic associated with the control could help both of
> us progressing knowing we won't step on each other toes...

Agreed, we should be consistent on what we decide.  Please do let me
know what the consensus is and I will be happy to follow it.

Regards,
Naush


> Thanks
>   j
>
>
> > >
> > >
> > > > this might open pandora's box, but: is the frame duration a property of
> > > > the camera, or of a single stream ?
> > >
> > > I've opened the same box in my previous e-mail :-) I think the frame
> > > duration is a property of the camera, as all streams share the same
> > > sensor, and are thus slave to the same frame rate. Some streams could
> > > drop frames, but I think I would then report that as a decimation factor
> > > per stream (not sure what the use cases would be though). This being
> > > said, the maximum frame rate (minimum frame duration) can depend on the
> > > configuration of the camera, due to bandwidth limitations. It shouldn't
> > > affect the (Ae)FrameDuration(Limits) control or metadata, but will
> > > affect the (Ae)FrameDurationLimits property. It's something we need to
> > > address eventually, not necessarily as part of this series, we could
> > > start with a limit that doesn't depend on the camera configuration and
> > > then build additional features on top.
> >
> > Agreed.
> >
> > >
> > > > > This looks good to me, but I'd like to discuss the corner cases I can
> > > > > already think about.
> > > > >
> > > > > - Are there use cases for an application to specify a minimum frame
> > > > >   duration only, or a maximum frame duration only (that is, any frame
> > > > >   rate lower than a limit, or any frame rate higher than a limit) ? If
> > > > >   so, how should that be specified ? We could set the minimum value to 0
> > > > >   to mean that the maximum frame rate is unbounded, and the maximum
> > > > >   value to UINT32_MAX to mean that the minimum frame rate is unbounded.
> > > > >   Is there anything I'm overlooking ?
> > > >
> > > > If this becames an AE-related control and you specify that, I think
> > > > you should provide both values. If you don't care about one of the two
> > > > just use the values of the (forthcoming) property that reports the
> > > > min and max frame durations of a camera. If an applications does not
> > > > specify this control, the AE routine runs free, in the limits
> > > > of the camera sensor capabilities.
> > > >
> > > > Does it make sense ?
> > >
> > > Yes that can work. Specifying anything below the minimum possible or
> > > above the maximum possible value supported by the camera will be clamped
> > > to the camera limits anyway.
> > >
> > > > > - Is 0 an acceptable value ? Or should 1 be the minimum value ?
> > > > >
> > > > > - What happens if the requested frame duration isn't achievable ? Should
> > > > >   we specify that the camera will use a frame duration as close to the
> > > > >   requested range as possible ? Or could there be cases where a
> > > > >   different behaviour would be needed ?
> > > > >
> > > > > - Not something we need to address now, but do you see any future
> > > > >   relation between this control and anti-banding (50 or 60Hz flicker
> > > > >   avoidance) ? Anti-banding will mostly restrict possible exposure
> > > > >   times, and should only indirectly interact with the frame duration if
> > > > >   I'm not mistaken, is that correct ?
> > > > >
> > > > > > +
> > > > > > +        \sa ExposureTime
> > > > > > +      size: [2]
> > > > > >  ...
> > >
> > > --
> > > Regards,
> > >
> > > Laurent Pinchart
> >
> > Regards,
> > Naush

More information about the libcamera-devel mailing list