capture_session: capture stop/wakeup protocol

With this change, a client (i.e., display driver) can register
a wakeup signal handler to be notified on the arrival of new data to
capture. The signal is delivered only when the client has stopped
capturing. The client propagates this condition to the server using
the new 'capture_stopped' RPC call.

This change in principle enables a display driver to suspend its
periodic mode of operation after a few frames without capturing any
new data. As the first driver, the fb_sdl driver has been adapted to
the new protocol. This change not only eliminates the driver's CPU
load when idle, it also reduces the latency of sporadic output
because the response to such GUI updates is no longer bound by a
fixed periodic interval.

Issue #5344

repos/os/include/capture_session/capture_session.h

@@ -68,6 +68,13 @@ struct Capture::Session : Genode::Session
 	 */
 	virtual void screen_size_sigh(Signal_context_capability) = 0;
 
+	/**
+	 * Register signal handler informed of new data to capture
+	 *
+	 * A wakeup signal is delivered only after a call of 'capture_stopped'.
+	 */
+	virtual void wakeup_sigh(Signal_context_capability) = 0;
+
 	enum class Buffer_result { OK, OUT_OF_RAM, OUT_OF_CAPS };
 
 	struct Buffer_attr
@@ -114,9 +121,24 @@ struct Capture::Session : Genode::Session
 	 *
 	 * \return  geometry information about the content that changed since the
 	 *          previous call of 'capture_at'
+	 *
+	 * A client should call 'capture_at' at intervals between 10 to 40 ms
+	 * (25-100 FPS). Should no change happen for more than 50 ms, the client
+	 * may stop the periodic capturing and call 'capture_stopped' once. As soon
+	 * as new changes becomes available for capturing, a wakeup signal tells
+	 * the client to resume the periodic capturing.
+	 *
+	 * The nitpicker GUI server reflects 'capture_at' calls as 'sync' signals
+	 * to its GUI clients, which thereby enables applications to synchronize
+	 * their output to the display's refresh rate.
 	 */
 	virtual Affected_rects capture_at(Point) = 0;
 
+	/**
+	 * Schedule wakeup signal
+	 */
+	virtual void capture_stopped() = 0;
+
 
 	/*********************
 	 ** RPC declaration **
@@ -124,12 +146,14 @@ struct Capture::Session : Genode::Session
 
 	GENODE_RPC(Rpc_screen_size, Area, screen_size);
 	GENODE_RPC(Rpc_screen_size_sigh, void, screen_size_sigh, Signal_context_capability);
+	GENODE_RPC(Rpc_wakeup_sigh, void, wakeup_sigh, Signal_context_capability);
 	GENODE_RPC(Rpc_buffer, Buffer_result, buffer, Buffer_attr);
 	GENODE_RPC(Rpc_dataspace, Dataspace_capability, dataspace);
 	GENODE_RPC(Rpc_capture_at, Affected_rects, capture_at, Point);
+	GENODE_RPC(Rpc_capture_stopped, void, capture_stopped);
 
-	GENODE_RPC_INTERFACE(Rpc_screen_size, Rpc_screen_size_sigh, Rpc_buffer,
-	                     Rpc_dataspace, Rpc_capture_at);
+	GENODE_RPC_INTERFACE(Rpc_screen_size, Rpc_screen_size_sigh, Rpc_wakeup_sigh,
+	                     Rpc_buffer, Rpc_dataspace, Rpc_capture_at, Rpc_capture_stopped);
 };
 
 #endif /* _INCLUDE__CAPTURE_SESSION__CAPTURE_SESSION_H_ */

repos/os/include/capture_session/connection.h

@@ -73,6 +73,11 @@ class Capture::Connection : private Genode::Connection<Session>
 			cap().call<Session::Rpc_screen_size_sigh>(sigh);
 		}
 
