bdrazhzhov / audio-service-mpris

Flutter audio_service plugin implementing Media Player Remote Interfacing Specification
MIT License
4 stars 3 forks source link

org.freedesktop.DBus.Properties implementation does not follow specifications #5

Open DomiStyle opened 2 weeks ago

DomiStyle commented 2 weeks ago

I recently tried capturing MPRIS data from Finamp, which uses this library and noticed that the property getter is not implemented correctly.

According to the specification (https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-properties) the Get function should always return a variant.

For example, org.freedesktop.DBus.Properties.Get("org.mpris.MediaPlayer2.Player", "PlaybackStatus) should return a variant containing a string. This library currently returns a string directly.

This does not seem to bother the KDE media widget but it's out of spec and probably should be fixed.

bdrazhzhov commented 2 weeks ago

Hello @DomiStyle,

Could you provide minimal reproducible example of the issues you're talking about or a pull request with the changes you need?

DomiStyle commented 2 weeks ago

Hey @bdrazhzhov

Unfortunately I'm not a Flutter dev and Finamp is not made by me, I just got pointed here since this is the library that is used by Finamp.

I can however provide a detailed description of what the library does and what it should do.

After looking at the code, the culprit is most likely the getProperty method here: https://github.com/bdrazhzhov/audio-service-mpris/blob/e0402529d09b727776860947437333422b6476ee/lib/mpris.dart#L432

Every property that is returned in there needs to be a DBusVariant(DBusTYPE(VARIABLE))] to meet the dbus specification. Currently this is only done for getPosition().

At the same time, every property that is returned inside of the getAllProperties() function should not be a variant to meet the MPRIS specification:

https://github.com/bdrazhzhov/audio-service-mpris/blob/e0402529d09b727776860947437333422b6476ee/lib/mpris.dart#L570

getPosition() currently does not do this and gets returned as a variant instead of a Int64.

For example, the PlaybackStatus needs to be returned as DBusMethodSuccessResponse(DBusVariant([DBusString(_playbackState)]) instead of a DBusMethodSuccessResponse([DBusString(_playbackState)]) when it is requested individually but it needs to stay a DBusMethodSuccessResponse([DBusString(_playbackState)]) when all properties are requested.

https://github.com/bdrazhzhov/audio-service-mpris/blob/e0402529d09b727776860947437333422b6476ee/lib/mpris.dart#L108

Again, no Flutter dev but that seems to be how it's done because getPosition() does it like this.

Here's what Finamp reports for the PlaybackStatus as viewed through Bustle:

image

Here's the same response but from VLC:

image

As you can see, VLC properly returns a variant while Finamp returns a string directly, which is out of spec.

Hope this helps, if you need anything more just let me know.

bdrazhzhov commented 2 weeks ago

Thank you @DomiStyle for the perfect explanation. I'll release the fixes soon.