[libcamera-devel,v5,2/2] cam: event_loop: Add timed events to event loop
diff mbox series

Message ID 20220331130610.31806-2-ecurtin@redhat.com
State Superseded
Headers show
Series
  • [libcamera-devel,v5,1/2] cam: sdl_sink: Add SDL sink
Related show

Commit Message

Eric Curtin March 31, 2022, 1:06 p.m. UTC
If you add an event with EventLoop::Default, that event will be executed
every 10ms or at a rate of 100fps. Used to handle resize of windows and
quit events in SDL.

Signed-off-by: Eric Curtin <ecurtin@redhat.com>
---
 src/cam/event_loop.cpp | 10 +++++++++-
 src/cam/event_loop.h   |  1 +
 src/cam/sdl_sink.cpp   |  2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart April 5, 2022, 11:07 p.m. UTC | #1
Hi Eric,

Thank you for the patch.

On Thu, Mar 31, 2022 at 02:06:10PM +0100, Eric Curtin via libcamera-devel wrote:
> If you add an event with EventLoop::Default, that event will be executed
> every 10ms or at a rate of 100fps. Used to handle resize of windows and
> quit events in SDL.

Could you move this patch first in the series ? It's best to first add
the infrastructure we need, and then make use of it in the SDL sink.
This patch will thus not touch sdl_sink.cpp.

The commit message could then be updated to something like the
following:

Extend the EventLoop class to support periodic timer events. This can be
used to run tasks periodically, such as handling the event loop of SDL.

> Signed-off-by: Eric Curtin <ecurtin@redhat.com>
> ---
>  src/cam/event_loop.cpp | 10 +++++++++-
>  src/cam/event_loop.h   |  1 +
>  src/cam/sdl_sink.cpp   |  2 +-
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
> index e25784c0..7a5ed4da 100644
> --- a/src/cam/event_loop.cpp
> +++ b/src/cam/event_loop.cpp
> @@ -75,7 +75,15 @@ void EventLoop::addEvent(int fd, EventType type,
>  		return;
>  	}
>  
> -	int ret = event_add(event->event_, nullptr);
> +	struct timeval *tp = nullptr;
> +	struct timeval tv;
> +	if (events == EV_PERSIST) { /* EventType = Default */
> +		tp = &tv;
> +		tv.tv_sec = 0;
> +		tv.tv_usec = 10000; /* every 10 ms */
> +	}
> +
> +	int ret = event_add(event->event_, tp);
>  	if (ret < 0) {
>  		std::cerr << "Failed to add event for fd " << fd << std::endl;
>  		return;
> diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
> index a4613eb2..8fd9bd20 100644
> --- a/src/cam/event_loop.h
> +++ b/src/cam/event_loop.h
> @@ -20,6 +20,7 @@ class EventLoop
>  {
>  public:
>  	enum EventType {
> +		Default = 0, /* the event can be triggered only by a timeout or by manual activation */
>  		Read = 1,
>  		Write = 2,
>  	};

I think it would be nicer to add a

	void addTimerEvent(std::chrono::microseconds period);

function instead of adding a new event type. This will allow selecting
the timer period in the caller instead of hardcoding it here, and will
avoid having to pass a random fd value to the function.

> diff --git a/src/cam/sdl_sink.cpp b/src/cam/sdl_sink.cpp
> index 28e59cbb..97a550b5 100644
> --- a/src/cam/sdl_sink.cpp
> +++ b/src/cam/sdl_sink.cpp
> @@ -117,7 +117,7 @@ int SDLSink::start()
>  		return -EINVAL;
>  	}
>  
> -	EventLoop::instance()->addEvent(-1, EventLoop::Read, std::bind(&SDLSink::processSDLEvents, this));
> +	EventLoop::instance()->addEvent(-1, EventLoop::Default, std::bind(&SDLSink::processSDLEvents, this));
>  
>  	return 0;
>  }

Patch
diff mbox series

diff --git a/src/cam/event_loop.cpp b/src/cam/event_loop.cpp
index e25784c0..7a5ed4da 100644
--- a/src/cam/event_loop.cpp
+++ b/src/cam/event_loop.cpp
@@ -75,7 +75,15 @@  void EventLoop::addEvent(int fd, EventType type,
 		return;
 	}
 
-	int ret = event_add(event->event_, nullptr);
+	struct timeval *tp = nullptr;
+	struct timeval tv;
+	if (events == EV_PERSIST) { /* EventType = Default */
+		tp = &tv;
+		tv.tv_sec = 0;
+		tv.tv_usec = 10000; /* every 10 ms */
+	}
+
+	int ret = event_add(event->event_, tp);
 	if (ret < 0) {
 		std::cerr << "Failed to add event for fd " << fd << std::endl;
 		return;
diff --git a/src/cam/event_loop.h b/src/cam/event_loop.h
index a4613eb2..8fd9bd20 100644
--- a/src/cam/event_loop.h
+++ b/src/cam/event_loop.h
@@ -20,6 +20,7 @@  class EventLoop
 {
 public:
 	enum EventType {
+		Default = 0, /* the event can be triggered only by a timeout or by manual activation */
 		Read = 1,
 		Write = 2,
 	};
diff --git a/src/cam/sdl_sink.cpp b/src/cam/sdl_sink.cpp
index 28e59cbb..97a550b5 100644
--- a/src/cam/sdl_sink.cpp
+++ b/src/cam/sdl_sink.cpp
@@ -117,7 +117,7 @@  int SDLSink::start()
 		return -EINVAL;
 	}
 
-	EventLoop::instance()->addEvent(-1, EventLoop::Read, std::bind(&SDLSink::processSDLEvents, this));
+	EventLoop::instance()->addEvent(-1, EventLoop::Default, std::bind(&SDLSink::processSDLEvents, this));
 
 	return 0;
 }