[libcamera-devel] [PATCH 9/9] android: Elevate min duration to 30 FPS if it's lower within 2%

[Umang Jain]

Hello,

On 4/6/22 12:39, Jacopo Mondi 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 1% duration was picked as per nautilus' use-case (i.e. numerical 
values of minFrameDuration reported on nautilus).

I didn't need more than anything more than 1% hence, we agreed that it's 
a good tolerance % that can go in.

>
> 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
>
>>>> 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