| Message ID | 20210907194107.803730-10-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Tue, Sep 07, 2021 at 09:40:59PM +0200, Jacopo Mondi wrote: > We currently hardcode 2560x1920@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@jmondi.org> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > Reviewed-by: Umang Jain <umang.jain@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 ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + } > + staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_STALL_DURATIONS, > + availableStallDurations); > + > uint8_t croppingType = ANDROID_SCALER_CROPPING_TYPE_CENTER_ONLY; > staticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, croppingType); >
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@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@jmondi.org> > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > Reviewed-by: Umang Jain <umang.jain@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@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
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); + } + staticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_STALL_DURATIONS, + availableStallDurations); + uint8_t croppingType = ANDROID_SCALER_CROPPING_TYPE_CENTER_ONLY; staticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, croppingType);