[{"id":14022,"web_url":"https://patchwork.libcamera.org/comment/14022/","msgid":"<20201201182455.GC3085@pendragon.ideasonboard.com>","date":"2020-12-01T18:24:55","subject":"Re: [libcamera-devel] [PATCH v2 4/4] simple-cam: Provide event-loop\n\tbacked by libevent","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Tue, Dec 01, 2020 at 11:13:14PM +0530, Umang Jain wrote:\n> libcamera moved its EventDispatcher and Timer API to its internal API,\n> since providing an event loop to applications should not be the job of\n> libcamera. Application utility like cam, were ported to use libevent,\n> hence inspired from that, un-break simple-cam by using the similar\n> implementation to replace the EventDispatcher and Timer functionality\n> by libevent.\n> \n> Signed-off-by: Umang Jain \n> ---\n> meson.build | 1 +\n> simple-cam.cpp | 101 +++++++++++++++++++++++++++++++++++++++++++------\n> 2 files changed, 91 insertions(+), 11 deletions(-)\n> \n> diff --git a/meson.build b/meson.build\n> index c312f2c..0eca0a1 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -14,6 +14,7 @@ src_files = files([\n> # libcamera install path camera.pc file ($prefix/lib/pkgconfig/camera.pc)\n> libcamera_deps = [\n> dependency('camera', required : true),\n> + dependency('libevent_pthreads', required : true),\n\nYou could drop required : true as that's the default.\n\n> ]\n> \n> cpp_arguments = [ '-Wno-unused-parameter', ]\n> diff --git a/simple-cam.cpp b/simple-cam.cpp\n> index 0e26895..dd785fc 100644\n> --- a/simple-cam.cpp\n> +++ b/simple-cam.cpp\n> @@ -8,12 +8,58 @@\n> #include \n> #include \n> #include \n> +#include \n> +\n> +#include \n> +#include \n\nCould you move the implementation of the event loop to a separate class,\nin a separate file ? The idea is to avoid cluttering simple-cam.cpp with\ncode that is out of scope for libcamera, leaving it as simple as\npossible. Feel free to copy the files from the cam application, with the\nminimum amount of changes to add other features simple-cam may need (I'm\nthinking in particular about a new function to register a timer, which\nshould probably just take a timeout parameter and an std::function).\n\n> \n> #include \n> \n> +#define TIMEOUT_SEC 3\n> +\n> using namespace libcamera;\n> std::shared_ptr camera;\n> \n> +/*\n> + * --------------------------------------------------------------------\n> + * Helper functions to interact and control the event loop.\n> + * \\todo Split event loop handling to a separate class and file.\n> + */\n> +bool capture = true;\n> +std::mutex lock;\n> +std::list> calls_;\n> +struct event_base *event;\n> +\n> +void interrupt()\n> +{\n> +\tevent_base_loopbreak(event);\n> +}\n> +\n> +void callLater(const std::function &func)\n> +{\n> +\t{\n> +\t\tstd::unique_lock locker(lock);\n> +\t\tcalls_.push_back(func);\n> +\t}\n> +\n> +\tinterrupt();\n> +}\n> +\n> +void dispatchCalls()\n> +{\n> +\tstd::unique_lock locker(lock);\n> +\n> +\tfor (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> +\t\tstd::function call = std::move(*iter);\n> +\n> +\t\titer = calls_.erase(iter);\n> +\n> +\t\tlocker.unlock();\n> +\t\tcall();\n> +\t\tlocker.lock();\n> +\t}\n> +}\n> +\n> /*\n> * --------------------------------------------------------------------\n> * Handle RequestComplete\n> @@ -21,13 +67,26 @@ std::shared_ptr camera;\n> * For each Camera::requestCompleted Signal emitted from the Camera the\n> * connected Slot is invoked.\n> *\n> + * The Slot is invoked in the CameraManager's thread, hence one should avoid\n> + * any heavy processing here. The processing of the request shall be re-directed\n> + * to the application's thread instead, so as not to block the CameraManager's\n> + * thread for large amount of time.\n> + *\n> * The Slot receives the Request as a parameter.\n> */\n> +\n> +static void processRequest(Request *request);\n> +\n> static void requestComplete(Request *request)\n> {\n> \tif (request->status() == Request::RequestCancelled)\n> \t\treturn;\n> \n> +\tcallLater(std::bind(&processRequest, request));\n> +}\n> +\n> +static void processRequest(Request *request)\n> +{\n> \tconst Request::BufferMap &buffers = request->buffers();\n> \n> \tfor (auto bufferPair : buffers) {\n> @@ -60,6 +119,15 @@ static void requestComplete(Request *request)\n> \tcamera->queueRequest(request);\n> }\n> \n> +void timeoutTriggered(int fd, short event, void *arg)\n> +{\n> +\tcapture = false;\n> +\tinterrupt();\n> +\n> +\tstd::cout << \"Capture ran for \" << TIMEOUT_SEC << \" seconds, Exiting.\"\n> +\t\t << std::endl;\n> +}\n> +\n> int main()\n> {\n> \t/*\n> @@ -321,18 +389,25 @@ int main()\n> \t * In order to dispatch events received from the video devices, such\n> \t * as buffer completions, an event loop has to be run.\n> \t *\n> -\t * Libcamera provides its own default event dispatcher realized by\n> -\t * polling a set of file descriptors, but applications can integrate\n> -\t * their own even loop with the Libcamera EventDispatcher.\n> -\t *\n> -\t * Here, as an example, run the poll-based EventDispatcher for 3\n> -\t * seconds.\n> +\t * simple-cam uses a base event loop provided by libevent, which is run\n> +\t * for TIMEOUT_SEC seconds after which, the event loop is interrupted\n> +\t * via interrupt() and begins the cleanup. The interval for which the\n> +\t * event loop is being run, completed requests are processed via\n> +\t * requestComplete() handler.\n> \t */\n> -\tEventDispatcher *dispatcher = cm->eventDispatcher();\n> -\tTimer timer;\n> -\ttimer.start(3000);\n> -\twhile (timer.isRunning())\n> -\t\tdispatcher->processEvents();\n> +\tstruct event *ev;\n> +\tstruct timeval tv;\n> +\n> +\ttv.tv_sec = TIMEOUT_SEC;\n> +\ttv.tv_usec = 0;\n> +\tevthread_use_pthreads();\n> +\tevent = event_base_new();\n> +\tev = event_new(event, -1, EV_TIMEOUT, &timeoutTriggered, NULL);\n> +\tevent_add(ev, &tv);\n> +\twhile (capture) {\n> +\t\tdispatchCalls();\n> +\t\tevent_base_loop(event, EVLOOP_NO_EXIT_ON_EMPTY);\n> +\t}\n> \n> \t/*\n> \t * --------------------------------------------------------------------\n> @@ -341,6 +416,10 @@ int main()\n> \t * Stop the Camera, release resources and stop the CameraManager.\n> \t * Libcamera has now released all resources it owned.\n> \t */\n> +\tevent_free(ev);\n> +\tevent_base_free(event);\n> +\tlibevent_global_shutdown();\n> +\n> \tcamera->stop();\n> \tallocator->free(stream);\n> \tdelete allocator;","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 81DD4BE176\n\tfor ;\n\tTue, 1 Dec 2020 18:25:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 141A963501;\n\tTue, 1 Dec 2020 19:25:06 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6745263460\n\tfor ;\n\tTue, 1 Dec 2020 19:25:04 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DCA6DDBD;\n\tTue, 1 Dec 2020 19:25:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"I1MSP/+W\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606847104;\n\tbh=rvwuZiJuBUFQUYq61WMAxjQmRDch/dXf+LR5GGcLJ80=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=I1MSP/+WJhFjFAJFOqFw1G88T6QX69KcHVjK738gL4B4t8WUiwTjKs3o3H2f2nK1t\n\tE/JYNQ80m94h9e1+jL/NXaxqMw89nubKYvF6tfaXnP8Xc9PtqH1HNguECh41LYvYRh\n\t8Q1aYGtUsDUNcLcazv6hVB/NApIe9hDXxJnjXPmw=","Date":"Tue, 1 Dec 2020 20:24:55 +0200","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"<20201201182455.GC3085@pendragon.ideasonboard.com>","References":"<20201201174314.12774-1-email@uajain.com>\n\t<20201201174314.12774-5-email@uajain.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201201174314.12774-5-email@uajain.com>","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] simple-cam: Provide event-loop\n\tbacked by libevent","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]