[libcamera-devel] [PATCH 15/30] cam: Move event loop execution from CameraSession to CamApp

[Kieran Bingham]

Hi Laurent,

On 07/07/2021 03:19, Laurent Pinchart wrote:
> To prepare for multiple concurrent camera sessions, move the event loop
> exec() call from the CameraSession class to the CamApp class.

There we go - now that global loop access is becoming clearer.

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/cam/camera_session.cpp | 43 +++++++++++++++++---------------------
>  src/cam/camera_session.h   |  6 ++++--
>  src/cam/main.cpp           | 12 ++++++++++-
>  3 files changed, 34 insertions(+), 27 deletions(-)
> 
> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> index 16c1c66a285a..b1200e60329c 100644
> --- a/src/cam/camera_session.cpp
> +++ b/src/cam/camera_session.cpp
> @@ -22,11 +22,11 @@ CameraSession::CameraSession(std::shared_ptr<Camera> camera,
>  			     CameraConfiguration *config)
>  	: camera_(camera), config_(config), writer_(nullptr), last_(0),
>  	  queueCount_(0), captureCount_(0), captureLimit_(0),
> -	  printMetadata_(false)
> +	  printMetadata_(false), allocator_(nullptr)
>  {
>  }
>  
> -int CameraSession::run(const OptionsParser::Options &options)
> +int CameraSession::start(const OptionsParser::Options &options)
>  {
>  	int ret;
>  
> @@ -61,36 +61,39 @@ int CameraSession::run(const OptionsParser::Options &options)
>  			writer_ = new BufferWriter();
>  	}
>  
> -	FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_);
> +	allocator_ = new FrameBufferAllocator(camera_);
>  
> -	ret = capture(allocator);
> +	return startCapture();
> +}
>  
> -	if (options.isSet(OptFile)) {
> -		delete writer_;
> -		writer_ = nullptr;
> -	}
> +void CameraSession::stop()
> +{
> +	int ret = camera_->stop();
> +	if (ret)
> +		std::cout << "Failed to stop capture" << std::endl;
> +
> +	delete writer_;
> +	writer_ = nullptr;
>  
>  	requests_.clear();
>  
> -	delete allocator;
> -
> -	return ret;
> +	delete allocator_;
>  }
>  
> -int CameraSession::capture(FrameBufferAllocator *allocator)
> +int CameraSession::startCapture()
>  {
>  	int ret;
>  
>  	/* Identify the stream with the least number of buffers. */
>  	unsigned int nbuffers = UINT_MAX;
>  	for (StreamConfiguration &cfg : *config_) {
> -		ret = allocator->allocate(cfg.stream());
> +		ret = allocator_->allocate(cfg.stream());
>  		if (ret < 0) {
>  			std::cerr << "Can't allocate buffers" << std::endl;
>  			return -ENOMEM;
>  		}
>  
> -		unsigned int allocated = allocator->buffers(cfg.stream()).size();
> +		unsigned int allocated = allocator_->buffers(cfg.stream()).size();
>  		nbuffers = std::min(nbuffers, allocated);
>  	}
>  
> @@ -109,7 +112,7 @@ int CameraSession::capture(FrameBufferAllocator *allocator)
>  		for (StreamConfiguration &cfg : *config_) {
>  			Stream *stream = cfg.stream();
>  			const std::vector<std::unique_ptr<FrameBuffer>> &buffers =
> -				allocator->buffers(stream);
> +				allocator_->buffers(stream);
>  			const std::unique_ptr<FrameBuffer> &buffer = buffers[i];
>  
>  			ret = request->addBuffer(stream, buffer.get());
> @@ -146,15 +149,7 @@ int CameraSession::capture(FrameBufferAllocator *allocator)
>  	else
>  		std::cout << "Capture until user interrupts by SIGINT" << std::endl;
>  
> -	ret = EventLoop::instance()->exec();

Without the context of the rest of the file at this point, is this the
last EventLoop access?

I guess we're still waiting on the removal of

> +	EventLoop::instance()->callLater([=]() { processRequest(request); });

from 13/30 before we can remove event_loop.h from the headers.

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


> -	if (ret)
> -		std::cout << "Failed to run capture loop" << std::endl;
> -
> -	ret = camera_->stop();
> -	if (ret)
> -		std::cout << "Failed to stop capture" << std::endl;
> -
> -	return ret;
> +	return 0;
>  }
>  
>  int CameraSession::queueRequest(Request *request)
> diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h
> index 2728d7607db2..5131cfd48b4e 100644
> --- a/src/cam/camera_session.h
> +++ b/src/cam/camera_session.h
> @@ -28,12 +28,13 @@ public:
>  	CameraSession(std::shared_ptr<libcamera::Camera> camera,
>  		      libcamera::CameraConfiguration *config);
>  
> -	int run(const OptionsParser::Options &options);
> +	int start(const OptionsParser::Options &options);
> +	void stop();
>  
>  	libcamera::Signal<> captureDone;
>  
>  private:
> -	int capture(libcamera::FrameBufferAllocator *allocator);
> +	int startCapture();
>  
>  	int queueRequest(libcamera::Request *request);
>  	void requestComplete(libcamera::Request *request);
> @@ -51,6 +52,7 @@ private:
>  	unsigned int captureLimit_;
>  	bool printMetadata_;
>  
> +	libcamera::FrameBufferAllocator *allocator_;
>  	std::vector<std::unique_ptr<libcamera::Request>> requests_;
>  };
>  
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index a567a7cc7653..b13230c374ad 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -371,7 +371,17 @@ int CamApp::run()
>  	if (options_.isSet(OptCapture)) {
>  		CameraSession session(camera_, config_.get());
>  		session.captureDone.connect(this, &CamApp::captureDone);
> -		return session.run(options_);
> +
> +		ret = session.start(options_);
> +		if (ret) {
> +			std::cout << "Failed to start camera session" << std::endl;
> +			return ret;
> +		}
> +
> +		loop_.exec();
> +
> +		session.stop();
> +		return 0;
>  	}
>  
>  	if (options_.isSet(OptMonitor)) {
>