[libcamera-devel] [RFC PATCH v2 06/10] qcam: main_window: Use offset mapping FrameBuffer

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Mon, Aug 23, 2021 at 10:12:17PM +0900, Hirokazu Honda wrote:
> FrameBuffer::Plane has offset info now. This uses the offset
> in mapping FrameBuffer in MainWindow.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/qcam/main_window.cpp | 16 +++++++++++-----
>  src/qcam/main_window.h   |  1 +
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 39d034de..7fdec4a8 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -10,6 +10,7 @@
>  #include <iomanip>
>  #include <string>
>  #include <sys/mman.h>
> +#include <unistd.h>
>  
>  #include <QComboBox>
>  #include <QCoreApplication>
> @@ -472,9 +473,12 @@ int MainWindow::startCapture()
>  		for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) {
>  			/* Map memory buffers and cache the mappings. */
>  			const FrameBuffer::Plane &plane = buffer->planes().front();
> -			void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
> +			size_t length = lseek(plane.fd.fd(), 0, SEEK_END);
> +			void *memory = mmap(NULL, length, PROT_READ, MAP_SHARED,
>  					    plane.fd.fd(), 0);
> -			mappedBuffers_[buffer.get()] = { memory, plane.length };
> +			mappedBuffers_[buffer.get()] = { memory, length };
> +			planeData_[buffer.get()] = { static_cast<uint8_t *>(memory) + plane.offset,
> +						     plane.length };
>  
>  			/* Store buffers on the free list. */
>  			freeBuffers_[stream].enqueue(buffer.get());
> @@ -541,6 +545,7 @@ error:
>  		munmap(buffer.memory, buffer.size);
>  	}
>  	mappedBuffers_.clear();
> +	planeData_.clear();
>  
>  	freeBuffers_.clear();
>  
> @@ -577,6 +582,7 @@ void MainWindow::stopCapture()
>  		munmap(buffer.memory, buffer.size);
>  	}
>  	mappedBuffers_.clear();
> +	planeData_.clear();
>  
>  	requests_.clear();
>  	freeQueue_.clear();
> @@ -673,10 +679,10 @@ void MainWindow::processRaw(FrameBuffer *buffer,
>  							"DNG Files (*.dng)");
>  
>  	if (!filename.isEmpty()) {
> -		const MappedBuffer &mapped = mappedBuffers_[buffer];
> +		void *memory = planeData_[buffer].data();
>  		DNGWriter::write(filename.toStdString().c_str(), camera_.get(),
>  				 rawStream_->configuration(), metadata, buffer,
> -				 mapped.memory);
> +				 memory);
>  	}
>  #endif
>  
> @@ -753,7 +759,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
>  		<< "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps;
>  
>  	/* Render the frame on the viewfinder. */
> -	viewfinder_->render(buffer, &mappedBuffers_[buffer]);
> +	// HACK HACK HACK viewfinder_->render(buffer, &planeData_[buffer]);

I'm afraid this needs a fix :-) Could it be as simple as

	viewfinder_->render(buffer, planeData_[buffer]);

?

>  }
>  
>  void MainWindow::queueRequest(FrameBuffer *buffer)
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 85d56ce4..f35cce36 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -107,6 +107,7 @@ private:
>  
>  	std::unique_ptr<CameraConfiguration> config_;
>  	std::map<FrameBuffer *, MappedBuffer> mappedBuffers_;
> +	std::map<FrameBuffer *, Span<uint8_t>> planeData_;
>  
>  	/* Capture state, buffers queue and statistics */
>  	bool isCapturing_;

-- 
Regards,

Laurent Pinchart