[libcamera-devel] [PATCH v2 04/16] libcamera: request: Add cookie to make request tracking easier

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2019-07-13 20:23:39 +0300, Laurent Pinchart wrote:
> Applications often have to map requests queued to a camera to external
> resources. To make this easy, add a 64-bit integer cookie to the Request
> class that is set when the request is created and can be retrieved at
> any time, especially in the request completion handler. The cookie is
> completely transparent for libcamera and is never modified.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  include/libcamera/camera.h  |  3 ++-
>  include/libcamera/request.h |  5 ++++-
>  src/libcamera/camera.cpp    | 10 ++++++++--
>  src/libcamera/request.cpp   | 18 ++++++++++++++++--
>  4 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 6d693d9a6c7a..21fac550f412 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -10,6 +10,7 @@
>  #include <map>
>  #include <memory>
>  #include <set>
> +#include <stdint.h>
>  #include <string>
>  
>  #include <libcamera/controls.h>
> @@ -93,7 +94,7 @@ public:
>  	int allocateBuffers();
>  	int freeBuffers();
>  
> -	Request *createRequest();
> +	Request *createRequest(uint64_t cookie = 0);
>  	int queueRequest(Request *request);
>  
>  	int start();
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index a93468d7c8b7..dd165bc21c03 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -8,6 +8,7 @@
>  #define __LIBCAMERA_REQUEST_H__
>  
>  #include <map>
> +#include <stdint.h>
>  #include <unordered_set>
>  
>  #include <libcamera/controls.h>
> @@ -29,7 +30,7 @@ public:
>  		RequestCancelled,
>  	};
>  
> -	explicit Request(Camera *camera);
> +	Request(Camera *camera, uint64_t cookie = 0);
>  	Request(const Request &) = delete;
>  	Request &operator=(const Request &) = delete;
>  
> @@ -38,6 +39,7 @@ public:
>  	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
>  	Buffer *findBuffer(Stream *stream) const;
>  
> +	uint64_t cookie() const { return cookie_; }
>  	Status status() const { return status_; }
>  
>  	bool hasPendingBuffers() const { return !pending_.empty(); }
> @@ -56,6 +58,7 @@ private:
>  	std::map<Stream *, Buffer *> bufferMap_;
>  	std::unordered_set<Buffer *> pending_;
>  
> +	uint64_t cookie_;

I think this should be const uint64_t, with this fixed.

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

>  	Status status_;
>  };
>  
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 810cb1295f34..1f307654ab01 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -754,10 +754,16 @@ int Camera::freeBuffers()
>  
>  /**
>   * \brief Create a request object for the camera
> + * \param[in] cookie Opaque cookie for application use
>   *
>   * This method creates an empty request for the application to fill with
>   * buffers and paramaters, and queue for capture.
>   *
> + * The \a cookie is stored in the request and is accessible through the
> + * Request::cookie() method at any time. It is typically used by applications
> + * to map the request to an external resource in the request completion
> + * handler, and is completely opaque to libcamera.
> + *
>   * The ownership of the returned request is passed to the caller, which is
>   * responsible for either queueing the request or deleting it.
>   *
> @@ -766,12 +772,12 @@ int Camera::freeBuffers()
>   *
>   * \return A pointer to the newly created request, or nullptr on error
>   */
> -Request *Camera::createRequest()
> +Request *Camera::createRequest(uint64_t cookie)
>  {
>  	if (disconnected_ || !stateBetween(CameraPrepared, CameraRunning))
>  		return nullptr;
>  
> -	return new Request(this);
> +	return new Request(this, cookie);
>  }
>  
>  /**
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index f0b5985814bd..8cf41a43a80e 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -46,9 +46,17 @@ LOG_DEFINE_CATEGORY(Request)
>  /**
>   * \brief Create a capture request for a camera
>   * \param[in] camera The camera that creates the request
> + * \param[in] cookie Opaque cookie for application use
> + *
> + * The \a cookie is stored in the request and is accessible through the
> + * cookie() method at any time. It is typically used by applications to map the
> + * request to an external resource in the request completion handler, and is
> + * completely opaque to libcamera.
> + *
>   */
> -Request::Request(Camera *camera)
> -	: camera_(camera), controls_(camera), status_(RequestPending)
> +Request::Request(Camera *camera, uint64_t cookie)
> +	: camera_(camera), controls_(camera), cookie_(cookie),
> +	  status_(RequestPending)
>  {
>  }
>  
> @@ -119,6 +127,12 @@ Buffer *Request::findBuffer(Stream *stream) const
>  	return it->second;
>  }
>  
> +/**
> + * \fn Request::cookie()
> + * \brief Retrieve the cookie set when the request was created
> + * \return The request cookie
> + */
> +
>  /**
>   * \fn Request::status()
>   * \brief Retrieve the request completion status
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund