Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dbus-crossroads backend does not immediately apply updates #48

Open
Beinsezii opened this issue Feb 5, 2024 · 4 comments
Open

dbus-crossroads backend does not immediately apply updates #48

Beinsezii opened this issue Feb 5, 2024 · 4 comments
Labels
bug Something isn't working linux Issues related to MPRIS

Comments

@Beinsezii
Copy link

The best way I can describe this is that frequent updates to the state will get queued but not actually applied until it's read

So for example, if you run an application containing something like

loop {
  controls.set_playback(MediaPlayback::Playing {progress: position});
  thread::sleep(Duration::from_secs(1));
}

for 10 seconds, then call watch -t 1 playerctl position, you'd expect it to look like
11.0
12.0
13.0
but what you actually get is
1.0
2.0
3.0

And if you shorten the watch duration to a smaller poll rate watch -t 0.5 playerctl position, you'd expect
11.0
11.0
12.0
12.0
because the app updates every second, but in reality it's
1.0
2.0
3.0
4.0
again, as it's working through a bunch of 'queued' updates I guess.

As a bonus kicker, the timeout is either extremely long or nonexistent as I can call playerctl position minutes after playback has stopped and it'll still have dozens or hundreds of updates 'queued' before I can read its true state.

The use_zbus backend does not demonstrate this issue, and shows accurate states even with hundreds of metadata updates a second.

I have basically no knowledge of how D-Bus operates, so if I can provide more information let me know.

  • MPRIS controller: playerctld 2.4.1
  • System dbus: dbus 1.14.10
  • Souvlaki: souvlaki = "0.7.3"
@Sinono3 Sinono3 added bug Something isn't working linux Issues related to MPRIS labels Mar 2, 2024
@Sinono3
Copy link
Owner

Sinono3 commented Mar 2, 2024

Okay. I think this is because we use a MPSC channel internally (std::sync::mpsc) and we check for these messages using if let instead of while let. This causes us to only get one message every 500 ms which is very bad. We can fix this in two ways:

  1. The quick way is changing that if let for a while let, which I think should work. If it blocks the thread for too long we might have to find a way to make it non-blocking.
  2. Switching our thread-shared state management to use Arc<Mutex<T>> or Arc<RwLock<T>>. The switch shouldn't be difficult.

Can you try doing one of this fixes on a local copy of souvlaki and see if it works?

@Sinono3 Sinono3 modified the milestone: v0.8 Mar 3, 2024
@Beinsezii
Copy link
Author

Why is conn.process() set to 1000ms by default? In fact why does it even need a duration?

Setting conn.process(Duration::from_millis(0))?; lets me read playback position in real-time like on zbus and so far there's no adverse side effects.

@Beinsezii
Copy link
Author

I also don't get the whole loop{if let} vs while let thing. Shouldn't they be functionally equivalent? Where's the 500ms come from?

@Beinsezii
Copy link
Author

Beinsezii commented Mar 9, 2024

Okay I see what you meant with while let.

diff --git a/src/platform/mpris/dbus/controls.rs b/src/platform/mpris/dbus/controls.rs
index 6aa5123..a59e228 100644
--- a/src/platform/mpris/dbus/controls.rs
+++ b/src/platform/mpris/dbus/controls.rs
@@ -242,10 +242,10 @@ where
         }),
     );
 
-    loop {
-        if let Ok(event) = event_channel.recv_timeout(Duration::from_millis(10)) {
+    'outer: loop {
+        while let Ok(event) = event_channel.recv_timeout(Duration::from_millis(10)) {
             if event == InternalEvent::Kill {
-                break;
+                break 'outer;
             }
 
             let mut changed_properties = HashMap::new();

To consume all the events at once before processing. That also works. I still find the 1000ms wild though.

jacksongoode added a commit to jpochyla/psst that referenced this issue Jul 6, 2024
* Bump all packages, move to zbus for Linux (see Sinono3/souvlaki#48)

* Windows fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working linux Issues related to MPRIS
Projects
None yet
Development

No branches or pull requests

2 participants