altdesktop / playerctl

🎧 mpris media player command-line controller for vlc, mpv, RhythmBox, web browsers, cmus, mpd, spotify and others.
GNU Lesser General Public License v3.0
2.48k stars 81 forks source link

Feature Request: API to list mpris players #47

Closed ilgarmehmetali closed 6 years ago

ilgarmehmetali commented 7 years ago

While using the library, a function to find players instead of dealing with dbus would be nice.

Some research data from me:

According to MPRIS D-Bus Interface Specification; Each media player must request a unique bus name which begins with org.mpris.MediaPlayer2.

And this project just pulled all the names and grepped the ones starting with org.mpris.MediaPlayer2;

function list-valid-player-targets {
    echo "$(dbus-send --session --dest=org.freedesktop.DBus \
        --type=method_call --print-reply /org/freedesktop/DBus \
        org.freedesktop.DBus.ListNames | grep org.mpris.MediaPlayer2 |
        awk -F\" '{print $2}' | cut -d '.' -f4- | sort )"
}
acrisci commented 7 years ago

Sounds good to me.

What would the interface look like in the library?

ilgarmehmetali commented 7 years ago

Maybe a list of player objects containing properties from MediaPlayer2 interface and name variable you use for playerctl_player_new function?

class Player {
    string name;
    string Identity;
    string desktop_entry;
}
Player[] players = get_available_players();

Other properties could also get included into PlayerctlPlayer;

Raise()
Quit()
canQuit 
isFullscreen 
canSetFullscreen
canRaise
hasTrackList
getSupportedUriSchemes
getSupportedMimeTypes

If above properties and methods also gets added, PlayerctlPlayer class could get connect-disconnect functions and get_available_players() could return disconnected PlayerctlPlayer objects and api user would connect to the one they need.

ilgarmehmetali commented 7 years ago
class PlayerctlPlayer {
    get_name()
    get_identity()
    get_desktop_entry()
    is_fullscreen()
    can_set_fullscreen()
    can_aise()
    has_trackList()
    get_supported_uri_schemes()
    get_supported_mime_types()

    connect()
    disconnect()

    // functions bellow wouldnt work before conneting

    on()
    play_pause()
    play()
    stop()
    seek()
    pause()
    next()
    previous()
    print_metadata_prop()
    get_artist()
    get_title()
    get_album()
    set_position()
}
acrisci commented 7 years ago

I think we need to consider adding each of those methods on a case by case basis. It's not a goal of the project to faithfully reproduce the mpris specification, but to provide a user friendly library that abstracts the interface for the purpose of scripting. If you have a different use case, I would like to hear it.

To that end, I don't see many of these properties in the org.mpris.MediaPlayer2 interface are really that important. Perhaps we could make some of them properties of the player, but should be discussed in another issue.

So to avoid some complexity in the method, I think it would be sufficient to just give a list of player names someone could give to the Player constructor. That would be enough to check if a particular player is running in a nicer way and select between running players in a particular order.

ilgarmehmetali commented 7 years ago

When i was writing my previous comment, i couldnt help but remember a xkcd comic.

Like you said just player information could be useful too.

class Player {
    string name; // name to give to player constructer
    string Identity; // display name
    string desktop_entry; // desktop file, could be used to get other info about player, eg. icon
}
acrisci commented 7 years ago

When i was writing my previous comment, i couldnt help but remember a xkcd comic.

That's a pretty funny comic. Making a new standard is definitely something I'm trying to avoid. Like the comic says, standards often try to cover the use cases from a wide variety of projects. This project has always been intended for user scripts. If you have another use case in mind for how you would like to use the library, I'd be happy to hear it.

Like you said just player information could be useful too.

It certainly could be, but I've yet to hear a use case to consider for this information.

Here is the problem with including all of this. You've chosen an odd mix of properties here. If I implement, for instance, desktop_entry then someone will make the perfectly valid argument that surely the status of the player is more important, so it should be included in the struct. Then someone else can come along and claim that artist and title are even more important and now that gets included. This leads to two completely different implementations of the domain object of the "player" which is bad api design.

The simplest way to avoid that situation is by only returning a list of player names as strings. If you have another suggestion we can consider that too.

ilgarmehmetali commented 7 years ago

I was thinking of using desktop_entry to get applications icon and show it with its name. We can as it isnt that important for my use case. On the other hand returning just player names as a list wouldnt work out for me. Name variable i wrote is to be used in dbus name, but identity is the name i will show to users.

A struct with just 2 of them(name,identity) would work for me. Or if you would insist on just returning a array of strings, they could be inserted side by side in the array. ["vlc", "VLC media player", "vlc.2", "VLC media player", "banshee", "Banshee"] Or arrays of arrays could be used too.

[
    ["vlc", "VLC media player"], 
    ["vlc.instance7389", "VLC media player"], 
    ["banshee", "Banshee"]
]
acrisci commented 7 years ago

Thank you for making me aware of the identity property. Now that I know about this, I want to use it in the CLI to display this information to users for a general summary of all the players with their information. This conversation has been productive so far in giving me some new ideas to improve the project.

So given that we support this property, the question is where to put it. The options are to put it either on the Player itself, in the list of players, or both.

I don't think putting it on both is a good idea because of the reasons I mentioned earlier. I like the idea of a single canonical way to access a certain piece of information within the api.

Having the Identity being part of the Player is attractive to me since it is actually a property of the player itself in a similar way to all the other properties. The only difference is that it is accessed from a different dbus interface, but I don't think my users need to be aware of the different interfaces. They should only interact with the Player object to get the information they need. I think the ability to not need to learn the mpris standard and dbus is the primary value which this project gives to my users who just want to make desktop customizations.

Another thing I beileve about good library design is that each function should do one thing and do it well. The use case for listing player names that I think will be most popular is to find a player to connect to. A list of strings is the best data structure to satisfy this use case. I think listing the identities along with the names is doing too much.

You could accomplish what you want by iterating through the names, creating a player for each, and then getting the identity property from each player to create whatever data structure you want.

ilgarmehmetali commented 7 years ago

You could accomplish what you want by iterating through the names, creating a player for each, and then getting the identity property from each player to create whatever data structure you want.

This would be okey for my use case i believe.

otommod commented 6 years ago

I'd say that exposing just the names is enough. If the identity is needed, add it as a property of the Player class. I can see how it can be useful, but I don't think it's that useful to warranty returning along with the name.

There is list_player_names() in playerctl-cli.c already, I think it can be reworked (to return a list) and exposed, probably as a "utility function" (i.e. not a method).