[libcamera-devel,DNI] : Fixes for CTS on nautilus(LIMITED)
diff mbox series

Message ID 20210604102606.218464-1-umang.jain@ideasonboard.com
State Not Applicable
Headers show
Series
  • [libcamera-devel,DNI] : Fixes for CTS on nautilus(LIMITED)
Related show

Commit Message

Umang Jain June 4, 2021, 10:26 a.m. UTC
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---

This patch is for reference and discussion purposes only.
Please DO NOT MERGE. If anything, there will be follow up
separate patches for merge separately.

Problem:
On nautilus, 26 tests were failing because of "Fail to open camera",
but more specifically from adb logcat:

```
E Camera2-Parameters: generated preview size list is empty!!
E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)
E CameraService: connectHelper: Could not initialize client from HAL.
I Camera2Client: Camera 0: Closed
```

was found to be root of the problem. The checks triggered are here:
> Parameters::getFilteredSizes()
  https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976

nautilus reports higher frame-duration to start with: 34100000
(in CameraDevice::getStaticMetadata()).
minFrameDurationNsec is meant to be re-adjusted down the line 
before round-up, *if* the difference is < 500 useconds.

On nautilus, since the difference was much larger than 500 useconds,
the re-adjustment failed and libcamera reported a much higher
frame-duration to upper-layers/libcameraservice. 
This led to the problem in Parameters::getFilteredSizes(),
where all potential streams for preview are skipped to be added,
due to high minFrameDuration.

To force this re-adjustment, the scope of difference was increased
to 1200 useconds as done in the patch.

CTS Results on nautilus after applying the changes:
Total Run time: 8m 42s
1/1 modules completed
Total Tests       : 221
PASSED            : 221
FAILED            : 0

---
 src/android/camera_device.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Paul Elder June 7, 2021, 6:24 a.m. UTC | #1
Hi Umang,

On Fri, Jun 04, 2021 at 03:56:06PM +0530, Umang Jain wrote:
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> 
> This patch is for reference and discussion purposes only.
> Please DO NOT MERGE. If anything, there will be follow up
> separate patches for merge separately.

Thank you for the investigation and starting the discussion.

> 
> Problem:
> On nautilus, 26 tests were failing because of "Fail to open camera",
> but more specifically from adb logcat:
> 
> ```
> E Camera2-Parameters: generated preview size list is empty!!
> E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)
> E CameraService: connectHelper: Could not initialize client from HAL.
> I Camera2Client: Camera 0: Closed
> ```
> 
> was found to be root of the problem. The checks triggered are here:
> > Parameters::getFilteredSizes()
>   https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976
> 
> nautilus reports higher frame-duration to start with: 34100000

iirc this was the minimum frame duration. This converts to 29.32 fps.

> (in CameraDevice::getStaticMetadata()).
> minFrameDurationNsec is meant to be re-adjusted down the line 
> before round-up, *if* the difference is < 500 useconds.

And the minimum frame duration gets compared to what android requires as
the maximum value of the minimum frame duration, which is 33366700ns =
29.97 fps.

> 
> On nautilus, since the difference was much larger than 500 useconds,

What this adjustment does is if the minimum frame duration (= max fps)
supported by the sensor is greater than the maximum allowed value (= the
supported max fps is lower than the max requirement), but it's within
500us, we can adjust it.

> the re-adjustment failed and libcamera reported a much higher
> frame-duration to upper-layers/libcameraservice. 

(Yes, it was rounded down instead to 29 fps and the corresponding
34482758ns, which is less/greater than 29.97fps / 33366700ns.)

> This led to the problem in Parameters::getFilteredSizes(),
> where all potential streams for preview are skipped to be added,
> due to high minFrameDuration.
> 
> To force this re-adjustment, the scope of difference was increased
> to 1200 useconds as done in the patch.

So by allowing 1200us, what we're saying is that we can still reasonably
say that we support a minimum frame duration of 33366700ns when the real
minimum that we support is 34566700ns. It's a whole 1.2ms off per frame.
Is this a reasonable assertion?

