[libcamera-devel] [PATCH v1 2/5] cam: Use libevent to implement event loop

[Kieran Bingham]

On 13/11/2020 06:38, Laurent Pinchart wrote:
> To prepare for removal of the EventDispatcher from the libcamera public
> API, switch to libevent to handle the event loop.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/cam/event_loop.cpp | 34 +++++++++++++++++++++++++++-------
>  src/cam/event_loop.h   | 15 ++++++++-------
>  src/cam/main.cpp       | 16 +++++-----------
>  src/cam/meson.build    | 13 ++++++++++++-
>  4 files changed, 52 insertions(+), 26 deletions(-)
> 
> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
> index e8ab861790ed..13f2583da0a1 100644
> --- a/src/cam/event_loop.cpp
> +++ b/src/cam/event_loop.cpp
> @@ -5,19 +5,34 @@
>   * event_loop.cpp - cam - Event loop
>   */
>  
> -#include <libcamera/event_dispatcher.h>
> -
>  #include "event_loop.h"
>  
> -using namespace libcamera;
> +#include <assert.h>
> +#include <event2/event.h>
> +#include <event2/thread.h>
>  
> -EventLoop::EventLoop(EventDispatcher *dispatcher)
> -	: dispatcher_(dispatcher)
> +EventLoop *EventLoop::instance_ = nullptr;
> +
> +EventLoop::EventLoop()
>  {
> +	assert(!instance_);
> +
> +	evthread_use_pthreads();
> +	event_ = event_base_new();
> +	instance_ = this;
>  }
>  
>  EventLoop::~EventLoop()
>  {
> +	instance_ = nullptr;
> +
> +	event_base_free(event_);
> +	libevent_global_shutdown();
> +}
> +
> +EventLoop *EventLoop::instance()
> +{
> +	return instance_;
>  }
>  
>  int EventLoop::exec()
> @@ -26,7 +41,7 @@ int EventLoop::exec()
>  	exit_.store(false, std::memory_order_release);
>  
>  	while (!exit_.load(std::memory_order_acquire))
> -		dispatcher_->processEvents();
> +		event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY);
>  
>  	return exitCode_;
>  }
> @@ -35,5 +50,10 @@ void EventLoop::exit(int code)
>  {
>  	exitCode_ = code;
>  	exit_.store(true, std::memory_order_release);
> -	dispatcher_->interrupt();
> +	interrupt();
> +}
> +
> +void EventLoop::interrupt()
> +{
> +	event_base_loopbreak(event_);
>  }
> diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
> index 581c7cba2fc4..b1c6bd103080 100644
> --- a/src/cam/event_loop.h
> +++ b/src/cam/event_loop.h
> @@ -9,26 +9,27 @@
>  
>  #include <atomic>
>  
> -#include <libcamera/event_notifier.h>
> -
> -namespace libcamera {
> -class EventDispatcher;
> -}
> +struct event_base;
>  
>  class EventLoop
>  {
>  public:
> -	EventLoop(libcamera::EventDispatcher *dispatcher);
> +	EventLoop();
>  	~EventLoop();
>  
> +	static EventLoop *instance();
> +
>  	int exec();
>  	void exit(int code = 0);
>  
>  private:
> -	libcamera::EventDispatcher *dispatcher_;
> +	static EventLoop *instance_;
>  
> +	struct event_base *event_;
>  	std::atomic<bool> exit_;
>  	int exitCode_;

It might not have been introduced by this patch, but it seems to have
been highlighted during this rebuild. Could you check if this is a real
issue or a false positive please?


If you supply a patch to fix this issue, please add the following tag:

Reported-by: Coverity CID=305971

Alternatively, if you believe it's a false positive, let me know and
I'll close it.



Hi,

Please find the latest report on new defect(s) introduced to libcamera
found with Coverity Scan.

1 new defect(s) introduced to libcamera found with Coverity Scan.
1 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)


** CID 305971:  Uninitialized members  (UNINIT_CTOR)
/home/linuxembedded/iob/libcamera/libcamera-daily/src/cam/event_loop.cpp: 23
in EventLoop::EventLoop()()


