[libcamera-devel] [RFC PATCH 0/5] Libcamera Controls

[Laurent Pinchart]

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 at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,

Laurent Pinchart