[libcamera-devel,RFC,v2,06/10] qcam: main_window: Use offset mapping FrameBuffer
diff mbox series

Message ID 20210823131221.1034059-7-hiroh@chromium.org
State Superseded
Headers show
Series
  • Add offset to FrameBuffer::Plane
Related show

Commit Message

Hirokazu Honda Aug. 23, 2021, 1:12 p.m. UTC 
FrameBuffer::Plane has offset info now. This uses the offset
in mapping FrameBuffer in MainWindow.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/qcam/main_window.cpp | 16 +++++++++++-----
 src/qcam/main_window.h   |  1 +
 2 files changed, 12 insertions(+), 5 deletions(-)

Comments

Laurent Pinchart Aug. 23, 2021, 11:05 p.m. UTC | #1 
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@chromium.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@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_;
Hirokazu Honda Aug. 25, 2021, 6:15 a.m. UTC | #2 
Hi Laurent,

On Tue, Aug 24, 2021 at 8:05 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> 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@chromium.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@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]);
>
> ?

Yeah, I commented out while I was waiting for your Span patch. :-)

-Hiro
>
> >  }
> >
> >  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

Patch
diff mbox series 

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]);
 }
 
 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_;