[libcamera-devel] [PATCH v4 04/11] libcamera: request: Add meta data information

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Thu, Oct 03, 2019 at 07:49:34PM +0200, Niklas Söderlund wrote:
> A new ControlList container is needed to hold meta data coming out of

According to https://en.wiktionary.org/wiki/meta_data, this should be
metadata. The method and field below should then be called metadata()
and metadata_.

> the IPA. The list of supported controls in this list is expected to grow
> and for now do not add a validator for the list.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/request.h |  2 ++
>  src/libcamera/request.cpp   | 12 ++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index e3db5243aaf3cf30..d3435d134c560f44 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -37,6 +37,7 @@ public:
>  	~Request();
>  
>  	ControlList &controls() { return *controls_; }
> +	ControlList &metaData() { return *metaData_; }
>  	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
>  	int addBuffer(std::unique_ptr<Buffer> buffer);
>  	Buffer *findBuffer(Stream *stream) const;
> @@ -58,6 +59,7 @@ private:
>  	Camera *camera_;
>  	CameraControlValidator *validator_;
>  	ControlList *controls_;
> +	ControlList *metaData_;
>  	std::map<Stream *, Buffer *> bufferMap_;
>  	std::unordered_set<Buffer *> pending_;
>  
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 19f6d0b9a0aeb03c..1e1418a9b57393b3 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -65,6 +65,11 @@ Request::Request(Camera *camera, uint64_t cookie)
>  	 */
>  	validator_ = new CameraControlValidator(camera);
>  	controls_ = new ControlList(validator_);
> +
> +	/**
> +	 * \todo: Add a validator for meta data controls.
> +	 */
> +	metaData_ = new ControlList(nullptr);
>  }
>  
>  Request::~Request()
> @@ -74,6 +79,7 @@ Request::~Request()
>  		delete buffer;
>  	}
>  
> +	delete metaData_;
>  	delete controls_;
>  	delete validator_;
>  }
> @@ -161,6 +167,12 @@ Buffer *Request::findBuffer(Stream *stream) const
>  	return it->second;
>  }
>  
> +/**
> + * \fn Request::metaData()
> + * \brief Retrieve the request's meta data
> + * \return The meta data associated with the request

metadata here too.

> + */

Could you add a todo to offer a read-only API towards applications while
keeping a read/write API internally ?

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +
>  /**
>   * \fn Request::cookie()
>   * \brief Retrieve the cookie set when the request was created

-- 
Regards,

Laurent Pinchart