[libcamera-devel,v2,23/25] qcam: Cache buffer memory mapping

2473 diff mbox series

With the buffer allocator in use it's possible to cache the dmabuf
memory mappings when starting the camera instead of mapping and
unmapping them each time.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/qcam/main_window.cpp | 28 ++++++++++++++++++++++------
 src/qcam/main_window.h   |  1 +
 2 files changed, 23 insertions(+), 6 deletions(-)

Hi Niklas,

Thank you for the patch.

On Mon, Dec 30, 2019 at 01:05:08PM +0100, Niklas Söderlund wrote:
> With the buffer allocator in use it's possible to cache the dmabuf
> memory mappings when starting the camera instead of mapping and
> unmapping them each time.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/qcam/main_window.cpp | 28 ++++++++++++++++++++++------
>  src/qcam/main_window.h   |  1 +
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index e53ed2801291f2c5..2008931075b96260 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -201,6 +201,13 @@ int MainWindow::startCapture()
>  		}
>  
>  		requests.push_back(request);
> +
> +		/* Cache buffer memory mapping. */

"Map memory buffers and cache the mappings."

> +		const FrameBuffer::Plane &plane = buffer->planes().front();
> +		void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
> +				    plane.fd.fd(), 0);
> +		mappedBuffers_[plane.fd.fd()] =
> +			std::make_pair(memory, plane.length);

I think we should create a helper for this, but it can be done on top.

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

>  	}
>  
>  	titleTimer_.start(2000);
> @@ -230,6 +237,13 @@ error:
>  	for (Request *request : requests)
>  		delete request;
>  
> +	for (auto &itr : mappedBuffers_) {

s/itr/iter/ or s/itr/it/ (that's what we use elsewhere in the code)

> +		void *memory = itr.second.first;
> +		unsigned int length = itr.second.second;
> +		munmap(memory, length);
> +	}
> +	mappedBuffers_.clear();
> +
>  	camera_->freeBuffers();
>  	return ret;
>  }
> @@ -243,6 +257,13 @@ void MainWindow::stopCapture()
>  	if (ret)
>  		std::cout << "Failed to stop capture" << std::endl;
>  
> +	for (auto &itr : mappedBuffers_) {
> +		void *memory = itr.second.first;
> +		unsigned int length = itr.second.second;
> +		munmap(memory, length);
> +	}
> +	mappedBuffers_.clear();
> +
>  	camera_->freeBuffers();
>  	isCapturing_ = false;
>  
> @@ -297,15 +318,10 @@ int MainWindow::display(FrameBuffer *buffer)
>  	if (buffer->planes().size() != 1)
>  		return -EINVAL;
>  
> -	/* \todo: Once the FrameBuffer is done cache mapped memory. */
>  	const FrameBuffer::Plane &plane = buffer->planes().front();
> -	void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
> -			    plane.fd.fd(), 0);
> -
> +	void *memory = mappedBuffers_[plane.fd.fd()].first;
>  	unsigned char *raw = static_cast<unsigned char *>(memory);
>  	viewfinder_->display(raw, buffer->metadata().planes[0].bytesused);
>  
> -	munmap(memory, plane.length);
> -
>  	return 0;
>  }
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 05cde4ceab5f7ea1..04fb9e3ea869c3fb 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -71,6 +71,7 @@ private:
>  	uint32_t framesCaptured_;
>  
>  	ViewFinder *viewfinder_;
> +	std::map<int, std::pair<void *, unsigned int>> mappedBuffers_;
>  };
>  
>  #endif /* __QCAM_MAIN_WINDOW__ */