[libcamera-devel] [PATCH 4/6] libcamera: v4l2_base: Add V4L2 control support
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jun 3 14:32:58 CEST 2019
Hi Jacopo,
Some thoughts out loud inline....
On 02/06/2019 14:04, Jacopo Mondi wrote:
> Implement operations to set and get V4L2 controls in the V4L2Base class.
> The operations allow to set and get a single or a list of controls.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/libcamera/include/v4l2_base.h | 12 ++
> src/libcamera/v4l2_base.cpp | 290 ++++++++++++++++++++++++++++++
> 2 files changed, 302 insertions(+)
>
> diff --git a/src/libcamera/include/v4l2_base.h b/src/libcamera/include/v4l2_base.h
> index 2fda81a960d2..7d4e238dadfa 100644
> --- a/src/libcamera/include/v4l2_base.h
> +++ b/src/libcamera/include/v4l2_base.h
> @@ -9,6 +9,8 @@
>
> #include <vector>
>
> +#include <v4l2_controls.h>
> +
> namespace libcamera {
>
> class V4L2Base
> @@ -22,8 +24,18 @@ public:
> virtual void close() = 0;
> bool isOpen() const;
>
> + std::vector<V4L2Control *> getControls(std::vector<unsigned int> &ctrl_ids);
In my controls, I pass a list <well a map, or a set; I have two
implementations currently> of the actual ControlValue objects, which get
filled in.
I wonder if there will be much differences between the two interface
designs.
> + V4L2Control *getControl(unsigned int ctrl_id);
> + int setControls(std::vector<V4L2Control *> &ctrls);
> + int setControl(V4L2Control *ctrl);
I think I'd group those as getControls setControls, and then getControl,
setControl, so that they are grouped by 'functionality' but that might
not be a preferred sorting... so either way.
> +
> protected:
> int fd_;
> +
> +private:
> + int queryControl(unsigned int ctrl_id,
> + struct v4l2_query_ext_ctrl *qext_ctrl);
> +
> };
>
> }; /* namespace libcamera */
> diff --git a/src/libcamera/v4l2_base.cpp b/src/libcamera/v4l2_base.cpp
> index 7d05a3c82e4d..423a4d84e276 100644
> --- a/src/libcamera/v4l2_base.cpp
> +++ b/src/libcamera/v4l2_base.cpp
> @@ -5,7 +5,13 @@
> * v4l2_base.cpp - Common base for V4L2 devices and subdevices
> */
>
> +#include <linux/v4l2-subdev.h>
> +#include <linux/videodev2.h>
> +#include <sys/ioctl.h>
> +
> +#include "log.h"
> #include "v4l2_base.h"
> +#include "v4l2_controls.h"
>
> /**
> * \file v4l2_base.h
> @@ -14,6 +20,8 @@
>
> namespace libcamera {
>
> +LOG_DEFINE_CATEGORY(V4L2Base)
> +
> /**
> * \class V4L2Base
> * \brief Base class for V4L2Device and V4L2Subdevice
> @@ -31,6 +39,7 @@ namespace libcamera {
> */
>
> /**
> + * \fn V4L2Base::open()
> * \brief Open a V4L2 device or subdevice
> *
> * Pure virtual method to be implemented by the derived classes.
> @@ -39,6 +48,7 @@ namespace libcamera {
> */
>
> /**
> + * \fn V4L2Base::close()
> * \brief Close the device or subdevice
> *
> * Pure virtual method to be implemented by the derived classes.
> @@ -53,6 +63,286 @@ bool V4L2Base::isOpen() const
> return fd_ != -1;
> }
>
> +int V4L2Base::queryControl(unsigned int id,
> + struct v4l2_query_ext_ctrl *qext_ctrl)
> +{
> + qext_ctrl->id = id;
> +
> + int ret = ioctl(fd_, VIDIOC_QUERY_EXT_CTRL, qext_ctrl);
> + if (ret) {
> + ret = -errno;
> + LOG(V4L2Base, Error)
> + << "Unable to query extended control 0x"
> + << std::hex << qext_ctrl->id
> + << ": " << strerror(-ret);
> + return ret;
> + }
> +
> + if (qext_ctrl->flags & V4L2_CTRL_FLAG_DISABLED) {
> + LOG(V4L2Base, Error)
> + << "Extended control 0x" << std::hex
> + << qext_ctrl->id << " is disabled";
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * \brief Get values of a list of control IDs
> + * \param ctrl_ids The list of control IDs to retrieve value of
> + * \return A list of V4L2Control on success or a empty list otherwise
> + */
> +std::vector<V4L2Control *> V4L2Base::getControls(std::vector<unsigned int> &ctrl_ids)
> +{
> + unsigned int count = ctrl_ids.size();
> + if (!count)
> + return std::vector<V4L2Control *> {};
> +
> + struct v4l2_ext_control *v4l2_ctrls = static_cast<struct v4l2_ext_control *>
> + (new struct v4l2_ext_control[count]);
> + if (!v4l2_ctrls)
> + return std::vector<V4L2Control *> {};
> +
> + /*
> + * Query each control in the vector to get back its type and expected
> + * size in order to get its current value.
> + */
> + for (unsigned int i = 0; i < count; ++i) {
> + struct v4l2_ext_control *v4l2_ctrl = &v4l2_ctrls[i];
> + unsigned int ctrl_id = ctrl_ids[i];
> + struct v4l2_query_ext_ctrl qext_ctrl = {};
> +
> + int ret = queryControl(ctrl_id, &qext_ctrl);
> + if (ret)
> + return std::vector<V4L2Control *> {};
> +
> + v4l2_ctrl->id = ctrl_id;
> + v4l2_ctrl->size = qext_ctrl.elem_size * qext_ctrl.elems;
> +
> + /* Reserve memory for controls with payload. */
> + if (!(qext_ctrl.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD))
> + continue;
> +
> + switch (qext_ctrl.type) {
> + case V4L2_CTRL_TYPE_U8:
> + v4l2_ctrl->p_u8 = static_cast<uint8_t *>
> + (new uint8_t[v4l2_ctrl->size]);
> + break;
> + case V4L2_CTRL_TYPE_U16:
> + v4l2_ctrl->p_u16 = static_cast<uint16_t *>
> + (new uint16_t[v4l2_ctrl->size]);
> + break;
> + case V4L2_CTRL_TYPE_U32:
> + v4l2_ctrl->p_u32 = static_cast<uint32_t *>
> + (new uint32_t[v4l2_ctrl->size]);
> + break;
> + }
> + }
> +
> + struct v4l2_ext_controls v4l2_ext_ctrls = {};
> + v4l2_ext_ctrls.which = V4L2_CTRL_WHICH_CUR_VAL;
> + v4l2_ext_ctrls.count = count;
> + v4l2_ext_ctrls.controls = v4l2_ctrls;
> +
> + int ret = ioctl(fd_, VIDIOC_G_EXT_CTRLS, &v4l2_ext_ctrls);
> + if (ret) {
> + ret = -errno;
> + LOG(V4L2Base, Error)
> + << "Unable to get extended controls: " << strerror(-ret);
> + return std::vector<V4L2Control *> {};
> + }
> +
> + /*
> + * For each requested control, create a V4L2Control which contains
> + * the current value and store it in the vector returned to the caller.
> + */
> + std::vector<V4L2Control *> controls = {};
> + for (unsigned int i = 0; i < count; ++i) {
> + struct v4l2_ext_control *v4l2_ctrl = &v4l2_ctrls[i];
> + struct v4l2_query_ext_ctrl qext_ctrl = {};
> +
> + int ret = queryControl(v4l2_ctrl->id, &qext_ctrl);
> + if (ret)
> + return std::vector<V4L2Control *> {};
> +
> + unsigned int size = qext_ctrl.elem_size * qext_ctrl.elem_size;
> + if (qext_ctrl.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) {
> + /* Control types with payload */
> + switch (qext_ctrl.type) {
> + case V4L2_CTRL_TYPE_U8:
> + {
> + V4L2U8Control *c =
> + new V4L2U8Control(v4l2_ctrl->id, size,
> + v4l2_ctrl->p_u8);
> + controls.push_back(c);
> + }
> + break;
> + case V4L2_CTRL_TYPE_U16:
> + {
> + V4L2U16Control *c =
> + new V4L2U16Control(v4l2_ctrl->id, size,
> + v4l2_ctrl->p_u16);
> + controls.push_back(c);
> + }
> + break;
> + case V4L2_CTRL_TYPE_U32:
> + {
> + V4L2U32Control *c =
> + new V4L2U32Control(v4l2_ctrl->id, size,
> + v4l2_ctrl->p_u32);
> + controls.push_back(c);
> + }
> + break;
> + default:
> + LOG(V4L2Base, Error)
> + << "Compound control types not supported: "
> + << std::hex << qext_ctrl.type;
> + return std::vector<V4L2Control *> {};
> + }
> + } else {
> + /* Control types without payload. */
> + switch (qext_ctrl.type) {
> + case V4L2_CTRL_TYPE_INTEGER64:
> + {
> + V4L2Int64Control *c =
> + new V4L2Int64Control(v4l2_ctrl->id,
> + v4l2_ctrl->value64);
> + controls.push_back(c);
I think this could be a
controls.emplace(V4L2Int64Control(v4l2_ctrl->id,
v4l2_ctrl->value64);
if you prefer that style ...
Aha - except you have a vector of pointers, rather than a vector of
objects ...
> + }
> + break;
> + case V4L2_CTRL_TYPE_INTEGER:
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + case V4L2_CTRL_TYPE_MENU:
> + case V4L2_CTRL_TYPE_BUTTON:
> + {
> + V4L2IntControl *c =
> + new V4L2IntControl(v4l2_ctrl->id,
> + v4l2_ctrl->value);
> + controls.push_back(c);
> + }
> + break;
> + default:
> + LOG(V4L2Base, Error)
> + << "Invalid non-compound control type :"
> + << std::hex << qext_ctrl.type;
> + return std::vector<V4L2Control *> {};
> + }
> + }
> + }
> +
> + return controls;
> +}
> +
> +/**
> + * \brief Get the value of control with id \a ctrl_id
> + * \param ctrl_id The control id
> + * \return The V4L2Control which represent the value of the control on success
> + * nullptr otherwise
> + */
> +V4L2Control *V4L2Base::getControl(unsigned int ctrl_id)
> +{
> + std::vector<unsigned int> v = { ctrl_id, };
> + std::vector<V4L2Control *> ctrls = getControls(v);
> +
> + if (ctrls.empty())
> + return nullptr;
> +
> + return ctrls[0];
> +}
> +
> +/**
> + * \brief Apply a list of controls to the device
> + * \param ctrls The list of controls to apply
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2Base::setControls(std::vector<V4L2Control *> &ctrls)
> +{
> + unsigned int count = ctrls.size();
> + if (!count)
> + return -EINVAL;
> +
> + struct v4l2_ext_control *v4l2_ctrls = static_cast<struct v4l2_ext_control *>
> + (new struct v4l2_ext_control[count]);
> + if (!v4l2_ctrls)
> + return -ENOMEM;
> +
> + /*
> + * Query each control in the vector to get back its type and expected
> + * size and set the new desired value in each v4l2_ext_control member.
> + */
> + for (unsigned int i = 0; i < count; ++i) {
> + struct v4l2_ext_control *v4l2_ctrl = &v4l2_ctrls[i];
> + struct v4l2_query_ext_ctrl qext_ctrl = {};
> + V4L2Control *ctrl = ctrls[i];
> +
> + int ret = queryControl(ctrl->id(), &qext_ctrl);
> + if (ret)
> + return ret;
> +
> + v4l2_ctrl->id = ctrl->id();
> + v4l2_ctrl->size = qext_ctrl.elem_size * qext_ctrl.elems;
> +
> + if (qext_ctrl.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) {
> + /** \todo: Support set controls with payload. */
> + LOG(V4L2Base, Error)
> + << "Controls with payload not supported";
> + return -EINVAL;
> + } else {
> + switch (qext_ctrl.type) {
> + case V4L2_CTRL_TYPE_INTEGER64:
> + {
> + V4L2Int64Control *c =
> + static_cast<V4L2Int64Control *>(ctrl);
It's these static_casts that feel a bit horrible to me. In my
implementation there is just single class, so there is no casting necessary,
> + v4l2_ctrl->value64 = c->value();
However, this would be c->getInteger64();
> + }
> + break;
> + case V4L2_CTRL_TYPE_INTEGER:
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + case V4L2_CTRL_TYPE_MENU:
> + case V4L2_CTRL_TYPE_BUTTON:
> + {
> + V4L2IntControl *c =
> + static_cast<V4L2IntControl *>(ctrl);
> + v4l2_ctrl->value = c->value();
and getInteger(); (although getBoolean() can be separated as well);
As we know the types in this switch, I wonder if there is much
difference int taste between the two approaches...
> + }
> + break;
> + default:
> + LOG(V4L2Base, Error)
> + << "Invalid non-compound control type :"
> + << qext_ctrl.type;
> + return -EINVAL;
> + }
> + }
> + }
> +
> + struct v4l2_ext_controls v4l2_ext_ctrls = {};
> + v4l2_ext_ctrls.which = V4L2_CTRL_WHICH_CUR_VAL;
> + v4l2_ext_ctrls.count = count;
> + v4l2_ext_ctrls.controls = v4l2_ctrls;
> +
> + int ret = ioctl(fd_, VIDIOC_S_EXT_CTRLS, &v4l2_ext_ctrls);
> + if (ret) {
> + ret = -errno;
> + LOG(V4L2Base, Error)
> + << "Unable to set extended controls: " << strerror(-ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * \brief Apply the control \a ctrl to the device
> + * \param ctrl The control to apply to the device
> + * \return 0 on success, a negative error code otherwise
> + */
> +int V4L2Base::setControl(V4L2Control *ctrl)
> +{
> + std::vector<V4L2Control *> v = { ctrl, };
> + return setControls(v);
> +}
> +
> /**
> * \var V4L2Base::fd_
> * \brief The V4L2 device or subdevice device node file descriptor
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list