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

click / pop sound at beginning and end of sound playback #406

Open
ChrisVeigl opened this issue Jul 6, 2021 · 26 comments
Open

click / pop sound at beginning and end of sound playback #406

ChrisVeigl opened this issue Jul 6, 2021 · 26 comments

Comments

@ChrisVeigl
Copy link

ChrisVeigl commented Jul 6, 2021

hi,

i read several issues about the click/pop noise when playing short sounds on the ESP32 via AudioOutputI2S, including
#68
#230
#367

As none of the above threads clearly shows a way how to slove the problem, i was wondering if it is possible at all (in software) - or why the issues have been closed.

I tried to remove the calls to i2s_zero_dma_buffer((i2s_port_t)portNo);
in

i2s_zero_dma_buffer((i2s_port_t)portNo);

and
i2s_zero_dma_buffer((i2s_port_t)portNo);

as suggested, which slightly improved the situation: click noise not as harsh, but somehow another kind of noise remains even after the playback has finished.... so not really a solution. (or am i missing something?)

Is there a strategy to fix this (i seems to be a showstopper for several applications where the noise is very annoying).

thank you for this great library,
chris

@ChrisVeigl
Copy link
Author

hi,

after a few experiments I found a work-around which perfomes quite nicely:

  • removed the calls to i2s_zero_dma_buffer as suggested
  • added a call to i2s_set_dac_mode(I2S_DAC_CHANNEL_DISABLE) when playback is finished
  • prevented repeated calls to i2s_driver_install (when already installed after first playback)
  • enabled the DAC only after an adjustable number of transferred samples (i chose 100 which is unnoticable for my sound files)

now sequential playback of multiple wav files works seamlessly :-)

here the commit, please check for possible side-effects:
ChrisVeigl@1ce70dc

@darkbull
Copy link

@ChrisVeigl Good job, your code works.

@ChrisVeigl
Copy link
Author

thanks! :-)
great to hear that the patch is useful!

@mazWaz
Copy link

mazWaz commented Oct 27, 2021

If you use ESP32 with internal dac use this code by Bitluni ULPSound
These are the generic instructions:

//Instructions:
AudioOutputULP out = new AudioOutputULP(); // stereo
//Connect left channel on pin 25
//Connect right channel on pin 26
//OR
//Connect mono channel on either of them (stereo samples are downmixed)
AudioOutputULP out = new AudioOutputULP(1); // mono, only DAC 1
//OR
AudioOutputULP out = new AudioOutputULP(2); // mono, only DAC 2

Just pick any example, include the AudioOutputULP.h file and replace the output object.

I would recommend starting with: PlayWAVFromPROGMEM.ino

Replace line 10 with #include "AudioOutputULP.h"

Then replace line 17 as follows: AudioOutputULP *out; and line 28: out = new AudioOutputULP();

@Koxx3
Copy link

Koxx3 commented Nov 21, 2021

Hi !
I applied the patch, but I still have issues :
image

image

any idea why the signal doesn't remains at 1.66V when no more sound to output ?

@ChrisVeigl
Copy link
Author

i didn't connect an oscilloscope by now - so i've no idea if the signal behaves the same on the M5Stack where I have deployed my application... I suppose this can be heard as click/pop noise ?
my application is now running smooth - also when short sounds are started/stopped/changed frequently.

@Koxx3
Copy link

Koxx3 commented Nov 21, 2021

yes, a nasty plop noise 😅

@ChrisVeigl
Copy link
Author

sounds as it looks ;-)

strange! did you use this fork?
https://github.com/ChrisVeigl/ESP8266Audio
as I find the time I'll hook up an oscilloscope and have a look!

