mirror of
https://github.com/librespot-org/librespot.git
synced 2026-04-27 08:15:50 +03:00
[GH-ISSUE #68] [Question] Regarding the use of Sink.stop #57
Labels
No labels
A-Alsa
SpotifyAPI
Tokio 1.0
audio
bug
can't reproduce
compilation
dependencies
duplicate
enhancement
good first issue
help wanted
high priority
imported
imported
invalid
new api
pull-request
question
reverse engineering
wiki
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/librespot#57
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @sashahilton00 on GitHub (Jan 29, 2018).
Original GitHub issue: https://github.com/librespot-org/librespot/issues/68
Friday Jul 21, 2017 at 15:42 GMT
Originally opened as https://github.com/plietar/librespot/issues/224
My problem: I have a Rpi3 + Hifiberry Digi+ Pro running VolumioNext (the new build that supports Digi+ Pro) and use the volspotconnect2 that in turn uses librespot. Whether I am using TOSLINK or Coax, I get a mute relay click from my DAC (Schiit Yggdrasil) whenever
Since in the Alsa backend, the format of the stream is fixed (signed 16,44100), the only way that make the DAC relay click is by stopping the output completely and restarting the output.
I've noticed that Sink.stop were called exactly 4 times in
and when there is a Vorbis error
Would it be possible to change the stop behaviour to not cut off the whole backend? What I perceive the possible solutions could be one of the following(please correct me if I am wrong):
self.0 = None;in stop, just do nothingSink.mutefunction that outputs 0 constantly (or does nothing) and useSink.muteinstead ofSink.stopfor the first 3 scenarios. Than we can keep theSink.stopbehavior for Vorbis errors.@sashahilton00 commented on GitHub (Jan 29, 2018):
Friday Jul 21, 2017 at 17:55 GMT
Releasing the ALSA device when stopped, like other applications do, is good, do you really want to lose that? If you don't release it you end up blocking any other applications form using it (unless you configure an ALSA dmix plugin). When you've got a bunch of audio playback applications trying to get along together this is quite important.
If I remember correctly (I might be wrong), the Android Spotify client doesn't actually have a stop button, so bizarrely in this case it's nice that pause has the same behaviour as stop in regards to this.
@sashahilton00 commented on GitHub (Jan 29, 2018):
Friday Jul 28, 2017 at 08:01 GMT
In my setup, the RPi should not have other audio apps competing with librespot for audio, if it has, it would be very distracting. Another thought would be to not stop ALSA for as long as librespot is connected to Spotify, and only stop it when the connection is broken.
Or, another way to do it is to actually break out stop into pause and real stops, and implement track switches using pause instead of stops. This way the stops really release the device as behaving applications do and pause would hold on to the device dearly and not surprise the user by playing something else from another application.
@sashahilton00 commented on GitHub (Jan 29, 2018):
Friday Jul 28, 2017 at 08:48 GMT
They are not actually playing audio at the same time. Only one program is ever actually in a playing state. Generally you would have it setup so each program's on-play function sends stop commands to all the other programs. So that the one program that is actually currently using the alsa device will then stop and release it. For this setup, ensuring that stop always releases the device (like your 2nd paragraph) is important. I hope I've explained this a bit better now.
@sashahilton00 commented on GitHub (Jan 29, 2018):
Friday Jul 28, 2017 at 09:56 GMT
So would it make more sense to implement a hard .stop and .pause and use .pause for Playercommand:{Load|Pause}? I think this would be required for gapless if it's implemented too
@sashahilton00 commented on GitHub (Jan 29, 2018):
Tuesday Aug 01, 2017 at 20:36 GMT
In its early versions (when only the portaudio backend was available), librespot never closed the audio device. I wished for this to be changed, and it was merged. You can see relevant changes and discussion here: https://github.com/plietar/librespot/pull/107
Maybe you could even just revert this commit for yourself.
In the meantime, I have also learned that there are situations where it would be better to keep the audio device open, at least as long librespot is selected as speaker. @plietar, maybe this should be switchable with a command-line-switch like
--close-audio-device-on-stop true(=default)|false?@sashahilton00 commented on GitHub (Jan 29, 2018):
Wednesday Aug 02, 2017 at 04:13 GMT
I went ahead to change .stop to do nothing (thus holding the audio device). As I am running from a plugin in Volumio, the only way right now is to restart the plugin in order to release the device. Not a huge hassle for me at the moment as Spotify is my only streaming source. I get sort of pseudo gapless playback as a bonus.
Another idea is to place a hook in handle_end_of_track to start a timer of X (like X=5 or 10) second, if there isn't any message coming in after timeout (either seek() or next()), it would probably mean that user has no interaction at the end of a playlist and we can release the device. But this is well beyond my understanding of Rust at this moment.
@ComlOnline commented on GitHub (Jan 29, 2018):
@herrernst do you think a flag like the one you suggested is still needed?
@herrernst commented on GitHub (Jan 29, 2018):
I prefer the current behaviour (audio device is released when paused/stopped). If somebody wants to change that, I would wish for a switch to at least optionally get the current behaviour back.
@ComlOnline commented on GitHub (Jan 29, 2018):
Oh I agree the current behavior is what I would expect. I was meaning in respect to this:
Are there still cases where it is needed? Or are these more edge cases
@herrernst commented on GitHub (Jan 29, 2018):
I personally don't need this edge case, but maybe @leafy7382 does.
@sashahilton00 commented on GitHub (Feb 1, 2018):
I don't need it, but looking over the code (though I'm uncertain of this), it looks like https://github.com/plietar/librespot/pull/107/files#diff-f8239df44acb1007c7514a788ccc6ae6R91 is responsible for killing the audio device, so if we just add a runtime flag to disable this line, wouldn't that be enough? If so, it's a trivial change.
@awiouy commented on GitHub (Feb 1, 2018):
I use this for LibreELEC:
github.com/awiouy/librespot@6e82353ba2run_onstart() and run_onstop() are respectively called when a Spotify app switches to and from librespot. The sink could be opened and closed accordingly.
As a bonus, artist and track are provided as environment variables when a track is first loaded..
@leafy7382 commented on GitHub (Feb 18, 2018):
Wow, great to have this thread back. Yes I'd love to have this edge case even if it's via a switch. Default behaviour makes my DAC click between every track change (it's mechanical so unavoidable) and lose about 1 sec of music. With this mod and fast enough network, it's almost as good as gapless playback, this is very important for classical works like operas.
@vladvasiliu commented on GitHub (Mar 9, 2018):
Hi,
I've been having this same problem with a hifiberry digi+ (standard version) and the DAC clicking is extremely annoying.
However, I use other sources on Volumio (local drive) and the behaviour is the same: whenever I change a track, the DAC goes click. The volumio issue is here: https://github.com/volumio/Volumio2/issues/1492
My point is that this sink release business would be just a workaround, and the way librespot does it is the way other apps do it too. This might actually be a problem with the digi+ itself and / or the driver.
As detailed in the mentioned issue, the exact same DAC with the exact same optical connection but on a ICH sound card doesn't have the clicking. Direct USB input to the DAC doesn't click either.
I'm guessing the sink is handled the same way by the player, though, in all the cases.
@sashahilton00 commented on GitHub (Mar 20, 2018):
If anyone would like to implement this runtime flag, I'd be happy to merge it, but the current behaviour should remain as default.
@benoitmahe commented on GitHub (May 1, 2018):
Same issue with Allo Digione and Bifrost, so a runtime flag to change this behaviour would be really appreciated :-)
@ashthespy commented on GitHub (Jun 8, 2018):
Was also facing similar clicks/pops on each track change - so added a simple delay with a runtime flag.
github.com/ashthespy/librespot@ab48ac8802Am testing it now - but it seems to be leading some buffer underruns with alsa on my setup.
@ashthespy commented on GitHub (Mar 13, 2020):
This should be solved with gapless playback landing in #430. For people who for some reason want the old behaviour, check out #444.