| Message ID | 20191013233314.4434-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your work. On 2019-10-14 02:33:14 +0300, Laurent Pinchart wrote: > The V4L2ControlList class only provides a convenience constructor for > the ControlList, which can easily be moved to the ControlList class and > may benefit it later (to construct a ControlList from controls supported > by a camera). Move the constructor and remove V4L2ControlList. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/controls.h | 1 + > src/ipa/rkisp1/rkisp1.cpp | 2 +- > src/libcamera/controls.cpp | 10 ++++++++++ > src/libcamera/include/v4l2_controls.h | 14 -------------- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > src/libcamera/pipeline/uvcvideo.cpp | 2 +- > src/libcamera/pipeline/vimc.cpp | 2 +- > src/libcamera/v4l2_controls.cpp | 20 -------------------- > test/v4l2_videodevice/controls.cpp | 2 +- > 9 files changed, 16 insertions(+), 39 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 80414c6f0748..0d279d508dcc 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -170,6 +170,7 @@ private: > > public: > ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); > + ControlList(const ControlInfoMap &info, ControlValidator *validator = nullptr); > > using iterator = ControlListMap::iterator; > using const_iterator = ControlListMap::const_iterator; > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index bd703898c523..570145ce71b2 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -212,7 +212,7 @@ void IPARkISP1::setControls(unsigned int frame) > IPAOperationData op; > op.operation = RKISP1_IPA_ACTION_V4L2_SET; > > - V4L2ControlList ctrls(ctrls_); > + ControlList ctrls(ctrls_); > ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_)); > ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_)); > op.controls.push_back(ctrls); > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index bf7634aab470..93ad2fc6a276 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -561,6 +561,16 @@ ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator) > { > } > > +/** > + * \brief Construct a ControlList with the idmap of a control info map > + * \param[in] info The ControlInfoMap for the control list target object > + * \param[in] validator The validator (may be null) > + */ > +ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator) > + : validator_(validator), idmap_(&info.idmap()) > +{ > +} > + > /** > * \typedef ControlList::iterator > * \brief Iterator for the controls contained within the list > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h > index e16c4957f9d1..882546a89340 100644 > --- a/src/libcamera/include/v4l2_controls.h > +++ b/src/libcamera/include/v4l2_controls.h > @@ -8,11 +8,6 @@ > #ifndef __LIBCAMERA_V4L2_CONTROLS_H__ > #define __LIBCAMERA_V4L2_CONTROLS_H__ > > -#include <map> > -#include <stdint.h> > -#include <string> > -#include <vector> > - > #include <linux/videodev2.h> > > #include <libcamera/controls.h> > @@ -31,15 +26,6 @@ public: > V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl); > }; > > -class V4L2ControlList : public ControlList > -{ > -public: > - V4L2ControlList(const ControlInfoMap &info) > - : ControlList(info.idmap()) > - { > - } > -}; > - > } /* namespace libcamera */ > > #endif /* __LIBCAMERA_V4L2_CONTROLS_H__ */ > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 9776b36b88cd..8d3ad568d16e 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -604,7 +604,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > return ret; > > /* Apply the "pipe_mode" control to the ImgU subdevice. */ > - V4L2ControlList ctrls(imgu->imgu_->controls()); > + ControlList ctrls(imgu->imgu_->controls()); > ctrls.set(V4L2_CID_IPU3_PIPE_MODE, > static_cast<int32_t>(vfStream->active_ ? IPU3PipeModeVideo : > IPU3PipeModeStillCapture)); > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index a64e1af4c550..fae0ffc4de30 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -228,7 +228,7 @@ void PipelineHandlerUVC::stop(Camera *camera) > > int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > { > - V4L2ControlList controls(data->video_->controls()); > + ControlList controls(data->video_->controls()); > > for (auto it : request->controls()) { > const ControlId &id = *it.first; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 6a4244119dc5..dcdaef120ad1 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -279,7 +279,7 @@ void PipelineHandlerVimc::stop(Camera *camera) > > int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > { > - V4L2ControlList controls(data->sensor_->controls()); > + ControlList controls(data->sensor_->controls()); > > for (auto it : request->controls()) { > const ControlId &id = *it.first; > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp > index bd5e18590b76..83ac7b0dc666 100644 > --- a/src/libcamera/v4l2_controls.cpp > +++ b/src/libcamera/v4l2_controls.cpp > @@ -134,24 +134,4 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl) > static_cast<int32_t>(ctrl.maximum))); > } > > -/** > - * \class V4L2ControlList > - * \brief A list of controls for a V4L2 device > - * > - * This class specialises the ControList class for use with V4L2 devices. It > - * offers a convenience API to create a ControlList from a ControlInfoMap. > - * > - * V4L2ControlList allows easy construction of a ControlList containing V4L2 > - * controls for a device. It can be used to construct the list of controls > - * passed to the V4L2Device::getControls() and V4L2Device::setControls() > - * methods. The class should however not be used in place of ControlList in > - * APIs. > - */ > - > -/** > - * \fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info) > - * \brief Construct a V4L2ControlList associated with a V4L2 device > - * \param[in] info The V4L2 device control info map > - */ > - > } /* namespace libcamera */ > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp > index 31879b794ed0..975c852b8cbf 100644 > --- a/test/v4l2_videodevice/controls.cpp > +++ b/test/v4l2_videodevice/controls.cpp > @@ -46,7 +46,7 @@ protected: > const ControlRange &saturation = info.find(V4L2_CID_SATURATION)->second; > > /* Test getting controls. */ > - V4L2ControlList ctrls(info); > + ControlList ctrls(info); > ctrls.set(V4L2_CID_BRIGHTNESS, -1); > ctrls.set(V4L2_CID_CONTRAST, -1); > ctrls.set(V4L2_CID_SATURATION, -1); > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Laurent, On Mon, Oct 14, 2019 at 02:33:14AM +0300, Laurent Pinchart wrote: > The V4L2ControlList class only provides a convenience constructor for > the ControlList, which can easily be moved to the ControlList class and > may benefit it later (to construct a ControlList from controls supported > by a camera). Move the constructor and remove V4L2ControlList. Ah scratch my previous comment then :) I think we should make this the only available constructor > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/controls.h | 1 + > src/ipa/rkisp1/rkisp1.cpp | 2 +- > src/libcamera/controls.cpp | 10 ++++++++++ > src/libcamera/include/v4l2_controls.h | 14 -------------- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > src/libcamera/pipeline/uvcvideo.cpp | 2 +- > src/libcamera/pipeline/vimc.cpp | 2 +- > src/libcamera/v4l2_controls.cpp | 20 -------------------- > test/v4l2_videodevice/controls.cpp | 2 +- > 9 files changed, 16 insertions(+), 39 deletions(-) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 80414c6f0748..0d279d508dcc 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -170,6 +170,7 @@ private: > > public: > ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); > + ControlList(const ControlInfoMap &info, ControlValidator *validator = nullptr); > > using iterator = ControlListMap::iterator; > using const_iterator = ControlListMap::const_iterator; > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index bd703898c523..570145ce71b2 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -212,7 +212,7 @@ void IPARkISP1::setControls(unsigned int frame) > IPAOperationData op; > op.operation = RKISP1_IPA_ACTION_V4L2_SET; > > - V4L2ControlList ctrls(ctrls_); > + ControlList ctrls(ctrls_); > ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_)); > ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_)); > op.controls.push_back(ctrls); > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index bf7634aab470..93ad2fc6a276 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -561,6 +561,16 @@ ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator) > { > } > > +/** > + * \brief Construct a ControlList with the idmap of a control info map > + * \param[in] info The ControlInfoMap for the control list target object > + * \param[in] validator The validator (may be null) > + */ > +ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator) > + : validator_(validator), idmap_(&info.idmap()) Maybe this is a bit to much to ask, but the different order of parameters and field declaration bothers me a bit. Anyway, this as the whole series look good and I hope we could merge it soon Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > +{ > +} > + > /** > * \typedef ControlList::iterator > * \brief Iterator for the controls contained within the list > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h > index e16c4957f9d1..882546a89340 100644 > --- a/src/libcamera/include/v4l2_controls.h > +++ b/src/libcamera/include/v4l2_controls.h > @@ -8,11 +8,6 @@ > #ifndef __LIBCAMERA_V4L2_CONTROLS_H__ > #define __LIBCAMERA_V4L2_CONTROLS_H__ > > -#include <map> > -#include <stdint.h> > -#include <string> > -#include <vector> > - > #include <linux/videodev2.h> > > #include <libcamera/controls.h> > @@ -31,15 +26,6 @@ public: > V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl); > }; > > -class V4L2ControlList : public ControlList > -{ > -public: > - V4L2ControlList(const ControlInfoMap &info) > - : ControlList(info.idmap()) > - { > - } > -}; > - > } /* namespace libcamera */ > > #endif /* __LIBCAMERA_V4L2_CONTROLS_H__ */ > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 9776b36b88cd..8d3ad568d16e 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -604,7 +604,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > return ret; > > /* Apply the "pipe_mode" control to the ImgU subdevice. */ > - V4L2ControlList ctrls(imgu->imgu_->controls()); > + ControlList ctrls(imgu->imgu_->controls()); > ctrls.set(V4L2_CID_IPU3_PIPE_MODE, > static_cast<int32_t>(vfStream->active_ ? IPU3PipeModeVideo : > IPU3PipeModeStillCapture)); > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index a64e1af4c550..fae0ffc4de30 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -228,7 +228,7 @@ void PipelineHandlerUVC::stop(Camera *camera) > > int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > { > - V4L2ControlList controls(data->video_->controls()); > + ControlList controls(data->video_->controls()); > > for (auto it : request->controls()) { > const ControlId &id = *it.first; > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 6a4244119dc5..dcdaef120ad1 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -279,7 +279,7 @@ void PipelineHandlerVimc::stop(Camera *camera) > > int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > { > - V4L2ControlList controls(data->sensor_->controls()); > + ControlList controls(data->sensor_->controls()); > > for (auto it : request->controls()) { > const ControlId &id = *it.first; > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp > index bd5e18590b76..83ac7b0dc666 100644 > --- a/src/libcamera/v4l2_controls.cpp > +++ b/src/libcamera/v4l2_controls.cpp > @@ -134,24 +134,4 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl) > static_cast<int32_t>(ctrl.maximum))); > } > > -/** > - * \class V4L2ControlList > - * \brief A list of controls for a V4L2 device > - * > - * This class specialises the ControList class for use with V4L2 devices. It > - * offers a convenience API to create a ControlList from a ControlInfoMap. > - * > - * V4L2ControlList allows easy construction of a ControlList containing V4L2 > - * controls for a device. It can be used to construct the list of controls > - * passed to the V4L2Device::getControls() and V4L2Device::setControls() > - * methods. The class should however not be used in place of ControlList in > - * APIs. > - */ > - > -/** > - * \fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info) > - * \brief Construct a V4L2ControlList associated with a V4L2 device > - * \param[in] info The V4L2 device control info map > - */ > - > } /* namespace libcamera */ > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp > index 31879b794ed0..975c852b8cbf 100644 > --- a/test/v4l2_videodevice/controls.cpp > +++ b/test/v4l2_videodevice/controls.cpp > @@ -46,7 +46,7 @@ protected: > const ControlRange &saturation = info.find(V4L2_CID_SATURATION)->second; > > /* Test getting controls. */ > - V4L2ControlList ctrls(info); > + ControlList ctrls(info); > ctrls.set(V4L2_CID_BRIGHTNESS, -1); > ctrls.set(V4L2_CID_CONTRAST, -1); > ctrls.set(V4L2_CID_SATURATION, -1); > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Tue, Oct 15, 2019 at 08:52:35PM +0200, Jacopo Mondi wrote: > On Mon, Oct 14, 2019 at 02:33:14AM +0300, Laurent Pinchart wrote: > > The V4L2ControlList class only provides a convenience constructor for > > the ControlList, which can easily be moved to the ControlList class and > > may benefit it later (to construct a ControlList from controls supported > > by a camera). Move the constructor and remove V4L2ControlList. > > Ah scratch my previous comment then :) > I think we should make this the only available constructor :-) I'm fine experimenting with this. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > include/libcamera/controls.h | 1 + > > src/ipa/rkisp1/rkisp1.cpp | 2 +- > > src/libcamera/controls.cpp | 10 ++++++++++ > > src/libcamera/include/v4l2_controls.h | 14 -------------- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > > src/libcamera/pipeline/uvcvideo.cpp | 2 +- > > src/libcamera/pipeline/vimc.cpp | 2 +- > > src/libcamera/v4l2_controls.cpp | 20 -------------------- > > test/v4l2_videodevice/controls.cpp | 2 +- > > 9 files changed, 16 insertions(+), 39 deletions(-) > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > > index 80414c6f0748..0d279d508dcc 100644 > > --- a/include/libcamera/controls.h > > +++ b/include/libcamera/controls.h > > @@ -170,6 +170,7 @@ private: > > > > public: > > ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); > > + ControlList(const ControlInfoMap &info, ControlValidator *validator = nullptr); > > > > using iterator = ControlListMap::iterator; > > using const_iterator = ControlListMap::const_iterator; > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > > index bd703898c523..570145ce71b2 100644 > > --- a/src/ipa/rkisp1/rkisp1.cpp > > +++ b/src/ipa/rkisp1/rkisp1.cpp > > @@ -212,7 +212,7 @@ void IPARkISP1::setControls(unsigned int frame) > > IPAOperationData op; > > op.operation = RKISP1_IPA_ACTION_V4L2_SET; > > > > - V4L2ControlList ctrls(ctrls_); > > + ControlList ctrls(ctrls_); > > ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_)); > > ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_)); > > op.controls.push_back(ctrls); > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > > index bf7634aab470..93ad2fc6a276 100644 > > --- a/src/libcamera/controls.cpp > > +++ b/src/libcamera/controls.cpp > > @@ -561,6 +561,16 @@ ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator) > > { > > } > > > > +/** > > + * \brief Construct a ControlList with the idmap of a control info map > > + * \param[in] info The ControlInfoMap for the control list target object > > + * \param[in] validator The validator (may be null) > > + */ > > +ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator) > > + : validator_(validator), idmap_(&info.idmap()) > > Maybe this is a bit to much to ask, but the different order of > parameters and field declaration bothers me a bit. info has to be the first argument otherwise validator can't have a default value. We can reorder idmap_ and validator_, I'll ack a patch that does so if you want to submit one. > Anyway, this as the whole series look good and I hope we could merge > it soon Done :-) Thank you for the review. > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > +{ > > +} > > + > > /** > > * \typedef ControlList::iterator > > * \brief Iterator for the controls contained within the list > > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h > > index e16c4957f9d1..882546a89340 100644 > > --- a/src/libcamera/include/v4l2_controls.h > > +++ b/src/libcamera/include/v4l2_controls.h > > @@ -8,11 +8,6 @@ > > #ifndef __LIBCAMERA_V4L2_CONTROLS_H__ > > #define __LIBCAMERA_V4L2_CONTROLS_H__ > > > > -#include <map> > > -#include <stdint.h> > > -#include <string> > > -#include <vector> > > - > > #include <linux/videodev2.h> > > > > #include <libcamera/controls.h> > > @@ -31,15 +26,6 @@ public: > > V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl); > > }; > > > > -class V4L2ControlList : public ControlList > > -{ > > -public: > > - V4L2ControlList(const ControlInfoMap &info) > > - : ControlList(info.idmap()) > > - { > > - } > > -}; > > - > > } /* namespace libcamera */ > > > > #endif /* __LIBCAMERA_V4L2_CONTROLS_H__ */ > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 9776b36b88cd..8d3ad568d16e 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -604,7 +604,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > > return ret; > > > > /* Apply the "pipe_mode" control to the ImgU subdevice. */ > > - V4L2ControlList ctrls(imgu->imgu_->controls()); > > + ControlList ctrls(imgu->imgu_->controls()); > > ctrls.set(V4L2_CID_IPU3_PIPE_MODE, > > static_cast<int32_t>(vfStream->active_ ? IPU3PipeModeVideo : > > IPU3PipeModeStillCapture)); > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > > index a64e1af4c550..fae0ffc4de30 100644 > > --- a/src/libcamera/pipeline/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo.cpp > > @@ -228,7 +228,7 @@ void PipelineHandlerUVC::stop(Camera *camera) > > > > int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > > { > > - V4L2ControlList controls(data->video_->controls()); > > + ControlList controls(data->video_->controls()); > > > > for (auto it : request->controls()) { > > const ControlId &id = *it.first; > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > > index 6a4244119dc5..dcdaef120ad1 100644 > > --- a/src/libcamera/pipeline/vimc.cpp > > +++ b/src/libcamera/pipeline/vimc.cpp > > @@ -279,7 +279,7 @@ void PipelineHandlerVimc::stop(Camera *camera) > > > > int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > > { > > - V4L2ControlList controls(data->sensor_->controls()); > > + ControlList controls(data->sensor_->controls()); > > > > for (auto it : request->controls()) { > > const ControlId &id = *it.first; > > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp > > index bd5e18590b76..83ac7b0dc666 100644 > > --- a/src/libcamera/v4l2_controls.cpp > > +++ b/src/libcamera/v4l2_controls.cpp > > @@ -134,24 +134,4 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl) > > static_cast<int32_t>(ctrl.maximum))); > > } > > > > -/** > > - * \class V4L2ControlList > > - * \brief A list of controls for a V4L2 device > > - * > > - * This class specialises the ControList class for use with V4L2 devices. It > > - * offers a convenience API to create a ControlList from a ControlInfoMap. > > - * > > - * V4L2ControlList allows easy construction of a ControlList containing V4L2 > > - * controls for a device. It can be used to construct the list of controls > > - * passed to the V4L2Device::getControls() and V4L2Device::setControls() > > - * methods. The class should however not be used in place of ControlList in > > - * APIs. > > - */ > > - > > -/** > > - * \fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info) > > - * \brief Construct a V4L2ControlList associated with a V4L2 device > > - * \param[in] info The V4L2 device control info map > > - */ > > - > > } /* namespace libcamera */ > > diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp > > index 31879b794ed0..975c852b8cbf 100644 > > --- a/test/v4l2_videodevice/controls.cpp > > +++ b/test/v4l2_videodevice/controls.cpp > > @@ -46,7 +46,7 @@ protected: > > const ControlRange &saturation = info.find(V4L2_CID_SATURATION)->second; > > > > /* Test getting controls. */ > > - V4L2ControlList ctrls(info); > > + ControlList ctrls(info); > > ctrls.set(V4L2_CID_BRIGHTNESS, -1); > > ctrls.set(V4L2_CID_CONTRAST, -1); > > ctrls.set(V4L2_CID_SATURATION, -1);
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 80414c6f0748..0d279d508dcc 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -170,6 +170,7 @@ private: public: ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); + ControlList(const ControlInfoMap &info, ControlValidator *validator = nullptr); using iterator = ControlListMap::iterator; using const_iterator = ControlListMap::const_iterator; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index bd703898c523..570145ce71b2 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -212,7 +212,7 @@ void IPARkISP1::setControls(unsigned int frame) IPAOperationData op; op.operation = RKISP1_IPA_ACTION_V4L2_SET; - V4L2ControlList ctrls(ctrls_); + ControlList ctrls(ctrls_); ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure_)); ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast<int32_t>(gain_)); op.controls.push_back(ctrls); diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index bf7634aab470..93ad2fc6a276 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -561,6 +561,16 @@ ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator) { } +/** + * \brief Construct a ControlList with the idmap of a control info map + * \param[in] info The ControlInfoMap for the control list target object + * \param[in] validator The validator (may be null) + */ +ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator) + : validator_(validator), idmap_(&info.idmap()) +{ +} + /** * \typedef ControlList::iterator * \brief Iterator for the controls contained within the list diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h index e16c4957f9d1..882546a89340 100644 --- a/src/libcamera/include/v4l2_controls.h +++ b/src/libcamera/include/v4l2_controls.h @@ -8,11 +8,6 @@ #ifndef __LIBCAMERA_V4L2_CONTROLS_H__ #define __LIBCAMERA_V4L2_CONTROLS_H__ -#include <map> -#include <stdint.h> -#include <string> -#include <vector> - #include <linux/videodev2.h> #include <libcamera/controls.h> @@ -31,15 +26,6 @@ public: V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl); }; -class V4L2ControlList : public ControlList -{ -public: - V4L2ControlList(const ControlInfoMap &info) - : ControlList(info.idmap()) - { - } -}; - } /* namespace libcamera */ #endif /* __LIBCAMERA_V4L2_CONTROLS_H__ */ diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 9776b36b88cd..8d3ad568d16e 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -604,7 +604,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) return ret; /* Apply the "pipe_mode" control to the ImgU subdevice. */ - V4L2ControlList ctrls(imgu->imgu_->controls()); + ControlList ctrls(imgu->imgu_->controls()); ctrls.set(V4L2_CID_IPU3_PIPE_MODE, static_cast<int32_t>(vfStream->active_ ? IPU3PipeModeVideo : IPU3PipeModeStillCapture)); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index a64e1af4c550..fae0ffc4de30 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -228,7 +228,7 @@ void PipelineHandlerUVC::stop(Camera *camera) int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) { - V4L2ControlList controls(data->video_->controls()); + ControlList controls(data->video_->controls()); for (auto it : request->controls()) { const ControlId &id = *it.first; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 6a4244119dc5..dcdaef120ad1 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -279,7 +279,7 @@ void PipelineHandlerVimc::stop(Camera *camera) int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) { - V4L2ControlList controls(data->sensor_->controls()); + ControlList controls(data->sensor_->controls()); for (auto it : request->controls()) { const ControlId &id = *it.first; diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp index bd5e18590b76..83ac7b0dc666 100644 --- a/src/libcamera/v4l2_controls.cpp +++ b/src/libcamera/v4l2_controls.cpp @@ -134,24 +134,4 @@ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl) static_cast<int32_t>(ctrl.maximum))); } -/** - * \class V4L2ControlList - * \brief A list of controls for a V4L2 device - * - * This class specialises the ControList class for use with V4L2 devices. It - * offers a convenience API to create a ControlList from a ControlInfoMap. - * - * V4L2ControlList allows easy construction of a ControlList containing V4L2 - * controls for a device. It can be used to construct the list of controls - * passed to the V4L2Device::getControls() and V4L2Device::setControls() - * methods. The class should however not be used in place of ControlList in - * APIs. - */ - -/** - * \fn V4L2ControlList::V4L2ControlList(const ControlInfoMap &info) - * \brief Construct a V4L2ControlList associated with a V4L2 device - * \param[in] info The V4L2 device control info map - */ - } /* namespace libcamera */ diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp index 31879b794ed0..975c852b8cbf 100644 --- a/test/v4l2_videodevice/controls.cpp +++ b/test/v4l2_videodevice/controls.cpp @@ -46,7 +46,7 @@ protected: const ControlRange &saturation = info.find(V4L2_CID_SATURATION)->second; /* Test getting controls. */ - V4L2ControlList ctrls(info); + ControlList ctrls(info); ctrls.set(V4L2_CID_BRIGHTNESS, -1); ctrls.set(V4L2_CID_CONTRAST, -1); ctrls.set(V4L2_CID_SATURATION, -1);
The V4L2ControlList class only provides a convenience constructor for the ControlList, which can easily be moved to the ControlList class and may benefit it later (to construct a ControlList from controls supported by a camera). Move the constructor and remove V4L2ControlList. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- include/libcamera/controls.h | 1 + src/ipa/rkisp1/rkisp1.cpp | 2 +- src/libcamera/controls.cpp | 10 ++++++++++ src/libcamera/include/v4l2_controls.h | 14 -------------- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/v4l2_controls.cpp | 20 -------------------- test/v4l2_videodevice/controls.cpp | 2 +- 9 files changed, 16 insertions(+), 39 deletions(-)