[libcamera-devel] [PATCH v4 5/7] android: Check exposure time range for manual sensor capability
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Tue Dec 21 00:09:35 CET 2021
Hi Kieran,
On Thu, Nov 25, 2021 at 11:09:17AM +0000, Kieran Bingham wrote:
> Quoting paul.elder at ideasonboard.com (2021-11-23 10:50:32)
> > Hello,
> >
> > On Tue, Nov 23, 2021 at 07:40:40PM +0900, Paul Elder wrote:
> > > In the manual sensor capability validator, add a check for the presence
> > > of the exposure time range key, and for the maximum exposure time. The
> > > minimum exposure time is a requirement for adding the key in the first
> > > place; add a check for this as well.
> > >
> > > If either requirement is not met, the manual sensor capability
> > > validation will fail, therefore disabling the FULL hardware level. The
> > > exposure time range key is optional in non-FULL hardware levels.
> > >
> > > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
> > >
> > > ---
> > > Changes in v4:
> > > - s/i32/i64
> > >
> > > Changes in v3:
> > > - squash "android: capabilities: Add exposure time keys only if
> > > available"
> > > - fix the minumum exposure time check
> > > - only make the exposure time range key available if this check
> > > passes. additionally, if the max exposure time passes its check,
> > > tick the box for manual sensor and FULL
> > > - update commit message accordingly
> > >
> > > Changes in v2:
> > > - fix comparator order (cosmetic)
> > > - change comparators and comments to "equal or", as that is what is
> > > specificied in the hal docs
> > > - add check for minimum exposure time when initializing static metadata
> > > ---
> > > src/android/camera_capabilities.cpp | 33 +++++++++++++++++++++++++----
> > > 1 file changed, 29 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > > index 875d38da..f7397d27 100644
> > > --- a/src/android/camera_capabilities.cpp
> > > +++ b/src/android/camera_capabilities.cpp
> > > @@ -217,6 +217,8 @@ std::vector<U> setMetadata(CameraMetadata *metadata, uint32_t tag,
> > >
> > > bool CameraCapabilities::validateManualSensorCapability()
> > > {
> > > + camera_metadata_ro_entry_t entry;
> > > +
> > > const char *noMode = "Manual sensor capability unavailable: ";
> > >
> > > if (!staticMetadata_->entryContains<uint8_t>(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> > > @@ -231,6 +233,19 @@ bool CameraCapabilities::validateManualSensorCapability()
> > > return false;
> > > }
> > >
> > > + if (!staticMetadata_->hasEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE)) {
> > > + LOG(HAL, Info) << noMode << "missing exposure time range";
> > > + return false;
> > > + }
> > > +
> > > + staticMetadata_->getEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, &entry);
> > > + if (entry.data.i64[1] <= 100000000) {
> > > + LOG(HAL, Info)
> > > + << noMode
> > > + << "exposure time range maximum must be larger than 100ms";
> > > + return false;
> > > + }
> > > +
> > > /*
> > > * \todo Return true here after we satisfy all the requirements:
> > > * https://developer.android.com/reference/android/hardware/camera2/CameraMetadata#REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR
> > > @@ -798,7 +813,6 @@ int CameraCapabilities::initializeStaticMetadata()
> > > ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,
> > > ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,
> > > ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,
> > > - ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,
> > > ANDROID_SENSOR_INFO_MAX_FRAME_DURATION,
> > > ANDROID_SENSOR_INFO_PHYSICAL_SIZE,
> > > ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
> > > @@ -876,7 +890,6 @@ int CameraCapabilities::initializeStaticMetadata()
> > > ANDROID_NOISE_REDUCTION_MODE,
> > > ANDROID_REQUEST_PIPELINE_DEPTH,
> > > ANDROID_SCALER_CROP_REGION,
> > > - ANDROID_SENSOR_EXPOSURE_TIME,
> > > ANDROID_SENSOR_FRAME_DURATION,
> > > ANDROID_SENSOR_ROLLING_SHUTTER_SKEW,
> > > ANDROID_SENSOR_TEST_PATTERN_MODE,
> > > @@ -1082,8 +1095,20 @@ int CameraCapabilities::initializeStaticMetadata()
> > > exposureInfo->second.min().get<int32_t>() * 1000LL,
> > > exposureInfo->second.max().get<int32_t>() * 1000LL,
> > > };
> > > - staticMetadata_->addEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,
> > > - exposureTimeRange, 2);
> > > +
> > > + if (exposureTimeRange[0] < 100000) {
> > > + staticMetadata_->addEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,
> > > + exposureTimeRange, 2);
> > > +
> > > + availableCharacteristicsKeys_.insert(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE);
> > > + availableRequestKeys_.insert(ANDROID_SENSOR_EXPOSURE_TIME);
> > > + availableResultKeys_.insert(ANDROID_SENSOR_EXPOSURE_TIME);
> >
> > Welp, it turns out this adds CTS failures:
> >
> > junit.framework.AssertionFailedError: Missing field: SENSOR_EXPOSURE_TIME
> >
> > java.lang.Throwable(Test failed for camera 0: Key android.sensor.exposureTime shouldn't be null)
> >
> > Is it fine to have these failures, since they'll be fixed soon with the
> > AE series? Or should I replace the result key one with a todo?
>
> I'd be tempted to move this patch to the series that adds the
> requirements.
Alright, I'll move it immediately after "android: Plumb all AE-related
controls".
Paul
>
> Then we don't lose the patch, and don't add a regression when testing.
>
>
>
> >
> >
> > Paul
> >
> > > + } else {
> > > + LOG(HAL, Info)
> > > + << "Minimum exposure time "
> > > + << exposureTimeRange[0]
> > > + << "ns is too big (should be smaller than 100us)";
> > > + }
> > > }
> > >
> > > staticMetadata_->addEntry(ANDROID_SENSOR_ORIENTATION, orientation_);
> > > --
> > > 2.27.0
> > >
More information about the libcamera-devel
mailing list