[libcamera-devel] [PATCH 9/9] android: Elevate min duration to 30 FPS if it's lower within 2%
Hanlin Chen
hanlinchen at chromium.org
Fri Apr 15 06:51:45 CEST 2022
Hi Jacopo,
Sorry for the separate email.
On Fri, Apr 15, 2022 at 4:29 AM Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi Han-Lin,
>
> On Thu, Apr 14, 2022 at 08:34:56PM +0800, Hanlin Chen wrote:
> > Hi Jacopo,
> > Sorry for the late reply.
> >
> > On Wed, Apr 6, 2022 at 3:09 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
> >
> > > Hello,
> > > sorry for being late
> > >
> > > On Wed, Apr 06, 2022 at 04:38:53AM +0300, Laurent Pinchart wrote:
> > > > On Wed, Feb 16, 2022 at 03:01:13AM +0200, Laurent Pinchart wrote:
> > > > > On Wed, Feb 16, 2022 at 03:00:43AM +0200, Laurent Pinchart wrote:
> > > > > > Hi Han-Lin,
> > > > > >
> > > > > > Thank you for the patch.
> > > > > >
> > > > > > I'm CC'ing Jacopo and Umang as they've both spent a large amount of
> > > time
> > > > > > solving issues related to frame durations.
> > > > >
> > > > > And now with Umang's correct address.
> > > >
> > > > Jacopo, Umang, any feedback on this ?
> > > >
> > >
> > > Let me recap a bit first.
> > >
> > > We currently adjust all durations > 33333333*101/100 to 33333333.
> > > The idea was to avoid reporting any frame duration < 10^9/30
> > > c7ae5a50c10de1e790855d66842192c766da4dd3
> > > to bring the HAL in par with what the Intel HAL does.
> > >
> > > Umang's 37c41aa6a69985f99f9540dc89d094df61770fdc adjusted the
> > > situation to allow slightly faster durations to be reported, to please
> > > CTS which would have otherwise complained if the actual duration was
> > > smaller than the reported on.
> > >
> > > Hence, so far, we have been dealing with -faster- cameras than the
> > > fixed 10^9/30 limit.
> > >
> > > Umang: do you recall how did we pick the 1% positive tolerance ?
> > >
> > > The issue here is instead that some -slower- cameras might need to be
> > > adjusted and report a smaller frame duration than what they actually
> > > produce. We had a lengthy discussion in the past [1] about how to report
> > > correctly the durations of Nautilus which was reporting a duration of
> > > 29.3 msec while the Intel HAL happily hard-coded 33.333 and CST was
> > > not complaining. We ended up fixing the driver in
> > > "[libcamera-devel] [PATCH 0/2] IMX258 driver fixes". For Soraka
> > > instead, if I'm not mistaken, resolutions that produce < 30 FPS are
> > > not reported as part of the preview streams list (so only resolutions
> > > up to 1080p are reported there).
> > >
> > > So I would like first to know what cameras and what resolutions
> > > is this patch trying to please.
> > >
> > > Also, reading again the thread in [1] I realized CTS allows a 1.5%
> > > tolerance
> > >
> > > " Frame duration must be in the range of [33333333, 33333333], value
> > > 34387000 is out of range [32833332, 33833332])"
> > >
> > > How was 2% picked here ?
> > >
> > > Sorry for making things difficult, but I'm always a bit concerned that
> > > moving a tiny thing here would make some CTS test failures creep in
> > > unnoticed.
> > >
> > > Thanks
> > > j
> > >
> > > [1] [libcamera-internal] Question on camera stream durations
> > >
> >
> > For the front camera of soraka, thus OV5685, which reports frame duration
> > 33336000 for resolution larger than 720p.
> > When calculating FPS, we did a slight elevation it due to CTS in
> > f78f714b4486dbfd62bd62d7a479abc1d98d7495:
> >
> > // unsigned int fps = static_cast<unsigned int> (floor(1e9 /
> > entry.minFrameDurationNsec + 0.05f));
> >
> > It gets floor(29.997600192 + 0.5) = 30, and passes the bar.
> > However, we still report the minFrameDuration as 33336000.
> >
> > I checked with the chrome team about their logic:
> > Chrome would only try the frame rate listed in
> > 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. As a
> > result, chrome rejects resolution > 720p for the front camera.
>
> Is this something new ? I recall we had 1080p for preview on Soraka.
> Am I mistaken ?
>
> > 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.
>
> Full sizes are in facts not reported as 30-FPS capable streams, if I
> recall correctly ?
Another reason is due to the configuration file. The config tuned by
Intel always sets the sensor to full size.
Since we calculate the FrameDurationLimit by sensor size. The min
duration would always be 33389000.
I suppose this is why shipped soraka does not encounter the 1% faster
problem in CTS, since it's actually slower.
It's a little awkward, since 1e9 / 33389000 = 29.49983, slightly less
than the bar 29.5. =.=
The only way I can think of is to change the FrameDurationLimit
calculation in close sourced IPA.
>
> >
> > After a rethink, maybe I should elevate the min frame duration together
> > with where we elevate the FPS, instead of a 2% cap.
> > Or try to centralize all adjustments on FPS/frame duration to an earlier
> > stage.
> >
>
> It would be indeed better to centralize all the adjustments, probably
> at CameraCapabilities::initializeStreamConfigurations() time.
>
> The way we handle minFrameDurations is
>
> CameraCapabilities::initializeStreamConfigurations() {
> int64_t minFrameDuration = frameDurations->second.min().get<int64_t>() * 1000;
> ..
>
> int64_t minFrameDurationCap = 1e9 / 30.0;
> if (minFrameDuration < minFrameDurationCap) {
> float tolerance =
> (minFrameDurationCap - minFrameDuration) * 100.0 / minFrameDurationCap;
>
> /*
> * If the tolerance is less than 1%, do not cap
> * the frame duration.
> */
> if (tolerance > 1.0)
> minFrameDuration = minFrameDurationCap;
> }
>
> streamConfigurations_.push_back({
> res, androidFormat, minFrameDuration, maxFrameDuration,
> });
>
> So here we adjust faster cameras to report max 30 FPS +- 1%
> This was due to get in par with the Intel HAL, but might cause errors
> as some streams are -faster- than what we report here.
>
> When populating the ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT
> camera property, as you noticed, we add a 0.05 to the calculated FPS to
> accept as 30-fps-capable streams that can do > 29.95.
>
> unsigned int fps = static_cast<unsigned int>
> (floor(1e9 / entry.minFrameDurationNsec + 0.05f));
> if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)
> continue;
>
> And we use the adjusted FPS to later populate
> AE_AVAILABLE_TARGET_FPS_RANGE
>
> /*
> * Collect the FPS of the maximum YUV output size to populate
> * AE_AVAILABLE_TARGET_FPS_RANGE
> */
> if (entry.androidFormat == HAL_PIXEL_FORMAT_YCbCr_420_888 &&
> entry.resolution > maxYUVSize) {
> maxYUVSize = entry.resolution;
> maxYUVFps = fps;
> }
>
> int32_t availableAeFpsTarget[] = {
> minFps, maxYUVFps, maxYUVFps, maxYUVFps,
> };
>
> staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,
> availableAeFpsTarget);
>
>
> Then we populate ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS with the
> entry.minFrameDurationNsec value, without adjusting it.
>
> /* Per-stream durations. */
> minFrameDurations.push_back(entry.androidFormat);
> minFrameDurations.push_back(entry.resolution.width);
> minFrameDurations.push_back(entry.resolution.height);
> minFrameDurations.push_back(entry.minFrameDurationNsec);
>
> staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
> minFrameDurations);
>
> So yes, we report a faster FPS in AE_AVAILABLE_TARGET_FPS_RANGE than the min
> frame duration reported in ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS.
>
> Now, a 2% tolerance seems a lot, but CTS has 1.5%, so it might be
> acceptable.
>
> I'm just afraid that if we adjust too much, then CTS will
> detect that we report a min duration that doesn't match the actual
> measured one and complains because a stream slower than expected is
> detected.
>
> Anyway, yes, we should adjust frame durations in a single place to
> avoid reporting inconsistent information in the camera capabilities.
> The question is how much to adjust ? :)
>
> }
> >
> > >
> > > > > > On Wed, Feb 09, 2022 at 03:19:17PM +0800, Han-Lin Chen wrote:
> > > > > > > It's notice that Chrome Camera App filters out the resolutions
> > > which cannot
> > > > > > > achieve 30 FPS. Elevate the minimum frame duration to 30 FPS if
> > > FPS is lower
> > > > > > > within 2%.
> > > > > >
> > > > > > Unless I'm mistaken this patch doesn't depend on the rest of the
> > > series,
> > > > > > so we can review and merge it separately.
> > > > > >
> > > > > > > Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> > > > > > > ---
> > > > > > > src/android/camera_capabilities.cpp | 31
> > > +++++++++++++++++++----------
> > > > > > > 1 file changed, 20 insertions(+), 11 deletions(-)
> > > > > > >
> > > > > > > diff --git a/src/android/camera_capabilities.cpp
> > > b/src/android/camera_capabilities.cpp
> > > > > > > index 55d651f3..e10ab036 100644
> > > > > > > --- a/src/android/camera_capabilities.cpp
> > > > > > > +++ b/src/android/camera_capabilities.cpp
> > > > > > > @@ -655,7 +655,9 @@ int
> > > CameraCapabilities::initializeStreamConfigurations()
> > > > > > > int64_t maxFrameDuration =
> > > frameDurations->second.max().get<int64_t>() * 1000;
> > > > > > >
> > > > > > > /*
> > > > > > > - * Cap min frame duration to 30 FPS with
> > > 1% tolerance.
> > > > > > > + * Cap min frame duration to 30 FPS with
> > > 1% tolerance,
> > > > > > > + * and elevate min frame duration to 30
> > > FPS with 2%
> > > > > > > + * tolerance.
> > > > > > > *
> > > > > > > * 30 frames per second has been validated
> > > as the most
> > > > > > > * opportune frame rate for quality
> > > tuning, and power
> > > > > > > @@ -675,17 +677,24 @@ int
> > > CameraCapabilities::initializeStreamConfigurations()
> > > > > > > * to the in-development configuration API
> > > rework.
> > > > > > > */
> > > > > > > int64_t minFrameDurationCap = 1e9 / 30.0;
> > > > > > > - if (minFrameDuration <
> > > minFrameDurationCap) {
> > > > > > > - float tolerance =
> > > > > > > - (minFrameDurationCap -
> > > minFrameDuration) * 100.0 / minFrameDurationCap;
> > > > > > > + float tolerance =
> > > > > > > + (minFrameDurationCap -
> > > minFrameDuration) * 100.0 / minFrameDurationCap;
> > > > > > >
> > > > > > > - /*
> > > > > > > - * If the tolerance is less than
> > > 1%, do not cap
> > > > > > > - * the frame duration.
> > > > > > > - */
> > > > > > > - if (tolerance > 1.0)
> > > > > > > - minFrameDuration =
> > > minFrameDurationCap;
> > > > > > > - }
> > > > > > > + /*
> > > > > > > + * If the tolerance is less than 1%, do
> > > not cap
> > > > > > > + * the frame duration.
> > > > > > > + */
> > > > > > > + if (tolerance > 1.0)
> > > > > > > + minFrameDuration =
> > > minFrameDurationCap;
> > > > > > > +
> > > > > > > + /*
> > > > > > > + * Some applications, ex: Chrome Camera
> > > App filters out
> > > > > > > + * the resolutions which cannot achieve 30
> > > FPS. Elevate
> > > > > > > + * the minimum frame duration to 30 FPS if
> > > FPS is lower
> > > > > > > + * within 2%.
> > > > > > > + */
> > > > > > > + if (tolerance < 0 && tolerance > -2.0)
> > > > > > > + minFrameDuration =
> > > minFrameDurationCap;
> > > > > > >
> > > > > > > streamConfigurations_.push_back({
> > > > > > > res, androidFormat,
> > > minFrameDuration, maxFrameDuration,
> > > >
> > > > --
> > > > Regards,
> > > >
> > > > Laurent Pinchart
> > >
More information about the libcamera-devel
mailing list