[libcamera-devel] [PATCH] [DNI]: Fixes for CTS on nautilus(LIMITED)
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Tue Jun 8 08:24:58 CEST 2021
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 at 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 at 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
>
>
More information about the libcamera-devel
mailing list