[libcamera-devel] [PATCH 08/12] libcamera: v4l2_controls: Use the ControlValue class for data storage
Jacopo Mondi
jacopo at jmondi.org
Sun Sep 29 12:10:25 CEST 2019
Hi Laurent,
On Sat, Sep 28, 2019 at 06:22:34PM +0300, Laurent Pinchart wrote:
> Use the ControlValue class to replace the manually crafted data storage
> in V4L2Control. This will help sharing code when more data types will be
> supported.
>
Happy indeed to see that at third attempt we might be getting there
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/libcamera/include/v4l2_controls.h | 15 +++++++++------
> src/libcamera/pipeline/uvcvideo.cpp | 2 +-
> src/libcamera/pipeline/vimc.cpp | 2 +-
> src/libcamera/v4l2_controls.cpp | 19 ++++++++++---------
> src/libcamera/v4l2_device.cpp | 8 ++++----
> 5 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h
> index 10b726504951..f2b67c5d44e1 100644
> --- a/src/libcamera/include/v4l2_controls.h
> +++ b/src/libcamera/include/v4l2_controls.h
> @@ -16,6 +16,8 @@
> #include <linux/v4l2-controls.h>
> #include <linux/videodev2.h>
>
> +#include <libcamera/controls.h>
> +
> namespace libcamera {
>
> class V4L2ControlInfo
> @@ -46,17 +48,18 @@ using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;
> class V4L2Control
> {
> public:
> - V4L2Control(unsigned int id, int value = 0)
> - : id_(id), value_(value) {}
> -
> - int64_t value() const { return value_; }
> - void setValue(int64_t value) { value_ = value; }
> + V4L2Control(unsigned int id, const ControlValue &value = ControlValue())
> + : id_(id), value_(value)
The default copy constructor is fine so far, as it copies the union
content. With compound controls this will fall short I fear.
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> + {
> + }
>
> unsigned int id() const { return id_; }
> + const ControlValue &value() const { return value_; }
> + ControlValue &value() { return value_; }
>
> private:
> unsigned int id_;
> - int64_t value_;
> + ControlValue value_;
> };
>
> class V4L2ControlList
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 3635ea83a7b1..a80f8a43a116 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -252,7 +252,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
> LOG(UVC, Debug)
> << "Setting control 0x"
> << std::hex << std::setw(8) << ctrl.id() << std::dec
> - << " to " << ctrl.value();
> + << " to " << ctrl.value().toString();
>
> int ret = data->video_->setControls(&controls);
> if (ret) {
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index d9dd031e173c..5d6909f58cf9 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -300,7 +300,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
> LOG(VIMC, Debug)
> << "Setting control 0x"
> << std::hex << std::setw(8) << ctrl.id() << std::dec
> - << " to " << ctrl.value();
> + << " to " << ctrl.value().toString();
>
> int ret = data->sensor_->setControls(&controls);
> if (ret) {
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index 84258d9954d0..64f0555fff7c 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -143,6 +143,16 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> * \param value The control value
> */
>
> +/**
> + * \fn V4L2Control::value() const
> + * \brief Retrieve the value of the control
> + *
> + * This method is a const version of V4L2Control::value(), returning a const
> + * reference to the value.
> + *
> + * \return The V4L2 control value
> + */
> +
> /**
> * \fn V4L2Control::value()
> * \brief Retrieve the value of the control
> @@ -154,15 +164,6 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> * \return The V4L2 control value
> */
>
> -/**
> - * \fn V4L2Control::setValue()
> - * \brief Set the value of the control
> - * \param value The new V4L2 control value
> - *
> - * This method stores the control value, which will be applied to the
> - * device when calling V4L2Device::setControls().
> - */
> -
> /**
> * \fn V4L2Control::id()
> * \brief Retrieve the control ID this instance refers to
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 349bf2d29704..fd4b9c6d5c62 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -264,14 +264,14 @@ int V4L2Device::setControls(V4L2ControlList *ctrls)
> /* Set the v4l2_ext_control value for the write operation. */
> switch (info->type()) {
> case V4L2_CTRL_TYPE_INTEGER64:
> - v4l2Ctrls[i].value64 = ctrl->value();
> + v4l2Ctrls[i].value64 = ctrl->value().get<int64_t>();
> break;
> default:
> /*
> * \todo To be changed when support for string and
> * compound controls will be added.
> */
> - v4l2Ctrls[i].value = ctrl->value();
> + v4l2Ctrls[i].value = ctrl->value().get<int32_t>();
> break;
> }
> }
> @@ -393,14 +393,14 @@ void V4L2Device::updateControls(V4L2ControlList *ctrls,
>
> switch (info->type()) {
> case V4L2_CTRL_TYPE_INTEGER64:
> - ctrl->setValue(v4l2Ctrl->value64);
> + ctrl->value().set<int64_t>(v4l2Ctrl->value64);
> break;
> default:
> /*
> * \todo To be changed when support for string and
> * compound controls will be added.
> */
> - ctrl->setValue(v4l2Ctrl->value);
> + ctrl->value().set<int32_t>(v4l2Ctrl->value);
> break;
> }
> }
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- 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/20190929/cefbae42/attachment.sig>
More information about the libcamera-devel
mailing list