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

[Umang Jain]

Hi Kieran

On 11/24/20 3:05 AM, Kieran Bingham wrote:
> Hi Umang,
>
> On 19/11/2020 12:34, 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.
>>
> Hrm, "simple" cam is starting to look not so simple ...
> but that's not your fault.
>
> The only thing holding me back on this patch is the idea that perhaps it
> might be 'simpler' (in the context of the actual libcamera parts) if the
> event handling was separated.
>
> But handling the events is somewhat now critical anyway.
>
> I guess the only way to separate the event handling, and still have it
> all as one file would be to put the event handling in it's own class
> (and the implementation inline in the class definition) to keep that all
> separate.
>
> I suspect that might get bulky too though.
I hear you. I actually started out this, with a "EventLoop" class that 
wraps the libevent specifics, similar to cam. But indeed, it turned out 
to be bulkier hence, I went the minimalistic libevent implementation way 
(without a wrapper) and still I don't like it 100%.
>
> I'm tempted to say we just merge this and fix the compilation faults,
> and consider later if we have better ways to re-simplify it again.
>
> Thoughts on a post-card anyone?
>
> Otherwise, I'll likely merge to get things working again tomorrow.
> --
> Kieran.
>
>
>
>
>> Signed-off-by: Umang Jain <email at uajain.com>
>> ---
>>   meson.build    |   1 +
>>   simple-cam.cpp | 100 +++++++++++++++++++++++++++++++++++++++++++------
>>   2 files changed, 90 insertions(+), 11 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index c312f2c..0eca0a1 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -14,6 +14,7 @@ src_files = files([
>>   # libcamera install path camera.pc file ($prefix/lib/pkgconfig/camera.pc)
>>   libcamera_deps = [
>>         dependency('camera', required : true),
>> +      dependency('libevent_pthreads', required : true),
>>   ]
>>   
>>   cpp_arguments = [ '-Wno-unused-parameter', ]
>> diff --git a/simple-cam.cpp b/simple-cam.cpp
>> index 65e0f14..7fbb890 100644
>> --- a/simple-cam.cpp
>> +++ b/simple-cam.cpp
>> @@ -8,12 +8,57 @@
>>   #include <iomanip>
>>   #include <iostream>
>>   #include <memory>
>> +#include <mutex>
>> +
>> +#include <event2/event.h>
>> +#include <event2/thread.h>
>>   
>>   #include <libcamera/libcamera.h>
>>   
>> +#define TIMEOUT_SEC 3
>> +
>>   using namespace libcamera;
>>   std::shared_ptr<Camera> camera;
>>   
>> +/*
>> + * --------------------------------------------------------------------
>> + * Helper functions to interact and control the event loop.
>> + */
>> +bool capture = true;
>> +std::mutex lock;
>> +std::list<std::function<void()>> calls_;
>> +struct event_base *event;
>> +
>> +void interrupt()
>> +{
>> +	event_base_loopbreak(event);
>> +}
>> +
>> +void callLater(const std::function<void()> &func)
>> +{
>> +	{
>> +		std::unique_lock<std::mutex> locker(lock);
>> +		calls_.push_back(func);
>> +	}
>> +
>> +	interrupt();
>> +}
>> +
>> +void 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();
>> +	}
>> +}
>> +
>>   /*
>>    * --------------------------------------------------------------------
>>    * Handle RequestComplete
>> @@ -21,13 +66,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;
>>   
>> +	callLater(std::bind(&processRequest, request));
>> +}
>> +
>> +static void processRequest(Request *request)
>> +{
>>   	const Request::BufferMap &buffers = request->buffers();
>>   
>>   	for (auto bufferPair : buffers) {
>> @@ -60,6 +118,15 @@ static void requestComplete(Request *request)
>>   	camera->queueRequest(request);
>>   }
>>   
>> +void timeoutTriggered(int fd, short event, void *arg)
>> +{
>> +	capture = false;
>> +	interrupt();
>> +
>> +	std::cout << "Capture ran for " << TIMEOUT_SEC << " seconds, Exiting."
>> +		  << std::endl;
>> +}
>> +
>>   int main()
>>   {
>>   	/*
>> @@ -321,18 +388,25 @@ 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.
>> +	 * simple-cam uses a base event loop provided by libevent, which is run
>> +	 * for TIMEOUT_SEC seconds after which, the event loop is interrupted
>> +	 * via interrupt() and begins the cleanup. The interval for which the
>> +	 * event loop is being run, completed requests are processed via
>> +	 * requestComplete() handler.
>>   	 */
>> -	EventDispatcher *dispatcher = cm->eventDispatcher();
>> -	Timer timer;
>> -	timer.start(3000);
>> -	while (timer.isRunning())
>> -		dispatcher->processEvents();
>> +	struct event *ev;
>> +	struct timeval tv;
>> +
>> +	tv.tv_sec = TIMEOUT_SEC;
>> +	tv.tv_usec = 0;
>> +	evthread_use_pthreads();
>> +	event = event_base_new();
>> +	ev = event_new(event, -1, EV_TIMEOUT, &timeoutTriggered, NULL);
>> +	event_add(ev, &tv);
>> +	while (capture) {
>> +		dispatchCalls();
>> +		event_base_loop(event, EVLOOP_NO_EXIT_ON_EMPTY);
>> +	}
>>   
>>   	/*
>>   	 * --------------------------------------------------------------------
>> @@ -341,6 +415,10 @@ int main()
>>   	 * Stop the Camera, release resources and stop the CameraManager.
>>   	 * Libcamera has now released all resources it owned.
>>   	 */
>> +	event_free(ev);
>> +	event_base_free(event);
>> +	libevent_global_shutdown();
>> +
>>   	camera->stop();
>>   	allocator->free(stream);
>>   	delete allocator;
>>