[{"id":13835,"web_url":"https://patchwork.libcamera.org/comment/13835/","msgid":"<38b82efd-a00f-bc4c-063c-d7cec2046956@ideasonboard.com>","date":"2020-11-23T21:35:08","subject":"Re: [libcamera-devel] [PATCH 3/3] simple-cam: Provide event-loop\n\tbacked by libevent","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Umang,\n\nOn 19/11/2020 12:34, 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\nHrm, \"simple\" cam is starting to look not so simple ...\nbut that's not your fault.\n\nThe only thing holding me back on this patch is the idea that perhaps it\nmight be 'simpler' (in the context of the actual libcamera parts) if the\nevent handling was separated.\n\nBut handling the events is somewhat now critical anyway.\n\nI guess the only way to separate the event handling, and still have it\nall as one file would be to put the event handling in it's own class\n(and the implementation inline in the class definition) to keep that all\nseparate.\n\nI suspect that might get bulky too though.\n\nI'm tempted to say we just merge this and fix the compilation faults,\nand consider later if we have better ways to re-simplify it again.\n\nThoughts on a post-card anyone?\n\nOtherwise, I'll likely merge to get things working again tomorrow.\n--\nKieran.\n\n\n\n\n> Signed-off-by: Umang Jain <email@uajain.com>\n> ---\n>  meson.build    |   1 +\n>  simple-cam.cpp | 100 +++++++++++++++++++++++++++++++++++++++++++------\n>  2 files changed, 90 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>  ]\n>  \n>  cpp_arguments = [ '-Wno-unused-parameter', ]\n> diff --git a/simple-cam.cpp b/simple-cam.cpp\n> index 65e0f14..7fbb890 100644\n> --- a/simple-cam.cpp\n> +++ b/simple-cam.cpp\n> @@ -8,12 +8,57 @@\n>  #include <iomanip>\n>  #include <iostream>\n>  #include <memory>\n> +#include <mutex>\n> +\n> +#include <event2/event.h>\n> +#include <event2/thread.h>\n>  \n>  #include <libcamera/libcamera.h>\n>  \n> +#define TIMEOUT_SEC 3\n> +\n>  using namespace libcamera;\n>  std::shared_ptr<Camera> camera;\n>  \n> +/*\n> + * --------------------------------------------------------------------\n> + * Helper functions to interact and control the event loop.\n> + */\n> +bool capture = true;\n> +std::mutex lock;\n> +std::list<std::function<void()>> calls_;\n> +struct event_base *event;\n> +\n> +void interrupt()\n> +{\n> +\tevent_base_loopbreak(event);\n> +}\n> +\n> +void callLater(const std::function<void()> &func)\n> +{\n> +\t{\n> +\t\tstd::unique_lock<std::mutex> locker(lock);\n> +\t\tcalls_.push_back(func);\n> +\t}\n> +\n> +\tinterrupt();\n> +}\n> +\n> +void dispatchCalls()\n> +{\n> +\tstd::unique_lock<std::mutex> locker(lock);\n> +\n> +\tfor (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> +\t\tstd::function<void()> 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 +66,26 @@ std::shared_ptr<Camera> 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 +118,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 +388,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 +415,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;\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 26B92BE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 23 Nov 2020 21:35:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8BDCE633F2;\n\tMon, 23 Nov 2020 22:35:12 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 378C8631D0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 23 Nov 2020 22:35:11 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B6A0271;\n\tMon, 23 Nov 2020 22:35:10 +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=\"IbjJLG/N\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606167310;\n\tbh=S7YSQ3WTqBv7Qk1gdDbHqoB0NPBXJhpnCZB81pb6NHY=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=IbjJLG/NhnrVe83UI/D30ptYmZQ2cdn/gPLNIAUrm98RQaYm+Osgk+uFzFY1m1/bS\n\txR2rf7RVYUv6CobbyFr0ZWGz6rNpboYOoW+f3NN40i279l5rc+SFxXVb3stxdu+NFv\n\tPZFj+9qrFPQhl7wddNK9mArHrhtLmndc9ti9qYyg=","To":"Umang Jain <email@uajain.com>, libcamera-devel@lists.libcamera.org","References":"<20201119123427.53864-1-email@uajain.com>\n\t<20201119123427.53864-4-email@uajain.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<38b82efd-a00f-bc4c-063c-d7cec2046956@ideasonboard.com>","Date":"Mon, 23 Nov 2020 21:35:08 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201119123427.53864-4-email@uajain.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 3/3] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13837,"web_url":"https://patchwork.libcamera.org/comment/13837/","msgid":"<206e4566-e170-842a-c701-a4b09fd8984e@uajain.com>","date":"2020-11-24T05:58:17","subject":"Re: [libcamera-devel] [PATCH 3/3] simple-cam: Provide event-loop\n\tbacked by libevent","submitter":{"id":1,"url":"https://patchwork.libcamera.org/api/people/1/","name":"Umang Jain","email":"email@uajain.com"},"content":"Hi Kieran\n\nOn 11/24/20 3:05 AM, Kieran Bingham wrote:\n> Hi Umang,\n>\n> On 19/11/2020 12:34, 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> Hrm, \"simple\" cam is starting to look not so simple ...\n> but that's not your fault.\n>\n> The only thing holding me back on this patch is the idea that perhaps it\n> might be 'simpler' (in the context of the actual libcamera parts) if the\n> event handling was separated.\n>\n> But handling the events is somewhat now critical anyway.\n>\n> I guess the only way to separate the event handling, and still have it\n> all as one file would be to put the event handling in it's own class\n> (and the implementation inline in the class definition) to keep that all\n> separate.\n>\n> I suspect that might get bulky too though.\nI hear you. I actually started out this, with a \"EventLoop\" class that \nwraps the libevent specifics, similar to cam. But indeed, it turned out \nto be bulkier hence, I went the minimalistic libevent implementation way \n(without a wrapper) and still I don't like it 100%.\n>\n> I'm tempted to say we just merge this and fix the compilation faults,\n> and consider later if we have better ways to re-simplify it again.\n>\n> Thoughts on a post-card anyone?\n>\n> Otherwise, I'll likely merge to get things working again tomorrow.\n> --\n> Kieran.\n>\n>\n>\n>\n>> Signed-off-by: Umang Jain <email@uajain.com>\n>> ---\n>>   meson.build    |   1 +\n>>   simple-cam.cpp | 100 +++++++++++++++++++++++++++++++++++++++++++------\n>>   2 files changed, 90 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>>   ]\n>>   \n>>   cpp_arguments = [ '-Wno-unused-parameter', ]\n>> diff --git a/simple-cam.cpp b/simple-cam.cpp\n>> index 65e0f14..7fbb890 100644\n>> --- a/simple-cam.cpp\n>> +++ b/simple-cam.cpp\n>> @@ -8,12 +8,57 @@\n>>   #include <iomanip>\n>>   #include <iostream>\n>>   #include <memory>\n>> +#include <mutex>\n>> +\n>> +#include <event2/event.h>\n>> +#include <event2/thread.h>\n>>   \n>>   #include <libcamera/libcamera.h>\n>>   \n>> +#define TIMEOUT_SEC 3\n>> +\n>>   using namespace libcamera;\n>>   std::shared_ptr<Camera> camera;\n>>   \n>> +/*\n>> + * --------------------------------------------------------------------\n>> + * Helper functions to interact and control the event loop.\n>> + */\n>> +bool capture = true;\n>> +std::mutex lock;\n>> +std::list<std::function<void()>> calls_;\n>> +struct event_base *event;\n>> +\n>> +void interrupt()\n>> +{\n>> +\tevent_base_loopbreak(event);\n>> +}\n>> +\n>> +void callLater(const std::function<void()> &func)\n>> +{\n>> +\t{\n>> +\t\tstd::unique_lock<std::mutex> locker(lock);\n>> +\t\tcalls_.push_back(func);\n>> +\t}\n>> +\n>> +\tinterrupt();\n>> +}\n>> +\n>> +void dispatchCalls()\n>> +{\n>> +\tstd::unique_lock<std::mutex> locker(lock);\n>> +\n>> +\tfor (auto iter = calls_.begin(); iter != calls_.end(); ) {\n>> +\t\tstd::function<void()> 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 +66,26 @@ std::shared_ptr<Camera> 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 +118,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 +388,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 +415,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;\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A704BBE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 24 Nov 2020 05:58:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1E70E633F6;\n\tTue, 24 Nov 2020 06:58:24 +0100 (CET)","from mail.uajain.com (static.126.159.217.95.clients.your-server.de\n\t[95.217.159.126])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 67B6E60331\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 24 Nov 2020 06:58:22 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=uajain.com header.i=@uajain.com\n\theader.b=\"fln8aING\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=uajain.com; s=mail;\n\tt=1606197501; bh=jmj6VFlds050cB21yOzkNBM4U+ai0A7svYVvRSeXmgc=;\n\th=Subject:To:References:From:In-Reply-To;\n\tb=fln8aINGk6oeeVj8joJm+d8jUjDFlGAV1Lik2DeXXjfLmgaT2cXb7qwT66BH1vfV4\n\tublSxCviN9BIQiDvtgQtG6WAuz/GLdXa26skIXmcfgvuCqZ20I4x8nGMGSMaVfxXBW\n\t7VUsFgThyD3tq+PXTeMm0Acoe0Pix350TvQFwqgUk1VnYFA88yB71goC+/rV94c0wL\n\tzdn+hTgJ+DnUIJxZKYUJr7vGHGNKsTZK/ca/x8INYe7lHpXWr+5PpW+pIXDUsqEJme\n\tEZ+srsrIwvyH2H5v72Z/tIhDU0ROdOZ1D2Mnofc420Il8No3LmI1UiHD1dWrV6fFd7\n\t8B4p76RELnByA==","To":"kieran.bingham@ideasonboard.com, libcamera-devel@lists.libcamera.org","References":"<20201119123427.53864-1-email@uajain.com>\n\t<20201119123427.53864-4-email@uajain.com>\n\t<38b82efd-a00f-bc4c-063c-d7cec2046956@ideasonboard.com>","From":"Umang Jain <email@uajain.com>","Message-ID":"<206e4566-e170-842a-c701-a4b09fd8984e@uajain.com>","Date":"Tue, 24 Nov 2020 11:28:17 +0530","Mime-Version":"1.0","In-Reply-To":"<38b82efd-a00f-bc4c-063c-d7cec2046956@ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 3/3] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13850,"web_url":"https://patchwork.libcamera.org/comment/13850/","msgid":"<20201124144723.GI4592@pendragon.ideasonboard.com>","date":"2020-11-24T14:47:23","subject":"Re: [libcamera-devel] [PATCH 3/3] 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 Kieran,\n\nOn Mon, Nov 23, 2020 at 09:35:08PM +0000, Kieran Bingham wrote:\n> Hi Umang,\n> \n> On 19/11/2020 12:34, 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> \n> Hrm, \"simple\" cam is starting to look not so simple ...\n> but that's not your fault.\n> \n> The only thing holding me back on this patch is the idea that perhaps it\n> might be 'simpler' (in the context of the actual libcamera parts) if the\n> event handling was separated.\n> \n> But handling the events is somewhat now critical anyway.\n> \n> I guess the only way to separate the event handling, and still have it\n> all as one file would be to put the event handling in it's own class\n> (and the implementation inline in the class definition) to keep that all\n> separate.\n> \n> I suspect that might get bulky too though.\n> \n> I'm tempted to say we just merge this and fix the compilation faults,\n> and consider later if we have better ways to re-simplify it again.\n> \n> Thoughts on a post-card anyone?\n> \n> Otherwise, I'll likely merge to get things working again tomorrow.\n\nsimple-cam should remain as simple as possible. It demonstrates the bare\nminimum to write an application. The event loop implementation is thus\nout of scope of the \"tutorial\" part of simple-cam, but it's nonetheless\nrequired as we really should not provide sample code that processes\nrequest completion synchronously.\n\nIn practice, most applications will use a GUI toolkit (Qt, gtk, ...),\nwhich will provide an event loop. We don't want to specialize simple-cam\nfor one particular toolkit (otherwise it will turn into qcam and won't\nbe simple anymore), hence the need for a custom event loop. I'd\nrecommend splitting all the code that is considered to be out-of-scope\nand provided by toolkits and frameworks to separate classes and separate\nfiles, to avoid it being in the way. The simple-cam.cpp file should have\na short comment explaining why an event loop is needed, that it is meant\nto be provided by the application toolkit/framework, and that a minimal\nimplementation based on libevent is used here.\n\nI'm fine merging this and improving on top, but I think we should split\nevent handling to a separate class and file.\n\n> > Signed-off-by: Umang Jain <email@uajain.com>\n> > ---\n> >  meson.build    |   1 +\n> >  simple-cam.cpp | 100 +++++++++++++++++++++++++++++++++++++++++++------\n> >  2 files changed, 90 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> >  ]\n> >  \n> >  cpp_arguments = [ '-Wno-unused-parameter', ]\n> > diff --git a/simple-cam.cpp b/simple-cam.cpp\n> > index 65e0f14..7fbb890 100644\n> > --- a/simple-cam.cpp\n> > +++ b/simple-cam.cpp\n> > @@ -8,12 +8,57 @@\n> >  #include <iomanip>\n> >  #include <iostream>\n> >  #include <memory>\n> > +#include <mutex>\n> > +\n> > +#include <event2/event.h>\n> > +#include <event2/thread.h>\n> >  \n> >  #include <libcamera/libcamera.h>\n> >  \n> > +#define TIMEOUT_SEC 3\n> > +\n> >  using namespace libcamera;\n> >  std::shared_ptr<Camera> camera;\n> >  \n> > +/*\n> > + * --------------------------------------------------------------------\n> > + * Helper functions to interact and control the event loop.\n> > + */\n> > +bool capture = true;\n> > +std::mutex lock;\n> > +std::list<std::function<void()>> calls_;\n> > +struct event_base *event;\n> > +\n> > +void interrupt()\n> > +{\n> > +\tevent_base_loopbreak(event);\n> > +}\n> > +\n> > +void callLater(const std::function<void()> &func)\n> > +{\n> > +\t{\n> > +\t\tstd::unique_lock<std::mutex> locker(lock);\n> > +\t\tcalls_.push_back(func);\n> > +\t}\n> > +\n> > +\tinterrupt();\n> > +}\n> > +\n> > +void dispatchCalls()\n> > +{\n> > +\tstd::unique_lock<std::mutex> locker(lock);\n> > +\n> > +\tfor (auto iter = calls_.begin(); iter != calls_.end(); ) {\n> > +\t\tstd::function<void()> 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 +66,26 @@ std::shared_ptr<Camera> 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 +118,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 +388,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 +415,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":"<libcamera-devel-bounces@lists.libcamera.org>","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 CA01CBE176\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 24 Nov 2020 14:47:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5AFC663402;\n\tTue, 24 Nov 2020 15:47:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F041563343\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 24 Nov 2020 15:47:30 +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 80F97292;\n\tTue, 24 Nov 2020 15:47:30 +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=\"KEkEGmfa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1606229250;\n\tbh=m7phcNguYF9gnXIELGU11ITRHDmMuFV6SFfSkuvBDHQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KEkEGmfaRGWP7OC3Qp80Rp/VBu++HraiqDVQicowyI9TTf7oPCzKnB1kR8ZG69SNm\n\tyrMJC3dygkWecmX1rNv53qcihh70JC+kRtyFTeSmzrtd3sd18tnuEJnZeqCVVYIMoV\n\t96XAe+W/Ci/sKvjhmPIxFRj5tvgQMFE/OwYGNIr4=","Date":"Tue, 24 Nov 2020 16:47:23 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20201124144723.GI4592@pendragon.ideasonboard.com>","References":"<20201119123427.53864-1-email@uajain.com>\n\t<20201119123427.53864-4-email@uajain.com>\n\t<38b82efd-a00f-bc4c-063c-d7cec2046956@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<38b82efd-a00f-bc4c-063c-d7cec2046956@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/3] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}}]