+		void wakeup_sigh(Signal_context_capability sigh)
+		{
+			cap().call<Session::Rpc_wakeup_sigh>(sigh);
+		}
+
 		Genode::Dataspace_capability dataspace()
 		{
 			return cap().call<Session::Rpc_dataspace>();
@@ -82,6 +87,8 @@ class Capture::Connection : private Genode::Connection<Session>
 		{
 			return cap().call<Session::Rpc_capture_at>(pos);
 		}
+
+		void capture_stopped() { cap().call<Session::Rpc_capture_stopped>(); }
 };
 
 

repos/os/src/driver/framebuffer/sdl/main.cc

@@ -69,7 +69,7 @@ struct Fb_sdl::Sdl : Noncopyable
 				.initial_size = { .w = node.attribute_value("width",  1024u),
 				                  .h = node.attribute_value("height",  768u) },
 				.fps  = node.attribute_value("fps", 60.0),
-				.idle = node.attribute_value("idle", ~0U)
+				.idle = node.attribute_value("idle", 3U)
 			};
 		}
 
@@ -329,8 +329,10 @@ void Fb_sdl::Sdl::_thread()
 			if (idle) {
 				if (_previous_frame.constructed()) {
 					_previous_frame->idle++;
-					if ((_attr.idle < ~0u) && (_previous_frame->idle > _attr.idle))
-						_previous_frame.destruct(); /* stop capturing */
+					if (_previous_frame->idle > _attr.idle) {
+						_previous_frame.destruct();
+						_capture.capture_stopped();
+					}
 				}
 			} else {
 				_schedule_next_frame();
@@ -465,9 +467,15 @@ struct Fb_sdl::Main
 			warning("SDL_PushEvent failed (", Cstring(SDL_GetError()), ")");
 	}
 
+	Signal_handler<Main> _capture_wakeup_handler {
+		_env.ep(), *this, &Main::_handle_capture_wakeup };
+
 	Sdl _sdl { _event, _capture, _env.rm(), Sdl::Attr::from_xml(_config.xml()) };
 
-	Main(Env &env) : _env(env) { }
+	Main(Env &env) : _env(env)
+	{
+		_capture.wakeup_sigh(_capture_wakeup_handler);
+	}
 };
 
 

repos/os/src/server/black_hole/capture.h

@@ -67,6 +67,8 @@ class Capture::Session_component : public Session_object<Capture::Session>
 
 		void screen_size_sigh(Signal_context_capability) override { }
 
+		void wakeup_sigh(Signal_context_capability) override { }
+
 		Buffer_result buffer(Buffer_attr attr) override
 		{
 			if (attr.px.count() == 0) {
@@ -94,6 +96,8 @@ class Capture::Session_component : public Session_object<Capture::Session>
 		{
 			return Affected_rects();
 		}
+
+		void capture_stopped() override { }
 };
 
 

repos/os/src/server/nitpicker/capture_session.h

@@ -50,10 +50,22 @@ class Nitpicker::Capture_session : public Session_object<Capture::Session>
 
 		Signal_context_capability _screen_size_sigh { };
 
+		Signal_context_capability _wakeup_sigh { };
+
+		bool _stopped = false;
+
 		using Dirty_rect = Genode::Dirty_rect<Rect, Affected_rects::NUM_RECTS>;
 
 		Dirty_rect _dirty_rect { };
 
+		void _wakeup_if_needed()
+		{
+			if (_stopped && !_dirty_rect.empty() && _wakeup_sigh.valid()) {
+				Signal_transmitter(_wakeup_sigh).submit();
+				_stopped = false;
+			}
+		}
+
 	public:
 
 		Capture_session(Env              &env,
@@ -84,6 +96,7 @@ class Nitpicker::Capture_session : public Session_object<Capture::Session>
 		void mark_as_damaged(Rect rect)
 		{
 			_dirty_rect.mark_as_dirty(rect);
+			_wakeup_if_needed();
 		}
 
 		void screen_size_changed()
@@ -104,6 +117,12 @@ class Nitpicker::Capture_session : public Session_object<Capture::Session>
 			_screen_size_sigh = sigh;
 		}
 
+		void wakeup_sigh(Signal_context_capability sigh) override
+		{
+			_wakeup_sigh = sigh;
+			_wakeup_if_needed();
+		}
+
 		Buffer_result buffer(Buffer_attr const attr) override
 		{
 			Buffer_result result = Buffer_result::OK;
@@ -123,6 +142,10 @@ class Nitpicker::Capture_session : public Session_object<Capture::Session>
 			catch (Out_of_caps) { result = Buffer_result::OUT_OF_CAPS; }
 
 			_handler.capture_buffer_size_changed();
+
+			/* report complete buffer as dirty on next call of 'capture_at' */
+			mark_as_damaged({ { 0, 0 }, attr.px });
+
 			return result;
 		}
 
@@ -160,6 +183,14 @@ class Nitpicker::Capture_session : public Session_object<Capture::Session>
 
 			return affected;
 		}
