Add M3U library provider

[benreid24]

Library provider is used by the core.tracklist.add method. Adding the provider allows m3u uris to be used with core.tracklist.add. Without it one cannot add a m3u playlist in one action and would instead have to list all the items then add them one at a time.

Resolves #2155

[kingosticks]

I've always struggled with what to do with this and previously come to the conclusion the two step approach is best. And it's what other clients do - you can add multiple uris to the tracklist at a time so it's not a big deal.

This is because M3u playlists are a bit weird in that they contain tracks from other backends. Library.lookup is supposed to return full Tracks, but this simply isn't possible in this case and it can essentially only return Refs dressed as Tracks, missing all the metadata you'd expect the tracks to have. These skeleton tracks can then be added to the tracklist but it's probably not what you want to have there. I appreciate the Spotify backend (and probably others) can do this, and it's annoying it's inconsistent, but can you see where I'm coming from?

[kingosticks]

I think for this to work properly we'd need tracklist.add to implement a two-stage process. Firstly to 'resolve' (or really, 'identify') the URIs it was given into Refs, and then secondly to do the actual lookup on those Refs and get back Tracks to add. I can see the value in doing this here rather than something similar in every client. I'm not against the idea.

EDIT: actually, I don't think this works. See #2156 (comment)

[benreid24]

Ah you're right. My motivation for being able to add the entire playlist with one URI is because it's not trivial in Home Assistant to capture, format, and use the response from one API call as the body for another. I want to be able to send a request that does add party playlist to queue rather than two requests fetch party playlist -> add all returned tracks to queue.

Regarding the two step approach: I do think it would be valuable for any part of the codebase to be able to identify when a track is URI only vs when a track is populated with all the expected metadata. I can think of a few ways to go about it:

1. Add a field to Track that signifies that it is essentially just a Ref. This would require any location that deals with Tracks (ie tracklist.add) to check and action on that, which is not ideal imho.
2. Provide a mechanism for Ref based backends (M3U) to lookup uri's and populate its result set prior to returning.

I think it makes more sense for the backend to carry the responsibility of populating the tracks it returns than for all of the call sites to handle Refs disguised as Tracks. Especially in the case where a backend knows that its entire set of Tracks are all refs at best. If LibraryProvider's had access to the LibraryController then Ref-based providers could add the second step lookup call internally.

Anyways, that's just my opinion on the approach, but I appreciate that I do not have nearly as much context as yourself and others. I'm happy to iterate on the approach and implement the change once decided.

[kingosticks]

Option 2 isn't compatible with our https://docs.mopidy.com/stable/api/architecture/#architecture, backends cannot call into core (or other backends). Doing so would create loops in the graph, that's a lot of complexity. I don't think we'd ever go down that route.

[kingosticks]

My option 3 to change TracklistController.add() to take Refs rather than Uris doesn't really work. Even though it now knows everything it needs to call either self.core.library.lookup() or self.core.playlists.lookup() (and extract the tracks) as appropriate, the tracks it gets back from M3UPlaylistProvider are useless skeleton tracks. It's got other fatal problems:

• We want to pass (a list of) simple strings rather than burdening frontends with knowledge of what's required to define our models/types; passing Tracks here caused issues previously. A Ref is far simpler, so maybe that's OK, but it's a bit weird conceptually.
• It only solves the issue for TracklistController.add(). A Frontend calling LibraryController.lookup() directly will hit the same problem and we're still in the state where some backends are stuck behaving very inconsistently.

Option 4 could be for LibraryController.lookup() to call the correct library/playlist backend provider for each URI. To do that it needs to know the type of each URI and we'd need a new backend API taking a URI and returning a "RefType" (or just a name-less Ref). PlaylistsController.lookup() could then also do something more sensible: a LibraryController.lookup for each track Ref (PlaylistProvider.lookup would return a PlaylistLite containing Refs instead of Tracks) and then create and return a real full-fat Playlist with everything.

The unavoidable and weird skeleton M3U tracks issue has been bugging me for years. I really like that option 4 might solve everything. It does mean a bunch of API changes but the good news is that now is a great time to make them! Am I mad? Any thoughts @jodal ?

when a track is URI only vs when a track is populated with all the expected metadata.

Ideally, a Track that is "URI only", should just be a Ref. I think that simple concept works everywhere except this situation where PlaylistsProvider has items from other backends e.g. our M3U backend. (The same dilemma also existed when Spotify playlists could contain local tracks, but helpfully, Spotify stopped supporting that). It would be great to fix it.

[benreid24]

That all sounds reasonable to me! Just to make sure I'm understanding your approach:

• Add a new controller that maps URI to Type where, at present, Type is one of Track, Playlist, etc
• Use that Type mapping to determine which controller in the core API to use when resolving a library lookup call
  • For Playlist type call into PlaylistController to get list of Ref then resolve those to full Tracks using the LibraryController
  • For Track type just resolve immediately using the appropriate backend
  • Etc for other types

Is that correct?

[kingosticks]

Yes, that's a much clearer version of my waffle!

It also needs something (a default implementation?) that allows calling the new identify/mapper API to still work for old backends that don't implement it yet.

But I'd like some more comments before we actually take this further because maybe there's a better approach. It's a fair bit of work. I think it's worth it but there's a risk I've got my blinkers on.

[benreid24]

Agreed! Once jodal and whoever else chime in I'm happy to start.

[jodal]

I think this might work, but I've only had a careful read through this thread while on the move, so I haven't cross-checked with code yet.

The msgspec PR actually adds a RefType enum if we want to return that from backend.library.identify(uri) instead of a nameless Ref.

Either way, I think the crux is if we can do all this in a way that doesn't require all existing extensions to be patched.

[benreid24]

It would be nice to reuse the new RefType enum. Maintaining original behavior with un-patched extensions may be tricky. Adding a default implementation of LibraryProvider.identify that returns None (or some new value to the enum which signals "unknown") would probably be best. Returning one of the existing types by default feels wrong.

If the general approach sounds good I can put together a PR that we can iterate on.

[kingosticks]

Maintaining original behavior with un-patched extensions may be tricky. Adding a default implementation of LibraryProvider.identify that returns None (or some new value to the enum which signals "unknown") would probably be best. Returning one of the existing types by default feels wrong.

I think a default implementation returning refType Track would be OK since it would then retain existing behaviour (pass it to backend.LibraryProvider.lookup and hope for the best). It doesn't feel too weird to me, given that lookup is focused around Tracks anyway but I think I see your point.

I thought about it some more yesterday and I'm confident we can keep existing behaviour for old extensions. I was planning on splitting this up into two issues (PlaylistController.lookup and LibraryController.lookup) and closing this PR so we can focus on the underlying shortcomings.

[benreid24]

Sounds good! I will put together something soon with what I'm thinking.