[libcamera-devel,15/21] qcam: viewfinder: Make the viewfinder hold a reference to a buffer

3257 diff mbox series

The viewfinder is currently expected to render frames to the screen
synchronously in the display() function, or at least to copy data so
that the buffer can be queued in a new request when the function
returns. This prevents optimisations when the capture format is
identical to the display format.

Make the viewfinder take ownership of the buffer, and notify of its
release through a signal. The release is currently still synchronous,
this will be addressed in a subsequent patch.

Rename the ViewFinder::display() function to render() to better describe
its purpose, as it's meant to start the rendering and not display the
frame synchronously.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/qcam/main_window.cpp | 13 ++++---------
 src/qcam/main_window.h   |  4 ++--
 src/qcam/meson.build     |  1 +
 src/qcam/viewfinder.cpp  |  5 +++--
 src/qcam/viewfinder.h    |  7 ++++++-
 5 files changed, 16 insertions(+), 14 deletions(-)

Hi Laurent,

On 23/03/2020 14:21, Laurent Pinchart wrote:
> The viewfinder is currently expected to render frames to the screen
> synchronously in the display() function, or at least to copy data so
> that the buffer can be queued in a new request when the function
> returns. This prevents optimisations when the capture format is
> identical to the display format.
> 
> Make the viewfinder take ownership of the buffer, and notify of its
> release through a signal. The release is currently still synchronous,
> this will be addressed in a subsequent patch.
> 
> Rename the ViewFinder::display() function to render() to better describe
> its purpose, as it's meant to start the rendering and not display the
> frame synchronously.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/qcam/main_window.cpp | 13 ++++---------
>  src/qcam/main_window.h   |  4 ++--
>  src/qcam/meson.build     |  1 +
>  src/qcam/viewfinder.cpp  |  5 +++--
>  src/qcam/viewfinder.h    |  7 ++++++-
>  5 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index ad0d6bf4f3e5..b68e171c5e01 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -64,6 +64,8 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  	connect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));
>  
>  	viewfinder_ = new ViewFinder(this);
> +	connect(viewfinder_, &ViewFinder::renderComplete,
> +		this, &MainWindow::queueRequest);
>  	setCentralWidget(viewfinder_);
>  	adjustSize();
>  
> @@ -513,15 +515,8 @@ void MainWindow::processCapture()
>  		<< "timestamp:" << metadata.timestamp
>  		<< "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps;
>  
> -	/* Display the buffer and requeue it to the camera. */
> -	display(buffer);
> -
> -	queueRequest(buffer);
> -}
> -
> -void MainWindow::display(FrameBuffer *buffer)
> -{
> -	viewfinder_->display(buffer, &mappedBuffers_[buffer]);
> +	/* Render the frame on the viewfinder. */
> +	viewfinder_->render(buffer, &mappedBuffers_[buffer]);
>  }
>  
>  void MainWindow::queueRequest(FrameBuffer *buffer)
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 03db761b58e4..1a2ccbd632b0 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -54,6 +54,8 @@ private Q_SLOTS:
>  
>  	void saveImageAs();
>  
> +	void queueRequest(FrameBuffer *buffer);
> +
>  private:
>  	int createToolbars();
>  
> @@ -65,8 +67,6 @@ private:
>  
>  	void requestComplete(Request *request);
>  	void processCapture();
> -	void display(FrameBuffer *buffer);
> -	void queueRequest(FrameBuffer *buffer);
>  
>  	/* UI elements */
>  	QToolBar *toolbar_;
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index 214bfb12aabb..c256d06f8ccf 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -8,6 +8,7 @@ qcam_sources = files([
>  
>  qcam_moc_headers = files([
>      'main_window.h',
> +    'viewfinder.h',
>  ])
>  
>  qcam_resources = files([
> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
> index b8feabd5d189..2a35932e0b79 100644
> --- a/src/qcam/viewfinder.cpp
> +++ b/src/qcam/viewfinder.cpp
> @@ -25,8 +25,7 @@ ViewFinder::~ViewFinder()
>  	delete image_;
>  }
>  
> -void ViewFinder::display(const libcamera::FrameBuffer *buffer,
> -			 MappedBuffer *map)
> +void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
>  {
>  	if (buffer->planes().size() != 1) {
>  		qWarning() << "Multi-planar buffers are not supported";
> @@ -44,6 +43,8 @@ void ViewFinder::display(const libcamera::FrameBuffer *buffer,
>  	converter_.convert(static_cast<unsigned char *>(map->memory),
>  			   buffer->metadata().planes[0].bytesused, image_);
>  	update();
> +
> +	renderComplete(buffer);
>  }
>  
>  QImage ViewFinder::getCurrentImage()
> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
> index 735a6b67e91d..784fcceda5bd 100644
> --- a/src/qcam/viewfinder.h
> +++ b/src/qcam/viewfinder.h
> @@ -27,15 +27,20 @@ struct MappedBuffer {
>  
>  class ViewFinder : public QWidget
>  {
> +	Q_OBJECT
> +
>  public:
>  	ViewFinder(QWidget *parent);
>  	~ViewFinder();
>  
>  	int setFormat(const libcamera::PixelFormat &format, const QSize &size);
> -	void display(const libcamera::FrameBuffer *buffer, MappedBuffer *map);
> +	void render(libcamera::FrameBuffer *buffer, MappedBuffer *map);
>  
>  	QImage getCurrentImage();
>  
> +Q_SIGNALS:
> +	void renderComplete(libcamera::FrameBuffer *buffer);
> +
>  protected:
>  	void paintEvent(QPaintEvent *) override;
>  	QSize sizeHint() const override;
>