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
1.98k stars 432 forks source link

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

Open ChrisVeigl opened 3 years ago

ChrisVeigl commented 3 years ago

hi,

i read several issues about the click/pop noise when playing short sounds on the ESP32 via AudioOutputI2S, including https://github.com/earlephilhower/ESP8266Audio/issues/68 https://github.com/earlephilhower/ESP8266Audio/issues/230 https://github.com/earlephilhower/ESP8266Audio/issues/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 https://github.com/earlephilhower/ESP8266Audio/blob/6ed2604015ac611806e5ea18007f44082113021d/src/AudioOutputI2S.cpp#L207 and https://github.com/earlephilhower/ESP8266Audio/blob/6ed2604015ac611806e5ea18007f44082113021d/src/AudioOutputI2S.cpp#L305

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 commented 3 years ago

hi,

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

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

here the commit, please check for possible side-effects: https://github.com/ChrisVeigl/ESP8266Audio/commit/1ce70dcd14a70cfe5ebe899e761cfc4f9e3ec278

darkbull commented 2 years ago

@ChrisVeigl Good job, your code works.

ChrisVeigl commented 2 years ago

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

mazWaz commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

yes, a nasty plop noise 😅

ChrisVeigl commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

softhack007 commented 2 years ago

:thinking: 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:

b) internal DAC (aka analogue output pin)

@earlephilhower : what do you think?

ChrisVeigl commented 2 years ago

i might add that the patch ChrisVeigl@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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 commented 10 months ago

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 commented 9 months ago

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 commented 9 months ago

@tylercamp why not do a PR for your suggestion?

fizicko commented 9 months ago

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!