[libcamera-devel] [PATCH v2 09/17] android: capabilties: Correctly populate STALL durations

[Jacopo Mondi]

Hi Laurent,

On Wed, Oct 06, 2021 at 04:41:25AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Tue, Sep 07, 2021 at 09:40:59PM +0200, Jacopo Mondi wrote:
> > We currently hardcode 2560x1920 at 30FPS as the only stalling frame duration.
> >
> > This is of course not correct, and all the required information to
> > properly populate the ANDROID_SCALER_AVAILABLE_STALL_DURATIONS static
> > metadata are available from initializeStaticMetadata().
> >
> > Use the collected stalling durations and sizes to properly popoulate the
> > static property.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> > Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> > ---
> >  src/android/camera_capabilities.cpp | 19 +++++++++++++------
> >  1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index 5d8293843f57..526127176678 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -1265,12 +1265,6 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS,
> >  				  availableStreamConfigurations);
> >
> > -	std::vector<int64_t> availableStallDurations = {
> > -		ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333,
> > -	};
> > -	staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
> > -				  availableStallDurations);
> > -
> >  	std::vector<int64_t> minFrameDurations;
> >  	minFrameDurations.reserve(streamConfigurations_.size() * 4);
> >  	for (const auto &entry : streamConfigurations_) {
> > @@ -1282,6 +1276,19 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,
> >  				  minFrameDurations);
> >
> > +	std::vector<int64_t> availableStallDurations;
> > +	for (const auto &entry : streamConfigurations_) {
> > +		if (entry.androidFormat != HAL_PIXEL_FORMAT_BLOB)
> > +			continue;
> > +
> > +		availableStallDurations.push_back(entry.androidFormat);
> > +		availableStallDurations.push_back(entry.resolution.width);
> > +		availableStallDurations.push_back(entry.resolution.height);
> > +		availableStallDurations.push_back(entry.minFrameDurationNsec);
>
> Should we add a break here, as there should be a single JPEG stream
> configuration ? Or have I misunderstood this ?

I don't think so... The ANDROID_SCALER_AVAILABLE_STALL_DURATIONS
characteristic is documented to have size (int64 x 4 x n) where 'n' I
assume it's the number of stream configuration in stalling formats

The following formats may always have a stall duration (ImageFormat#JPEG
and ImageFormat#RAW_SENSOR) but we only support JPEG at the moment.

>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Thanks
   j

>
> > +	}
> > +	staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_STALL_DURATIONS,
> > +				  availableStallDurations);
> > +
> >  	uint8_t croppingType = ANDROID_SCALER_CROPPING_TYPE_CENTER_ONLY;
> >  	staticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, croppingType);
> >
>
> --
> Regards,
>
> Laurent Pinchart