[libcamera-devel] [PATCH 5/5] libcamera: camera_sensor: Retrieve sensor sizes

[Jacopo Mondi]

Hi Laurent,

On Sat, Aug 17, 2019 at 07:28:37PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Sat, Aug 17, 2019 at 12:59:37PM +0200, Jacopo Mondi wrote:
> > Retrieve the camera sensor pixel array sizes and the active pixel sizes
> > using the V4L2 selection APIs.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/camera_sensor.cpp       | 13 +++++++++++++
> >  src/libcamera/include/camera_sensor.h |  2 ++
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index 2703d10c719e..e6b01c242328 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -114,6 +114,19 @@ int CameraSensor::init()
> >  		return -EINVAL;
> >  	}
> >
> > +	/* Retrieve and store the sensor pixel array and active area sizes. */
>
> Nit-picking, the active area is a rectangle, not a size unlike the pixel
> array.
>
> > +	ret = subdev_->getCropBounds(0, &activeAreaSize_);
> > +	if (ret)
> > +		return ret;
> > +
> > +	Rectangle rect = {};
>
> I think you can skip the initialisation of rect.
>
> > +	ret = subdev_->getNativeSize(0, &rect);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pixelArraySize_.width = rect.w;
> > +	pixelArraySize_.height = rect.h;
> > +
> >  	/* Enumerate and cache media bus codes and sizes. */
> >  	const ImageFormats formats = subdev_->formats(0);
> >  	if (formats.isEmpty()) {
> > diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> > index a237a1684605..f6b184bf2838 100644
> > --- a/src/libcamera/include/camera_sensor.h
> > +++ b/src/libcamera/include/camera_sensor.h
> > @@ -58,6 +58,8 @@ private:
> >  	std::vector<Size> sizes_;
> >
> >  	CameraLocation location_;
> > +	Size pixelArraySize_;
> > +	Rectangle activeAreaSize_;
>
> Should you provide get accessors for these (with documentation :-)) ?
>

That, or expose them as properties, that are gathered together with
the Camera reported ones in a single list of Camera properties. I
would prefer going for properties directly instead of going through
accessors and build the properties list from there.

> Do we also need to expose the sensor physical size ?
>

For android HAL integration purposes, yes:
https://jmondi.org/android_metadata_tags/ddocs.html#static_android.sensor.info.physicalSize
as well as rotation:
https://jmondi.org/android_metadata_tags/docs.html#static_android.sensor.orientation
and sensitiviy range:
https://jmondi.org/android_metadata_tags/docs.html#static_android.sensor.info.sensitivityRange

With these we would complete support for static metadata tags marked as "BC"
required to support the LEGACY hardware level.

Goging forward there are much more sensor/lens related properties that
will need to be retrieved, in particular to support the RAW mode.

But that's for later.


> >  };
> >
> >  } /* namespace libcamera */
>
> --
> Regards,
>
> Laurent Pinchart
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190819/88ad35ee/attachment.sig>