Interest in expanding Metadata impls for Track and Album, adding a ContextChanged event mechanism

[capnfabs]

Hi team,

I'm making some changes to spotifyd so that it can obtain enough information for its MPRIS implementation from librespot. I'm interested in making some changes to librespot to support this, and wanted to check if that seemed reasonable before opening PRs :)

The main thing I'd like for this is to expand the metadata::Track and metadata::Album structs so that they have a more complete set of info. It seems like this information is available in the proto that's used to create these structs, but that it's not plumbed through yet. The change seems simple (4f54dc8); would it be ok to submit a PR for this?

Another thing I'd like a way of knowing when the context changes (both context URI, and track number from playlist). I put together a way of doing that using a mechanism similar to PlayerEvents in b28bea3. Does this seem more broadly useful, and would a PR for this be worthwhile? Is there a better way to do it?

Finally, is there a way of retrieving Loop/Shuffle status from librespot? I'm interested because spotifyd's MPRIS implementation pulls this from the Spotify API (using rspotify), which interacts badly with playerctl, resulting in delays when running playerctl play-pause targeting spotifyd (details here: Spotifyd/spotifyd#977 (comment)).

Thanks in advance, and thanks also for all your work on librespot!

[roderickvd]

I'm making some changes to spotifyd so that it can obtain enough information for its MPRIS implementation from librespot. I'm interested in making some changes to librespot to support this, and wanted to check if that seemed reasonable before opening PRs :)

Thanks for chiming in.

The main thing I'd like for this is to expand the metadata::Track and metadata::Album structs so that they have a more complete set of info. It seems like this information is available in the proto that's used to create these structs, but that it's not plumbed through yet. The change seems simple (4f54dc8); would it be ok to submit a PR for this?

Yeah, let's get that in a PR.

Another thing I'd like a way of knowing when the context changes (both context URI, and track number from playlist). I put together a way of doing that using a mechanism similar to PlayerEvents in b28bea3. Does this seem more broadly useful, and would a PR for this be worthwhile? Is there a better way to do it?

Ideally it would all be dispatched under the same --onevent handler.

Finally, is there a way of retrieving Loop/Shuffle status from librespot? I'm interested because spotifyd's MPRIS implementation pulls this from the Spotify API (using rspotify), which interacts badly with playerctl, resulting in delays when running playerctl play-pause targeting spotifyd (details here: Spotifyd/spotifyd#977 (comment)).

I've been asking this question myself as I've been working on new-api. From the proto's, I don't think so? To double-check, you could best read through the librespot-java code. If it's there, @devgianlu will have implemented it.

[eladyn]

Yeah, let's get that in a PR.

As far as I can see, there hasn't yet been a PR for this, right? Would you still be accepting such changes, and, if so, would you prefer it to be against dev or new-api? I could then adapt (or “steal” :) @capnfabs code and open the PR.

[roderickvd]

Metadata actually already got a mega-update in new-api. It might just be in there, have you checked? If not please add it there.

[ashthespy]

The main thing I'd like for this is to expand the metadata::Track and metadata::Album structs so that they have a more complete set of info. It seems like this information is available in the proto that's used to create these structs, but that it's not plumbed through yet. The change seems simple (4f54dc8); would it be ok to submit a PR for this?

I am a bit weary of that -- it moves from a single mercury request to 4+(if there are more artists).

And as player.rs will await this call, introducing a delay even before the track is fetched/decrypted, which might be tangible.
A more prudent approach would be to resolve these extra metadata asynchronously while the track is being downloaded/decrypted..

[capnfabs]

Hey @ashthespy! I'm only proposing adding the data that's already in the response for the given mercury request, as far as I can tell (relevant change is in this commit: 4f54dc8). Have I misunderstood something about how this works?

Definitely wouldn't want to do this if it introduced an extra delay.

[ashthespy]

Ah, so you don't make another mercury call, but just try and parse the artist directly off the artist key from the track response. Should be fine!

[capnfabs]

I certainly have done nothing about this, and feel free to steal / rework my code to make this happen :!)

On June 29, 2022, Ryan Summers ***@***.***> wrote: > Yeah, let's get that in a PR. > As far as I can see, there hasn't yet been a PR for this, right? Would you still be accepting such changes, and, if so, would you prefer it to be against dev or new-api? I could then adapt (or “steal” :) @capnfabs <https://github.com/capnfabs> code and open the PR.

— Reply to this email directly, view it on GitHub <https://github.com/librespot- org/librespot/discussions/834#discussioncomment-3050168>, or unsubscribe <https://github.com/notifications/unsubscribe- auth/AAKGJPGCNJKK22GDF42KCMTVRR6XDANCNFSM5BYJ7QFQ>. You are receiving this because you were mentioned.Message ID: <librespot-org/librespot/repo- ***@***.***>