[libcamera-devel] [PATCH 3/6] libcamera: Add V4L2Controls
Jacopo Mondi
jacopo at jmondi.org
Mon Jun 10 09:53:23 CEST 2019
Hi Laurent,
On Sat, Jun 08, 2019 at 07:17:33PM +0300, Laurent Pinchart wrote:
> Hello Jacopo,
>
> Thank you for the patch.
>
> On Mon, Jun 03, 2019 at 10:47:00AM +0100, Kieran Bingham wrote:
> > On 02/06/2019 14:04, Jacopo Mondi wrote:
> > > Add Libcamera V4L2 control support, implemented using the V4L2 Extended
> > > Control APIs. This patch defines the types used to define and manage controls.
> >
> >
> > I haven't done a full documentation review here, I'm just trying to
> > focus on the code parts first.
> >
That's fine, thanks anyway!
> > --
> > KB
> >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > > src/libcamera/include/v4l2_controls.h | 147 +++++++++++++
> > > src/libcamera/meson.build | 1 +
> > > src/libcamera/v4l2_controls.cpp | 301 ++++++++++++++++++++++++++
> > > 3 files changed, 449 insertions(+)
> > > create mode 100644 src/libcamera/include/v4l2_controls.h
> > > create mode 100644 src/libcamera/v4l2_controls.cpp
> > >
> > > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> > > new file mode 100644
> > > index 000000000000..df955c2e10b2
> > > --- /dev/null
> > > +++ b/src/libcamera/include/v4l2_controls.h
> > > @@ -0,0 +1,147 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2019, Google Inc.
> > > + *
> > > + * v4l2_controls.h - V4L2 Extended Control Support
> > > + */
> > > +
> > > +#ifndef __LIBCAMERA_V4L2_CONTROL_H__
> > > +#define __LIBCAMERA_V4L2_CONTROL_H__
>
> s/CONTROL/CONTROLS/
>
> > > +
> > > +#include <cstring>
> > > +
> > > +#include <linux/v4l2-controls.h>
> > > +#include <linux/videodev2.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +class V4L2Control
> > > +{
> > > +public:
> > > + virtual ~V4L2Control()
> > > + {
> > > + }
> > > +
> > > + unsigned int id() { return id_; }
> > > + unsigned int size() { return size_; }
> > > + enum v4l2_ctrl_type type() const { return type_; }
> > > +
> > > +protected:
> > > + V4L2Control(unsigned int id, unsigned int size,
> > > + enum v4l2_ctrl_type type)
> > > + : id_(id), size_(size), type_(type)
> > > + {
> > > + }
> > > +
> > > +private:
> > > + unsigned int id_;
> > > + unsigned int size_;
> > > + enum v4l2_ctrl_type type_;
> > > +};
> > > +
> > > +template<typename T>
> > > +class V4L2ControlValue : public V4L2Control
> >
> > Interesting, I thought about templating to do the types for the
> > Libcamera Controls, but instead ended up embedding the type in the
> > object to avoid having handle casting things ...
> >
> > Although that then leads me to require having a getBool, getInteger,
> > getXXX so we can evaluate both options side by side, and perhaps then
> > unify the model for both.
> >
> > (I'm hoping to get some patches posted soon which will show the
> > difference in approach)
> >
> > The templating might organise memory better, as in my implementation I
> > can add a bool, and int to a union, but not a std::string object, so I
> > incur wasted space.
>
> The trouble with the template is that it requires passing containers of
> V4L2Control pointers, making it complicated for the caller (especially
> when deleting all the elements).
>
> An alternative would be to create a V4L2Controls class that would wrap
> around the container, and provide accessors to get/set values, and to
> iterate over controls. How about the following pseudo-code from the
> point of view of the caller ?
>
> V4L2Device *dev = ...;
> V4L2Controls ctrls;
I like this API but
>
> ctrls.set(V4L2_CID_EXPOSURE, 42);
> ctrls.set(V4L2_CID_GAIN, 100);
>
I uinderstand I could overload on the second parameter type (but that
required called to be very precise, as in
ctrls.set(V4L2_CID_...,
static_cast<uint8_t *>(val));
ctrls.set(V4L2_CID_...,
static_cast<uint16_t *>(val2));
Or I could make this a template and specialize based on the type
returned by query_ctrl()
> dev->setControls(&ctrls);
>
> and
>
> V4L2Device *dev = ...;
> V4L2Controls ctrls;
>
> ctrls.get(V4L2_CID_EXPOSURE);
> ctrls.get(V4L2_CID_GAIN);
>
> dev->getControls(&ctrls);
>
> unsigned int exposure = ctrls.value(V4L2_CID_EXPOSURE);
I'm not sure I can think how that would ever work considering
overloading on the return type and deducing the type of a template on
return value is not possible.
Soemthing like this should not work afaict:
T V4L2Controls::value(unsigned int);
As that could only be called on a specialized instance where T has a
type assigned already, and the V4L2Control class that wraps the
container should not expose any of the specialized derived classes
to the API users.
I can try to play around a bit with this approach, but if you have any
track I should follow let me know.
>
> It's important to make the API easy to use for pipeline handlers and
> efficient in terms of both CPU time and memory. I expect tradeoffs of
> course :-)
>
> > > +{
> > > +public:
> > > + T value() const { return value_; }
> > > + T *mem() const { return memvalue_; }
> >
> > s/memvalue_/mem_/? or /mem()/memvalue()/?
> >
> > (I prefer mem())
> >
> > > +
> > > +protected:
> > > + V4L2ControlValue(unsigned int id, unsigned int size,
> > > + enum v4l2_ctrl_type type, T value)
> > > + : V4L2Control(id, size, type)
> > > + {
> > > + value_ = value;
> > > + memvalue_ = nullptr;
> > > + }
> > > +
> > > + V4L2ControlValue(unsigned int id, unsigned int size,
> > > + enum v4l2_ctrl_type type, T *value)
> > > + : V4L2Control(id, size, type)
> > > + {
> > > + value_ = 0;
> > > + memvalue_ = static_cast<T *>(new T[size]);
> > > + memcpy(memvalue_, value, size);
> > > + }
> > > +
> > > + ~V4L2ControlValue()
> > > + {
> > > + delete[] memvalue_;
> > > + }
> > > +
> > > +private:
> > > + T value_;
> > > + T *memvalue_;
> > > +};
> > > +
> > > +class V4L2IntControl : public V4L2ControlValue<int32_t>
> > > +{
> > > +public:
> > > + V4L2IntControl(unsigned int id, int32_t value)
> > > + : V4L2ControlValue<int32_t>(id, sizeof(int32_t),
> > > + V4L2_CTRL_TYPE_INTEGER, value)
> > > + {
> > > + }
> > > +};
> > > +
> > > +class V4L2Int64Control : public V4L2ControlValue<int64_t>
> > > +{
> > > +public:
> > > + V4L2Int64Control(unsigned int id, int64_t value)
> > > + : V4L2ControlValue<int64_t>(id, sizeof(int64_t),
> > > + V4L2_CTRL_TYPE_INTEGER64, value)
> > > + {
> > > + }
> > > +};
> > > +
> > > +class V4L2BoolControl : public V4L2ControlValue<bool>
> > > +{
> > > +public:
> > > + V4L2BoolControl(unsigned int id, bool value)
> > > + : V4L2ControlValue<bool>(id, sizeof(bool),
> > > + V4L2_CTRL_TYPE_BOOLEAN, value)
> > > + {
> > > + }
> > > +};
> > > +
> > > +class V4L2StringControl : public V4L2ControlValue<std::string>
> >
> > You might want/need to to support char* types too.
> >
> > > +{
> > > +public:
> > > + V4L2StringControl(unsigned int id, std::string value)
> > > + : V4L2ControlValue<std::string>(id, value.length(),
> > > + V4L2_CTRL_TYPE_STRING, value)
> > > + {
> > > + }
> > > +};
> > > +
> > > +class V4L2U8Control : public V4L2ControlValue<uint8_t>
> > > +{
> > > +public:
> > > + V4L2U8Control(unsigned int id, unsigned int size, uint8_t *mem)
> > > + : V4L2ControlValue<uint8_t>(id, size, V4L2_CTRL_TYPE_U8, mem)
> > > + {
> > > + }
> > > +};
> > > +
> > > +class V4L2U16Control : public V4L2ControlValue<uint16_t>
> > > +{
> > > +public:
> > > + V4L2U16Control(unsigned int id, unsigned int size, uint16_t *mem)
> > > + : V4L2ControlValue<uint16_t>(id, size, V4L2_CTRL_TYPE_U16, mem)
> > > + {
> > > + }
> > > +};
> > > +
> > > +class V4L2U32Control : public V4L2ControlValue<uint32_t>
> > > +{
> > > +public:
> > > + V4L2U32Control(unsigned int id, unsigned int size, uint32_t *mem)
> > > + : V4L2ControlValue<uint32_t>(id, size, V4L2_CTRL_TYPE_U32, mem)
> > > + {
> > > + }
> > > +};
> > > +
> > > +}; /* namespace libcamera */
> > > +
> > > +#endif /* __LIBCAMERA_V4L2_CONTROL_H__ */
> > > +
> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > > index 6d858a22531e..fa1fbcb5faf5 100644
> > > --- a/src/libcamera/meson.build
> > > +++ b/src/libcamera/meson.build
> > > @@ -22,6 +22,7 @@ libcamera_sources = files([
> > > 'timer.cpp',
> > > 'utils.cpp',
> > > 'v4l2_base.cpp',
> > > + 'v4l2_controls.cpp',
> > > 'v4l2_device.cpp',
> > > 'v4l2_subdevice.cpp',
> > > ])
> > > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> > > new file mode 100644
> > > index 000000000000..f50f3cdfa7d7
> > > --- /dev/null
> > > +++ b/src/libcamera/v4l2_controls.cpp
> > > @@ -0,0 +1,301 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2019, Google Inc.
> > > + *
> > > + * v4l2_controls.cpp - V4L2 Extended Control Support
> > > + */
> > > +
> > > +/**
> > > + * \file v4l2_controls.h
> > > + * \brief Support for V4L2 Controls using the V4L2 Extended Controls APIs.
> > > + *
> > > + * The V4L2 defined "Control API" allows application to inspect and modify set
> >
> > "an application" or "applications"
> >
> > "a set of"
> >
> > application? Are we talking about applications that use libcamera? Or
> > applications in general?
> >
> >
> > > + * of configurable parameters on the video device or subdevice of interest. The
> >
> > /. The/. The/ <double space>
> >
> >
> >
> > > + * nature of the parameters an application could modify using the control
> > > + * framework depends on what the driver implements support for, and on the
> > > + * characteristics of the underlying hardware platform. Generally controls are
> > > + * used to modify user visible settings, such as the image brightness and
> > > + * exposure time, or non-standard parameters which cannot be controlled through
> > > + * the V4L2 format negotiation API.
> > > + *
> > > + * Controls are identified by a numerical id, defined by the V4L2 kernel headers
> > > + * and have an associated type and class. Each control has a 'value', which is
> > > + * the data that can be modified with a 'setControl()' operation call or
> > > + * retrieved with a 'getControl()' one.
> > > + *
> > > + * A control class defines the control purpose while its type (along with the
> > > + * control's flags) defines the type of the control's value content. Controls
> > > + * might transport a single data value stored in variable inside the control, or
> > > + * they might as well deal with more complex data types, such as arrays of
> > > + * matrixes, stored in a contiguous memory locations associated with the control
> >
> > s/matrixes/matrices/
> >
> > > + * and called 'the payload'. The content have to be opportunely copied into the
> > > + * application memory when retrieving a control's value and provided to the V4L2
> > > + * device when setting it.
> > > + *
> > > + * Libcamera supports control using the V4L2 'Extended Control' framework, which
> > > + * allows easier handling of controls with payloads of arbitrary sizes.
> > > + *
> > > + * The Libcamera V4L2 Controls framework operates on lists of controls, to match
> > > + * the V4L2 extended controls framework, but also provides operations to get
> > > + * and set single controls. The interface to set and get control is implemented
> > > + * by the V4L2Base class, while this file only provides the data type
> > > + * definitions.
> > > + *
> > > + * The Libcamera V4L2 control framework data types define a base class
> > > + * V4L2Control that contains the fields common to all controls, regardless of
> > > + * their type, such as the control's id, its type and the expected control's
> > > + * value data size. The V4L2Control class is not meant to be instantiated
> > > + * directly, but is instead used as placeholder to store controls in the
> > > + * standard library provided containers.
> > > + *
> > > + * A parametric derived class V4L2ControlValue stores the control's value and
> > > + * provide accessors to the value itself for control with no payload, or to the
> > > + * memory area that contains the control's data payload. This class is not
> > > + * intended to be directly used and cannot be constructed, but it is used
> > > + * instead to define specialized derived classes which specialize the control's
> > > + * data value type.
> > > + *
> > > + * In order to set and get controls, user of the Libcamera V4L2 control
> > > + * framework should create instances of the specialized derived classes,
> > > + * which are publicly constructible, and use them to access the control's data
> > > + * content using the 'value()' method for controls with no payload, or
> > > + * retrieving reference to the memory location that contains the data payload
> > > + * with the 'mem()' operation for controls which transport a data payload.
> > > + *
> > > + * \todo Support setting controls with data payload.
> > > + */
> > > +
> > > +namespace libcamera {
> > > +
> > > +/**
> > > + * \class V4L2Control
> > > + * \brief Base class for V4L2 Controls
> > > + *
> > > + * The V4L2Control base class is the base class that contains the fields common
> > > + * to all controls (id, type and size).
> > > + *
> > > + * This class is not meant to be instantiated directly, but is instead used as a
> > > + * place holder to store controls in arrays or other containers. User of the
> > > + * libcamera V4L2 control framework should access the controls content by
> > > + * instantiating one of the provided specialized derived classes.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2Control::V4L2Control
> > > + * \brief Construct a V4L2Control instance
> > > + * \param id The control's id
> > > + * \param size The control's data size
> > > + * \param type The control's type
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2Control::id()
> > > + * \brief Retrieve the control's id
> > > + *
> > > + * Retrieve the control's numerical id value as defined by the V4L2
> > > + * specification.
> > > + *
> > > + * \return The control's id
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2Control::size()
> > > + * \brief Retrieve the control's data value size in bytes
> > > + *
> > > + * \todo Better define the value of size() for controls with payload data.
> > > + *
> > > + * \return The control's size
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2Control::type()
> > > + * \brief Retrieve the control's type
> > > + *
> > > + * Retrieve the control's type as defined by the V4L2 specification.
> > > + *
> > > + * \return The control's type
> > > + */
> > > +
> > > +/**
> > > + * \class V4L2ControlValue
> > > + * \brief Template base class that represent values of a V4L2 control
> > > + *
> > > + * The V4L2ControlValue template base class represents a V4L2 control with
> > > + * its different value types.
> > > + *
> > > + * It provides two operations to access the control's value or the pointer
> > > + * to the memory location that contains to the control's value data payload.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2ControlValue::V4L2ControlValue(unsigned int id, unsigned int size, enum v4l2_ctrl_type type, T value)
> > > + * \brief Contruct a V4L2ControlValue with a value
> >
> > /Contruct/Construct/
> >
> > > + * \param id The control's id
> > > + * \param size The control's size
> > > + * \param type The control's type
> > > + * \param value The control's value
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2ControlValue::V4L2ControlValue(unsigned int id, unsigned int size, enum v4l2_ctrl_type type, T* value)
> > > + * \brief Contruct a V4L2ControlValue with a pointer to a data payload
> >
> > /Contruct/Construct/
> >
Thanks Kieran, I'll take you suggestions on comments in.
Thanks
j
> > > + * \param id The control's id
> > > + * \param size The control's size
> > > + * \param type The control's type
> > > + * \param value The pointer to the control's data payload
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2ControlValue::~V4L2ControlValue
> > > + * \brief Release the memory reserved for the control's data payload, if any
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2ControlValue::value()
> > > + * \brief Retrieve the control's value
> > > + *
> > > + * Retrieve the control's value. Valid only for controls with no payload.
> > > + * Access a control's value by calling the value() operation on instances
> > > + * of the specialized derived classes.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2ControlValue::mem()
> > > + * \brief Retrieve a pointer to the memory location of the control's data
> > > + *
> > > + * Retrieve a pointer to the memory location that contains the control's data
> > > + * payload. Valid only for controls with data payload. Access the memory
> > > + * location of the control's data by calling the mem() operation on instances
> > > + * of the specialized derived classes.
> > > + */
> > > +
> > > +/**
> > > + * \class V4L2IntControl
> > > + * \brief Specialized V4L2Control class that handles controls of
> > > + * V4L2_CTRL_TYPE_INTEGER type.
> > > + *
> > > + * Access the control's data value by using the value() operation.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2IntControl::V4L2IntControl()
> > > + * \brief Construct a V4L2Control that contains an int value
> > > + * \param id The control's id
> > > + * \param value The control's value
> > > + */
> > > +
> > > +/**
> > > + * \class V4L2Int64Control
> > > + * \brief Specialized V4L2Control class that handles controls of
> > > + * V4L2_CTRL_TYPE_INTEGER64 type.
> > > + *
> > > + * Access the control's data value by using the value() operation.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2Int64Control::V4L2Int64Control()
> > > + * \brief Construct a V4L2Control that contains an int64 value
> > > + * \param id The control's id
> > > + * \param value The control's value
> > > + */
> > > +
> > > +/**
> > > + * \class V4L2BoolControl
> > > + * \brief Specialized V4L2Control class that handles controls of
> > > + * V4L2_CTRL_TYPE_BOOLEAN type.
> > > + *
> > > + * Access the control's data value by using the value() operation.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2BoolControl::V4L2BoolControl()
> > > + * \brief Construct a V4L2Control that contains a boolean value
> > > + * \param id The control's id
> > > + * \param value The control's value
> > > + */
> > > +
> > > +/**
> > > + * \class V4L2StringControl
> > > + * \brief Specialized V4L2Control class that handles controls of
> > > + * V4L2_CTRL_TYPE_STRING type.
> > > + *
> > > + * Access the control's data value by using the value() operation.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2StringControl::V4L2StringControl()
> > > + * \brief Construct a V4L2Control that contains a string value
> > > + * \param id The control's id
> > > + * \param value The control's value
> > > + */
> > > +
> > > +/**
> > > + * \class V4L2U8Control
> > > + * \brief Specialized V4L2Control class that handles controls with payload of
> > > + * V4L2_CTRL_TYPE_U8 type.
> > > + *
> > > + * Access the control's data value by using the retrieving the memory location
> > > + * where the data payload is stored with the mem() operation. The size of
> > > + * the payload data can be retrieved with the size() operation.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2U8Control::V4L2U8Control()
> > > + * \brief Construct a V4L2Control with payload of uin8_t data
> > > + * \param id The control's id
> > > + * \param size The size in bytes of the payload content
> > > + * \param mem Pointer to the memory location of the payload content
> > > + *
> > > + * Memory is reserved in the newly created instance to hold the data payload
> > > + * and the data content is copied there. The reserved memory is then freed when
> > > + * the Control is destroyed. The memory where the control's payload was copied
> > > + * from should be released by the caller.
> > > + */
> > > +
> > > +/**
> > > + * \class V4L2U16Control
> > > + * \brief Specialized V4L2Control class that handles controls with payload of
> > > + * V4L2_CTRL_TYPE_U16 type.
> > > + *
> > > + * Access the control's data value by using the retrieving the memory location
> > > + * where the data payload is stored with the mem() operation. The size of
> > > + * the payload data can be retrieved with the size() operation.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2U16Control::V4L2U16Control(unsigned int id, unsigned int size, uint16_t *mem)
> > > + * \brief Construct a V4L2Control with payload of uin16_t data
> > > + * \param id The control's id
> > > + * \param size The size in bytes of the payload content
> > > + * \param mem Pointer to the memory location of the payload content
> > > + *
> > > + * Memory is reserved in the newly created instance to hold the data payload
> > > + * and the data content is copied there. The reserved memory is then freed when
> > > + * the Control is destroyed. The memory where the control's payload was copied
> > > + * from should be released by the caller.
> > > + */
> > > +
> > > +/**
> > > + * \class V4L2U32Control
> > > + * \brief Specialized V4L2Control class that handles controls with payload of
> > > + * V4L2_CTRL_TYPE_U32 type.
> > > + *
> > > + * Access the control's data value by using the retrieving the memory location
> > > + * where the data payload is stored with the mem() operation. The size of
> > > + * the payload data can be retrieved with the size() operation.
> > > + */
> > > +
> > > +/**
> > > + * \fn V4L2U32Control::V4L2U32Control(unsigned int id, unsigned int size, uint32_t *mem)
> > > + * \brief Construct a V4L2Control with payload of uin32_t data
> > > + * \param id The control's id
> > > + * \param size The size in bytes of the payload content
> > > + * \param mem Pointer to the memory location of the payload content
> > > + *
> > > + * Memory is reserved in the newly created instance to hold the data payload
> > > + * and the data content is copied there. The reserved memory is then freed when
> > > + * the Control is destroyed. The memory where the control's payload was copied
> > > + * from should be released by the caller.
> > > + */
> > > +
> > > +}; /* 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/20190610/3b06f4dc/attachment.sig>
More information about the libcamera-devel
mailing list