search

librespot-org / librespot

Open Source Spotify client library
MIT License
4.8k stars 603 forks source link

add gstreamer as audio backend #28

Closed sashahilton00 closed 4 years ago

sashahilton00 commented 6 years ago

Issue by psych0d0g Sunday Mar 13, 2016 at 22:14 GMT Originally opened as https://github.com/plietar/librespot/issues/60

https://github.com/arturoc/gstreamer1.0-rs

Since portaudio has some bugs on mipsel i would like to request the addition of gstreamer,

Pros: LGPL lib. X-Platform: Linux, Android, Windows, Max OS X, iOS, as well as most BSDs, commercial Unixes, Solaris, and Symbian X-Architecture: x86, ARM, MIPS, SPARC and PowerPC, on 32-bit as well as 64-bit, and little endian or big endian Complete debugging system for both core and plugin/app developers

List of applications gstreamer is used in: https://gstreamer.freedesktop.org/apps/

And now: Open for discussion :)

sashahilton00 commented 6 years ago

Comment by plietar Monday Mar 14, 2016 at 03:36 GMT

GStreamer is a powerful but pretty big dependency, both in terms of size and complexity. I'd rather avoid it if possible. Nevertheless, I've refactored the Audio Output code to use a Sink trait. This makes it easier to add new backends for different platforms. There's an initial ALSA implementation on the alsa branch. It is only used on linux, other platforms still use portaudio.

sashahilton00 commented 6 years ago

Comment by joerg-krause Monday Mar 14, 2016 at 05:51 GMT

It's good news that you add an audio backend. Gstreamer is not applicable for low-end embedded devices, but I am glad about the ALSA implementation. I'd like to give it a try.

sashahilton00 commented 6 years ago

Comment by psych0d0g Monday Mar 14, 2016 at 11:18 GMT

@joerg-krause what is a low-end embedded device by your definition? for me my VU+ Ultimo applies as one (400mhz mips dualcode sock) and all of its multimedia features are based on gstreamer, which works just fine here.

sashahilton00 commented 6 years ago

Comment by joerg-krause Monday Mar 14, 2016 at 12:42 GMT

@psych0d0g I mean something like the Carambola 2 with 16 MB Flash and 64 MB RAM.

sashahilton00 commented 6 years ago

Comment by psych0d0g Tuesday Mar 15, 2016 at 13:29 GMT

well now with the sink api i would vote for gstreamer as additional sink backend

sashahilton00 commented 6 years ago

Comment by psych0d0g Thursday Mar 17, 2016 at 20:14 GMT

so now after some more research from my side: this is an example pipeline that works on enigma2 mipsel: gst-launch-1.0 -v -m audiotestsrc ! audioconvert ! dvbaudiosink

the issues im facing are not mipsel specific but completely enigma2 specific. I would like to have a feature switch introduced: --with-features enigma2 which enables the audio output via gstreamer on compile time.

sashahilton00 commented 6 years ago

Comment by psych0d0g Thursday Mar 17, 2016 at 20:28 GMT

i pushed my work so far here: https://github.com/psych0d0g/librespot/tree/alsa the build switch is working, aswell as the inclusiong of gstreamer rust bindings. whats not working is the gstreamer audio backend itself so far. i cant figure out what needs to be returned from the gstreamer function in order to accept audio data

sashahilton00 commented 6 years ago

Comment by psych0d0g Friday Mar 18, 2016 at 15:43 GMT

so with a little help from @plietar and https://github.com/astro , there is now a basic working gstreamer branch: https://github.com/plietar/librespot/tree/gst compile with --features enigma2 for now

sashahilton00 commented 6 years ago

Comment by plietar Friday Mar 18, 2016 at 15:47 GMT

See my comment there https://github.com/plietar/librespot/issues/63#issuecomment-198417728 This will have to wait until I reorganise the audio backends

sashahilton00 commented 6 years ago

@psych0d0g this would be a cool feature to have in librespot. Would you be interested in porting it to librespot now it's matured a bit by any chance?

kingosticks commented 5 years ago

This is great, I can port this.

sashahilton00 commented 5 years ago

I'm not sure we want it though? See paul's above point about dependency size, and also it's yet another audio backend we have to support...

although frankly, we have so many backends now, as long as it's conditionally compiled, I'm not sure we care. Anyone else have any thoughts?

kingosticks commented 5 years ago

This can replace the pulse and alsa backends and give you some others for free. I'm not sure about what there is on the mac side of things.

I'll take a look at it anyway, it's fine if we don't want to merge (although I personally see no harm as your edit says). If it proves unpopular then can just chuck it out as part of the move to librespotd!

sashahilton00 commented 5 years ago

