[libcamera-devel] [RFC 5/7] libcamera: sensor: ov5670: Register pixel array properties

[Jacopo Mondi]

Hello Sakari,
    happy to have you here!

On Mon, Jan 20, 2020 at 11:18:56PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Wed, Dec 18, 2019 at 03:49:59PM +0100, Jacopo Mondi wrote:
> > Implement sensor specific pixel array properties initialization for the
> > OV5670 sensor driver by overriding CameraSensor::initProperties()
> > method.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/sensor/ov5670.cpp | 19 +++++++++++++++++++
> >  src/libcamera/sensor/ov5670.h   |  3 +++
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/src/libcamera/sensor/ov5670.cpp b/src/libcamera/sensor/ov5670.cpp
> > index ca9f3c1d544f..c2d996785717 100644
> > --- a/src/libcamera/sensor/ov5670.cpp
> > +++ b/src/libcamera/sensor/ov5670.cpp
> > @@ -6,6 +6,10 @@
> >   */
> >
> >  #include "ov5670.h"
> > +
> > +#include <libcamera/controls.h>
> > +#include <libcamera/property_ids.h>
> > +
> >  #include "camera_sensor.h"
> >
> >  namespace libcamera {
> > @@ -15,4 +19,19 @@ OV5670CameraSensor::OV5670CameraSensor(const MediaEntity *entity)
> >  {
> >  }
> >
> > +int OV5670CameraSensor::initProperties(const ControlInfoMap &controlMap)
> > +{
> > +	/* Pixel Array Properties. */
> > +	properties_.set(properties::PixelArraySize, { 2.9f, 1.18f });
> > +	properties_.set(properties::PixelArrayBounds, { 2592, 1944 });
> > +	properties_.set(properties::PixelArrays, { 2592, 1944 });
> > +	properties_.set(properties::ActiveAreaSize, { 16, 6, 2560, 1920 });
>
> Could these two be obtained from the driver?
>
> At least in principle there's the NATIVE_SIZE target. Although not all
> drivers support modes without cropping...
>

Do you mean something like this ? Maybe adjusted to report the full
pixel array size through NATIVE_SIZE and the active area through
CROP_BOUNDS...

https://lore.kernel.org/linux-media/20190827092339.8858-1-jacopo@jmondi.org/T/#med4e833fddd3f1b21e2a4ea82fc2b4fceaeb4bc3

I recall I decided to drop those patches from the next iterations of
that series as we there where controversy there, which to be honest, I
don't fully remember. Would you be fine seeing patches for the sensor
driver adding those two selection drivers ?

> > +	int32_t bayerFilter = properties::BayerFilterGRBG;
> > +	properties_.set(properties::BayerFilterArrangement, bayerFilter);
>
> This one as well, perhaps?
>

do we have a control for that ? I haven't seen any in the Camera and
Image source control classes. Are you suggesting to try and add a new
one ;) ?

> > +	properties_.set(properties::ISOSensitivityRange, { 50, 800 });
> > +
> > +	return CameraSensor::initProperties(controlMap);
> > +}
> > +
> >  }; /* namespace libcamera */
> > +
> > diff --git a/src/libcamera/sensor/ov5670.h b/src/libcamera/sensor/ov5670.h
> > index f84239c8411d..564a4546d69b 100644
> > --- a/src/libcamera/sensor/ov5670.h
> > +++ b/src/libcamera/sensor/ov5670.h
> > @@ -11,8 +11,11 @@
> >
> >  namespace libcamera {
> >
> > +class ControlInfoMap;
> >  class OV5670CameraSensor final : public CameraSensor
> >  {
> > +	int initProperties(const ControlInfoMap &controlMap);
> > +
> >  private:
> >  	OV5670CameraSensor(const MediaEntity *entity);
> >  	friend CameraSensorFactory;
>
> --
> Regards,
>
> Sakari Ailus
-------------- 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/20200121/f1db757e/attachment.sig>