[libcamera-devel] [RFC PATCH 2/5] libcamera: request: add a control set

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thu, Jun 06, 2019 at 09:56:51PM +0100, Kieran Bingham wrote:
> Provide a set to contain all controls applicable to the request.
> The set contains all controls whether they are write, read, or write-read controls.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  include/libcamera/request.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 58de6f00a554..5fae0d5fc838 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -10,6 +10,7 @@
>  #include <map>
>  #include <unordered_set>
>  
> +#include <libcamera/controls.h>
>  #include <libcamera/signal.h>
>  
>  namespace libcamera {
> @@ -36,6 +37,8 @@ public:
>  	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
>  	Buffer *findBuffer(Stream *stream) const;
>  
> +	std::set<Control> &controls() { return controls_; };

I think we will need something a bit more complicated than a std::set.
Here's what I was thinking about.

- Let's split control information from control value. The former are
  static for the lifetime of the camera (or at least of the capture
  session), while the latter vary per-request.

- The control values in the request could be stored in a
  std::map<const ControlInfo &, ControlValue>. This would make copies as
  cheap as possible as we won't have to duplicate the control
  information.

- The return value of the controls() function should be a custom class
  wrapping around the std::map. This is needed to perform control
  validation for instance (min/max/step), and will likely come handy to
  implement other custom behaviours.

- The control information should be exposed by the camera so that
  application can enumerate supported controls and query their
  information.

- We need to think of the common use cases from an application point of
  view. In particular I think applications will need an API to get a
  default set of control values from libcamera, that they will then
  possibly modify and set for the first request. Subsequent requests
  will likely just modify a small number of controls, and will likely be
  constructed from scratch. For control that applications want to read,
  I expect most requests to contain the same controls, so there should
  be an easy and efficient way to handle that. Splitting read and write
  controls may be a good idea.

> +
>  	Status status() const { return status_; }
>  
>  	bool hasPendingBuffers() const { return !pending_.empty(); }
> @@ -52,6 +55,7 @@ private:
>  	Camera *camera_;
>  	std::map<Stream *, Buffer *> bufferMap_;
>  	std::unordered_set<Buffer *> pending_;
> +	std::set<Control> controls_;
>  
>  	Status status_;
>  };

-- 
Regards,

Laurent Pinchart