<div dir="ltr"><div dir="ltr">Hi Jacopo,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 10 Dec 2020 at 17:44, Jacopo Mondi <<a href="mailto:jacopo@jmondi.org">jacopo@jmondi.org</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>
thanks for the update<br>
<br>
On Thu, Dec 10, 2020 at 04:33:35PM +0000, Naushir Patuck wrote:<br>
> Add a float 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>
> ---<br>
> src/libcamera/control_ids.yaml | 40 ++++++++++++++++++++++++++++++++++<br>
> 1 file changed, 40 insertions(+)<br>
><br>
> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml<br>
> index 6d6f0fee..5ee31865 100644<br>
> --- a/src/libcamera/control_ids.yaml<br>
> +++ b/src/libcamera/control_ids.yaml<br>
> @@ -306,6 +306,46 @@ 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>
> + When provided by applications, the control specifies the sensor frame<br>
> + duration interval the pipeline has to use. This could also limit the<br>
> + largest exposure times the sensor can use. For example, if a maximum<br>
<br>
Is 'times' intentional ? Or did you mean 'time' ? Honest question..<br></blockquote><div><br></div><div>You are correct, it should be 'time' :)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> + frame duration of 33ms is requested (corresponding to 30 frames per<br>
> + second), the sensor will not be able to raise the exposure time above<br>
> + 33ms. A fixed frame duration is achieved by setting the minimum and<br>
> + maximum values to be the same. Setting both values to 0 resets the<br>
> + frame duration to the IPA defaults.<br>
<br>
I would drop the last statement as we should better think where<br>
defaults will be specified (a property or implicitly in the IPA). I'll<br>
rather have a \todo.<br></blockquote><div><br></div><div>Good point. I will add a todo for this. FYI, for Raspberry Pi, if you set 0, it does return back to the IPA specified default. If we decide this is not appropriate, it is an easy modification.</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>
> + The maximum frame duration provides the absolute limit to the shutter<br>
> + speed available to the AE algorithm. This limit also overrides any<br>
> + limits set by the exposure mode (AeExposureMode). Similarly, a manual<br>
> + ExposureTime value provided by the application may also be clipped by<br>
> + this limit.<br>
<br>
I would re-phrase this as:<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 in the limits set by this control.<br>
<br>
to make it more 'imperative'. It might sound all very "must/shall", but<br>
I fear documenting things as "may/might" as it means one application<br>
might behave differently when run on different platforms. I would<br>
prefer to start strict, and in case the need arise, re-think how to<br>
make these precedence relationships controllable ?<br></blockquote><div><br></div><div>Sure, I will reword that to your suggested text.</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>
> + \sa AeExposureMode<br>
> + \sa ExposureTime<br>
> +<br>
> + \todo Refer to the frame duration limits property to describe how<br>
> + application-provided values gets clipped.<br>
<br>
If you agree the statement above about reset should be dropped, I<br>
would "gets clipped and reset".<br></blockquote><div><br></div><div>Agreed. Will post a new version with the updates shortly.</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>
> + When reported by pipelines, the control expresses the duration of the<br>
> + sensor frame used to produce streams part of the completed Request.<br>
> + The minimum and maximum values shall then be the same, as the sensor<br>
> + frame duration is a fixed parameter. The sensor frame duration is one<br>
> + of the parameter that defines the capture frame rate but it does not<br>
> + alone provide enough information to fully calculate it as it does not<br>
> + account for pipeline processing delays.<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>
<br>
Thanks, minor apart this looks good<br>
Reviewed-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br>
<br>
Thanks<br>
j<br>
<br>
> # ----------------------------------------------------------------------------<br>
> # Draft controls section<br>
><br>
> --<br>
> 2.25.1<br>
><br>
> _______________________________________________<br>
> libcamera-devel mailing list<br>
> <a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a><br>
> <a href="https://lists.libcamera.org/listinfo/libcamera-devel" rel="noreferrer" target="_blank">https://lists.libcamera.org/listinfo/libcamera-devel</a><br>
</blockquote></div></div>