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

[Kieran Bingham]

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 at ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart at 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;
> 

-- 
Regards
--
Kieran