Gapless play and next button

[jrfaller]

Hi! First thanks a lot for this amazing project! I use it daily and I really love it!

I have a USB DAC that emits a small pop when an audio sink is released or created. Fortunately, librespot uses gapless playback and therefore when I listen to a song till the end and the next song is played, I do not get this pop.
However, I have remarked that when I click on the next button on my Spotify client, there is a pop that is emitted, meaning that in this case, there is no gapless play? I was not able to find out where to look in the code to be sure.

Would it be possible to enable gapless play also when clicking the next button? I imagine that the problem is that there is no preloaded data for the next song in this case, but would it be possible to keep the sink open?

Cheers!

[kingosticks]

I think https://github.com/librespot-org/librespot/blob/dev/playback/src/player.rs#L941 is the culprit but I don't know/remember why that

In any case we go into a Loading state to load the track

means we also need to stop the sink (and cause your pop).

It might be possible to trigger TimeToPreloadNextTrack event (or similar) before we do the actual load but it probably needs something blocking. Maybe something like preload_data_before_playback() e.g. preload_data_before_next(). These both feel like sticking plasters.

[jrfaller]

Thanks for the pointer!

Fun fact, if I wait to be near the end of the song (a little after the total time of the song minus PRELOAD_NEXT_TRACK_BEFORE_END_DURATION_MS) and I click next, then there is no pop, meaning there is probably enough data to make a gapless play. Therefore, it makes me wonder two things:

• I am not sure why it's a good thing to wait this long before preloading the data since triggering the next button can happen anytime. Could data be loaded as soon as the buffer for the current song is full?
• When clicking next, it is expected that there will be a new song to be played, so even with no available data for the next song, why release the sink?

Maybe it would be a good idea regardless of my pop problem to have a "gapless" next for instance if one-day crossfading gets implemented.

Cheers!

[roderickvd]

As part of the shift to the HTTP CDN in dev, I also rewrote large parts of the preloading logic. Unfortunately I got little to no feedback at that time, so not sure if this is a regression or not.

For purpose of diagnostics, would you please try the latest 0.4 release and see what happens there?

I think https://github.com/librespot-org/librespot/blob/dev/playback/src/player.rs#L941 is the culprit but I don't know/remember why that

In any case we go into a Loading state to load the track

means we also need to stop the sink (and cause your pop).

I also think that's wrong. This already deals with this correctly:

librespot/playback/src/player.rs

Line 1773 in 054074c

if !self.config.gapless {

So I think this would be safe to remove:

librespot/playback/src/player.rs

Line 1915 in 054074c

self.ensure_sink_stopped(play);

It might be possible to trigger TimeToPreloadNextTrack event (or similar) before we do the actual load but it probably needs something blocking. Maybe something like preload_data_before_playback() e.g. preload_data_before_next(). These both feel like sticking plasters.

Fun fact, if I wait to be near the end of the song (a little after the total time of the song minus PRELOAD_NEXT_TRACK_BEFORE_END_DURATION_MS) and I click next, then there is no pop, meaning there is probably enough data to make a gapless play. Therefore, it makes me wonder two things:

• I am not sure why it's a good thing to wait this long before preloading the data since triggering the next button can happen anytime. Could data be loaded as soon as the buffer for the current song is full?

This preloading implementation, like the previous one, estimates when it should start downloading depending on network throughput and time left. So it's optimised to save bandwidth. I never gave it much thought, but I think you are right. In this age there is little reason not start the preload immediately. Though I'm no longer an active developer, from what I remember this should be easy to do.

Of course, it's also entirely possible that a user selects a "next" song which is not next in queue. That could still trigger your pop. It never did for me, so I guess it depends on the output device. So an alternative to preloading immediately, covering this case as well, is to a quick mute-unmute cycle when manually changing songs. Somewhere in the area of 20 ms ramp time should work fine. This will take a little more effort to put in.

[jrfaller]

Thanks for the detailed answer!

For purpose of diagnostics, would you please try the latest 0.4 release and see what happens there?

I ran into the issue with the following release on moodeaudio: [2023-10-25T18:31:33Z INFO librespot] librespot 0.4.2 22f8aed (Built on 2022-08-18, Build ID: LQ4hikXT, Profile: release) would you like me to try with other releases?

This preloading implementation, like the previous one, estimates when it should start downloading depending on network throughput and time left. So it's optimised to save bandwidth. I never gave it much thought, but I think you are right. In this age there is little reason not start the preload immediately. Though I'm no longer an active developer, from what I remember this should be easy to do.

I believe that it would be nice to have a more eager strategy for preloading since users with bad bandwidth can disable gapless playing

Of course, it's also entirely possible that a user selects a "next" song which is not next in queue. That could still trigger your pop. It never did for me, so I guess it depends on the output device. So an alternative to preloading immediately, covering this case as well, is to a quick mute-unmute cycle when manually changing songs. Somewhere in the area of 20 ms ramp time should work fine. This will take a little more effort to put in.

The pop is clearly a design mistake from my DAC manufacturer, I totally can live with a pop if my playlist has ended, and I do not have autoplay 😆

[roderickvd]

Yeah in that case please try compiling from dev?

[kingosticks]

covering this case as well, is to a quick mute-unmute cycle when manually changing songs.

That's a nice idea.

[jrfaller]

covering this case as well, is to a quick mute-unmute cycle when manually changing songs.

That's a nice idea.

Oooh maybe I did not understand the proposal, it means that when clicking the next button librespot would mute / load the next song / unmute instead of recreating a sink?

Yeah in that case please try compiling from dev?

Ok tried that, surprisingly easy on the raspberry. Got [2023-10-26T18:36:41Z INFO librespot] librespot 0.5.0-dev 054074c (Built on 2023-10-26, Build ID: GBF8F7wi, Profile: debug) running. Unfortunately, ran over some ERROR librespot_core::mercury] error 403 for uri hm://keymaster/token/authenticated?scope=playlist-read&client_id= errors that I do not understand because connexion and authentication seemed to work fine (and got no sound to test the pop).

[jrfaller]

Status update: got librespot working fine using @kingosticks fix for #1179: librespot] librespot 0.5.0-dev 1896611 (Built on 2023-10-27, Build ID: Rjfa9F0L, Profile: debug).

On this version, the behavior is the same as 0.4.2.

I tried removing self.ensure_sink_stopped(play); as suggested by @roderickvd and 🥁 there is no longer a pop when clicking next, which fixes my problem 👍🏻

[jrfaller]

Another testing update: not only does the removal of self.ensure_sink_stopped(play); fix the sink closing when clicking on the next button but it also avoids closing the sink when directly changing to another song when a song is playing. As far as I can tell, no regression was detected!

[roderickvd]

Could you put in a PR to fix it for everyone?

[jrfaller]

Of course! Here it is at #1223