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

Spotifyd support #13

Open
mihirlad55 opened this issue Jun 14, 2020 · 5 comments
Open

Spotifyd support #13

mihirlad55 opened this issue Jun 14, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@mihirlad55
Copy link
Owner

The program mostly works out of the box with spotifyd. Only a few additional code changes are required to support spotifyd.

The main problem with full spotifyd support is spotifyd's PropertiesChanged event does not always fire which causes the polybar modules to fail to update. This isn't a problem with polybar-spotify-module, but a problem with spotifyd itself

See the following links. It looks like the issue is actively being worked on:

@mihirlad55 mihirlad55 added the enhancement New feature or request label Jun 14, 2020
@mihirlad55 mihirlad55 self-assigned this Jul 31, 2020
@johnfromoptus
Copy link

Can you please detail the exact changes required to get it to work with spotifyd?

@mihirlad55
Copy link
Owner Author

mihirlad55 commented Oct 31, 2020

Sorry for the late reply, I've been busy with school. I played around with this a while ago. The following is a lazy patch that allows spotifyd support:

diff --git a/src/spotify-listener.c b/src/spotify-listener.c
index d058281..21ae6c0 100644
--- a/src/spotify-listener.c
+++ b/src/spotify-listener.c
@@ -265,7 +265,9 @@ DBusHandlerResult name_owner_changed_handler(DBusConnection *connection,
     }

     // If name matches spotify and new owner is "", spotify disconnected
