[libcamera-devel] [PATCH v2 05/21] qcam: main_window: Move capture event processing to main thread

[Kieran Bingham]

On 23/03/2020 17:35, Laurent Pinchart wrote:
> To avoid blocking the camera manager for a long amount of time, move
> capture event processing to the main thread. Captured buffers are added
> to a queue and an event is posted to the main window to signal
> availability of a buffer.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I believe I added this to v1:

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

> ---
> Changes since v1:
> 
> - Add blank line
> ---
>  src/qcam/main_window.cpp | 55 +++++++++++++++++++++++++++++++++++++++-
>  src/qcam/main_window.h   |  8 ++++++
>  2 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 354a53367d0f..805690d5006a 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -19,6 +19,7 @@
>  #include <QImage>
>  #include <QImageWriter>
>  #include <QInputDialog>
> +#include <QMutexLocker>
>  #include <QStandardPaths>
>  #include <QTimer>
>  #include <QToolBar>
> @@ -31,6 +32,21 @@
>  
>  using namespace libcamera;
>  
> +class CaptureEvent : public QEvent
> +{
> +public:
> +	CaptureEvent()
> +		: QEvent(type())
> +	{
> +	}
> +
> +	static Type type()
> +	{
> +		static int type = QEvent::registerEventType();
> +		return static_cast<Type>(type);
> +	}
> +};
> +
>  MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)
>  	: options_(options), cm_(cm), allocator_(nullptr), isCapturing_(false)
>  {
> @@ -63,6 +79,16 @@ MainWindow::~MainWindow()
>  	}
>  }
>  
> +bool MainWindow::event(QEvent *e)
> +{
> +	if (e->type() == CaptureEvent::type()) {
> +		processCapture();
> +		return true;
> +	}
> +
> +	return QMainWindow::event(e);
> +}
> +
>  int MainWindow::createToolbars()
>  {
>  	QAction *action;
> @@ -343,6 +369,13 @@ void MainWindow::stopCapture()
>  
>  	config_.reset();
>  
> +	/*
> +	 * A CaptureEvent may have been posted before we stopped the camera,
> +	 * but not processed yet. Clear the queue of done buffers to avoid
> +	 * racing with the event handler.
> +	 */
> +	doneQueue_.clear();
> +
>  	titleTimer_.stop();
>  	setWindowTitle(title_);
>  }
> @@ -371,10 +404,30 @@ void MainWindow::requestComplete(Request *request)
>  		return;
>  
>  	const std::map<Stream *, FrameBuffer *> &buffers = request->buffers();
> +	FrameBuffer *buffer = buffers.begin()->second;
> +
> +	{
> +		QMutexLocker locker(&mutex_);
> +		doneQueue_.enqueue(buffer);
> +	}
> +
> +	QCoreApplication::postEvent(this, new CaptureEvent);
> +}
> +
> +void MainWindow::processCapture()
> +{
> +	FrameBuffer *buffer;
> +
> +	{
> +		QMutexLocker locker(&mutex_);
> +		if (doneQueue_.isEmpty())
> +			return;
> +
> +		buffer = doneQueue_.dequeue();
> +	}
>  
>  	framesCaptured_++;
>  
> -	FrameBuffer *buffer = buffers.begin()->second;
>  	const FrameMetadata &metadata = buffer->metadata();
>  
>  	double fps = metadata.timestamp - lastBufferTime_;
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 720a3393e3dc..c623120d5894 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -11,7 +11,9 @@
>  
>  #include <QElapsedTimer>
>  #include <QMainWindow>
> +#include <QMutex>
>  #include <QObject>
> +#include <QQueue>
>  #include <QTimer>
>  
>  #include <libcamera/buffer.h>
> @@ -40,6 +42,8 @@ public:
>  	MainWindow(CameraManager *cm, const OptionsParser::Options &options);
>  	~MainWindow();
>  
> +	bool event(QEvent *e) override;
> +
>  private Q_SLOTS:
>  	void quit();
>  	void updateTitle();
> @@ -57,6 +61,7 @@ private:
>  	int openCamera();
>  
>  	void requestComplete(Request *request);
> +	void processCapture();
>  	int display(FrameBuffer *buffer);
>  	void queueRequest(FrameBuffer *buffer);
>  
> @@ -78,6 +83,9 @@ private:
>  	uint32_t previousFrames_;
>  	uint32_t framesCaptured_;
>  
> +	QMutex mutex_;
> +	QQueue<FrameBuffer *> doneQueue_;
> +
>  	QToolBar *toolbar_;
>  	ViewFinder *viewfinder_;
>  	std::map<int, std::pair<void *, unsigned int>> mappedBuffers_;
> 

-- 
Regards
--
Kieran