librespot-org / librespot

Open Source Spotify client library
MIT License
4.48k stars 542 forks source link

Unbuffered sinks write packets as they become availible ending track after a few seconds #1195

Closed JoeyEamigh closed 10 months ago

JoeyEamigh commented 11 months ago

Describe the bug When using a sink that doesn't control the rate at which audio is received such as the pipe sink, packets are written as soon as they are received, causing the song to end after just a few seconds of playback. Same issue occurred with gstreamer when piping to fakesink, if that helps.

This is a resurfacing of #521.

To reproduce Steps to reproduce the behavior:

  1. Launch librespot with '--backend pipe --device /dev/null'
  2. Connect and press play on a song
  3. Observe as the song is skipped in just a few seconds.

Log The verbose log doesn't help much here, as everything logged is working as it should. I was able to narrow down the issue to this line: https://github.com/librespot-org/librespot/blob/f037e46aee631837a0553ccfdbc7866752fd0f5d/playback/src/player.rs#L1660

By timing the function when using the alsa or pulseaudio backend, sink.write takes about 40ms a packet with default settings, whereas the same function with the pipe backend piping to /dev/null will complete the write in about 220μs. Once the packet write is complete, the loop inside impl Future for PlayerInternal will restart, immediately starting the process of passing the next packet.

Host (what you are running librespot on):

Additional context Unfortunately, I am slightly out of my depth when it comes to properly timing all of the different types of audio packets that librespot supports, nor do I understand the intricacies of the different audio backends and how they handle buffering and such. Pull request #659 ascribed this to the decoder, which does not seem to be the case because even when using symphonia packets are written at the same speed, even if it takes marginally longer to decode them.

The throttling could happen in the decoder if determined appropriate, but that may become a bottleneck on lower-end chips. Instead, I am going to explore passing the packet position to the sink's write function and throttling it from there. I will publish my work on a fork, but since I do not know the extend this affects other backends nor why it works with some fifo pipes, I will let y'all point me in the right direction as to how to upstream a comprehensive fix.

This took a lot of debugging to narrow down the issue, so I hope it can help improve this amazing tool!

kingosticks commented 11 months ago

This has come up a few times before, I can't find the other examples now. The key point here which you've already mentioned is that this is all working as intended and we've previously not considered this a bug. It's always been up the consumer to block librespot, that's the correct way to use the library.

JoeyEamigh commented 10 months ago

Thank you for the response!