-    if (strcmp(name, "org.mpris.MediaPlayer2.spotify") == 0 &&
+    // Use strncmp instead of strcmp, so the check covers
+    // org.mpris.MediaPlayer2.spotifyd
+    if (strncmp(name, "org.mpris.MediaPlayer2.spotify", 30) == 0 &&
         strcmp(new_owner, "") == 0) {
         puts("Spotify disconnected");
         spotify_exited();
diff --git a/src/spotifyctl.c b/src/spotifyctl.c
index 776586e..d2c3eb1 100644
--- a/src/spotifyctl.c
+++ b/src/spotifyctl.c
@@ -8,7 +8,8 @@
 #include "../include/utils.h"

 /*************** Constants for DBus ***************/
-const char *DESTINATION = "org.mpris.MediaPlayer2.spotify";
+const char *DESTINATION_SPOTIFY = "org.mpris.MediaPlayer2.spotify";
+const char *DESTINATION_SPOTIFYD = "org.mpris.MediaPlayer2.spotifyd";
 const char *PATH = "/org/mpris/MediaPlayer2";

 const char *STATUS_IFACE = "org.freedesktop.DBus.Properties";

The last time I checked, spotifyd had issues and was inconsistent with how often it announced changes on D-Bus. That's why I didn't spend too much time trying to implement spotifyd support.

Design/architecture wise, I believe the best way to implement this would be some code that abstracts a generic D-Bus player and allows choosing or automatically chooses the correct player to listen for. This also may be best implemented as part of issue #16.

@juanscr
Copy link

juanscr commented Feb 19, 2021

Using the patch from above just yielded an error, as the DESTINATION variable was hard coded somewhere else. I made a patch that works but still has the same problem of the polybar module not working with spotifyd as the issues with their Dbus implementation are still unresolved. Let's just hope they fix that soon enough.

Here is the patch if anyone is interested:

diff --git a/src/spotify-listener.c b/src/spotify-listener.c
index d058281..21ae6c0 100644
--- a/src/spotify-listener.c
+++ b/src/spotify-listener.c
@@ -265,7 +265,9 @@ DBusHandlerResult name_owner_changed_handler(DBusConnection *connection,
     }
 
     // If name matches spotify and new owner is "", spotify disconnected
-    if (strcmp(name, "org.mpris.MediaPlayer2.spotify") == 0 &&
+    // Use strncmp instead of strcmp, so the check covers
+    // org.mpris.MediaPlayer2.spotifyd
+    if (strncmp(name, "org.mpris.MediaPlayer2.spotify", 30) == 0 &&
         strcmp(new_owner, "") == 0) {
         puts("Spotify disconnected");
         spotify_exited();
diff --git a/src/spotifyctl.c b/src/spotifyctl.c
index 776586e..44bc8b1 100644
--- a/src/spotifyctl.c
+++ b/src/spotifyctl.c
@@ -8,7 +8,9 @@
 #include "../include/utils.h"
 
 /*************** Constants for DBus ***************/
-const char *DESTINATION = "org.mpris.MediaPlayer2.spotify";
+const char *DESTINATIONS[] = {"org.mpris.MediaPlayer2.spotify",
+                              "org.mpris.MediaPlayer2.spotifyd"};
+const int NUM_OF_DESTS = 2;
 const char *PATH = "/org/mpris/MediaPlayer2";
 
 const char *STATUS_IFACE = "org.freedesktop.DBus.Properties";
@@ -193,27 +195,36 @@ char *format_output(const char *artist, const char *title,
 void get_status(DBusConnection *connection, const int max_artist_length,
                 const int max_title_length, const int max_length,
                 const char *format, const char *trunc) {
-    DBusError err;
-    dbus_error_init(&err);
 
-    // Send a message requesting the properties
-    DBusMessage *msg = dbus_message_new_method_call(
-        DESTINATION, PATH, STATUS_IFACE, STATUS_METHOD);
-
-    // Message looks like this:
-    // string "org.mpris.MediaPlayer2.Player"
-    // string "Metadata"
-    dbus_message_append_args(
-        msg, DBUS_TYPE_STRING, &STATUS_METHOD_ARG_IFACE_NAME, DBUS_TYPE_STRING,
-        &STATUS_METHOD_ARG_PROPERTY_NAME, DBUS_TYPE_INVALID);
-
-    // Send and receive reply
+    int errors = 0;
+    DBusError err;
     DBusMessage *reply;
-    reply =
-        dbus_connection_send_with_reply_and_block(connection, msg, 10000, &err);
-    dbus_message_unref(msg);
-
-    if (dbus_error_is_set(&err)) {
+    for (int i = 0; i < NUM_OF_DESTS; i++) {
+        const char* destination = DESTINATIONS[i];
+        dbus_error_init(&err);
+
+        // Send a message requesting the properties
+        DBusMessage *msg = dbus_message_new_method_call(
+            destination, PATH, STATUS_IFACE, STATUS_METHOD);
+
+        // Message looks like this:
+        // string "org.mpris.MediaPlayer2.Player"
+        // string "Metadata"
+        dbus_message_append_args(
+            msg, DBUS_TYPE_STRING, &STATUS_METHOD_ARG_IFACE_NAME, DBUS_TYPE_STRING,
+            &STATUS_METHOD_ARG_PROPERTY_NAME, DBUS_TYPE_INVALID);
+
+        // Send and receive reply
+        reply =
+            dbus_connection_send_with_reply_and_block(connection, msg, 10000, &err);
+        dbus_message_unref(msg);
+
+        if (dbus_error_is_set(&err))
+            errors += 1;
+        else
+            break;
+    }
+    if (errors == NUM_OF_DESTS) {
         if (!SUPPRESS_ERRORS) fputs(err.message, stderr);
         exit(1);
     }
@@ -234,17 +245,25 @@ void get_status(DBusConnection *connection, const int max_artist_length,
 }
 
 void spotify_player_call(DBusConnection *connection, const char *method) {
+    int errors = 0;
     DBusError err;
-    dbus_error_init(&err);
+    for (int i = 0; i < NUM_OF_DESTS; i++) {
+        const char* destination = DESTINATIONS[i];
+        dbus_error_init(&err);
 
-    // Call a org.mpris.MediaPlayer2.Player method
-    DBusMessage *msg =
-        dbus_message_new_method_call(DESTINATION, PATH, PLAYER_IFACE, method);
+        // Call a org.mpris.MediaPlayer2.Player method
+        DBusMessage *msg =
+            dbus_message_new_method_call(destination, PATH, PLAYER_IFACE, method);
 
-    dbus_connection_send_with_reply_and_block(connection, msg, 10000, &err);
-    dbus_message_unref(msg);
+        dbus_connection_send_with_reply_and_block(connection, msg, 10000, &err);
+        dbus_message_unref(msg);
 
-    if (dbus_error_is_set(&err)) {
+        if (dbus_error_is_set(&err))
+            errors += 1;
+        else
+            break;
+    }
+    if (errors == NUM_OF_DESTS) {
         if (!SUPPRESS_ERRORS) fputs(err.message, stderr);
         exit(1);
     }

@francocalvo
Copy link

I can't really contribute much with the code as I don't know much C, but I'm using spotify-tui and it'd be really nice if this was added.

@julianinsua
Copy link

Same here, not a C developer. But It'd be great to get spotify-tui to work with the module

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants