[libcamera-devel] [PATCH v3] android: camera_capabilities: Adjust minimum frame duration to match FPS

[Umang Jain]

Hi Han-Lin,

On 4/28/22 00:40, Umang Jain via libcamera-devel wrote:
> Hi
>
> On 4/27/22 20:43, Han-Lin Chen via libcamera-devel wrote:
>> CTS calculates FPS with a rounding formula: See
>> Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()
>>
>> fps = floor(1e9 / minFrameDuration + 0.05f)
>
>
> It seems(to me at-least) that the placement of the + 0.05f adjustment 
> is changed as per this patch.
>
>>
>> The android adapter reports it as the AE target FPS. The patch 
>> adjusts the
>> reported minimum frame duration to match the reported FPS.
>
>
> Hence we are not introducing any 'new adjustments' rather changing the 
> placement of the adjustment.


Being clarified by Jacopo on IRC, that the patch makes the FPS and 
minFrameDuration in-sync, such that
the adjustment is now reflected for both. That makes perfect sense now 
that I am looking, hence,

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>
> Given my inference is correct (of course I might be missing the entire 
> point here, please do correct me
> then), I would re-pharse this commit message  to reflect that this 
> patch doesn't introduce any functional
> changes.
>
>     android: camera_capabilities: Move minimum frame duration adjustment
>
>     CTS calculates FPS with a rounding formula: See
> Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()
>
>     fps = floor(1e9 / minFrameDuration + 0.05f)
>
>     The android adapter reports it as the AE target FPS. The patch
>     moves this adjustment while populating the stream configuration's
>     minimum frame duration itself.
>
> What do you think ?
>
>>
>> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
>> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>> ---
>>   src/android/camera_capabilities.cpp | 16 +++++++++++-----
>>   1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/android/camera_capabilities.cpp 
>> b/src/android/camera_capabilities.cpp
>> index 55d651f3..ee17cdc7 100644
>> --- a/src/android/camera_capabilities.cpp
>> +++ b/src/android/camera_capabilities.cpp
>> @@ -687,6 +687,14 @@ int 
>> CameraCapabilities::initializeStreamConfigurations()
>>                       minFrameDuration = minFrameDurationCap;
>>               }
>>   +            /*
>> +             * Calculate FPS as CTS does and adjust the minimum
>> +             * frame duration accordingly: see
>> +             * 
>> Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()
>> +             */
>> +            minFrameDuration =
>> +                1e9 / static_cast<unsigned int>(floor(1e9 / 
>> minFrameDuration + 0.05f));
>> +
>>               streamConfigurations_.push_back({
>>                   res, androidFormat, minFrameDuration, 
>> maxFrameDuration,
>>               });
>> @@ -1287,12 +1295,10 @@ int 
>> CameraCapabilities::initializeStaticMetadata()
>>            * recording profile. Inspecting the Intel IPU3 HAL
>>            * implementation confirms this but no reference has been 
>> found
>>            * in the metadata documentation.
>> -         *
>> -         * Calculate FPS as CTS does: see
>> -         * 
>> Camera2SurfaceViewTestCase.java:getSuitableFpsRangeForDuration()
>>            */
>> -        unsigned int fps = static_cast<unsigned int>
>> -                   (floor(1e9 / entry.minFrameDurationNsec + 0.05f));
>> +        unsigned int fps =
>> +            static_cast<unsigned int>(floor(1e9 / 
>> entry.minFrameDurationNsec));
>> +
>>           if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB && fps < 30)
>>               continue;