[libcamera-devel] [PATCH v2 9/9] android: camera_device: Report sensor physical size

[Jacopo Mondi]

Hi Laurent,

On Wed, Dec 23, 2020 at 07:14:42AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, Dec 18, 2020 at 05:47:54PM +0100, Jacopo Mondi wrote:
> > Report the ANDROID_SENSOR_INFO_PHYSICAL_SIZE property inspecting
> > the draft SensorPhysicalSize property reported by libcamera.
> >
> > Maintain a default value to support pipelines that do not register
> > the camera property.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index bad8a51ae7f7..f50a539e907d 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -919,12 +919,20 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  				  testPatterModes.data(),
> >  				  testPatterModes.size());
> >
> > -	std::vector<float> physicalSize = {
> > -		2592, 1944,
> > -	};
> > +	std::vector<float> physicalSize(2);
> > +	if (properties.contains(properties::draft::SensorPhysicalSize)) {
> > +		const Span<const float> data =
> > +			properties.get<Span<const float>>(properties::draft::SensorPhysicalSize);
> > +		physicalSize = { data[0], data[1] };
> > +	} else {
> > +		/*
> > +		 * \todo Drop the default once all pipelines report the
> > +		 * property.
> > +		 */
>
> Down the line this kind of issue will be caught by a pipeline handler
> test suite. I'm wondering if we could already drop the fallback here, as
> we log a warning when the sensor database doesn't include information
> for the sensor.

I recently broken cros with such a decision. I would keep defaults
around as we do for the active area for a little longer.

>
> This will leave open the question of how to handle UVC cameras, as the
> sensor database isn't applicable to them. I suppose we can't do much in
> that case, all we need is to ensure the HAL won't crash. Later we'll
> probably need to retrieve the information from a configuration file for
> built-in UVC cameras.
>
> > +		physicalSize = { 2.592, 1.944 };
> > +	}
> >  	staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,
> > -				  physicalSize.data(),
> > -				  physicalSize.size());
> > +				  physicalSize.data(), physicalSize.size());
> >
> >  	uint8_t timestampSource = ANDROID_SENSOR_INFO_TIMESTAMP_SOURCE_UNKNOWN;
> >  	staticMetadata_->addEntry(ANDROID_SENSOR_INFO_TIMESTAMP_SOURCE,
>
> --
> Regards,
>
> Laurent Pinchart