> 
> CTS Results on nautilus after applying the changes:
> Total Run time: 8m 42s
> 1/1 modules completed
> Total Tests       : 221
> PASSED            : 221
> FAILED            : 0
> 
> ---
>  src/android/camera_device.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index fe332ec3..d0676a7f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -877,13 +877,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		 * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service
>  		 * implementation).
>  		 *
> -		 * If we're close enough (+ 500 useconds) to that value, round
> +		 * If we're close enough (+ 1200 useconds) to that value, round
>  		 * the minimum frame duration of the camera to an accepted
>  		 * value.
>  		 */
>  		static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;
>  		if (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&
> -		    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)
> +		    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)
>  			minFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;
>  
>  		/*
> @@ -1335,6 +1335,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
>  		ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
>  		ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,
> +		ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,

What's this for?


Thanks,

Paul

>  		ANDROID_SCALER_CROPPING_TYPE,
>  		ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,
>  		ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,
> -- 
> 2.31.0
>
Umang Jain June 8, 2021, 5:38 a.m. UTC | #2
Hi Paul,

Thanks for pitching in.

On 6/7/21 11:54 AM, paul.elder@ideasonboard.com wrote:
> Hi Umang,
>
> On Fri, Jun 04, 2021 at 03:56:06PM +0530, Umang Jain wrote:
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>
>> This patch is for reference and discussion purposes only.
>> Please DO NOT MERGE. If anything, there will be follow up
>> separate patches for merge separately.
> Thank you for the investigation and starting the discussion.
>
>> Problem:
>> On nautilus, 26 tests were failing because of "Fail to open camera",
>> but more specifically from adb logcat:
>>
>> ```
>> E Camera2-Parameters: generated preview size list is empty!!
>> E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)
>> E CameraService: connectHelper: Could not initialize client from HAL.
>> I Camera2Client: Camera 0: Closed
>> ```
>>
>> was found to be root of the problem. The checks triggered are here:
>>> Parameters::getFilteredSizes()
>>    https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976
>>
>> nautilus reports higher frame-duration to start with: 34100000
> iirc this was the minimum frame duration. This converts to 29.32 fps.
>
>> (in CameraDevice::getStaticMetadata()).
>> minFrameDurationNsec is meant to be re-adjusted down the line
>> before round-up, *if* the difference is < 500 useconds.
> And the minimum frame duration gets compared to what android requires as
> the maximum value of the minimum frame duration, which is 33366700ns =
> 29.97 fps.
>
>> On nautilus, since the difference was much larger than 500 useconds,
> What this adjustment does is if the minimum frame duration (= max fps)
> supported by the sensor is greater than the maximum allowed value (= the
> supported max fps is lower than the max requirement), but it's within
> 500us, we can adjust it.
>
>> the re-adjustment failed and libcamera reported a much higher
>> frame-duration to upper-layers/libcameraservice.
> (Yes, it was rounded down instead to 29 fps and the corresponding
> 34482758ns, which is less/greater than 29.97fps / 33366700ns.)
>
>> This led to the problem in Parameters::getFilteredSizes(),
>> where all potential streams for preview are skipped to be added,
>> due to high minFrameDuration.
>>
>> To force this re-adjustment, the scope of difference was increased
>> to 1200 useconds as done in the patch.
> So by allowing 1200us, what we're saying is that we can still reasonably
> say that we support a minimum frame duration of 33366700ns when the real
> minimum that we support is 34566700ns. It's a whole 1.2ms off per frame.
> Is this a reasonable assertion?
Indeed, that's what is in question. I plan to discuss this in today's 
weekly.
>
>> CTS Results on nautilus after applying the changes:
>> Total Run time: 8m 42s
>> 1/1 modules completed
>> Total Tests       : 221
>> PASSED            : 221
>> FAILED            : 0
>>
>> ---
>>   src/android/camera_device.cpp | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index fe332ec3..d0676a7f 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -877,13 +877,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>>   		 * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service
>>   		 * implementation).
>>   		 *
>> -		 * If we're close enough (+ 500 useconds) to that value, round
>> +		 * If we're close enough (+ 1200 useconds) to that value, round
>>   		 * the minimum frame duration of the camera to an accepted
>>   		 * value.
>>   		 */
>>   		static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;
>>   		if (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&
>> -		    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)
>> +		    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)
>>   			minFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;
>>   
>>   		/*
>> @@ -1335,6 +1335,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>>   		ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
>>   		ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
>>   		ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,
>> +		ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,
> What's this for?


The 
https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976
check , reads in ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT 
key, and I realized we don't report it hence, I thought it's a similar case
as 51c09236c4e8 
<https://git.linuxtv.org/libcamera.git/commit/?id=51c09236c4e8bec2b0a272ebecaa33f32432208a> 
("android: Make FRAME_DURATION result key available in static metadata")


However this morning, I ran CTS without
 > +ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT
and it ran just fine. Maybe I don't understand keys-related magic not 
well-enough.


>
>
> Thanks,
>
> Paul
>
>>   		ANDROID_SCALER_CROPPING_TYPE,
>>   		ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,
>>   		ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,
>> -- 
>> 2.31.0
>>
Paul Elder June 8, 2021, 6:24 a.m. UTC | #3
Hi Umang,

On Tue, Jun 08, 2021 at 11:08:00AM +0530, Umang Jain wrote:
> Hi Paul,
> 
> Thanks for pitching in.
> 
> On 6/7/21 11:54 AM, paul.elder@ideasonboard.com wrote:
> 
>     Hi Umang,
> 
>     On Fri, Jun 04, 2021 at 03:56:06PM +0530, Umang Jain wrote:
> 
>         Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>         ---
> 
>         This patch is for reference and discussion purposes only.
>         Please DO NOT MERGE. If anything, there will be follow up
>         separate patches for merge separately.
> 
>     Thank you for the investigation and starting the discussion.
> 
> 
>         Problem:
>         On nautilus, 26 tests were failing because of "Fail to open camera",
>         but more specifically from adb logcat:
> 
>         ```
>         E Camera2-Parameters: generated preview size list is empty!!
>         E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)
>         E CameraService: connectHelper: Could not initialize client from HAL.
>         I Camera2Client: Camera 0: Closed
>         ```
> 
>         was found to be root of the problem. The checks triggered are here:
> 
>             Parameters::getFilteredSizes()
> 
>           https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976
> 
>         nautilus reports higher frame-duration to start with: 34100000
> 
>     iirc this was the minimum frame duration. This converts to 29.32 fps.
> 
> 
>         (in CameraDevice::getStaticMetadata()).
>         minFrameDurationNsec is meant to be re-adjusted down the line
>         before round-up, *if* the difference is < 500 useconds.
> 
>     And the minimum frame duration gets compared to what android requires as
>     the maximum value of the minimum frame duration, which is 33366700ns =
>     29.97 fps.
> 
> 
>         On nautilus, since the difference was much larger than 500 useconds,
> 
>     What this adjustment does is if the minimum frame duration (= max fps)
>     supported by the sensor is greater than the maximum allowed value (= the
>     supported max fps is lower than the max requirement), but it's within
>     500us, we can adjust it.
> 
> 
>         the re-adjustment failed and libcamera reported a much higher
>         frame-duration to upper-layers/libcameraservice.
> 
>     (Yes, it was rounded down instead to 29 fps and the corresponding
>     34482758ns, which is less/greater than 29.97fps / 33366700ns.)
> 
> 
>         This led to the problem in Parameters::getFilteredSizes(),
>         where all potential streams for preview are skipped to be added,
>         due to high minFrameDuration.
> 
>         To force this re-adjustment, the scope of difference was increased
>         to 1200 useconds as done in the patch.
> 
>     So by allowing 1200us, what we're saying is that we can still reasonably
>     say that we support a minimum frame duration of 33366700ns when the real
>     minimum that we support is 34566700ns. It's a whole 1.2ms off per frame.
>     Is this a reasonable assertion?
> 
> Indeed, that's what is in question. I plan to discuss this in today's weekly.
> 
> 
> 
>         CTS Results on nautilus after applying the changes:
>         Total Run time: 8m 42s
>         1/1 modules completed
>         Total Tests       : 221
>         PASSED            : 221
>         FAILED            : 0
> 
>         ---
>          src/android/camera_device.cpp | 5 +++--
>          1 file changed, 3 insertions(+), 2 deletions(-)
> 
>         diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>         index fe332ec3..d0676a7f 100644
>         --- a/src/android/camera_device.cpp
>         +++ b/src/android/camera_device.cpp
>         @@ -877,13 +877,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>                          * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service
>                          * implementation).
>                          *
>         -                * If we're close enough (+ 500 useconds) to that value, round
>         +                * If we're close enough (+ 1200 useconds) to that value, round
>                          * the minimum frame duration of the camera to an accepted
>                          * value.
>                          */
>                         static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;
>                         if (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&
>         -                   minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)
>         +                   minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)
>                                 minFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;
> 
>                         /*
>         @@ -1335,6 +1335,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>                         ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
>                         ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
>                         ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,
>         +               ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,
> 
>     What's this for?
> 
> 
> The https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master
> /services/camera/libcameraservice/api1/client2/Parameters.cpp#2976
> check , reads in ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT key, and
> I realized we don't report it hence, I thought it's a similar case
> as 51c09236c4e8 ("android: Make FRAME_DURATION result key available in static
> metadata")

I'm pretty sure that section corresponds to [1], so it should indeed be
fine without declaring the key.

[1] https://git.linuxtv.org/libcamera.git/tree/src/android/camera_device.cpp#n1215


Paul

> 
> 
> However this morning, I ran CTS without
> > +ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT
> and it ran just fine. Maybe I don't understand keys-related magic not
> well-enough.
> 
> 
> 
>                         ANDROID_SCALER_CROPPING_TYPE,
>                         ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,
>                         ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,
>         --
>         2.31.0
> 
>
Jacopo Mondi June 8, 2021, 7:38 a.m. UTC | #4
Hello Umang, Paul

On Tue, Jun 08, 2021 at 03:24:58PM +0900, paul.elder@ideasonboard.com wrote:
> Hi Umang,
>
> On Tue, Jun 08, 2021 at 11:08:00AM +0530, Umang Jain wrote:
> > Hi Paul,
> >
> > Thanks for pitching in.
> >
> > On 6/7/21 11:54 AM, paul.elder@ideasonboard.com wrote:
> >
> >     Hi Umang,
> >
> >     On Fri, Jun 04, 2021 at 03:56:06PM +0530, Umang Jain wrote:
> >
> >         Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> >         ---
> >
> >         This patch is for reference and discussion purposes only.
> >         Please DO NOT MERGE. If anything, there will be follow up
> >         separate patches for merge separately.
> >
> >     Thank you for the investigation and starting the discussion.
> >
> >
> >         Problem:
> >         On nautilus, 26 tests were failing because of "Fail to open camera",
> >         but more specifically from adb logcat:
> >
> >         ```
> >         E Camera2-Parameters: generated preview size list is empty!!
> >         E Camera2Client: initializeImpl: Camera 0: unable to build defaults: Invalid argument (-22)
> >         E CameraService: connectHelper: Could not initialize client from HAL.
> >         I Camera2Client: Camera 0: Closed
> >         ```
> >
> >         was found to be root of the problem. The checks triggered are here:
> >
> >             Parameters::getFilteredSizes()
> >
> >           https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master/services/camera/libcameraservice/api1/client2/Parameters.cpp#2976
> >
> >         nautilus reports higher frame-duration to start with: 34100000
> >
> >     iirc this was the minimum frame duration. This converts to 29.32 fps.
> >
> >
> >         (in CameraDevice::getStaticMetadata()).
> >         minFrameDurationNsec is meant to be re-adjusted down the line
> >         before round-up, *if* the difference is < 500 useconds.
> >
> >     And the minimum frame duration gets compared to what android requires as
> >     the maximum value of the minimum frame duration, which is 33366700ns =
> >     29.97 fps.
> >
> >
> >         On nautilus, since the difference was much larger than 500 useconds,
> >
> >     What this adjustment does is if the minimum frame duration (= max fps)
> >     supported by the sensor is greater than the maximum allowed value (= the
> >     supported max fps is lower than the max requirement), but it's within
> >     500us, we can adjust it.
> >
> >
> >         the re-adjustment failed and libcamera reported a much higher
> >         frame-duration to upper-layers/libcameraservice.
> >
> >     (Yes, it was rounded down instead to 29 fps and the corresponding
> >     34482758ns, which is less/greater than 29.97fps / 33366700ns.)
> >
> >
> >         This led to the problem in Parameters::getFilteredSizes(),
> >         where all potential streams for preview are skipped to be added,
> >         due to high minFrameDuration.
> >
> >         To force this re-adjustment, the scope of difference was increased
> >         to 1200 useconds as done in the patch.
> >
> >     So by allowing 1200us, what we're saying is that we can still reasonably
> >     say that we support a minimum frame duration of 33366700ns when the real
> >     minimum that we support is 34566700ns. It's a whole 1.2ms off per frame.
> >     Is this a reasonable assertion?
> >
> > Indeed, that's what is in question. I plan to discuss this in today's weekly.
> >
> >
> >
> >         CTS Results on nautilus after applying the changes:
> >         Total Run time: 8m 42s
> >         1/1 modules completed
> >         Total Tests       : 221
> >         PASSED            : 221
> >         FAILED            : 0
> >
> >         ---
> >          src/android/camera_device.cpp | 5 +++--
> >          1 file changed, 3 insertions(+), 2 deletions(-)
> >
> >         diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> >         index fe332ec3..d0676a7f 100644
> >         --- a/src/android/camera_device.cpp
> >         +++ b/src/android/camera_device.cpp
> >         @@ -877,13 +877,13 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >                          * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service
> >                          * implementation).
> >                          *
> >         -                * If we're close enough (+ 500 useconds) to that value, round
> >         +                * If we're close enough (+ 1200 useconds) to that value, round
> >                          * the minimum frame duration of the camera to an accepted
> >                          * value.
> >                          */
> >                         static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;
> >                         if (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&
> >         -                   minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)
> >         +                   minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)
> >                                 minFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;
> >
> >                         /*
> >         @@ -1335,6 +1335,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >                         ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
> >                         ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
> >                         ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,
> >         +               ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,
> >
> >     What's this for?
> >
> >
> > The https://android.googlesource.com/platform/frameworks/av/+/refs/heads/master
> > /services/camera/libcameraservice/api1/client2/Parameters.cpp#2976
> > check , reads in ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT key, and
> > I realized we don't report it hence, I thought it's a similar case
> > as 51c09236c4e8 ("android: Make FRAME_DURATION result key available in static
> > metadata")
>
> I'm pretty sure that section corresponds to [1], so it should indeed be
> fine without declaring the key.
>
> [1] https://git.linuxtv.org/libcamera.git/tree/src/android/camera_device.cpp#n1215

More than that, ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT
(and the ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_INPUT
couterpart) are flags that are expected to be used to populate the 4th
field of each stream confiuration reported as available through
ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, which aims to report
the stream direction (output == produced by the camera, input ==
provided by application for reprocessing).

It should indeed not be added to the list of available characteristics
keys.

Thanks
   j

>
>
> Paul
>
> >
> >
> > However this morning, I ran CTS without
> > > +ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT
> > and it ran just fine. Maybe I don't understand keys-related magic not
> > well-enough.
> >
> >
> >
> >                         ANDROID_SCALER_CROPPING_TYPE,
> >                         ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,
> >                         ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,
> >         --
> >         2.31.0
> >
> >

Patch
diff mbox series

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index fe332ec3..d0676a7f 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -877,13 +877,13 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 		 * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service
 		 * implementation).
 		 *
-		 * If we're close enough (+ 500 useconds) to that value, round
+		 * If we're close enough (+ 1200 useconds) to that value, round
 		 * the minimum frame duration of the camera to an accepted
 		 * value.
 		 */
 		static constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;
 		if (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&
-		    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)
+		    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 1200000)
 			minFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;
 
 		/*
@@ -1335,6 +1335,7 @@  const camera_metadata_t *CameraDevice::getStaticMetadata()
 		ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
 		ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
 		ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,
+		ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT,
 		ANDROID_SCALER_CROPPING_TYPE,
 		ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,
 		ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,