[libcamera-devel] [PATCH v3 5/6] libcamera: sensor: Add OV5670 camera sensor

[Kieran Bingham]

Hi Jacopo

On 09/03/2020 18:04, Jacopo Mondi wrote:
> Add OV5670CameraSensor class to handle Omnivision OV5670 image sensor

/Add /Add an /

/handle Omnivision /handle the Omnivision /

> and register it to the camera sensor factory and initialize its pixel
> array properties by overriding CameraSensor::initProperties() method.
> 

In General, I think this is a pretty good way to collate both the fixed
sensor specific data, along with the 'dynamic control' data which will
be obtained from V4L2.

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Now we just need to add some more starting point sensor devices here...

> 
> ---
> v2 -> v3:
> - Squash class introduction and patches registration
> - Update to use the new properties names
> ---
>  src/libcamera/meson.build        |  1 +
>  src/libcamera/sensor/meson.build |  3 ++
>  src/libcamera/sensor/ov5670.cpp  | 60 ++++++++++++++++++++++++++++++++
>  3 files changed, 64 insertions(+)
>  create mode 100644 src/libcamera/sensor/meson.build
>  create mode 100644 src/libcamera/sensor/ov5670.cpp
> 
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 87fa09cde63d..358c784baa20 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -57,6 +57,7 @@ includes = [
>  
>  subdir('pipeline')
>  subdir('proxy')
> +subdir('sensor')
>  
>  libatomic = cc.find_library('atomic', required : false)
>  libdl = cc.find_library('dl')
> diff --git a/src/libcamera/sensor/meson.build b/src/libcamera/sensor/meson.build
> new file mode 100644
> index 000000000000..7af70370cf5c
> --- /dev/null
> +++ b/src/libcamera/sensor/meson.build
> @@ -0,0 +1,3 @@
> +libcamera_sources += files([
> +    'ov5670.cpp',
> +])
> diff --git a/src/libcamera/sensor/ov5670.cpp b/src/libcamera/sensor/ov5670.cpp
> new file mode 100644
> index 000000000000..a027a0816e58
> --- /dev/null
> +++ b/src/libcamera/sensor/ov5670.cpp
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * ov5670.cpp - OV5670 camera sensor
> + */
> +
> +#include <libcamera/property_ids.h>
> +
> +#include "camera_sensor.h"
> +
> +/**
> + * \file ov5670.cpp
> + * \brief Omnivision OV5670 image sensor
> + */
> +
> +namespace libcamera {
> +
> +class OV5670 final : public CameraSensor
> +{
> +public:
> +	OV5670(const MediaEntity *entity);
> +	int initProperties();
> +};
> +
> +/**
> + * \class OV5670
> + * \brief Camera sensor class for Omnivision OV5670 image sensor
> + */
> +
> +/**
> + * \brief Construct the ov5670 sensor class
> + * \param[in] entity The media entity representing the sensor
> + */
> +OV5670::OV5670(const MediaEntity *entity)
> +	: CameraSensor(entity)
> +{
> +}
> +
> +/**
> + * \brief Initialize Camera properties with ov5670 specific values
> + * \return 0 on success, a negative error code otherwise
> + */
> +int OV5670::initProperties()
> +{
> +	/* Pixel Array Properties. */
> +	properties_.set(properties::PixelArraySize, { 2.9457f, 2.214f });
> +	properties_.set(properties::PixelArrayBounds, { 2592, 1944 });
> +	properties_.set(properties::PixelArrays, { 2592, 1944 });
> +	properties_.set(properties::ActiveAreaSizes, { 16, 6, 2560, 1920 });
> +	properties_.set(properties::BayerFilterArrangement,
> +			properties::BayerFilterGRBG);

Ah, now I see this in use - I see how the 'name spacing' is used on the
properties..

> +	properties_.set(properties::ISOSensitivityRange, { 50, 800 });

I wonder if we need a discrete list of ISO sensitivities or if it's
valid to use a continuous range between the min/max?

> +
> +	return CameraSensor::initProperties();
> +}
> +
> +REGISTER_CAMERA_SENSOR(OV5670, "ov5670");
> +
> +}; /* namespace libcamera */
> 

-- 
Regards
--
Kieran