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

[Laurent Pinchart]

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 at 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

-- 
Regards,

Laurent Pinchart