<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 14 Jan 2021 at 06:04, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
<br>
On Tue, Jan 12, 2021 at 07:17:14AM +0000, Naushir Patuck wrote:<br>
> On Mon, 11 Jan 2021 at 23:07, Laurent Pinchart wrote:<br>
> > On Wed, Jan 06, 2021 at 10:06:57AM +0000, Naushir Patuck wrote:<br>
> > > Add an int64_t array control (controls::FrameDurations) to specify the<br>
> > > minimum and maximum (in that order) frame duration to be used by the<br>
> > > camera sensor.<br>
> > ><br>
> > > Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> > > Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> > > Tested-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> > > Reviewed-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br>
> > > ---<br>
> > > src/libcamera/control_ids.yaml | 38 ++++++++++++++++++++++++++++++++++<br>
> > > 1 file changed, 38 insertions(+)<br>
> > ><br>
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml<br>
> > > index 6d6f0fee..d1240beb 100644<br>
> > > --- a/src/libcamera/control_ids.yaml<br>
> > > +++ b/src/libcamera/control_ids.yaml<br>
> > > @@ -306,6 +306,44 @@ controls:<br>
> > > maximum valid value is given by the properties::ScalerCropMaximum<br>
> > > property, and the two can be used to implement digital zoom.<br>
> > ><br>
> > > + - FrameDurations:<br>
> > > + type: int64_t<br>
> > > + description: |<br>
> > > + The minimum and maximum (in that order) frame duration,<br>
> > > + expressed in micro-seconds.<br>
> ><br>
> > s/micro-seconds/microseconds/<br>
> ><br>
> > (The indentation should be 2 spaces btw)<br>
> ><br>
> > > +<br>
> > > + When provided by applications, the control specifies the sensor frame<br>
> > > + duration interval the pipeline has to use. This limits the largest<br>
> > > + exposure time the sensor can use. For example, if a maximum frame<br>
> > > + duration of 33ms is requested (corresponding to 30 frames per second),<br>
> > > + the sensor will not be able to raise the exposure time above 33ms.<br>
> > > + A fixed frame duration is achieved by setting the minimum and maximum<br>
> > > + values to be the same. Setting both values to 0 reverts to using the<br>
> > > + IPA provided defaults.<br>
> > > +<br>
> > > + The maximum frame duration provides the absolute limit to the shutter<br>
> > > + speed computed by the AE algorithm and it overrides any exposure mode<br>
> > > + setting specified with controls::AeExposureMode. Similarly, when a<br>
> > > + manual exposure time is set through controls::ExposureTime, it also<br>
> > > + gets clipped to the limits set by this control.<br>
> ><br>
> > This looks good.<br>
> ><br>
> > > When reported in<br>
> > > + metadata, the control expresses the minimum and maximum frame<br>
> > > + durations used after being clipped to these limits.<br>
> > > +<br>
> ><br>
> > But this sounds weird. The previous part states that FrameDurations has<br>
> > higher priority than all other parameters, but then this sentence says<br>
> > it's clipped by "these limits".<br>
> <br>
> You are right, this does not read correct. I wanted to express that the<br>
> frame durations provided may be further limited by what the sensor mode can<br>
> actually achieve. How about replacing the above paragraph of text with the<br>
> following:<br>
> <br>
> When reported in metadata, the control expresses the minimum and maximum<br>
> frame durations used after being clipped to the sensor provided frame<br>
> duration limits.<br>
<br>
Sounds good to me.<br>
<br>
> > > + \sa AeExposureMode<br>
> > > + \sa ExposureTime<br>
> > > +<br>
> > > + \todo Refer to the frame duration limits property to describe how<br>
> > > + application-provided values gets clipped and reset.<br>
> ><br>
> > It hasn't been long, and the context is already starting to escape me.<br>
> > Would it be possible to expand this just a little bit so that we'll know<br>
> > what it means in 3 months time ?<br>
> <br>
> Perhaps this makes more sense given the rewording above? Or maybe a reword<br>
> as follows:<br>
> <br>
> \todo Refer to the frame duration limits property to describe how<br>
> application-provided values get clipped to what is supported by the sensor<br>
> mode.<br>
> <br>
> Hopefully that makes things more readable?<br>
<br>
Not quite I'm afraid, but maybe it's just too early in the morning :-)<br>
<br>
Is this about documenting how other properties also get clipped by the<br>
sensor mode ? Or something else ?<br></blockquote><div><br></div><div>It's about how the frame durations are clipped by the sensor mode limits - as advertised by the sensor properties in the future.</div><div>We can remove this statement entirely if you do not think it's appropriate, or a rewording as follows:</div><div><br></div><div>\todo Refer to the frame duration limits property (when available) to obtain sensor</div><div>mode limits used for clipping the application-provided values.<br></div><div><br></div><div>Regards,</div><div>Naush</div><div><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> > > +<br>
> > > + \todo Define how to calculate the capture frame rate by<br>
> > > + defining controls to report additional delays introduced by<br>
> > > + the capture pipeline or post-processing stages (ie JPEG<br>
> > > + conversion, frame scaling).<br>
> > > +<br>
> > > + \todo Provide an explicit definition of default control values, for<br>
> > > + this and all other controls.<br>
> > > + size: [2]<br>
> > > +<br>
> > > # ----------------------------------------------------------------------------<br>
> > > # Draft controls section<br>
> > ><br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>