________________________________________________________________________________________________________
*** CID 305971:  Uninitialized members  (UNINIT_CTOR)
/home/linuxembedded/iob/libcamera/libcamera-daily/src/cam/event_loop.cpp: 23
in EventLoop::EventLoop()()
17     {
18     	assert(!instance_);
19
20     	evthread_use_pthreads();
21     	event_ = event_base_new();
22     	instance_ = this;
>>>     CID 305971:  Uninitialized members  (UNINIT_CTOR)
>>>     Non-static class member "exitCode_" is not initialized in this
constructor nor in any functions that it calls.
23     }
24
25     EventLoop::~EventLoop()
26     {
27     	instance_ = nullptr;
28





> +
> +	void interrupt();
>  };
>  
>  #endif /* __CAM_EVENT_LOOP_H__ */
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index b3a8d94f5739..e01be63a7058 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -52,7 +52,7 @@ private:
>  	CameraManager *cm_;
>  	std::shared_ptr<Camera> camera_;
>  	std::unique_ptr<libcamera::CameraConfiguration> config_;
> -	EventLoop *loop_;
> +	EventLoop loop_;
>  
>  	bool strictFormats_;
>  };
> @@ -60,7 +60,7 @@ private:
>  CamApp *CamApp::app_ = nullptr;
>  
>  CamApp::CamApp()
> -	: cm_(nullptr), camera_(nullptr), config_(nullptr), loop_(nullptr),
> +	: cm_(nullptr), camera_(nullptr), config_(nullptr),
>  	  strictFormats_(false)
>  {
>  	CamApp::app_ = this;
> @@ -134,16 +134,11 @@ int CamApp::init(int argc, char **argv)
>  		std::cout << "Monitoring new hotplug and unplug events" << std::endl;
>  	}
>  
> -	loop_ = new EventLoop(cm_->eventDispatcher());
> -
>  	return 0;
>  }
>  
>  void CamApp::cleanup()
>  {
> -	delete loop_;
> -	loop_ = nullptr;
> -
>  	if (camera_) {
>  		camera_->release();
>  		camera_.reset();
> @@ -166,8 +161,7 @@ int CamApp::exec()
>  
>  void CamApp::quit()
>  {
> -	if (loop_)
> -		loop_->exit();
> +	loop_.exit();
>  }
>  
>  int CamApp::parseOptions(int argc, char *argv[])
> @@ -366,13 +360,13 @@ int CamApp::run()
>  	}
>  
>  	if (options_.isSet(OptCapture)) {
> -		Capture capture(camera_, config_.get(), loop_);
> +		Capture capture(camera_, config_.get(), &loop_);
>  		return capture.run(options_);
>  	}
>  
>  	if (options_.isSet(OptMonitor)) {
>  		std::cout << "Press Ctrl-C to interrupt" << std::endl;
> -		ret = loop_->exec();
> +		ret = loop_.exec();
>  		if (ret)
>  			std::cout << "Failed to run monitor loop" << std::endl;
>  	}
> diff --git a/src/cam/meson.build b/src/cam/meson.build
> index 89e124fbae2a..62003c58dad0 100644
> --- a/src/cam/meson.build
> +++ b/src/cam/meson.build
> @@ -1,5 +1,12 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
> +libevent = dependency('libevent_pthreads', required : false)
> +
> +if not libevent.found()
> +    warning('libevent_pthreads not found,\'cam\' application will not be compiled')
> +    subdir_done()
> +endif
> +
>  cam_sources = files([
>      'buffer_writer.cpp',
>      'capture.cpp',
> @@ -10,5 +17,9 @@ cam_sources = files([
>  ])
>  
>  cam  = executable('cam', cam_sources,
> -                  dependencies : [ libatomic, libcamera_dep ],
> +                  dependencies : [
> +                      libatomic,
> +                      libcamera_dep,
> +                      libevent,
> +                  ],
>                    install : true)
> 

-- 
Regards
--
Kieran