Repository: Spotifyd/spotifyd (language: rust)

Issue:
Unecessary dithering?
**Description**
It looks like either Spotifyd or Librespot is dithering unnecessarily for S32 and F32 audio formats.  As described in Librespot [here](https://github.com/librespot-org/librespot/blob/6537c441c3082aa2a7bbfe20e2afc316962dfc6a/playback/src/dither.rs#L26), dithering shouldn't be used in these cases. Librespot also says [here](https://github.com/librespot-org/librespot/blob/6537c441c3082aa2a7bbfe20e2afc316962dfc6a/src/main.rs#L405) 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.

<!-- if you compiled spotifyd yourself. Alsa backend enabled by default unless compiled with the `--no-default-features` flag -->
**Compilation flags**
- [x] dbus_mpris
- [x] dbus_keyring
- [x] alsa_backend
- [ ] portaudio_backend
- [x] pulseaudio_backend
- [ ] rodio_backend

**Versions (please complete the following information):**
<!-- DO NOT use words like `latest`. Please specify the exact version/commit hash -->
- OS: Gentoo
- Spotifyd: 0.3.5
- cargo: 1.75.0

**Additional context**
The patch below (against 0.3.5) explicitly disables dithering.
```diff
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()
     };
```


Discussion / hints:

Fix patch:
diff --git a/src/config.rs b/src/config.rs
index 0cfa0b1e..b62adf40 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -8,8 +8,9 @@ use gethostname::gethostname;
 use librespot_core::{
     cache::Cache, config::DeviceType as LSDeviceType, config::SessionConfig, version,
 };
-use librespot_playback::config::{
-    AudioFormat as LSAudioFormat, Bitrate as LSBitrate, PlayerConfig,
+use librespot_playback::{
+    config::{AudioFormat as LSAudioFormat, Bitrate as LSBitrate, PlayerConfig},
+    dither::{mk_ditherer, DithererBuilder, TriangularDitherer},
 };
 use log::{error, info, warn};
 use serde::{de::Error, de::Unexpected, Deserialize, Deserializer};
@@ -829,6 +830,14 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig {
         None => info!("No proxy specified"),
     }
 
+    // choose default ditherer the same way librespot does
+    let ditherer: Option<DithererBuilder> = match audio_format {
+        LSAudioFormat::S16 | LSAudioFormat::S24 | LSAudioFormat::S24_3 => {
+            Some(mk_ditherer::<TriangularDitherer>)
+        }
+        _ => None,
+    };
+
     // TODO: when we were on librespot 0.1.5, all PlayerConfig values were available in the
     //  Spotifyd config. The upgrade to librespot 0.2.0 introduces new config variables, and we
     //  should consider adding them to Spotifyd's config system.
@@ -837,6 +846,7 @@ pub(crate) fn get_internal_config(config: CliConfig) -> SpotifydConfig {
         normalisation: config.shared_config.volume_normalisation,
         normalisation_pregain_db: normalisation_pregain,
         gapless: true,
+        ditherer,
         ..Default::default()
     };
 


Test patch: