[mpris] [RFC] MPRIS2 Release Candidate

[Mirsal Ennaime]

Hello and sorry for the late reply:

On Sat, Aug 7, 2010 at 8:20 PM, Lennart Poettering
<lennart at poettering.net> wrote:
> A few comments:
>
> - please use the recently standardized property change signals when
>  properties change. Don't cook your own signals.

This definitely should be done, but i don't think it is a good idea to
switch to it now, as it is not yet ubiquitous in bindings and it would
require to completely rethink a large part of the spec.

IMHO, we should stick with the legacy signals in v2 and use
org.freedesktop.DBus.Properties.PropertiesChanged signal in v3 (along
with other disruptive changes you are suggesting)

> - i am not a particular fan of using integers for bit flag fields/enums
>  on D-Bus. I think it is usually much nicer to use strings for
>  enums. And bitfields should probably be just seperate bool
>  properties. modern bindings such as gdbus optimize this nicely and
>  retrieve all flags with a single call.

on the one hand, this looks more like the dbus way of doing things, on
the other hand, we would lose atomicity in the capabilities (when not
using gdbus) as well as the handyness of binary operators.

> - It makes little sense to me that "capabilities" might change. What's
>  the rationale? CapabilitiesChanged is redundant I believe.

The Player capabilities are supposed to change often (they depend on
the organization of the tracklist, on the stream being played and on
the internal state of the media player, which can all change at
runtime)

> - simplify the version stuff. One version counter should be more than
>  sufficient. There is no need for major+minor. Also, why do you need
>  both the version stuff ad the capabilities stuff? choose one. Finally,
>  I think versioning should be done via interface names and
>  suchlike. See http://0pointer.de/blog/projects/versioning-dbus.html

Definitely ! org.mpris.MediaPlayer2 it is, and the MprisVersion
property is no more.

> - i wonder how scalabale the playlist stuff is. i.e. does it always make
>  sense to send the list of playlist in a single msg? If there's a
>  substantial amount of playlists (and there might very likely be as
>  most music on the internet ship with an .m3u or .pls file). I think
>  this needs some logic to allow splitting up requests and responses
>  into chunks, so that multiple small messages may be sent instead of a
>  single big one.
>
> - Why is there no signal that notifies about added/removed playlists?

Multiple playlists support is going out of the spec, people who need
this (https://wiki.ubuntu.com/SoundMenu) will use an extension to the
spec living on a separate interface.

> - The tracks stuff is definitely not scalable. You definitely don't want
>  to send the the track info for everything in my music collection in a
>  single message. It's gigantic and the bus would even refuse delivering
>  messages as big as that. requesting the track list must be doable in
>  chunks.

Let me clarify the use case of the tracklist: it is by no means a way
to browse through all the available tracks but a way for the media
player to give context to the currently playing item by exposing a
*short* list of tracks that either have been played recently or will
be played shortly. (as an example it can be the tracks in the same
album of the currently playing one, or the contents of the rhythmbox
play queue)

We should indicate that the tracklist should not contain more than a
few tens of tracks.

> - what unit is the value for AdjustVolume in? dB? linear? probably
>  neither? given that for many audio devices the volume scale is unknown
>  it is probably not smart using dB/linear here. Hence you should probably say that 0.0 is
>  silence and 1.0 is a "sensible maximum volume", but you probably
>  should allow the volume to go beyond 1.0. and everything in between
>  should be in a scale that "feels right" to the user.

Ack.
(note that the current docstrings specify a linear scale between
silence and the maximum volume level supported by the media player,
you might have missed it)

> - If you pass times use 64bit integers and use usec as unit. That's what
>  most media systems do these days and i think is a the only unit that
>  makes remotely sense.

Ack

> - the player state shouldn't be a struct. Just seperate bool props.

Ack

> Also, I wonder how this actually relates to the grilo dbus apis? also,
> the mediaserver dbus spec seems similar a bit too (if only for the track list
> part). There might be a good opportunity to reuse existing interfaces or
> at least make the interfaces feel similar in style.

I think they do serve a separate purpose, the mpris really is about
controlling playback, not browsing a media library.
It would be nice to make the simultaneous use of these specs as
friction-less as possible, this can be one of the focus points of the
next version.

Thanks for your input, it is very valuable.

Best regards,
-- 
Mirsal