Message ID | 20200309162414.720306-8-jacopo@jmondi.org |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Jacopo, Thank you for the patch. On Mon, Mar 09, 2020 at 05:24:10PM +0100, Jacopo Mondi wrote: > Add support to write array controls of type V4L2_CTRL_TYPE_U8. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/v4l2_device.cpp | 32 ++++++++++++++++++++++++++++---- > 1 file changed, 28 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 65e97f92b01f..950e6286b84d 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -267,11 +267,25 @@ int V4L2Device::setControls(ControlList *ctrls) > case ControlTypeInteger64: > v4l2Ctrls[i].value64 = value.get<int64_t>(); > break; > + case ControlTypeByte: { > + if (!value.isArray()) > + /* > + * \todo This happens if a V4L2_CTRL_TYPE_U8 > + * control is set with a non-array ControlValue. > + * > + * Should we fail loudly here ? > + */ > + break; Yes we should fail loudly, it's an error. > + > + auto values = value.get<Span<const uint8_t>>(); Span<const uint8_t> data = value.get<Span<const uint8_t>>(); > + v4l2Ctrls[i].p_u8 = const_cast<uint8_t *>(values.data()); The VIDIOC_S_EXT_CTRLS call below will potentially update the value of the control in the memory pointed by p_u8, so the const_cast here is really not nice. > + v4l2Ctrls[i].size = values.size_bytes(); > + > + break; > + } > + > default: > - /* > - * \todo To be changed when support for string and > - * compound controls will be added. > - */ > + /* \todo To be changed to support strings. */ > v4l2Ctrls[i].value = value.get<int32_t>(); > break; > } > @@ -413,6 +427,16 @@ void V4L2Device::updateControls(ControlList *ctrls, > case ControlTypeInteger64: > value.set<int64_t>(v4l2Ctrl->value64); > break; > + > + case ControlTypeByte: { > + std::vector<uint8_t> data = { > + v4l2Ctrl->p_u8, v4l2Ctrl->p_u8 + v4l2Ctrl->size > + }; You're creating a copy of the data, you should instead create the span directly. Span<const uint8_t> data(v4l2Ctrl->p_u8, v4l2Ctrl->size); value.set(data); > + Span<const uint8_t> values(data); > + value.set<Span<const uint8_t>>(values); > + break; But in any case, all this isn't needed as the ioctl hs already updated the value, as explained above. > + } > + > default: > /* > * \todo To be changed when support for string and
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 65e97f92b01f..950e6286b84d 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -267,11 +267,25 @@ int V4L2Device::setControls(ControlList *ctrls) case ControlTypeInteger64: v4l2Ctrls[i].value64 = value.get<int64_t>(); break; + case ControlTypeByte: { + if (!value.isArray()) + /* + * \todo This happens if a V4L2_CTRL_TYPE_U8 + * control is set with a non-array ControlValue. + * + * Should we fail loudly here ? + */ + break; + + auto values = value.get<Span<const uint8_t>>(); + v4l2Ctrls[i].p_u8 = const_cast<uint8_t *>(values.data()); + v4l2Ctrls[i].size = values.size_bytes(); + + break; + } + default: - /* - * \todo To be changed when support for string and - * compound controls will be added. - */ + /* \todo To be changed to support strings. */ v4l2Ctrls[i].value = value.get<int32_t>(); break; } @@ -413,6 +427,16 @@ void V4L2Device::updateControls(ControlList *ctrls, case ControlTypeInteger64: value.set<int64_t>(v4l2Ctrl->value64); break; + + case ControlTypeByte: { + std::vector<uint8_t> data = { + v4l2Ctrl->p_u8, v4l2Ctrl->p_u8 + v4l2Ctrl->size + }; + Span<const uint8_t> values(data); + value.set<Span<const uint8_t>>(values); + break; + } + default: /* * \todo To be changed when support for string and
Add support to write array controls of type V4L2_CTRL_TYPE_U8. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/v4l2_device.cpp | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-)