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

[Kieran Bingham]

On 13/11/2020 10:08, Laurent Pinchart wrote:
> On Fri, Nov 13, 2020 at 09:59:02AM +0000, Kieran Bingham wrote:
>> 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>
>>
>> This is missing an update to the README.rst to update the new package
>> dependencies.
> 
> diff --git a/README.rst b/README.rst
> index 5c4b6989f7cf..251291b77c62 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -78,6 +78,9 @@ for documentation: [optional]
>  for gstreamer: [optional]
>  	libgstreamer1.0-dev libgstreamer-plugins-base1.0-dev
> 
> +for cam: [optional]
> +        libevent-dev
> +


Looks good to me ;-)

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

>  for qcam: [optional]
>  	qtbase5-dev libqt5core5a libqt5gui5 libqt5widgets5 qttools5-dev-tools libtiff-dev
> 
>> And a small whitespace nit below.
>>
>> With those:
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham 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_;
>>> +
>>> +	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')
>>
>> missing a space after comma,
>>
>>> +    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