From f593f17ca3703a868fe0f38f80fbdd29eee8ca3f Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Wed, 29 Apr 2020 03:20:14 -0400 Subject: [PATCH 01/11] implement a shift feature for playerctld This implements the feature suggested in #170 --- playerctl/playerctl-daemon.c | 56 +++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index f8b184c..32791d0 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -332,9 +332,32 @@ static void context_remove_player(struct PlayerctldContext *ctx, struct Player * } } +static void context_shift_active_player(struct PlayerctldContext *ctx) { + GError *error = NULL; + struct Player *p; + + p = context_get_active_player(ctx); + context_remove_player(ctx, p); + context_add_player(ctx, p); + context_emit_active_player_changed(ctx, &error); + + p = context_get_active_player(ctx); + player_update_position_sync(p, ctx, &error); + if (error != NULL) { + g_warning("could not emit active player change: %s", error->message); + g_clear_error(&error); + } + if (error != NULL) { + g_warning("could not update player position: %s", error->message); + g_clear_error(&error); + } +} + static const char *playerctld_introspection_xml = "\n" " \n" + " \n" + " \n" " \n" " \n" " \n" @@ -484,6 +507,31 @@ static void player_method_call_proxy_callback(GDBusConnection *connection, const g_object_unref(message); } +static void playerctld_method_call_func(GDBusConnection *connection, const char *sender, + const char *object_path, const char *interface_name, + const char *method_name, GVariant *parameters, + GDBusMethodInvocation *invocation, + gpointer user_data) { + g_debug("got method call: sender=%s, object_path=%s, interface_name=%s, method_name=%s", sender, + object_path, interface_name, method_name); + struct PlayerctldContext *ctx = user_data; + struct Player *active_player = context_get_active_player(ctx); + if (active_player == NULL) { + g_debug("no active player, returning error"); + g_dbus_method_invocation_return_dbus_error( + invocation, "com.github.altdesktop.playerctld.NoActivePlayer", + "No player is being controlled by playerctld"); + return; + } + + if (strcmp(method_name, "Shift")) { + g_dbus_method_invocation_return_dbus_error( + invocation, "com.github.altdesktop.playerctld.InvalidMethod", + "This method is not valid"); + } + context_shift_active_player(ctx); +} + static GVariant *playerctld_get_property_func(GDBusConnection *connection, const gchar *sender, const gchar *object_path, const gchar *interface_name, const gchar *property_name, GError **error, @@ -502,7 +550,7 @@ static GDBusInterfaceVTable vtable_player = {player_method_call_proxy_callback, static GDBusInterfaceVTable vtable_root = {player_method_call_proxy_callback, NULL, NULL, {0}}; -static GDBusInterfaceVTable vtable_playerctld = {NULL, playerctld_get_property_func, NULL, {0}}; +static GDBusInterfaceVTable vtable_playerctld = {playerctld_method_call_func, playerctld_get_property_func, NULL, {0}}; static void on_bus_acquired(GDBusConnection *connection, const char *name, gpointer user_data) { GError *error = NULL; @@ -737,6 +785,12 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha } if (player != context_get_active_player(ctx)) { + GVariantDict dict; + g_variant_dict_init(&dict, player->player_properties); + GVariant *prop_variant = g_variant_dict_lookup_value(&dict, "PlaybackStatus", G_VARIANT_TYPE_STRING); + const gchar *status = g_variant_get_string(prop_variant, NULL); + if (!!g_strcmp0(status, "Playing")) + return; g_debug("new active player: %s", player->well_known); context_set_active_player(ctx, player); player_update_position_sync(player, ctx, &error); From b2676b45f54cec809fb4fca6b819a71247f9669b Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Thu, 30 Apr 2020 04:24:03 -0400 Subject: [PATCH 02/11] ShiftPlayPause and proper cycling --- playerctl/playerctl-daemon.c | 131 +++++++++++++++++++++++++++++------ 1 file changed, 108 insertions(+), 23 deletions(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index 32791d0..36b9b2b 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -332,17 +332,71 @@ static void context_remove_player(struct PlayerctldContext *ctx, struct Player * } } -static void context_shift_active_player(struct PlayerctldContext *ctx) { +static void playerctl_play_pause_player(struct PlayerctldContext *ctx, struct Player *p, bool play) { GError *error = NULL; - struct Player *p; - p = context_get_active_player(ctx); - context_remove_player(ctx, p); - context_add_player(ctx, p); - context_emit_active_player_changed(ctx, &error); + g_dbus_connection_call_sync(ctx->connection, p->unique, MPRIS_PATH, + PLAYER_INTERFACE, play ? "Play" : "Pause", + NULL, NULL, + G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); + if (error != NULL) { + g_warning("%s", error->message); + g_clear_error(&error); + } +} + +static void context_shift_active_player(struct PlayerctldContext *ctx, int play_pause) { + guint len; + gboolean can_play = true; + GError *error = NULL; + GVariant *prop; + GVariantDict dict_from, dict_to; + struct Player *from, *to = NULL; + + len = g_queue_get_length(ctx->players); + + from = context_get_active_player(ctx); + + for (guint i = 1; i < len && !to; i++) { + to = g_queue_peek_nth(ctx->players, i); + if (strcmp(from->unique, to->unique) == 0) + continue; + + g_variant_dict_init(&dict_to, to->player_properties); + prop = g_variant_dict_lookup_value(&dict_to, "CanPlay", G_VARIANT_TYPE_BOOLEAN); + can_play = true; + if (prop) { + can_play = g_variant_get_boolean(prop); + g_variant_unref(prop); + } - p = context_get_active_player(ctx); - player_update_position_sync(p, ctx, &error); + if (!can_play) { + to = NULL; + } + } + + if (to == NULL) + return; + + context_remove_player(ctx, from); + context_add_player(ctx, from); + context_set_active_player(ctx, to); + + if (play_pause) { + g_variant_dict_init(&dict_from, from->player_properties); + prop = g_variant_dict_lookup_value(&dict_from, "PlaybackStatus", G_VARIANT_TYPE_STRING); + if (prop) { + const gchar *status = g_variant_get_string(prop, NULL); + g_variant_unref(prop); + if (g_strcmp0(status, "Playing") == 0) { + playerctl_play_pause_player(ctx, from, false); + playerctl_play_pause_player(ctx, to, true); + } + } + } + + context_emit_active_player_changed(ctx, &error); + player_update_position_sync(to, ctx, &error); if (error != NULL) { g_warning("could not emit active player change: %s", error->message); g_clear_error(&error); @@ -358,6 +412,8 @@ static const char *playerctld_introspection_xml = " \n" " \n" " \n" + " \n" + " \n" " \n" " \n" " \n" @@ -524,12 +580,17 @@ static void playerctld_method_call_func(GDBusConnection *connection, const char return; } - if (strcmp(method_name, "Shift")) { + if (strcmp(method_name, "Shift") == 0) { + context_shift_active_player(ctx, 0); + g_dbus_method_invocation_return_value(invocation, NULL); + } else if (strcmp(method_name, "ShiftPlayPause") == 0) { + context_shift_active_player(ctx, 1); + g_dbus_method_invocation_return_value(invocation, NULL); + } else { g_dbus_method_invocation_return_dbus_error( invocation, "com.github.altdesktop.playerctld.InvalidMethod", "This method is not valid"); } - context_shift_active_player(ctx); } static GVariant *playerctld_get_property_func(GDBusConnection *connection, const gchar *sender, @@ -787,10 +848,13 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha if (player != context_get_active_player(ctx)) { GVariantDict dict; g_variant_dict_init(&dict, player->player_properties); - GVariant *prop_variant = g_variant_dict_lookup_value(&dict, "PlaybackStatus", G_VARIANT_TYPE_STRING); - const gchar *status = g_variant_get_string(prop_variant, NULL); - if (!!g_strcmp0(status, "Playing")) - return; + GVariant *prop = g_variant_dict_lookup_value(&dict, "PlaybackStatus", G_VARIANT_TYPE_STRING); + if (prop) { + const gchar *status = g_variant_get_string(prop, NULL); + g_variant_unref(prop); + if (g_strcmp0(status, "Playing") != 0) + return; + } g_debug("new active player: %s", player->well_known); context_set_active_player(ctx, player); player_update_position_sync(player, ctx, &error); @@ -817,11 +881,40 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha int main(int argc, char *argv[]) { struct PlayerctldContext ctx = {0}; GError *error = NULL; + + ctx.connection = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, &error); + if (error != NULL) { + g_printerr("%s", error->message); + return 1; + } + + g_debug("connected to dbus: %s", g_dbus_connection_get_unique_name(ctx.connection)); + + if (argc > 1) { + char *method; + if (strcmp(argv[1], "--shift") == 0) { + method = "Shift"; + } else if (strcmp(argv[1], "--shift-play-pause") == 0) { + method = "ShiftPlayPause"; + } else { + g_printerr("unrecognized option '%s'\n", argv[1]); + return 1; + } + g_dbus_connection_call_sync( + ctx.connection, "org.mpris.MediaPlayer2.playerctld", + MPRIS_PATH, PLAYERCTLD_INTERFACE, method, NULL, NULL, + G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); + if (error != NULL) { + g_printerr("%s", error->message); + return 1; + } + return 0; + } + GDBusNodeInfo *mpris_introspection_data = NULL; GDBusNodeInfo *playerctld_introspection_data = NULL; ctx.players = g_queue_new(); ctx.pending_players = g_queue_new(); - ctx.loop = g_main_loop_new(NULL, FALSE); // Load introspection data and split into separate interfaces @@ -843,14 +936,6 @@ int main(int argc, char *argv[]) { ctx.playerctld_interface_info = g_dbus_node_info_lookup_interface( playerctld_introspection_data, "com.github.altdesktop.playerctld"); - ctx.connection = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, &error); - if (error != NULL) { - g_printerr("%s", error->message); - return 1; - } - - g_debug("connected to dbus: %s", g_dbus_connection_get_unique_name(ctx.connection)); - GVariant *names_reply = g_dbus_connection_call_sync( ctx.connection, DBUS_NAME, DBUS_PATH, DBUS_INTERFACE, "ListNames", NULL, NULL, G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); From 4a208ee9117ae94b83e5630ab59ed41d8cb32909 Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Sun, 3 May 2020 21:38:15 -0400 Subject: [PATCH 03/11] remove ShiftPlayPause, set active player by metadate changed --- playerctl/playerctl-daemon.c | 130 ++++++++++------------------------- 1 file changed, 35 insertions(+), 95 deletions(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index 36b9b2b..29c4772 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -73,12 +73,14 @@ static gint player_compare(gconstpointer a, gconstpointer b) { return 0; } -static void player_update_properties(struct Player *player, const char *interface_name, +static bool player_update_properties(struct Player *player, const char *interface_name, GVariant *properties) { GVariantDict cached_properties; GVariantIter iter; GVariant *child; + const gchar *prop_status, *cache_status; bool is_player_interface = false; // otherwise, the root interface + bool changed = true; if (g_strcmp0(interface_name, PLAYER_INTERFACE) == 0) { g_variant_dict_init(&cached_properties, player->player_properties); @@ -108,6 +110,11 @@ static void player_update_properties(struct Player *player, const char *interfac } GVariant *cache_value = g_variant_dict_lookup_value(&cached_properties, key, NULL); if (cache_value != NULL) { + if (is_player_interface && g_strcmp0(key, "PlaybackStatus") == 0) { + cache_status = g_variant_get_string(cache_value, NULL); + prop_status = g_variant_get_string(prop_value, NULL); + changed = (g_strcmp0(prop_status, cache_status) != 0); + } g_variant_unref(cache_value); } g_variant_dict_insert_value(&cached_properties, key, prop_value); @@ -129,6 +136,7 @@ static void player_update_properties(struct Player *player, const char *interfac } player->root_properties = g_variant_ref_sink(g_variant_dict_end(&cached_properties)); } + return changed; } static void player_update_position_sync(struct Player *player, struct PlayerctldContext *ctx, @@ -332,75 +340,21 @@ static void context_remove_player(struct PlayerctldContext *ctx, struct Player * } } -static void playerctl_play_pause_player(struct PlayerctldContext *ctx, struct Player *p, bool play) { - GError *error = NULL; - - g_dbus_connection_call_sync(ctx->connection, p->unique, MPRIS_PATH, - PLAYER_INTERFACE, play ? "Play" : "Pause", - NULL, NULL, - G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); - if (error != NULL) { - g_warning("%s", error->message); - g_clear_error(&error); - } -} - -static void context_shift_active_player(struct PlayerctldContext *ctx, int play_pause) { - guint len; - gboolean can_play = true; +static void context_shift_active_player(struct PlayerctldContext *ctx) { GError *error = NULL; - GVariant *prop; - GVariantDict dict_from, dict_to; - struct Player *from, *to = NULL; - - len = g_queue_get_length(ctx->players); + struct Player *p; - from = context_get_active_player(ctx); - - for (guint i = 1; i < len && !to; i++) { - to = g_queue_peek_nth(ctx->players, i); - if (strcmp(from->unique, to->unique) == 0) - continue; - - g_variant_dict_init(&dict_to, to->player_properties); - prop = g_variant_dict_lookup_value(&dict_to, "CanPlay", G_VARIANT_TYPE_BOOLEAN); - can_play = true; - if (prop) { - can_play = g_variant_get_boolean(prop); - g_variant_unref(prop); - } - - if (!can_play) { - to = NULL; - } - } - - if (to == NULL) - return; - - context_remove_player(ctx, from); - context_add_player(ctx, from); - context_set_active_player(ctx, to); - - if (play_pause) { - g_variant_dict_init(&dict_from, from->player_properties); - prop = g_variant_dict_lookup_value(&dict_from, "PlaybackStatus", G_VARIANT_TYPE_STRING); - if (prop) { - const gchar *status = g_variant_get_string(prop, NULL); - g_variant_unref(prop); - if (g_strcmp0(status, "Playing") == 0) { - playerctl_play_pause_player(ctx, from, false); - playerctl_play_pause_player(ctx, to, true); - } - } - } + p = context_get_active_player(ctx); + context_remove_player(ctx, p); + context_add_player(ctx, p); + p = context_get_active_player(ctx); context_emit_active_player_changed(ctx, &error); - player_update_position_sync(to, ctx, &error); if (error != NULL) { g_warning("could not emit active player change: %s", error->message); g_clear_error(&error); } + player_update_position_sync(p, ctx, &error); if (error != NULL) { g_warning("could not update player position: %s", error->message); g_clear_error(&error); @@ -412,8 +366,6 @@ static const char *playerctld_introspection_xml = " \n" " \n" " \n" - " \n" - " \n" " \n" " \n" " \n" @@ -581,10 +533,7 @@ static void playerctld_method_call_func(GDBusConnection *connection, const char } if (strcmp(method_name, "Shift") == 0) { - context_shift_active_player(ctx, 0); - g_dbus_method_invocation_return_value(invocation, NULL); - } else if (strcmp(method_name, "ShiftPlayPause") == 0) { - context_shift_active_player(ctx, 1); + context_shift_active_player(ctx); g_dbus_method_invocation_return_value(invocation, NULL); } else { g_dbus_method_invocation_return_dbus_error( @@ -840,21 +789,13 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha if (is_properties_changed) { GVariant *interface = g_variant_get_child_value(parameters, 0); GVariant *properties = g_variant_get_child_value(parameters, 1); - player_update_properties(player, g_variant_get_string(interface, 0), properties); + is_properties_changed = + player_update_properties(player, g_variant_get_string(interface, 0), properties); g_variant_unref(interface); g_variant_unref(properties); } - if (player != context_get_active_player(ctx)) { - GVariantDict dict; - g_variant_dict_init(&dict, player->player_properties); - GVariant *prop = g_variant_dict_lookup_value(&dict, "PlaybackStatus", G_VARIANT_TYPE_STRING); - if (prop) { - const gchar *status = g_variant_get_string(prop, NULL); - g_variant_unref(prop); - if (g_strcmp0(status, "Playing") != 0) - return; - } + if (is_properties_changed && player != context_get_active_player(ctx)) { g_debug("new active player: %s", player->well_known); context_set_active_player(ctx, player); player_update_position_sync(player, ctx, &error); @@ -870,11 +811,13 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha } } - g_dbus_connection_emit_signal(ctx->connection, NULL, object_path, interface_name, signal_name, - parameters, &error); - if (error != NULL) { - g_debug("could not emit signal: %s", error->message); - g_clear_error(&error); + if (is_properties_changed) { + g_dbus_connection_emit_signal(ctx->connection, NULL, object_path, interface_name, signal_name, + parameters, &error); + if (error != NULL) { + g_debug("could not emit signal: %s", error->message); + g_clear_error(&error); + } } } @@ -882,6 +825,11 @@ int main(int argc, char *argv[]) { struct PlayerctldContext ctx = {0}; GError *error = NULL; + if (argc > 2 || (argc == 2 && strcmp(argv[1], "--shift") != 0)) { + g_printerr("usage: playerctld [--shift]\n"); + return 1; + } + ctx.connection = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, &error); if (error != NULL) { g_printerr("%s", error->message); @@ -890,20 +838,12 @@ int main(int argc, char *argv[]) { g_debug("connected to dbus: %s", g_dbus_connection_get_unique_name(ctx.connection)); - if (argc > 1) { - char *method; - if (strcmp(argv[1], "--shift") == 0) { - method = "Shift"; - } else if (strcmp(argv[1], "--shift-play-pause") == 0) { - method = "ShiftPlayPause"; - } else { - g_printerr("unrecognized option '%s'\n", argv[1]); - return 1; - } + if (argc == 2) { g_dbus_connection_call_sync( ctx.connection, "org.mpris.MediaPlayer2.playerctld", - MPRIS_PATH, PLAYERCTLD_INTERFACE, method, NULL, NULL, + MPRIS_PATH, PLAYERCTLD_INTERFACE, "Shift", NULL, NULL, G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); + g_object_unref(ctx.connection); if (error != NULL) { g_printerr("%s", error->message); return 1; From de4cc0a703cf1af2454cf990cf91f838538ccba8 Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Sun, 3 May 2020 23:57:09 -0400 Subject: [PATCH 04/11] add changed from playerctld --- playerctl/playerctl-daemon.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index 29c4772..ebeeb6e 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -516,10 +516,9 @@ static void player_method_call_proxy_callback(GDBusConnection *connection, const } static void playerctld_method_call_func(GDBusConnection *connection, const char *sender, - const char *object_path, const char *interface_name, - const char *method_name, GVariant *parameters, - GDBusMethodInvocation *invocation, - gpointer user_data) { + const char *object_path, const char *interface_name, + const char *method_name, GVariant *parameters, + GDBusMethodInvocation *invocation, gpointer user_data) { g_debug("got method call: sender=%s, object_path=%s, interface_name=%s, method_name=%s", sender, object_path, interface_name, method_name); struct PlayerctldContext *ctx = user_data; @@ -536,9 +535,9 @@ static void playerctld_method_call_func(GDBusConnection *connection, const char context_shift_active_player(ctx); g_dbus_method_invocation_return_value(invocation, NULL); } else { - g_dbus_method_invocation_return_dbus_error( - invocation, "com.github.altdesktop.playerctld.InvalidMethod", - "This method is not valid"); + g_dbus_method_invocation_return_dbus_error(invocation, + "com.github.altdesktop.playerctld.InvalidMethod", + "This method is not valid"); } } @@ -560,7 +559,8 @@ static GDBusInterfaceVTable vtable_player = {player_method_call_proxy_callback, static GDBusInterfaceVTable vtable_root = {player_method_call_proxy_callback, NULL, NULL, {0}}; -static GDBusInterfaceVTable vtable_playerctld = {playerctld_method_call_func, playerctld_get_property_func, NULL, {0}}; +static GDBusInterfaceVTable vtable_playerctld = { + playerctld_method_call_func, playerctld_get_property_func, NULL, {0}}; static void on_bus_acquired(GDBusConnection *connection, const char *name, gpointer user_data) { GError *error = NULL; @@ -812,8 +812,8 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha } if (is_properties_changed) { - g_dbus_connection_emit_signal(ctx->connection, NULL, object_path, interface_name, signal_name, - parameters, &error); + g_dbus_connection_emit_signal(ctx->connection, NULL, object_path, interface_name, + signal_name, parameters, &error); if (error != NULL) { g_debug("could not emit signal: %s", error->message); g_clear_error(&error); @@ -839,10 +839,9 @@ int main(int argc, char *argv[]) { g_debug("connected to dbus: %s", g_dbus_connection_get_unique_name(ctx.connection)); if (argc == 2) { - g_dbus_connection_call_sync( - ctx.connection, "org.mpris.MediaPlayer2.playerctld", - MPRIS_PATH, PLAYERCTLD_INTERFACE, "Shift", NULL, NULL, - G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); + g_dbus_connection_call_sync(ctx.connection, "org.mpris.MediaPlayer2.playerctld", MPRIS_PATH, + PLAYERCTLD_INTERFACE, "Shift", NULL, NULL, + G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); g_object_unref(ctx.connection); if (error != NULL) { g_printerr("%s", error->message); From b924bf3f050a30a1b0c15f95762a900c751ea18e Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Mon, 4 May 2020 00:32:00 -0400 Subject: [PATCH 05/11] always emit signal for the active player --- playerctl/playerctl-daemon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index ebeeb6e..d51b8aa 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -811,7 +811,7 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha } } - if (is_properties_changed) { + if (is_properties_changed || player == context_get_active_player(ctx)) { g_dbus_connection_emit_signal(ctx->connection, NULL, object_path, interface_name, signal_name, parameters, &error); if (error != NULL) { From 91aa380fc7446d316057e561143a173f98bf31a0 Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Sun, 24 May 2020 02:06:21 -0400 Subject: [PATCH 06/11] implement glib parser and refactor --- playerctl/playerctl-daemon.c | 154 ++++++++++++++++++++++++++--------- 1 file changed, 117 insertions(+), 37 deletions(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index d51b8aa..6dc7c04 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -73,6 +73,12 @@ static gint player_compare(gconstpointer a, gconstpointer b) { return 0; } +/** + * Return value indicates if playback status has actually changed. + * Used to filter out PropertiesChanged signals that don't actually + * report a change to the player status that don't modify the playback + * status or a title change. + */ static bool player_update_properties(struct Player *player, const char *interface_name, GVariant *properties) { GVariantDict cached_properties; @@ -80,7 +86,7 @@ static bool player_update_properties(struct Player *player, const char *interfac GVariant *child; const gchar *prop_status, *cache_status; bool is_player_interface = false; // otherwise, the root interface - bool changed = true; + bool is_properties_updated = false; // have the properties actually updated if (g_strcmp0(interface_name, PLAYER_INTERFACE) == 0) { g_variant_dict_init(&cached_properties, player->player_properties); @@ -109,14 +115,35 @@ static bool player_update_properties(struct Player *player, const char *interfac goto loop_out; } GVariant *cache_value = g_variant_dict_lookup_value(&cached_properties, key, NULL); - if (cache_value != NULL) { - if (is_player_interface && g_strcmp0(key, "PlaybackStatus") == 0) { + if (cache_value != NULL && is_player_interface) { + if (g_strcmp0(key, "PlaybackStatus") == 0) { cache_status = g_variant_get_string(cache_value, NULL); prop_status = g_variant_get_string(prop_value, NULL); - changed = (g_strcmp0(prop_status, cache_status) != 0); + is_properties_updated = (g_strcmp0(prop_status, cache_status) != 0); + } else if ( g_strcmp0(key, "Metadata") == 0) { + GVariantDict cache_metadata, new_metadata; + GVariant *cache_vt, *new_vt; + const gchar *cache_title, *new_title; + + g_variant_dict_init(&cache_metadata, cache_value); + g_variant_dict_init(&new_metadata, prop_value); + + cache_vt = g_variant_dict_lookup_value(&cache_metadata, "xesam:title", NULL); + new_vt = g_variant_dict_lookup_value(&new_metadata, "xesam:title", NULL); + + if (cache_vt && new_vt) { + cache_title = g_variant_get_string(cache_vt, NULL); + new_title = g_variant_get_string(new_vt, NULL); + is_properties_updated = (g_strcmp0(cache_title, new_title) != 0); + } + if (cache_vt) + g_variant_unref(cache_vt); + if (new_vt) + g_variant_unref(new_vt); } - g_variant_unref(cache_value); } + if (cache_value != NULL) + g_variant_unref(cache_value); g_variant_dict_insert_value(&cached_properties, key, prop_value); loop_out: g_variant_unref(prop_value); @@ -136,7 +163,7 @@ static bool player_update_properties(struct Player *player, const char *interfac } player->root_properties = g_variant_ref_sink(g_variant_dict_end(&cached_properties)); } - return changed; + return is_properties_updated; } static void player_update_position_sync(struct Player *player, struct PlayerctldContext *ctx, @@ -340,11 +367,16 @@ static void context_remove_player(struct PlayerctldContext *ctx, struct Player * } } -static void context_shift_active_player(struct PlayerctldContext *ctx) { +/** + * Returns the newly activated player + */ +static struct Player *context_shift_active_player(struct PlayerctldContext *ctx) { GError *error = NULL; struct Player *p; - p = context_get_active_player(ctx); + if (!(p = context_get_active_player(ctx))) { + return NULL; + } context_remove_player(ctx, p); context_add_player(ctx, p); p = context_get_active_player(ctx); @@ -359,12 +391,15 @@ static void context_shift_active_player(struct PlayerctldContext *ctx) { g_warning("could not update player position: %s", error->message); g_clear_error(&error); } + + return p; } static const char *playerctld_introspection_xml = "\n" " \n" " \n" + " \n" " \n" " \n" " \n" @@ -521,23 +556,27 @@ static void playerctld_method_call_func(GDBusConnection *connection, const char GDBusMethodInvocation *invocation, gpointer user_data) { g_debug("got method call: sender=%s, object_path=%s, interface_name=%s, method_name=%s", sender, object_path, interface_name, method_name); + struct PlayerctldContext *ctx = user_data; - struct Player *active_player = context_get_active_player(ctx); - if (active_player == NULL) { - g_debug("no active player, returning error"); - g_dbus_method_invocation_return_dbus_error( - invocation, "com.github.altdesktop.playerctld.NoActivePlayer", - "No player is being controlled by playerctld"); - return; - } + struct Player *active_player; if (strcmp(method_name, "Shift") == 0) { - context_shift_active_player(ctx); - g_dbus_method_invocation_return_value(invocation, NULL); + if ((active_player = context_shift_active_player(ctx))) { + g_dbus_method_invocation_return_value( + invocation, + g_variant_new("(s)", active_player->well_known)); + } else { + g_debug("no active player, returning error"); + g_dbus_method_invocation_return_dbus_error( + invocation, "com.github.altdesktop.playerctld.NoActivePlayer", + "No player is being controlled by playerctld"); + } } else { - g_dbus_method_invocation_return_dbus_error(invocation, - "com.github.altdesktop.playerctld.InvalidMethod", - "This method is not valid"); + g_dbus_method_invocation_return_dbus_error( + invocation, + "com.github.altdesktop.playerctld.InvalidMethod", + "This method is not valid" + ); } } @@ -785,17 +824,18 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha } bool is_properties_changed = (g_strcmp0(signal_name, "PropertiesChanged") == 0); + bool is_properties_updated = is_properties_changed; if (is_properties_changed) { GVariant *interface = g_variant_get_child_value(parameters, 0); GVariant *properties = g_variant_get_child_value(parameters, 1); - is_properties_changed = + is_properties_updated = player_update_properties(player, g_variant_get_string(interface, 0), properties); g_variant_unref(interface); g_variant_unref(properties); } - if (is_properties_changed && player != context_get_active_player(ctx)) { + if (is_properties_updated && player != context_get_active_player(ctx)) { g_debug("new active player: %s", player->well_known); context_set_active_player(ctx, player); player_update_position_sync(player, ctx, &error); @@ -811,7 +851,7 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha } } - if (is_properties_changed || player == context_get_active_player(ctx)) { + if (is_properties_updated || player == context_get_active_player(ctx)) { g_dbus_connection_emit_signal(ctx->connection, NULL, object_path, interface_name, signal_name, parameters, &error); if (error != NULL) { @@ -821,13 +861,61 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha } } +static gchar **command_arg = NULL; + +static const GOptionEntry entries[] = { + {G_OPTION_REMAINING, 0, 0, G_OPTION_ARG_STRING_ARRAY, &command_arg, NULL, "COMMAND"}, + {NULL}, +}; + +static gboolean parse_setup_options(int argc, char **argv, GError **error) { + static const gchar *description = + "Available Commands:" + "\n shift Shift to next player"; + + GOptionContext *context; + gboolean success; + + context = g_option_context_new("- Playerctl Daemon"); + g_option_context_add_main_entries(context, entries, NULL); + g_option_context_set_description(context, description); + + success = g_option_context_parse(context, &argc, &argv, error); + + if (success && command_arg && g_strcmp0(command_arg[0], "shift") != 0) { + gchar *help = g_option_context_get_help(context, TRUE, NULL); + printf("%s\n", help); + g_option_context_free(context); + g_free(help); + exit(1); + } + + g_option_context_free(context); + return success; +} + +int playercmd_shift(GDBusConnection *connection) { + GError *error = NULL; + + g_dbus_connection_call_sync(connection, "org.mpris.MediaPlayer2.playerctld", MPRIS_PATH, + PLAYERCTLD_INTERFACE, "Shift", NULL, NULL, + G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); + g_object_unref(connection); + if (error != NULL) { + g_printerr("%s", error->message); + return 1; + } + return 0; +} + int main(int argc, char *argv[]) { struct PlayerctldContext ctx = {0}; GError *error = NULL; - if (argc > 2 || (argc == 2 && strcmp(argv[1], "--shift") != 0)) { - g_printerr("usage: playerctld [--shift]\n"); - return 1; + if (!parse_setup_options(argc, argv, &error)) { + g_printerr("%s\n", error->message); + g_clear_error(&error); + exit(0); } ctx.connection = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, &error); @@ -838,16 +926,8 @@ int main(int argc, char *argv[]) { g_debug("connected to dbus: %s", g_dbus_connection_get_unique_name(ctx.connection)); - if (argc == 2) { - g_dbus_connection_call_sync(ctx.connection, "org.mpris.MediaPlayer2.playerctld", MPRIS_PATH, - PLAYERCTLD_INTERFACE, "Shift", NULL, NULL, - G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); - g_object_unref(ctx.connection); - if (error != NULL) { - g_printerr("%s", error->message); - return 1; - } - return 0; + if (command_arg && g_strcmp0(command_arg[0], "shift") == 0) { + return playercmd_shift(ctx.connection); } GDBusNodeInfo *mpris_introspection_data = NULL; From fc92009b2660c7d819a375307ce12cc788963409 Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Sun, 24 May 2020 02:35:54 -0400 Subject: [PATCH 07/11] default to accepting metadata changes --- playerctl/playerctl-daemon.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index 6dc7c04..29165e0 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -86,7 +86,7 @@ static bool player_update_properties(struct Player *player, const char *interfac GVariant *child; const gchar *prop_status, *cache_status; bool is_player_interface = false; // otherwise, the root interface - bool is_properties_updated = false; // have the properties actually updated + bool is_properties_updated = true; // have the properties actually updated if (g_strcmp0(interface_name, PLAYER_INTERFACE) == 0) { g_variant_dict_init(&cached_properties, player->player_properties); @@ -115,7 +115,7 @@ static bool player_update_properties(struct Player *player, const char *interfac goto loop_out; } GVariant *cache_value = g_variant_dict_lookup_value(&cached_properties, key, NULL); - if (cache_value != NULL && is_player_interface) { + if (cache_value != NULL && is_player_interface && is_properties_updated) { if (g_strcmp0(key, "PlaybackStatus") == 0) { cache_status = g_variant_get_string(cache_value, NULL); prop_status = g_variant_get_string(prop_value, NULL); From f3135fb0ea293bc0d75b3b2680a123b0936bce47 Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Sun, 24 May 2020 03:39:03 -0400 Subject: [PATCH 08/11] create a simple test for playerctld shift --- test/test_daemon.py | 46 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/test_daemon.py b/test/test_daemon.py index 7e26e7b..89f9297 100644 --- a/test/test_daemon.py +++ b/test/test_daemon.py @@ -148,3 +148,49 @@ async def test_daemon_follow(bus_address): proc.proc.terminate() await proc.proc.wait() await playerctld_proc.wait() + +async def playerctld_shift(bus_address): + env = os.environ.copy() + env['DBUS_SESSION_BUS_ADDRESS'] = bus_address + env['G_MESSAGES_DEBUG'] = 'playerctld_shift' + shift = await asyncio.create_subprocess_shell( + 'playerctld shift', + env=env, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.STDOUT) + await shift.wait() + +@pytest.mark.asyncio +async def test_daemon_shift_simple(bus_address): + playerctld_proc = await start_playerctld(bus_address) + + [mpris1, mpris2] = await setup_mpris('player1', + 'player2', + bus_address=bus_address) + playerctl = PlayerctlCli(bus_address) + pctl_cmd = '--player playerctld metadata --format "{{playerInstance}}: {{artist}} - {{title}}" --follow' + proc = await playerctl.start(pctl_cmd) + + await mpris1.set_artist_title('artist1', 'title1') + line = await proc.queue.get() + assert line == 'playerctld: artist1 - title1', proc.queue + + await mpris2.set_artist_title('artist2', 'title2') + line = await proc.queue.get() + assert line == 'playerctld: artist2 - title2', proc.queue + + await playerctld_shift(bus_address) + line = await proc.queue.get() + assert line == 'playerctld: artist1 - title1', proc.queue + + await playerctld_shift(bus_address) + line = await proc.queue.get() + assert line == 'playerctld: artist2 - title2', proc.queue + + for mpris in (mpris1, mpris2): + mpris.disconnect() + + playerctld_proc.terminate() + proc.proc.terminate() + await proc.proc.wait() + await playerctld_proc.wait() From 2a1e9e2710fb51de070df04df43468c24044a194 Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Sun, 24 May 2020 21:14:35 -0400 Subject: [PATCH 09/11] remove active player modifications --- playerctl/playerctl-daemon.c | 59 ++++++------------------------------ 1 file changed, 10 insertions(+), 49 deletions(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index 29165e0..f7e8440 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -73,20 +73,12 @@ static gint player_compare(gconstpointer a, gconstpointer b) { return 0; } -/** - * Return value indicates if playback status has actually changed. - * Used to filter out PropertiesChanged signals that don't actually - * report a change to the player status that don't modify the playback - * status or a title change. - */ -static bool player_update_properties(struct Player *player, const char *interface_name, +static void player_update_properties(struct Player *player, const char *interface_name, GVariant *properties) { GVariantDict cached_properties; GVariantIter iter; GVariant *child; - const gchar *prop_status, *cache_status; bool is_player_interface = false; // otherwise, the root interface - bool is_properties_updated = true; // have the properties actually updated if (g_strcmp0(interface_name, PLAYER_INTERFACE) == 0) { g_variant_dict_init(&cached_properties, player->player_properties); @@ -115,35 +107,9 @@ static bool player_update_properties(struct Player *player, const char *interfac goto loop_out; } GVariant *cache_value = g_variant_dict_lookup_value(&cached_properties, key, NULL); - if (cache_value != NULL && is_player_interface && is_properties_updated) { - if (g_strcmp0(key, "PlaybackStatus") == 0) { - cache_status = g_variant_get_string(cache_value, NULL); - prop_status = g_variant_get_string(prop_value, NULL); - is_properties_updated = (g_strcmp0(prop_status, cache_status) != 0); - } else if ( g_strcmp0(key, "Metadata") == 0) { - GVariantDict cache_metadata, new_metadata; - GVariant *cache_vt, *new_vt; - const gchar *cache_title, *new_title; - - g_variant_dict_init(&cache_metadata, cache_value); - g_variant_dict_init(&new_metadata, prop_value); - - cache_vt = g_variant_dict_lookup_value(&cache_metadata, "xesam:title", NULL); - new_vt = g_variant_dict_lookup_value(&new_metadata, "xesam:title", NULL); - - if (cache_vt && new_vt) { - cache_title = g_variant_get_string(cache_vt, NULL); - new_title = g_variant_get_string(new_vt, NULL); - is_properties_updated = (g_strcmp0(cache_title, new_title) != 0); - } - if (cache_vt) - g_variant_unref(cache_vt); - if (new_vt) - g_variant_unref(new_vt); - } - } - if (cache_value != NULL) + if (cache_value != NULL) { g_variant_unref(cache_value); + } g_variant_dict_insert_value(&cached_properties, key, prop_value); loop_out: g_variant_unref(prop_value); @@ -163,7 +129,6 @@ static bool player_update_properties(struct Player *player, const char *interfac } player->root_properties = g_variant_ref_sink(g_variant_dict_end(&cached_properties)); } - return is_properties_updated; } static void player_update_position_sync(struct Player *player, struct PlayerctldContext *ctx, @@ -824,18 +789,16 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha } bool is_properties_changed = (g_strcmp0(signal_name, "PropertiesChanged") == 0); - bool is_properties_updated = is_properties_changed; if (is_properties_changed) { GVariant *interface = g_variant_get_child_value(parameters, 0); GVariant *properties = g_variant_get_child_value(parameters, 1); - is_properties_updated = - player_update_properties(player, g_variant_get_string(interface, 0), properties); + player_update_properties(player, g_variant_get_string(interface, 0), properties); g_variant_unref(interface); g_variant_unref(properties); } - if (is_properties_updated && player != context_get_active_player(ctx)) { + if (player != context_get_active_player(ctx)) { g_debug("new active player: %s", player->well_known); context_set_active_player(ctx, player); player_update_position_sync(player, ctx, &error); @@ -851,13 +814,11 @@ static void player_signal_proxy_callback(GDBusConnection *connection, const gcha } } - if (is_properties_updated || player == context_get_active_player(ctx)) { - g_dbus_connection_emit_signal(ctx->connection, NULL, object_path, interface_name, - signal_name, parameters, &error); - if (error != NULL) { - g_debug("could not emit signal: %s", error->message); - g_clear_error(&error); - } + g_dbus_connection_emit_signal(ctx->connection, NULL, object_path, interface_name, signal_name, + parameters, &error); + if (error != NULL) { + g_debug("could not emit signal: %s", error->message); + g_clear_error(&error); } } From 8ece9efd5ce0a6ab0f14f4cfd1e194d4736fa76e Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Sun, 24 May 2020 22:58:41 -0400 Subject: [PATCH 10/11] only emit signals if the player has changed --- playerctl/playerctl-daemon.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index f7e8440..d73cfca 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -337,27 +337,26 @@ static void context_remove_player(struct PlayerctldContext *ctx, struct Player * */ static struct Player *context_shift_active_player(struct PlayerctldContext *ctx) { GError *error = NULL; - struct Player *p; + struct Player *previous, *current; - if (!(p = context_get_active_player(ctx))) { + if (!(previous = current = context_get_active_player(ctx))) { return NULL; } - context_remove_player(ctx, p); - context_add_player(ctx, p); - p = context_get_active_player(ctx); - - context_emit_active_player_changed(ctx, &error); - if (error != NULL) { - g_warning("could not emit active player change: %s", error->message); - g_clear_error(&error); - } - player_update_position_sync(p, ctx, &error); - if (error != NULL) { - g_warning("could not update player position: %s", error->message); - g_clear_error(&error); + context_remove_player(ctx, previous); + context_add_player(ctx, previous); + if ((current = context_get_active_player(ctx)) != previous) { + player_update_position_sync(current, ctx, &error); + if (error != NULL) { + g_warning("could not update player position: %s", error->message); + g_clear_error(&error); + } + context_emit_active_player_changed(ctx, &error); + if (error != NULL) { + g_warning("could not emit active player change: %s", error->message); + g_clear_error(&error); + } } - - return p; + return current; } static const char *playerctld_introspection_xml = From afd253db59b66939d7b6b55faa460a1ce654154c Mon Sep 17 00:00:00 2001 From: Dov Salomon Date: Thu, 4 Jun 2020 12:40:31 -0400 Subject: [PATCH 11/11] add tests for playerctld shift error conditions --- playerctl/playerctl-daemon.c | 2 +- test/test_daemon.py | 31 ++++++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/playerctl/playerctl-daemon.c b/playerctl/playerctl-daemon.c index d73cfca..28ce8fe 100644 --- a/playerctl/playerctl-daemon.c +++ b/playerctl/playerctl-daemon.c @@ -862,7 +862,7 @@ int playercmd_shift(GDBusConnection *connection) { G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error); g_object_unref(connection); if (error != NULL) { - g_printerr("%s", error->message); + g_printerr("Cannot shift: %s\n", error->message); return 1; } return 0; diff --git a/test/test_daemon.py b/test/test_daemon.py index 89f9297..7819e76 100644 --- a/test/test_daemon.py +++ b/test/test_daemon.py @@ -158,7 +158,7 @@ async def playerctld_shift(bus_address): env=env, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.STDOUT) - await shift.wait() + return await shift.wait() @pytest.mark.asyncio async def test_daemon_shift_simple(bus_address): @@ -179,11 +179,13 @@ async def test_daemon_shift_simple(bus_address): line = await proc.queue.get() assert line == 'playerctld: artist2 - title2', proc.queue - await playerctld_shift(bus_address) + code = await playerctld_shift(bus_address) + assert code == 0 line = await proc.queue.get() assert line == 'playerctld: artist1 - title1', proc.queue - await playerctld_shift(bus_address) + code = await playerctld_shift(bus_address) + assert code == 0 line = await proc.queue.get() assert line == 'playerctld: artist2 - title2', proc.queue @@ -194,3 +196,26 @@ async def test_daemon_shift_simple(bus_address): proc.proc.terminate() await proc.proc.wait() await playerctld_proc.wait() + +@pytest.mark.asyncio +async def test_daemon_shift_no_player(bus_address): + playerctld_proc = await start_playerctld(bus_address) + + playerctl = PlayerctlCli(bus_address) + pctl_cmd = '--player playerctld metadata --format "{{playerInstance}}: {{artist}} - {{title}}" --follow' + proc = await playerctl.start(pctl_cmd) + + code = await playerctld_shift(bus_address) + assert code == 1 + + [mpris1] = await setup_mpris('player1', + bus_address=bus_address) + code = await playerctld_shift(bus_address) + assert code == 0 + + mpris1.disconnect() + code = await playerctld_shift(bus_address) + assert code == 1 + + playerctld_proc.terminate() + await playerctld_proc.wait()