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

[Laurent Pinchart]

Hi Jacopo,

On Mon, Aug 19, 2019 at 09:45:51AM +0200, Jacopo Mondi wrote:
> On Sat, Aug 17, 2019 at 07:28:37PM +0300, Laurent Pinchart wrote:
> > 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.

I'm not entirely sure about that. I think at least the active area would
be useful for pippeline handlers to know, and going through properties
may not be the best API for that. Furthermore, I wonder if pipeline
handlers wouldn't need to mangle data from the camera sensor before
exposing them as controls. I suppose we'll figure it out as we move
forward.

> > 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.

Sure :-)

> > >  };
> > >
> > >  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart