[libcamera-devel,v3,4/4] simple-cam: Provide event-loop backed by libevent

10556 diff mbox series

libcamera moved its EventDispatcher and Timer API to its internal API,
since providing an event loop to applications should not be the job of
libcamera. Application utility like cam, were ported to use libevent,
hence inspired from that, un-break simple-cam by using the similar
implementation to replace the EventDispatcher and Timer functionality
by libevent.

Signed-off-by: Umang Jain <email@uajain.com>
---
 event_loop.cpp | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
 event_loop.h   |  44 +++++++++++++++++++++
 meson.build    |   2 +
 simple-cam.cpp |  35 ++++++++++-------
 4 files changed, 171 insertions(+), 13 deletions(-)
 create mode 100644 event_loop.cpp
 create mode 100644 event_loop.h

Hi reviewers,

On 12/4/20 12:24 PM, Umang Jain wrote:
> libcamera moved its EventDispatcher and Timer API to its internal API,
> since providing an event loop to applications should not be the job of
> libcamera. Application utility like cam, were ported to use libevent,
> hence inspired from that, un-break simple-cam by using the similar
> implementation to replace the EventDispatcher and Timer functionality
> by libevent.
>
> Signed-off-by: Umang Jain <email@uajain.com>
> ---
>   event_loop.cpp | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
>   event_loop.h   |  44 +++++++++++++++++++++
>   meson.build    |   2 +
>   simple-cam.cpp |  35 ++++++++++-------
>   4 files changed, 171 insertions(+), 13 deletions(-)
>   create mode 100644 event_loop.cpp
>   create mode 100644 event_loop.h
>
> diff --git a/event_loop.cpp b/event_loop.cpp
> new file mode 100644
> index 0000000..f40a635
> --- /dev/null
> +++ b/event_loop.cpp
> @@ -0,0 +1,103 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * event_loop.cpp - Event loop based on cam
> + */
> +
> +#include "event_loop.h"
> +
> +#include <assert.h>
> +#include <event2/event.h>
> +#include <event2/thread.h>
> +
> +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();
> +}
> +
> +int EventLoop::exec()
> +{
> +	exitCode_ = -1;
> +	exit_.store(false, std::memory_order_release);
> +
> +	while (!exit_.load(std::memory_order_acquire)) {
> +		dispatchCalls();
> +		event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY);
> +	}
> +
> +	return exitCode_;
> +}
> +
> +void EventLoop::exit(int code)
> +{
> +	exitCode_ = code;
> +	exit_.store(true, std::memory_order_release);
> +	interrupt();
> +}
> +
> +void EventLoop::interrupt()
> +{
> +	event_base_loopbreak(event_);
> +}
> +
> +void EventLoop::timeoutTriggered()
> +{
> +	exit();
> +}
> +
> +void timeoutCb(int fd, short event, void *arg)
> +{
> +	EventLoop *ptr = static_cast<EventLoop *>(arg);
> +	ptr->timeoutTriggered();
> +}
This is a bit special. A non-member function acting as proxy callback to 
the EventLoop::timeout(). I didn't find a way to pass a member function, 
apparently it seems impossible because you need a object to invoke the 
member function upon. Digging a bit more into this practice, I found 
this particular issue's solution/recommendation here:
https://isocpp.org/wiki/faq/pointers-to-members#memfnptr-vs-fnptr
> +
> +void EventLoop::timeout(unsigned int sec)
> +{
> +	struct event *ev;
> +	struct timeval tv;
> +
> +	tv.tv_sec = sec;
> +	tv.tv_usec = 0;
> +	ev = evtimer_new(event_, &timeoutCb, this);
> +	evtimer_add(ev, &tv);
> +}
> +
> +void EventLoop::callLater(const std::function<void()> &func)
> +{
> +	{
> +		std::unique_lock<std::mutex> locker(lock_);
> +		calls_.push_back(func);
> +	}
> +
> +	interrupt();
> +}
> +
> +void EventLoop::dispatchCalls()
> +{
> +	std::unique_lock<std::mutex> locker(lock_);
> +
> +	for (auto iter = calls_.begin(); iter != calls_.end(); ) {
> +		std::function<void()> call = std::move(*iter);
> +
> +		iter = calls_.erase(iter);
> +
> +		locker.unlock();
> +		call();
> +		locker.lock();
> +	}
> +}
> diff --git a/event_loop.h b/event_loop.h
> new file mode 100644
> index 0000000..fcb964f
> --- /dev/null
> +++ b/event_loop.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * event_loop.h - Event loop based on cam's
> + */
> +#ifndef __SIMPLE_CAM_EVENT_LOOP_H__
> +#define __SIMPLE_CAM_EVENT_LOOP_H__
> +
> +#include <atomic>
> +#include <functional>
> +#include <list>
> +#include <mutex>
> +
> +struct event_base;
> +
> +class EventLoop
> +{
> +public:
> +	EventLoop();
> +	~EventLoop();
> +
> +	void exit(int code = 0);
> +	int exec();
> +
> +	void timeout(unsigned int sec);
> +	void timeoutTriggered();
> +	void callLater(const std::function<void()> &func);
> +
> +private:
> +	static EventLoop *instance_;
> +
> +	struct event_base *event_;
> +	std::atomic<bool> exit_;
> +	int exitCode_;
> +
> +	std::list<std::function<void()>> calls_;
> +	std::mutex lock_;
> +
> +	void interrupt();
> +	void dispatchCalls();
> +};
> +
> +#endif /* __SIMPLE_CAM_EVENT_LOOP_H__ */
> diff --git a/meson.build b/meson.build
> index c312f2c..4d580c2 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -8,12 +8,14 @@ project('simple-cam', 'c', 'cpp',
>   # simple-cam.cpp is the fully commented application
>   src_files = files([
>   	'simple-cam.cpp',
> +	'event_loop.cpp',
>   ])
>   
>   # Point your PKG_CONFIG_PATH environment variable to the
>   # libcamera install path camera.pc file ($prefix/lib/pkgconfig/camera.pc)
>   libcamera_deps = [
>         dependency('camera', required : true),
> +      dependency('libevent_pthreads'),
>   ]
>   
>   cpp_arguments = [ '-Wno-unused-parameter', ]
> diff --git a/simple-cam.cpp b/simple-cam.cpp
> index bfe30d7..6d1d84f 100644
> --- a/simple-cam.cpp
> +++ b/simple-cam.cpp
> @@ -11,8 +11,13 @@
>   
>   #include <libcamera/libcamera.h>
>   
> +#include "event_loop.h"
> +
> +#define TIMEOUT_SEC 3
> +
>   using namespace libcamera;
>   std::shared_ptr<Camera> camera;
> +EventLoop loop;
>   
>   /*
>    * --------------------------------------------------------------------
> @@ -21,13 +26,26 @@ std::shared_ptr<Camera> camera;
>    * For each Camera::requestCompleted Signal emitted from the Camera the
>    * connected Slot is invoked.
>    *
> + * The Slot is invoked in the CameraManager's thread, hence one should avoid
> + * any heavy processing here. The processing of the request shall be re-directed
> + * to the application's thread instead, so as not to block the CameraManager's
> + * thread for large amount of time.
> + *
>    * The Slot receives the Request as a parameter.
>    */
> +
> +static void processRequest(Request *request);
> +
>   static void requestComplete(Request *request)
>   {
>   	if (request->status() == Request::RequestCancelled)
>   		return;
>   
> +	loop.callLater(std::bind(&processRequest, request));
> +}
> +
> +static void processRequest(Request *request)
> +{
>   	const Request::BufferMap &buffers = request->buffers();
>   
>   	for (auto bufferPair : buffers) {
> @@ -320,20 +338,11 @@ int main()
>   	 *
>   	 * In order to dispatch events received from the video devices, such
>   	 * as buffer completions, an event loop has to be run.
> -	 *
> -	 * Libcamera provides its own default event dispatcher realized by
> -	 * polling a set of file descriptors, but applications can integrate
> -	 * their own even loop with the Libcamera EventDispatcher.
> -	 *
> -	 * Here, as an example, run the poll-based EventDispatcher for 3
> -	 * seconds.
>   	 */
> -	EventDispatcher *dispatcher = cm->eventDispatcher();
> -	Timer timer;
> -	timer.start(3000);
> -	while (timer.isRunning())
> -		dispatcher->processEvents();
> -
> +	loop.timeout(TIMEOUT_SEC);
> +	int ret = loop.exec();
> +	std::cout << "Capture ran for " << TIMEOUT_SEC << " seconds and "
> +		  << "stopped with exit status: " << ret << std::endl;
>   	/*
>   	 * --------------------------------------------------------------------
>   	 * Clean Up

Hi Umang,

On Fri, Dec 04, 2020 at 12:30:10PM +0530, Umang Jain wrote:
> Hi reviewers,
> 
> On 12/4/20 12:24 PM, Umang Jain wrote:
> > libcamera moved its EventDispatcher and Timer API to its internal API,
> > since providing an event loop to applications should not be the job of
> > libcamera. Application utility like cam, were ported to use libevent,
> > hence inspired from that, un-break simple-cam by using the similar
> > implementation to replace the EventDispatcher and Timer functionality
> > by libevent.
> >
> > Signed-off-by: Umang Jain <email@uajain.com>
> > ---
> >   event_loop.cpp | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
> >   event_loop.h   |  44 +++++++++++++++++++++
> >   meson.build    |   2 +
> >   simple-cam.cpp |  35 ++++++++++-------
> >   4 files changed, 171 insertions(+), 13 deletions(-)
> >   create mode 100644 event_loop.cpp
> >   create mode 100644 event_loop.h
> >
> > diff --git a/event_loop.cpp b/event_loop.cpp
> > new file mode 100644
> > index 0000000..f40a635
> > --- /dev/null
> > +++ b/event_loop.cpp
> > @@ -0,0 +1,103 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * event_loop.cpp - Event loop based on cam
> > + */
> > +
> > +#include "event_loop.h"
> > +
> > +#include <assert.h>
> > +#include <event2/event.h>
> > +#include <event2/thread.h>
> > +
> > +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();
> > +}
> > +
> > +int EventLoop::exec()
> > +{
> > +	exitCode_ = -1;
> > +	exit_.store(false, std::memory_order_release);
> > +
> > +	while (!exit_.load(std::memory_order_acquire)) {
> > +		dispatchCalls();
> > +		event_base_loop(event_, EVLOOP_NO_EXIT_ON_EMPTY);
> > +	}
> > +
> > +	return exitCode_;
> > +}
> > +
> > +void EventLoop::exit(int code)
> > +{
> > +	exitCode_ = code;
> > +	exit_.store(true, std::memory_order_release);
> > +	interrupt();
> > +}
> > +
> > +void EventLoop::interrupt()
> > +{
> > +	event_base_loopbreak(event_);
> > +}
> > +
> > +void EventLoop::timeoutTriggered()
> > +{
> > +	exit();
> > +}
> > +
> > +void timeoutCb(int fd, short event, void *arg)
> > +{
> > +	EventLoop *ptr = static_cast<EventLoop *>(arg);
> > +	ptr->timeoutTriggered();
> > +}
>
> This is a bit special. A non-member function acting as proxy callback to 
> the EventLoop::timeout(). I didn't find a way to pass a member function, 
> apparently it seems impossible because you need a object to invoke the 
> member function upon. Digging a bit more into this practice, I found 
> this particular issue's solution/recommendation here:
> https://isocpp.org/wiki/faq/pointers-to-members#memfnptr-vs-fnptr

This is correct, you can't pass a non-static member function, but you
can pass a static member function. This could thus be written

void EventLoop::timeoutTriggered(int fd, short event, void *arg)
{
	EventLoop *self = static_cast<EventLoop *>(arg);
	self->exit();
}

with the function declared as

	static void timeoutTriggered(int fd, short event, void *arg);

in the private section of the EventLoop class.

> > +
> > +void EventLoop::timeout(unsigned int sec)
> > +{
> > +	struct event *ev;
> > +	struct timeval tv;
> > +
> > +	tv.tv_sec = sec;
> > +	tv.tv_usec = 0;
> > +	ev = evtimer_new(event_, &timeoutCb, this);
> > +	evtimer_add(ev, &tv);
> > +}
> > +
> > +void EventLoop::callLater(const std::function<void()> &func)
> > +{
> > +	{
> > +		std::unique_lock<std::mutex> locker(lock_);
> > +		calls_.push_back(func);
> > +	}
> > +
> > +	interrupt();
> > +}
> > +
> > +void EventLoop::dispatchCalls()
> > +{
> > +	std::unique_lock<std::mutex> locker(lock_);
> > +
> > +	for (auto iter = calls_.begin(); iter != calls_.end(); ) {
> > +		std::function<void()> call = std::move(*iter);
> > +
> > +		iter = calls_.erase(iter);
> > +
> > +		locker.unlock();
> > +		call();
> > +		locker.lock();
> > +	}
> > +}
> > diff --git a/event_loop.h b/event_loop.h
> > new file mode 100644
> > index 0000000..fcb964f
> > --- /dev/null
> > +++ b/event_loop.h
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * event_loop.h - Event loop based on cam's
> > + */
> > +#ifndef __SIMPLE_CAM_EVENT_LOOP_H__
> > +#define __SIMPLE_CAM_EVENT_LOOP_H__
> > +
> > +#include <atomic>
> > +#include <functional>
> > +#include <list>
> > +#include <mutex>
> > +
> > +struct event_base;
> > +
> > +class EventLoop
> > +{
> > +public:
> > +	EventLoop();
> > +	~EventLoop();
> > +
> > +	void exit(int code = 0);
> > +	int exec();
> > +
> > +	void timeout(unsigned int sec);
> > +	void timeoutTriggered();
> > +	void callLater(const std::function<void()> &func);
> > +
> > +private:
> > +	static EventLoop *instance_;
> > +
> > +	struct event_base *event_;
> > +	std::atomic<bool> exit_;
> > +	int exitCode_;
> > +
> > +	std::list<std::function<void()>> calls_;
> > +	std::mutex lock_;
> > +
> > +	void interrupt();
> > +	void dispatchCalls();
> > +};
> > +
> > +#endif /* __SIMPLE_CAM_EVENT_LOOP_H__ */
> > diff --git a/meson.build b/meson.build
> > index c312f2c..4d580c2 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -8,12 +8,14 @@ project('simple-cam', 'c', 'cpp',
> >   # simple-cam.cpp is the fully commented application
> >   src_files = files([
> >   	'simple-cam.cpp',
> > +	'event_loop.cpp',
> >   ])
> >   
> >   # Point your PKG_CONFIG_PATH environment variable to the
> >   # libcamera install path camera.pc file ($prefix/lib/pkgconfig/camera.pc)
> >   libcamera_deps = [
> >         dependency('camera', required : true),
> > +      dependency('libevent_pthreads'),
> >   ]
> >   
> >   cpp_arguments = [ '-Wno-unused-parameter', ]
> > diff --git a/simple-cam.cpp b/simple-cam.cpp
> > index bfe30d7..6d1d84f 100644
> > --- a/simple-cam.cpp
> > +++ b/simple-cam.cpp
> > @@ -11,8 +11,13 @@
> >   
> >   #include <libcamera/libcamera.h>
> >   
> > +#include "event_loop.h"
> > +
> > +#define TIMEOUT_SEC 3
> > +
> >   using namespace libcamera;
> >   std::shared_ptr<Camera> camera;
> > +EventLoop loop;
> >   
> >   /*
> >    * --------------------------------------------------------------------
> > @@ -21,13 +26,26 @@ std::shared_ptr<Camera> camera;
> >    * For each Camera::requestCompleted Signal emitted from the Camera the
> >    * connected Slot is invoked.
> >    *
> > + * The Slot is invoked in the CameraManager's thread, hence one should avoid
> > + * any heavy processing here. The processing of the request shall be re-directed
> > + * to the application's thread instead, so as not to block the CameraManager's
> > + * thread for large amount of time.
> > + *
> >    * The Slot receives the Request as a parameter.
> >    */
> > +
> > +static void processRequest(Request *request);
> > +
> >   static void requestComplete(Request *request)
> >   {
> >   	if (request->status() == Request::RequestCancelled)
> >   		return;
> >   
> > +	loop.callLater(std::bind(&processRequest, request));
> > +}
> > +
> > +static void processRequest(Request *request)
> > +{
> >   	const Request::BufferMap &buffers = request->buffers();
> >   
> >   	for (auto bufferPair : buffers) {
> > @@ -320,20 +338,11 @@ int main()
> >   	 *
> >   	 * In order to dispatch events received from the video devices, such
> >   	 * as buffer completions, an event loop has to be run.
> > -	 *
> > -	 * Libcamera provides its own default event dispatcher realized by
> > -	 * polling a set of file descriptors, but applications can integrate
> > -	 * their own even loop with the Libcamera EventDispatcher.
> > -	 *
> > -	 * Here, as an example, run the poll-based EventDispatcher for 3
> > -	 * seconds.
> >   	 */
> > -	EventDispatcher *dispatcher = cm->eventDispatcher();
> > -	Timer timer;
> > -	timer.start(3000);
> > -	while (timer.isRunning())
> > -		dispatcher->processEvents();
> > -
> > +	loop.timeout(TIMEOUT_SEC);
> > +	int ret = loop.exec();
> > +	std::cout << "Capture ran for " << TIMEOUT_SEC << " seconds and "
> > +		  << "stopped with exit status: " << ret << std::endl;
> >   	/*
> >   	 * --------------------------------------------------------------------
> >   	 * Clean Up