[MPRIS] [RFC] MPRIS 2.3

[Alex Merry]

On 05/04/13 02:16, mirsal wrote:
> Hello !
> 
> On Thu, 2013-04-04 at 17:04 +0100, Alex Merry wrote:
>> We've got some things proposed for a possible MPRIS v2.3 [1], and I'd
>> like to get some feedback about which, if any, we want in.
> 
> I think these points are all sensible, though they are also rather
> tricky in a way and there are quite a lot of things to discuss before a
> draft can be produced.

I suppose I should reply to this...  Someone's proposing an extention to
Amarok's D-Bus interface for editing the rating, and it already has an
extension for Mute.

>> First up is a Mute property [2], as this is often orthogonal to volume;
>> this should be on the Player interface:
>> property b Mute
>>  - emits PropertiesChanged
> 
> Indeed, though I think we should define how the media players should
> behave with regards to the Volume property, that is, we should either
> have all media players treating muting as orthogonal to the volume or
> have changing the volume affect Mute in all of them.
> 
> Mute and Volume being orthogonal seems rather natural to me, although it
> might force media players to keep some state for the only sake of the
> mpris if they handle muting internally as simply setting the volume to
> zero.
> 
> (That point is probably moot as these media players might simply choose
> not to implement Mute instead, however I think the spec should not leave
> any ambiguity about that)

I agree on all fronts (including that media players that don't want to
implement Mute as orthogonal can simply not implement it).

>> Second is metadata editing [3], on both Player and TrackList:
>> method UpdateMetadata(in a{sv} updatedEntries, in as deletedEntries)
>> - what happens to entries appearing in both arguments is undefined
>> - wait for PropertiesChanged on Metadata to see if it worked
>> property as EditableMetadataEntries
>>  - emits PropertiesChanged
> 
> This looks racy to me.
> Maybe the old value should be supplied along with the new one ?

That's not the sort of race we need to be concerned about.  But you're
right, it is racy: we should have the track id as an argument.

> I don't really like the fact that there would be no way to detect
> failure besides an arbitrary timeout (adding tracks has the same issue
> though and there might not be much we can do) 

On the basis that clients are most likely to want to change properties
one-at-a-time, we could instead have
method SetMetadataField(in s field, in v value, in o trackId)
and
method DeleteMetadataField(in s field, in o trackId)
which could either be allowed to return a D-Bus error or have an "out b
success" argument.

With the existing modification methods in the spec (AddTrack, for
example, or setting properties) we are either silent about whether
errors can be raised or explicitly forbid errors (unless Can* is false).
 In retrospect, I'm not sure that was a sensible decision, but we can't
really demand that errors be raised now.

>> Third is HasPlaylists, which should make it easier for clients to see
>> whether there is a Playlists interface implemented (fewer round trips if
>> they use GetAll).  Not certain whether this is worth it, but we already
>> have HasTrackList.
>> property b HasPlaylists
> 
> I think the right question to ask is: Would changes at runtime need to
> be signaled ?
> 
> If not the case, then dbus introspection should suffice.

True, I guess.  One extra roundtrip for those clients that care about
Playlists isn't that much; they can just try to get the properties on
the Playlists interface and see if it works.  In fact, that's probably
the approach we should take with all new interfaces.

>> Fourth is a variant of Raise with a timestamp, which helps avoid
>> focus-stealing prevention in window managers.
>> method RaiseWithTimestamp(in x timestamp)
> 
> Yes, that is definitely needed, however RaiseWithTimestamp as a name
> would probably feel very awkward to client application developers not
> familiar with focus stealing prevention mechanisms' implementations.
> Maybe calling it something like Focus and explaining the difference in
> terms of effects (merely requiring attention in the case of Raise with
> focus stealing prevention vs sending the to the front unconditionally
> for Focus/RaiseWithTimestamp) would be a bit better.

OTOH, you need to know something about focus stealing prevention to
correctly use this API.  That timestamp is supposed to come from a user
interaction message, and the name is meant to mirror what toolkits like
GTK+ use.

Also, the difference in behaviour is down to window managers.  The
timestamp is used to decide whether to allow the window to be raised;
including it does not unconditionally raise it.

>> - arg is unix timestamp of the input event that triggered the call
>>
>> property b HasRaiseWithTimestamp
>> - introspection parsing sucks; use in combination with CanRaise
> 
> I agree that it sucks though the bindings should get fixed instead of
> the MPRIS working around their lacks.

I suppose.

> I don't think it makes sense for this to change at runtime independently
> of CanRaise.

Yeah, it wasn't meant to change.

Alex