indicator-sound-developers team mailing list archive

[Johannes Sasongko <sasongko@xxxxxxxxx>]

Hi Mirsal,

>> I'd suggest placing it under another namespace, though, perhaps org.mpris.players.
>
> This might be more correct indeed. I thought about doing this, but I
> arrived to the conclusion that media player names are very unlikely to
> cause conflicts with eventual revisions of the spec which would define
> other bus names.

The risk is when, for example, you add an "org.mpris.Something" bus
name for special use. Older clients that don't know about this bus
name will confuse "Something" as a player ID. I realise that full
backwards-compatibility is not always possible, but this one is a
potential problem that can be prevented now.

>> Another thing that can help simplify the interaction with clients is
>> this: in addition to grabbing the player-specific bus name, each
>> player also tries to grab a well-known bus name (e.g.
>> org.mpris.MediaPlayer). The D-Bus queuing mechanism [1] will take care
>> of which player holds the name, and a simple client can just connect
>> to this well-known bus name without caring about choosing one
>> particular player.
>
> This is how the pre-1.0 spec used to define bus names, and it doesn't
> work very well in real life scenarios, without simplifying
> significantly the interaction with a media player on the bus. (listing
> media players using the current bus name policy is really just a dbus
> method call away)

The advantage is that it would create the concept of one "active"
player and several inactive players. With the current spec there is
only n players, and a client doesn't have a clear way to select one
without asking the user to choose (or choosing one randomly, which
could create inconsistencies).

What I propose is for each player to try grabbing *both*
org.mpris.<player_name> and org.mpris.MediaPlayer. A simple client
that doesn't want to care about the specific media players running can
use the generic interface, while advanced clients can allow the user
to choose which client they want to work on by using the
player-specific interfaces.

>>> org.mpris.MediaPlayer
>>
>> Should have a method to present / show / activate / bring up the
>> actual player. I'll leave you to choose the best name for this.
>
> Yes, this can be very useful, though it should not be mandatory.

Why so? Do you think it should be another capability (CAN_SHOW_PLAYER,
perhaps)? I can't think of a use case for a player to prevent showing
its window, though

>>> org.mpris.MediaPlayer.TrackList
>>
>>> GetTracksMetadata ( as: TrackIds ) → aa{sv}
>>
>> Should we have a GetAllTracksMetadata method as well? This would cater
>> for the common case of startup, rather than requiring the client to
>> send in all track IDs.
>
> In order for this to work, media players would be forced to fetch
> metadata for the entire tracklist,
> which could be problematic for large tracklists, as it would negate
> media players' efforts to reduce ressource usage by not fetching all
> the metadata (this is the case of VLC, for instance)

I see. IMHO that's worth mentioning in the spec, as a "best practices" of sort.

>>> org.mpris.MediaPlayer.Player
>>
>>> Repeat ( b: State ) → nothing
>>> GetStatus ( ) → (iiii)
>>
>> These have a few problems:
>> 1. repeat_current and endless can be thought of one status, as they
>> are mutually exclusive.
>
> Well, they are not mutually exclusive, as repeat implies endless.

I imagine a player would allow the user to switch between the 3 states
{no repeat, repeat current, repeat playlist}. Unless I'm missing
something here?

>> 2. There is currently no way to set the shuffle and endless statuses.
>
> the shuffle status can be set using the Shuffle method on the Player interface
> the endless one is either set by Repeat or by Loop

You're right, sorry. Don't know how I missed those.

>> 3. The Capabilities property should include whether these statuses can
>> be set at all. For example, Exaile doesn't support repeat_current,
>> while a Last.fm player can't set any of them.
>
> There are PLAYER_CAN_REPEAT and PLAYER_CAN_LOOP but I forgot the
> shuffle one, thanks :)

Another thing I missed as well.

>> 4. Naming: Repeat here should be SetRepeatCurrent.
>
> It would be inconsistant with Loop and Shuffle. Do you think they
> should be called SetLoop and SetShuffle instead ?

Yeah, I think having them explicitly called Set* would be more natural.

>>> Position − i (Time_In_Ms), read-only
>>
>> There should be an note that--to change the position--the client needs
>> to use SetPosition, which works around the issue of race conditions by
>> requiring the track ID.
>
> Yes, I thought it was obvious enough not to require a note.

Just a clarity issue, I think. While skimming through the spec, I was
wondering why Position is read-only, until I noticed that there's also
a SetPosition method with a track ID parameter.

>> Next, from http://xmms2.org/wiki/MPRIS_Metadata :
>>
>>> "time"
>>> (uint32) The duration in seconds
>>> "mtime"
>>> (uint32) The duration in milliseconds
>>
>> For consistency with Position and SetPosition, I believe this should
>> be "length" and always in milliseconds.
>
> You're right, though the metadata guidelines should be specified in a
> more formal document.
> We shouldn't blindly modify this wiki page as it is still used by v1
> implementations.

Ah, I see. I wasn't aware that the wiki page hasn't been updated for v2.

> Thanks a lot for your feedback. I'll add what's missing asap.

Thanks for your work in pushing this draft spec forward.

Johannes