[libcamera-devel] [PATCH 9/9] android: Elevate min duration to 30 FPS if it's lower within 2%
Jacopo Mondi
jacopo at jmondi.org
Fri Apr 15 09:49:52 CEST 2022
Hi Han-Lin,
On Fri, Apr 15, 2022 at 12:51:45PM +0800, Hanlin Chen wrote:
> 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.
Almost. In the revision I have here for ov13858 9 modes gets generated
from smaller sensor resolutions. It only happens to small-ish modes
though [1]
> 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. =.=
Be aware that the IPU pipeline handler tries to use a sensor
resolution as close as possible to the maximum requested size
src/libcamera/pipeline/ipu3/ipu3.cpp:
/*
* Generate raw configuration from CIO2.
*
* The output YUV streams will be limited in size to the maximum frame
* size requested for the RAW stream, if present.
*
* If no raw stream is requested generate a size as large as the maximum
* requested YUV size aligned to the ImgU constraints and bound by the
* sensor's maximum resolution. See
* https://bugs.libcamera.org/show_bug.cgi?id=32
*/
if (rawSize.isNull())
rawSize = maxYuvSize.expandedTo({ ImgUDevice::kIFMaxCropWidth,
ImgUDevice::kIFMaxCropHeight })
.grownBy({ ImgUDevice::kOutputMarginWidth,
ImgUDevice::kOutputMarginHeight })
.boundedTo(data_->cio2_.sensor()->resolution());
And looking at the ov13858 driver supported modes
static const struct ov13858_mode supported_modes[] = {
{
.width = 4224,
.height = 3136,
.vts_def = OV13858_VTS_30FPS,
.vts_min = OV13858_VTS_30FPS,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_4224x3136_regs),
.regs = mode_4224x3136_regs,
},
.link_freq_index = OV13858_LINK_FREQ_INDEX_0,
},
{
.width = 2112,
.height = 1568,
.vts_def = OV13858_VTS_30FPS,
.vts_min = 1608,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_2112x1568_regs),
.regs = mode_2112x1568_regs,
},
.link_freq_index = OV13858_LINK_FREQ_INDEX_1,
},
{
.width = 2112,
.height = 1188,
.vts_def = OV13858_VTS_30FPS,
.vts_min = 1608,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_2112x1188_regs),
.regs = mode_2112x1188_regs,
},
.link_freq_index = OV13858_LINK_FREQ_INDEX_1,
},
{
.width = 1056,
.height = 784,
.vts_def = OV13858_VTS_30FPS,
.vts_min = 804,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mode_1056x784_regs),
.regs = mode_1056x784_regs,
},
.link_freq_index = OV13858_LINK_FREQ_INDEX_1,
}
};
We might get to use full size only for full-resolution mode.
(not verified, only by looking at the code)
Clarifying what sensor mode gets selected for what output mode, and
what is the reported frame durations for each sensor mode, might help
clarify what actually happens.
Looking at numbers from "Question on camera stream durations" [2] ov13858
can do 60fps at 1080p (which is likely produced by 2112x1568 sensor mode),
and can do 29 at full-res. Hence I'm missing why 1080p doesn't get registered
as 30-FPS-capable in you case.
For the Soraka ov5670 front camera that instead can do 33336000 for
all modes > 720p so far we got away with the 0.05 rounding, but as you
said we miss adjusting the AE_AVAILABLE_TARGET_FPS_RANGE.
> The only way I can think of is to change the FrameDurationLimit
> calculation in close sourced IPA.
As a general consideration, this is an android specific requirement
and I see it better placed in the HAL. However the closed source IPA
serves only for Android so far...
What I would do is:
- Move the 0.05 adjustment to when minFrameDuration is calculated, so
that we keep AE_AVAILABLE_TARGET_FPS_RANGE and
ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT in sync.
The adjustment can be expressed as a percentage, and since CTS
accepts a 1.5% tolerance we could do the same (which means adjusting
to 30FPS streams that can only do 29.55 which seems -a lot- of
adjustment to me).
- Clarify how 1080p gets generated on Soraka ov13858, from which sensor mode
and what durations are reported. From the calculations in [2] it
should be capable of 60 FPS (capped at 30), but you see CCA falling
back to 480p.
Apart from this, if sensor full resolution can do 29.94 I think a
tiny 0.2% adjustment is fine as we can register full resolution as
30-FPS capable if CTS doesn't complain. Be aware though that full
resolution modes for both the front and back camera are reported as
being 24FPS in the Intel camera HAL as per the discussion in [2]
(this is said to be because of the ImgU processing bandwidth
limitations iirc) so if we want to get in-par we should really only
care about 1080p being @30FPS.
Thanks
j
>
> >
> > >
> > > 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.
> > >
[1]
cat ./media-libs/cros-camera-hal-configs-poppy/files/gcss/graph_settings_ov13858.xml | grep -A3 \<input | grep -v 4224 | grep \<input -A3
<input width="2112" height="1568" format="CIO2" />
<vf width="960" height="1280" format="NV12" peer="vf"/>
<main width="960" height="1280" format="NV12" peer="main"/>
--
<input width="2112" height="1568" format="CIO2" />
<vf width="720" height="1280" format="NV12" peer="vf"/>
<main width="960" height="1280" format="NV12" peer="main"/>
--
<input width="2112" height="1568" format="CIO2" />
<vf width="480" height="640" format="NV12" peer="vf"/>
<main width="960" height="1280" format="NV12" peer="main"/>
--
<input width="2112" height="1568" format="CIO2" />
<vf width="480" height="640" format="NV12" peer="vf"/>
<main width="720" height="1280" format="NV12" peer="main"/>
--
<input width="1056" height="784" format="CIO2" />
<vf width="480" height="640" format="NV12" peer="vf"/>
<main width="480" height="640" format="NV12" peer="main"/>
--
<input width="2112" height="1568" format="CIO2" />
<vf width="240" height="320" format="NV12" peer="vf"/>
<main width="960" height="1280" format="NV12" peer="main"/>
--
<input width="2112" height="1568" format="CIO2" />
<vf width="240" height="320" format="NV12" peer="vf"/>
<main width="720" height="1280" format="NV12" peer="main"/>
--
<input width="1056" height="784" format="CIO2" />
<vf width="240" height="320" format="NV12" peer="vf"/>
<main width="480" height="640" format="NV12" peer="main"/>
--
<input width="1056" height="784" format="CIO2" />
<vf width="240" height="320" format="NV12" peer="vf"/>
<main width="240" height="320" format="NV12" peer="main"/>
[2]
- ov5670
(320x240)[16149000]@61
(640x480)[16149000]@61
(1280x720)[33336000]@30
(1280x960)[33336000]@30
(1920x1080)[33336000]@30
(2560x1920)[33336000]@30
- ov13858
(320x240)[8352000]@119
(640x480)[8352000]@119
(1280x720)[16705000]@59
(1920x1080)[16705000]@59
(4160x3104)[33389000]@29
- imx258
(320x240)[17644000]@56
(640x480)[17644000]@56
(1280x720)[33282000]@30
(1920x1080)[34100000]@29
(4160x3104)[34100000]@29
Which can be summarized as:
- ov5670 can do 30FPS at max res and 1080p
- ov13858 can do 29.94 FPS at max res and 60 in 1080p
- imx258 can do 29.3 FPS at max res and 1080p
> >
> > 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