| Message ID | 20200309162414.720306-9-jacopo@jmondi.org |
|---|---|
| State | Superseded, archived |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Mon, Mar 09, 2020 at 05:24:11PM +0100, Jacopo Mondi wrote: > Add support to retrieve the value of array controls of type > V4L2_CTRL_TYPE_U8. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/v4l2_device.cpp | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 950e6286b84d..464df6a1fe18 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -185,6 +185,22 @@ int V4L2Device::getControls(ControlList *ctrls) > return -EINVAL; > } > > + const struct v4l2_query_ext_ctrl &info = ctrlsInfo_[id]; > + if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) { > + if (info.type != V4L2_CTRL_TYPE_U8) { > + /* > + * \todo Fail loudly to notify support for > + * other array control types is not implemented. > + */ > + LOG(V4L2, Error) << "Unsupported type"; > + return -EINVAL; > + } > + > + size_t sizeBytes = info.elems * info.elem_size; > + v4l2Ctrls[i].p_u8 = new uint8_t[sizeBytes]; > + v4l2Ctrls[i].size = sizeBytes; Having to allocate extra memory here isn't very nice :-S I think that we should try to instead write directly in the ControlValue data, especially given that the updateControls() function should lose handling of the V4L2_CTRL_TYPE_U8 type as explained in the review of patch 07/11. I have a patch to implement this in ControlValue, I'll send it. > + } > + > v4l2Ctrls[i].id = id; > i++; > } > @@ -214,6 +230,16 @@ int V4L2Device::getControls(ControlList *ctrls) > > updateControls(ctrls, v4l2Ctrls, count); > > + /* Release memory reserved for array controls. */ > + for (unsigned int i = 0; i < count; i++) { > + unsigned int id = v4l2Ctrls[i].id; > + const struct v4l2_query_ext_ctrl &info = ctrlsInfo_[id]; > + > + /* \todo Adjust to support more array control types. */ > + if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) > + delete[] v4l2Ctrls[i].p_u8; > + } > + > return ret; > } >
And s/libcamera/libcamera/ in the subject line. On Fri, Mar 20, 2020 at 11:30:27PM +0200, Laurent Pinchart wrote: > On Mon, Mar 09, 2020 at 05:24:11PM +0100, Jacopo Mondi wrote: > > Add support to retrieve the value of array controls of type > > V4L2_CTRL_TYPE_U8. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/v4l2_device.cpp | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 950e6286b84d..464df6a1fe18 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -185,6 +185,22 @@ int V4L2Device::getControls(ControlList *ctrls) > > return -EINVAL; > > } > > > > + const struct v4l2_query_ext_ctrl &info = ctrlsInfo_[id]; > > + if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) { > > + if (info.type != V4L2_CTRL_TYPE_U8) { > > + /* > > + * \todo Fail loudly to notify support for > > + * other array control types is not implemented. > > + */ > > + LOG(V4L2, Error) << "Unsupported type"; > > + return -EINVAL; > > + } > > + > > + size_t sizeBytes = info.elems * info.elem_size; > > + v4l2Ctrls[i].p_u8 = new uint8_t[sizeBytes]; > > + v4l2Ctrls[i].size = sizeBytes; > > Having to allocate extra memory here isn't very nice :-S I think that we > should try to instead write directly in the ControlValue data, > especially given that the updateControls() function should lose handling > of the V4L2_CTRL_TYPE_U8 type as explained in the review of patch 07/11. > I have a patch to implement this in ControlValue, I'll send it. > > > + } > > + > > v4l2Ctrls[i].id = id; > > i++; > > } > > @@ -214,6 +230,16 @@ int V4L2Device::getControls(ControlList *ctrls) > > > > updateControls(ctrls, v4l2Ctrls, count); > > > > + /* Release memory reserved for array controls. */ > > + for (unsigned int i = 0; i < count; i++) { > > + unsigned int id = v4l2Ctrls[i].id; > > + const struct v4l2_query_ext_ctrl &info = ctrlsInfo_[id]; > > + > > + /* \todo Adjust to support more array control types. */ > > + if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) > > + delete[] v4l2Ctrls[i].p_u8; > > + } > > + > > return ret; > > } > >
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 950e6286b84d..464df6a1fe18 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -185,6 +185,22 @@ int V4L2Device::getControls(ControlList *ctrls) return -EINVAL; } + const struct v4l2_query_ext_ctrl &info = ctrlsInfo_[id]; + if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) { + if (info.type != V4L2_CTRL_TYPE_U8) { + /* + * \todo Fail loudly to notify support for + * other array control types is not implemented. + */ + LOG(V4L2, Error) << "Unsupported type"; + return -EINVAL; + } + + size_t sizeBytes = info.elems * info.elem_size; + v4l2Ctrls[i].p_u8 = new uint8_t[sizeBytes]; + v4l2Ctrls[i].size = sizeBytes; + } + v4l2Ctrls[i].id = id; i++; } @@ -214,6 +230,16 @@ int V4L2Device::getControls(ControlList *ctrls) updateControls(ctrls, v4l2Ctrls, count); + /* Release memory reserved for array controls. */ + for (unsigned int i = 0; i < count; i++) { + unsigned int id = v4l2Ctrls[i].id; + const struct v4l2_query_ext_ctrl &info = ctrlsInfo_[id]; + + /* \todo Adjust to support more array control types. */ + if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) + delete[] v4l2Ctrls[i].p_u8; + } + return ret; }
Add support to retrieve the value of array controls of type V4L2_CTRL_TYPE_U8. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/v4l2_device.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)