Spotifyd / spotifyd

A spotify daemon
https://spotifyd.rs
GNU General Public License v3.0
9.83k stars 450 forks source link

Unecessary dithering? #1277

Closed sad-goldfish closed 4 months ago

sad-goldfish commented 5 months ago

Description It looks like either Spotifyd or Librespot is dithering unnecessarily for S32 and F32 audio formats. As described in Librespot here, dithering shouldn't be used in these cases. Librespot also says here that it should default to none in these cases, so it could well be an upstream issue. I am posting it here to rule out the possibility that it is a downstream issue before reporting it upstream.

IMO, if the Pulseaudio backend is used, the default audio format should be F32 and dithering should be disabled as, AFAIK, Pulseaudio or Pipewire should be able to transcode/dither if (and only if) needed. Where it is not needed, this should be the configuration with the least potentially lossy operations. Although, the Spotify stream is only 16 bit in the first place so someone more familiar with the codebase (and in particular what the decoded stream looks like in librespot) would have to decide how much change in quality this actually makes.

To Reproduce When I run Spotifyd with the Pulseaudio backend and S32 audio format, I get

Loading config from "/home/$USER/.config/spotifyd/spotifyd.conf"
No proxy specified
Using no volume controller.
Ignoring blacklisted access point SOMETHING.spotify.com:443
Connecting to AP "SOMETHING_ELSE.spotify.com:443"
Authenticated as "$USER" !
Country: "COUNTRY"
Converting with ditherer: tpdf
Using PulseAudioSink with format: S32
Couldn't fetch metadata from spotify: Nothing playing at the moment.
Couldn't fetch metadata from spotify: Nothing playing at the moment.

So the tpdf ditherer is used. I get similar output on F32.

Expected behavior No Converting with ditherer: tpdf message.

Logs Not providing verbose logs so that I don't dox myself. Let me know if you can't reproduce or need any specific information.

Compilation flags

Versions (please complete the following information):

Additional context The patch below (against 0.3.5) explicitly disables dithering.

diff --git a/src/config.rs b/src/config.rs
index d81fb16..3653a39 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -833,6 +833,7 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig {
         normalisation: config.shared_config.volume_normalisation,
         normalisation_pregain_db: normalisation_pregain,
         gapless: true,
+        ditherer: None,
         ..Default::default()
     };