search

mihirlad55 / polybar-spotify-module

A collection of lightweight programs for use with a Polybar Spotify Module
GNU General Public License v3.0
140 stars 9 forks source link

Spotifyd support #13

Open mihirlad55 opened 4 years ago

mihirlad55 commented 4 years ago

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:

johnfromoptus commented 3 years ago

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

mihirlad55 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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