| Message ID | 20190606205654.9311-1-kieran.bingham@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
On Thu, Jun 06, 2019 at 09:56:49PM +0100, Kieran Bingham wrote: > I've been sketching out these controls for a bit, and I wanted to get them on > the list as a refernce point for some furthur discussions. > > The patches are based on top of a combination of both Jacopo's V4L2 Control > series, and Niklas' enum series. > > I am still undecided as to whether the best route forwards is to keep the > controls as a std::set <Control> with the Control class handling the ID and > type checking, or if a std::map<ControlID, ControlValue> might be more > flexible. > > With a control class, I can make sure the construction logic is contained, but > I lose the ease of 'mapping' an ID to the control value within the std::set. > I've implemented a comparator, which allows the use of .find(), but I'm not > sure I'm fond of that yet, especially in the pain points of handling searching > for an ID which isn't in the set. > > I'd love to be able to make a set operate more like a map using the internal > key, or perhaps make a map look more like a set, so that the ID and Value are > more closely associated. Couldn't you use an std::unordered_set with custom comparison and hash functions that only take the ID into account ? > I have also considered that I could update Jacopo's V4L2 controls to utilise > the ControlValue type, which would then allow for any controls which want to > map directly to a V4L2 control - the ControlValue itself could be passed to the > V4L2 layer and populated there. However this would only then push the > requirement to 'find' the appropriate ControlValue object down to the V4L2 > layer. I haven't read the patches from this series yet, but sharing a control value class could possibly make sense. > Anyway, there's a lovely demo at the end which shows setting the Brightness > control from the qcam application as a sine wave which makes for a visual > clarification that the controls are successfully being set. > > As noted there, We will likely want to extend controls such that a particular > control knows about it's max/min/default values. And that then (to me) provides > another argument to use the class Control with a set over a map. We may want to separate the control value, which will be set per request, from the control information that will be global. > Anyway, any thoughts on a postcard sized reply. Please don't focus on typos' or > grammar, as this is an RFC about the usage and API more than the patches > themselves. > > > Kieran Bingham (5): > libcamera: Add control handling > libcamera: request: add a control set > libcamera: pipeline: Add readControls(), writeControl() interfaces > QCam: Set a read control on each request to get Gain value > [PoC] QCam: Control demo: A SineWave Brightness > > include/libcamera/controls.h | 106 ++++++++ > include/libcamera/meson.build | 1 + > include/libcamera/request.h | 4 + > src/libcamera/controls.cpp | 310 +++++++++++++++++++++++ > src/libcamera/include/pipeline_handler.h | 3 + > src/libcamera/meson.build | 1 + > src/libcamera/pipeline/ipu3/ipu3.cpp | 19 ++ > src/libcamera/pipeline/raspberrypi.cpp | 108 +++++++- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++ > src/libcamera/pipeline/uvcvideo.cpp | 127 +++++++++- > src/libcamera/pipeline/vimc.cpp | 19 ++ > src/qcam/main_window.cpp | 24 ++ > 12 files changed, 739 insertions(+), 2 deletions(-) > create mode 100644 include/libcamera/controls.h > create mode 100644 src/libcamera/controls.cpp > > -- > 2.20.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel