[mpris] [RFC] MPRIS2 Release Candidate

[Alex Merry]

  On 29/07/10 14:29, Mirsal Ennaime wrote:
> Hello,
>
> I would like to propose this document as a mpris 2.0 release candidate:
>
> http://www.mpris.org/mirsal/2.0-draft/spec.html
>
> Could you have a look at it and share any remark, concern, improvement
> proposal you have in mind ?

OK, I'll start with comments on the actual spec (as opposed to just 
nitpicking over the docs - see at the end).  These are all on the Player 
interface.

Why do we have SetRate and SetPosition, but Shuffle, Loop and Repeat?  
We should be consistent here.  "Seek", of course, is an immediate action 
(rather than setting a property), so definitely shouldn't start with Set.

Rate: is this intended to be an actual multiplier?  So that 0.5 means 
half-speed, 1.5 means 1.5 seconds of track are played every second, 
etc?  I think this would be best.  If so, we need to say that.

(Note: at some point soon, I'll update/rewrite 
http://qt-apps.org/content/show.php/MPRIS+Tester?content=85539 to test 
this spec).

I'm away over the weekend, so won't be able to reply to any emails until 
Sunday evening.

Alex




The rest is just wording bits.  I'm happy to do these changes myself in 
a branch, and send a merge request to you, so you can ignore the rest of 
the email if you like - just let me know if you want me to make the changes.

Quit:
"The media player may not allow clients to shut it down."
would be better as
"The media player may refuse to allow clients to shut it down."

Raise:
"The media player may not be able to control how its user interface is 
displayed"
would be slightly better as
"The media player may be unable to control how its user interface is 
displayed"

Playlists:
It's not easy to find the documentation for Playlist_Struct from here, 
as it just links to Playlist_Structs_List, which is completely 
uninformative.

Capability_Flags:
Any way we could have the actual values in the documentation, rather 
than the C bitshifts?


Tracklist interface:

"To make use of this interface, the first thing that a client should do 
is to fetch all the properties at once, using GetAll on the D-Bus 
Properties interface. This will ensure all the properties are in a 
consistent state. The client will probably then want to fetch the 
metadata for the playlist (or possibly only part of it) using 
GetTracksMetadata."

In this incarnation, there is no reason to use GetAll.  Which is good, 
because in Qt, for example, it's not possible.

We probably need to say more about track ids, though.  How about:

> Each track in the tracklist has a unique identifier.  The intention is 
> that this uniquely identifies the track within the scope of the 
> tracklist.  In particular, if a media item (a particular music file, 
> say) occurs twice in the track list, each occurrence should have a 
> different identifier.  If a track is removed from the middle of the 
> playlist, it should not affect the track ids of any other tracks in 
> the playlist.
>
> As a result, the traditional track identifiers of URLs and position in 
> the playlist cannot be used.  It is recognised that this may pose some 
> difficulties for media player implementations.  A potential scheme 
> would be to use the URI and, in the case of duplicates, appending a 
> fragment and a number (eg: the second "file:///home/jim/foo.mp3" 
> occurance could be "file:///home/jim/foo.mp3#2", which will keep this 
> track id even if the first occurance is then removed).  Any scheme 
> that satisfies the uniqueness requirements is valid, however, as 
> clients are not permitted to make any assumptions about the value of 
> the track id beyond the fact that it is a unique identifier.
>
> Note that the (memory and processing) burden of implementing the 
> TrackList interface and maintaining unique track ids for the playlist 
> may be mitigated by only exposing part of the playlist when it is very 
> long (the 20 or so tracks around the currently playing track, for 
> example).

AddTrack:

"Add an URI" should be "Adds a URI".

"This has the same effect as a call to GoTo afterwards." should be 
"Setting this to true has the same effect as calling GoTo afterwards."

We should say that if the media player cannot find or open the URI, the 
track will not be added, and so clients should not assume that the call 
was successful unless and until the TrackAdded (or potentially 
TrackListReplaced) signal is emitted.

The |TRACKLIST_CAN_EDIT_TRACKS property should be mentioned.|

RemoveTrack:

We should say that specifying a track id not in the playlist has no 
effect (rather than throwing an error, or whatever), so clients should 
not assume that the call was successful unless and until the 
TrackRemoved (or potentially TrackListReplaced) signal is emitted.

The |TRACKLIST_CAN_EDIT_TRACKS property should be mentioned.|


Playlist interface:

Some introduction about what this interface is meant to achieve would be 
nice (just something like "This interface implements the methods for 
querying and providing basic control over what is currently playing.").

Next, Prev:
In the second line of each, the comma should be after "(and looping is 
off)".  Also, there should be full stops (".") at the end of the sentences.

Seeked signal:
"When this signal is not received, clients should assume that when 
Playing, the position progresses according to real time, and when 
Paused, it remains constant." - we need some mention of the playback 
rate here.