Exposes the PlayerTrackLoader

[Josh-00FF00]

Hi,

The motivation behind this is that I want to stream Audio tracks from Spotify, but I don't want the entire Player infrastructure behind it. I've already got a queueing system, and the two of them did not like each other.

By exposing this I can easily handle one Track's lifetime, instead of an entire Sink's

Let me know if there's a better way to change this so we can still get at individual tracks

Thanks!

[photovoltex]

I would have thought that the player itself can be used independently, there shouldn't be any kind of queue that keeps track of tracks as far as I'm aware of. So do you refer to the Spirc maybe? Just asking out of curiosity and missing knowledge to get a bigger picture of the use case^^.

[Josh-00FF00]

I don't think I'm refering to Spirc, but I might have missed that in my snooping around.

What I'm aiming for is to stream Audio tracks into Songbird (https://docs.rs/songbird/latest/songbird/) for a Discord bot. I originally created a Sink for respot to send the audio into, then I created a MediaSource that would read that back out for Songbird.

The issue with this approach is that Songbird wanted discrete audio tracks, but respot wanted a Sink to just send bytes down from its own player. I tried keeping tabs on when a Track ends then closing channels but that got quite hacky and was unreliable. Plus I didn't need the track handling and player stuff from respot, since Songbird required its own

After a deeper dive I made these pub, and wrote this as a source: https://github.com/Josh-00FF00/parrot/blob/feature/librespot/src/sources/librespot.rs#L152 (Excuse the hacky code!) which worked well since I can rely on the decoder to tell me when its specific track has ended in the read call

Does that sound like a reasonble use-case?

Thanks for the quick response btw, impresive!

[photovoltex]

So I did dig a bit around and found a struct named SubFile inside the load_track method of Player, which implements MediaSource out of the box. Maybe it would be nicer to separate some logic inside load_track and only create a method that only retrieves the SubFile. That might already be enough to get what you want without exposing other internal endpoints?

But I'm not so familiar with any audio logic, just saw that this might be already enough to get to your goal :)

[roderickvd]

Please note that under the Spotify ToS you are not allowed to transfer or broadcast to another network.

[photovoltex]

Good reminder! But that probably also means we don't want to expose such internal apis that would allow easier access for those kind of applications and by that promote it to some sort of degree. Unless we find a way that doesn't look like we are endorsing the transfer or broadcasting to other network.

[Josh-00FF00]

Please note that under the Spotify ToS you are not allowed to transfer or broadcast to another network.

I had a similar concern, and consequently I went back and forth on starting and publishing librespot integration into the bot. In the end I thought it would be cool, so my plan is to add a disclaimer similar to another project I looked at: https://docs.rs/crate/stdinman/latest and leave everything hanging on a branch of my fork unadvertised & un-upstreamed.

probably also means we don't want to expose such internal apis

I don't want to cause an issue for the project, and I'd be happy to close this PR out with that outcome. My thoughts are responsibility lies on those who choose to use librespot functionality in contravention of ToS, and not the library itself. But I see how this could be percieved to endorse ToS violations in general if taken too far.

As for the technical notes:

SubFile inside the load_track method of Player, which implements MediaSource out of the box.

This also caught my eye, but I didn't think it would suit since I needed to do some audio processing; specifically turning the f64 PCM samples from the Decoder into f32 samples. I don't think there's a way to specify the desired output type from the SubFile MediaSource. If there is that would be a nicer way of doing things!

[roderickvd]

Thanks for already thinking about it.

The project you linked to could already do what you want, when you use librespot’s pipe backend.

It sure is a grey line. I mean, Rust allows us to make it so should we not expose that either? Clearly it is up to users to use a tool responsibly, but that does not mean that all tools are equally available.

[photovoltex]

Just so I got that right, you would be fine with exposing those internal items? And let users decide if they want to go into that gray zone, instead of enforcing/setting clear boundaries?

not like they couldn't do it anyways by fork it and including their forks as dependency

---

This also caught my eye, but I didn't think it would suit since I needed to do some audio processing; specifically turning the f64 PCM samples from the Decoder into f32 samples. I don't think there's a way to specify the desired output type from the SubFile MediaSource. If there is that would be a nicer way of doing things!

Ah I see. Well I'm not quite familiar with the code myself. So I can't really help here in detail. I also stumbled over the passthrough decoder, which seems to change the AudioPacket used from Samples(f64) to Raw(u8). But I don't think thats of any use here. Just wanted to not here if that may be of any use.

[kingosticks]

@Josh-00FF00 have you seen the BufferSink used in spotifyaudiosrc? Might that method work for you?

[Josh-00FF00]

@kingosticks Oh wow I did not know that GStreamer had a spotify plugin. That's awesome, probably wouldn't have dived into adding librespot as a cargo dep if I knew I could play a single track using a CLI tool. But I've got this far, and seen behind the curtain!

Interestingly I think this is an example of another project that would appreciate this PR/something else that exposes a single track interface, instead of a full Player. The linked function is close to the dance I attempted using a Sink and a Tokio runtime in another thread to handle Track termination. Which does work, but seemed needlessly inefficient spawning runtimes, starting threads, and blocking on channels when I realised I only needed the load_track function which has most of the logic already.

So I'd prefer to be able to access PlayerTrackLoader::load_track directly, but if that's not deemed viable I'll definitely look at incorporating gstreamer-spotify directly, or borrowing their approach. And probably their disclaimer as well :)

EDIT: Ah I realise you've contributed to spotifyaudiosrc, please replace my "I think this is an example" with your actual thoughts

[roderickvd]

librespot-playback is actually an "example" that grew out of of hand 😆

On the one hand you could say "why not make it public if it helps", but on the other hand I don't like exposing fields whose state is handled by the struct methods itself. It may work for your use-case, but is confusing and enables foot-shooting for the general case.

Beyond piping to stdout with librespot's pipe backend, @kingosticks triggered me on another idea: why not create a separate "audio backend" if you need something specific? Although I imagine that piping with passthrough gives you exactly the encoded audio file that you need.

[Josh-00FF00]

why not create a separate "audio backend"

Is that the Sink trait? I did give that a go but found it a awkward to use.

On the one hand you could say "why not make it public if it helps", but on the other hand I don't like exposing fields whose state is handled by the struct methods itself. It may work for your use-case, but is confusing and enables foot-shooting for the general case.

I understand, I aimed for minimal code change but I'm happy to do a bigger refactor if it would be better. I could add a new method to the PlayerTrackLoader and hide their internals, and/or move the thread spawning (https://github.com/librespot-org/librespot/blob/dev/playback/src/player.rs#L2209) into a new function?

What do you think would be a good interface to expose here?

[roderickvd]

why not create a separate "audio backend"

Is that the Sink trait? I did give that a go but found it an awkward to use.

Yes.

I understand, I aimed for minimal code change but I'm happy to do a bigger refactor if it would be better. I could add a new method to the PlayerTrackLoader and hide their internals, and/or move the thread spawning (https://github.com/librespot-org/librespot/blob/dev/playback/src/player.rs#L2209) into a new function?

I do remember that when I worked on the thread spawning, it was hackish, I knew little of Futures and JoinHandles, and was already glad it compiled. If you manage to come up with something that fits the general case as well as your particular one, by all means propose it.