Yeah, that works for me. Let's not lose the Alsa + PulseAudio backends atm, I suspect some people are using them, brings this to mind https://xkcd.com/1172/. Ultimately we'll strip out the cruft when we start work on the daemon, at which point we'll need to add the cpal/rodio stuff anyway. Having taken a quick look at it, we can also strip out most of the decoding stuff like lewton if we move to rodio as well, since all of that is handled by the package.

allquixotic commented 4 years ago

Well guys, I finally got a gstreamer-1.0 backend working with the latest gstreamer bindings! :) https://github.com/allquixotic/librespot/tree/gst1.0-2020

The porting effort from plietar's first PoC was pretty extensive because of all the changes to GStreamer and even the librespot sink API, but it wasn't too bad. I've only barely tested it at this time but I know it works.

I wouldn't say it's ready to merge, but it definitely works. I think this backend brings unique capabilities to the table that others don't, like the ability to do local DSP (equalizers, reverb or whatever) and people can build programs to customize where the sound goes or what it sounds like based on the pipeline, which is customizable at daemon launch time by setting the device parameter.

And yes, it is trivial to pass a device argument that ends in filesink and you all know what that means, but it's pretty useless because each song would overwrite the same file and there's no mechanism in the code to change the filename with each track (intentionally). So for that one "possibly tenuous" use case, it's still not possible to do it with unmodified binaries -- at least, no more or less possible than the pipe backend or the pulseaudio backend.

I don't suggest removing other backends, but at least this opens up entirely new backends that haven't been considered before that happen to be implemented as a sink in gstreamer.

kingosticks commented 4 years ago

Fantastic. I'll give this is a go in the next few days. Thanks for your efforts.

allquixotic commented 4 years ago

BTW, I also got it to compile for Windows by enabling the x86_64-pc-windows-gnu target on Rust 1.40 on Fedora 31. The gstreamer bundled with this has gst-plugins-base, good and bad with nearly all plugins enabled that make any modicum of sense on Windows (and some that don't, like pulseaudio). It doesn't have -ugly because I didn't need anything from it for my use case and also because that would make for a major redistributability problem when I'm sharing this to folks :P https://drive.google.com/file/d/1f8mKh1V2n936aIAM-iWConYUtmkuCmPQ/view?usp=sharing

allquixotic commented 4 years ago

Quick update: I found that the behavior on track change was pretty bad: you would hear a hiccup when the player called stop() on the GstreamerSink, which would set the pipeline state to PAUSED; then it would be set back to PLAYING via start() on the sink -- and because transitions between PAUSED and PLAYING do not flush buffers, the last bit of the previous song would be played after the hiccup, then the next song would start. That's clearly not a good behavior.

If the audio sink of librespot is allowed to block, probably the correct behavior is to blocking flush the buffers and then set the state of the pipeline to READY in stop().

I just made a commit in my branch that improves the behavior a bit: stop() transitions the pipeline to READY state, and I made buffer handling errors in the push_buffer() thread non-fatal (because you can't push buffers to the AppSrc when your pipeline is in READY state and this thread may still be processing data when you get a stop() call).

I'm going to iterate some more and see if I can get it to handle pause, track change, and track end events cleanly. The Sink API might be improved by adding a pause handler. I'll check if it's there but I just missed it, and if not, it may need to be added. In Gstreamer there is a significant difference between pause and stop behavior.

With the latest commit I have as of this writing, the remaining bugs are certainly suggesting that the buffers queued up when stop() is called are just being discarded instead of drained:

  1. When you click pause in your Spotify GUI, then play again, you miss out on some audio (this depends on the latency of your gst pipeline, but probably at least 0.2 seconds, and more if you have queues, etc. in the pipeline or a high-latency sink.)

  2. When a track ends, the respot player calls stop(), which causes gst to discard whatever buffers have been pushed but not drained in the sink.

UPDATE: The latest commit is about the best I can do for now:

The delay is a lot less noticeable on low-latency sinks like using PulseAudio with time-based realtime scheduling on Linux, compared to DirectSound on Windows, where the default sink latency is quite high to protect against dropouts. If we had a separate pause operation for the Sink, it would be possible to eliminate this delay by manipulating the pipeline state. As it stands, the disadvantages of changing the pipeline state are worse than just leaving it at PLAYING all the time and just blocking in the AppSrc when there's no data to send.

It's not too bad, though, especially considering the advantages of gapless playback.

plietar had the right idea when he worked on this in 2016. It just needed a few years of love for the gstreamer-rs bindings and a little bit of tuning. Anything I'm missing still? Well, documentation, and tests...

allquixotic commented 4 years ago

kaymes' gapless code (that does some kind of clever time syncing) is nearly magic; it actually fixed most of the major issues I was having with this backend. So I cleaned it up and submitted it as a PR.

gdesmott commented 2 years ago

FYI I implemented a gstreamer spotify plugin adding a new spotify source element. It should make spotify integration more inlined with GStreamer's design. The element is quite simple at the moment but I'm happily taking suggestions and patches to improve it to fit your needs.