Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Handle xesam:albumArtist being a string and not an array #243

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

leosunmo
Copy link

Got tripped up by this when using https://github.com/Sinono3/souvlaki through https://github.com/jpochyla/psst.
It uses xesam:albumArtist instead of xesam:artist and the xesam:albumArtist is a String, not Array, so we should probably handle that, even if it's not convention.

Added a basic test for string vs array. Probably not the best way of doing it, but it works.

@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@leosunmo
Copy link
Author

@googlebot I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Copy link
Owner

@soumya92 soumya92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a deviation from https://www.freedesktop.org/wiki/Specifications/mpris-spec/metadata/#xesam:albumartist.

How common is this? I would like to avoid a workaround here, but if many players are not following the spec then I'd be more open to this change.

@soumya92
Copy link
Owner

I wonder if maybe the better PR would be to just change https://github.com/Sinono3/souvlaki/blob/287797cd926fd7be802d5310fcc3836b0f5bc556/src/platform/linux/mod.rs#L303 😃

@leosunmo
Copy link
Author

From my somewhat limited experience, most players are not quite adhering to the spec. But I agree that the proper way would be to change Souvlaki here.

@soumya92
Copy link
Owner

Okay, if its more than just this one, let's make the change. Can you also do the same for artist just to be consistent? Other than that this looks good to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants