From 09461c51bdffc35b9e0d482dbf16d302bb5cfdca Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Tue, 17 Sep 2024 17:29:00 +0200 Subject: [PATCH] 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 --- .../include/capture_session/capture_session.h | 28 ++++++++++- repos/os/include/capture_session/connection.h | 7 +++ repos/os/src/driver/framebuffer/sdl/main.cc | 16 +++++-- repos/os/src/server/black_hole/capture.h | 4 ++ .../os/src/server/nitpicker/capture_session.h | 31 ++++++++++++ repos/os/src/test/framebuffer/main.cc | 47 +++++++++++++++---- 6 files changed, 117 insertions(+), 16 deletions(-) diff --git a/repos/os/include/capture_session/capture_session.h b/repos/os/include/capture_session/capture_session.h index 79d309e957..4313c88b9a 100644 --- a/repos/os/include/capture_session/capture_session.h +++ b/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_ */ diff --git a/repos/os/include/capture_session/connection.h b/repos/os/include/capture_session/connection.h index b97ac73e13..f152057df0 100644 --- a/repos/os/include/capture_session/connection.h +++ b/repos/os/include/capture_session/connection.h @@ -73,6 +73,11 @@ class Capture::Connection : private Genode::Connection cap().call(sigh); } + void wakeup_sigh(Signal_context_capability sigh) + { + cap().call(sigh); + } + Genode::Dataspace_capability dataspace() { return cap().call(); @@ -82,6 +87,8 @@ class Capture::Connection : private Genode::Connection { return cap().call(pos); } + + void capture_stopped() { cap().call(); } }; diff --git a/repos/os/src/driver/framebuffer/sdl/main.cc b/repos/os/src/driver/framebuffer/sdl/main.cc index 96899928ec..d88cf5c8f6 100644 --- a/repos/os/src/driver/framebuffer/sdl/main.cc +++ b/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
_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); + } }; diff --git a/repos/os/src/server/black_hole/capture.h b/repos/os/src/server/black_hole/capture.h index 11c5cf7fc5..7e8e7e5b92 100644 --- a/repos/os/src/server/black_hole/capture.h +++ b/repos/os/src/server/black_hole/capture.h @@ -67,6 +67,8 @@ class Capture::Session_component : public Session_object 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 { return Affected_rects(); } + + void capture_stopped() override { } }; diff --git a/repos/os/src/server/nitpicker/capture_session.h b/repos/os/src/server/nitpicker/capture_session.h index cab23f1ec0..c9d4ab731a 100644 --- a/repos/os/src/server/nitpicker/capture_session.h +++ b/repos/os/src/server/nitpicker/capture_session.h @@ -50,10 +50,22 @@ class Nitpicker::Capture_session : public Session_object Signal_context_capability _screen_size_sigh { }; + Signal_context_capability _wakeup_sigh { }; + + bool _stopped = false; + using Dirty_rect = Genode::Dirty_rect; 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 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 _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 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 return affected; } + + void capture_stopped() override + { + _stopped = true; + + /* dirty pixels may be pending */ + _wakeup_if_needed(); + } }; #endif /* _CAPTURE_SESSION_H_ */ diff --git a/repos/os/src/test/framebuffer/main.cc b/repos/os/src/test/framebuffer/main.cc index 628490395a..55ef20cfca 100644 --- a/repos/os/src/test/framebuffer/main.cc +++ b/repos/os/src/test/framebuffer/main.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -53,15 +54,38 @@ struct Test::Capture_session : Rpc_object 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 _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 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 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; } };