[libcamera-devel] [PATCH 9/9] android: Elevate min duration to 30 FPS if it's lower within 2%
Jacopo Mondi
jacopo at jmondi.org
Thu Apr 14 22:29:17 CEST 2022
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 ?
>
> 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