BTW: why do you delete the file (and how could that work if FILENAME doesn't change)?

@Koxx3
Copy link

Koxx3 commented Nov 21, 2021

i applied the patch. i didn't see the fork 😉
i'll try it. in case i missed something.
it's just a repeating test sound for testing.

@ChrisVeigl
Copy link
Author

the fork has no other changes than the patch, though ..

@softhack007
Copy link
Contributor

softhack007 commented Jan 10, 2022

🤔 hmmm ... this could be a tricky one. It seems that several problem reports are basicially complaining about similar "pops and clicks". My guess is that the problem needs different solutions, depending on the output device that is used. Actually I don't have a solution (yet), but I played around with my version of ESP8266Audio (ESP32, internal DAC, arduino-esp32 version 1.0.6). and tried to understand some documentation.

a) external I2S DAC:

  • high resolution, 16Bit or more
  • usually they are working on "AC" level, meaning that samples have to be centred around 0
  • Some DACs have an explicit "HARD mute" PIN that could be controlled by using dedicated GPIO signal
  • PCM5102 based ones even have a PIN for "soft mute" - doing a kind of fade-in/fade-out on the first/last ~120 samples
  • DACs can detect "silence" (longer period of "0") and mute their output automatically
  • my conclusions:
    • i2s_zero_dma_buffer makes sense for these devices - it creates silence and allows the DAC to go into 'mute'
    • Possible improvement for the AudioOutputI2S class: allow control of "mute" or "soft mute" lines
    • i2s_start(portNo) and i2s_stop(portNo) could be useful - but would need some testing to confirm

