Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ChuckMash
Copy link
Contributor

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
Copy link
Contributor Author

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

@lyusupov
Copy link

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
Copy link
Contributor Author

ChuckMash commented Dec 17, 2023

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
Copy link
Contributor Author

#406 (comment) is relevant.
This user also suggests not calling SetRate unnecessarily to avoid a noisy click.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants