[libcamera-devel] [PATCH v2 4/6] libcamera: v4l2_base: Add V4L2 control support
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Jun 11 13:41:26 CEST 2019
Hi Jacopo,
Thank you for the patch.
On Mon, Jun 10, 2019 at 06:40:50PM +0200, 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 | 11 ++
> src/libcamera/v4l2_base.cpp | 238 ++++++++++++++++++++++++++++++
> 2 files changed, 249 insertions(+)
>
> diff --git a/src/libcamera/include/v4l2_base.h b/src/libcamera/include/v4l2_base.h
> index 2fda81a960d2..17ea734c8f49 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>
Just forward-declare the required classes and structures.
> +
> namespace libcamera {
>
> class V4L2Base
> @@ -22,8 +24,17 @@ public:
> virtual void close() = 0;
> bool isOpen() const;
>
> + int getControls(std::vector<unsigned int> &ctrl_ids,
> + V4L2Controls *ctrls);
const std::vector<unsigned int> &ctrl_ids,
I still wonder if it wouldn't be simpler for applications to fill the
V4L2Controls with the control IDs to read, and just pass the
V4L2Controls pointer to this function. In any case, this is better than
the first version.
> + int setControls(V4L2Controls &ctrls);
You should pass a const V4L2Controls &ctrls. Or, maybe you should
instead pass a non-const pointer, as setControls() should return the
value set for each control.
> +
> 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..cbd7a551130f 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,234 @@ 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
> + */
> +int V4L2Base::getControls(std::vector<unsigned int> &ctrl_ids,
> + V4L2Controls *controls)
> +{
> + unsigned int count = ctrl_ids.size();
> + if (!count)
> + return -EINVAL;
> +
> + controls->clear();
> +
> + struct v4l2_ext_control *v4l2_ctrls = new struct v4l2_ext_control[count];
You're leaking memory. The simplest solution would be
struct v4l2_ext_controls v4l2_ctrls[count];
> +
> + /*
> + * 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 ret;
> +
> + 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;
This is lots of new's, and they're all leaked. A better solution would
be to populate ctrls with the V4L2Control instances already, and use the
pointer to the internal memory buffers directly.
> + }
> + }
> +
> + 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 ret;
> + }
> +
> + /*
> + * For each requested control, create a V4L2Control which contains
> + * the current value and store it in the vector returned to the caller.
> + */
> + LOG(Error) << __func__;
> + 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 ret;
> +
> + 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:
> + controls->set(v4l2_ctrl->id, size,
> + v4l2_ctrl->p_u8);
> + break;
> + case V4L2_CTRL_TYPE_U16:
> + controls->set(v4l2_ctrl->id, size,
> + v4l2_ctrl->p_u16);
> + break;
> + case V4L2_CTRL_TYPE_U32:
> + controls->set(v4l2_ctrl->id, size,
> + v4l2_ctrl->p_u32);
> + break;
> + default:
> + LOG(V4L2Base, Error)
> + << "Compound control types not supported: "
> + << std::hex << qext_ctrl.type;
> + return -EINVAL;
> + }
> + } else {
> + /* Control types without payload. */
> + switch (qext_ctrl.type) {
> + case V4L2_CTRL_TYPE_INTEGER64:
> + controls->set(v4l2_ctrl->id, static_cast<int64_t>
> + (v4l2_ctrl->value64));
> + break;
> + case V4L2_CTRL_TYPE_INTEGER:
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + case V4L2_CTRL_TYPE_MENU:
> + case V4L2_CTRL_TYPE_BUTTON:
> + controls->set(v4l2_ctrl->id, v4l2_ctrl->value);
> + break;
> + default:
> + LOG(V4L2Base, Error)
> + << "Invalid non-compound control type :"
> + << std::hex << qext_ctrl.type;
> + return -EINVAL;
> + }
> + }
> + }
> +
> + return 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(V4L2Controls &ctrls)
> +{
> + unsigned int count = ctrls.size();
> + if (!count)
> + return -EINVAL;
> +
> + struct v4l2_ext_control *v4l2_ctrls = new struct v4l2_ext_control[count];
> +
> + /*
> + * 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);
> + v4l2_ctrl->value64 = c->value();
> + }
> + 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();
> + }
> + 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 controls: " << strerror(-ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> /**
> * \var V4L2Base::fd_
> * \brief The V4L2 device or subdevice device node file descriptor
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list