earlephilhower / ESP8266Audio

Arduino library to play MOD, WAV, FLAC, MIDI, RTTTL, MP3, and AAC files on I2S DACs or with a software emulated delta-sigma DAC on the ESP8266 and ESP32
GNU General Public License v3.0
2.02k stars 432 forks source link

Don't SetRate() if hz already matches, fixes noisy click #656

Open ChuckMash opened 9 months ago

ChuckMash commented 9 months ago

Addresses #655 and likely others.

I tracked down a nasty click sound when using mixer to mix 2 stubs with MP3s , one is an long ongoing background track and the other is various sound clips played at various times.

The issue is that there is a audible click each time the second stub with the sound clip is used.

The issue seems to be related ultimately with calling i2s_set_sample_rates((i2s_port_t)portNo, AdjustI2SRate(hz)); for the second stub while the first stub is still running.

This PR adds a quick check to see if rate actually needs updating or if it matches what is currently set, eliminating the above issue as long as rates match.

ChuckMash commented 9 months ago

Might also be useful in AudioOutputPWM.cpp and AudioOutputSPDIF.cpp, but I am unable to check those at the moment.

lyusupov commented 9 months ago

The patch will likely have negative effect for 44100 Hz sample rate setting: https://github.com/earlephilhower/ESP8266Audio/blob/master/src/AudioOutputI2S.cpp#L48

SetRate() will ignore to apply 44100 Hz sample rate setting upon very first run.

ChuckMash commented 9 months ago

The patch will likely have negative effect for 44100 Hz sample rate setting

I am using 44100 Hz sample rate, with no ill-effects. In this case, the default i2s sample rate might already match 44100, but I suppose that may not always be the case.

I can add check for if i2s rate has ever been set, and allow the initial set to occur.

ChuckMash commented 9 months ago

https://github.com/earlephilhower/ESP8266Audio/issues/406#issuecomment-1736466226 is relevant. This user also suggests not calling SetRate unnecessarily to avoid a noisy click.