[{"id":22591,"web_url":"https://patchwork.libcamera.org/comment/22591/","msgid":"","date":"2022-04-05T23:07:28","subject":"Re: [libcamera-devel] [PATCH v5 2/2] cam: event_loop: Add timed\n\tevents to event loop","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Eric,\n\nThank you for the patch.\n\nOn Thu, Mar 31, 2022 at 02:06:10PM +0100, Eric Curtin via libcamera-devel wrote:\n> If you add an event with EventLoop::Default, that event will be executed\n> every 10ms or at a rate of 100fps. Used to handle resize of windows and\n> quit events in SDL.\n\nCould you move this patch first in the series ? It's best to first add\nthe infrastructure we need, and then make use of it in the SDL sink.\nThis patch will thus not touch sdl_sink.cpp.\n\nThe commit message could then be updated to something like the\nfollowing:\n\nExtend the EventLoop class to support periodic timer events. This can be\nused to run tasks periodically, such as handling the event loop of SDL.\n\n> Signed-off-by: Eric Curtin \n> ---\n> src/cam/event_loop.cpp | 10 +++++++++-\n> src/cam/event_loop.h | 1 +\n> src/cam/sdl_sink.cpp | 2 +-\n> 3 files changed, 11 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp\n> index e25784c0..7a5ed4da 100644\n> --- a/src/cam/event_loop.cpp\n> +++ b/src/cam/event_loop.cpp\n> @@ -75,7 +75,15 @@ void EventLoop::addEvent(int fd, EventType type,\n> \t\treturn;\n> \t}\n> \n> -\tint ret = event_add(event->event_, nullptr);\n> +\tstruct timeval *tp = nullptr;\n> +\tstruct timeval tv;\n> +\tif (events == EV_PERSIST) { /* EventType = Default */\n> +\t\ttp = &tv;\n> +\t\ttv.tv_sec = 0;\n> +\t\ttv.tv_usec = 10000; /* every 10 ms */\n> +\t}\n> +\n> +\tint ret = event_add(event->event_, tp);\n> \tif (ret < 0) {\n> \t\tstd::cerr << \"Failed to add event for fd \" << fd << std::endl;\n> \t\treturn;\n> diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h\n> index a4613eb2..8fd9bd20 100644\n> --- a/src/cam/event_loop.h\n> +++ b/src/cam/event_loop.h\n> @@ -20,6 +20,7 @@ class EventLoop\n> {\n> public:\n> \tenum EventType {\n> +\t\tDefault = 0, /* the event can be triggered only by a timeout or by manual activation */\n> \t\tRead = 1,\n> \t\tWrite = 2,\n> \t};\n\nI think it would be nicer to add a\n\n\tvoid addTimerEvent(std::chrono::microseconds period);\n\nfunction instead of adding a new event type. This will allow selecting\nthe timer period in the caller instead of hardcoding it here, and will\navoid having to pass a random fd value to the function.\n\n> diff --git a/src/cam/sdl_sink.cpp b/src/cam/sdl_sink.cpp\n> index 28e59cbb..97a550b5 100644\n> --- a/src/cam/sdl_sink.cpp\n> +++ b/src/cam/sdl_sink.cpp\n> @@ -117,7 +117,7 @@ int SDLSink::start()\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\tEventLoop::instance()->addEvent(-1, EventLoop::Read, std::bind(&SDLSink::processSDLEvents, this));\n> +\tEventLoop::instance()->addEvent(-1, EventLoop::Default, std::bind(&SDLSink::processSDLEvents, this));\n> \n> \treturn 0;\n> }","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 734D8C3256\n\tfor ;\n\tTue, 5 Apr 2022 23:07:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B715965642;\n\tWed, 6 Apr 2022 01:07:33 +0200 (CEST)","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 592C2633A5\n\tfor ;\n\tWed, 6 Apr 2022 01:07:32 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D8671482;\n\tWed, 6 Apr 2022 01:07:31 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649200053;\n\tbh=WHKVow+L+8fghz9pqzJpV/cBlAN21CvtvPiiAGSot3w=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=anVcyHYe1FgickVzBOqOlNokVS4h2SJ03MHuzpuywJLq3S700V0fJteGDNUmyLal4\n\tg7qXy13/+kHs0LiuEzRNef/2S3ii5xPnq4KR42O5qfgmSCX+NH0i5t8b/bY9d5HRBz\n\tY0IaMjaPW3MaIoBFfoZ2R4y9Ll3EECdRjD2/Eo/LpXBvbBslOSLtYofURjivC2wkGP\n\td+adTgi+dJOCrdFownLBdn+e+dqt/gbUY5feG2oqZFwHnKh+N3tWo6sCv2yqAHjz8Q\n\tmJGM0YCkEn1TLyyGTP1KmkfpoBbaa3tIrHXbYDO18PdT33BKT06vwl5kP48a4LEA6d\n\tvpnYcTdkxW70g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649200052;\n\tbh=WHKVow+L+8fghz9pqzJpV/cBlAN21CvtvPiiAGSot3w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZezmteJ/WV6E7FkbRG9MAYErPuukpCAX6iJLjFfM3bSQKWSbnAvw6JdafWFfcwg8R\n\tBj5jHsp6d9k/nxa0SJ5N3+DXKXcsGTjSVIzyrOij0Ifhs6JUl6KyOpoQG5wEJsvmMc\n\tftt6yq+YQTyhhXSNM52W5LmL5Klmiqa/Ux0jo9pI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ZezmteJ/\"; dkim-atps=neutral","Date":"Wed, 6 Apr 2022 02:07:28 +0300","To":"Eric Curtin ","Message-ID":"","References":"<20220331130610.31806-1-ecurtin@redhat.com>\n\t<20220331130610.31806-2-ecurtin@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220331130610.31806-2-ecurtin@redhat.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/2] cam: event_loop: Add timed\n\tevents to event loop","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]