[libcamera-devel] [PATCH v3 7/8] libcamera: pipeline: Add method to retrieve Request from Buffer

[Niklas Söderlund]

Hi again,

On 2019-04-03 17:07:34 +0200, Jacopo Mondi wrote:
> Add a method to CameraData base class to retrieve a pointer to the
> Request that contains a given buffer. Intended users are buffer
> completion slots that needs to associate a Request to a just completed
> Buffer.
> 
> In preparation to support multiple requests from different streams,
> update all the pipeline handler implementations to use this method
> instead of using the last queued request.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/request.h              |  2 ++
>  src/libcamera/include/pipeline_handler.h |  3 +++
>  src/libcamera/pipeline/ipu3/ipu3.cpp     |  3 ++-
>  src/libcamera/pipeline/uvcvideo.cpp      |  3 ++-
>  src/libcamera/pipeline/vimc.cpp          |  3 ++-
>  src/libcamera/pipeline_handler.cpp       | 29 ++++++++++++++++++++++++
>  src/libcamera/request.cpp                |  7 ++++++
>  7 files changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 5ac4d20d1d9f..8f5892fd3111 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -38,6 +38,8 @@ public:
>  
>  	const std::list<Stream *> streams() const;
>  
> +	const std::unordered_set<Buffer *> &pending() const { return pending_; }
> +

You could break adding of this function out to a separate patch or merge 
it with the one adding streams() earlier in the series.

>  	Status status() const { return status_; }
>  
>  private:
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 920b57609470..6cdadcbdc3ea 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -39,6 +39,9 @@ public:
>  	PipelineHandler *pipe_;
>  	std::list<Request *> queuedRequests_;
>  
> +protected:
> +	Request *requestFromBuffer(Buffer *buffer);
> +
>  private:
>  	CameraData(const CameraData &) = delete;
>  	CameraData &operator=(const CameraData &) = delete;
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 8c67cf985d1e..17e3e8677e28 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -801,7 +801,8 @@ void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer)
>   */
>  void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
>  {
> -	Request *request = queuedRequests_.front();
> +	Request *request = requestFromBuffer(buffer);
> +	ASSERT(request);
>  
>  	pipe_->completeBuffer(camera_, request, buffer);
>  	pipe_->completeRequest(camera_, request);
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 128f0c49dba3..d571b8b4ea83 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -226,7 +226,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>  
>  void PipelineHandlerUVC::UVCCameraData::bufferReady(Buffer *buffer)
>  {
> -	Request *request = queuedRequests_.front();
> +	Request *request = requestFromBuffer(buffer);
> +	ASSERT(request);
>  
>  	pipe_->completeBuffer(camera_, request, buffer);
>  	pipe_->completeRequest(camera_, request);
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 6735940799d8..e83416effad8 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -223,7 +223,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  
>  void PipelineHandlerVimc::VimcCameraData::bufferReady(Buffer *buffer)
>  {
> -	Request *request = queuedRequests_.front();
> +	Request *request = requestFromBuffer(buffer);
> +	ASSERT(request);
>  
>  	pipe_->completeBuffer(camera_, request, buffer);
>  	pipe_->completeRequest(camera_, request);
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 9a8a4fde57e6..830ff354ed3e 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -86,6 +86,35 @@ LOG_DEFINE_CATEGORY(Pipeline)
>   * PipelineHandler::completeRequest()
>   */
>  
> +/**
> + * \brief Retrieve the pending request that contains \a buffer
> + * \param[in] buffer The buffer contained in the returned request
> + *
> + * Return the request that contains \a buffer, or nullptr if no such request
> + * is found. The intended callers of this method are buffer completion slots
> + * implemented in CameraData sub-classes which needs to associated a request
> + * to the just completed buffer. It is up to the caller of this function to
> + * deal with the case the buffer does not belong to any previously queued
> + * request or the request has already completed, possibly because of a
> + * duplicated buffer completion notification. This is generally considered
> + * a fatal error, and callers are expected to assert the validity of the
> + * returned request.
> + *
> + * \return A pointer to the pending Request that contains the Buffer \a buffer,
> + * or nullptr if no such request is found
> + */
> +Request *CameraData::requestFromBuffer(Buffer *buffer)
> +{
> +	for (Request *req : queuedRequests_) {
> +		for (Buffer *b : req->pending()) {
> +			if (b == buffer)
> +				return req;
> +		}
> +	}
> +
> +	return nullptr;
> +}
> +
>  /**
>   * \class PipelineHandler
>   * \brief Create and manage cameras based on a set of media devices
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 3a7841fb2bb3..c555752b2c0b 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -108,6 +108,13 @@ const std::list<Stream *> Request::streams() const
>  	return streams;
>  }
>  
> +/**
> + * \fn Request::pending()
> + * \brief Retrieve the list of not-yet-completed buffers
> + *
> + * \return The set of pending buffers
> + */
> +
>  /**
>   * \fn Request::status()
>   * \brief Retrieve the request completion status
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund