[libcamera-devel,7/9] libcamera: request: Support buffer mapping

1628 diff mbox series

Use the Stream class buffer mapping operation to save the association in
the request at Request::findBuffer() time and reverse it with a new
operation Request::unmapBuffer() used by the pipeline handler base class
at buffer completion time, to return to the applications the Buffer they
originally provided with the Request without involving the pipeline
handlers in the process.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/request.h        |  4 ++-
 src/libcamera/pipeline_handler.cpp |  6 ++--
 src/libcamera/request.cpp          | 45 ++++++++++++++++++++++++++++--
 3 files changed, 50 insertions(+), 5 deletions(-)

Hi Jacopo,

Thanks for your patch, I really like it!

On 2019-07-05 00:53:32 +0200, Jacopo Mondi wrote:
> Use the Stream class buffer mapping operation to save the association in
> the request at Request::findBuffer() time and reverse it with a new
> operation Request::unmapBuffer() used by the pipeline handler base class
> at buffer completion time, to return to the applications the Buffer they
> originally provided with the Request without involving the pipeline
> handlers in the process.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/request.h        |  4 ++-
>  src/libcamera/pipeline_handler.cpp |  6 ++--
>  src/libcamera/request.cpp          | 45 ++++++++++++++++++++++++++++--
>  3 files changed, 50 insertions(+), 5 deletions(-)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 70f6d7fa7eeb..3353f037945e 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -36,7 +36,8 @@ public:
>  	ControlList &controls() { return controls_; }
>  	const std::map<Stream *, Buffer *> &buffers() const { return buffers_; }
>  	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
> -	Buffer *findBuffer(Stream *stream) const;
> +	Buffer *findBuffer(Stream *stream);
> +	Buffer *unmapBuffer(Buffer *streamBuffer);
>  
>  	Status status() const { return status_; }
>  
> @@ -55,6 +56,7 @@ private:
>  	ControlList controls_;
>  	std::map<Stream *, Buffer *> buffers_;
>  	std::unordered_set<Buffer *> pending_;
> +	std::map<Buffer *, Buffer *> bufferMap_;
>  
>  	Status status_;
>  };
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 67b215483847..a47411ecf345 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -402,8 +402,10 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)
>  bool PipelineHandler::completeBuffer(Camera *camera, Request *request,
>  				     Buffer *buffer)
>  {
> -	camera->bufferCompleted.emit(request, buffer);
> -	return request->completeBuffer(buffer);
> +	Buffer *requestBuffer = request->unmapBuffer(buffer);
> +
> +	camera->bufferCompleted.emit(request, requestBuffer);
> +	return request->completeBuffer(requestBuffer);
>  }
>  
>  /**
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 9ff0abbf119c..0e07d39f8941 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -105,17 +105,58 @@ int Request::setBuffers(const std::map<Stream *, Buffer *> &streamMap)
>   */
>  
>  /**
> - * \brief Return the buffer associated with a stream
> + * \brief Retrieve the stream buffer associated with a stream
>   * \param[in] stream The stream the buffer is associated to
> + *
> + * Depending on the configured memory type, the buffers originally provided
> + * by the application might get mapped to streams internal buffers.
> + *
> + * \sa Stream::mapBuffer()
> + *
>   * \return The buffer associated with the stream, or nullptr if the stream is
>   * not part of this request
>   */
> -Buffer *Request::findBuffer(Stream *stream) const
> +Buffer *Request::findBuffer(Stream *stream)
>  {
>  	auto it = buffers_.find(stream);
>  	if (it == buffers_.end())
>  		return nullptr;
>  
> +	/*
> +	 * Streams with internal memory mode do not need to perform any mapping
> +	 * between the application provided buffers (part of the request)
> +	 * and the one actually used by the Stream.
> +	 *
> +	 * Streams using externally allocated buffers need to create a mapping
> +	 * between the application provided buffers and the one used by pipeline
> +	 * handlers.
> +	 */
> +	Buffer *requestBuffer = it->second;
> +	Buffer *mappedBuffer = stream->memoryType() == InternalMemory ?
> +			       it->second : stream->mapBuffer(it->second);
> +	bufferMap_[mappedBuffer] = requestBuffer;

nit-pick: This is hard to read.


    Buffer *requestBuffer, *mappedBuffer;

    requestBuffer = it->second;

    if (stream->memoryType() == InternalMemory)
        mappedBuffer = requestBuffer;
    else
        mappedBuffer = stream->mapBuffer(requestBuffer);

    bufferMap_[mappedBuffer] = requestBuffer;

With or without this fixed,

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

> +
> +	return mappedBuffer;
> +}
> +
> +/**
> + * \brief Retrieve the application buffer associated with \a streamBuffer
> + * \param streamBuffer The stream buffer returned from Request::findBuffer()
> + *
> + * This operation is used by the PipelineHandler base class to retrieve the
> + * buffer the application originally associated with the stream in the request.
> + * Depending on the configured memory type, application provided buffers might
> + * be mapped to streams internal buffers at Request::findBuffer() time.
> + *
> + * \sa Stream::mapBuffer()
> + *
> + * \return The application buffer provided to Request::findBuffer()
> + */
> +Buffer *Request::unmapBuffer(Buffer *streamBuffer)
> +{
> +	auto it = bufferMap_.find(streamBuffer);
> +	ASSERT(it != bufferMap_.end());
> +
>  	return it->second;
>  }
>  
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel