[libcamera-devel] [PATCH v4 1/6] libcamera: Add V4L2Controls

Jacopo Mondi jacopo at jmondi.org
Thu Jun 20 10:55:52 CEST 2019


Hi Laurent,

On Wed, Jun 19, 2019 at 10:41:27PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Jun 19, 2019 at 01:08:53PM +0200, 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.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/include/v4l2_controls.h |  96 ++++++++
> >  src/libcamera/meson.build             |   1 +
> >  src/libcamera/v4l2_controls.cpp       | 337 ++++++++++++++++++++++++++
> >  3 files changed, 434 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..d7b12801329e
> > --- /dev/null
> > +++ b/src/libcamera/include/v4l2_controls.h
> > @@ -0,0 +1,96 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * v4l2_controls.h - V4L2 Extended Control Support
>
> Maybe just V4L2 Control Support ?
>
> > + */
> > +
> > +#ifndef __LIBCAMERA_V4L2_CONTROLS_H__
> > +#define __LIBCAMERA_V4L2_CONTROLS_H__
> > +
> > +#include <cstring>
> > +#include <string>
> > +#include <vector>
> > +
> > +#include <stdint.h>
> > +
> > +#include <linux/v4l2-controls.h>
> > +#include <linux/videodev2.h>
> > +
> > +namespace libcamera {
> > +
> > +class V4L2ControlInfo
> > +{
> > +public:
> > +	V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
> > +
> > +	unsigned int id() const { return id_; }
> > +	unsigned int type() const { return type_; }
> > +	size_t size() const { return size_; }
> > +	const std::string &name() const { return name_; }
>
> The name is only used in patch 6/6 to print messages. Do you think we
> should keep it, or can we drop it to save memory ? It's an open
> question.
>

I think the human readable name might be quite useful for debug messages,
and I do expect pipeline handlers developers might want to access it.

It could be added when required, as patch 6/6 is just an hack to
demonstrate how to use controls, but I would keep it there to be
honest.


> > +
> > +private:
> > +	unsigned int type_;
> > +	std::string name_;
> > +	unsigned int id_;
> > +	size_t size_;
> > +};
> > +
> > +class V4L2Control
> > +{
> > +public:
> > +	unsigned int id() const { return id_; }
> > +
> > +private:
> > +	friend class V4L2Device;
> > +	friend class V4L2Controls;
> > +
> > +	V4L2Control(unsigned int id)
> > +		: id_(id) {}
> > +	V4L2Control(unsigned int id, int value)
> > +		: id_(id), value_(value) {}
> > +
> > +	long int value() const { return value_; }
> > +	void setValue(long int value) { value_ = value; }
>
> long int and int are equivalent. I would use types with an explicit size
> (uint_* or int_*). I would also use the same type for the value in the
> constructor.
>

Not on 64bits platforms :)

$ ./a.out
sizeof(long): 8
sizeof(int): 4

Anyway, to avoid this incongruences, I should probably use types with
an explicit size.

However, I wonder if that would require users to go through a cast
everytime they set the value, which I don't really like.

So my idea was to always use a long int (fixed at 8 Bytes in my head)
and eventually cast it down to 32bits for controls that use an int32_t
type.

> > +
> > +	unsigned int id_;
> > +	long int value_;
> > +};
> > +
> > +class V4L2Controls
> > +{
> > +public:
> > +	V4L2Controls &operator=(const V4L2Controls &) = delete;
>
> Should you then also delete the copy constructor ?
>

I was undecided. Do we want to be able to copy controls?

> > +
> > +	using iterator = std::vector<V4L2Control>::iterator;
> > +	using const_iterator = std::vector<V4L2Control>::const_iterator;
> > +
> > +	iterator begin() { return controls_.begin(); }
> > +	const_iterator begin() const { return controls_.begin(); }
> > +	iterator end() { return controls_.end(); }
> > +	const_iterator end() const { return controls_.end(); }
> > +
> > +	bool empty() const { return controls_.empty(); }
> > +	size_t size() { return controls_.size(); }
>
> This method can be const too.
>
> > +	void reset() { controls_.clear(); }
> > +
> > +	V4L2Control *operator[](unsigned int index)
> > +	{
> > +		return &controls_[index];
> > +	}
> > +
> > +	void add(unsigned int id, long int value);
> > +	void add(unsigned int id);
>
> You could merge these two functions into one with
>
> 	void add(unsigned int id, long int value = 0);
>
> That way all control values will be initialised to a value, there will
> be no uninitialised memory.
>

Indeed, good suggestion.

> > +
> > +	int get(unsigned int id);
> > +	int set(unsigned int id, long int value);
>
> Integer types with explicit lengths should be used here too.
>
> What is the purpose of the set() function ? Is it used by V4L2Device
> only, to update the value after a get or set ? If so I would make the
> controls_ vector available to V4L2Device, which could iterate over it
> without having to check for IDs, as the internal controls array that is
> used for the ioctls will be in the same order as the vector. That would
> optimise the getControls() and setControls() operations.

Not just by V4L2Device, but if you look at patch 6/6 you'll see that
users of the controls API might want to set values of a control
instance to have it then written to the device.

The usage patter is:
V4L2Controls controls;
controls.add(V4L2_CID_...);
v4l2Device->getControls(&controls);

/* Inspect the control, if you don't like the value then change it */
controls.set(V4L2_CID_..., $newvalue)l
v4l2Device->setControl(&controls);

So I think we should keep set() as re-using a control already part of
a V4L2Controls instance is something useful.
>
> I also had a look at the usage pattern of the get() function in patch
> 6/6, and it looks a bit inefficient. The caller loops over controls
> using the public iterators, to then extract the control ID using the
> V4L2Control::id() method, and then calls V4L2Controls::get() with the
> ID, which searches the controls_ vector again. We start with an item in
> the vector, and then look it up.

Indeed, it sucks a bit, as it requires O(n^2) iterations on the
controls_ vector.

Anyway, I do not expect users to loop over controls in the way I'm
doing in 6/6 but rather access single controls with get() as they
should already know the control ID they're looking for.

>
> One option would be to drop the get() method here, and make the value()
> method public on V4L2Control. Both get() and set() would then be
> removed, with only add() staying.

But then in that case you could only access a single control by
iterating on the V4L2Controls instance. I'm not sure this is the only
usage patter we want to enforce. In that case, being able to access
the control value directly might be useful to avoid an unecessary
iteration on controls_ again, so I might consider making it public,
but I won't remove get() completely.

>
> > +
> > +private:
> > +	V4L2Control *getControl(unsigned int id);
> > +
> > +	std::vector<V4L2Control> controls_;
>
> This makes control lookup by ID a bit inefficient (and requires the
> getControl() method). Should we have a std::map<unsigned int,
> V4L2Control *> that stores pointers to the controls in the controls_
> vector ? getControl() could then be dropped. It would cost a bit more
> memory and a bit more CPU time in the add() operation, but would save
> time at set() and get() time. If set() gets dropped as proposed above,
> only get() will remain, and I wonder if it will be worth it. For small
> sets of controls possibly not, but for large sets it could make a
> difference. What do you think ?
>

As getControl() is private to V4L2Controls it could be dropped an a
map could be used instead. However, I'm not sure what is the
advantage, considering the number of controls we expect to have in a
device. In other words, the backing storage of maps are RB trees, and
the complexity of an access is O(log N) compared to the linear search
of a vector. However insertion and deletion is a bit more costly than
vector, and we cannot replace the vector completely as you have
pointed out, as the order in which to set controls is relevant.

So we're going to increase the memory occupation for little gain in my
opinion.

(What I read online is also that for small sets ( < hundreds of
elements) vectors might be more efficient than maps, but since it's
just something I found on stackoverflow without too much information
in support of the claim, I take it with a grain of salt).

To sum up, I could add a map to support the existing vector, but not
replace it completely and the gain is not that clear to me, also
considering the expected number of elements in the containers.

> > +};
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_V4L2_CONTROLS_H__ */
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index f26ad5b2dc57..985aa7e8ab0e 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -23,6 +23,7 @@ libcamera_sources = files([
> >      'stream.cpp',
> >      'timer.cpp',
> >      'utils.cpp',
> > +    'v4l2_controls.cpp',
> >      'v4l2_device.cpp',
> >      'v4l2_subdevice.cpp',
> >      'v4l2_videodevice.cpp',
> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> > new file mode 100644
> > index 000000000000..75b9c29ca133
> > --- /dev/null
> > +++ b/src/libcamera/v4l2_controls.cpp
> > @@ -0,0 +1,337 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2019, Google Inc.
> > + *
> > + * v4l2_controls.cpp - V4L2 Extended Control Support
> > + */
> > +
> > +#include "v4l2_controls.h"
> > +
> > +/**
> > + * \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
> > + * of configurable parameters on the video device or subdevice of interest. The
> > + * 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
>
> I would drop ' and class', we don't care about control classes.
>
> > + * the data that can be modified with a call to  the 'V4L2Device::setControls()'
>
> s/  / /
>
> > + * operation or retrieved with a call to 'V4L2Device::getControls()'.
>
> I don't think you need to enclose the method name in quotes, you can
> write
>
>  * Each control has a value, which is the data that can be modified with
>  * V4L2Device::setControls() or retrieved with V4L2Device::getControls().
>
> > + *
> > + * A control class defines the control purpose while its type (along with the
>
> Similarly, you can drop the class here.
>
> > + * 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
> > + * matrices, stored in a contiguous memory locations associated with the control
> > + * and called 'the payload'. Such controls are called 'compound controls' and
> > + * are currently not supported by the Libcamera V4L2 control framework.
>
> s/Libcamera/libcamera/
>
> > + *
> > + * \todo Add support for compound controls
> > + *
> > + * Libcamera implements support for control using the V4L2 'Extended Control'
>
> s/Libcamera/libcamera/
> s/control/controls/
>
> > + * framework, which allows easier future handling of controls with payloads
>
> s/framework/API/
> s/easier //
>
> > + * of arbitrary sizes.
> > + *
> > + * The Libcamera V4L2 Controls framework operates on lists of controls, wrapped
>
> s/Libcamera/libcamera/
>
> > + * by the V4L2Controls class, to match the V4L2 extended controls framework.
>
> s/framework/API/
>
> > + * The interface to set and get control is implemented by the V4L2Device class,
> > + * and this file only provides the data type definitions.
> > + */
> > +
> > +namespace libcamera {
> > +
> > +/**
> > + * \class V4L2ControlInfo
> > + * \brief Information on a V4L2 control
> > + *
> > + * The V4L2ControlInfo class represent all the informations relative to
>
> s/informations/information/ (uncoutable)
> s/relative/related/
>
> > + * a V4L2 control, such as its id, its type, its user-readable name and the
> > + * expected size of its value data.
> > + *
> > + * V4L2ControlInfo instances are created by inspecting the field of
> > + * a struct v4l2_query_ext_ctrl structure, after it has been filled by the
> > + * device driver as a consequence of an VIDIOC_QUERY_EXT_CTRL ioctl call.
>
> s/of an/of a/
>
> > + *
> > + * This class does not contain the control value, but only static informations
>
> s/informations/information/
>
> > + * on the control, which should ideally be cached the first time the control
>
> I'd be a bit more directive here, "which shall be cached by the caller
> at initialisation time or the first time the control information is
> needed." and drop the rest of the sentence.
>
> > + * is accessed to be reused each time the control value need to be read or
>
> s/need/needs/
>
> > + * applied to the hardware.
> > + */
> > +
> > +/**
> > + * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl
> > + */
> > +V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> > +{
> > +	id_ = ctrl.id;
> > +	type_ = ctrl.type;
> > +	name_ = static_cast<const char *>(ctrl.name);
> > +	size_ = ctrl.elem_size * ctrl.elems;
> > +}
> > +
> > +/**
> > + * \fn V4L2ControlInfo::id()
> > + * \brief Retrieve the control id this instance refers to
>
> I would just say "Retrieve the control ID"
>
> > + * \return The V4L2 control id
>
> s/id/ID/ (as well as in other locations below)
>
> > + */
> > +
> > +/**
> > + * \fn V4L2ControlInfo::type()
> > + * \brief Retrieve the control type as defined by V4L2_CTRL_TYPE_
>
> Maybe V4L2_CTRL_TYPE_* ?
>
> > + * \return The V4L2 control type
> > + */
> > +
> > +/**
> > + * \fn V4L2ControlInfo::size()
> > + * \brief Retrieve the control value data size (in bytes)
> > + * \return The V4L2 control value data size
> > + */
> > +
> > +/**
> > + * \fn V4L2ControlInfo::name()
> > + * \brief Retrieve the control user readable name
> > + * \return The V4L2 control user readable name
> > + */
> > +
> > +/**
> > + * \class V4L2Control
> > + * \brief A V4L2 control value
> > + *
> > + * The V4L2Control class represent the value of a V4L2 control.
> > + * The class stores values that have been read from or will be applied to
> > + * a V4L2 device.
> > + *
> > + * The value stored in the class instances does not reflect what is actually
> > + * applied to the hardware but is a pure software cache optionally initialized
> > + * at control creation time and only modified by a control read operation or a
> > + * call to V4L2Controls::set().
> > + *
> > + * The V4L2Control class instances are not meant to be directly used but are
> > + * instead intended to be grouped in V4L2Controls instances, which are then
> > + * passed as parameters to the set and get control operations implemented in
> > + * V4L2Device::setControls() and V4L2Device::setControls() respectively.
>
> The second one should be V4L2Device::getControls(), or you could write
>
> "passed as parameter to V4L2Device::getControls() and
> V4L2Device::setControls()."
>
> > + *
> > + * In facts, access to the class constructor and to the control value accessor
> > + * and modifier are restricted to the friend V4L2Controls and V4L2Device
> > + * classes.
>
> I think you can drop this paragraph.
>
> > + */
> > +
> > +/**
> > + * \fn V4L2Control::id()
> > + * \brief Retrieve the control id this instance refers to
> > + * \return The V4L2Control id
> > + */
> > +
> > +/**
> > + * \class V4L2Controls
> > + * \brief Wraps a list of V4L2Control
>
> I would say "Container of V4L2Control instance" and replace wrapper by
> container below.
>
> > + *
> > + * The V4L2Controls class works as a wrapper for a list of V4L2Control
> > + * instances. The class provides operations to add a new control to the list,
> > + * get back a control value, and reset the list of controls it contains.
> > + *
> > + * In order to set and get controls, user of the Libcamera V4L2 control
>
> s/Libcamera/libcamera/
>
> > + * framework should operate on instances of the V4L2Controls class, and use
> > + * them as argument for the V4L2Device::setControls() and
> > + * V4L2Device::getControls() operations, which write and read a list of
> > + * controls from or to a V4L2 device (a video device or a subdevice).
> > + *
> > + * Controls are added to a V4L2Controls instance with the add() method, with
> > + * or without an initial value.
> > + *
> > + * To write controls to a device, the controls of interest should be added
>
> s/should/shall/
>
> > + * with an initial value by calling V4L2Controls::add(unsigned int id, long
> > + * int value) to prepare for a write operation. The value of controls which
> > + * are already part of the instance could be updated with a call to
> > + * V4L2Controls::set() operation.
>
> Is there a use case for this ?
>

See the above example.

> > Once the values of all controls of interest
> > + * have been initialized in a V4L2Controls instance, this should be then
> > + * passed to the V4L2Device::setControls() operation, which applies each
> > + * control in the list to the hardware.
>
> Once all desired control have been added, the V4L2Controls instance is
> passed to V4L2Device::setControls(), which sets the controls on the
> device.
>
> > + *
> > + * Alternatively a control can be add without any initial value by calling
> > + * V4L2Controls::add(unsigned int id) and then read from the device passing
> > + * the V4L2Controls instance to V4L2Device::getControls() which read each
> > + * control value from the hardware.
> > + *
> > + * Reading controls with an initialized value from the device, overwrites the
> > + * control's value, reflecting what has been actually read from the hardware.
>
> Let's write this like the previous paragraph.
>
> "To read controls from a device, the desired controls are added with
> V4L2Controls::add(unsigned int id) to prepare for a read operation. The
> V4L2Controls instance is then passed to V4L2Device::getControls(), which
> reads the controls from the device and updates the values stored in
> V4L2Controls."
>
> > + *
> > + * V4L2Controls instances can be reset to remove all controls it contains and
>
> s/it contains/they contain/
>
> > + * prepare to be re-used for a new control write/read sequence. Alternatively,
> > + * the value of a control already part of the instance could be updated by using
> > + * the V4L2Controls::set() method, to avoid going through a reset() when a
> > + * control has to be read then written to the hardware in sequence.
>
> Given how set() is implemented, creating a new V4L2Controls instance
> could be cheaper. Do we really want to reuse V4L2Controls instances with
> set() ? If so I think we need to think about the use cases, and decide
> how to implement it in an efficient way. Alternatively, I would start
> simple and extend the API later.
>

So how would you re-use the same V4L2Controls instance for a
getControls()/setControls() sequence? Go through a reset and re-add
all controls?

> > + *
> > + * In facts, the following pseudo-code sequences lead to the same result:
>
> s/In facts, the/The/
>
> > + *
> > + * V4L2Controls controls;
> > + * controls.add(V4L2_CID_xx);
> > + * dev->getControls(&controls);
> > + * controls.set(V4L2_CID_xx, value);
> > + * dev->setControls(&controls);
> > + *
> > + * V4L2Controls controls;
> > + * controls.add(V4L2_CID_xx);
> > + * dev->getControls(&controls);
> > + * controls.reset();
> > + * controls.add(V4L2_CID_xx, value);
> > + * dev->setControls(&controls);
> > + */
> > +
> > +/**
> > + * \typedef V4L2Controls::iterator
> > + * \brief Iterator on the V4L2 controls contained in the instance
> > + */
> > +
> > +/**
> > + * \typedef V4L2Controls::const_iterator
> > + * \brief Const iterator on the  V4L2 controls contained in the instance
> > + */
> > +
> > +/**
> > + * \fn iterator V4L2Controls::begin()
> > + * \brief Retrieve an iterator to the first V4L2Control in the instance
> > + * \return An iterator to the first V4L2 control
> > + */
> > +
> > +/**
> > + * \fn const_iterator V4L2Controls::begin() const
> > + * \brief Retrieve a constant iterator to the first V4L2Control in the instance
> > + * \return A constant iterator to the first V4L2 control
> > + */
> > +
> > +/**
> > + * \fn iterator V4L2Controls::end()
> > + * \brief Retrieve an iterator pointing to the past-the-end V4L2Control in the
> > + * instance
> > + * \return An iterator to the element following the last V4L2 control in the
> > + * instance
> > + */
> > +
> > +/**
> > + * \fn const_iterator V4L2Controls::end() const
> > + * \brief Retrieve a constant iterator pointing to the past-the-end V4L2Control
> > + * in the instance
> > + * \return A constant iterator to the element following the last V4L2 control
> > + * in the instance
> > + */
> > +
> > +/**
> > + * \fn V4L2Controls::empty()
> > + * \brief Verify if the instance does not contain any control
> > + * \return True if the instance does not contain any control, false otherwise
> > + */
> > +
> > +/**
> > + * \fn V4L2Controls::size()
> > + * \brief Retrieve the number on controls in the instance
> > + * \return The number of V4L2Control stored in the instance
> > + */
> > +
> > +/**
> > + * \fn V4L2Controls::reset()
> > + * \brief Removes all controls in the instance
> > + */
> > +
> > +/**
> > + * \fn V4L2Controls::operator[](unsigned int index)
> > + * \brief Access the control at index \a index
> > + * \param index The index to access
>
> \return ?
>
> > + */
> > +
> > +/**
> > + * \brief Add control \a id with a \a value to the instance
> > + * \param id The V4L2 control id (V4L2_CID_xx)
>
> s/control id/control ID/
> s/V4L2_CID_xx/V4L2_CID_*/
>
> > + * \param value The V4L2 control value
> > + */
> > +void V4L2Controls::add(unsigned int id, long int value)
> > +{
> > +	/* Cannot use emplace_back() as the V4L2Control ctor is private. */
> > +	V4L2Control ctrl(id, value);
> > +	controls_.push_back(ctrl);
> > +}
> > +
> > +/**
> > + * \brief Add control \a id without an initial value  to the instance
>
> s/  / /
>
> > + * \param id The V4L2 control id (V4L2_CID_xx)
>
> s/control id/control ID/
> s/V4L2_CID_xx/V4L2_CID_*/
>
> Same in a few locations below.
>

Thanks for all the comments in documentation, I'll take them in.

> > + */
> > +void V4L2Controls::add(unsigned int id)
> > +{
> > +	/* Cannot use emplace_back() as the V4L2Control ctor is private. */
>
> Would it make sense to make the constructor public then ? You have
> documented extensively that V4L2Control isn't supposed to be
> instantiated directly by the application, and there will be no way to
> use a directly instantiated V4L2Control anway, so things should be fine.
>

If the only price to pay is not to use emplace_back() I don't think
so.

Shouldn't the following code be equally efficient than emplace_back()?
Isn't the new instance constructed inside the container directly
instead of copied? (emplace_back would just be a syntactic sugar in
that case).

> > +	V4L2Control ctrl(id);
> > +	controls_.push_back(ctrl);
> > +}
> > +
> > +/**
> > + * \brief Retrieve the value of the control with \a id
> > + * \param id The V4L2 control id (V4L2_CID_xx)
> > + *
> > + * Retrieve the value of the control with id \a id.
> > + *
> > + * It is important to notice that the returned value is not read from the
> > + * hardware at the time this operation is called, but it's the cached value
> > + * obtained the most recent time the control has been read with
> > + * V4L2Device::getControls() or the value of the control has been set to by the
> > + * user with the add() or set() operation.
> > + *
> > + * \return The V4L2 control value or a negative error code if the control id
> > + * is not part of this instance
> > + */
> > +int V4L2Controls::get(unsigned int id)
> > +{
> > +	V4L2Control *ctrl = getControl(id);
> > +	if (!ctrl)
> > +		return -EINVAL;
> > +
> > +	return ctrl->value();
> > +}
> > +
> > +/**
> > + * \brief Set the value of the control with \a id
> > + * \param id The V4L2 control id (V4L2_CID_xx)
> > + * \param value The new V4L2 control value
> > + *
> > + * Set the value of the control with id \a id.
> > + *
> > + * It is important to notice that the value is not applied to the
> > + * hardware at the time this operation is called, but it will only
> > + * be actually pplied only by calling V4L2Device::setControls().
> > + *
> > + * \return 0 on success or a negative error code if the control id
> > + * is not part of this instance
> > + */
> > +int V4L2Controls::set(unsigned int id, long int value)
> > +{
> > +	V4L2Control *ctrl = getControl(id);
> > +	if (!ctrl)
> > +		return -EINVAL;
> > +
> > +	ctrl->setValue(value);
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * \brief Get a pointer the control with \a id
> > + * \param id The V4L2 control id (V4L2_CID_xx)
> > + * \return A pointer to the V4L2Control with id \a id or nullptr  if the
> > + * control id is not part of this instance.
> > + */
> > +V4L2Control *V4L2Controls::getControl(unsigned int id)
> > +{
> > +	for (V4L2Control &ctrl : controls_) {
> > +		if (ctrl.id() == id)
> > +			return &ctrl;
> > +	}
> > +
> > +	return nullptr;
> > +}
> > +
> > +} /* 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/20190620/f92db750/attachment-0001.sig>


More information about the libcamera-devel mailing list