[mpris] [ANN] MPRIS v2.0

[Mirsal Ennaime]

On Thu, Aug 12, 2010 at 2:07 PM, Marc-André Lureau
<marcandre.lureau at gmail.com> wrote:
> Hi

hey !

> On Thu, Aug 12, 2010 at 5:07 AM, Mirsal Ennaime
> Sorry to comment after the release, but the RC period was a bit too
> short for me. Overall, the changes are great, and a lot of good
> feedback has been made during the RC already.  Some things I would
> consider for future releases, maybe for 2.1?
>
> - (minor) Bus Name: org.mpris.mediaplayers - this is in conflict with
> version 1.0, that expect org.mpris.* to be used by 1.0 compliant MP.
> Have you considered org.mpris2? But since 1.0 was so broken in many
> ways, I guess there is not much we could do anyway.

I came to the conclusion that anyway, a "correct" 1.0 client
implementation should check GetMprisVersion before considering a
busname as belonging to a valid media player. org.mpris.MediaPlayer2
makes more sense conceptually than org.mpris2 as a bus name versioning
scheme and I think it would be a mistake to let 1.0's brokenness
influence this kind of things

> - (comment) org.mpris.MediaPlayer2 interface looks so generic that I
> wish it would be a standard "org.freedesktop.application" in the
> future.

yeah, there might be something really interesting to do here.

> - (comment) the "CanDoThat" or "HasThis" pattern could probably use
> introspection instead, if DBus common usage permits having
> "incomplete" interfaces.

This would reduce drastically the number of properties, but it would
also make it more difficult to implement
the spec, as generating / parsing dynamic XML is much more complicated
and error-prone than using boolean flags.

> - (normal) AddTrack / RemoveTrack. It is generally advised to group
> methods to avoid roundtrips. It's quite common to add/remove a bunch
> of URI in a track list. So I would prefer AddTracks/RemoveTracks. That
> can probably be easily added in 2.1 without breaking compatibility
> with 2.0

Indeed.

> - (very minor) Why not put TrackMetadataChanged in a seperate
> MediaPlayer2.Track interface ?

because to be correct then, the signal would have to be emited from an
object which represents the track, not from the media player object,
and we don't want clients to spend their life connecting to and
disconnecting from signal handlers every time the tracklist changes,
it's not worth it.

> - (comment) I understood that normal volume range is between 0.0 and
> 1.0. It would be nice to add in the specification that 1.5 is allowed
> for amplified volume. Or perhaps there should be a MaximumVolume
> property?

An uncapped volume here is consistent with how PA does things,

Quoting Lennart:
"The lower layers of the stack are actually without
limit. Since this spec should be considered a "lower layer of the stack"
it shouldn't limit this either."

> - (normal) there is MediaPlayer2.TrackList.GoTo() and
> MediaPlayer2.Player.SetPosition(). Both look similar to me except that
> SetPosition() takes an offset argument. However "Position" property is
> only the current time.

The trackid argument of SetPosition is only here to avoid races. When
this argument is different from the currently playing track, It does
*not* trigger a track change, the media player should ignore the
method call.

> - (normal) the spec probably lack standard error codes and return
> values. I could contribute patches for that for 2.1.

That would be great.

> - (minor). SetPosition(x) and Position property is read-only. But we
> have Volume property read-write. Why this inconsistency? Especially
> since we don't have exceptions/errors codes.

The idea behind this is that a track change would invalidate a request
to change the playback position, as conceptually, a position in a
track depends on which track it is, while the media player volume does
not.

Thanks !
-- 
Mirsal