Distorted audio when using dynamic normalisation

[rbuch]

Audio sounds highly distorted and seems to be clipping when using the current default dynamic normalisation added in #660. Changing back to using basic normalisation or disabling normalisation altogether fixes the issue.

(Discovered via ncspot, the corresponding issue there is: hrkfdn/ncspot#522.)

[roderickvd]

Could you please provide the following information:

1. Complete debug logs;
2. Whether the issue also occurs on vanilla librespot, all else being equal (same pregain, attack, threshold, knee, and so on);
3. As above, but now also using --backend alsa or --backend rodio instead of PulseAudio or PipeWire.

Finally, please explain what its meant by "even when in-app volume is set to 0%" in that downstream issue. As I understand it, the audio is distorted even when volume is set to 0 or mute? Does the distortion differ depending on the volume setting?

[Johannesd3]

And if you use PipeWire, which backend? IIRC ALSA, Jack, GStreamer, Pulseaudio, or Rodio would be possible. Does it happen with other backends + PipeWire as well?

[rbuch]

I did a bit of digging, I couldn't get this to reproduce in vanilla librespot with any of the backends or changing any other options. However, comparing debug output between librespot and ncspot uncovered the issue, the normalisation_threshold in ncspot is incorrect.

ncspot initializes librespot_playback::config::PlayerConfig with the following code:

let player_config = PlayerConfig {
    gapless: cfg.values().gapless.unwrap_or(false),
    bitrate: bitrate.unwrap_or(Bitrate::Bitrate320),
    normalisation: cfg.values().volnorm.unwrap_or(false),
    normalisation_pregain: cfg.values().volnorm_pregain.unwrap_or(0.0),
    ..Default::default()
};

Thus, it uses the default value for normalisation_threshold (which is -1.0) directly, whereas librespot passes the default value through NormalisationData::db_to_ratio when it constructs the object (resulting in ~0.8912509). Explicitly passing in this adjusted value in the ncspot constructor fixes the issue, otherwise the effective threshold after converting to dB in the ncspot case ends up being NaN.

So I suppose the question now is can the logic to convert the value from dB to ratio be pushed into an object creation function for PlayerConfig to avoid this problem?

[Johannesd3]

Why can't ncspot use NormalisationData::db_to_ratio themselves?

[rbuch]

Why can't ncspot use NormalisationData::db_to_ratio themselves?

It can, of course, but it feels like a leaky abstraction for an external application to have to pull a default value out, transform it, and then explicitly pass it back in when it just wants it to be default initialized.

[roderickvd]

I agree with @rbuch the default should just make sense. This was an oversight on my end. I'll update it later this weekend.

[roderickvd]

Thanks for digging around 😅 I was getting worried.

[roderickvd]

@rbuch could you please verify #749 and report back there?

[rbuch]

@rbuch could you please verify #749 and report back there?

Building ncspot with #749 does indeed fix the default value issue and it all sounds like it should. Thanks for the quick fix!