[libcamera-devel] [PATCH v2] android: Check exposure time range for manual sensor capability
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Nov 18 12:56:47 CET 2021
On Thu, Nov 18, 2021 at 12:10:58PM +0100, Jean-Michel Hautbois wrote:
> Hi Paul,
>
> Thanks for the patch !
>
> On 18/11/2021 10:48, Paul Elder wrote:
> > In the manual sensor capability validator, add a check for the exposure
> > time range. While at it, since the minimum exposure time is not limited
> > to the manual sensor capability, add a check for it when initializing
> > static metadata.
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > ---
> > 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
> > - this only prints error, should we return -EINVAL instead?
> > ---
> > src/android/camera_capabilities.cpp | 30 +++++++++++++++++++++++++++++
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index f357902e..6e46f163 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,26 @@ 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.i32[0] >= 100000) {
> > + LOG(HAL, Info)
> > + << noMode
> > + << "exposure time range minimum must not be equal nor larger than 100us";
Maybe "must be smaller than 100us" ?
> > + return false;
> > + }
> > +
> > + if (entry.data.i32[1] <= 100000000) {
> > + LOG(HAL, Info)
> > + << noMode
> > + << "exposure time range maximum must not be equal nor smaller than 100ms";
Same here, "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
> > @@ -1074,6 +1096,14 @@ int CameraCapabilities::initializeStaticMetadata()
> > exposureInfo->second.min().get<int32_t>() * 1000LL,
> > exposureInfo->second.max().get<int32_t>() * 1000LL,
> > };
> > +
> > + if (exposureTimeRange[0] >= 100000) {
> > + LOG(HAL, Error)
> > + << "Minimum exposure time "
> > + << exposureTimeRange[0]
> > + << "ns is too big (should be smaller than 100us)";
Do we need to handle the error somehow ?
> > + }
>
> I read the doc (better now than never :-)) in
> https://developer.android.com/reference/android/hardware/camera2/CameraCharacteristics#SENSOR_INFO_EXPOSURE_TIME_RANGE
>
> Interestingly, the ExposureTimeRange is in nanoseconds. Our
> Controls::ExposureTime metadata is in microseconds. Should it be changed ?
I think we should standardize all times in nanoseconds in libcamera at
some point, unless there's a good technical reason not to do so.
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>
> > +
> > staticMetadata_->addEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,
> > exposureTimeRange, 2);
> > }
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list