b) internal DAC (aka analogue output pin)

  • low resolution - typically 8 bit
  • at least in arduino-esp32 v 1.0.6, we have to send "DC" level samples to the I2S driver - centred around 32678 (so-called DC component)
  • no mute, no way to detect "silence"
  • depending on your amp board, you might get additional "pops" when the DC component suddenly changes. For example by instantly changing from "0V" (default output PIN level) --> "1.6V" (silence level for audio).
  • my observations:
    • 👍 avoiding to re-install the i2s audio driver (after output->stop) removes some loud pops and clicks after playback, and before consecutive playback of another file
    • 🤔 In the Audio generator classes, using explicit output->flush() before output->stop() sometimes helps (but introduces a small delay)
    • i2s_zero_dma_buffer causes pops/clicks, because it drives the internal DAC to 0.0V (while "silence" should be ~1.6V)
    • I tried to add i2s_start() and i2s_stop() inside AudioOutputI2S::stop() and AudioOutputI2S::begin() -> causes reproducible pops/clicks between playback (see click / pop sound at beginning and end of sound playback #406 (comment))
    • same (bad) results with explicit use of i2s_set_dac_mode(I2S_DAC_CHANNEL_DISABLE) / i2s_set_dac_mode(I2S_DAC_CHANNEL_BOTH_EN)

@earlephilhower : what do you think?

@ChrisVeigl
Copy link
Author

i might add that the patch ChrisVeigl/ESP8266Audio@1ce70dc
works flawlessly in our application with the M5Stack, where we play many short wav/mp3 files sequentially in order to create sentences. (The M5Stack uses an NS4168 I2C audio amp for driving the speaker, see http://docs.m5stack.com/en/core/core2).

@softhack007
Copy link
Contributor

@ChrisVeigl the NS4168 is an "external DAC" with built-in amplifier, correct? So the parts of your patch related to INTERNAL_DAC are not relevant for that configuration?

@ChrisVeigl
Copy link
Author

yes - but i think this would just be the line https://github.com/ChrisVeigl/ESP8266Audio/blob/1ce70dcd14a70cfe5ebe899e761cfc4f9e3ec278/src/AudioOutputI2S.cpp#L207

I did not try the patch for other configurations. I suppose @Koxx3 is using the internal DAC - where the clicks/pops still happen...

should we try to alter i2s_zero_dma_buffer so that 32678 is used instead of 0 (in case the internal DAC is in operation)?

@softhack007
Copy link
Contributor

Hi,
not sure if i2s_zero_dma_buffer() can be made into filling the buffers with anything different than 0 - it's part of the ESP-IDF, so we can't change it.

.

About the problem from @Koxx3, my recommendation would be

  1. in the user sketch, add out->flush(); directly before mp3->stop();. This will ensure that the complete sound is played, including all samples still queued in the I2S internal buffers.

  2. in the patch from @ChrisVeigl
    a. re-enable the first i2s_set_dac_mode(I2S_DAC_CHANNEL_BOTH_EN); in AudioOutputI2S::begin
    --> https://github.com/ChrisVeigl/ESP8266Audio/blob/1ce70dcd14a70cfe5ebe899e761cfc4f9e3ec278/src/AudioOutputI2S.cpp#L207

    b. comment out the calls to i2s_set_dac_mode(...) in AudioOutputI2S::ConsumeSample and AudioOutputI2S::stop, because these are likely to cause "pops" on INTERNAL_DAC
    --> https://github.com/ChrisVeigl/ESP8266Audio/blob/1ce70dcd14a70cfe5ebe899e761cfc4f9e3ec278/src/AudioOutputI2S.cpp#L271
    --> https://github.com/ChrisVeigl/ESP8266Audio/blob/1ce70dcd14a70cfe5ebe899e761cfc4f9e3ec278/src/AudioOutputI2S.cpp#L317

Maybe this helps. Please keep in mind that my proposal is more a "dirty hack" than a well-engineered and sound solution - because a bit more thinking may be needed to really solve the problem instead of curing symptoms.

@ChrisVeigl
Copy link
Author

not sure if i2s_zero_dma_buffer() can be made into filling the buffers with anything different than 0 - it's part of the ESP-IDF, so we can't change it.

we could try to copy the function
https://github.com/espressif/esp-idf/blob/df1f9ec26e81138494462451889e7cb05dfc4ad4/components/driver/i2s.c#L889

into our source file and replace 0 by 32768 - just to test if this is a promising strategy.

regarding the other recommendations: well - these changes made it work for me in the first place - so i'd like to keep them ;-)
but it would make sense to apply them only for the external DAC...

it would be nice to cooperate for testing different audio setups!

@softhack007
Copy link
Contributor

softhack007 commented Jan 11, 2022

regarding the other recommendations: well - these changes made it work for me in the first place - so i'd like to keep them ;-)
but it would make sense to apply them only for the external DAC...

Actually I'm wondering why i2s_set_dac_mode() has an effect on external DAC at all - maybe there is some "cross-talk" because they are using the same pin? The docs from expressif seem to state that this function is for "internal DAC" only:
https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/peripherals/i2s.html?highlight=i2s_dac_mode_t#_CPPv414i2s_dac_mode_t

Did you also try i2s_start() and i2s_stop(), instead of i2s_set_dac_mode() ?

it would be nice to cooperate for testing different audio setups!

I'm here :-)
In the next few days i'll get a neat external PCM5102 based DAC, which will be connected to my nice little PAM8406 amp. So I could check what happens on the external DAC, compared to internal DAC.

cheers,
Frank.

@softhack007
Copy link
Contributor

I'm here :-)

my proposal: I could open a new branch in my fork of ESP8266Audio, so we can work together on a PR until it works for both of us. Then I make an "official PR" so that @earlephilhower can review it, and decide if he want to merge it into master. What do you think?

Just give me a few days, this week will get very busy at my job..

@ChrisVeigl
Copy link
Author

ChrisVeigl commented Jan 13, 2022

I have now a setup available which uses the internal DAC (an ODroid-Go device, see: https://github.com/hardkernel/ODROID-GO/blob/master/Documents/ODROID-GO_REV0.1_20180518.pdf)

I can confirm that my patch creates noise in this setup; the patched code should only execute if an external DAC is used. I also agree that it is strange that calling i2s_set_dac_mode() clears the problem when an external DAC is used...

i do not hear clicks/pops when i use the original i2c.cpp file from the master branch, when using the internal DAC.
so the question is why @Koxx3 got these problems in first place - and if others also experience this problem.

anyhow: the audio quality is quite bad and the sound stops during playback (especially at high amplitudes, I have to apply a gain of 0.5 in order to get continuous playback of a demo wav....) - seems to be another issue...?

@softhack007
Copy link
Contributor

softhack007 commented Feb 10, 2022

Hi @ChrisVeigl,

Sorry was "drowning in other business" for some time, now I think I'll find some time over the weekend to continue looking into this topic. I'll check what happens on my two ESP32 boards - one with internal and the other with external I2S dac.

@nauen
Copy link

nauen commented Apr 14, 2022

Dear all,

I'm testing the ESP8266Audio Lib with ESP12 and noDAC option.

@ChrisVeigl I don't understand how you are able to prevent the plop sound with the original EXP8266Audio Lib and i2c.cpp from master branch. ;) When I'm using the original one, the plop sound is very loud.
With using your patched version, I still have a plop but less loudness.

Which versions of the libraries you use?

I figured as well, if you play a sound with silence directly after another file, there is no popping sound. BUT There ist some higher noise instead. Could be that this noise is there as well, when some file is played but not noticable.

ESP12AudioTest.ino.zip

@murarduino
Copy link

murarduino commented Dec 9, 2022

Hi ! I applied the patch, but I still have issues : image

image

any idea why the signal doesn't remains at 1.66V when no more sound to output ?

Hi ! I applied the patch, but I still have issues : image

image

any idea why the signal doesn't remains at 1.66V when no more sound to output ?

AudioOutputI2S(0,1) used INTERNAL_DAC.
Use AudioOutputI2S()--"EXTERNAL_I2S,try Again.

@ktownsend-personal
Copy link

ktownsend-personal commented Aug 27, 2023

I found the conversation in this issue helpful in taming my pops with the internal DAC. Some of the things mentioned got me looking in the right places and the scope picture helped me realize what was happening.

I haven't seen this mentioned so I figured I would describe what solved it for me. The i2s_config.tx_desc_auto_clear setting of true was causing my popping between playbacks, and I see in your source code you have it set to true. That feature is nice in principle, as it detects that you're not feeding samples and it automatically stuffs the buffer to ensure silence. Unfortunately, it's stuffing the buffer with the minimum value rather than the mid-level value and the sharp transition makes the popping sound. I turned that setting off and my code stuffs the buffer with the mid-level value. This ensures the audio idles at the mid-level indefinitely until I start feeding samples again.

I still get a startup pop when calling i2s_driver_install, which is caused by it suddenly pulling the floating output down to min-value, but I am able to prevent a secondary pop by ramping up to the mid-value right before I start feeding samples. As I mentioned in previous paragraph, once I've got control of the buffer I'm able to keep it at mid-level when idle by stuffing the buffer when I'm done feeding samples. In my case I never uninstall the driver, so that initial startup pop is only one time when the ESP32 boots. It's not critical in my scenario because the speaker is not usually connected at boot (I'm making a telephone simulator), but the perfectionist in me still wants to control that startup pop.

It's worth mentioning that you can use that ramp approach before and after each playback if you want to keep that i2s_config.tx_desc_auto_clear enabled. I was doing that for a while successfully, but I ran in to a situation with an audio stitching feature where the ramps were adding too much delay between stitched segments. Disabling the feature and stuffing the correct silence value into the buffer avoided that problem more elegantly for me.

@tylercamp
Copy link

tylercamp commented Sep 27, 2023

I found that it also helped to add if (this->hertz == hz) return true; to the beginning of AudioOutputI2S::SetRate, avoiding the i2s_set_sample_rates call for each .begin()

I've used a combination of removing i2s_zero_dma_buffer, removing i2sOn = false in stop(), and the check mentioned before, to (finally) get seamless audio looping with my external DAC (MAX98357). Effectively, no i2s_* calls are made (other than i2s_write) during continuous/looped audio playback.

Thanks for the tips everyone!

@earlephilhower
Copy link
Owner

@tylercamp why not do a PR for your suggestion?

@fizicko
Copy link

fizicko commented Oct 11, 2023

I need to smoothly change the playback frequency to simulate an engine. But I realized that any change using i2s_set_sample_rates makes clicks in the wave.

Is there any other way to change the frequency of the sound?

Thanks for your library!

drtrigon added a commit to drtrigon/singsang that referenced this issue Dec 15, 2023
           ! issue bug report upstream
             * first playback after init of class (call of constructor)
             * may be similar like earlephilhower/ESP8266Audio#406
           ! may be check whether this is hardware dependend; may be just internal speaker affected and line out of RCA module not
           ! may be check wheteher use mic at same time as playback, could avoid i2s re-config and thus clic-noise
             * see schreibfaul1/ESP32-audioI2S#604
             * problem is there is i2s config we don't have control of
           ! may be use i2s_write(...) with a zero buffer instead of connect, loop and stop playing
         * prevent clic-noise in beginning of recorded file by skipping some data
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

No branches or pull requests