[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 05:56:35 CEST 2022
Hi Jacopo,
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 ?
CCA will accept 1080p but silently fall back to 480p. I notice it by
checking its expert mode accidentally.
>
> > 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 ? :)
Yes, I was thinking of elevating it to where we elevate the FPS. Maybe:
unsigned int fps = static_cast<unsigned int>
(floor(1e9 / entry.minFrameDurationNsec + 0.05f));
/*
* Since CTS calculate FPS with additional 0.05f before taking
* floor. Adjust the reported min frame duration accordingly.
*/
int64_t adjustedMinFrameDurationNsec = 1e9 / fps;
if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)
continue;
And then we can abandon the 2% in the patch.
>
> }
> >
> > >
> > > > > > 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