[libcamera-devel] [PATCH 06/16] android: capabilties: Assume controls::FrameDurationLimits is supported

[Jacopo Mondi]

Hi Paul,

On Tue, Aug 31, 2021 at 11:21:02AM +0900, Paul Elder wrote:
> Hi Jacopo,
>
> On Fri, Aug 27, 2021 at 02:07:47PM +0200, Jacopo Mondi wrote:
> > As we now collect the per-stream frame durations at
> > initializeStreamConfigurations() times, the Camera is now guaranteed to
> > support the controls::FrameDurationLimits control.
> >
> > Remove the check for its presence when populating the
> > ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES static metadata.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/camera_capabilities.cpp | 83 ++++++++++++++---------------
> >  1 file changed, 41 insertions(+), 42 deletions(-)
> >
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index 723a4fd5a880..c45d74189aaa 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -858,55 +858,54 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> >  				  aeAvailableModes);
> >
> > +	/* Initialize the AE frame duration limits. */
> >  	int64_t minFrameDurationNsec = -1;
> >  	int64_t maxFrameDurationNsec = -1;
> >  	const auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurationLimits);
> > -	if (frameDurationsInfo != controlsInfo.end()) {
> > -		minFrameDurationNsec = frameDurationsInfo->second.min().get<int64_t>() * 1000;
> > -		maxFrameDurationNsec = frameDurationsInfo->second.max().get<int64_t>() * 1000;
> > +	minFrameDurationNsec = frameDurationsInfo->second.min().get<int64_t>() * 1000;
> > +	maxFrameDurationNsec = frameDurationsInfo->second.max().get<int64_t>() * 1000;
>
> Why not just initialize these directly?

You're right, I'll do so!

Thanks
   j

>
>
> Otherwise, looks good.
>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>
> >
> > -		/*
> > -		 * Adjust the minimum frame duration to comply with Android
> > -		 * requirements. The camera service mandates all preview/record
> > -		 * streams to have a minimum frame duration < 33,366 milliseconds
> > -		 * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service
> > -		 * implementation).
> > -		 *
> > -		 * If we're close enough (+ 500 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 - 1000;
> > +	/*
> > +	 * Adjust the minimum frame duration to comply with Android
> > +	 * requirements. The camera service mandates all preview/record
> > +	 * streams to have a minimum frame duration < 33,366 milliseconds
> > +	 * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service
> > +	 * implementation).
> > +	 *
> > +	 * If we're close enough (+ 500 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 - 1000;
> >
> > -		/*
> > -		 * The AE routine frame rate limits are computed using the frame
> > -		 * duration limits, as libcamera clips the AE routine to the
> > -		 * frame durations.
> > -		 */
> > -		int32_t maxFps = std::round(1e9 / minFrameDurationNsec);
> > -		int32_t minFps = std::round(1e9 / maxFrameDurationNsec);
> > -		minFps = std::max(1, minFps);
> > +	/*
> > +	 * The AE routine frame rate limits are computed using the frame
> > +	 * duration limits, as libcamera clips the AE routine to the
> > +	 * frame durations.
> > +	 */
> > +	int32_t maxFps = std::round(1e9 / minFrameDurationNsec);
> > +	int32_t minFps = std::round(1e9 / maxFrameDurationNsec);
> > +	minFps = std::max(1, minFps);
> >
> > -		/*
> > -		 * Force rounding errors so that we have the proper frame
> > -		 * durations for when we reuse these variables later
> > -		 */
> > -		minFrameDurationNsec = 1e9 / maxFps;
> > -		maxFrameDurationNsec = 1e9 / minFps;
> > +	/*
> > +	 * Force rounding errors so that we have the proper frame
> > +	 * durations for when we reuse these variables later
> > +	 */
> > +	minFrameDurationNsec = 1e9 / maxFps;
> > +	maxFrameDurationNsec = 1e9 / minFps;
> >
> > -		/*
> > -		 * Register to the camera service {min, max} and {max, max}
> > -		 * intervals as requested by the metadata documentation.
> > -		 */
> > -		int32_t availableAeFpsTarget[] = {
> > -			minFps, maxFps, maxFps, maxFps
> > -		};
> > -		staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,
> > -					  availableAeFpsTarget);
> > -	}
> > +	/*
> > +	 * Register to the camera service {min, max} and {max, max}
> > +	 * intervals as requested by the metadata documentation.
> > +	 */
> > +	int32_t availableAeFpsTarget[] = {
> > +		minFps, maxFps, maxFps, maxFps
> > +	};
> > +	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,
> > +				  availableAeFpsTarget);
> >
> >  	std::vector<int32_t> aeCompensationRange = {
> >  		0, 0,
> > --
> > 2.32.0
> >