<div dir="ltr"><div dir="ltr">Hi Jacopo,<div>Sorry for the late reply.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Apr 6, 2022 at 3:09 PM 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">Hello,<br>
sorry for being late<br>
<br>
On Wed, Apr 06, 2022 at 04:38:53AM +0300, Laurent Pinchart wrote:<br>
> On Wed, Feb 16, 2022 at 03:01:13AM +0200, Laurent Pinchart wrote:<br>
> > On Wed, Feb 16, 2022 at 03:00:43AM +0200, Laurent Pinchart wrote:<br>
> > > Hi Han-Lin,<br>
> > ><br>
> > > Thank you for the patch.<br>
> > ><br>
> > > I'm CC'ing Jacopo and Umang as they've both spent a large amount of time<br>
> > > solving issues related to frame durations.<br>
> ><br>
> > And now with Umang's correct address.<br>
><br>
> Jacopo, Umang, any feedback on this ?<br>
><br>
<br>
Let me recap a bit first.<br>
<br>
We currently adjust all durations > 33333333*101/100 to 33333333.<br>
The idea was to avoid reporting any frame duration < 10^9/30<br>
c7ae5a50c10de1e790855d66842192c766da4dd3<br>
to bring the HAL in par with what the Intel HAL does.<br>
<br>
Umang's 37c41aa6a69985f99f9540dc89d094df61770fdc adjusted the<br>
situation to allow slightly faster durations to be reported, to please<br>
CTS which would have otherwise complained if the actual duration was<br>
smaller than the reported on.<br>
<br>
Hence, so far, we have been dealing with -faster- cameras than the<br>
fixed 10^9/30 limit.<br>
<br>
Umang: do you recall how did we pick the 1% positive tolerance ?<br>
<br>
The issue here is instead that some -slower- cameras might need to be<br>
adjusted and report a smaller frame duration than what they actually<br>
produce. We had a lengthy discussion in the past [1] about how to report<br>
correctly the durations of Nautilus which was reporting a duration of<br>
29.3 msec while the Intel HAL happily hard-coded 33.333 and CST was<br>
not complaining. We ended up fixing the driver in<br>
"[libcamera-devel] [PATCH 0/2] IMX258 driver fixes". For Soraka<br>
instead, if I'm not mistaken, resolutions that produce < 30 FPS are<br>
not reported as part of the preview streams list (so only resolutions<br>
up to 1080p are reported there).<br>
<br>
So I would like first to know what cameras and what resolutions<br>
is this patch trying to please.<br>
<br>
Also, reading again the thread in [1] I realized CTS allows a 1.5%<br>
tolerance<br>
<br>
" Frame duration must be in the range of [33333333, 33333333], value<br>
34387000 is out of range [32833332, 33833332])"<br>
<br>
How was 2% picked here ?<br>
<br>
Sorry for making things difficult, but I'm always a bit concerned that<br>
moving a tiny thing here would make some CTS test failures creep in<br>
unnoticed.<br>
<br>
Thanks<br>
j<br>
<br>
[1] [libcamera-internal] Question on camera stream durations<br></blockquote><div><br></div><div><font face="arial, sans-serif" style="background-color:rgb(255,255,255)">For the front camera of soraka, thus OV5685, which reports frame duration 33336000 for resolution larger than 720p.</font></div><div><font face="arial, sans-serif" style="background-color:rgb(255,255,255)">When calculating FPS, we did a slight elevation it due to CTS in </font>f78f714b4486dbfd62bd62d7a479abc1d98d7495<span style="font-family:arial,sans-serif">:</span></div><div><span style="font-family:arial,sans-serif"><br></span></div><div><font face="arial, sans-serif" style="background-color:rgb(255,255,255)">// unsigned int fps = static_cast<unsigned int> (floor(1e9 / entry.minFrameDurationNsec + 0.05f));</font></div><div><font face="arial, sans-serif" style="background-color:rgb(255,255,255)"><br></font></div><div><font face="arial, sans-serif" style="background-color:rgb(255,255,255)">It gets floor(29.997600192 + 0.5) = 30, and passes the bar.</font></div><div><font face="arial, sans-serif" style="background-color:rgb(255,255,255)">However, we still report the minFrameDuration as 33336000.</font></div><div><font face="arial, sans-serif" style="background-color:rgb(255,255,255)"><br></font></div><div><font face="arial, sans-serif" style="background-color:rgb(255,255,255)">I checked with the chrome team about their logic:</font></div><div><font face="arial, sans-serif" style="background-color:rgb(255,255,255)">Chrome would only try the frame rate listed in <span style="color:rgb(32,33,36);font-size:14px;white-space:pre-wrap">ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES, which is elevated to 30 in the case, and filter out the stream with frame duration with 33336000, since its calculated FPS would be 29.97. </span></font><span style="font-family:arial,sans-serif;color:rgb(32,33,36);font-size:14px;white-space:pre-wrap">As a result, chrome rejects resolution > 720p for the front camera.</span></div><div><span style="color:rgb(32,33,36);font-family:arial,sans-serif;font-size:14px;white-space:pre-wrap">On the other hand, the back camera reports 33389000 for the full size. The 2% is from the value, which is about 1.6% from 33333333.</span><span style="color:rgb(32,33,36);font-size:14px;white-space:pre-wrap;background-color:rgb(255,255,255)"><font face="arial, sans-serif" style=""><br></font></span></div><div><span style="color:rgb(32,33,36);font-family:arial,sans-serif;font-size:14px;white-space:pre-wrap"><br></span></div><div><span style="color:rgb(32,33,36);font-family:arial,sans-serif;font-size:14px;white-space:pre-wrap">After a rethink, maybe I should </span><span style="color:rgb(32,33,36);font-family:arial,sans-serif;font-size:14px;white-space:pre-wrap">elevate the min frame duration together with where we elevate the FPS, instead of a 2% cap.</span></div><div><span style="color:rgb(32,33,36);font-family:arial,sans-serif;font-size:14px;white-space:pre-wrap">Or try to centralize all adjustments on FPS/frame duration to an earlier stage.</span></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>
> > > On Wed, Feb 09, 2022 at 03:19:17PM +0800, Han-Lin Chen wrote:<br>
> > > > It's notice that Chrome Camera App filters out the resolutions which cannot<br>
> > > > achieve 30 FPS. Elevate the minimum frame duration to 30 FPS if FPS is lower<br>
> > > > within 2%.<br>
> > ><br>
> > > Unless I'm mistaken this patch doesn't depend on the rest of the series,<br>
> > > so we can review and merge it separately.<br>
> > ><br>
> > > > Signed-off-by: Han-Lin Chen <<a href="mailto:hanlinchen@chromium.org" target="_blank">hanlinchen@chromium.org</a>><br>
> > > > ---<br>
> > > > src/android/camera_capabilities.cpp | 31 +++++++++++++++++++----------<br>
> > > > 1 file changed, 20 insertions(+), 11 deletions(-)<br>
> > > ><br>
> > > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp<br>
> > > > index 55d651f3..e10ab036 100644<br>
> > > > --- a/src/android/camera_capabilities.cpp<br>
> > > > +++ b/src/android/camera_capabilities.cpp<br>
> > > > @@ -655,7 +655,9 @@ int CameraCapabilities::initializeStreamConfigurations()<br>
> > > > int64_t maxFrameDuration = frameDurations->second.max().get<int64_t>() * 1000;<br>
> > > ><br>
> > > > /*<br>
> > > > - * Cap min frame duration to 30 FPS with 1% tolerance.<br>
> > > > + * Cap min frame duration to 30 FPS with 1% tolerance,<br>
> > > > + * and elevate min frame duration to 30 FPS with 2%<br>
> > > > + * tolerance.<br>
> > > > *<br>
> > > > * 30 frames per second has been validated as the most<br>
> > > > * opportune frame rate for quality tuning, and power<br>
> > > > @@ -675,17 +677,24 @@ int CameraCapabilities::initializeStreamConfigurations()<br>
> > > > * to the in-development configuration API rework.<br>
> > > > */<br>
> > > > int64_t minFrameDurationCap = 1e9 / 30.0;<br>
> > > > - if (minFrameDuration < minFrameDurationCap) {<br>
> > > > - float tolerance =<br>
> > > > - (minFrameDurationCap - minFrameDuration) * 100.0 / minFrameDurationCap;<br>
> > > > + float tolerance =<br>
> > > > + (minFrameDurationCap - minFrameDuration) * 100.0 / minFrameDurationCap;<br>
> > > ><br>
> > > > - /*<br>
> > > > - * If the tolerance is less than 1%, do not cap<br>
> > > > - * the frame duration.<br>
> > > > - */<br>
> > > > - if (tolerance > 1.0)<br>
> > > > - minFrameDuration = minFrameDurationCap;<br>
> > > > - }<br>
> > > > + /*<br>
> > > > + * If the tolerance is less than 1%, do not cap<br>
> > > > + * the frame duration.<br>
> > > > + */<br>
> > > > + if (tolerance > 1.0)<br>
> > > > + minFrameDuration = minFrameDurationCap;<br>
> > > > +<br>
> > > > + /*<br>
> > > > + * Some applications, ex: Chrome Camera App filters out<br>
> > > > + * the resolutions which cannot achieve 30 FPS. Elevate<br>
> > > > + * the minimum frame duration to 30 FPS if FPS is lower<br>
> > > > + * within 2%.<br>
> > > > + */<br>
> > > > + if (tolerance < 0 && tolerance > -2.0)<br>
> > > > + minFrameDuration = minFrameDurationCap;<br>
> > > ><br>
> > > > streamConfigurations_.push_back({<br>
> > > > res, androidFormat, minFrameDuration, maxFrameDuration,<br>
><br>
> --<br>
> Regards,<br>
><br>
> Laurent Pinchart<br>
</blockquote></div></div>