+
+		void capture_stopped() override
+		{
+			_stopped = true;
+
+			/* dirty pixels may be pending */
+			_wakeup_if_needed();
+		}
 };
 
 #endif /* _CAPTURE_SESSION_H_ */

repos/os/src/test/framebuffer/main.cc

@@ -19,6 +19,7 @@
 #include <base/log.h>
 #include <os/static_root.h>
 #include <capture_session/capture_session.h>
+#include <timer_session/connection.h>
 #include <base/attached_ram_dataspace.h>
 #include <util/reconstructible.h>
 
@@ -53,15 +54,38 @@ struct Test::Capture_session : Rpc_object<Capture::Session>
 
 	State _state = STRIPES;
 
-	unsigned long _sync_cnt = 0;
-
-	enum { FRAME_CNT = 200 };
-
 	void _draw();
-
 	void _draw_frame(Pixel *, Pixel, Area);
 
-	Capture_session(Env &env) : _env(env) { }
+	bool _dirty           = false; /* true if there is data not yet delivered */
+	bool _capture_stopped = false;
+
+	Signal_context_capability _wakeup_sigh { };
+
+	void _wakeup_if_needed()
+	{
+		if (_capture_stopped && _dirty && _wakeup_sigh.valid()) {
+			Signal_transmitter(_wakeup_sigh).submit();
+			_capture_stopped = false;
+		}
+	}
+
+	Timer::Connection _timer { _env };
+
+	Signal_handler<Capture_session> _timer_handler {
+		_env.ep(), *this, &Capture_session::_handle_timer };
+
+	void _handle_timer()
+	{
+		_dirty = true;
+		_wakeup_if_needed();
+	}
+
+	Capture_session(Env &env) : _env(env)
+	{
+		_timer.sigh(_timer_handler);
+		_timer.trigger_periodic(1000*1000);
+	}
 
 
 	/********************************
@@ -72,6 +96,8 @@ struct Test::Capture_session : Rpc_object<Capture::Session>
 
 	void screen_size_sigh(Signal_context_capability) override { }
 
+	void wakeup_sigh(Signal_context_capability sigh) override { _wakeup_sigh = sigh; }
+
 	Buffer_result buffer(Buffer_attr attr) override
 	{
 		try {
@@ -93,14 +119,15 @@ struct Test::Capture_session : Rpc_object<Capture::Session>
 	Affected_rects capture_at(Point) override
 	{
 		Affected_rects affected { };
-
-		if (_sync_cnt++ % FRAME_CNT == 0) {
+		if (_dirty) {
 			_draw();
-			affected.rects[0] = Rect(Point(0, 0), _size);
+			affected.rects[0] = Rect { { }, _size };
+			_dirty = false;
 		}
-
 		return affected;
 	}
+
+	void capture_stopped() override { _capture_stopped = true; }
 };